From cc30cc44cb33cc6f0e2a061b79090c13c413ed60 Mon Sep 17 00:00:00 2001 From: Trezy Date: Sun, 12 Apr 2026 09:36:01 -0500 Subject: [PATCH] fix: switch from /oauth/client-metadata.json to /oauth-client-metadata.json --- docs/reference/changelog.md | 2 +- src/auth/routes.rs | 2 +- src/main.rs | 22 +++++----- src/server.rs | 21 +++------ tests/e2e_settings.rs | 44 ++++--------------- web/src/app/dashboard/settings/oauth/page.tsx | 1 - 6 files changed, 28 insertions(+), 64 deletions(-) diff --git a/docs/reference/changelog.md b/docs/reference/changelog.md index f94f356..a6dfb1c 100644 --- a/docs/reference/changelog.md +++ b/docs/reference/changelog.md @@ -4,7 +4,7 @@ - **Built-in OAuth** — replaced external AIP OAuth dependency with native `atrium-oauth` integration; HappyView manages the full OAuth flow internally - **Instance settings** — new `instance_settings` key/value table for configurable instance metadata (app name, logo, ToS, privacy policy) with env var fallback -- **OAuth branding** — authorization screens now show configurable app name, logo, terms of service, and privacy policy links via the `/oauth/client-metadata.json` endpoint +- **OAuth branding** — authorization screens now show configurable app name, logo, terms of service, and privacy policy links via the `/oauth-client-metadata.json` endpoint - **Logo upload** — upload a logo image via `PUT /admin/settings/logo` (stored in DB, served at `GET /settings/logo`) - **`settings:manage` permission** — new permission for managing instance settings, included in Manager and Full Access templates - **Redirect URI support** — `/auth/login` accepts optional `redirect_uri` parameter for post-login navigation diff --git a/src/auth/routes.rs b/src/auth/routes.rs index a0202cb..e5de216 100644 --- a/src/auth/routes.rs +++ b/src/auth/routes.rs @@ -60,7 +60,7 @@ async fn login( tracing::debug!(handle = %query.handle, redirect_uri = ?query.redirect_uri, "login request"); // Read the configured scopes from the settings DB. Falls back to `atproto` - // only if unset — that matches what we serve from `/oauth/client-metadata.json`. + // only if unset — that matches what we serve from `/oauth-client-metadata.json`. let scopes = match crate::admin::settings::get_setting( &state.db, "oauth_scopes", diff --git a/src/main.rs b/src/main.rs index a196bb7..5890403 100644 --- a/src/main.rs +++ b/src/main.rs @@ -297,12 +297,18 @@ async fn main() { let oauth_state_store = DbStateStore::new(db_pool.clone(), db_backend); - // Load OAuth scopes from the settings DB. Falls back to just `atproto` if unset. - // The live `/oauth/client-metadata.json` endpoint also reads this setting at - // request time, so non-loopback deployments pick up changes without a restart. + // Load OAuth scopes from the settings DB so they can be managed at runtime + // without restarting HappyView. Falls back to just `atproto` if unset. let oauth_scopes = match happyview::admin::settings::get_setting(&db_pool, "oauth_scopes", db_backend).await { - Some(s) => happyview::auth::parse_scope_string(&s), + Some(s) => { + let parsed = happyview::auth::parse_scope_string(&s); + if parsed.is_empty() { + vec![Scope::Known(KnownScope::Atproto)] + } else { + parsed + } + } None => vec![Scope::Known(KnownScope::Atproto)], }; @@ -311,7 +317,7 @@ async fn main() { atrium_oauth::OAuthClient::new(OAuthClientConfig { client_metadata: AtprotoLocalhostClientMetadata { redirect_uris: Some(vec![callback_url]), - scopes: Some(oauth_scopes.clone()), + scopes: Some(oauth_scopes), }, keys: None, state_store: oauth_state_store.clone(), @@ -320,14 +326,10 @@ async fn main() { }) .expect("Failed to create OAuth client") } else { - // For non-loopback, `client_id` is a URL to `/oauth/client-metadata.json` which - // the authorization server fetches at runtime. That endpoint reads scopes from - // the DB, so the value we put here only matters for the initial validity check - // (which requires `atproto` to be present). atrium_oauth::OAuthClient::new(OAuthClientConfig { client_metadata: AtprotoClientMetadata { client_id: format!( - "{}/oauth/client-metadata.json", + "{}/oauth-client-metadata.json", config.public_url.trim_end_matches('/') ), client_uri: Some(config.public_url.clone()), diff --git a/src/server.rs b/src/server.rs index 78a9dec..738a90b 100644 --- a/src/server.rs +++ b/src/server.rs @@ -1,4 +1,4 @@ -use axum::extract::{DefaultBodyLimit, OriginalUri, State}; +use axum::extract::{DefaultBodyLimit, State}; use axum::http::HeaderMap; use axum::http::{Method, header}; use axum::response::{IntoResponse, Response}; @@ -68,12 +68,7 @@ pub fn router(state: AppState) -> Router { .nest("/admin", admin::admin_routes(state.clone())) .nest("/auth", crate::auth::routes::routes()) .nest("/external-auth", crate::external_auth::routes()) - // The ATProto OAuth spec allows either filename convention for the - // client metadata document. We serve both so deployments can opt into - // whichever URL their client_id points at. - // // https://atproto.com/specs/oauth#types-of-clients - .route("/oauth/client-metadata.json", get(client_metadata)) .route("/oauth-client-metadata.json", get(client_metadata)) .route("/xrpc/app.bsky.actor.getProfile", get(get_profile)) .route( @@ -103,20 +98,14 @@ async fn config_endpoint(State(state): State) -> Json, - OriginalUri(uri): OriginalUri, -) -> Json { +async fn client_metadata(State(state): State) -> Json { let mut metadata = serde_json::to_value(&state.oauth.client_metadata).unwrap_or_default(); // The `client_id` field in the response must exactly match the URL the - // authorization server fetched. Construct it from the public URL + this - // request's path so both `/oauth/client-metadata.json` and - // `/oauth-client-metadata.json` work correctly. + // authorization server fetched. let client_id = format!( - "{}{}", - state.config.public_url.trim_end_matches('/'), - uri.path() + "{}/oauth-client-metadata.json", + state.config.public_url.trim_end_matches('/') ); metadata["client_id"] = serde_json::Value::String(client_id); diff --git a/tests/e2e_settings.rs b/tests/e2e_settings.rs index 9490c7e..030f1b9 100644 --- a/tests/e2e_settings.rs +++ b/tests/e2e_settings.rs @@ -282,13 +282,13 @@ async fn client_metadata_includes_settings() { resp.status() ); - // GET /oauth/client-metadata.json (no auth) and verify client_name + // GET /oauth-client-metadata.json (no auth) and verify client_name let resp = app .router .clone() .oneshot( Request::builder() - .uri("/oauth/client-metadata.json") + .uri("/oauth-client-metadata.json") .body(Body::empty()) .unwrap(), ) @@ -307,30 +307,9 @@ async fn client_metadata_includes_settings() { #[tokio::test] #[serial] #[ignore] -async fn client_metadata_dual_route_client_id_matches_path() { +async fn client_metadata_client_id_matches_path() { let app = TestApp::new().await; - // Fetch the classic path - let resp = app - .router - .clone() - .oneshot( - Request::builder() - .uri("/oauth/client-metadata.json") - .body(Body::empty()) - .unwrap(), - ) - .await - .unwrap(); - assert_eq!(resp.status(), StatusCode::OK); - let classic = json_body(resp).await; - let classic_client_id = classic["client_id"].as_str().expect("client_id missing"); - assert!( - classic_client_id.ends_with("/oauth/client-metadata.json"), - "client_id should match requested path, got {classic_client_id}" - ); - - // Fetch the alternate path — must succeed and mirror its own URL let resp = app .router .clone() @@ -343,16 +322,11 @@ async fn client_metadata_dual_route_client_id_matches_path() { .await .unwrap(); assert_eq!(resp.status(), StatusCode::OK); - let alt = json_body(resp).await; - let alt_client_id = alt["client_id"].as_str().expect("client_id missing"); + let json = json_body(resp).await; + let client_id = json["client_id"].as_str().expect("client_id missing"); assert!( - alt_client_id.ends_with("/oauth-client-metadata.json"), - "client_id should match requested path, got {alt_client_id}" - ); - - assert_ne!( - classic_client_id, alt_client_id, - "dual routes must produce distinct client_id values" + client_id.ends_with("/oauth-client-metadata.json"), + "client_id should end with /oauth-client-metadata.json, got {client_id}" ); } @@ -379,7 +353,7 @@ async fn client_metadata_scope_overridden_by_setting() { .clone() .oneshot( Request::builder() - .uri("/oauth/client-metadata.json") + .uri("/oauth-client-metadata.json") .body(Body::empty()) .unwrap(), ) @@ -417,7 +391,7 @@ async fn client_metadata_client_uri_overridden_by_setting() { .clone() .oneshot( Request::builder() - .uri("/oauth/client-metadata.json") + .uri("/oauth-client-metadata.json") .body(Body::empty()) .unwrap(), ) diff --git a/web/src/app/dashboard/settings/oauth/page.tsx b/web/src/app/dashboard/settings/oauth/page.tsx index 8290179..13c0763 100644 --- a/web/src/app/dashboard/settings/oauth/page.tsx +++ b/web/src/app/dashboard/settings/oauth/page.tsx @@ -203,7 +203,6 @@ export default function OAuthSettingsPage() {

Client Metadata

These values are served from{" "} - /oauth/client-metadata.json and{" "} /oauth-client-metadata.json and shown on the OAuth consent screen.

-- 2.51.2