MSC3861: Next-generation auth for Matrix, based on OAuth 2.0/OIDC

[hughns]

Rendered

Core sub-proposals:

• MSC2964: Usage of OAuth 2.0 authorization code grant and refresh token grant #2964
• MSC2965: OAuth 2.0 Authorization Server Metadata discovery #2965
• MSC2966: Usage of OAuth 2.0 Dynamic Client Registration in Matrix #2966
• MSC2967: API scopes #2967
• MSC4254: Usage of RFC7009 Token Revocation for Matrix client logout #4254

Additional proposals:

• MSC3824: OIDC aware clients #3824
• MSC4108: Mechanism to allow OIDC sign in and E2EE set up via QR code #4108
• MSC4190: Device management for application services #4190
• MSC4191: Account management deep-linking #4191
• MSC4198: Usage of OIDC login_hint #4198

---

SCT stuff:

checklist

[turt2live]

just a mid-read review. Submitting before I forget about it

[hughns]

I've added references to the following MSCs that this proposal assumes are generally accepted:

• MSC3967: Do not require UIA when first uploading cross signing keys #3967
• MSC3970: Scope transaction IDs to devices #3970

[turt2live]

Overall, this looks great to me. Thanks for putting so much time and energy into this project.

My review here is long, but the vast majority are non-blocking concerns for stating FCP. They will ultimately block FCP from finishing, but aren't critical enough to warrant a total block on progress, in my opinion.

The theme for the blocking concerns is largely confusion for this MSC's purpose: it feels like this proposal is trying to confirm that we want to go down this road for authentication, but promptly puts up dependencies which would make it a net-zero change once they're all accepted. I've left some suggestions on how to change the wording slightly to flip this around: we would use this MSC to confirm the direction, and leave implementation/specification details to the underlying MSCs. The suggestions use "or similar" language to cover a process quirk where we know that we're most likely going to put the listed MSCs forward for FCP, but there's a non-zero chance that an alternative shows up, so we escape the pigeon hole early with "or similar".

If the intention is to have the listed MSCs accepted first, I'm not clear on this MSC's intended purpose. Clarification in the MSC body would be appreciated.

[turt2live]

non-blocking concern: wrap lines to ~100 characters please

(tables are exempt from this because it's impossible)

[sandhose]

Is this a hard requirement? I personally prefer using one sentence per line for the reasons outlined here: https://nick.groenen.me/notes/one-sentence-per-line/

[turt2live]

non-blocking concern: it may be good to mention that servers are supposed to have fallback UI for UIA anyways, which is likely subject to many of the same concerns in a full production build. Like UIA fallback though, servers can also do the bare minimum for their web UI if they want.

[turt2live]

MSCs proposed for Final Comment Period (FCP) should meet the requirements outlined in the checklist prior to being accepted into the spec. This checklist is a bit long, but aims to reduce the number of follow-on MSCs after a feature lands.

SCT members: please check off things you check for, and raise a concern against FCP if the checklist is incomplete. If an item doesn't apply, prefer to check it rather than remove it. Unchecking items is encouraged where applicable.

Checklist:

• Are appropriate implementation(s)
  specified in the MSC’s PR description?
• Are all MSCs that this MSC depends on already accepted?
• For each new endpoint that is introduced:
  • Have authentication requirements been specified?
  • Have rate-limiting requirements been specified?
  • Have guest access requirements been specified?
  • Are error responses specified?
    • Does each error case have a specified errcode (e.g. M_FORBIDDEN) and HTTP status code?
      • If a new errcode is introduced, is it clear that it is new?
• Will the MSC require a new room version, and if so, has that been made clear?
  • Is the reason for a new room version clearly stated? For example,
    modifying the set of redacted fields changes how event IDs are calculated,
    thus requiring a new room version.
• Are backwards-compatibility concerns appropriately addressed?
• Are the endpoint conventions honoured?
  • Do HTTP endpoints use_underscores_like_this?
  • Will the endpoint return unbounded data? If so, has pagination been considered?
  • If the endpoint utilises pagination, is it consistent with
    the appendices?
• An introduction exists and clearly outlines the problem being solved.
  Ideally, the first paragraph should be understandable by a non-technical audience.
• All outstanding threads are resolved
  • All feedback is incorporated into the proposal text itself, either as a fix or noted as an alternative
• While the exact sections do not need to be present,
  the details implied by the proposal template are covered. Namely:
  • Introduction
  • Proposal text
  • Potential issues
  • Alternatives
  • Dependencies
• Stable identifiers are used throughout the proposal, except for the unstable prefix section
  • Unstable prefixes consider the awkward accepted-but-not-merged state
  • Chosen unstable prefixes do not pollute any global namespace (use “org.matrix.mscXXXX”, not “org.matrix”).
• Changes have applicable Sign Off from all authors/editors/contributors
• There is a dedicated "Security Considerations" section which detail
  any possible attacks/vulnerabilities this proposal may introduce, even if this is "None.".
  See RFC3552 for things to think about,
  but in particular pay attention to the OWASP Top Ten.