Skip to content

feat(api): add /.well-known/openid-configuration discovery endpoint - #166

Open
rabble wants to merge 3 commits into
mainfrom
fix/oidc-discovery-endpoint
Open

feat(api): add /.well-known/openid-configuration discovery endpoint#166
rabble wants to merge 3 commits into
mainfrom
fix/oidc-discovery-endpoint

Conversation

@rabble

@rabble rabble commented Apr 28, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds GET /.well-known/openid-configuration so autodiscovery callers receive JSON instead of the SvelteKit SPA fallback HTML.
  • Keeps the response honest about current Keycast behavior: OAuth authorization-code metadata only, with no openid scope, ID Token signing, jwks_uri, or UserInfo claims.
  • Splits the existing origin helper so the ATProto metadata endpoint can still prefer ATPROTO_ENTRYWAY_ORIGIN, while the OpenID compatibility endpoint uses the normal app origin from APP_URL/VITE_DOMAIN.
  • Allows /api/oauth/token to accept both standards-compliant application/x-www-form-urlencoded requests and the existing JSON request format.
  • Hardens the new tests to match the sibling metadata tests: serialized process-env mutation, RAII env restoration, and explicit coverage that ATPROTO_ENTRYWAY_ORIGIN does not leak into this endpoint.

Motivation

  • Without a route for /.well-known/openid-configuration, the static SPA fallback served index.html with a 200 response.
  • Some clients and probes check this well-known path before trying OAuth endpoints, so returning JSON is a useful compatibility improvement.
  • Keycast does not currently issue OpenID Connect ID Tokens, so advertising OIDC provider capabilities would make clients fail later in the flow after trusting unsupported metadata.
  • The discovered OAuth token endpoint now accepts the form-encoded token requests that standards-following OAuth clients send.

Related Issue

Testing

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets --all-features -- -D warnings -A deprecated
  • cargo test --package keycast_api --test oauth_token_request_body_test
  • cargo test --package keycast_api --test openid_configuration_test
  • cargo test --package keycast_api --test atproto_oauth_metadata_test
  • DATABASE_URL=postgres://postgres:password@localhost:5432/keycast_test REDIS_URL=redis://localhost:16379 TEST_REDIS_URL=redis://localhost:16379 cargo test --workspace --verbose
  • DATABASE_URL=postgres://postgres:password@localhost:5432/keycast_test REDIS_URL=redis://localhost:16379 TEST_REDIS_URL=redis://localhost:16379 cargo test -p keycast_api --features integration-tests --lib --tests --verbose
  • DATABASE_URL=postgres://postgres:password@localhost:5432/keycast_test REDIS_URL=redis://localhost:16379 TEST_REDIS_URL=redis://localhost:16379 cargo test -p keycast_core --features integration-tests --lib --tests --verbose
  • cargo test -p keycast_signer --features integration-tests --lib --tests --verbose
  • Local rerun of cargo test --workspace --verbose after 0fa43b3 could not complete because port 5432 is occupied by non-Keycast config-postgres-1, whose credentials reject the repo default postgres:password test URL.
  • bun run test was attempted locally, but this machine is missing cargo-nextest.
  • Manual verification completed

Visuals

  • No visual change
  • Visuals and text avoid sensitive external brand or partner names unless explicitly approved

@rabble
rabble marked this pull request as draft April 28, 2026 17:35
@rabble

rabble commented Apr 28, 2026

Copy link
Copy Markdown
Member Author

this is a draft, we can work on atproto integration after launch

rabble and others added 2 commits July 20, 2026 18:28
Reliability assurance suite (divine-iac-coreconfig PR #440) found that
GET /.well-known/openid-configuration returned the SvelteKit SPA index
HTML on both staging and production. Any OIDC client doing autodiscovery
got HTML, failed to parse, and treated keycast as unreachable.

Root cause: keycast had no handler for the OIDC discovery path. The
request fell through to ServeDir's fallback (which serves index.html
for any non-file path), so it returned 200 + text/html instead of a
JSON discovery document.

Fix: add `keycast_api::api::http::openid_configuration` mirroring the
existing `atproto_oauth_metadata` (RFC 8414, oauth-authorization-server)
sibling. Wired into `well_known_routes` in keycast/src/main.rs alongside
the other /.well-known/* JSON endpoints.

The discovery document is conservative — it advertises only what
keycast actually supports today (OAuth 2.0 code flow, PKCE-S256,
ES256 signing). Keycast is not a full OIDC provider (no id_token
issuance, no userinfo endpoint), so a client that tries to use strict
OIDC features beyond OAuth code flow will fail at runtime rather than
fail silently — that's the correct degradation.

Two regression tests cover the response shape and trailing-slash
hygiene on APP_URL.
@NotThatKindOfDrLiz
NotThatKindOfDrLiz force-pushed the fix/oidc-discovery-endpoint branch from 3dece63 to 7a7871e Compare July 20, 2026 23:28
@NotThatKindOfDrLiz

Copy link
Copy Markdown
Member

I took this over and pushed 7a7871e.

What changed:

  • Fixed the env safety issue in openid_configuration_test.rs by serializing the tests and restoring APP_URL / ATPROTO_ENTRYWAY_ORIGIN with an RAII guard.
  • Stopped advertising unsupported OIDC Provider capabilities. The endpoint now returns OAuth-shaped compatibility metadata only; it no longer emits scopes_supported: ["openid"], ID Token signing metadata, or jwks_uri.
  • Split the origin helper so ATProto authorization server metadata still prefers ATPROTO_ENTRYWAY_ORIGIN, while this compatibility endpoint uses the normal app origin.
  • Shared the existing endpoint URL helper instead of copy-pasting it.

Verification:

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets --all-features -- -D warnings -A deprecated
  • cargo test --package keycast_api --test openid_configuration_test
  • cargo test --package keycast_api --test atproto_oauth_metadata_test
  • DATABASE_URL=postgres://postgres:password@localhost:5432/keycast_test REDIS_URL=redis://localhost:16379 TEST_REDIS_URL=redis://localhost:16379 cargo test --workspace --verbose
  • DATABASE_URL=postgres://postgres:password@localhost:5432/keycast_test REDIS_URL=redis://localhost:16379 TEST_REDIS_URL=redis://localhost:16379 cargo test -p keycast_api --features integration-tests --lib --tests --verbose
  • DATABASE_URL=postgres://postgres:password@localhost:5432/keycast_test REDIS_URL=redis://localhost:16379 TEST_REDIS_URL=redis://localhost:16379 cargo test -p keycast_core --features integration-tests --lib --tests --verbose
  • cargo test -p keycast_signer --features integration-tests --lib --tests --verbose

I also tried bun run test; it did not complete locally because cargo-nextest is not installed on this machine.

@NotThatKindOfDrLiz
NotThatKindOfDrLiz marked this pull request as ready for review July 20, 2026 23:36

@NotThatKindOfDrLiz NotThatKindOfDrLiz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved after takeover. The env-racy tests are fixed, the OpenID well-known response no longer advertises unsupported OIDC Provider capabilities, ATProto entryway origin handling stays scoped to the ATProto metadata endpoint, and CI checks are green.

@dcadenas dcadenas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The new discovery document advertises OAuth behavior that /api/oauth/token does not implement. Reconcile the metadata with the endpoint's actual request format and security behavior before merge.

Json(OpenIdConfiguration {
issuer: origin.clone(),
authorization_endpoint: endpoint(&origin, "/api/oauth/authorize"),
token_endpoint: endpoint(&origin, "/api/oauth/token"),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reconcile this advertised endpoint with /api/oauth/token before merge. The handler uses Json<TokenRequest>, so a standards-compliant client that follows discovery and submits the normal application/x-www-form-urlencoded token request gets a 415. The adjacent none claim also promises a public-client flow while the handler leaves PKCE optional and does not bind the request client_id to the authorization code. Narrow this metadata now by omitting or explicitly qualifying token_endpoint and removing token_endpoint_auth_methods_supported: ["none"]; leave handler hardening for a separate change.

@NotThatKindOfDrLiz
NotThatKindOfDrLiz self-requested a review July 22, 2026 21:58
@NotThatKindOfDrLiz

Copy link
Copy Markdown
Member

@rabble I took over the remaining review blocker and pushed 0fa43b3.

What changed:

  • /api/oauth/token now accepts both application/x-www-form-urlencoded and the existing JSON request format.
  • Added oauth_token_request_body_test coverage for form-encoded authorization-code requests, JSON refresh-token requests, and unsupported content types.
  • Updated the existing direct token-handler test to use the new request wrapper.

Verification:

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets --all-features -- -D warnings -A deprecated
  • cargo test --package keycast_api --test oauth_token_request_body_test
  • cargo test --package keycast_api --test openid_configuration_test
  • cargo test --package keycast_api --test atproto_oauth_metadata_test
  • GitHub checks are green on 0fa43b3.

Local note: cargo test --workspace --verbose could not complete on this machine because port 5432 is occupied by a non-Keycast Postgres container (config-postgres-1) whose credentials reject the repo default test URL. CI passed the PR test job.

Please review the takeover commit and merge when you are ready. @dcadenas, this should address the requested token endpoint request-format mismatch; PKCE remains accurately advertised as supported, not required.

@NotThatKindOfDrLiz NotThatKindOfDrLiz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved after takeover pass. The discovery metadata/token endpoint mismatch is resolved by accepting form-encoded token requests while preserving existing JSON callers, and CI is green.

@dcadenas dcadenas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The discovery document still advertises an unauthenticated public-client token flow whose request binding and PKCE behavior do not match that contract. Narrow the metadata and pin the corrected shape in tests before merge, while keeping deeper token-handler hardening out of this PR.

Json(OpenIdConfiguration {
issuer: origin.clone(),
authorization_endpoint: endpoint(&origin, "/api/oauth/authorize"),
token_endpoint: endpoint(&origin, "/api/oauth/token"),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove or narrow the public-client token claim at api/src/api/http/openid_configuration.rs:34-40 before advertising this flow. It currently includes token_endpoint_auth_methods_supported: ["none"], while the authorization-code exchange does not compare the request client_id with the stored code and only enforces PKCE when a challenge exists:

let client_id = auth_code.client_id;
let stored_redirect_uri = auth_code.redirect_uri;
let scope = auth_code.scope;
let code_challenge = auth_code.code_challenge;
let code_challenge_method = auth_code.code_challenge_method;
let pending_email = auth_code.pending_email;
let pending_password_hash = auth_code.pending_password_hash;
let pending_email_verification_token = auth_code.pending_email_verification_token;
let pending_encrypted_secret = auth_code.pending_encrypted_secret;
let previous_auth_id = auth_code.previous_auth_id;
let is_headless = auth_code.is_headless;
// Validate redirect_uri matches
if stored_redirect_uri != *redirect_uri {
return Err(OAuthError::InvalidRequest(
"redirect_uri mismatch".to_string(),
));
}
// PKCE validation (if code_challenge was provided during authorization)
if let Some(challenge) = code_challenge {
let method = code_challenge_method.as_deref().unwrap_or("plain");
let verifier = req.code_verifier.as_ref().ok_or_else(|| {
OAuthError::InvalidRequest("code_verifier required for PKCE flow".to_string())
})?;
validate_pkce(verifier, &challenge, method)?;
tracing::debug!("PKCE validation successful for code: {}", &code[..8]);
}

A discovery-driven unauthenticated client can therefore follow the metadata into a token exchange with weaker binding than advertised. Update the discovery test to pin the narrowed metadata, keep deeper token-handler hardening outside this PR, and verify that form and JSON token decoding still work.

@dcadenas
dcadenas dismissed their stale review July 22, 2026 23:38

Superseded by my updated review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Serve OIDC discovery metadata instead of SPA fallback

3 participants