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

BREAKING: implement Native OIDC as per MSC 3861 #2024

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

TheOneWithTheBraid
Copy link
Contributor

@TheOneWithTheBraid TheOneWithTheBraid commented Feb 7, 2025

BREAKING: changes to the database API to better reflect the OIDC state

This implementation is based on the draft of native OIDC I implemented in < polycule >. It can be found here. Unlike in my initial draft, this implementation of course does not rely on any Flutter package but ships a minimal OIDC state machine relying on the matrix client to provide the native callback result via a Completer.

This implementation does not aim to comply to the full OIDC standard but only the subset required as per the MSCs stated below.

This PR does not aim to implement OIDC device flow as per MSC 4108 nor the changes for the application services as per MSC 4190.

Roadmap:

Implements:

  • MSC 3861 - Next-generation auth for Matrix, based on OAuth 2.0/OIDC
    • MSC 1597 - Better spec for matrix identifiers
    • MSC 2964 - Usage of OAuth 2.0 authorization code grant and refresh token grant
    • MSC 2965 - OAuth 2.0 Authorization Server Metadata discovery
    • MSC 2966 - Usage of OAuth 2.0 Dynamic Client Registration in Matrix
    • MSC 2967 - API scopes
    • MSC 3824 - OIDC aware clients
    • MSC 4108 - Mechanism to allow OIDC sign in and E2EE set up via QR code - not planned for this PR
    • MSC 4190 - Device management for application services - not planned for this PR
    • MSC 4191 - Account management deep-linking

Implements:

- MSC 3861 - Next-generation auth for Matrix, based on OAuth 2.0/OIDC
  - MSC 1597 - Better spec for matrix identifiers
  - MSC 2964 - Usage of OAuth 2.0 authorization code grant and refresh token grant
  - MSC 2965 - OAuth 2.0 Authorization Server Metadata discovery
  - MSC 2966 - Usage of OAuth 2.0 Dynamic Client Registration in Matrix
  - MSC 2967 - API scopes
  - MSC 3824 - OIDC aware clients
  - MSC 4191 - Account management deep-linking

Signed-off-by: The one with the braid <[email protected]>
@TheOneWithTheBraid
Copy link
Contributor Author

A comment on the changes to the database API:

I initially intended to just keep all OIDC-related state (device ID, OAuth2.0 client ID, auth metadata) in memory until the OIDC flow completes. I sadly figured out the Dart process might get killed by the platform for battery optimization. In order to complete the flow after the main process was killed, I decided to store the OAuth2 client ID and the homeserver's auth metadata in the database as well as to split the device ID from the remaining client store since now the device ID is present a long time before the rest of the account is available.

Even though I so far already implemented the database API changes with this in mind, there is no way to resume an OIDC flow from a new thread yet.

Signed-off-by: The one with the braid <[email protected]>
@TheOneWithTheBraid
Copy link
Contributor Author

TheOneWithTheBraid commented Feb 7, 2025

Outdated review hint

Review notice: So far, the default behavior of the updated Client.getWellknown is to only check the brand new /auth_metadata endpoint. This might require Synapse nightly builds to work. Client developers however are free to use the previous revision of the MSC suggesting to use the /auth_issuer endpoint or the .well-known entry containing the org.matrix.msc2965.authentication field. All three possible implementations are supported in this PR. Since the default is definitely going to be the new /auth_metadata, the implementation already suggests this at this point.

EDIT: I adjusted the behavior of Client.getWellknown to cope with all three permitted approaches of OIDC discovery. The implementation will therefore work with both stable Synapse releases as well as cutting edge Synapse builds already using the newest approach.

You can now easily use e.g. synapse-oidc.element.dev for testing.

@TheOneWithTheBraid TheOneWithTheBraid marked this pull request as ready for review February 9, 2025 11:56
@TheOneWithTheBraid TheOneWithTheBraid requested a review from a team as a code owner February 9, 2025 11:56
@TheOneWithTheBraid
Copy link
Contributor Author

The < polycule > side implementation is now merged and works like a charm. Maybe that's helpful for testing this PR in real world: https://gitlab.com/polycule_client/polycule/-/merge_requests/245

@TheOneWithTheBraid
Copy link
Contributor Author

Since I unaskedly opened this PR, I'd kindly offer to take care of the OIDC code's maintenance in future too. You folks all have my MXID and can always poke me if there are future issues about the OIDC code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant