[PiquelChips]

Auth's OAuth errors drifted from RFC 6749, RFC 6750 and RFC 9126 in small ways that clients and the conformance suite trip on: state= sent back to clients that sent none (oauth4webapi rejects it), a Basic challenge for clients that never used Basic, and codes that don't say what went wrong.

Now auth answers each case the standard way:

• Codes. unsupported_response_type for anything but response_type=code; unauthorized_client for a client that must push its requests; insufficient_scope (403, with its Bearer challenge naming openid) for a UserInfo token without openid; invalid_request for a missing code, refresh_token or response_type.
• Challenges and statuses. invalid_client is 401 with a Basic challenge only when the client sent an Authorization header, otherwise 400. Nothing answers 401 without a challenge (a page refusing the browser's session is now 403). UserInfo without any token gets a bare Bearer challenge.
• state is optional end to end (nullable column, migration 0049), so none goes back when none was sent, and an oversized one fails before any redirect carries it.
• Token responses carry Pragma: no-cache beside Cache-Control: no-store.
• Pushed requests. A request_uri is read before every other parameter, so max_age=garbage beside it no longer fails the claim, and a push must name client_id even with Basic. Both SDKs now always send client_id.

An error before the redirect URI is verified already shows auth's error page since #518, so this no longer moves /oauth/authorize and /oidc/logout into the pages.

Tests cover each case (oidc/tests/errors.rs, plus updated expectations elsewhere), and the OIDC reference, troubleshooting page and architecture doc describe the behaviour.

Stacked on #521.

Closes #275

Note

Written by Claude Opus 5.5 in Claude Code (via T3 Code)

Summary

This change aligns OAuth 2.0 and OIDC error codes, HTTP status codes, WWW-Authenticate challenges, and state handling with RFC 6749, RFC 6750, and RFC 9126. It also updates PAR request handling, token response headers, and the related SDKs, database schema, tests, and documentation.

Test results and conformance suite results are not available from the supplied evidence.

Protocol and error model

• crates/common/src/protocol/oauth.rs: ErrorCode adds UnauthorizedClient (unauthorized_client), InsufficientScope (insufficient_scope), and UnsupportedResponseType (unsupported_response_type).
• apps/auth-service/src/oidc/error.rs:
  • OAuthError::Protocol stores an optional credential challenge.
  • A new Challenge type represents Basic and Bearer challenges. It supplies the WWW-Authenticate value and the response status.
  • Basic challenges return 401. A Bearer challenge uses the status from BearerChallenge, and falls back to 401 if that status is invalid.
  • OAuthError::challenged attaches a challenge only to protocol rejections. Internal errors do not receive one.
  • into_response emits the stored challenge header.
  • The code-based challenge selection is removed. Callers now assign challenges.
  • Unchallenged InvalidToken and InsufficientScope responses return 403.
  • InvalidClient and UnauthorizedClient use the default 400 status instead of 401.
• apps/auth-service/src/oidc/inline.rs: InlineResponse.state is optional. It is omitted from the JSON when absent.

Client authentication and redirects

• apps/auth-service/src/oidc/mod.rs:
  • Client authentication separates requests with an Authorization header from requests without one. Failures with a header receive a Basic challenge. Failures without a header have no challenge.
  • Authentication logic moves into a helper that receives the optional header.
  • ClientRedirect accepts an optional state. It appends state only when present. iss is still appended.

Authorization endpoint (/oauth/authorize)

• apps/auth-service/src/oidc/authorize/mod.rs:
  • The handler accepts raw query or form bytes and parses them in two stages.
  • First, it reads request_uri and its optional client_id. If request_uri is present, it claims the pushed request and does not parse other parameters.
  • Otherwise, it deserializes and validates the full browser request.
  • Malformed raw input or parameters return invalid_request.
  • A missing state is rejected before the client redirect is built.
  • A client that requires pushed requests receives unauthorized_client instead of invalid_request.
  • start_authorization returns the redirect URL directly. resume remains a separate redirect handler.
  • Client redirects pass state and the stored client_state as optional values.
• apps/auth-service/src/oidc/authorize/parameters.rs:
  • AuthorizationParameters.state changes from String to Option<String>.
  • The 512-byte limit applies only when state is present. Oversized values still return InvalidRequest.
  • state() becomes pub(super).
  • An empty response_type returns InvalidRequest.
  • An unsupported non-empty response_type returns UnsupportedResponseType instead of InvalidRequest.
• apps/auth-service/src/oidc/authorize/consent.rs: decide passes request.client_state.as_deref() to client_redirect.

Pushed authorization requests (PAR)

• apps/auth-service/src/oidc/authorize/pushed.rs:
  • push_authorization returns invalid_request when client_id is absent. This check runs before client authentication.
  • When client_id is present, it always passes client_id to client authentication. This includes requests that use Basic credentials.

Token endpoint and UserInfo

• apps/auth-service/src/oidc/token.rs:
  • OidcApplication::token returns a Pragma: no-cache header with the token JSON. Its signature changes to return ([(HeaderName, &'static str); 1], Json<TokenResponse>) in the success case.
  • Missing code and missing refresh_token return invalid_request instead of invalid_grant.
  • userinfo_response attaches Bearer challenges to missing, malformed, or unverifiable access tokens. These return invalid_token.
  • A token without the openid scope returns insufficient_scope. The challenge names the required scope.

Database

• crates/database/migrations/0049_optional_state.sql: removes the NOT NULL constraint from authorization_requests.client_state.
• crates/database/src/repositories/oidc/mod.rs: InsertAuthorizationRequest::client_state and StoredAuthorizationRequest::client_state change from String to Option<String>. Their documentation describes state as optional.
• crates/database/src/repositories/oidc/tests.rs: the PAR test sets client_state to Some("state").
• crates/database/.sqlx/*.json: three SQLx metadata files update nullability for redirect_uri and one other column. The metadata matches the schema change.

Client SDKs

• crates/auth-sdk/src/client.rs: Client::post adds client_id to every form. This includes confidential-client requests that use HTTP Basic. Previously only public-client forms included it.
• packages/auth/src/client.ts: Client.#post adds client_id to the form for public and confidential clients. Public-client form behavior does not change.
• packages/auth/src/protocol.ts: ErrorCode adds "unauthorized_client", "unsupported_response_type", and "insufficient_scope". The open-ended string branch is unchanged.
• packages/auth/test/client.test.ts: the confidential-client refresh test expects client_id: 'notes-web' in the token request form. The public-client test title changes to "with its ID alone."

Web and UI

• crates/ui/src/oidc.rs: the InvalidRequest failure message no longer names a sign-in request. UnauthorizedClient, InsufficientScope, and UnsupportedResponseType use the generic request-failure message.
• apps/auth-service/src/web/auth/login/tests.rs: the expired or invalid sign-in message assertion changes from "This sign-in request is invalid or expired." to "This request is invalid or expired."

Tests

• apps/auth-service/src/oidc/tests/errors.rs (new, included through tests/mod.rs): six ignored, database-dependent async tests. They cover:
  • unsupported and missing response types
  • optional and oversized state
  • pushed-request parameter handling
  • token endpoint authentication and malformed grants
  • UserInfo scope enforcement
• apps/auth-service/src/oidc/tests/authorization.rs: two consent-flow tests expect FORBIDDEN instead of UNAUTHORIZED.
• apps/auth-service/src/oidc/tests/protocol.rs: a tokenless UserInfo request expects 401 with WWW-Authenticate: Bearer. The previous challenge included error="invalid_token".
• apps/auth-service/src/oidc/tests/pushed.rs:
  • An unregistered client's push request expects 400 with invalid_client.
  • An unclaimable request_uri checks the rendered error page for either of two messages.
  • A plain authorization request from a client that requires PAR expects unauthorized_client.
• apps/auth-service/src/oidc/tests/tokens.rs: an unauthenticated authorization-code exchange expects BAD_REQUEST. The test notes that RFC 6749 §5.2 does not require a challenge here.
• apps/auth-service/src/clients/tests/registry.rs: the authenticates helper parses the OAuth error response from the token endpoint. A client counts as authenticated unless its error code is InvalidClient. The previous check rejected only HTTP 401. The tests also use Axum's bounded to_bytes reader.

Dependencies

• apps/auth-service/Cargo.toml: serde_urlencoded moves from [dev-dependencies] to [dependencies].

Documentation

• docs/dev/architecture/oidc.md: the architecture table documents oidc/error.rs. The authorization flow section describes the two-stage PAR path and the bounded state. It also covers unsupported_response_type and unauthorized_client.
• docs/user/apps/troubleshooting.md:
  • Redirect-URI errors include iss before state when present.
  • A separate state limit of 512 bytes is documented.
  • Guidance covers missing client_id in pushed requests, missing code or refresh_token, and HTTP Basic responses (400 and 401).
• docs/user/reference/oidc.md: the PAR section requires client_id even with HTTP Basic. It also adds an Errors section. The section covers error codes, local versus redirected errors, iss, conditional state, the 512-byte limit, endpoint-specific invalid_client responses, UserInfo bearer errors, and no-cache token headers.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The changes update OIDC authorization parsing, optional state, pushed-request requirements, client authentication, and OAuth error responses. They also update token and UserInfo behavior, SDK request forms, persistence, tests, and protocol documentation.

Changes: OIDC protocol behavior

Layer / File(s)	Summary
OAuth error and challenge handling crates/common/src/protocol/oauth.rs, crates/auth-service/src/oidc/error.rs, crates/auth-service/src/oidc/mod.rs, crates/ui/src/oidc.rs, packages/auth/src/protocol.ts, apps/auth-service/src/clients/tests/registry.rs, apps/auth-service/src/oidc/tests/*, apps/auth-service/src/web/auth/login/tests.rs, docs/dev/architecture/oidc.md, docs/user/apps/troubleshooting.md	OAuth error codes now include unauthorized_client, insufficient_scope, and unsupported_response_type. Protocol errors can carry Basic or Bearer challenges, and authentication errors use a Basic challenge when the request includes an Authorization header. Related status expectations and UI messages are updated.
Authorization parsing and optional state apps/auth-service/Cargo.toml, apps/auth-service/src/oidc/authorize/*, apps/auth-service/src/oidc/inline.rs, apps/auth-service/src/oidc/mod.rs, crates/database/migrations/0049_optional_state.sql, crates/database/src/repositories/oidc/*, crates/database/.sqlx/*, crates/auth-sdk/src/client.rs, packages/auth/src/client.ts, packages/auth/test/client.test.ts, apps/auth-service/src/oidc/tests/errors.rs, apps/auth-service/src/oidc/tests/pushed.rs, docs/dev/architecture/oidc.md, docs/user/apps/troubleshooting.md, docs/user/reference/oidc.md	Authorization reads request_uri and optional client_id before parsing other parameters. The flow supports missing state, limits supplied state to 512 bytes, and requires client_id for pushed requests. SDK clients include client_id in confidential-client forms.
Token and UserInfo responses apps/auth-service/src/oidc/token.rs, apps/auth-service/src/oidc/tests/*, docs/user/reference/oidc.md	Token responses add Pragma: no-cache, and missing grant parameters return invalid_request. UserInfo applies Bearer challenges to missing or rejected tokens and returns insufficient_scope when a token lacks openid.

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant authorize
  participant start_authorization
  participant AuthorizationRequest
  authorize->>start_authorization: raw form request
  start_authorization->>AuthorizationRequest: read PushedReference
  AuthorizationRequest-->>start_authorization: client_id and request_uri
  alt request_uri is present
    start_authorization->>start_authorization: claim pushed request with client_id
  else request_uri is absent
    start_authorization->>AuthorizationRequest: deserialize full authorization parameters
  end
  start_authorization-->>authorize: authorization URL

Loading

Merge Risk: Moderate · up to 74b59

The OAuth error and state changes look consistent with their stated intent, but the required conformance CI job fails at this head. Confirm whether the mismatches are only the expected logout outcomes and update the expected-results list, or fix the cause, before merging.

Pre-merge checks: 3 · 1

Failed checks: 1 inconclusive

Check name	Status	Explanation	Resolution
Linked Issues check		Issue #275 has coding objectives, and the current PR addresses most of them. Evidence is the PR summary, the head doc at docs/user/reference/oidc.md, and the attested stack summaries. Stack status: PR…	Provide the OP basic conformance suite result for the head commit 74b5994, or confirm that the conformance check is not required for this assessment. Provide a review of PR #521 (head b7aecc2…

Passed checks: 3 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the fix and its scope: OAuth error codes, HTTP statuses, and challenges aligned with the RFCs. The changeset covers all three: new ErrorCode variants (unauthorized_client, insufficient…
Out of Scope Changes check		Each change in #522 maps to an issue #275 objective or to documentation for that objective: - Error code additions in crates/common and packages/auth/src/protocol.ts: directly required. - Migration 0…

Full details: Title check

Explanation

The title names the fix and its scope: OAuth error codes, HTTP statuses, and challenges aligned with the RFCs. The changeset covers all three: new ErrorCode variants (unauthorized_client, insufficient_scope, unsupported_response_type), status changes to 401/403/400 handling, and WWW-Authenticate challenge logic in oidc/error.rs. The title does not mention the optional state, PAR handling, or Pragma header, but it does not need to cover every detail. It is a single, specific sentence that a teammate scanning history would understand.

Full details: Out of Scope Changes check

Explanation

Each change in #522 maps to an issue #275 objective or to documentation for that objective:

• Error code additions in crates/common and packages/auth/src/protocol.ts: directly required.
• Migration 0049, .sqlx metadata, and database repository Option<String> changes for client_state: required for optional state.
• oidc/error.rs, oidc/mod.rs, oidc/inline.rs, and token.rs: required for challenges, statuses, Pragma, and optional state.
• SDK changes (crates/auth-sdk/src/client.rs, packages/auth/src/client.ts) that always send client_id: required by the PAR client_id objective. Their test updates (packages/auth/test/client.test.ts) support the same change.
• authorize/mod.rs, parameters.rs, pushed.rs, consent.rs: required for parse order, optional state, and unauthorized_client.
• Test updates in oidc/tests/authorization.rs, protocol.rs, pushed.rs, tokens.rs, registry.rs: expectations that change with the new statuses and codes.
• web/auth/login/tests.rs and crates/ui/src/oidc.rs message text: the invalid_request message is changed to a generic form. This follows the change to InvalidRequest handling. It is a small user-visible text change. It is connected to the objective, not unrelated.
• docs/dev/architecture/oidc.md, docs/user/apps/troubleshooting.md, docs/user/reference/oidc.md: documentation of the changed behaviour.

No change was found that lacks a connection to the issue objectives.

Full details: Linked Issues check

Explanation

Issue #275 has coding objectives, and the current PR addresses most of them. Evidence is the PR summary, the head doc at docs/user/reference/oidc.md, and the attested stack summaries. Stack status: PR #522 (current) is reviewed. PR #521 (base of this PR, the merge base is its head) is uncertain. PR #523 (downstream) is uncertain. Objectives by status:

• [#275] Add and use unsupported_response_type, unauthorized_client, insufficient_scope in ErrorCode: Addressed by #522 (crates/common/src/protocol/oauth.rs; authorize/parameters.rs; authorize/mod.rs; token.rs for insufficient_scope 403).
• [#275] invalid_request for missing code, refresh_token: Addressed by #522 (token.rs).
• [#275] state optional end to end (nullable column, migration 0049, repositories/oidc, inline.rs, mod.rs): Addressed by #522.
• [#275] Oversized state checked before redirect: Addressed by #522 (parameters.rs, authorize/mod.rs; 512-byte limit documented in docs/user/reference/oidc.md).
• [#275] Basic challenge only when Basic was used; 401 responses carry a challenge: Addressed by #522 (oidc/error.rs, oidc/mod.rs). Unchallenged rejections are 400 or 403 by design, as documented.
• [#275] Pragma: no-cache on token responses: Addressed by #522 (token.rs).
• [#275] PAR path chosen before other fields parsed; client_id required in PAR even with Basic: Addressed by #522 (authorize/mod.rs, pushed.rs, crates/auth-sdk/src/client.rs, packages/auth/src/client.ts).
• [#275] Errors before redirect verification render the HTML error page: Addressed by existing behaviour. The reference doc at head states that until the client and redirect URI are checked, errors appear on the auth error page. The PR cites #518 as the source. This PR does not change that path.
• [#275] Test per case: Addressed in part by #522 (oidc/tests/errors.rs, plus updated expectations). The six new tests are #[ignore]d database-dependent tests. Their pass status is not shown.
• [#275] Conformance suite OP basic plan passes these checks: Uncertain. The PR states it does not report a conformance result. No evidence is available.

Stack uncertainty: #521 and #523 have no current review evidence. This PR is stated to be stacked on #521. No objective was shown to depend on #521 code, but this cannot be excluded from the available evidence.

Resolution

Provide the OP basic conformance suite result for the head commit 74b5994, or confirm that the conformance check is not required for this assessment. Provide a review of PR #521 (head b7aecc2) that confirms no required objective depends on code outside this PR. Provide a test run result for oidc/tests/errors.rs, or confirm that the ignored database tests were run.

Finishing Touches

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit reads the query’s trail,
And skips the fields beyond the veil.
If state is gone, no blank string grows,
A challenge comes when auth header shows.
Then tokens rest beneath no-cache,
While errors find their proper place.}

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 905e865e-bf79-4e2f-90c9-3e704efd7ee8

Commits

Reviewing files that changed from the base of the PR and between b7aecc2 and 74b5994.

Files selected for processing: 32

• apps/auth-service/Cargo.toml
• apps/auth-service/src/clients/tests/registry.rs
• apps/auth-service/src/oidc/authorize/consent.rs
• apps/auth-service/src/oidc/authorize/mod.rs
• apps/auth-service/src/oidc/authorize/parameters.rs
• apps/auth-service/src/oidc/authorize/pushed.rs
• apps/auth-service/src/oidc/error.rs
• apps/auth-service/src/oidc/inline.rs
• apps/auth-service/src/oidc/mod.rs
• apps/auth-service/src/oidc/tests/authorization.rs
• apps/auth-service/src/oidc/tests/errors.rs
• apps/auth-service/src/oidc/tests/mod.rs
• apps/auth-service/src/oidc/tests/protocol.rs
• apps/auth-service/src/oidc/tests/pushed.rs
• apps/auth-service/src/oidc/tests/tokens.rs
• apps/auth-service/src/oidc/token.rs
• apps/auth-service/src/web/auth/login/tests.rs
• crates/auth-sdk/src/client.rs
• crates/common/src/protocol/oauth.rs
• crates/database/.sqlx/query-42ac9c52f8c14b06434dccc3564591b2c73a8adbaf5bc25db5fe0afa34995bf6.json
• crates/database/.sqlx/query-b55c00b8d90ca13fe21b71bd171bf41a8ac9169e67ddb23346b5c9d285849012.json
• crates/database/.sqlx/query-f2738a32dac0d1b49340ecbc82b2d0a4fc9ddf1b34b5b1003ed84f08d73156ac.json
• crates/database/migrations/0049_optional_state.sql
• crates/database/src/repositories/oidc/mod.rs
• crates/database/src/repositories/oidc/tests.rs
• crates/ui/src/oidc.rs
• docs/dev/architecture/oidc.md
• docs/user/apps/troubleshooting.md
• docs/user/reference/oidc.md
• packages/auth/src/client.ts
• packages/auth/src/protocol.ts
• packages/auth/test/client.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.