From 93a841d3befe727149750fc36af7071219377a31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Ren=C3=A9=20White=20Enciso?= Date: Tue, 31 Mar 2026 16:02:08 -0600 Subject: [PATCH] fix(thalos-bff): harden oidc callback redirects --- docs/api/identity-edge-api.md | 2 + docs/runbooks/containerization.md | 2 + .../Security/Oidc/GoogleOidcFlowService.cs | 17 ++++++++ src/Thalos.Bff.Rest/Program.cs | 42 ++++++++++++++++--- .../GoogleOidcFlowServiceTests.cs | 29 +++++++++++++ 5 files changed, 86 insertions(+), 6 deletions(-) diff --git a/docs/api/identity-edge-api.md b/docs/api/identity-edge-api.md index 1a503e1..fef6a78 100644 --- a/docs/api/identity-edge-api.md +++ b/docs/api/identity-edge-api.md @@ -27,6 +27,8 @@ - Session login and refresh call canonical thalos-service session gRPC operations. - OIDC start/callback handlers generate and validate PKCE/state/nonce payloads. - Session cookies are managed at the BFF edge (`thalos_session`, `thalos_refresh`) with env-driven secure/domain policy. +- Callback failures are redirected back to the central auth UX with stable `authError` and `correlationId` query values instead of returning a raw provider-facing JSON payload. +- The temporary OIDC state cookie is cleared on both callback success and callback failure paths. - Token issuance and policy evaluation contracts remain available for compatibility calls. - Business orchestration remains in thalos-service. - Identity abstractions remain owned by Thalos repositories. diff --git a/docs/runbooks/containerization.md b/docs/runbooks/containerization.md index ef192c2..4ebfad8 100644 --- a/docs/runbooks/containerization.md +++ b/docs/runbooks/containerization.md @@ -34,3 +34,5 @@ docker run --rm -p 8080:8080 \ - gRPC client contract protobuf is vendored at `src/Thalos.Bff.Rest/Protos/identity_runtime.proto` to keep image builds repo-local. - OIDC callback requires `ThalosBff__Oidc__Google__ClientId`, `ClientSecret`, `RedirectUri`, and `StateSigningSecret`. - For cross-subdomain SPA auth, set `ThalosBff__SessionCookieDomain=.dream-views.com` and secure cookies in non-local environments. +- Callback failures should land back on the central auth host (or another allowlisted return host) with `authError` and `correlationId` query values for UX recovery and support diagnostics. +- The OIDC state cookie is transient and should be cleared after any callback attempt, successful or failed. diff --git a/src/Thalos.Bff.Application/Security/Oidc/GoogleOidcFlowService.cs b/src/Thalos.Bff.Application/Security/Oidc/GoogleOidcFlowService.cs index 2f598c2..93ee249 100644 --- a/src/Thalos.Bff.Application/Security/Oidc/GoogleOidcFlowService.cs +++ b/src/Thalos.Bff.Application/Security/Oidc/GoogleOidcFlowService.cs @@ -98,6 +98,23 @@ public sealed class GoogleOidcFlowService( return options.DefaultReturnUrl; } + /// + /// Builds a safe callback-failure redirect back to the central auth experience or an allowed caller return URL. + /// + /// Optional caller return URL that must pass the same host allowlist policy as start flow. + /// Stable auth error code for UI handling. + /// Correlation identifier for support/troubleshooting. + /// Safe redirect URL with error context query values. + public string BuildFailureRedirectUrl(string? requestedReturnUrl, string errorCode, string correlationId) + { + var redirectBaseUrl = ResolveReturnUrl(requestedReturnUrl); + return BuildUrlWithQueryString(redirectBaseUrl, new Dictionary + { + ["authError"] = errorCode, + ["correlationId"] = correlationId + }); + } + /// /// Gets token endpoint URI configured for Google code exchange. /// diff --git a/src/Thalos.Bff.Rest/Program.cs b/src/Thalos.Bff.Rest/Program.cs index a516dd0..bdf8b02 100644 --- a/src/Thalos.Bff.Rest/Program.cs +++ b/src/Thalos.Bff.Rest/Program.cs @@ -117,7 +117,12 @@ app.MapGet($"{EndpointConventions.ApiPrefix}/oidc/google/callback", async ( var providerError = context.Request.Query["error"].ToString(); if (!string.IsNullOrWhiteSpace(providerError)) { - return ErrorResult(StatusCodes.Status401Unauthorized, "oidc_provider_error", $"Provider rejected authentication: {providerError}.", correlationId); + DeleteOidcStateCookie(context, configuration); + return Results.Redirect( + oidcFlowService.BuildFailureRedirectUrl( + null, + "oidc_provider_error", + correlationId)); } var callbackState = context.Request.Query["state"].ToString(); @@ -126,12 +131,22 @@ app.MapGet($"{EndpointConventions.ApiPrefix}/oidc/google/callback", async ( if (!context.Request.Cookies.TryGetValue(OidcStateCookieName, out var encodedState) || !oidcFlowService.TryValidateCallbackState(encodedState, callbackState, out var payload)) { - return ErrorResult(StatusCodes.Status401Unauthorized, "oidc_state_invalid", "OIDC callback state is invalid.", correlationId); + DeleteOidcStateCookie(context, configuration); + return Results.Redirect( + oidcFlowService.BuildFailureRedirectUrl( + null, + "oidc_state_invalid", + correlationId)); } if (string.IsNullOrWhiteSpace(code)) { - return ErrorResult(StatusCodes.Status401Unauthorized, "oidc_code_missing", "OIDC callback code is missing.", correlationId); + DeleteOidcStateCookie(context, configuration); + return Results.Redirect( + oidcFlowService.BuildFailureRedirectUrl( + payload.ReturnUrl, + "oidc_code_missing", + correlationId)); } var tokenResponse = await ExchangeGoogleCodeForTokenAsync( @@ -144,7 +159,12 @@ app.MapGet($"{EndpointConventions.ApiPrefix}/oidc/google/callback", async ( if (tokenResponse is null || string.IsNullOrWhiteSpace(tokenResponse.IdToken)) { - return ErrorResult(StatusCodes.Status401Unauthorized, "oidc_exchange_failed", "Unable to exchange provider code.", correlationId); + DeleteOidcStateCookie(context, configuration); + return Results.Redirect( + oidcFlowService.BuildFailureRedirectUrl( + payload.ReturnUrl, + "oidc_exchange_failed", + correlationId)); } var serviceRequest = new IssueIdentityTokenRequest( @@ -156,11 +176,16 @@ app.MapGet($"{EndpointConventions.ApiPrefix}/oidc/google/callback", async ( if (string.IsNullOrWhiteSpace(sessionTokens.AccessToken) || string.IsNullOrWhiteSpace(sessionTokens.RefreshToken)) { - return ErrorResult(StatusCodes.Status401Unauthorized, "session_login_failed", "Unable to issue session.", correlationId); + DeleteOidcStateCookie(context, configuration); + return Results.Redirect( + oidcFlowService.BuildFailureRedirectUrl( + payload.ReturnUrl, + "session_login_failed", + correlationId)); } WriteSessionCookies(context, sessionTokens, configuration); - context.Response.Cookies.Delete(OidcStateCookieName, CreateOidcStateCookieOptions(configuration)); + DeleteOidcStateCookie(context, configuration); return Results.Redirect(payload.ReturnUrl); }); @@ -421,6 +446,11 @@ void DeleteSessionCookies(HttpContext context, IConfiguration configuration) context.Response.Cookies.Delete(SessionRefreshCookieName, options); } +void DeleteOidcStateCookie(HttpContext context, IConfiguration configuration) +{ + context.Response.Cookies.Delete(OidcStateCookieName, CreateOidcStateCookieOptions(configuration)); +} + CookieOptions CreateCookieOptions(bool secure, int expiresInSeconds) { var domain = builder.Configuration["ThalosBff:SessionCookieDomain"]; diff --git a/tests/Thalos.Bff.Application.UnitTests/GoogleOidcFlowServiceTests.cs b/tests/Thalos.Bff.Application.UnitTests/GoogleOidcFlowServiceTests.cs index 2466f07..5bcd549 100644 --- a/tests/Thalos.Bff.Application.UnitTests/GoogleOidcFlowServiceTests.cs +++ b/tests/Thalos.Bff.Application.UnitTests/GoogleOidcFlowServiceTests.cs @@ -52,6 +52,35 @@ public class GoogleOidcFlowServiceTests Assert.False(ok); } + [Fact] + public void BuildFailureRedirectUrl_WhenReturnUrlAllowed_PreservesCallerHostAndAddsErrorContext() + { + var service = new GoogleOidcFlowService(CreateOptions(), "state-signing-secret"); + + var redirectUrl = service.BuildFailureRedirectUrl( + "https://furniture-display-demo.dream-views.com/dashboard", + "oidc_exchange_failed", + "corr-123"); + + Assert.StartsWith("https://furniture-display-demo.dream-views.com/dashboard", redirectUrl, StringComparison.Ordinal); + Assert.Contains("authError=oidc_exchange_failed", redirectUrl, StringComparison.Ordinal); + Assert.Contains("correlationId=corr-123", redirectUrl, StringComparison.Ordinal); + } + + [Fact] + public void BuildFailureRedirectUrl_WhenReturnUrlNotAllowed_UsesDefaultReturnUrl() + { + var service = new GoogleOidcFlowService(CreateOptions(), "state-signing-secret"); + + var redirectUrl = service.BuildFailureRedirectUrl( + "https://malicious.example.com/redirect", + "oidc_state_invalid", + "corr-456"); + + Assert.StartsWith("https://auth.dream-views.com/", redirectUrl, StringComparison.Ordinal); + Assert.Contains("authError=oidc_state_invalid", redirectUrl, StringComparison.Ordinal); + } + private static GoogleOidcOptions CreateOptions() { return new GoogleOidcOptions(