fix(oidc-provider): surface federation failures as actionable errors, not opaque 500s#66
Merged
Merged
Conversation
Backend federation failures all collapsed into ProxyError::Internal, so a rejected STS exchange, a malformed signing key, or an unreachable broker each returned an opaque 500 InternalError / "Internal server error" with the real cause discarded — undiagnosable from the response and unlogged. Map each cause to a status reflecting whose problem it is, and log the full (possibly ARN-bearing) provider detail at the conversion site, where it is the last place still available and must not reach the caller: - STS rejection (InvalidIdentityToken, …) -> 502 BackendAuthenticationFailed - STS AccessDenied (trust policy/perms) -> 403 AccessDenied - bad OIDC_PROVIDER_KEY / signing failure -> 502 (logged) - broker unreachable / unparseable reply -> 503 ServiceUnavailable Adds ProxyError::BackendAuthError, which carries the provider error *code* only (safe to surface); the raw message is logged, never returned. Unit tests cover each mapping and assert no opaque 500 / no message leak. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the openssl commands for SESSION_TOKEN_KEY and OIDC_PROVIDER_KEY, and call out that OIDC_PROVIDER_KEY must be a PKCS#8 PEM RSA key (genpkey, not genrsa) — a random/symmetric value fails to parse and the worker then 500s on every request. Note preview and staging must share the same key. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
📖 Docs preview deployed to https://multistore-docs-pr-66.development-seed.workers.dev
|
|
🚀 Latest commit deployed to https://multistore-proxy-pr-66.development-seed.workers.dev
|
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Every backend-federation failure collapsed into
ProxyError::Internal, so the proxy returned a generic500 InternalError/ "Internal server error" regardless of cause, with the real reason discarded — both from the response and from logs. We hit this repeatedly while debugging thefederated-testsmoke test: a key/issuer mismatch (AWS STSInvalidIdentityToken) was indistinguishable from an actual proxy crash.Root cause (
crates/oidc-provider/src/lib.rs):Change
Map each federation failure to a status that reflects whose problem it is, and log the full (possibly ARN-bearing) provider detail at the conversion site — the last place it's available and somewhere it must not reach the caller:
InvalidIdentityToken, …)InternalErrorBackendAuthenticationFailedAccessDenied(trust policy / perms)InternalErrorAccessDeniedOIDC_PROVIDER_KEY/ signing failureInternalErrorInternalErrorServiceUnavailableAdds
ProxyError::BackendAuthError, which carries the provider error code only (e.g.InvalidIdentityToken) — safe to surface; the raw message is logged viatracing::error!, never returned. With the worker's console subscriber,wrangler tailnow shows the actual STS code + message instead of nothing.Tests
Unit tests for every mapping (
crates/oidc-provider/src/lib.rs,crates/core/src/error.rs): each asserts the expected status, that it is not an opaque 500, and that the raw provider message does not leak intosafe_message().Verified locally with the CI command set:
cargo check,cargo clippy -- -D warnings,cargo test, andcargo check -p multistore-cf-workers --target wasm32-unknown-unknown— all green.🤖 Generated with Claude Code