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.

Sign up for GitHub

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

Jump to bottom

MSC2965: OAuth 2.0 Authorization Server Metadata discovery #2965

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

MSC2965: OAuth 2.0 Authorization Server Metadata discovery #2965

wants to merge 40 commits into from

Conversation

sandhose
Copy link
Member

@sandhose sandhose commented Jan 14, 2021 •
edited
Loading

Rendered

Status:

  • Spec is feature complete
  • Reviewed for consistency with MSC3861
  • Implementations believed to be complete enough

Dependencies:

Clients and homeservers currently implement an older version of this proposal, and need to be updated:

SCT:

tickyboxes
checklist

@turt2live turt2live changed the title MSC2965: [WIP] OIDC Provider discovery [WIP] MSC2965: OIDC Provider discovery Jan 14, 2021
@turt2live turt2live marked this pull request as draft January 14, 2021 17:27
@turt2live turt2live added kind:feature MSC for not-core and not-maintenance stuff proposal A matrix spec change proposal labels Jan 14, 2021
@sandhose sandhose mentioned this pull request Jan 15, 2021
Open
9 tasks
@turt2live turt2live added the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Jun 8, 2021
This was referenced Jun 22, 2022
Merged
Closed
@hughns hughns mentioned this pull request Aug 4, 2022
Open
@erlend-sh
Copy link

erlend-sh commented Aug 9, 2022 •
edited
Loading

Keycloak in OIDC Playground

Are any other examples planned?

I’m using Ory for several apps that I’d like to also connect together with Matrix. It also strikes me as a conveniently lightweight example for Matrix, which also aligns well with Dendrite since it’s in Go.

@hughns hughns mentioned this pull request Aug 14, 2022
Open
@hughns
Copy link
Member

hughns commented Aug 14, 2022

@erlend-sh Good suggestion, thank you - I've added element-hq/oidc-playground#3 to track this.

@hughns hughns changed the title [WIP] MSC2965: OIDC Provider discovery MSC2965: OIDC Provider discovery Sep 22, 2022
@hughns hughns marked this pull request as ready for review September 22, 2022 16:08
@hughns hughns mentioned this pull request Jan 9, 2023
Merged
20 tasks
@turt2live turt2live added the matrix-2.0 Required for Matrix 2.0 label Mar 3, 2023
@turt2live
Copy link
Member

Implementation lgtm. Concerns are relatively minor as well:

@mscbot fcp merge

@mscbot
Copy link
Collaborator

mscbot commented Jan 22, 2025 •
edited by richvdh
Loading

Team member @mscbot has proposed to merge this. The next step is review by the rest of the tagged people:

Concerns:

  • Error codes for not using OIDC
  • Include rationale for Client-Server API endpoint over .well-known
  • Checklist is incomplete

Once at least 75% of reviewers approve (and there are no outstanding concerns), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for information about what commands tagged team members can give me.

@mscbot mscbot added proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. disposition-merge labels Jan 22, 2025
@turt2live turt2live removed the implementation-needs-checking The MSC has an implementation, but the SCT has not yet checked it. label Jan 22, 2025
@turt2live
Copy link
Member

@mscbot concern Error codes for not using OIDC
@mscbot concern Include rationale for Client-Server API endpoint over .well-known
@mscbot concern Checklist is incomplete

@mscbot mscbot added the unresolved-concerns This proposal has at least one outstanding concern label Jan 22, 2025
@sandhose sandhose requested a review from turt2live January 22, 2025 16:17
turt2live
turt2live approved these changes Jan 25, 2025
View reviewed changes
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

read-up-to marker

(thanks for keeping these updated)

@turt2live
Copy link
Member

@mscbot resolve Error codes for not using OIDC
@mscbot resolve Include rationale for Client-Server API endpoint over .well-known
@mscbot resolve Checklist is incomplete

@mscbot mscbot removed the unresolved-concerns This proposal has at least one outstanding concern label Jan 25, 2025
richvdh
richvdh reviewed Feb 4, 2025
View reviewed changes
proposals/2965-auth-metadata.md

If the homeserver does not offer next-generation authentication as described in [MSC3861], this endpoint should return a 404 with the `M_UNRECOGNIZED` error code.

In this case, clients should fall back to using the User-Interactive Authentication flows instead to authenticate the user.
Copy link
Member

Choose a reason for hiding this comment

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

or /login ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't /login the main User-Interactive Authentication flow?

clokep
clokep reviewed Feb 4, 2025
View reviewed changes
proposals/2965-auth-metadata.md Outdated
The first option would require making well-known documents mandatory on the server name domain, with a document that may need to be updated more frequently than existing ones.
This isn't practical for some server deployments, and clients may find it challenging to consistently perform this discovery.

The second option is also impractical, as all other Matrix APIs on this domain are prefixed with `/_matrix`, and it could easily be confused with the set of well-known documents hosted on the server name domain.
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand this -- I think the thread (https://github.com/matrix-org/matrix-spec-proposals/pull/2965/files#r1924524746) talks about before/after finding the client API. That rationale seems to be missing here.

Frankly to me, I'd expect this to be under well-known.

@TheOneWithTheBraid TheOneWithTheBraid mentioned this pull request Feb 7, 2025
Open
14 tasks
@zecakeh zecakeh mentioned this pull request Feb 15, 2025
Merged
bnjbvr pushed a commit to matrix-org/matrix-rust-sdk that referenced this pull request Feb 18, 2025
@zecakeh
refactor(oidc): Use the GET /auth_metadata Matrix endpoint (#4673)
2eb2ae7 
This is the method to get the server metadata in the latest draft of
[MSC2965](matrix-org/matrix-spec-proposals#2965).

We still keep the old behavior with `GET /auth_issuer` as fallback for
now because it has wider server support.

There are some pre-main commit cleanups to simplify the main commit.
This can be reviewed commit by commit.

The changes were tested with the oidc_cli example on beta.matrix.org.

Closes #4550.

---------

Signed-off-by: Kévin Commaille <[email protected]>
@sandhose sandhose requested review from erikjohnston and richvdh March 5, 2025 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andybalaam andybalaam andybalaam left review comments

@clokep clokep clokep left review comments

@Johennes Johennes Johennes left review comments

@oliverpool oliverpool oliverpool left review comments

@tonkku107 tonkku107 tonkku107 left review comments

@zecakeh zecakeh zecakeh left review comments

@turt2live turt2live turt2live approved these changes

@erikjohnston erikjohnston Awaiting requested review from erikjohnston

@richvdh richvdh Awaiting requested review from richvdh

Assignees
No one assigned
Labels
disposition-merge kind:core MSC which is critical to the protocol's success matrix-2.0 Required for Matrix 2.0 proposal A matrix spec change proposal proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period.
Projects
Spec Core Team Backlog
Status: Ready for FCP ticks
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.