Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add SAML integration #19

Merged
merged 44 commits into from
Aug 30, 2023
Merged

Conversation

amandahla
Copy link
Collaborator

@amandahla amandahla commented Aug 11, 2023

Tested with the change merged on this PR canonical/saml-integrator-operator#37

This PR adds SAML integration by using the SAML Integrator charm library.

What happens after merging it:

  • Refactor of database observer.
  • Add integration with SAML integrator so the user can enable SAML authentication.
  • Add new type SAMLConfiguration.
  • Add new observer SAMLObserver.
  • Add new configuration public_baseurl.
  • Add new container Synapse NGINX as a reverse proxy.
  • Add unit and integration tests.

@amandahla amandahla requested a review from a team as a code owner August 11, 2023 20:25
@amandahla amandahla marked this pull request as draft August 11, 2023 20:25
@amandahla amandahla changed the title WIP - Add SAML integration Add SAML integration Aug 16, 2023
@amandahla amandahla marked this pull request as ready for review August 16, 2023 18:20
arturo-seijas
arturo-seijas previously approved these changes Aug 17, 2023
Copy link
Collaborator

@arturo-seijas arturo-seijas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

src/pebble.py Show resolved Hide resolved
src/saml_observer.py Outdated Show resolved Hide resolved
tests/integration/conftest.py Outdated Show resolved Hide resolved
tests/integration/test_charm.py Outdated Show resolved Hide resolved
tests/unit/conftest.py Show resolved Hide resolved
tests/unit/test_charm.py Show resolved Hide resolved
tests/unit/test_synapse_workload.py Outdated Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

license-eye has totally checked 69 files.

Valid Invalid Ignored Fixed
38 1 30 0
Click to see the invalid file list
  • synapse_rock/attributemaps/u1staging.py

synapse_rock/attributemaps/u1staging.py Show resolved Hide resolved
arturo-seijas
arturo-seijas previously approved these changes Aug 28, 2023
Copy link
Collaborator

@arturo-seijas arturo-seijas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

yanksyoon
yanksyoon previously approved these changes Aug 29, 2023
Copy link
Contributor

@yanksyoon yanksyoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small questions, 🎉 LGTM!

metadata.yaml Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/synapse/workload.py Show resolved Hide resolved
synapse_rock/attributemaps/u1staging.py Outdated Show resolved Hide resolved
weiiwang01
weiiwang01 previously approved these changes Aug 29, 2023
@github-actions
Copy link
Contributor

Test coverage for 84f761e

Name                            Stmts   Miss Branch BrPart  Cover   Missing
---------------------------------------------------------------------------
src/actions/__init__.py             3      0      0      0   100%
src/actions/register_user.py       20      0      2      0   100%
src/actions/reset_instance.py      21      3      2      1    83%   54-58
src/charm.py                      110      8     12      1    93%   80-83, 109-110, 170-171
src/charm_state.py                 53      2     10      2    94%   22, 82
src/charm_types.py                 11      0      0      0   100%
src/constants.py                   14      0      0      0   100%
src/database_client.py             53      1     10      3    94%   35, 47->exit, 69->exit
src/database_observer.py           54      4      6      0    93%   70-72, 88
src/exceptions.py                   4      0      0      0   100%
src/observability.py                9      0      0      0   100%
src/pebble.py                      65      4      2      1    93%   87-88, 106-107
src/saml_observer.py               45      1      8      0    98%   64
src/synapse/__init__.py             3      0      0      0   100%
src/synapse/api.py                 83      2     10      1    97%   138-139
src/synapse/workload.py           126      4     18      4    94%   183, 208->211, 227->230, 266, 397-402
src/user.py                        23      0      4      0   100%
---------------------------------------------------------------------------
TOTAL                             697     29     84     13    95%

Static code analysis report

Run started:2023-08-29 13:27:58.597718

Test results:
  No issues identified.

Code scanned:
  Total lines of code: 3041
  Total lines skipped (#nosec): 1
  Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
  Total issues (by severity):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
  Total issues (by confidence):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
Files skipped (0):

@amandahla amandahla merged commit eea3963 into main Aug 30, 2023
20 checks passed
@amandahla amandahla deleted the ISD-797-synapse-add-saml-configuration branch August 30, 2023 11:28
@Thanhphan1147 Thanhphan1147 mentioned this pull request Sep 13, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants