feat(auth): unauthenticated RFC 8414 on the MCP origin #94

Open

jlxq0 wants to merge 3 commits from feat/as-well-known into main

pull from: feat/as-well-known

merge into: jlxq0:main

jlxq0:main

jlxq0:read-the-log

jlxq0:permission-relay

jlxq0:base64-padding

jlxq0:clippy-floor

jlxq0:channel-download

jlxq0:fix/channel-push-observability

jlxq0:build-jmap-image

jlxq0:tts/julian-recipe-defaults

Conversation 3 Commits 3 Files changed 2 +71 -5

jlxq0 commented 2026-08-17 04:21:34 +00:00

Owner

Copy link

Summary

• GET /.well-known/oauth-authorization-server is 200 and advertises the MAS registration_endpoint.
• allow_insecure_uris stays false.

Test plan

• cargo test as_metadata_advertises
• live AS 200 (not 401)

## Summary - GET /.well-known/oauth-authorization-server is 200 and advertises the MAS registration_endpoint. - allow_insecure_uris stays false. ## Test plan - [x] cargo test as_metadata_advertises - [ ] live AS 200 (not 401)

jlxq0 added 1 commit 2026-08-17 04:21:34 +00:00

feat(auth): serve RFC 8414 metadata on the MCP origin ... a0a389d308

Unauthenticated GET /.well-known/oauth-authorization-server was falling
through to a 401. Advertise MAS registration_endpoint so DCR probes
succeed without flipping allow_insecure_uris.

jlxq0 referenced this pull request 2026-08-27 02:01:06 +00:00

docs(agents): why main requires cargo and not docker #131

jlxq0 force-pushed feat/as-well-known from a0a389d308 to 395b32c233 2026-09-02 02:40:32 +00:00 Compare

jlxq0 commented 2026-09-02 02:41:03 +00:00

Author

Owner

Copy link

Blocked on a finding, not on review capacity. Rebased onto main and pushed; the cargo gate had been red since 17 August on one unformatted assert_eq! in this file's own test, which is why it read as fifteen days of nobody looking. The gate now passes locally. What follows is why it should still not merge as written.

Read against the question can this new code do the wrong thing in the case it was written for, and then checked against the live MAS document rather than against the RFCs alone.

issuer names the wrong service

issuer: cfg.resource_url.clone(),
// RFC 8414: issuer must match the origin this document is served from.

The comment is the mistake. RFC 8414 §2 defines issuer as the authorization server's identifier, not the origin serving the document. Measured against the live MAS at https://matrixauthservice.kampong.social:

field                    MAS advertises                              this PR emits
issuer                   https://matrixauthservice.kampong.social/   https://matrix-mcp.kampong.social

A client that records the metadata issuer as the expected authorization-server identity, sends the user to MAS, and then compares the RFC 9207 iss in the authorization response, compares matrixauthservice… against matrix-mcp… and must reject the code. That failure lands after the user has authenticated, which is the expensive place for it. And the trailing slash is load-bearing: MATRIX_MCP_AUTHORIZATION_SERVER is set to https://matrixauthservice.kampong.social/ and authorization_server_keeps_its_trailing_slash already exists to preserve it, so the correct value is cfg.authorization_server verbatim rather than the authorization_server_base() form used for the endpoints.

The route cannot be made compliant, only less wrong

Setting issuer to MAS does not rescue it. RFC 8414 §3.3 has the client check that the document's issuer matches the issuer it derived the retrieval URL from, and the retrieval URL here is our origin. So both values fail a strict client, one at retrieval and one after authentication. That is not an argument for either value; it is the reason to say plainly what this route is, which is a compatibility shim for MCP clients predating the 2025-06-18 move to RFC 9728.

The strict path already works and needs none of this:

/.well-known/oauth-protected-resource/mcp   200
/.well-known/oauth-authorization-server     401 with
  WWW-Authenticate: Bearer resource_metadata="…/.well-known/oauth-protected-resource/mcp", scope="…"

So a current client is told where to go by the 401 it already gets. This PR is for clients that do not read that header, and the PR should say so, because the next reader will otherwise take the route as the compliant one and the 9728 pair as legacy.

A hand-built copy of another service's document drifts, and this one already has

Two fields disagree with MAS today, before anything changes:

code_challenge_methods_supported   MAS: plain, S256                          PR: S256
grant_types_supported              MAS: authorization_code, refresh_token,   PR: authorization_code,
                                        client_credentials, device_code           refresh_token

Both narrowings are safe in themselves and neither is the point. Nothing keeps the copy in step with MAS, no test compares them, and the three endpoint paths are string-built here rather than read from the source that defines them. They happen to match exactly today, which I checked:

authorization_endpoint   match
token_endpoint           match
registration_endpoint    match

That is a measurement of one afternoon, not a property. registration_endpoint is the one that bites hardest if it ever stops matching, since asserting it means a client doing dynamic registration POSTs there and the whole connection setup fails.

A 302 to https://matrixauthservice.kampong.social/.well-known/oauth-authorization-server cannot drift, gives a lenient client the authoritative document with MAS's own issuer in it, and fails a strict client at exactly the same place the copy does. It is fewer lines than the struct it replaces.

What I would change before merging

• Redirect rather than copy, or if the copy is wanted for a client that will not follow one, set issuer to cfg.authorization_server verbatim and add a test that fetches MAS's document and compares the fields, so drift is a red build rather than a client's problem.
• Say in the code comment that the route is a pre-2025-06-18 compatibility shim and that RFC 9728 is the compliant path, since the current comment states the opposite of RFC 8414 §2.
• Drop the /mcp variant or justify it. Its retrieval URL implies issuer …/mcp, which the document does not return under either candidate value.

Cross-engine review by Codex found the issuer fault and the §3.3 consequence; the drift measurement and the live-MAS comparison are mine. Codex also flagged registration_endpoint as unsafe if MAS lacks RFC 7591, which does not hold here — MAS advertises it at exactly the constructed path.

**Blocked on a finding, not on review capacity.** Rebased onto `main` and pushed; the cargo gate had been red since 17 August on one unformatted `assert_eq!` in this file's own test, which is why it read as fifteen days of nobody looking. The gate now passes locally. What follows is why it should still not merge as written. Read against the question *can this new code do the wrong thing in the case it was written for*, and then checked against the live MAS document rather than against the RFCs alone. ## `issuer` names the wrong service issuer: cfg.resource_url.clone(), // RFC 8414: issuer must match the origin this document is served from. The comment is the mistake. RFC 8414 §2 defines `issuer` as **the authorization server's identifier**, not the origin serving the document. Measured against the live MAS at `https://matrixauthservice.kampong.social`: field MAS advertises this PR emits issuer https://matrixauthservice.kampong.social/ https://matrix-mcp.kampong.social A client that records the metadata `issuer` as the expected authorization-server identity, sends the user to MAS, and then compares the RFC 9207 `iss` in the authorization response, compares `matrixauthservice…` against `matrix-mcp…` and must reject the code. **That failure lands after the user has authenticated**, which is the expensive place for it. And the trailing slash is load-bearing: `MATRIX_MCP_AUTHORIZATION_SERVER` is set to `https://matrixauthservice.kampong.social/` and `authorization_server_keeps_its_trailing_slash` already exists to preserve it, so the correct value is `cfg.authorization_server` verbatim rather than the `authorization_server_base()` form used for the endpoints. ## The route cannot be made compliant, only less wrong Setting `issuer` to MAS does not rescue it. RFC 8414 §3.3 has the client check that the document's issuer matches the issuer it derived the retrieval URL from, and the retrieval URL here is our origin. So **both values fail a strict client**, one at retrieval and one after authentication. That is not an argument for either value; it is the reason to say plainly what this route is, which is a compatibility shim for MCP clients predating the 2025-06-18 move to RFC 9728. The strict path already works and needs none of this: /.well-known/oauth-protected-resource/mcp 200 /.well-known/oauth-authorization-server 401 with WWW-Authenticate: Bearer resource_metadata="…/.well-known/oauth-protected-resource/mcp", scope="…" So a current client is told where to go by the 401 it already gets. **This PR is for clients that do not read that header**, and the PR should say so, because the next reader will otherwise take the route as the compliant one and the 9728 pair as legacy. ## A hand-built copy of another service's document drifts, and this one already has Two fields disagree with MAS today, before anything changes: code_challenge_methods_supported MAS: plain, S256 PR: S256 grant_types_supported MAS: authorization_code, refresh_token, PR: authorization_code, client_credentials, device_code refresh_token Both narrowings are safe in themselves and neither is the point. **Nothing keeps the copy in step with MAS**, no test compares them, and the three endpoint paths are string-built here rather than read from the source that defines them. They happen to match exactly today, which I checked: authorization_endpoint match token_endpoint match registration_endpoint match That is a measurement of one afternoon, not a property. `registration_endpoint` is the one that bites hardest if it ever stops matching, since asserting it means a client doing dynamic registration POSTs there and the whole connection setup fails. **A 302 to `https://matrixauthservice.kampong.social/.well-known/oauth-authorization-server` cannot drift**, gives a lenient client the authoritative document with MAS's own issuer in it, and fails a strict client at exactly the same place the copy does. It is fewer lines than the struct it replaces. ## What I would change before merging - Redirect rather than copy, or if the copy is wanted for a client that will not follow one, set `issuer` to `cfg.authorization_server` verbatim and add a test that fetches MAS's document and compares the fields, so drift is a red build rather than a client's problem. - Say in the code comment that the route is a pre-2025-06-18 compatibility shim and that RFC 9728 is the compliant path, since the current comment states the opposite of RFC 8414 §2. - Drop the `/mcp` variant or justify it. Its retrieval URL implies issuer `…/mcp`, which the document does not return under either candidate value. Cross-engine review by Codex found the `issuer` fault and the §3.3 consequence; the drift measurement and the live-MAS comparison are mine. Codex also flagged `registration_endpoint` as unsafe if MAS lacks RFC 7591, which **does not hold here** — MAS advertises it at exactly the constructed path.

jlxq0 force-pushed feat/as-well-known from 395b32c233 to bb1b4bde7f 2026-09-02 02:52:38 +00:00 Compare

jlxq0 commented 2026-09-02 02:52:52 +00:00

Author

Owner

Copy link

Two mechanical blockers, both now fixed and pushed; the design finding above still stands.

The cargo gate had two failures stacked, and the first hid the second:

cargo fmt --all --check                     one unformatted assert_eq! in this file's own test
cargo clippy --all-targets --all-features   build_router is 103 lines against the 100 ceiling

The second is caused by this PR and would have been caused by any PR adding a route. too_many_lines counts the whole function, so the eight lines here tipped a function that was already at 95. Lifted the four .well-known routes into well_known_routes() -> Router<Config> with no path, handler or ordering change.

Gates run with CI's exact flags on 1.98.0, which is the pin in ci.yml:

cargo fmt --all --check                                    rc=0
cargo clippy --all-targets --all-features --locked -D warnings  rc=0
cargo test --all-features --locked                         rc=0, 313 passed

And a note on how I nearly reported this wrong. My first three runs went through build-slot.sh -- <cmd>, which the wrapper rejects with a usage error, so clippy and the tests never executed; I then read the background task's exit code, which was the last command in the compound rather than the one I cared about. Two of this repository's own AGENTS.md rules in one command. The numbers above come from runs whose rc was captured immediately after the command and before any pipe.

So the state of this PR is: green mechanically, held on the issuer value and on whether this origin should serve a copy of MAS's document at all.

**Two mechanical blockers, both now fixed and pushed; the design finding above still stands.** The cargo gate had two failures stacked, and the first hid the second: cargo fmt --all --check one unformatted assert_eq! in this file's own test cargo clippy --all-targets --all-features build_router is 103 lines against the 100 ceiling **The second is caused by this PR and would have been caused by any PR adding a route.** `too_many_lines` counts the whole function, so the eight lines here tipped a function that was already at 95. Lifted the four `.well-known` routes into `well_known_routes() -> Router<Config>` with no path, handler or ordering change. Gates run with CI's exact flags on 1.98.0, which is the pin in `ci.yml`: cargo fmt --all --check rc=0 cargo clippy --all-targets --all-features --locked -D warnings rc=0 cargo test --all-features --locked rc=0, 313 passed **And a note on how I nearly reported this wrong.** My first three runs went through `build-slot.sh -- <cmd>`, which the wrapper rejects with a usage error, so clippy and the tests never executed; I then read the background task's exit code, which was the last command in the compound rather than the one I cared about. Two of this repository's own `AGENTS.md` rules in one command. The numbers above come from runs whose rc was captured immediately after the command and before any pipe. So the state of this PR is: **green mechanically, held on the `issuer` value and on whether this origin should serve a copy of MAS's document at all.**

jlxq0 added the

blocked

label 2026-09-02 12:04:07 +00:00

jlxq0 commented 2026-09-02 12:04:08 +00:00

Author

Owner

Copy link

blocked on #148. Mechanically green as of bb1b4bd: rebased, cargo fmt --all --check, cargo clippy --all-targets --all-features --locked -D warnings and cargo test --all-features --locked all rc=0 locally on the pinned 1.98.0, and CI / cargo and CI / docker both success on the forge.

What remains is a decision rather than work: whether this origin serves a copy of MAS's RFC 8414 document, a 302 to it, or nothing. The measurements and the three options are in #148 so they outlive this PR and this session.

**blocked on #148.** Mechanically green as of `bb1b4bd`: rebased, `cargo fmt --all --check`, `cargo clippy --all-targets --all-features --locked -D warnings` and `cargo test --all-features --locked` all rc=0 locally on the pinned 1.98.0, and `CI / cargo` and `CI / docker` both success on the forge. What remains is a decision rather than work: whether this origin serves a copy of MAS's RFC 8414 document, a 302 to it, or nothing. The measurements and the three options are in #148 so they outlive this PR and this session.

All checks were successful

Hide all checks

CI / cargo (pull_request) Successful in 2m4s

Required

Details

CI / docker (pull_request) Has been skipped

Details

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin feat/as-well-known:feat/as-well-known

git switch feat/as-well-known

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

blocked

Held on a named, checkable blocker: an issue number, a credential or a measurement. Not "held".

  

bug

Something isn't working

  

documentation

Improvements or additions to documentation

  

duplicate

This issue or pull request already exists

  

enhancement

New feature or request

  

good first issue

Good for newcomers

  

help wanted

Extra attention is needed

  

invalid

This doesn't seem right

  

question

Further information is requested

  

waiting-on-julian

Blocked on Julian. The ask is in the issue.

  

wontfix

This will not be worked on

No labels

blocked

bug

documentation

duplicate

enhancement

good first issue

help wanted

invalid

question

waiting-on-julian

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

jlxq0/matrix-mcp!94

No description provided.