diff --git a/backend/crates/providers/openai/src/admin.rs b/backend/crates/providers/openai/src/admin.rs index d487de312..3196a2958 100644 --- a/backend/crates/providers/openai/src/admin.rs +++ b/backend/crates/providers/openai/src/admin.rs @@ -250,11 +250,22 @@ impl ProviderAdmin for OpenAiAdminProvider { command.document.into_provider_data().into_inner(), )) .await + .inspect_err(|error| { + log_import_failure("prepare_document", credential_admin_error_code(*error)); + }) .map_err(map_credential_admin_error)?; let observed_at = Utc::now(); let mut credentials = Vec::with_capacity(prepared.accounts().len()); for account in prepared.into_accounts() { - let account = self.preserve_existing_installation_id(account).await?; + let account = self + .preserve_existing_installation_id(account) + .await + .inspect_err(|error| { + log_import_failure( + "preserve_installation_id", + provider_admin_error_code(error.kind()), + ); + })?; credentials.push(prepared_create(account, observed_at)?); } Ok(PreparedCredentialImport { @@ -286,6 +297,7 @@ impl ProviderAdmin for OpenAiAdminProvider { &self, command: CompleteAuthorization, ) -> Result { + let request_id = command.context.request_id.clone(); let binding = AuthorizationOwnerBinding::from_context(&command.context); let completed = self .oauth @@ -295,6 +307,14 @@ impl ProviderAdmin for OpenAiAdminProvider { callback_url: SecretString::from(command.callback_url), }) .await + .inspect_err(|error| { + tracing::warn!( + request_id = %request_id, + oauth_stage = "complete_authorization", + oauth_error = oauth_error_code(error), + "OpenAI OAuth authorization completion failed" + ); + }) .map_err(map_oauth_error)?; let credential = match completed.credential { CompletedCodexOAuthCredential::Create(credential) => { @@ -1211,6 +1231,44 @@ fn map_credential_admin_error(error: CodexCredentialAdminError) -> ProviderAdmin }) } +const fn credential_admin_error_code(error: CodexCredentialAdminError) -> &'static str { + match error { + CodexCredentialAdminError::InvalidInput => "invalid_input", + CodexCredentialAdminError::IdentityMismatch => "identity_mismatch", + CodexCredentialAdminError::InvalidCredential => "invalid_credential", + CodexCredentialAdminError::NotFound => "not_found", + CodexCredentialAdminError::RevisionConflict => "revision_conflict", + CodexCredentialAdminError::StoreUnavailable => "store_unavailable", + CodexCredentialAdminError::MissingRefreshToken => "missing_refresh_token", + CodexCredentialAdminError::RefreshLeaseUnavailable => "refresh_lease_unavailable", + CodexCredentialAdminError::RefreshRejected => "refresh_rejected", + CodexCredentialAdminError::AccountBanned => "account_banned", + CodexCredentialAdminError::RefreshUnavailable => "refresh_unavailable", + CodexCredentialAdminError::RefreshAmbiguous => "refresh_ambiguous", + CodexCredentialAdminError::IdentityRejected => "identity_rejected", + CodexCredentialAdminError::IdentityUnavailable => "identity_unavailable", + } +} + +const fn provider_admin_error_code(kind: ProviderAdminErrorKind) -> &'static str { + match kind { + ProviderAdminErrorKind::Invalid => "invalid", + ProviderAdminErrorKind::Unsupported => "unsupported", + ProviderAdminErrorKind::NotFound => "not_found", + ProviderAdminErrorKind::Conflict => "conflict", + ProviderAdminErrorKind::Unavailable => "unavailable", + ProviderAdminErrorKind::Internal => "internal", + } +} + +fn log_import_failure(stage: &'static str, error: &'static str) { + tracing::warn!( + import_stage = stage, + import_error = error, + "OpenAI credential import preparation failed" + ); +} + fn map_oauth_error(error: CodexOAuthAdminError) -> ProviderAdminError { use CodexOAuthAdminError as Error; provider_admin_error(match error { @@ -1225,6 +1283,20 @@ fn map_oauth_error(error: CodexOAuthAdminError) -> ProviderAdminError { }) } +const fn oauth_error_code(error: &CodexOAuthAdminError) -> &'static str { + match error { + CodexOAuthAdminError::InvalidInput => "invalid_input", + CodexOAuthAdminError::NotFound => "not_found", + CodexOAuthAdminError::Conflict => "conflict", + CodexOAuthAdminError::FlowExpired => "flow_expired", + CodexOAuthAdminError::UpstreamRejected => "upstream_rejected", + CodexOAuthAdminError::UpstreamUnavailable => "upstream_unavailable", + CodexOAuthAdminError::Ambiguous => "ambiguous", + CodexOAuthAdminError::StorageUnavailable => "storage_unavailable", + CodexOAuthAdminError::Credential => "credential", + } +} + fn map_quota_error(error: CodexCredentialQuotaError) -> ProviderAdminError { use CodexCredentialQuotaError as Error; provider_admin_error(match error { diff --git a/backend/crates/providers/openai/src/credential/oauth.rs b/backend/crates/providers/openai/src/credential/oauth.rs index abcbe9964..d2e283962 100644 --- a/backend/crates/providers/openai/src/credential/oauth.rs +++ b/backend/crates/providers/openai/src/credential/oauth.rs @@ -635,9 +635,19 @@ fn callback_parts(value: &str) -> Result<(SecretString, SecretString), CodexOAut let mut state = None; for (key, value) in url.query_pairs() { match key.as_ref() { - "code" if code.is_none() => code = Some(value.into_owned()), - "state" if state.is_none() => state = Some(value.into_owned()), - _ => return Err(CodexOAuthAdminError::UpstreamRejected), + "code" => { + if code.replace(value.into_owned()).is_some() { + return Err(CodexOAuthAdminError::UpstreamRejected); + } + } + "state" => { + if state.replace(value.into_owned()).is_some() { + return Err(CodexOAuthAdminError::UpstreamRejected); + } + } + // OAuth 服务可能附加 `scope`、`iss` 等标准参数。 + // 固定回调地址与唯一的 `code`/`state` 才是安全边界，无关参数不参与校验。 + _ => {} } } let code = code.filter(|value| valid_secret(value)); @@ -718,8 +728,11 @@ fn map_exchange_error(error: AuthorizationCodeExchangeError) -> CodexOAuthAdminE } } -fn map_identity_error(_: CodexIdentityVerificationError) -> CodexOAuthAdminError { - CodexOAuthAdminError::UpstreamRejected +fn map_identity_error(error: CodexIdentityVerificationError) -> CodexOAuthAdminError { + match error { + CodexIdentityVerificationError::Rejected => CodexOAuthAdminError::UpstreamRejected, + CodexIdentityVerificationError::Unavailable => CodexOAuthAdminError::UpstreamUnavailable, + } } fn map_admin_error(_: CodexCredentialAdminError) -> CodexOAuthAdminError { diff --git a/backend/crates/providers/openai/tests/credential/oauth.rs b/backend/crates/providers/openai/tests/credential/oauth.rs index 9661fd44b..b83ae1da3 100644 --- a/backend/crates/providers/openai/tests/credential/oauth.rs +++ b/backend/crates/providers/openai/tests/credential/oauth.rs @@ -265,6 +265,129 @@ async fn completion_returns_verified_core_account_without_writing_store() { ); } +#[tokio::test] +async fn completion_accepts_unrelated_oauth_callback_parameters() { + let service = service(); + let started = started(&service).await; + let state = Url::parse(&started.authorization_url) + .expect("authorization URL") + .query_pairs() + .find_map(|(key, value)| (key == "state").then(|| value.into_owned())) + .expect("state"); + + let prepared = service + .complete_authorization(CompleteCodexOAuthAuthorization { + owner_ref: owner_ref(), + flow_id: started.flow_id, + callback_url: SecretString::from(format!( + "http://localhost:1455/auth/callback?code=authorization-code&state={state}&scope=openid%20profile&iss=https%3A%2F%2Fauth.openai.com" + )), + }) + .await + .expect("complete OAuth with standard callback parameters"); + + assert!(matches!( + prepared.credential, + CompletedCodexOAuthCredential::Create(_) + )); +} + +#[tokio::test] +async fn completion_rejects_repeated_code_callback_parameter() { + let service = service(); + let started = started(&service).await; + let state = Url::parse(&started.authorization_url) + .expect("authorization URL") + .query_pairs() + .find_map(|(key, value)| (key == "state").then(|| value.into_owned())) + .expect("state"); + + let error = service + .complete_authorization(CompleteCodexOAuthAuthorization { + owner_ref: owner_ref(), + flow_id: started.flow_id, + callback_url: SecretString::from(format!( + "http://localhost:1455/auth/callback?code=authorization-code&code=another-code&state={state}" + )), + }) + .await + .expect_err("repeated code"); + + assert_eq!(error, CodexOAuthAdminError::UpstreamRejected); +} + +#[tokio::test] +async fn completion_rejects_repeated_state_callback_parameter() { + let service = service(); + let started = started(&service).await; + let state = Url::parse(&started.authorization_url) + .expect("authorization URL") + .query_pairs() + .find_map(|(key, value)| (key == "state").then(|| value.into_owned())) + .expect("state"); + + let error = service + .complete_authorization(CompleteCodexOAuthAuthorization { + owner_ref: owner_ref(), + flow_id: started.flow_id, + callback_url: SecretString::from(format!( + "http://localhost:1455/auth/callback?code=authorization-code&state={state}&state=another-state-value" + )), + }) + .await + .expect_err("repeated state"); + + assert_eq!(error, CodexOAuthAdminError::UpstreamRejected); +} + +struct UnavailableVerifier; + +#[async_trait] +impl CodexAccountIdentityVerifier for UnavailableVerifier { + async fn verify( + &self, + _: &CodexOAuthSecret, + _: &CodexIdentityExpectation, + ) -> Result { + Err(CodexIdentityVerificationError::Unavailable) + } + + async fn verify_authorization( + &self, + _: &CodexOAuthSecret, + _: &SecretString, + _: &SecretString, + _: &CodexIdentityExpectation, + ) -> Result { + Err(CodexIdentityVerificationError::Unavailable) + } +} + +#[tokio::test] +async fn completion_classifies_unavailable_identity_verification_as_upstream_unavailable() { + let store = Arc::new(MemoryAccountStore::default()); + let service = service_with_store(store, Arc::new(UnavailableVerifier)); + let started = started(&service).await; + let state = Url::parse(&started.authorization_url) + .expect("authorization URL") + .query_pairs() + .find_map(|(key, value)| (key == "state").then(|| value.into_owned())) + .expect("state"); + + let error = service + .complete_authorization(CompleteCodexOAuthAuthorization { + owner_ref: owner_ref(), + flow_id: started.flow_id, + callback_url: SecretString::from(format!( + "http://localhost:1455/auth/callback?code=authorization-code&state={state}" + )), + }) + .await + .expect_err("identity verification unavailable"); + + assert_eq!(error, CodexOAuthAdminError::UpstreamUnavailable); +} + #[tokio::test] async fn callback_state_mismatch_fails_closed_and_consumes_flow() { let service = service();