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.

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

MSC2966: Usage of OAuth 2.0 Dynamic Client Registration in Matrix #2966

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

Conversation

sandhose
Copy link
Member

@sandhose sandhose commented Jan 14, 2021

@turt2live turt2live changed the title MSC2966: [WIP] Usage of OAuth 2.0 Dynamic Client Registration in Matrix [WIP] MSC2966: Usage of OAuth 2.0 Dynamic Client Registration in Matrix 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
@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
proposals/2966-oauth2-dynamic-registration.md Outdated Show resolved Hide resolved
proposals/2966-oauth2-dynamic-registration.md Outdated Show resolved Hide resolved
proposals/2966-oauth2-dynamic-registration.md Outdated Show resolved Hide resolved
proposals/2966-oauth2-dynamic-registration.md Outdated Show resolved Hide resolved
@turt2live turt2live added the matrix-2.0 Required for Matrix 2.0 label Mar 3, 2023
@sandhose sandhose closed this Sep 3, 2024
@sandhose sandhose deleted the msc/sandhose/oauth2-dynamic-registration branch September 3, 2024 10:23
@sandhose sandhose restored the msc/sandhose/oauth2-dynamic-registration branch September 3, 2024 10:24
@sandhose sandhose reopened this Sep 3, 2024
@sandhose sandhose changed the base branch from old_master to main September 3, 2024 10:25
 - makes some metadata optional
 - better explain how each metadata field is used
 - better explain what the restrictions on redirect_uris are
 - remove the signed metadata part
 - mention the client metadata JSON document alternative
@sandhose sandhose changed the title [WIP] MSC2966: Usage of OAuth 2.0 Dynamic Client Registration in Matrix MSC2966: Usage of OAuth 2.0 Dynamic Client Registration in Matrix Sep 13, 2024
@sandhose sandhose marked this pull request as ready for review September 13, 2024 15:41
@sandhose sandhose force-pushed the msc/sandhose/oauth2-dynamic-registration branch from 85d1958 to 0e2f0f1 Compare January 17, 2025 09:14
@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
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 looks good, though I think it needs implementations to be able to move to FCP. I'll document those requirements in a separate review.

proposals/2966-oauth2-dynamic-registration.md Outdated Show resolved Hide resolved
proposals/2966-oauth2-dynamic-registration.md Outdated Show resolved Hide resolved
proposals/2966-oauth2-dynamic-registration.md Show resolved Hide resolved
proposals/2966-oauth2-dynamic-registration.md Outdated Show resolved Hide resolved
proposals/2966-oauth2-dynamic-registration.md Outdated Show resolved Hide resolved
proposals/2966-oauth2-dynamic-registration.md Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Implementation requirements:

  • Client using web flow
  • Client using native flow
  • Server supporting both flows (may be two different servers)

Copy link
Member Author

Choose a reason for hiding this comment

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

Known working clients with the web flow:

  • Element Web

Known working clients with the native flow:

  • Element Desktop
  • Element X iOS
  • Element X Android

Server supporting both:

  • Synapse with Matrix Authentication Service
  • Dendrite with Matrix Authentication Service (PR)

Copy link
Member

Choose a reason for hiding this comment

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

@turt2live turt2live added needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. and removed implementation-needs-checking The MSC has an implementation, but the SCT has not yet checked it. labels Jan 22, 2025
@sandhose sandhose requested a review from turt2live January 22, 2025 14:37
@turt2live turt2live removed the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Jan 25, 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.

@turt2live
Copy link
Member

@mscbot fcp merge

@mscbot
Copy link
Collaborator

mscbot commented Jan 25, 2025

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

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 25, 2025
**Note**: in this example, the server has not registered the locale-specific values for `client_name`, `tos_uri`, and `policy_uri`, which is why they are not present in the response. The server also does not support the `urn:ietf:params:oauth:grant-type:token-exchange` grant type, which is why it is not present in the response.

The client MUST store the `client_id` for future use.
It SHOULD reuse the `client_id` for all future authorization requests done against the same homeserver.
Copy link
Contributor

Choose a reason for hiding this comment

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

So to be clear, the client should associate the client_id to the server name of the homeserver? The issuer field from RFC8414 is ignored, since it doesn't appear to be mentioned in any MSC?

Won't that complicate the migration of current implementations that should have associated their client_id to the issuer?

Comment on lines +26 to +27
#### Localizable metadata

Copy link
Member

Choose a reason for hiding this comment

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

This section appears to be empty?

Comment on lines +84 to +149
#### Redirect URI validation

The redirect URI plays a critical role in validating the authenticity of the client.
The client 'proves' its identity by demonstrating that it controls the redirect URI.
This is why it is critical to have strict validation of the redirect URI.

The `application_type` metadata is used to determine the type of client.
It defaults to `web` if not present, and can be set to `native` to indicate that the client is a native application.

In all cases, the redirect URI MUST not have a fragment component.

#### Web clients

`web` clients can use redirect URIs that:

