From 230d90262e5b77a2a9640603ce199e2d07fde983 Mon Sep 17 00:00:00 2001 From: lewis Date: Tue, 20 Jan 2026 19:55:47 +0200 Subject: [PATCH] fix: delegated acc passkey auth --- .../src/oauth/endpoints/authorize.rs | 105 +++++++++++-- .../src/oauth/endpoints/delegation.rs | 8 +- crates/tranquil-pds/tests/oauth_security.rs | 138 ++++++++++++++++++ frontend/src/routes/OAuthDelegation.svelte | 3 +- 4 files changed, 241 insertions(+), 13 deletions(-) diff --git a/crates/tranquil-pds/src/oauth/endpoints/authorize.rs b/crates/tranquil-pds/src/oauth/endpoints/authorize.rs index 8590813..e3f97b6 100644 --- a/crates/tranquil-pds/src/oauth/endpoints/authorize.rs +++ b/crates/tranquil-pds/src/oauth/endpoints/authorize.rs @@ -311,6 +311,17 @@ pub async fn authorize_get( if is_delegated { tracing::info!("Redirecting to delegation auth"); + if let Err(e) = state + .oauth_repo + .set_request_did(&request_id, &user.did) + .await + { + tracing::error!(error = %e, "Failed to set delegated DID on authorization request"); + return redirect_to_frontend_error( + "server_error", + "Failed to initialize delegation flow", + ); + } return redirect_see_other(&format!( "/app/oauth/delegation?request_uri={}&delegated_did={}", url_encode(&request_uri), @@ -2137,6 +2148,7 @@ pub async fn check_user_security_status( pub struct PasskeyStartInput { pub request_uri: String, pub identifier: String, + pub delegated_did: Option, } #[derive(Debug, Serialize)] @@ -2394,20 +2406,85 @@ pub async fn passkey_start( .into_response(); } - if state + let delegation_from_param = match &form.delegated_did { + Some(delegated_did_str) => { + match delegated_did_str.parse::() { + Ok(delegated_did) if delegated_did != user.did => { + match state + .delegation_repo + .get_delegation(&delegated_did, &user.did) + .await + { + Ok(Some(_)) => Some(delegated_did), + Ok(None) => None, + Err(e) => { + tracing::warn!( + error = %e, + delegated_did = %delegated_did, + controller_did = %user.did, + "Failed to verify delegation relationship" + ); + None + } + } + } + _ => None, + } + } + None => None, + }; + + let is_delegation_flow = delegation_from_param.is_some() + || request_data.did.as_ref().map_or(false, |existing_did| { + existing_did + .parse::() + .ok() + .map_or(false, |parsed| parsed != user.did) + }); + + if let Some(delegated_did) = delegation_from_param { + tracing::info!( + delegated_did = %delegated_did, + controller_did = %user.did, + "Passkey auth with delegated_did param - setting delegation flow" + ); + if state + .oauth_repo + .set_authorization_did(&passkey_start_request_id, &delegated_did, None) + .await + .is_err() + { + return OAuthError::ServerError("An error occurred.".into()).into_response(); + } + if state + .oauth_repo + .set_controller_did(&passkey_start_request_id, &user.did) + .await + .is_err() + { + return OAuthError::ServerError("An error occurred.".into()).into_response(); + } + } else if is_delegation_flow { + tracing::info!( + delegated_did = ?request_data.did, + controller_did = %user.did, + "Passkey auth in delegation flow - preserving delegated DID" + ); + if state + .oauth_repo + .set_controller_did(&passkey_start_request_id, &user.did) + .await + .is_err() + { + return OAuthError::ServerError("An error occurred.".into()).into_response(); + } + } else if state .oauth_repo .set_authorization_did(&passkey_start_request_id, &user.did, None) .await .is_err() { - return ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ - "error": "server_error", - "error_description": "An error occurred." - })), - ) - .into_response(); + return OAuthError::ServerError("An error occurred.".into()).into_response(); } let options = serde_json::to_value(&rcr).unwrap_or(serde_json::json!({})); @@ -2497,9 +2574,15 @@ pub async fn passkey_finish( } }; + let controller_did: Option = request_data + .controller_did + .as_ref() + .and_then(|s| s.parse().ok()); + let passkey_owner_did = controller_did.as_ref().unwrap_or(&did); + let auth_state_json = match state .user_repo - .load_webauthn_challenge(&did, "authentication") + .load_webauthn_challenge(passkey_owner_did, "authentication") .await { Ok(Some(s)) => s, @@ -2591,7 +2674,7 @@ pub async fn passkey_finish( if let Err(e) = state .user_repo - .delete_webauthn_challenge(&did, "authentication") + .delete_webauthn_challenge(passkey_owner_did, "authentication") .await { tracing::warn!(error = %e, "Failed to delete authentication state"); diff --git a/crates/tranquil-pds/src/oauth/endpoints/delegation.rs b/crates/tranquil-pds/src/oauth/endpoints/delegation.rs index 30a43c4..217ca1e 100644 --- a/crates/tranquil-pds/src/oauth/endpoints/delegation.rs +++ b/crates/tranquil-pds/src/oauth/endpoints/delegation.rs @@ -127,7 +127,13 @@ pub async fn delegation_auth( .await .is_err() { - tracing::warn!("Failed to set delegated DID on authorization request"); + return Json(DelegationAuthResponse { + success: false, + needs_totp: None, + redirect_uri: None, + error: Some("Failed to update authorization request".to_string()), + }) + .into_response(); } let grant = match state diff --git a/crates/tranquil-pds/tests/oauth_security.rs b/crates/tranquil-pds/tests/oauth_security.rs index a1349da..7fd85d4 100644 --- a/crates/tranquil-pds/tests/oauth_security.rs +++ b/crates/tranquil-pds/tests/oauth_security.rs @@ -1250,3 +1250,141 @@ async fn test_delegation_viewer_scope_cannot_write() { "Error should be InsufficientScope" ); } + +#[tokio::test] +async fn test_delegation_oauth_token_sub_is_delegated_account() { + let url = base_url().await; + let http_client = client(); + let suffix = &uuid::Uuid::new_v4().simple().to_string()[..8]; + + let (controller_jwt, controller_did) = create_account_and_login(&http_client).await; + + let delegated_handle = format!("dlgsub{}", suffix); + let delegated_res = http_client + .post(format!("{}/xrpc/_delegation.createDelegatedAccount", url)) + .bearer_auth(&controller_jwt) + .json(&json!({ + "handle": delegated_handle, + "controllerScopes": "atproto" + })) + .send() + .await + .unwrap(); + assert_eq!( + delegated_res.status(), + StatusCode::OK, + "Should create delegated account" + ); + let delegated_account: Value = delegated_res.json().await.unwrap(); + let delegated_did = delegated_account["did"].as_str().unwrap(); + + assert_ne!( + delegated_did, controller_did, + "Delegated DID should be different from controller DID" + ); + + let redirect_uri = "https://example.com/deleg-sub-callback"; + let mock_client = setup_mock_client_metadata(redirect_uri).await; + let client_id = mock_client.uri(); + let (code_verifier, code_challenge) = generate_pkce(); + + let par_body: Value = http_client + .post(format!("{}/oauth/par", url)) + .form(&[ + ("response_type", "code"), + ("client_id", &client_id), + ("redirect_uri", redirect_uri), + ("code_challenge", &code_challenge), + ("code_challenge_method", "S256"), + ("scope", "atproto"), + ("login_hint", delegated_did), + ]) + .send() + .await + .unwrap() + .json() + .await + .unwrap(); + let request_uri = par_body["request_uri"].as_str().unwrap(); + + let auth_res = http_client + .post(format!("{}/oauth/delegation/auth", url)) + .header("Content-Type", "application/json") + .json(&json!({ + "request_uri": request_uri, + "delegated_did": delegated_did, + "controller_did": controller_did, + "password": "Testpass123!", + "remember_device": false + })) + .send() + .await + .unwrap(); + assert_eq!( + auth_res.status(), + StatusCode::OK, + "Delegation auth should succeed" + ); + let auth_body: Value = auth_res.json().await.unwrap(); + assert!( + auth_body["success"].as_bool().unwrap_or(false), + "Delegation auth should report success: {:?}", + auth_body + ); + + let consent_res = http_client + .post(format!("{}/oauth/authorize/consent", url)) + .header("Content-Type", "application/json") + .json(&json!({ + "request_uri": request_uri, + "approved_scopes": ["atproto"], + "remember": false + })) + .send() + .await + .unwrap(); + assert_eq!( + consent_res.status(), + StatusCode::OK, + "Consent should succeed" + ); + let consent_body: Value = consent_res.json().await.unwrap(); + let redirect_location = consent_body["redirect_uri"] + .as_str() + .expect("Expected redirect_uri"); + + let code = redirect_location + .split("code=") + .nth(1) + .unwrap() + .split('&') + .next() + .unwrap(); + + let token_res = http_client + .post(format!("{}/oauth/token", url)) + .form(&[ + ("grant_type", "authorization_code"), + ("code", code), + ("redirect_uri", redirect_uri), + ("code_verifier", &code_verifier), + ("client_id", &client_id), + ]) + .send() + .await + .unwrap(); + assert_eq!(token_res.status(), StatusCode::OK, "Token exchange should succeed"); + let tokens: Value = token_res.json().await.unwrap(); + + let sub = tokens["sub"].as_str().expect("Token response should have sub claim"); + + assert_eq!( + sub, delegated_did, + "Token sub claim should be the DELEGATED account's DID, not the controller's. Got {} but expected {}", + sub, delegated_did + ); + assert_ne!( + sub, controller_did, + "Token sub claim should NOT be the controller's DID" + ); +} diff --git a/frontend/src/routes/OAuthDelegation.svelte b/frontend/src/routes/OAuthDelegation.svelte index 77d0c19..4557bfa 100644 --- a/frontend/src/routes/OAuthDelegation.svelte +++ b/frontend/src/routes/OAuthDelegation.svelte @@ -127,7 +127,8 @@ }, body: JSON.stringify({ request_uri: requestUri, - identifier: controllerIdentifier.trim().replace(/^@/, '') + identifier: controllerIdentifier.trim().replace(/^@/, ''), + delegated_did: delegatedDid }) })