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

ISD-2761 Add oauth lib and requirer #621

Draft
wants to merge 72 commits into
base: 2/main
Choose a base branch
from

Conversation

Thanhphan1147
Copy link
Collaborator

Overview

Rationale

Juju Events Changes

Module Changes

Library Changes

Checklist

@Thanhphan1147 Thanhphan1147 force-pushed the add_oauth_lib_and_requirer branch from f14b722 to 74248fc Compare December 13, 2024 15:31
requirements.txt Outdated
requests ==2.32.3
python-ulid ==3.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep alphabetic ordering

Comment on lines +116 to +118
self.framework.observe(self._oauth.on.oauth_info_changed, self._on_config_changed)
self.framework.observe(self._oauth.on.oauth_info_removed, self._on_config_changed)
self.framework.observe(self._oauth.on.invalid_client_config, self._on_config_changed)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rename the hook method to something more "generic"

@@ -401,6 +419,11 @@ def reconcile( # noqa: C901 pylint: disable=too-many-branches,too-many-statemen
ignore_order=True,
ignore_string_case=True,
)

restart_mas(container, rendered_mas_configuration)
Copy link
Contributor

Choose a reason for hiding this comment

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

To be set before restart_syname with a comment explaning it must be done before synapse

src/pebble.py Outdated

restart_mas(container, rendered_mas_configuration)
# Activate MAS on synapse
synapse.configure_mas(current_synapse_config, synapse_msc3861_configuration)
Copy link
Contributor

Choose a reason for hiding this comment

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

Move it before the DeepDiff to allow synapse to restart when the config has changed

Copy link
Contributor

Lint checks failed for 3c0236b

```

--- /home/runner/work/synapse-operator/synapse-operator/src/charm.py 2024-12-13 20:47:48.483328+00:00
+++ /home/runner/work/synapse-operator/synapse-operator/src/charm.py 2024-12-13 20:48:12.998148+00:00
@@ -224,17 +224,17 @@
self.model.unit.status = ops.MaintenanceStatus("Configuring Synapse")

     oauth_client_config = generate_oauth_client_config(
         mas_configuration, charm_state.synapse_config
     )
  •    logger.info('Generated oauth client config: %s', oauth_client_config)
    
  •    logger.info("Generated oauth client config: %s", oauth_client_config)
       self._oauth.update_client_config(oauth_client_config)
       oauth_provider_info = None
       if self._oauth.is_client_created():
           oauth_provider_info = self._oauth.get_provider_info()
    
  •    logger.info('IS client created: %s', self._oauth.is_client_created())
    
  •    logger.info("IS client created: %s", self._oauth.is_client_created())
    
       rendered_mas_configuration = generate_mas_config(
           mas_configuration,
           charm_state.synapse_config,
           oauth_provider_info,
    

--- /home/runner/work/synapse-operator/synapse-operator/tests/unit/conftest.py 2024-12-13 20:47:48.487328+00:00
+++ /home/runner/work/synapse-operator/synapse-operator/tests/unit/conftest.py 2024-12-13 20:48:13.160731+00:00
@@ -122,13 +122,11 @@
monkeypatch.setattr(time, "sleep", lambda *_args, **_kwargs: "")
# Assume that MAS is working properly
monkeypatch.setattr(
"state.mas.MASConfiguration.from_charm", MagicMock(return_value=MagicMock())
)

  • monkeypatch.setattr(
  •    "charm.generate_oauth_client_config", MagicMock(return_value=None)
    
  • )
  • monkeypatch.setattr("charm.generate_oauth_client_config", MagicMock(return_value=None))
    monkeypatch.setattr("pebble._push_mas_config", MagicMock())
    monkeypatch.setattr("charm.generate_mas_config", MagicMock(return_value=""))
    monkeypatch.setattr("charm.generate_synapse_msc3861_config", MagicMock(return_value={}))

    harness = Harness(SynapseCharm)

Copy link
Contributor

Test coverage for 3c0236b

Name                                    Stmts   Miss Branch BrPart  Cover   Missing
-----------------------------------------------------------------------------------
src/admin_access_token.py                   9      0      0      0   100%
src/auth/__init__.py                        0      0      0      0   100%
src/auth/mas.py                            68      8      2      1    87%   71-73, 117->119, 122-124, 260-264
src/backup.py                             175      5     20      2    96%   353-354, 423-424, 481->483, 484
src/backup_observer.py                    134     16     12      0    89%   132-135, 140-143, 179-182, 211-214
src/charm.py                              330     29     74     13    90%   154->156, 159, 233, 292, 296-297, 303-304, 325-326, 355, 362, 442-446, 449-450, 478-480, 500, 536-537, 544-546, 558-559, 566-568
src/charm_types.py                         30      0      0      0   100%
src/database_client.py                     57      1      8      4    92%   35, 47->exit, 69->exit, 88->98
src/database_observer.py                   49      4      2      0    92%   61-64
src/exceptions.py                           3      0      0      0   100%
src/matrix_auth_observer.py                68      8     12      3    86%   63, 66, 145, 159-163
src/media_observer.py                      45      4      2      1    89%   60-62, 81
src/mjolnir.py                            102      3     30      3    95%   85, 94->110, 115-119
src/observability.py                       14      0      0      0   100%
src/pebble.py                             217     29     44     12    84%   64->69, 183->exit, 194-198, 242-243, 263-264, 282-285, 344->349, 354-355, 367-368, 370-371, 402, 404, 406, 408, 410, 443, 495-500
src/redis_observer.py                      39      3      4      0    93%   63-66
src/s3_parameters.py                       22      0      4      0   100%
src/smtp_observer.py                       61      4     14      2    92%   82-86, 89, 108->113
src/state/__init__.py                       0      0      0      0   100%
src/state/charm_state.py                  128      9     32      7    90%   166, 170, 191, 216, 222, 228, 232-233, 316
src/state/mas.py                           73      7      4      2    88%   147, 153-154, 180-182, 197
src/state/validation.py                    36      3      2      0    92%   105-107
src/synapse/__init__.py                     4      0      0      0   100%
src/synapse/admin.py                       19      2      2      0    90%   40-41
src/synapse/api.py                        175      3     20      3    97%   176, 229, 402
src/synapse/workload.py                   138      6     24      0    94%   401-402, 412-415
src/synapse/workload_configuration.py     149     26     34     12    79%   90->exit, 94-95, 143-144, 173, 193-194, 226-227, 260, 269-270, 285, 290-291, 312-313, 332->337, 338, 356->358, 368-369, 397, 405->407, 407->409, 414-415, 435->442, 445, 465-466
src/user.py                                23      0      2      0   100%
-----------------------------------------------------------------------------------
TOTAL                                    2168    170    348     65    91%

Static code analysis report

Working... ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00
Run started:2024-12-13 20:48:50.348887

Test results:
>> Issue: [B105:hardcoded_password_string] Possible hardcoded password: 'client_secret_basic'
 Severity: Low   Confidence: Medium
 CWE: CWE-259 (https://cwe.mitre.org/data/definitions/259.html)
 More Info: https://bandit.readthedocs.io/en/1.8.0/plugins/b105_hardcoded_password_string.html
 Location: /home/runner/work/synapse-operator/synapse-operator/src/auth/mas.py:42:33
41	MAS_AUTHORIZATION_GRANT = ["authorization_code"]
42	MAS_TOKEN_ENDPOINT_AUTH_METHOD = "client_secret_basic"
43	MAS_OIDC_SCOPE = "openid profile email"

--------------------------------------------------

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

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

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.

2 participants