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-2817 Add mas to pebble plan #620

Open
wants to merge 102 commits into
base: 2/main
Choose a base branch
from
Open

Conversation

Thanhphan1147
Copy link
Collaborator

@Thanhphan1147 Thanhphan1147 commented Dec 5, 2024

This PR adds a pebble layer that starts the Matrix Authentication Service:

  • Adds a method to generate synapse's msc3861 config to activate MAS
  • Update nginx config template to route the correct traffic to MAS
  • Rework the register-user action and add a new verify-user-email action based on the mas-cli
  • Update tests

❗This PR also removes support for mjolnir. As it is currently incompatible with MAS.

Mjolnir relies on synapse's /_synapse/admin/v1/users/:user_id/admin API for most of its commands (ref: https://github.com/matrix-org/mjolnir/blob/a7f49b816022112a78ea35a2e30a06092e77c2eb/src/Mjolnir.ts#L528) which has been disabled in MAS as per this PR : matrix-org/synapse#15582

The following admin APIs are disabled:

/users/:user_id/admin
/users/:user_id/login
/registration_tokens
/registration_tokens/new
/registration_tokens/:token
/reset_password
/register

Checklist

src/auth/mas.py Outdated Show resolved Hide resolved
container: Synapse container.
"""
command = [MAS_EXECUTABLE_PATH, "config", "check", "-c", MAS_CONFIGURATION_PATH]
process = container.exec(command=command, working_dir=MAS_WORKING_DIR)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should wrap the pebble exec error into a business error, same comment for sync_mas_config

username,
]

process = container.exec(command=command, working_dir=MAS_WORKING_DIR, combine_stderr=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should wrap pebble exec error

@amandahla
Copy link
Collaborator

Considering a refresh/upgrade scenario where Mjolnir was enabled, should the charm delete the previously created management room (also the rate limit configuration or anything related to mjolnir)?

room_id = synapse.create_management_room(admin_access_token=admin_access_token)

@Thanhphan1147
Copy link
Collaborator Author

Thanhphan1147 commented Jan 9, 2025

Considering a refresh/upgrade scenario where Mjolnir was enabled, should the charm delete the previously created management room (also the rate limit configuration or anything related to mjolnir)?

room_id = synapse.create_management_room(admin_access_token=admin_access_token)

@amandahla We don't support automatic upgrade from older revisions to avoid the complexity related to migration of SAML users to OIDC in MAS (which involves modifying the external-id in synapse db, aligning the local subject with the upstream OIDC subject claim, having OIDC configured before user migration...). So any upgrades we do will be manual and there we just need to keep in mind to remove mjolnir-related user and rooms.

Copy link
Contributor

github-actions bot commented Jan 9, 2025

Test coverage for 2d02ad7

Name                                    Stmts   Miss Branch BrPart  Cover   Missing
-----------------------------------------------------------------------------------
src/auth/__init__.py                        0      0      0      0   100%
src/auth/mas.py                            64      0      2      1    98%   113->115
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                              273     18     64     10    92%   140->142, 145, 270, 274-275, 281-282, 303-304, 328, 335, 410-414, 417-418, 446-448, 468
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/observability.py                       14      0      0      0   100%
src/pebble.py                             194     23     44     11    86%   157->exit, 168-172, 216-217, 237-238, 256-259, 320->325, 330-331, 343-344, 346-347, 378, 380, 382, 384, 386, 417
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      1     14      2    96%   89, 108->113
src/state/__init__.py                       0      0      0      0   100%
src/state/charm_state.py                  127      9     32      7    90%   164, 168, 189, 214, 220, 226, 230-231, 314
src/state/mas.py                           78      8      6      3    87%   152, 158-159, 183-185, 200, 220
src/state/validation.py                    36      3      2      0    92%   108-110
src/synapse/__init__.py                     3      0      0      0   100%
src/synapse/api.py                        129      6     18      4    93%   111-112, 161, 172, 174, 314
src/synapse/workload.py                   118      4     24      0    94%   365-368
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                                    1891    139    306     60    91%

Static code analysis report

Working... ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00
Run started:2025-01-09 17:24:07.209773

Test results:
  No issues identified.

Code scanned:
  Total lines of code: 9705
  Total lines skipped (#nosec): 3
  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):


For more details and implementation guidance, refer to the [Mjolnir GitHub repository](https://github.com/matrix-org/mjolnir).

With the arrival of MAS mjolnir has been temporary disabled.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Regarding what we discussed about refresh, can you add a note here in the doc about manually removing what was created?

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.

3 participants