BREAKING: implement Native OIDC as per MSC 3861 #2024

TheOneWithTheBraid · 2025-02-07T18:21:24Z

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:

initial viable implementation
migrate the < polycule > implementation to this OIDC stack
excessive testing
figure out a test concept

Implements:

TheOneWithTheBraid · 2025-02-07T19:04:16Z

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.

TheOneWithTheBraid · 2025-02-07T22:29:09Z

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 · 2025-02-09T13:30:52Z

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 · 2025-02-10T10:11:14Z

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.

TheOneWithTheBraid · 2025-02-11T15:33:41Z

Clients might require the following workaround for #2027 : #2027 (comment)

krille-chan · 2025-05-09T07:05:06Z

lib/encryption/utils/bootstrap.dart

+        // and token refresh trouble
        for (final entry in secretsToStore.entries) {
-          storeFutures.add(newSsssKey!.store(entry.key, entry.value));
+          await newSsssKey!.store(entry.key, entry.value);


Is this related?

Yes, the token rotation might otherwise happen during the requests as mentioned in the comment in the line above.

I don't get it. Can you please explain?

TheOneWithTheBraid

Is there any update on review of this PR ? It's now been months and months of waiting and I still did not even get a response on whether you are actually willing to ever accept this contribution. Instead only vaguely related comments.

TheOneWithTheBraid · 2025-05-15T11:23:03Z

lib/encryption/utils/bootstrap.dart

+        // and token refresh trouble
        for (final entry in secretsToStore.entries) {
-          storeFutures.add(newSsssKey!.store(entry.key, entry.value));
+          await newSsssKey!.store(entry.key, entry.value);


Yes, the token rotation might otherwise happen during the requests as mentioned in the comment in the line above.

td-famedly · 2025-05-19T16:01:39Z

helo :3 sorry for the delay. I will now start reading the msc(s) and try to get back to this asap!

(still expecting a marklin set on my bday!)

edit:

It's now been months and months of waiting and I still did not even get a response on whether you are actually willing to ever accept this contribution. Instead only vaguely related comments.

Sorry for the delay, everyone internally has been super busy with "work". I have not heard anything about not accepting this contribution so I will review it with the end goal of getting it merged asap! Again thank you very much for the contribution and sorry for the delay!

td-famedly

very draft review, will try to go through the whole flow and look at the client implementation after this

lib/src/database/matrix_sdk_database.dart

lib/src/client.dart

td-famedly · 2025-05-20T11:38:51Z

test/database_api_test.dart

          'name',
          'homeserverUrl',
          'token',
          now,


would it be possible to add tests for this?

lib/src/client.dart

td-famedly · 2025-05-20T12:21:39Z

...c_extensions/msc_3861_native_oidc/msc4191_account_management/msc4191_account_management.dart

@@ -0,0 +1,44 @@
+import 'package:matrix/matrix.dart';
+
+extension Msc4191AccountManagementExtension on Client {


only a little bit concerned that this is a draft msc

Yeah, it's a draft but a) it doesn't break anything and b) it's so much of a simple MSC, I don't see much regression in it - even if we should happen to adjust the behavior later in time.

..._oidc/msc2966_oidc_dynamic_client_registration/msc2966_oidc_dynamic_client_registration.dart

...msc_extensions/msc_3861_native_oidc/msc2964_oidc_oauth_grants/msc2964_oidc_oauth_grants.dart

lib/src/database/database_api.dart

.../msc_3861_native_oidc/msc1597_matrix_identifier_syntax/msc1597_matrix_identifier_syntax.dart

krille-chan · 2025-07-14T12:12:48Z

...msc_extensions/msc_3861_native_oidc/msc2964_oidc_oauth_grants/msc2964_oidc_oauth_grants.dart

+import 'package:matrix/src/utils/crypto/crypto.dart';
+
+extension OidcOauthGrantFlowExtension on Client {
+  Future<void> oidcAuthorizationGrantFlow({


The method name says nothing about what it does? No idea what this is flow. Please switch this to a proper name with dart docs

I'm still not happy with this method name. A flow is a thing, not an action but a method is an action so it would better to be more intuitive to use a verb here like: loginWithOidcAuthGrantFlow()

It's not called oidcLoginAuthorizationGrantFlow. All methods related to OIDC use this prefix and I'd avoid having a gap word such as with in a method name.

Maybe the major confusion comes from the work login : Using OIDC you do not "log in" using the matrix dart SDK but you login to your browser and use a token to authenticate. This process is simply not a login. That's why I prefer the technically accurate term oidcAuthorizationGrantFlow.

lib/src/client.dart

krille-chan

Looks quite good so far, though I'm not that into the MSCs of Matrix. Will test out if it works with oidc login on FluffyChat soon...

krille-chan

Can you add an example how to actually use this?

Signed-off-by: The one with the braid <[email protected]>

…i to client Signed-off-by: The one with the braid <[email protected]>

Signed-off-by: The one with the braid <[email protected]>

krille-chan · 2025-08-12T09:29:20Z

...msc_extensions/msc_3861_native_oidc/msc2964_oidc_oauth_grants/msc2964_oidc_oauth_grants.dart

+  /// - [prompt]: the OAuth2.0 authorization prompt. This must be a valid prompt from the [getOidcDiscoveryInformation] as by https://openid.net/specs/openid-connect-prompt-create-1_0.html
+  Future<void> oidcLoginAuthorizationGrantFlow({
+    required Completer<OidcCallbackResponse> nativeCompleter,
+    required String oidcClientId,


If oidc is already part of the method name and there is no other clientId we can IMO just name this clientId. afaik other APIs using OIDC treat it the same

Sure, renamed !

krille-chan

This now contains 22 commits. Can you clean up the commit history please?

krille-chan · 2025-08-12T09:35:30Z

lib/src/database/database_api.dart

+
+  Future<Map<String, Object?>?> getOidcAuthMetadata();
+
+  Future<void> storeOidcDynamicClientId(String? oidcClientId);


I'm not sure if this is the right approach to create methods for every single thing we store in the Client Database. This is now already messed up as we use getClient and updateClient to update single fields but now for other single fields we have methods. What about just adding one generic method getClientData(ClientData key) and putClientData(ClientData key, value) where ClientData is an enum of all possible fields

I migrated the String based key-value access on the client box contents to your proposed enum-based approach. I'd suggest to further extend this to a type safe approach including the values of the box but this is way beyond the scope of this OIDC-related PR.

Signed-off-by: The one with the braid <[email protected]>

TheOneWithTheBraid · 2025-08-12T14:23:45Z

This now contains 22 commits. Can you clean up the commit history please?

You can squash the commits as soon as you want to merge this PR. It does not make sense to rewrite the commit history before review is done. Squashing the commits is a task for the person merging this PR at the end.

krille-chan · 2025-10-01T14:21:18Z

lib/msc_extensions/msc_3861_native_oidc/README.md

+  );
+
+  // You can e.g. use `package:app_links` to forward the called deep link into this completer
+  final nativeCompleter = Completer<OidcCallbackResponse>();


Seeing this I've no idea how to use it with for example flutter_web_auth(2) package. In the end I get a URL I need to open where I pass a redirect_uri and receive a token back, right? How is this done here?

Is a Completer really the best choice? Would it be possible to use a callback instead? Something like String Function(Uri uri) onAuth which we then set like this:

await client.oidcAuthorizationGrantFlow( // ... onAuth: (Uri uri) async { final token = await FlutterWebAuth2.authenticate(uri); return token; } );

Another question: In browser apps it might be desired to open it in the same tab which in a flutter context would mean that the app unloads and then starts again. So we need to store information somewhere and continue with the received token later. Is this also possible here?

krille-chan · 2025-10-01T14:23:38Z

This now contains 22 commits. Can you clean up the commit history please?

You can squash the commits as soon as you want to merge this PR. It does not make sense to rewrite the commit history before review is done. Squashing the commits is a task for the person merging this PR at the end.

Not sure. A commit message should always explain all changes and why. Just a squashed feat: Implement OIDC wouldn't explain the changes in the database for example. Having this split up into multiple commits which can be reviewed one after another would be preferred. Something like:

refactor: Use typed enums for database client access
Implement MSC1, MSC2, MSC3
Implement MSC4, MSC5

and so on. Preferrable with commit messages which explain a little bit more.

krille-chan · 2025-10-01T14:23:55Z

I'm not yet familiar with the OIDC MSCs in Matrix so this would need a review from someone else

TheOneWithTheBraid marked this pull request as ready for review February 9, 2025 11:56

TheOneWithTheBraid requested a review from a team as a code owner February 9, 2025 11:56

TheOneWithTheBraid force-pushed the braid/msc3861-native-oidc branch 3 times, most recently from ff732a4 to 83f4ec1 Compare February 11, 2025 15:33

TheOneWithTheBraid mentioned this pull request Feb 13, 2025

feat: add refresh token handling http client #2033

Open

TheOneWithTheBraid force-pushed the braid/msc3861-native-oidc branch from 261932d to 0ae4e30 Compare February 15, 2025 12:31

TheOneWithTheBraid force-pushed the braid/msc3861-native-oidc branch 2 times, most recently from 55bd288 to 2732c27 Compare March 13, 2025 07:03

TheOneWithTheBraid force-pushed the braid/msc3861-native-oidc branch from 2732c27 to c483116 Compare April 2, 2025 08:17

krille-chan reviewed May 9, 2025

View reviewed changes

TheOneWithTheBraid commented May 15, 2025

View reviewed changes

TheOneWithTheBraid force-pushed the braid/msc3861-native-oidc branch from c483116 to 895bb82 Compare May 15, 2025 11:34

td-famedly self-requested a review May 19, 2025 16:00

TheOneWithTheBraid force-pushed the braid/msc3861-native-oidc branch from 9ad6df5 to 0c90f2b Compare May 20, 2025 08:54

td-famedly reviewed May 20, 2025

View reviewed changes

TheOneWithTheBraid force-pushed the braid/msc3861-native-oidc branch from f287859 to 48bb3b3 Compare June 12, 2025 07:37

krille-chan reviewed Jul 14, 2025

View reviewed changes

lib/src/database/database_api.dart Outdated Show resolved Hide resolved

krille-chan reviewed Jul 14, 2025

View reviewed changes

.../msc_3861_native_oidc/msc1597_matrix_identifier_syntax/msc1597_matrix_identifier_syntax.dart Outdated Show resolved Hide resolved

krille-chan reviewed Jul 14, 2025

View reviewed changes

.../msc_3861_native_oidc/msc1597_matrix_identifier_syntax/msc1597_matrix_identifier_syntax.dart Show resolved Hide resolved

krille-chan reviewed Jul 14, 2025

View reviewed changes

lib/src/client.dart Outdated Show resolved Hide resolved

krille-chan requested changes Jul 14, 2025

View reviewed changes

TheOneWithTheBraid added 21 commits August 12, 2025 11:03

fix: wrong response_type parameter

5939954

Signed-off-by: The one with the braid <[email protected]>

feat: implement a callback to hand over the OAuth2.0 authorization ur…

390c254

…i to client Signed-off-by: The one with the braid <[email protected]>

feat: add parameter to enforce new dynamic client registration

e3cdc0e

Signed-off-by: The one with the braid <[email protected]>

fix: still fetch auth metadata when well known is not served

bc446fc

Signed-off-by: The one with the braid <[email protected]>

feat: improve documentation

fbf3b1c

Signed-off-by: The one with the braid <[email protected]>

fix: include scope into oauth2 redirect

d070d13

Signed-off-by: The one with the braid <[email protected]>

fix: revert unwanted change in Client

acd5f46

Signed-off-by: The one with the braid <[email protected]>

fix: properly calculate PKCE for OAuth2 token exchange

c6b0cb2

Signed-off-by: The one with the braid <[email protected]>

fix: correctly assign request form body for OAuth2 token requests

d27dc1d

Signed-off-by: The one with the braid <[email protected]>

fix: migration of device ID to individual database entry

852e8c8

Signed-off-by: The one with the braid <[email protected]>

chore: better parse redirect uri query

823252b

Signed-off-by: The one with the braid <[email protected]>

feat: support all possible methods for OIDC discovery

d1f1985

Signed-off-by: The one with the braid <[email protected]>

fix: return .well-known after checking OIDC capabilities

d254546

Signed-off-by: The one with the braid <[email protected]>

feat: separate OIDC discovery from .well-known

be69f2c

Signed-off-by: The one with the braid <[email protected]>

fix: avoid rate limits in Bootstrap.askSetupCrossSigning

51ef282

Signed-off-by: The one with the braid <[email protected]>

fix: ensure to store device IDs for legacy login

116d3d2

Signed-off-by: The one with the braid <[email protected]>

fix: non-nullable database in Client.init

786f658

Signed-off-by: The one with the braid <[email protected]>

fix: clean up bad version response

c9f5a73

Signed-off-by: The one with the braid <[email protected]>

fix: PR review

ff04de0

Signed-off-by: The one with the braid <[email protected]>

fix: do not wait for auth metadata present

34ab897

Signed-off-by: The one with the braid <[email protected]>

feat: add example

8ab97e0

Signed-off-by: The one with the braid <[email protected]>

TheOneWithTheBraid force-pushed the braid/msc3861-native-oidc branch from cff4fd8 to 8ab97e0 Compare August 12, 2025 09:04

krille-chan reviewed Aug 12, 2025

View reviewed changes

krille-chan requested changes Aug 12, 2025

View reviewed changes

krille-chan reviewed Aug 12, 2025

View reviewed changes

chore: migrate client box keys to enum

82ad905

Signed-off-by: The one with the braid <[email protected]>

krille-chan reviewed Oct 1, 2025

View reviewed changes

		@@ -0,0 +1,44 @@
		import 'package:matrix/matrix.dart';

		extension Msc4191AccountManagementExtension on Client {


		Future<Map<String, Object?>?> getOidcAuthMetadata();

		Future<void> storeOidcDynamicClientId(String? oidcClientId);

BREAKING: implement Native OIDC as per MSC 3861 #2024

Are you sure you want to change the base?

BREAKING: implement Native OIDC as per MSC 3861 #2024

Uh oh!

Conversation

TheOneWithTheBraid commented Feb 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheOneWithTheBraid commented Feb 7, 2025

Uh oh!

TheOneWithTheBraid commented Feb 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheOneWithTheBraid commented Feb 9, 2025

Uh oh!

TheOneWithTheBraid commented Feb 10, 2025

Uh oh!

TheOneWithTheBraid commented Feb 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheOneWithTheBraid left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

td-famedly commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

td-famedly left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

krille-chan left a comment

Choose a reason for hiding this comment

Uh oh!

krille-chan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krille-chan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheOneWithTheBraid commented Aug 12, 2025

TheOneWithTheBraid commented Feb 7, 2025 •

edited

Loading

TheOneWithTheBraid commented Feb 7, 2025 •

edited

Loading

td-famedly commented May 19, 2025 •

edited

Loading