- MUST use the `https` scheme
- MUST omit the port (to use the default port for https: 443)
- MUST not use a user or password in the authority component of the URI
- MUST use the client URI as a common base for the authority component

Examples of valid redirect URIs (with `https://example.com/` as the client URI):

- `https://example.com/callback`
- `https://app.example.com/callback`
- `https://example.com/?query=value`

Examples of invalid redirect URIs (with `https://example.com/` as the client URI):

- `https://example.com/callback#fragment`
- `https://example.com:8080/callback`
- `http://example.com/callback`
- `http://localhost/`

#### Native clients

`native` clients can use three types of redirect URIs:

1. Private-Use URI Scheme:
- the scheme MUST be prefixed with the client URI hostname in reverse-DNS notation. For example, if the client URI is `https://example.com/`, then a valid custom URI scheme would be `com.example.app:/`.
- the URI MUST not have an authority component. This means that it MUST have either a single slash or none immediately following the scheme, with no hostname, username, or port.
2. "http" URIs on the loopback interface:
- it MUST use the `http` scheme
- the host part MUST be `localhost`, `127.0.0.1`, or `[::1]`
- it MUST have no port registered. The homeserver MUST then accept any port number during the authorization flow.
3. Claimed "https" Scheme URI:
- some operating systems allow apps to claim "https" scheme URIs in the domains they control
- when the browser encounters a claimed URI, instead of the page being loaded in the browser, the native app is launched with the URI supplied as a launch parameter
- the same rules as for `web` clients apply

