Skip to content

refactor(oauth): Allow to use any registration method with any login function#4827

Merged
Hywan merged 9 commits intomatrix-org:mainfrom
zecakeh:qr-login-registration-store
Mar 21, 2025
Merged

refactor(oauth): Allow to use any registration method with any login function#4827
Hywan merged 9 commits intomatrix-org:mainfrom
zecakeh:qr-login-registration-store

Conversation

@zecakeh
Copy link
Copy Markdown
Collaborator

@zecakeh zecakeh commented Mar 21, 2025

This creates a ClientRegistrationMethod enum and use it for OAuth::login and OAuth::login_with_qr_code, allowing notable to use the OAuthRegistrationStore every time.

We can get rid of url_for_oidc since everything can be done with OAuth::login now, and we also merge OAuth::finish_login and OAuth::login_with_oidc_callback() to use the simplest API.

Finally there are some optimizations to reduce the number of requests by reusing the AuthorizationServerMetadata.

This can be reviewed commit by commit.

zecakeh added 4 commits March 21, 2025 14:25
There is actually no way to get an error.

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
…istration

They are one- or two-liners and are only used once.

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
The name is more explicit about what the function does.

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
…code

Introduces the ClientRegistrationMethod type

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
@zecakeh zecakeh requested a review from a team as a code owner March 21, 2025 15:06
@zecakeh zecakeh requested review from Hywan and removed request for a team March 21, 2025 15:06
zecakeh added 5 commits March 21, 2025 16:08
Gets rid of OAuth::url_for_oidc since it can be replaced by a call to
OAuth::login now.

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
…inish_login()

Accept a URL or a query string for simplicity.

That way we don't need to expose AuthorizationResponse.

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Avoids repeated calls to the same endpoint in the same flow.

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
@zecakeh zecakeh force-pushed the qr-login-registration-store branch from 1f63fd4 to c6d0bae Compare March 21, 2025 15:16
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 21, 2025

Codecov Report

Attention: Patch coverage is 95.06173% with 4 lines in your changes missing coverage. Please review.

Project coverage is 86.50%. Comparing base (22cbce8) to head (c6d0bae).
Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
crates/matrix-sdk/src/authentication/oauth/mod.rs 94.44% 3 Missing ⚠️
...atrix-sdk/src/authentication/oauth/qrcode/login.rs 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4827      +/-   ##
==========================================
+ Coverage   86.47%   86.50%   +0.02%     
==========================================
  Files         297      297              
  Lines       34584    34600      +16     
==========================================
+ Hits        29907    29930      +23     
+ Misses       4677     4670       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@Hywan Hywan left a comment

Choose a reason for hiding this comment

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

More clean ups 👏! More clean ups 👏!

Thanks 🙇.

@Hywan Hywan merged commit 94f0bee into matrix-org:main Mar 21, 2025
43 checks passed
@zecakeh zecakeh deleted the qr-login-registration-store branch March 21, 2025 18:42
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.

2 participants