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

MSC2967: API scopes #2967

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

MSC2967: API scopes #2967

wants to merge 13 commits into from

Conversation

sandhose
Copy link
Member

@sandhose sandhose commented Jan 14, 2021 •
edited by turt2live
Loading

Rendered

  • Spec is feature complete
    • Device handling
    • Allow for APIs other than Client-Server in the urn:matrix namespace
    • Security considerations section
  • Reviewed for consistency with MSC3861
  • Implementations believed to be complete enough

Dependencies:

Implementations:

Homeservers

Homeservers:

  • Synapse with Matrix Authentication Service in OIDC Playground: https://auth-oidc.element.dev
    • urn:matrix:client:api:*
    • urn:matrix:client:device:XXXX

Clients

Clients need to request scopes appropriately.

  • Element X iOS and Android
    • urn:matrix:client:api:*
    • urn:matrix:client:device:XXXX
  • Element Web
    • urn:matrix:client:api:*
    • urn:matrix:client:device:XXXX
  • hydrogen
    • urn:matrix:client:api:*
    • urn:matrix:client:device:XXXX
  • files-sdk-demo
    • urn:matrix:client:api:*
    • urn:matrix:client:device:XXXX

SCT stuff:

checklist

@turt2live turt2live changed the title MSC2967: [WIP] API scope restriction [WIP] MSC2967: API scope restriction Jan 14, 2021
@turt2live turt2live marked this pull request as draft January 14, 2021 17:28
@turt2live turt2live added kind:feature MSC for not-core and not-maintenance stuff proposal A matrix spec change proposal labels Jan 14, 2021
This was referenced Jan 15, 2021
Open
Closed
erkinalp
erkinalp suggested changes Jan 31, 2021
View reviewed changes
Copy link

@erkinalp erkinalp left a comment

Choose a reason for hiding this comment

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

Don't use the word sudo as we do not have a concept of superusers or substitute users in Matrix.

proposals/2967-api-scopes.md Outdated Show resolved Hide resolved
@aaronraimist
Copy link
Contributor

Related: https://github.com/matrix-org/matrix-doc/issues/2889

@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
@hughns hughns mentioned this pull request May 25, 2022
Open
@zecakeh zecakeh mentioned this pull request Jul 25, 2022
Merged
@hughns hughns changed the title [WIP] MSC2967: API scope restriction [WIP] MSC2967: API scopes Aug 3, 2022
@hughns hughns mentioned this pull request Aug 4, 2022
Open
@hughns hughns changed the title [WIP] MSC2967: API scopes [WIP] MSC2967: OIDC API scopes Feb 28, 2023
@turt2live turt2live added the matrix-2.0 Required for Matrix 2.0 label Mar 3, 2023
clokep
clokep reviewed May 16, 2023
View reviewed changes
proposals/2967-api-scopes.md

#### Device ID handling

Presently a device ID is typically generated by the homeserver and is associated with a specific access token. In OAuth 2.0 there is no such thing as a session and so a mapping cannot be handled using the same mechanism.
Copy link
Member

Choose a reason for hiding this comment

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

Is this still true after refresh tokens (MSC2918)? I thought we did a bunch of work in Synapse related to this recently, but maybe I'm confusing different types of tokens.

@kerryarchibald kerryarchibald mentioned this pull request Jun 13, 2023
Closed
@kerryarchibald kerryarchibald mentioned this pull request Jun 23, 2023
Closed
@zecakeh zecakeh mentioned this pull request Aug 5, 2023
Merged
tonkku107
tonkku107 reviewed Aug 30, 2023
View reviewed changes
proposals/2967-api-scopes.md

#### Device ID handling

Presently a device ID is typically generated by the homeserver and is associated with a specific access token. In OAuth 2.0 there is no such thing as a session and so a mapping cannot be handled using the same mechanism.
Copy link

Choose a reason for hiding this comment

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

In OAuth 2.0 there is no such thing as a session and so a mapping cannot be handled using the same mechanism.

In OIDC they define a sid (Session ID) claim within the id_token in the Front-Channel Logout spec (also referenced in other specs)

I would also like to mention that since clients are expected to use dynamic registration, client_ids resemble device IDs quite a bit

@hughns hughns mentioned this pull request May 10, 2024
Closed
@sandhose sandhose changed the base branch from old_master to main September 3, 2024 11:35
@sandhose
Rework MSC
f65aef3 
- Remove insufficient privilege response
- Remove guest scopes
- Reword some parts
@sandhose sandhose changed the title [WIP] MSC2967: OIDC API scopes MSC2967: OIDC API scopes Sep 16, 2024
@sandhose sandhose marked this pull request as ready for review September 16, 2024 13:11
@sandhose sandhose changed the title MSC2967: OIDC API scopes MSC2967: API scopes Sep 20, 2024
@t3chguy t3chguy mentioned this pull request Dec 19, 2024
Closed
@turt2live turt2live added kind:core MSC which is critical to the protocol's success implementation-needs-checking The MSC has an implementation, but the SCT has not yet checked it. and removed kind:feature MSC for not-core and not-maintenance stuff needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. labels Jan 18, 2025
turt2live
turt2live requested changes Jan 27, 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.

Overall this MSC looks ready to go, though there's some existing comments which should be addressed/incorporated before FCP is proposed.

proposals/2967-api-scopes.md

| Scope | Purpose | Implementation notes |
| - | - | - |
| `urn:matrix:client:api:*` | Grants full access to the Client-Server API | The OP can issue a refresh token for grants with this scope. |
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| `urn:matrix:client:api:*` | Grants full access to the Client-Server API | The OP can issue a refresh token for grants with this scope. |
| `urn:matrix:client:api:*` | Grants full access to the Client-Server API | The homeserver can issue a refresh token for grants with this scope. |

@turt2live turt2live removed the implementation-needs-checking The MSC has an implementation, but the SCT has not yet checked it. label Jan 27, 2025
@turt2live
Copy link
Member

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.

@TheOneWithTheBraid TheOneWithTheBraid mentioned this pull request Feb 7, 2025
Draft
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clokep clokep clokep left review comments

@tonkku107 tonkku107 tonkku107 left review comments

@hughns hughns hughns left review comments

@kegsay kegsay kegsay left review comments

@turt2live turt2live turt2live requested changes

@erkinalp erkinalp erkinalp requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
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
Projects
Spec Core Team Backlog
Status: Proposed for FCP readiness
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@sandhose @aaronraimist @turt2live @clokep @tonkku107 @erkinalp @hughns @kegsay