These restrictions are the same as defined by [RFC8252 sec. 7](https://tools.ietf.org/html/rfc8252#section-7).

Examples of valid redirect URIs (with `https://example.com/` as the client URI):

- `com.example.app:/callback`
- `com.example:/`
- `com.example:callback`
- `http://localhost/callback`
- `http://127.0.0.1/callback`
- `http://[::1]/callback`

Examples of invalid redirect URIs (with `https://example.com/` as the client URI):

- `example:/callback`
- `com.example.app://callback`
- `https://localhost/callback`
- `http://localhost:1234/callback`
Copy link
Member

Choose a reason for hiding this comment

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

If these three sections are all about redirect URIs, it would be really helpful to say so. I was expecting a section called "Web clients" to be more general.

This proposal is part of the broader [MSC3861: Next-generation auth for Matrix, based on OAuth 2.0/OIDC](https://github.com/matrix-org/matrix-spec-proposals/pull/3861).

This MSC specifies how Matrix clients SHOULD leverage the OAuth 2.0 Dynamic Client Registration Protocol ([RFC 7591](https://tools.ietf.org/html/rfc7591)) to register themselves before initiating an authorization flow.

Copy link
Member

Choose a reason for hiding this comment

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

I'd find it really helpful to give a one-line summary of how this MSC fits in with the others, and also explain, to those of us less familiar with RFC7591, how often we expect clients to hit this endpoint.

Suggested change
In brief, once a client has obtained the homeserver's OAuth 2.0 metadata per [MSC2965](https://github.com/matrix-org/matrix-spec-proposals/pull/2965), and before it can initiate an authorization request per [MSC2964](https://github.com/matrix-org/matrix-spec-proposals/pull/2964), the client must register itself with that homeserver to obtain a `client_id`.
It stores this `client_id` and uses it for all future authorization requests against the homeserver.

(There is something to this effect in the "client metadata" section, but it feels a bit lost in there, given that's just one of three h3-level sections in the proposal)

Comment on lines +151 to +212
### Dynamic client registration

Before initiating an authorization flow, the client MUST advertise its metadata to the homeserver to get back a `client_id`.

This is done through the `registration_endpoint` as described by [RFC7591 sec. 3](https://tools.ietf.org/html/rfc7591#section-3).

To register, the client sends an HTTP POST to the `registration_endpoint` with its metadata as JSON in the body.
For example, the client could send the following registration request:

```http
POST /register HTTP/1.1
Content-Type: application/json
Accept: application/json
Server: auth.example.com
```

```json
{
"client_name": "My App",
"client_name#fr": "Mon application",
"client_uri": "https://example.com/",
"logo_uri": "https://example.com/logo.png",
"tos_uri": "https://example.com/tos.html",
"tos_uri#fr": "https://example.com/fr/tos.html",
"policy_uri": "https://example.com/policy.html",
"policy_uri#fr": "https://example.com/fr/policy.html",
"redirect_uris": ["https://app.example.com/callback"],
"token_endpoint_auth_method": "none",
"response_types": ["code"],
"grant_types": [
"authorization_code",
"refresh_token",
"urn:ietf:params:oauth:grant-type:token-exchange"
],
"application_type": "web"
}
```

The server replies with a JSON object containing the `client_id` allocated, as well as all the metadata values that the server registered.
It MUST ignore fields, `grant_types` and `response_types` that are not understood by the server.

With the previous registration request, the server would reply with:

```json
{
"client_id": "s6BhdRkqt3",
"client_name": "My App",
"client_uri": "https://example.com/",
"logo_uri": "https://example.com/logo.png",
"tos_uri": "https://example.com/tos.html",
"policy_uri": "https://example.com/policy.html",
"redirect_uris": ["https://app.example.com/callback"],
"response_types": ["code"],
"grant_types": ["authorization_code", "refresh_token"],
"application_type": "web"
}
```

**Note**: in this example, the server has not registered the locale-specific values for `client_name`, `tos_uri`, and `policy_uri`, which is why they are not present in the response. The server also does not support the `urn:ietf:params:oauth:grant-type:token-exchange` grant type, which is why it is not present in the response.

The client MUST store the `client_id` for future use.
It SHOULD reuse the `client_id` for all future authorization requests done against the same homeserver.
Copy link
Member

Choose a reason for hiding this comment

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

Personally, I'd find this proposal way easier to grok if the API was defined before the relevant metadata (with words like: "the client MUST advertise its metadata to the homeserver to get back a client_id. The metadata is as descripted under 'Client metadata' below").

Otherwise I have to read all through the "metadata" section before I get an answer to "ok, but what is all this stuff actually for?"

Not insisting on a change here, but just thinking it might help future readers.

Comment on lines +192 to +207
With the previous registration request, the server would reply with:

```json
{
"client_id": "s6BhdRkqt3",
"client_name": "My App",
"client_uri": "https://example.com/",
"logo_uri": "https://example.com/logo.png",
"tos_uri": "https://example.com/tos.html",
"policy_uri": "https://example.com/policy.html",
"redirect_uris": ["https://app.example.com/callback"],
"response_types": ["code"],
"grant_types": ["authorization_code", "refresh_token"],
"application_type": "web"
}
```
Copy link
Member

Choose a reason for hiding this comment

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

I notice that RFC7591 specifies client_secret as an optional field here. Do we want to allow matrix homeservers to return a client_secret and expect clients to know what to do with it? I guess the answer is yes, given RFC7591 says so, but it wouldn't hurt to be explicit.

Comment on lines +28 to +31
#### `client_uri` and relationship with other URIs

Per [RFC 7591](https://tools.ietf.org/html/rfc7591), the `client_uri` MUST be a valid URL that SHOULD give the user more information about the client.
This URL SHOULD NOT require authentication to access.
Copy link
Member

Choose a reason for hiding this comment

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

is client_uri required?

If not, some "If present" wording would be helpful, and explain how the constraints on the other URIs work when it is absent.

If it is required, call it out explicitly, and maybe say what should happen if it's not there?

The `application_type` metadata is used to determine the type of client.
It defaults to `web` if not present, and can be set to `native` to indicate that the client is a native application.

In all cases, the redirect URI MUST not have a fragment component.
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
In all cases, the redirect URI MUST not have a fragment component.
In all cases, the redirect URI MUST NOT have a fragment component.


1. Private-Use URI Scheme:
- the scheme MUST be prefixed with the client URI hostname in reverse-DNS notation. For example, if the client URI is `https://example.com/`, then a valid custom URI scheme would be `com.example.app:/`.
- the URI MUST not have an authority component. This means that it MUST have either a single slash or none immediately following the scheme, with no hostname, username, or port.
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
- the URI MUST not have an authority component. This means that it MUST have either a single slash or none immediately following the scheme, with no hostname, username, or port.
- the URI MUST NOT have an authority component. This means that it MUST have either a single slash or none immediately following the scheme, with no hostname, username, or port.

2. "http" URIs on the loopback interface:
- it MUST use the `http` scheme
- the host part MUST be `localhost`, `127.0.0.1`, or `[::1]`
- it MUST have no port registered. The homeserver MUST then accept any port number during the authorization flow.
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
- it MUST have no port registered. The homeserver MUST then accept any port number during the authorization flow.
- it (the registered redirect URI) MUST NOT contain a port number. The homeserver MUST then accept any port number during the authorization flow.

## Alternatives

An alternative approach would be to have the client host a JSON file containing its metadata and use that URL as the `client_id`.
This is what the following [*OAuth Client ID Metadata Document* draft](https://datatracker.ietf.org/doc/html/draft-parecki-oauth-client-id-metadata-document) proposes.
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
This is what the following [*OAuth Client ID Metadata Document* draft](https://datatracker.ietf.org/doc/html/draft-parecki-oauth-client-id-metadata-document) proposes.
This is what the [*OAuth Client ID Metadata Document* draft](https://datatracker.ietf.org/doc/html/draft-parecki-oauth-client-id-metadata-document) proposes.

Copy link
Member

Choose a reason for hiding this comment

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

or:

Suggested change
This is what the following [*OAuth Client ID Metadata Document* draft](https://datatracker.ietf.org/doc/html/draft-parecki-oauth-client-id-metadata-document) proposes.
This is what the following document proposes: [*OAuth Client ID Metadata Document* draft](https://datatracker.ietf.org/doc/html/draft-parecki-oauth-client-id-metadata-document).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Status: Ready for FCP ticks
Development

Successfully merging this pull request may close these issues.

9 participants