diff --git a/api/flow-execution.yaml b/api/flow-execution.yaml index 0ecdcf608c..55e9f2d02b 100644 --- a/api/flow-execution.yaml +++ b/api/flow-execution.yaml @@ -378,6 +378,12 @@ components: example: "ERROR" error: $ref: '#/components/schemas/Error' + errorAssertion: + type: string + description: | + Optionally returned signed JWT error assertion. It carries the flow error type and description bound to + that authorization request. + example: "" Data: type: object @@ -555,6 +561,12 @@ components: $ref: '#/components/schemas/I18nMessage' description: $ref: '#/components/schemas/I18nMessage' + errorAssertion: + type: string + description: | + Optionally returned signed JWT error assertion. It carries the flow error type and description bound to + that authorization request. + example: "" I18nMessage: type: object diff --git a/api/oauth2.yaml b/api/oauth2.yaml index 2b58122a53..859c2066a9 100644 --- a/api/oauth2.yaml +++ b/api/oauth2.yaml @@ -166,9 +166,13 @@ paths: post: summary: Authorization callback description: | - Internal endpoint invoked by the flow engine after a successful authentication ceremony. - The flow engine submits a signed assertion JWT; this endpoint validates it, issues an - authorization code, and returns the redirect URI for the client. + Internal endpoint invoked by the Gate after a flow terminates. It submits the flow's signed + assertion JWT, which this endpoint validates before acting on it. A completed flow yields an + authentication assertion and an authorization code is issued (or, for CIBA, the request is + marked authenticated). A terminally failed flow yields an error assertion instead and the + failure is propagated to the waiting request (authorization code: an error redirect; CIBA: the + request is denied or marked failed). The two are distinguished by the assertion's own claims, + so callers submit both in the same field. tags: - Authorization requestBody: @@ -190,7 +194,7 @@ paths: example: redirect_uri: "https://client.example.com/callback?code=SplxlOBeZQQYbYS6WxSbIA&state=xyz&iss=https%3A%2F%2Fserver.example.com" "400": - description: Bad Request — missing or invalid authId / assertion. + description: Bad Request, missing authId or assertion. content: application/json: schema: @@ -632,7 +636,10 @@ components: description: The authorization session identifier returned by the flow engine. assertion: type: string - description: Signed JWT assertion from the flow engine containing authentication result. + description: Signed JWT assertion from the flow engine carrying the terminal flow outcome. A completed flow returns an authentication assertion (relay the flow response's assertion field); a terminally failed flow returns an error assertion carrying the flow error type and description (relay the flow response's errorAssertion field). The server validates the signature and the binding to this authorization request before acting, and tells the two apart by the assertion's own claims. + type: + type: string + description: OAuth grant type of the request. Defaults to the authorization code flow when absent. AuthCallbackResponse: type: object diff --git a/backend/cmd/server/config/default.json b/backend/cmd/server/config/default.json index 2893247c0d..462c141e52 100644 --- a/backend/cmd/server/config/default.json +++ b/backend/cmd/server/config/default.json @@ -151,6 +151,7 @@ "max_jti_length": 256 }, "allow_wildcard_redirect_uri": false, + "send_server_errors_to_client": false, "allowed_auth_methods" :["client_secret_basic", "client_secret_post", "private_key_jwt", "none"], "allowed_response_types" : ["code"], "allowed_grant_types" : ["client_credentials", "authorization_code", "refresh_token", "urn:ietf:params:oauth:grant-type:token-exchange", "urn:openid:params:grant-type:ciba", "urn:ietf:params:oauth:grant-type:jwt-bearer"], diff --git a/backend/cmd/server/servicemanager.go b/backend/cmd/server/servicemanager.go index 3e5b071436..feed3d7092 100644 --- a/backend/cmd/server/servicemanager.go +++ b/backend/cmd/server/servicemanager.go @@ -465,7 +465,7 @@ func registerServices(mux *http.ServeMux, cacheManager cache.CacheManagerInterfa attestationProvider := initAttestationProvider(ctx, logger, runtimeCryptoSvc) flowExecService, err := flowexec.Initialize(mux, flowMgtService, actorProvider, execRegistry, interceptorRegistry, observabilitySvc, runtimeCryptoSvc, attestationProvider, - graphBuilder, runtimeStoreProvider, transactioner, serverConfigService, flowConfig) + graphBuilder, jwtService, runtimeStoreProvider, transactioner, serverConfigService, flowConfig) fatalOnError(ctx, logger, err, "Failed to initialize flow execution service") // Initialize OAuth services. diff --git a/backend/internal/flow/common/constants.go b/backend/internal/flow/common/constants.go index 8511b574b4..f8341ea75b 100644 --- a/backend/internal/flow/common/constants.go +++ b/backend/internal/flow/common/constants.go @@ -70,6 +70,8 @@ const ( DataStepTimeout = "stepTimeout" // DataInviteLink is the key used for the invite link in the flow response additional data. DataInviteLink = "inviteLink" + // DataCallbackType is the OAuth grant type surfaced on the terminal flow response's additional data. + DataCallbackType = "callbackType" // DataEmailSent is the key used to indicate that an email was sent successfully in the flow response. DataEmailSent = "emailSent" // DataSMSSent is the key used to indicate that an SMS was sent successfully in the flow response. @@ -86,6 +88,20 @@ const ( DataOpenID4VPWalletURI = "openid4vpWalletUri" ) +// Error assertion claims. +const ( + ClaimAuthorizationRequestID = "authorization_request_id" + ClaimFlowErrorType = "flow_error_type" + ClaimFlowErrorDescription = "flow_error_description" +) + +// FlowErrorType defines the type of error that occurred during flow execution. +const ( + FlowErrorTypeServer = "server_error" + FlowErrorTypeClient = "client_error" + FlowErrorTypeEndUser = "end_user_error" +) + // DefaultHTTPTimeout defines the default timeout duration for HTTP requests. const DefaultHTTPTimeout = 5 * time.Second @@ -200,6 +216,10 @@ const ( // RuntimeKeyAuthorizationRequestID holds the auth request identifier bound to the current flow // execution (the OAuth authorize authId or the CIBA auth_req_id), if applicable. RuntimeKeyAuthorizationRequestID = "authorizationRequestId" + // RuntimeKeyCallbackType holds the OAuth grant type of the initiating request, seeded by the OAuth + // initiator and surfaced onto the terminal flow response as DataCallbackType so the Gate/SDK routes + // to the correct callback handler. Absent for non-OAuth flows. + RuntimeKeyCallbackType = "callbackType" // RuntimeKeySSOSessionPresent is the prefix of the per-checkpoint flag recording whether the // SSO-Check node found a live session that already has this checkpoint's snapshot ("true") or not. // It is scoped per checkpoint via SSOCheckpointKey; the paired Session node reads it to choose diff --git a/backend/internal/flow/executor/auth_assert_executor.go b/backend/internal/flow/executor/auth_assert_executor.go index 473aadb2e5..e9e920ad38 100644 --- a/backend/internal/flow/executor/auth_assert_executor.go +++ b/backend/internal/flow/executor/auth_assert_executor.go @@ -111,9 +111,6 @@ func (a *authAssertExecutor) Execute(ctx *providers.NodeContext) (*providers.Exe execResp.Status = providers.ExecComplete execResp.Assertion = token - if callbackType, ok := ctx.NodeProperties[propertyKeyCallbackType].(string); ok && callbackType != "" { - execResp.AdditionalData[propertyKeyCallbackType] = callbackType - } } else { execResp.Status = providers.ExecFailure execResp.Error = &ErrUserNotAuthenticated diff --git a/backend/internal/flow/executor/auth_assert_executor_test.go b/backend/internal/flow/executor/auth_assert_executor_test.go index a3c0538cd4..ff53b0b075 100644 --- a/backend/internal/flow/executor/auth_assert_executor_test.go +++ b/backend/internal/flow/executor/auth_assert_executor_test.go @@ -1622,60 +1622,6 @@ func (suite *AuthAssertExecutorTestSuite) TestIntersectPermissionSpaceList_NoOve assert.Equal(suite.T(), "", intersectPermissionSpaceList("a b", "c d")) } -func (suite *AuthAssertExecutorTestSuite) TestExecute_CallbackType_EmittedWhenSet() { - ctx := &providers.NodeContext{ - ExecutionID: "flow-ciba", - EntityID: "app-1", - FlowType: providers.FlowTypeAuthentication, - AuthUser: newTestAuthenticatedAuthUser(), - NodeProperties: map[string]interface{}{ - propertyKeyCallbackType: "urn:openid:params:grant-type:ciba", - }, - RuntimeData: map[string]string{}, - ExecutionHistory: map[string]*providers.NodeExecutionRecord{}, - } - - suite.setupGetEntityReference("INTERNAL", testAuthOUID) - suite.setupGetUserAttributesEmpty() - - suite.mockJWTService.On("GenerateJWT", mock.Anything, "user-123", mock.Anything, mock.Anything, - mock.Anything, mock.Anything, mock.Anything).Return("jwt-token", int64(3600), nil) - - resp, err := suite.executor.Execute(ctx) - - assert.NoError(suite.T(), err) - assert.NotNil(suite.T(), resp) - assert.Equal(suite.T(), providers.ExecComplete, resp.Status) - assert.Equal(suite.T(), "jwt-token", resp.Assertion) - assert.Equal(suite.T(), "urn:openid:params:grant-type:ciba", resp.AdditionalData[propertyKeyCallbackType]) -} - -func (suite *AuthAssertExecutorTestSuite) TestExecute_CallbackType_AbsentWhenNotSet() { - ctx := &providers.NodeContext{ - ExecutionID: "flow-authcode", - EntityID: "app-1", - FlowType: providers.FlowTypeAuthentication, - AuthUser: newTestAuthenticatedAuthUser(), - NodeProperties: map[string]interface{}{}, - RuntimeData: map[string]string{}, - ExecutionHistory: map[string]*providers.NodeExecutionRecord{}, - } - - suite.setupGetEntityReference("INTERNAL", testAuthOUID) - suite.setupGetUserAttributesEmpty() - - suite.mockJWTService.On("GenerateJWT", mock.Anything, "user-123", mock.Anything, mock.Anything, - mock.Anything, mock.Anything, mock.Anything).Return("jwt-token", int64(3600), nil) - - resp, err := suite.executor.Execute(ctx) - - assert.NoError(suite.T(), err) - assert.NotNil(suite.T(), resp) - assert.Equal(suite.T(), providers.ExecComplete, resp.Status) - _, hasCallbackType := resp.AdditionalData[propertyKeyCallbackType] - assert.False(suite.T(), hasCallbackType, "callbackType must not be present for auth code flows") -} - func (suite *AuthAssertExecutorTestSuite) TestResolveSubject() { const defaultSub = "entity-123" tests := []struct { diff --git a/backend/internal/flow/flowexec/handler.go b/backend/internal/flow/flowexec/handler.go index 0cca7ac9b4..2cf7a9b3c5 100644 --- a/backend/internal/flow/flowexec/handler.go +++ b/backend/internal/flow/flowexec/handler.go @@ -64,7 +64,11 @@ func (h *flowExecutionHandler) HandleFlowExecutionRequest(w http.ResponseWriter, flowSecret, attestationToken) if flowErr != nil { - handleFlowError(r.Context(), w, flowErr) + errorAssertion := "" + if flowStep != nil { + errorAssertion = flowStep.ErrorAssertion + } + handleFlowError(r.Context(), w, flowErr, errorAssertion) return } @@ -96,6 +100,7 @@ func (h *flowExecutionHandler) HandleFlowExecutionRequest(w http.ResponseWriter, Type: string(flowStep.Type), Data: flowStep.Data, Assertion: flowStep.Assertion, + ErrorAssertion: flowStep.ErrorAssertion, Error: stepErrorResp, ChallengeToken: flowStep.ChallengeToken, } @@ -106,8 +111,17 @@ func (h *flowExecutionHandler) HandleFlowExecutionRequest(w http.ResponseWriter, log.String(log.LoggerKeyExecutionID, flowResp.ExecutionID)) } +// flowErrorResponse is the error body for a failed flow execution. It carries the standard API error +// plus a signed error assertion (when the flow was OAuth-initiated) that the Gate/SDK relays to the +// OAuth callback so the failure reaches the waiting authorization request. +type flowErrorResponse struct { + apierror.ErrorResponse + ErrorAssertion string `json:"errorAssertion,omitempty"` +} + // handleFlowError handles errors that occur during flow execution as an API error response. -func handleFlowError(ctx context.Context, w http.ResponseWriter, flowErr *tidcommon.ServiceError) { +func handleFlowError(ctx context.Context, w http.ResponseWriter, flowErr *tidcommon.ServiceError, + errorAssertion string) { errResp := apierror.ErrorResponse{ Code: flowErr.Code, Message: flowErr.Error, @@ -127,6 +141,12 @@ func handleFlowError(ctx context.Context, w http.ResponseWriter, flowErr *tidcom } } + if errorAssertion != "" { + sysutils.WriteSuccessResponse(ctx, w, statusCode, + flowErrorResponse{ErrorResponse: errResp, ErrorAssertion: errorAssertion}) + return + } + sysutils.WriteErrorResponse(ctx, w, statusCode, errResp) } diff --git a/backend/internal/flow/flowexec/handler_test.go b/backend/internal/flow/flowexec/handler_test.go index b911b89750..b2303884f0 100644 --- a/backend/internal/flow/flowexec/handler_test.go +++ b/backend/internal/flow/flowexec/handler_test.go @@ -6,6 +6,7 @@ package flowexec import ( "bytes" "context" + "encoding/json" "net/http" "net/http/httptest" "testing" @@ -48,13 +49,13 @@ func (s *HandlerTestSuite) TestHandleFlowError_ClientError_Returns400() { DefaultValue: "bad request", }, } - handleFlowError(context.Background(), w, svcErr) + handleFlowError(context.Background(), w, svcErr, "") s.Equal(http.StatusBadRequest, w.Code) } func (s *HandlerTestSuite) TestHandleFlowError_ForbiddenError_Returns403() { w := httptest.NewRecorder() - handleFlowError(context.Background(), w, &ErrorDirectFlowInitiationNotPermitted) + handleFlowError(context.Background(), w, &ErrorDirectFlowInitiationNotPermitted, "") s.Equal(http.StatusForbidden, w.Code) } @@ -68,7 +69,7 @@ func (s *HandlerTestSuite) TestHandleFlowError_ServerError_Returns500() { DefaultValue: "internal error", }, } - handleFlowError(context.Background(), w, svcErr) + handleFlowError(context.Background(), w, svcErr, "") s.Equal(http.StatusInternalServerError, w.Code) } @@ -252,3 +253,47 @@ func (s *HandlerTestSuite) TestHandleFlowExecutionRequest_StepWithError() { h.HandleFlowExecutionRequest(w, req) s.Equal(http.StatusOK, w.Code) } + +func (s *HandlerTestSuite) TestHandleFlowError_WithErrorAssertion_IncludesAssertionInBody() { + w := httptest.NewRecorder() + svcErr := &tidcommon.ServiceError{ + Code: "FES-1013", + Type: tidcommon.ClientErrorType, + Error: tidcommon.I18nMessage{ + Key: "client.error", + DefaultValue: "bad request", + }, + } + + handleFlowError(context.Background(), w, svcErr, "signed-error-assertion") + + s.Equal(http.StatusBadRequest, w.Code) + + var body map[string]interface{} + s.Require().NoError(json.Unmarshal(w.Body.Bytes(), &body)) + s.Equal("FES-1013", body["code"]) + s.Equal("signed-error-assertion", body["errorAssertion"]) + s.NotNil(body["message"]) +} + +func (s *HandlerTestSuite) TestHandleFlowError_WithoutErrorAssertion_OmitsAssertionField() { + w := httptest.NewRecorder() + svcErr := &tidcommon.ServiceError{ + Code: "FES-4001", + Type: tidcommon.ClientErrorType, + Error: tidcommon.I18nMessage{ + Key: "client.error", + DefaultValue: "bad request", + }, + } + + handleFlowError(context.Background(), w, svcErr, "") + + s.Equal(http.StatusBadRequest, w.Code) + + var body map[string]interface{} + s.Require().NoError(json.Unmarshal(w.Body.Bytes(), &body)) + s.Equal("FES-4001", body["code"]) + _, hasAssertion := body["errorAssertion"] + s.False(hasAssertion) +} diff --git a/backend/internal/flow/flowexec/init.go b/backend/internal/flow/flowexec/init.go index 9a11158615..7be04ca94c 100644 --- a/backend/internal/flow/flowexec/init.go +++ b/backend/internal/flow/flowexec/init.go @@ -11,6 +11,7 @@ import ( "github.com/thunder-id/thunderid/internal/flow/graphbuilder" "github.com/thunder-id/thunderid/internal/flow/interceptor" "github.com/thunder-id/thunderid/internal/flow/session" + "github.com/thunder-id/thunderid/internal/system/jose/jwt" "github.com/thunder-id/thunderid/internal/system/middleware" "github.com/thunder-id/thunderid/pkg/thunderidengine/providers" ) @@ -26,6 +27,7 @@ func Initialize( cryptoSvc providers.RuntimeCryptoProvider, attestationVerifier providers.AttestationProvider, graphBuilder graphbuilder.GraphBuilderInterface, + jwtService jwt.JWTServiceInterface, storeProvider providers.RuntimeStoreProvider, transactioner providers.Transactioner, serverConfigSvc serverConfigProvider, @@ -37,7 +39,7 @@ func Initialize( flowProvider, graphBuilder) flowExecService := newFlowExecService(flowProvider, flowStore, flowEngine, actorProvider, observabilitySvc, transactioner, cryptoSvc, attestationVerifier, - graphBuilder, serverConfigSvc, cfg) + graphBuilder, jwtService, serverConfigSvc, cfg) // Mark the SSO cookie Secure unless the deployment is configured to serve over plain HTTP, and // bound its lifetime to the session's configured absolute timeout (same fallback as the session diff --git a/backend/internal/flow/flowexec/model.go b/backend/internal/flow/flowexec/model.go index 3d3826bb38..59265e0512 100644 --- a/backend/internal/flow/flowexec/model.go +++ b/backend/internal/flow/flowexec/model.go @@ -223,6 +223,7 @@ type FlowStep struct { ChallengeToken string Data FlowData Assertion string + ErrorAssertion string Error *tidcommon.ServiceError // SSOHandleOut / SSOFlowID carry an SSO session handle minted during this step back to the @@ -255,6 +256,7 @@ type FlowResponse struct { ChallengeToken string `json:"challengeToken,omitempty"` Data FlowData `json:"data,omitempty"` Assertion string `json:"assertion,omitempty"` + ErrorAssertion string `json:"errorAssertion,omitempty"` Error *apierror.ErrorResponse `json:"error,omitempty"` } diff --git a/backend/internal/flow/flowexec/service.go b/backend/internal/flow/flowexec/service.go index 3937fbbbc2..33a07e42bf 100644 --- a/backend/internal/flow/flowexec/service.go +++ b/backend/internal/flow/flowexec/service.go @@ -22,6 +22,7 @@ import ( "github.com/thunder-id/thunderid/internal/flow/session" sysContext "github.com/thunder-id/thunderid/internal/system/context" "github.com/thunder-id/thunderid/internal/system/cryptolib" + "github.com/thunder-id/thunderid/internal/system/jose/jwt" "github.com/thunder-id/thunderid/internal/system/log" "github.com/thunder-id/thunderid/internal/system/observability/event" sysutils "github.com/thunder-id/thunderid/internal/system/utils" @@ -46,6 +47,7 @@ type flowExecService struct { transactioner providers.Transactioner cryptoSvc providers.RuntimeCryptoProvider attestationVerifier providers.AttestationProvider + jwtService jwt.JWTServiceInterface serverConfigSvc serverConfigProvider cfg flowconfig.Config } @@ -59,6 +61,7 @@ func newFlowExecService(flowProvider providers.FlowProvider, cryptoSvc providers.RuntimeCryptoProvider, attestationVerifier providers.AttestationProvider, graphBuilder graphbuilder.GraphBuilderInterface, + jwtService jwt.JWTServiceInterface, serverConfigSvc serverConfigProvider, cfg flowconfig.Config) FlowExecServiceInterface { return &flowExecService{ @@ -71,6 +74,7 @@ func newFlowExecService(flowProvider providers.FlowProvider, cryptoSvc: cryptoSvc, attestationVerifier: attestationVerifier, graphBuilder: graphBuilder, + jwtService: jwtService, serverConfigSvc: serverConfigSvc, cfg: cfg, } @@ -140,9 +144,42 @@ func (s *flowExecService) Execute(ctx context.Context, return nil, &tidcommon.InternalServerError } } + // An engine failure is reported as a 4xx/5xx, which has no flow response to carry the error + // assertion, so return a bare step alongside the error for the handler to serialize. + errorType := common.FlowErrorTypeServer + if flowErr.Type == tidcommon.ClientErrorType { + errorType = common.FlowErrorTypeClient + } + if assertion := s.buildErrorAssertion(ctx, engineCtx, errorType, + flowErr.ErrorDescription.String(), logger); assertion != "" { + return &FlowStep{ErrorAssertion: assertion}, flowErr + } return nil, flowErr } + // Surface the OAuth callback type (grant type) from runtime data onto the terminal response so the + // Gate/SDK routes the completion or failure to the correct callback handler (e.g. CIBA). + // TODO: Remove once the OAuth callback handler can determine the grant type without runtime data. + if flowStep.Status == providers.FlowStatusComplete || flowStep.Status == providers.FlowStatusError { + if callbackType := engineCtx.RuntimeData[common.RuntimeKeyCallbackType]; callbackType != "" { + if flowStep.Data.AdditionalData == nil { + flowStep.Data.AdditionalData = make(map[string]string) + } + flowStep.Data.AdditionalData[common.DataCallbackType] = callbackType + } + } + + // Build a signed error assertion for an in-band flow failure so the OAuth callback can verify and + // propagate it to the waiting authorization request. + if flowStep.Status == providers.FlowStatusError { + description := "" + if flowStep.Error != nil { + description = flowStep.Error.ErrorDescription.String() + } + flowStep.ErrorAssertion = s.buildErrorAssertion(ctx, engineCtx, + common.FlowErrorTypeEndUser, description, logger) + } + if isComplete(flowStep) { if !isNewFlow(executionID) { if removeErr := s.removeContext(ctx, engineCtx.ExecutionID, logger); removeErr != nil { @@ -170,6 +207,37 @@ func (s *flowExecService) Execute(ctx context.Context, return &flowStep, nil } +// buildErrorAssertion signs an assertion binding the flow error type and description to the OAuth +// authorization request +func (s *flowExecService) buildErrorAssertion(ctx context.Context, engineCtx *EngineContext, + errorType, description string, logger *log.Logger) string { + authReqID := engineCtx.RuntimeData[common.RuntimeKeyAuthorizationRequestID] + if authReqID == "" { + return "" + } + + // Bound to the same validity as the success assertion (AuthAssertExecutor), since both are + // consumed by the callback within the same request cycle. + validityPeriod := int64(0) + if engineCtx.Application.Assertion != nil { + validityPeriod = engineCtx.Application.Assertion.ValidityPeriod + } + + claims := map[string]interface{}{ + "aud": engineCtx.AppID, + common.ClaimAuthorizationRequestID: authReqID, + common.ClaimFlowErrorType: errorType, + common.ClaimFlowErrorDescription: description, + } + token, _, err := s.jwtService.GenerateJWT(ctx, "", "", validityPeriod, claims, jwt.TokenTypeJWT, "") + if err != nil { + logger.Error(ctx, "Failed to build flow error assertion", + log.String("error", err.Error.DefaultValue)) + return "" + } + return token +} + // applyInboundSSO selects the SSO handle carried for this flow from the request-scoped // transport inputs and stashes it on the engine context for the SSO-Check node to consume. // It is a no-op when no inbound transport is present. diff --git a/backend/internal/oauth/oauth2/authz/model.go b/backend/internal/oauth/oauth2/authz/model.go index dfd2f1c349..044b5b9a35 100644 --- a/backend/internal/oauth/oauth2/authz/model.go +++ b/backend/internal/oauth/oauth2/authz/model.go @@ -79,4 +79,5 @@ type assertionClaims struct { completedACR string authorizationRequestID string tokenFamilyID string + flowErrorType string } diff --git a/backend/internal/oauth/oauth2/authz/service.go b/backend/internal/oauth/oauth2/authz/service.go index f585940b20..85f78266f4 100644 --- a/backend/internal/oauth/oauth2/authz/service.go +++ b/backend/internal/oauth/oauth2/authz/service.go @@ -471,10 +471,53 @@ func (as *authorizeService) initiateFlowAndStoreRequest( return &AuthorizationInitResult{QueryParams: queryParams}, nil } -// HandleAuthorizationCallback processes the callback assertion from the flow engine. -// Returns the client redirect URI (with authorization code) on success, or a structured error. +// HandleAuthorizationCallback processes the callback assertion from the flow engine. The assertion is +// either an authentication assertion from a completed flow or a signed error assertion minted when the +// flow terminated in failure. Returns the client redirect URI (with authorization code) on success, or a structured +// error. func (as *authorizeService) HandleAuthorizationCallback(ctx context.Context, authID string, assertion string) ( string, *AuthorizationError) { + if assertion == "" { + return "", &AuthorizationError{ + Code: oauth2const.ErrorInvalidRequest, + Message: "Invalid authorization request", + } + } + + // Verify before either branch runs. This keeps an unverified assertion from burning a live authID, + // and it means the branch below is selected by a claim that the signature covers. + if verifyErr := as.verifyAssertion(ctx, assertion); verifyErr != nil { + as.logger.Debug(ctx, "Assertion verification failed", log.Error(verifyErr)) + return "", &AuthorizationError{ + Code: oauth2const.ErrorInvalidRequest, + Message: "Authorization request failed", + } + } + + claims, authTime, decodeErr := decodeAttributesFromAssertion(assertion) + if decodeErr != nil { + // An assertion whose claims cannot be read cannot be shown to be bound to this request, so it is + // rejected without loading it. Loading consumes the request, and a caller holding a malformed + // assertion must not be able to destroy a live authID + as.logger.Debug(ctx, "Failed to decode the assertion", log.Error(decodeErr)) + return "", &AuthorizationError{ + Code: oauth2const.ErrorInvalidRequest, + Message: "Authorization request failed", + } + } + + if claims.flowErrorType != "" { + errClaims, _ := oauth2utils.DecodeFlowErrorAssertionClaims(assertion) + return "", as.handleFailedCallback(ctx, authID, errClaims) + } + + return as.handleSuccessCallback(ctx, authID, claims, authTime) +} + +// handleSuccessCallback mints an authorization code for a verified authentication assertion and +// returns the client redirect URI carrying it. +func (as *authorizeService) handleSuccessCallback(ctx context.Context, authID string, + claims assertionClaims, authTime time.Time) (string, *AuthorizationError) { var redirectURI string var authErr *AuthorizationError @@ -497,54 +540,6 @@ func (as *authorizeService) HandleAuthorizationCallback(ctx context.Context, aut return err } - if assertion == "" { - authErr = &AuthorizationError{ - Code: oauth2const.ErrorInvalidRequest, - Message: "Invalid authorization request", - SendErrorToClient: true, - ClientRedirectURI: authRequestCtx.OAuthParameters.RedirectURI, - State: authRequestCtx.OAuthParameters.State, - } - return errors.New("assertion is empty") - } - - // Verify the assertion. - if err := as.verifyAssertion(ctx, assertion); err != nil { - as.logger.Debug(ctx, "Assertion verification failed", log.Error(err)) - authErr = &AuthorizationError{ - Code: oauth2const.ErrorInvalidRequest, - Message: "Authorization request failed", - SendErrorToClient: true, - ClientRedirectURI: authRequestCtx.OAuthParameters.RedirectURI, - State: authRequestCtx.OAuthParameters.State, - } - return err - } - - // Decode user attributes from the assertion. - claims, authTime, err := decodeAttributesFromAssertion(assertion) - if err != nil { - if errors.Is(err, errAssertionClaimInvalid) { - as.logger.Debug(ctx, "Assertion contains a malformed claim", log.Error(err)) - authErr = &AuthorizationError{ - Code: oauth2const.ErrorInvalidRequest, - Message: "Assertion contains a malformed claim", - SendErrorToClient: true, - ClientRedirectURI: authRequestCtx.OAuthParameters.RedirectURI, - State: authRequestCtx.OAuthParameters.State, - } - return err - } - authErr = &AuthorizationError{ - Code: oauth2const.ErrorServerError, - Message: "Failed to process authorization request", - SendErrorToClient: true, - ClientRedirectURI: authRequestCtx.OAuthParameters.RedirectURI, - State: authRequestCtx.OAuthParameters.State, - } - return err - } - // Bind the assertion to the specific authorization request if claims.authorizationRequestID == "" || claims.authorizationRequestID != authID { as.logger.Debug(ctx, "Assertion is not bound to the authorization request") @@ -663,6 +658,51 @@ func (as *authorizeService) HandleAuthorizationCallback(ctx context.Context, aut return redirectURI, nil } +// handleFailedCallback constructs the OAuth error response for a verified error assertion. The +// assertion is bound to this authorization request before the request context is loaded, since +// loading consumes it and an assertion minted for another request must not burn a live authID. +func (as *authorizeService) handleFailedCallback( + ctx context.Context, authID string, claims oauth2utils.FlowErrorAssertionClaims) *AuthorizationError { + if claims.AuthorizationRequestID != authID { + as.logger.Debug(ctx, "Error assertion is not bound to the authorization request") + return &AuthorizationError{ + Code: oauth2const.ErrorInvalidRequest, + Message: "Error assertion does not match the authorization request", + } + } + + authRequestCtx, err := as.loadAuthRequestContext(ctx, authID) + if err != nil { + if errors.Is(err, errAuthRequestNotFound) { + return &AuthorizationError{ + Code: oauth2const.ErrorInvalidRequest, + Message: "Invalid authorization request", + } + } + as.logger.Error(ctx, "Failed to load authorization request context", log.Error(err)) + return &AuthorizationError{ + Code: oauth2const.ErrorServerError, + Message: "Failed to process authorization request", + } + } + + code, message := mapFlowErrorTypeToOAuthError(claims.ErrorType, claims.Description) + // Denials are always reported. Server errors are reported unless the deployment opts out, in + // which case the failure surfaces on the error page and the client is left to time out. + sendToClient := code != oauth2const.ErrorServerError || as.cfg.OAuth.SendServerErrorsToClientEnabled() + as.logger.Debug(ctx, "Propagating flow failure", + log.String("flowErrorType", claims.ErrorType), + log.String("flowErrorDescription", claims.Description), + log.Bool("sendToClient", sendToClient)) + return &AuthorizationError{ + Code: code, + Message: message, + SendErrorToClient: sendToClient, + ClientRedirectURI: authRequestCtx.OAuthParameters.RedirectURI, + State: authRequestCtx.OAuthParameters.State, + } +} + // loadAuthRequestContext loads the authorization request context from the store using the auth ID. func (as *authorizeService) loadAuthRequestContext(ctx context.Context, authID string) (*authRequestContext, error) { ok, authRequestCtx, err := as.authReqStore.GetRequest(ctx, authID) @@ -714,6 +754,10 @@ func decodeAttributesFromAssertion(assertion string) (assertionClaims, time.Time claims.tokenFamilyID = v } + if v, ok := payload[flowcm.ClaimFlowErrorType].(string); ok { + claims.flowErrorType = v + } + if v, ok := payload[oauth2const.ClaimAuthorizationRequestID]; ok { strValue, ok := v.(string) if !ok { @@ -1014,3 +1058,18 @@ func (as *authorizeService) resolveUserAttributesCacheTTL(app *providers.OAuthCl authCodeTTL := as.cfg.OAuth.AuthorizationCode.ValidityPeriod return maxTTL + authCodeTTL + oauth2const.AttributeCacheTTLBufferSeconds } + +func mapFlowErrorTypeToOAuthError(errorType, description string) (string, string) { + code := oauth2const.ErrorServerError + message := "Failed to process authorization request" + if errorType == flowcm.FlowErrorTypeEndUser { + code = oauth2const.ErrorAccessDenied + message = "Access denied" + } + + if sanitized := oauth2utils.SanitizeErrorDescription(description); sanitized != "" { + message = sanitized + } + + return code, message +} diff --git a/backend/internal/oauth/oauth2/authz/service_test.go b/backend/internal/oauth/oauth2/authz/service_test.go index 8aeaa4531d..a9a98ac86b 100644 --- a/backend/internal/oauth/oauth2/authz/service_test.go +++ b/backend/internal/oauth/oauth2/authz/service_test.go @@ -6,6 +6,7 @@ package authz import ( "context" "errors" + "fmt" "net/url" "strings" "testing" @@ -27,6 +28,7 @@ import ( oauth2const "github.com/thunder-id/thunderid/internal/oauth/oauth2/constants" oauth2model "github.com/thunder-id/thunderid/internal/oauth/oauth2/model" "github.com/thunder-id/thunderid/internal/oauth/oauth2/revocation" + oauth2utils "github.com/thunder-id/thunderid/internal/oauth/oauth2/utils" "github.com/thunder-id/thunderid/internal/system/config" "github.com/thunder-id/thunderid/internal/system/jose/jwt" "github.com/thunder-id/thunderid/internal/system/log" @@ -598,10 +600,11 @@ func (suite *AuthorizeServiceTestSuite) TestHandleInitialAuthorizationRequest_Se } func (suite *AuthorizeServiceTestSuite) TestHandleAuthorizationCallback_InvalidAuthID() { + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, svcJWTWithIat, "", "").Return(nil) suite.mockAuthReqStore.EXPECT().GetRequest(mock.Anything, "invalid-key").Return(false, authRequestContext{}, nil) svc := suite.newService() - redirectURI, authErr := svc.HandleAuthorizationCallback(context.Background(), "invalid-key", "test-assertion") + redirectURI, authErr := svc.HandleAuthorizationCallback(context.Background(), "invalid-key", svcJWTWithIat) assert.Empty(suite.T(), redirectURI) assert.NotNil(suite.T(), authErr) @@ -609,49 +612,37 @@ func (suite *AuthorizeServiceTestSuite) TestHandleAuthorizationCallback_InvalidA } func (suite *AuthorizeServiceTestSuite) TestHandleAuthorizationCallback_StoreError() { + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, svcJWTWithIat, "", "").Return(nil) suite.mockAuthReqStore.EXPECT().GetRequest(mock.Anything, "db-fail-key"). Return(false, authRequestContext{}, errors.New("db connection error")) svc := suite.newService() - redirectURI, authErr := svc.HandleAuthorizationCallback(context.Background(), "db-fail-key", "test-assertion") + redirectURI, authErr := svc.HandleAuthorizationCallback(context.Background(), "db-fail-key", svcJWTWithIat) assert.Empty(suite.T(), redirectURI) assert.NotNil(suite.T(), authErr) assert.Equal(suite.T(), oauth2const.ErrorServerError, authErr.Code) } +// TestHandleAuthorizationCallback_MissingAssertion verifies that an empty assertion is rejected before +// the authorization request is touched, so a caller holding only an authID cannot destroy it. func (suite *AuthorizeServiceTestSuite) TestHandleAuthorizationCallback_MissingAssertion() { - authCtx := authRequestContext{ - OAuthParameters: oauth2model.OAuthParameters{ - ClientID: "test-client", - RedirectURI: "https://client.example.com/callback", - State: "test-state", - }, - } - suite.mockAuthReqStore.EXPECT().GetRequest(mock.Anything, testAuthID).Return(true, authCtx, nil) - suite.mockAuthReqStore.EXPECT().ClearRequest(mock.Anything, testAuthID).Return(nil) - svc := suite.newService() redirectURI, authErr := svc.HandleAuthorizationCallback(context.Background(), testAuthID, "") assert.Empty(suite.T(), redirectURI) - assert.NotNil(suite.T(), authErr) + suite.Require().NotNil(authErr) assert.Equal(suite.T(), oauth2const.ErrorInvalidRequest, authErr.Code) - assert.Equal(suite.T(), "test-state", authErr.State) - assert.True(suite.T(), authErr.SendErrorToClient) - assert.Equal(suite.T(), "https://client.example.com/callback", authErr.ClientRedirectURI) + assert.False(suite.T(), authErr.SendErrorToClient) + suite.mockAuthReqStore.AssertNotCalled(suite.T(), "GetRequest", mock.Anything, mock.Anything) + suite.mockAuthReqStore.AssertNotCalled(suite.T(), "ClearRequest", mock.Anything, mock.Anything) } +// TestHandleAuthorizationCallback_InvalidAssertionSignature verifies that verification runs before the +// authorization request is loaded, since loading consumes it. An assertion whose signature does not +// verify is not evidence that any flow ran, so the request survives and the error goes to the error +// page rather than to the client, which is why no redirect URI is populated here. func (suite *AuthorizeServiceTestSuite) TestHandleAuthorizationCallback_InvalidAssertionSignature() { - authCtx := authRequestContext{ - OAuthParameters: oauth2model.OAuthParameters{ - ClientID: "test-client", - RedirectURI: "https://client.example.com/callback", - State: "test-state", - }, - } - suite.mockAuthReqStore.EXPECT().GetRequest(mock.Anything, testAuthID).Return(true, authCtx, nil) - suite.mockAuthReqStore.EXPECT().ClearRequest(mock.Anything, testAuthID).Return(nil) suite.mockJWTService.EXPECT(). VerifyJWT(mock.Anything, "invalid-assertion", "", "").Return(&jwt.ErrorInvalidTokenSignature) @@ -659,23 +650,18 @@ func (suite *AuthorizeServiceTestSuite) TestHandleAuthorizationCallback_InvalidA redirectURI, authErr := svc.HandleAuthorizationCallback(context.Background(), testAuthID, "invalid-assertion") assert.Empty(suite.T(), redirectURI) - assert.NotNil(suite.T(), authErr) + suite.Require().NotNil(authErr) assert.Equal(suite.T(), oauth2const.ErrorInvalidRequest, authErr.Code) - assert.Equal(suite.T(), "test-state", authErr.State) - assert.True(suite.T(), authErr.SendErrorToClient) - assert.Equal(suite.T(), "https://client.example.com/callback", authErr.ClientRedirectURI) + assert.False(suite.T(), authErr.SendErrorToClient) + suite.mockAuthReqStore.AssertNotCalled(suite.T(), "GetRequest", mock.Anything, mock.Anything) + suite.mockAuthReqStore.AssertNotCalled(suite.T(), "ClearRequest", mock.Anything, mock.Anything) } +// TestHandleAuthorizationCallback_FailedToDecodeAssertion verifies that an assertion whose claims +// cannot be read is rejected without touching the authorization request. Its claims cannot show it was +// minted for this request, so consuming the request on it would let a caller holding one malformed +// assertion destroy any live authID it can name. func (suite *AuthorizeServiceTestSuite) TestHandleAuthorizationCallback_FailedToDecodeAssertion() { - authCtx := authRequestContext{ - OAuthParameters: oauth2model.OAuthParameters{ - ClientID: "test-client", - RedirectURI: "https://client.example.com/callback", - State: "test-state", - }, - } - suite.mockAuthReqStore.EXPECT().GetRequest(mock.Anything, testAuthID).Return(true, authCtx, nil) - suite.mockAuthReqStore.EXPECT().ClearRequest(mock.Anything, testAuthID).Return(nil) // VerifyJWT succeeds but "not.valid.jwt" cannot be decoded as a valid JWT payload. suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, "not.valid.jwt", "", "").Return(nil) @@ -683,12 +669,12 @@ func (suite *AuthorizeServiceTestSuite) TestHandleAuthorizationCallback_FailedTo redirectURI, authErr := svc.HandleAuthorizationCallback(context.Background(), testAuthID, "not.valid.jwt") assert.Empty(suite.T(), redirectURI) - assert.NotNil(suite.T(), authErr) - assert.Equal(suite.T(), oauth2const.ErrorServerError, authErr.Code) - assert.Equal(suite.T(), "Failed to process authorization request", authErr.Message) - assert.Equal(suite.T(), "test-state", authErr.State) - assert.True(suite.T(), authErr.SendErrorToClient) - assert.Equal(suite.T(), "https://client.example.com/callback", authErr.ClientRedirectURI) + suite.Require().NotNil(authErr) + assert.Equal(suite.T(), oauth2const.ErrorInvalidRequest, authErr.Code) + assert.False(suite.T(), authErr.SendErrorToClient) + assert.Empty(suite.T(), authErr.ClientRedirectURI) + suite.mockAuthReqStore.AssertNotCalled(suite.T(), "GetRequest", mock.Anything, mock.Anything) + suite.mockAuthReqStore.AssertNotCalled(suite.T(), "ClearRequest", mock.Anything, mock.Anything) } func (suite *AuthorizeServiceTestSuite) TestHandleAuthorizationCallback_UnboundAssertion() { @@ -739,28 +725,20 @@ func (suite *AuthorizeServiceTestSuite) TestHandleAuthorizationCallback_Mismatch } func (suite *AuthorizeServiceTestSuite) TestHandleAuthorizationCallback_NonStringAuthReqID() { - // Assertion's authorization_request_id claim is not a string → malformed client input, - // mapped to invalid_request rather than server_error. - authCtx := authRequestContext{ - OAuthParameters: oauth2model.OAuthParameters{ - ClientID: "test-client", - RedirectURI: "https://client.example.com/callback", - State: "test-state", - }, - } - suite.mockAuthReqStore.EXPECT().GetRequest(mock.Anything, testAuthID).Return(true, authCtx, nil) - suite.mockAuthReqStore.EXPECT().ClearRequest(mock.Anything, testAuthID).Return(nil) + // The assertion's authorization_request_id claim is not a string, so it is malformed client input, + // mapped to invalid_request. The unreadable claim is the binding itself, so the assertion cannot be + // tied to this request and it is rejected without consuming it. suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, svcJWTNonStringAuthReqID, "", "").Return(nil) svc := suite.newService() redirectURI, authErr := svc.HandleAuthorizationCallback(context.Background(), testAuthID, svcJWTNonStringAuthReqID) assert.Empty(suite.T(), redirectURI) - assert.NotNil(suite.T(), authErr) + suite.Require().NotNil(authErr) assert.Equal(suite.T(), oauth2const.ErrorInvalidRequest, authErr.Code) - assert.True(suite.T(), authErr.SendErrorToClient) - assert.Equal(suite.T(), "https://client.example.com/callback", authErr.ClientRedirectURI) - assert.Equal(suite.T(), "test-state", authErr.State) + assert.False(suite.T(), authErr.SendErrorToClient) + suite.mockAuthReqStore.AssertNotCalled(suite.T(), "GetRequest", mock.Anything, mock.Anything) + suite.mockAuthReqStore.AssertNotCalled(suite.T(), "ClearRequest", mock.Anything, mock.Anything) } func (suite *AuthorizeServiceTestSuite) TestHandleAuthorizationCallback_PersistAuthCodeError() { @@ -2244,3 +2222,318 @@ func (suite *AuthorizeServiceTestSuite) TestHandleInitialAuthorizationRequest_Mu assert.NotNil(suite.T(), authErr) assert.Equal(suite.T(), oauth2const.ErrorInvalidTarget, authErr.Code) } + +// Error assertion fixtures for HandleAuthorizationCallback failure branch. All use alg "none" since the +// signature is checked by the mocked JWT service, not by decoding. +const ( + // Payload: authorization_request_id=test-auth-id, flow_error_type=end_user_error, + // flow_error_description="User denied consent" + errAssertionEndUser = "eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0." + + "eyJhdXRob3JpemF0aW9uX3JlcXVlc3RfaWQiOiJ0ZXN0LWF1dGgtaWQiLCJmbG93X2Vycm9yX3R5cGUiOiJlbmRfdXNlcl9" + + "lcnJvciIsImZsb3dfZXJyb3JfZGVzY3JpcHRpb24iOiJVc2VyIGRlbmllZCBjb25zZW50In0." + // Payload: flow_error_type=server_error, flow_error_description="Flow engine failure" + errAssertionServerError = "eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0." + + "eyJhdXRob3JpemF0aW9uX3JlcXVlc3RfaWQiOiJ0ZXN0LWF1dGgtaWQiLCJmbG93X2Vycm9yX3R5cGUiOiJzZXJ2ZXJfZXJ" + + "yb3IiLCJmbG93X2Vycm9yX2Rlc2NyaXB0aW9uIjoiRmxvdyBlbmdpbmUgZmFpbHVyZSJ9." + // Payload: flow_error_type=end_user_error, no flow_error_description + errAssertionNoDescription = "eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0." + + "eyJhdXRob3JpemF0aW9uX3JlcXVlc3RfaWQiOiJ0ZXN0LWF1dGgtaWQiLCJmbG93X2Vycm9yX3R5cGUiOiJlbmRfdXNlcl9lcnJvciJ9." + // Payload: flow_error_description contains a quote, a newline, a non-ASCII rune and a backslash, + // all of which RFC 6749 disallows in error_description. + errAssertionDirtyDescription = "eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0." + + "eyJhdXRob3JpemF0aW9uX3JlcXVlc3RfaWQiOiJ0ZXN0LWF1dGgtaWQiLCJmbG93X2Vycm9yX3R5cGUiOiJlbmRfdXNlcl9" + + "lcnJvciIsImZsb3dfZXJyb3JfZGVzY3JpcHRpb24iOiJEZW5pZWQgXCJlbWFpbFwiXG5cdTAwZTkgYmFja1xcc2xhc2gifQ." + // Payload: authorization_request_id=other-auth-id — not bound to testAuthID. + errAssertionMismatched = "eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0." + + "eyJhdXRob3JpemF0aW9uX3JlcXVlc3RfaWQiOiJvdGhlci1hdXRoLWlkIiwiZmxvd19lcnJvcl90eXBlIjoiZW5kX3VzZXJfZXJyb3IifQ." + // Payload: flow_error_type is an unrecognized value. + errAssertionUnknownType = "eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0." + + "eyJhdXRob3JpemF0aW9uX3JlcXVlc3RfaWQiOiJ0ZXN0LWF1dGgtaWQiLCJmbG93X2Vycm9yX3R5cGUiOiJ0b3RhbGx5X3Vua25vd24ifQ." + // Payload: flow_error_type=client_error, flow_error_description="Max call depth exceeded" + errAssertionClientError = "eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0." + + "eyJhdXRob3JpemF0aW9uX3JlcXVlc3RfaWQiOiJ0ZXN0LWF1dGgtaWQiLCJmbG93X2Vycm9yX3R5cGUiOiJjbGllbnRfZXJyb3" + + "IiLCJmbG93X2Vycm9yX2Rlc2NyaXB0aW9uIjoiTWF4IGNhbGwgZGVwdGggZXhjZWVkZWQifQ." +) + +// TestHandleAuthorizationCallback_ClassifiesByFlowErrorTypeClaim is the core of the single-field +// callback contract: success and failure assertions arrive in the same argument, and the flow error +// type claim alone decides which branch runs. The claim is covered by the signature, which is verified +// first, so a caller cannot steer an assertion into the other branch. +func (suite *AuthorizeServiceTestSuite) TestHandleAuthorizationCallback_ClassifiesByFlowErrorTypeClaim() { + suite.Run("AssertionWithFlowErrorTypeIsAFailure", func() { + suite.SetupTest() + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, errAssertionEndUser, "", "").Return(nil) + suite.mockAuthReqStore.EXPECT(). + GetRequest(mock.Anything, testAuthID).Return(true, failedCallbackAuthCtx(), nil) + suite.mockAuthReqStore.EXPECT().ClearRequest(mock.Anything, testAuthID).Return(nil) + + svc := suite.newService() + redirectURI, authErr := svc.HandleAuthorizationCallback(context.Background(), testAuthID, errAssertionEndUser) + + // The failure branch never mints a code; it only produces an error for the client. + assert.Empty(suite.T(), redirectURI) + suite.Require().NotNil(authErr) + assert.Equal(suite.T(), oauth2const.ErrorAccessDenied, authErr.Code) + assert.Equal(suite.T(), "User denied consent", authErr.Message) + suite.mockAuthzCodeStore.AssertNotCalled(suite.T(), "InsertAuthorizationCode", + mock.Anything, mock.Anything) + }) + + suite.Run("AssertionWithoutFlowErrorTypeIsASuccess", func() { + suite.SetupTest() + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, svcJWTWithIat, "", "").Return(nil) + suite.mockAuthReqStore.EXPECT(). + GetRequest(mock.Anything, testAuthID).Return(true, failedCallbackAuthCtx(), nil) + suite.mockAuthReqStore.EXPECT().ClearRequest(mock.Anything, testAuthID).Return(nil) + suite.mockAuthzCodeStore.EXPECT().InsertAuthorizationCode(mock.Anything, mock.Anything).Return(nil) + + svc := suite.newService() + redirectURI, authErr := svc.HandleAuthorizationCallback(context.Background(), testAuthID, svcJWTWithIat) + + assert.Nil(suite.T(), authErr) + assert.Contains(suite.T(), redirectURI, "code=") + }) +} + +// failedCallbackAuthCtx is the stored authorization request the failure callback resolves +// redirect_uri and state from. +func failedCallbackAuthCtx() authRequestContext { + return authRequestContext{ + OAuthParameters: oauth2model.OAuthParameters{ + ClientID: "test-client", + RedirectURI: "https://client.example.com/callback", + State: "test-state", + }, + } +} + +// TestHandleFailedCallback_MapsErrorTypeAndDescription pins the flow error type to OAuth code and +// description mapping, so it enables oauth.send_server_errors_to_client to keep every row on the +// client redirect path. Suppressing server errors is the default and is covered by +// TestHandleFailedCallback_UnsetToggleSuppressesServerErrors. +func (suite *AuthorizeServiceTestSuite) TestHandleFailedCallback_MapsErrorTypeAndDescription() { + tests := []struct { + name string + assertion string + expectedCode string + expectedDescription string + }{ + { + name: "EndUserErrorBecomesAccessDenied", + assertion: errAssertionEndUser, + expectedCode: oauth2const.ErrorAccessDenied, + expectedDescription: "User denied consent", + }, + { + name: "ServerErrorBecomesServerError", + assertion: errAssertionServerError, + expectedCode: oauth2const.ErrorServerError, + expectedDescription: "Flow engine failure", + }, + { + name: "UnknownTypeFallsBackToServerError", + assertion: errAssertionUnknownType, + expectedCode: oauth2const.ErrorServerError, + expectedDescription: "Failed to process authorization request", + }, + { + name: "MissingDescriptionFallsBackToFixedMessage", + assertion: errAssertionNoDescription, + expectedCode: oauth2const.ErrorAccessDenied, + expectedDescription: "Access denied", + }, + } + + for _, tt := range tests { + suite.Run(tt.name, func() { + suite.SetupTest() + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, tt.assertion, "", "").Return(nil) + suite.mockAuthReqStore.EXPECT(). + GetRequest(mock.Anything, testAuthID).Return(true, failedCallbackAuthCtx(), nil) + suite.mockAuthReqStore.EXPECT().ClearRequest(mock.Anything, testAuthID).Return(nil) + + svc := suite.newService() + svc.cfg.OAuth.SendServerErrorsToClient = new(true) + _, authErr := svc.HandleAuthorizationCallback(context.Background(), testAuthID, tt.assertion) + + assert.NotNil(suite.T(), authErr) + assert.Equal(suite.T(), tt.expectedCode, authErr.Code) + assert.Equal(suite.T(), tt.expectedDescription, authErr.Message) + assert.True(suite.T(), authErr.SendErrorToClient) + assert.Equal(suite.T(), "https://client.example.com/callback", authErr.ClientRedirectURI) + assert.Equal(suite.T(), "test-state", authErr.State) + }) + } +} + +func (suite *AuthorizeServiceTestSuite) TestHandleFailedCallback_SanitizesDescription() { + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, errAssertionDirtyDescription, "", "").Return(nil) + suite.mockAuthReqStore.EXPECT(). + GetRequest(mock.Anything, testAuthID).Return(true, failedCallbackAuthCtx(), nil) + suite.mockAuthReqStore.EXPECT().ClearRequest(mock.Anything, testAuthID).Return(nil) + + svc := suite.newService() + _, authErr := svc.HandleAuthorizationCallback(context.Background(), testAuthID, errAssertionDirtyDescription) + + assert.Equal(suite.T(), oauth2const.ErrorAccessDenied, authErr.Code) + // The quote, newline, non-ASCII rune and backslash are all dropped. + assert.Equal(suite.T(), "Denied email backslash", authErr.Message) + + // The sanitized description must still build a client redirect; an unsanitized one would fail + // validation and strand the user on the error page instead of notifying the client. + redirectURI, err := oauth2utils.GetURIWithQueryParams(authErr.ClientRedirectURI, map[string]string{ + oauth2const.RequestParamError: authErr.Code, + oauth2const.RequestParamErrorDescription: authErr.Message, + oauth2const.RequestParamState: authErr.State, + }) + assert.NoError(suite.T(), err) + assert.Contains(suite.T(), redirectURI, "error=access_denied") +} + +func (suite *AuthorizeServiceTestSuite) TestHandleFailedCallback_InvalidSignature_PreservesRequest() { + // Verification runs before the request is loaded, so a bad assertion must not consume the authID. + suite.mockJWTService.EXPECT(). + VerifyJWT(mock.Anything, "tampered-assertion", "", "").Return(&jwt.ErrorInvalidTokenSignature) + + svc := suite.newService() + _, authErr := svc.HandleAuthorizationCallback(context.Background(), testAuthID, "tampered-assertion") + + assert.NotNil(suite.T(), authErr) + assert.Equal(suite.T(), oauth2const.ErrorInvalidRequest, authErr.Code) + assert.False(suite.T(), authErr.SendErrorToClient) + suite.mockAuthReqStore.AssertNotCalled(suite.T(), "GetRequest", mock.Anything, mock.Anything) + suite.mockAuthReqStore.AssertNotCalled(suite.T(), "ClearRequest", mock.Anything, mock.Anything) +} + +func (suite *AuthorizeServiceTestSuite) TestHandleFailedCallback_BindingMismatch_PreservesRequest() { + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, errAssertionMismatched, "", "").Return(nil) + + svc := suite.newService() + _, authErr := svc.HandleAuthorizationCallback(context.Background(), testAuthID, errAssertionMismatched) + + assert.NotNil(suite.T(), authErr) + assert.Equal(suite.T(), oauth2const.ErrorInvalidRequest, authErr.Code) + assert.False(suite.T(), authErr.SendErrorToClient) + suite.mockAuthReqStore.AssertNotCalled(suite.T(), "GetRequest", mock.Anything, mock.Anything) + suite.mockAuthReqStore.AssertNotCalled(suite.T(), "ClearRequest", mock.Anything, mock.Anything) +} + +func (suite *AuthorizeServiceTestSuite) TestHandleFailedCallback_UnknownAuthID_NotSentToClient() { + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, errAssertionEndUser, "", "").Return(nil) + suite.mockAuthReqStore.EXPECT(). + GetRequest(mock.Anything, testAuthID).Return(false, authRequestContext{}, nil) + + svc := suite.newService() + _, authErr := svc.HandleAuthorizationCallback(context.Background(), testAuthID, errAssertionEndUser) + + assert.NotNil(suite.T(), authErr) + assert.Equal(suite.T(), oauth2const.ErrorInvalidRequest, authErr.Code) + assert.False(suite.T(), authErr.SendErrorToClient) +} + +func (suite *AuthorizeServiceTestSuite) TestHandleFailedCallback_StoreError_ReturnsServerError() { + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, errAssertionEndUser, "", "").Return(nil) + suite.mockAuthReqStore.EXPECT(). + GetRequest(mock.Anything, testAuthID).Return(false, authRequestContext{}, errors.New("db down")) + + svc := suite.newService() + _, authErr := svc.HandleAuthorizationCallback(context.Background(), testAuthID, errAssertionEndUser) + + assert.Equal(suite.T(), oauth2const.ErrorServerError, authErr.Code) + assert.False(suite.T(), authErr.SendErrorToClient) +} + +func (suite *AuthorizeServiceTestSuite) TestHandleFailedCallback_ConsumesRequest() { + // The request is single-use: it is cleared on load, so a replay finds nothing. + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, errAssertionEndUser, "", "").Return(nil).Twice() + suite.mockAuthReqStore.EXPECT(). + GetRequest(mock.Anything, testAuthID).Return(true, failedCallbackAuthCtx(), nil).Once() + suite.mockAuthReqStore.EXPECT().ClearRequest(mock.Anything, testAuthID).Return(nil).Once() + + svc := suite.newService() + _, first := svc.HandleAuthorizationCallback(context.Background(), testAuthID, errAssertionEndUser) + assert.True(suite.T(), first.SendErrorToClient) + + suite.mockAuthReqStore.EXPECT(). + GetRequest(mock.Anything, testAuthID).Return(false, authRequestContext{}, nil).Once() + _, replay := svc.HandleAuthorizationCallback(context.Background(), testAuthID, errAssertionEndUser) + + assert.Equal(suite.T(), oauth2const.ErrorInvalidRequest, replay.Code) + assert.False(suite.T(), replay.SendErrorToClient) +} + +// TestHandleFailedCallback_SendServerErrorsToClientToggle verifies that +// oauth.send_server_errors_to_client gates only the server_error code. A denial is the client's +// business and is always reported; every flow error type that maps to server_error is suppressed +// together, so client_error and an unrecognized type follow server_error. +func (suite *AuthorizeServiceTestSuite) TestHandleFailedCallback_SendServerErrorsToClientToggle() { + tests := []struct { + name string + assertion string + code string + // gated is true when the toggle decides whether this error reaches the client. + gated bool + }{ + {name: "EndUserErrorAlwaysReported", assertion: errAssertionEndUser, + code: oauth2const.ErrorAccessDenied, gated: false}, + {name: "ServerErrorIsGated", assertion: errAssertionServerError, + code: oauth2const.ErrorServerError, gated: true}, + {name: "ClientErrorIsGated", assertion: errAssertionClientError, + code: oauth2const.ErrorServerError, gated: true}, + {name: "UnknownTypeIsGated", assertion: errAssertionUnknownType, + code: oauth2const.ErrorServerError, gated: true}, + } + + for _, tt := range tests { + for _, enabled := range []bool{true, false} { + suite.Run(fmt.Sprintf("%s/enabled=%t", tt.name, enabled), func() { + suite.SetupTest() + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, tt.assertion, "", "").Return(nil) + suite.mockAuthReqStore.EXPECT(). + GetRequest(mock.Anything, testAuthID).Return(true, failedCallbackAuthCtx(), nil) + suite.mockAuthReqStore.EXPECT().ClearRequest(mock.Anything, testAuthID).Return(nil) + + svc := suite.newService() + svc.cfg.OAuth.SendServerErrorsToClient = new(enabled) + _, authErr := svc.HandleAuthorizationCallback(context.Background(), testAuthID, tt.assertion) + + suite.Require().NotNil(authErr) + assert.Equal(suite.T(), tt.code, authErr.Code) + assert.Equal(suite.T(), enabled || !tt.gated, authErr.SendErrorToClient) + }) + } + } +} + +// TestHandleFailedCallback_SuppressedServerErrorStillConsumesRequest verifies that +// declining to report a server error does not resurrect the authorization request. The request is +// dead either way; only the notification to the client is suppressed. +func (suite *AuthorizeServiceTestSuite) TestHandleFailedCallback_SuppressedServerErrorStillConsumesRequest() { + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, errAssertionServerError, "", "").Return(nil) + suite.mockAuthReqStore.EXPECT(). + GetRequest(mock.Anything, testAuthID).Return(true, failedCallbackAuthCtx(), nil).Once() + suite.mockAuthReqStore.EXPECT().ClearRequest(mock.Anything, testAuthID).Return(nil).Once() + + svc := suite.newService() + svc.cfg.OAuth.SendServerErrorsToClient = new(false) + _, authErr := svc.HandleAuthorizationCallback(context.Background(), testAuthID, errAssertionServerError) + + assert.False(suite.T(), authErr.SendErrorToClient) + suite.mockAuthReqStore.AssertExpectations(suite.T()) +} + +// TestHandleFailedCallback_UnsetToggleSuppressesServerErrors verifies the default: a deployment that +// never sets the key keeps the server error off the client redirect, so the error page handles it. +func (suite *AuthorizeServiceTestSuite) TestHandleFailedCallback_UnsetToggleSuppressesServerErrors() { + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, errAssertionServerError, "", "").Return(nil) + suite.mockAuthReqStore.EXPECT(). + GetRequest(mock.Anything, testAuthID).Return(true, failedCallbackAuthCtx(), nil) + suite.mockAuthReqStore.EXPECT().ClearRequest(mock.Anything, testAuthID).Return(nil) + + svc := suite.newService() + svc.cfg.OAuth.SendServerErrorsToClient = nil + _, authErr := svc.HandleAuthorizationCallback(context.Background(), testAuthID, errAssertionServerError) + + assert.Equal(suite.T(), oauth2const.ErrorServerError, authErr.Code) + assert.False(suite.T(), authErr.SendErrorToClient) +} diff --git a/backend/internal/oauth/oauth2/callback/callback.go b/backend/internal/oauth/oauth2/callback/callback.go index 6776a90071..979fe0c0b9 100644 --- a/backend/internal/oauth/oauth2/callback/callback.go +++ b/backend/internal/oauth/oauth2/callback/callback.go @@ -25,8 +25,10 @@ import ( ) // flowCallbackRequest is the request body sent by the Gate UI to the flow callback endpoint. -// Type identifies which grant-type handler processes the completed assertion. When absent it -// defaults to authorization_code, preserving existing behavior for the auth code flow. +// Assertion carries the terminal flow outcome, either an authentication assertion or a signed error +// assertion; the grant-type handler tells them apart from the assertion's own claims. Type identifies +// which handler processes it. When absent it defaults to authorization_code, preserving existing +// behavior for the auth code flow. type flowCallbackRequest struct { AuthID string `json:"authId"` Assertion string `json:"assertion"` @@ -83,8 +85,13 @@ func (d *callbackDispatcher) handleFlowCallback(w http.ResponseWriter, r *http.R return } - if req.AuthID == "" || req.Assertion == "" { - utils.WriteJSONError(ctx, w, oauth2const.ErrorInvalidRequest, "authId and assertion are required", + if req.AuthID == "" { + utils.WriteJSONError(ctx, w, oauth2const.ErrorInvalidRequest, "authId is required", + http.StatusBadRequest, nil) + return + } + if req.Assertion == "" { + utils.WriteJSONError(ctx, w, oauth2const.ErrorInvalidRequest, "assertion is required", http.StatusBadRequest, nil) return } diff --git a/backend/internal/oauth/oauth2/callback/callback_test.go b/backend/internal/oauth/oauth2/callback/callback_test.go index 0d9a75b8cb..cca4ddbe1a 100644 --- a/backend/internal/oauth/oauth2/callback/callback_test.go +++ b/backend/internal/oauth/oauth2/callback/callback_test.go @@ -355,3 +355,55 @@ func (suite *CallbackDispatcherTestSuite) TestWriteErrorPageRedirect_WithoutStat suite.Contains(resp.RedirectURI, "errorCode="+oauth2const.ErrorServerError) suite.NotContains(resp.RedirectURI, "state=") } + +// --- handleFlowCallback: failure outcomes --- +// +// Success and failure assertions share the assertion field, and the dispatcher passes it through +// opaquely, so a failure produces exactly the same routing as any other error from the grant-type +// handler (covered above). Which branch the assertion takes is decided inside the services, and is +// tested in authz/service_test.go and ciba/service_test.go. Only the outcomes those tests cannot +// reach from the service layer live here. + +// TestHandleFlowCallback_SuppressedServerErrorNeverContactsClient covers the dispatcher half of +// oauth.send_server_errors_to_client: a server_error that is not for the client goes to the error +// page and the client redirect URI, though present on the error, is not used. +func (suite *CallbackDispatcherTestSuite) TestHandleFlowCallback_SuppressedServerErrorNeverContactsClient() { + authErr := &oauth2authz.AuthorizationError{ + Code: oauth2const.ErrorServerError, + Message: "Flow engine failure", + SendErrorToClient: false, + ClientRedirectURI: "https://client.example.com/cb", + State: "state-abc", + } + suite.mockAuthZ.EXPECT(). + HandleAuthorizationCallback(mock.Anything, "auth-1", "the-assertion"). + Return("", authErr) + + w := suite.postCallback(`{"authId":"auth-1","assertion":"the-assertion"}`) + + suite.Equal(http.StatusOK, w.Code) + var resp oauth2authz.AuthZPostResponse + suite.NoError(json.NewDecoder(w.Body).Decode(&resp)) + suite.Contains(resp.RedirectURI, "/error") + suite.Contains(resp.RedirectURI, "errorCode="+oauth2const.ErrorServerError) + suite.NotContains(resp.RedirectURI, "client.example.com") +} + +// TestHandleFlowCallback_CIBA_ServerError_Returns500 pins the status mapping a CIBA server-side flow +// failure relies on: the polling client must see a 500, not the 400 every other CIBA error produces. +func (suite *CallbackDispatcherTestSuite) TestHandleFlowCallback_CIBA_ServerError_Returns500() { + suite.mockCIBA.EXPECT(). + HandleCallback(mock.Anything, "auth-req-1", "the-assertion"). + Return(&ciba.CIBAError{Code: oauth2const.ErrorServerError, Message: "flow failed"}) + + w := suite.postCallback( + `{"authId":"auth-req-1","assertion":"the-assertion","type":"urn:openid:params:grant-type:ciba"}`) + + suite.Equal(http.StatusInternalServerError, w.Code) + var body map[string]string + suite.NoError(json.NewDecoder(w.Body).Decode(&body)) + suite.Equal(oauth2const.ErrorServerError, body["error"]) + // The CIBA client is never redirected; it learns the outcome by polling the token endpoint. + suite.mockAuthZ.AssertNotCalled(suite.T(), "HandleAuthorizationCallback", + mock.Anything, mock.Anything, mock.Anything) +} diff --git a/backend/internal/oauth/oauth2/ciba/model.go b/backend/internal/oauth/oauth2/ciba/model.go index b1c80b6e56..ec17780eef 100644 --- a/backend/internal/oauth/oauth2/ciba/model.go +++ b/backend/internal/oauth/oauth2/ciba/model.go @@ -20,6 +20,8 @@ const ( CIBAStateConsumed CIBARequestState = "CONSUMED" // CIBAStateDenied indicates the user denied the authentication request. CIBAStateDenied CIBARequestState = "DENIED" + // CIBAStateFailed indicates the authentication flow terminated due to a server-side error. + CIBAStateFailed CIBARequestState = "FAILED" // CIBAStateExpired indicates the request expired before completion. CIBAStateExpired CIBARequestState = "EXPIRED" ) @@ -75,4 +77,5 @@ type assertionClaims struct { completedACR string authReqID string authorizedPermissions string + flowErrorType string } diff --git a/backend/internal/oauth/oauth2/ciba/service.go b/backend/internal/oauth/oauth2/ciba/service.go index 5d6d311031..fd491c1ba0 100644 --- a/backend/internal/oauth/oauth2/ciba/service.go +++ b/backend/internal/oauth/oauth2/ciba/service.go @@ -146,6 +146,7 @@ func (s *cibaService) InitiateBackchannelAuth( runtimeData := map[string]string{ flowcm.RuntimeKeyAuthorizationRequestID: authReqID, + flowcm.RuntimeKeyCallbackType: string(providers.GrantTypeCIBA), flowcm.RuntimeKeyClientID: oauthApp.ClientID, flowcm.RuntimeKeyRequestedPermissions: utils.StringifyStringArray(permissionScopes, " "), flowcm.RuntimeKeyResourceServerIdentifier: resourceServerIdentifier, @@ -219,10 +220,13 @@ func (s *cibaService) InitiateBackchannelAuth( }, nil } -// HandleCallback verifies the flow assertion, enforces the sub binding, and marks the request authenticated. -func (s *cibaService) HandleCallback(ctx context.Context, authReqID, assertion string) *CIBAError { +// loadPendingRequestForCallback loads the request a flow callback refers to and verifies the assertion +// against it: the request must exist, still be pending and unexpired, and the assertion must carry a +// valid signature for the owning client's audience. Shared by the success and failure callbacks. +func (s *cibaService) loadPendingRequestForCallback( + ctx context.Context, authReqID, assertion string) (*CIBAAuthRequest, *CIBAError) { if authReqID == "" || assertion == "" { - return &CIBAError{ + return nil, &CIBAError{ Code: oauth2const.ErrorInvalidRequest, Message: "auth_req_id and assertion are required", } @@ -231,26 +235,26 @@ func (s *cibaService) HandleCallback(ctx context.Context, authReqID, assertion s record, err := s.store.GetByID(ctx, authReqID) if err != nil { if errors.Is(err, ErrCIBARequestNotFound) { - return &CIBAError{ + return nil, &CIBAError{ Code: oauth2const.ErrorInvalidRequest, Message: "Invalid auth_req_id", } } s.logger.Error(ctx, "Failed to retrieve CIBA authentication request", log.Error(err)) - return &CIBAError{ + return nil, &CIBAError{ Code: oauth2const.ErrorServerError, Message: "Failed to process backchannel authentication callback", } } if record.State != CIBAStatePending { - return &CIBAError{ + return nil, &CIBAError{ Code: oauth2const.ErrorInvalidRequest, Message: "Backchannel authentication request is not pending", } } if record.ExpiryTime.Before(time.Now()) { - return &CIBAError{ + return nil, &CIBAError{ Code: oauth2const.ErrorExpiredToken, Message: "Backchannel authentication request has expired", } @@ -263,12 +267,25 @@ func (s *cibaService) HandleCallback(ctx context.Context, authReqID, assertion s if verifyErr := s.jwtService.VerifyJWT(ctx, assertion, expectedAud, ""); verifyErr != nil { s.logger.Debug(ctx, "Assertion verification failed", log.String("error", verifyErr.Error.DefaultValue)) - return &CIBAError{ + return nil, &CIBAError{ Code: oauth2const.ErrorInvalidRequest, Message: "Invalid assertion signature", } } + return record, nil +} + +// HandleCallback verifies the flow assertion and applies its outcome to the request. The assertion is +// either an authentication assertion from a completed flow or a signed error assertion minted when the +// flow terminated in failure; only the latter carries the flow error type claim, so that claim selects +// the branch. +func (s *cibaService) HandleCallback(ctx context.Context, authReqID, assertion string) *CIBAError { + record, cibaErr := s.loadPendingRequestForCallback(ctx, authReqID, assertion) + if cibaErr != nil { + return cibaErr + } + claims, authTime, decodeErr := decodeAttributesFromAssertion(assertion) if decodeErr != nil { s.logger.Error(ctx, "Failed to decode assertion claims", log.Error(decodeErr)) @@ -278,6 +295,21 @@ func (s *cibaService) HandleCallback(ctx context.Context, authReqID, assertion s } } + if claims.flowErrorType != "" { + // Cannot fail: the same assertion decoded successfully above. + errClaims, _ := oauth2utils.DecodeFlowErrorAssertionClaims(assertion) + return s.handleFailedCallback(ctx, record, errClaims) + } + + return s.handleSuccessCallback(ctx, record, claims, authTime) +} + +// handleSuccessCallback enforces the sub binding of a verified authentication assertion and marks the +// request authenticated. +func (s *cibaService) handleSuccessCallback(ctx context.Context, record *CIBAAuthRequest, + claims assertionClaims, authTime time.Time) *CIBAError { + authReqID := record.AuthReqID + // Bind the assertion to this specific CIBA request. The auth_req_id is threaded through the // flow runtime data into the assertion as the authorization_request_id claim; requiring it to // match the record prevents an assertion minted for one CIBA request from authorizing another @@ -328,6 +360,49 @@ func (s *cibaService) HandleCallback(ctx context.Context, authReqID, assertion s return nil } +// handleFailedCallback marks the request DENIED (end-user failure) or FAILED (server-side failure) for +// a verified error assertion, so the polling token endpoint returns access_denied or a 500. It returns +// nil once the state transition succeeds; the *CIBAError return only reports whether the callback op +// itself succeeded. +func (s *cibaService) handleFailedCallback( + ctx context.Context, record *CIBAAuthRequest, claims oauth2utils.FlowErrorAssertionClaims) *CIBAError { + authReqID := record.AuthReqID + + // Bind the assertion to this specific CIBA request (same protection as the success callback). + if claims.AuthorizationRequestID != record.AuthReqID { + s.logger.Debug(ctx, "Error assertion is not bound to the backchannel authentication request", + log.MaskedString("auth_req_id", authReqID)) + return &CIBAError{ + Code: oauth2const.ErrorInvalidRequest, + Message: "Error assertion does not match the backchannel authentication request", + } + } + + targetState := CIBAStateFailed + if claims.ErrorType == flowcm.FlowErrorTypeEndUser { + targetState = CIBAStateDenied + } else if !s.cfg.OAuth.SendServerErrorsToClientEnabled() { + // The deployment opts out of reporting server errors. There is no error page to fall back + // to here, so the request is left pending for the polling client to time out on. + s.logger.Debug(ctx, "Not failing backchannel authentication request on a server error", + log.String("flowErrorType", claims.ErrorType)) + return nil + } + s.logger.Debug(ctx, "Failing backchannel authentication request", + log.String("state", string(targetState)), + log.String("flowErrorType", claims.ErrorType), + log.String("flowErrorDescription", claims.Description)) + if err := s.UpdateState(ctx, authReqID, targetState); err != nil { + s.logger.Error(ctx, "Failed to update CIBA authentication request state", + log.String("state", string(targetState)), log.Error(err)) + return &CIBAError{ + Code: oauth2const.ErrorServerError, + Message: "Failed to process backchannel authentication callback", + } + } + return nil +} + // resolveExpectedAudience resolves the app entity ID for the given client ID, which the flow uses // as the assertion `aud`. It returns an empty string (skipping the audience check) on lookup // failure; the authorization_request_id binding remains the primary protection in that case. @@ -554,5 +629,9 @@ func decodeAttributesFromAssertion(assertion string) (assertionClaims, time.Time claims.authorizedPermissions = v } + if v, ok := payload[flowcm.ClaimFlowErrorType].(string); ok { + claims.flowErrorType = v + } + return claims, base.AuthTime, nil } diff --git a/backend/internal/oauth/oauth2/ciba/service_test.go b/backend/internal/oauth/oauth2/ciba/service_test.go index 8440ae7c4d..32a5553ef4 100644 --- a/backend/internal/oauth/oauth2/ciba/service_test.go +++ b/backend/internal/oauth/oauth2/ciba/service_test.go @@ -24,6 +24,7 @@ import ( "github.com/thunder-id/thunderid/internal/flow/flowexec" oauth2const "github.com/thunder-id/thunderid/internal/oauth/oauth2/constants" "github.com/thunder-id/thunderid/internal/system/config" + "github.com/thunder-id/thunderid/internal/system/jose/jwt" "github.com/thunder-id/thunderid/tests/mocks/authnprovider/managermock" "github.com/thunder-id/thunderid/tests/mocks/entityprovidermock" "github.com/thunder-id/thunderid/tests/mocks/flow/flowexecmock" @@ -1165,3 +1166,236 @@ func (suite *CIBAServiceTestSuite) TestInitiate_WithIDTokenHint_ExpiredWithinThr func noopAuthnMgr() *managermock.AuthnProviderManagerMock { return &managermock.AuthnProviderManagerMock{} } + +// ------------------------------------------------------------------- +// HandleFailedCallback tests +// ------------------------------------------------------------------- + +// buildErrorAssertion builds a flow error assertion bound to authReqID. +func buildErrorAssertion(authReqID, errorType, description string) string { + claims := map[string]interface{}{ + flowcm.ClaimAuthorizationRequestID: authReqID, + flowcm.ClaimFlowErrorType: errorType, + } + if description != "" { + claims[flowcm.ClaimFlowErrorDescription] = description + } + return buildTestAssertion(claims) +} + +// TestHandleCallback_ClassifiesByFlowErrorTypeClaim is the core of the single-field callback +// contract: success and failure assertions arrive in the same argument, and the flow error type claim +// alone decides which branch runs. The claim is covered by the signature, which loadPendingRequestForCallback +// verifies first, so a caller cannot steer an assertion into the other branch. +func (suite *CIBAServiceTestSuite) TestHandleCallback_ClassifiesByFlowErrorTypeClaim() { + suite.Run("AssertionWithFlowErrorTypeIsAFailure", func() { + suite.SetupTest() + assertion := buildErrorAssertion("auth-req-1", flowcm.FlowErrorTypeEndUser, "user denied consent") + suite.mockStore.EXPECT().GetByID(mock.Anything, "auth-req-1").Return(suite.pendingRecord(), nil) + suite.expectAudienceResolution() + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, assertion, "app-1", "").Return(nil) + suite.mockStore.EXPECT().UpdateState(mock.Anything, "auth-req-1", CIBAStateDenied).Return(nil) + + suite.Nil(suite.service.HandleCallback(context.Background(), "auth-req-1", assertion)) + // The failure branch never authenticates the request. + suite.mockStore.AssertNotCalled(suite.T(), "MarkAuthenticated") + }) + + suite.Run("AssertionWithoutFlowErrorTypeIsASuccess", func() { + suite.SetupTest() + assertion := buildTestAssertion(map[string]interface{}{ + "sub": testUserID, + "authorization_request_id": "auth-req-1", + "iat": float64(time.Now().Unix()), + }) + suite.mockStore.EXPECT().GetByID(mock.Anything, "auth-req-1").Return(suite.pendingRecord(), nil) + suite.expectAudienceResolution() + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, assertion, "app-1", "").Return(nil) + suite.mockStore.EXPECT().MarkAuthenticated( + mock.Anything, "auth-req-1", testUserID, + mock.AnythingOfType("string"), "", "", mock.AnythingOfType("time.Time")).Return(nil) + + suite.Nil(suite.service.HandleCallback(context.Background(), "auth-req-1", assertion)) + // The success branch never transitions the request out of PENDING. + suite.mockStore.AssertNotCalled(suite.T(), "UpdateState") + }) +} + +func (suite *CIBAServiceTestSuite) TestHandleCallback_Failure_TransitionsStateByErrorType() { + tests := []struct { + name string + errorType string + expectedState CIBARequestState + }{ + {"EndUserErrorDenies", flowcm.FlowErrorTypeEndUser, CIBAStateDenied}, + {"ServerErrorFails", flowcm.FlowErrorTypeServer, CIBAStateFailed}, + {"ClientErrorFails", flowcm.FlowErrorTypeClient, CIBAStateFailed}, + {"UnknownTypeFails", "totally_unknown", CIBAStateFailed}, + } + + for _, tt := range tests { + suite.Run(tt.name, func() { + suite.SetupTest() + assertion := buildErrorAssertion("auth-req-1", tt.errorType, "flow failed") + suite.mockStore.EXPECT().GetByID(mock.Anything, "auth-req-1").Return(suite.pendingRecord(), nil) + suite.expectAudienceResolution() + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, assertion, "app-1", "").Return(nil) + suite.mockStore.EXPECT().UpdateState(mock.Anything, "auth-req-1", tt.expectedState).Return(nil) + + svc := suite.serviceWithServerErrorReporting(true) + cibaErr := svc.HandleCallback(context.Background(), "auth-req-1", assertion) + + // A nil return means the callback op succeeded; the client's outcome comes from the state. + suite.Nil(cibaErr) + }) + } +} + +// serviceWithServerErrorReporting rebuilds the service under test with +// oauth.send_server_errors_to_client set explicitly. +func (suite *CIBAServiceTestSuite) serviceWithServerErrorReporting(enabled bool) CIBAServiceInterface { + cfg := testhelpers.OAuthConfig() + cfg.OAuth.SendServerErrorsToClient = &enabled + actorProv := actorprovider.Initialize(suite.mockInboundClient, suite.mockEntityProvider, noopAuthnMgr(), nil) + return newCIBAService(suite.mockStore, suite.mockFlowExec, + suite.mockJWTService, actorProv, suite.mockResourceSvc, cfg) +} + +// TestHandleCallback_Failure_ServerErrorsNotReported verifies that with +// oauth.send_server_errors_to_client disabled, a server-side flow failure leaves the request PENDING +// so the polling client times out rather than being told the authorization server failed. There is +// no error page to fall back to on this path, unlike the authorization code flow. +func (suite *CIBAServiceTestSuite) TestHandleCallback_Failure_ServerErrorsNotReported() { + for _, errorType := range []string{flowcm.FlowErrorTypeServer, flowcm.FlowErrorTypeClient, "totally_unknown"} { + suite.Run(errorType, func() { + suite.SetupTest() + assertion := buildErrorAssertion("auth-req-1", errorType, "flow failed") + suite.mockStore.EXPECT().GetByID(mock.Anything, "auth-req-1").Return(suite.pendingRecord(), nil) + suite.expectAudienceResolution() + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, assertion, "app-1", "").Return(nil) + + svc := suite.serviceWithServerErrorReporting(false) + cibaErr := svc.HandleCallback(context.Background(), "auth-req-1", assertion) + + suite.Nil(cibaErr) + suite.mockStore.AssertNotCalled(suite.T(), "UpdateState", mock.Anything, mock.Anything, mock.Anything) + }) + } +} + +// TestHandleCallback_Failure_DenialReportedRegardlessOfToggle verifies the toggle does not reach denials: an +// end-user failure still transitions to DENIED so the client gets access_denied on the next poll. +func (suite *CIBAServiceTestSuite) TestHandleCallback_Failure_DenialReportedRegardlessOfToggle() { + assertion := buildErrorAssertion("auth-req-1", flowcm.FlowErrorTypeEndUser, "user denied consent") + suite.mockStore.EXPECT().GetByID(mock.Anything, "auth-req-1").Return(suite.pendingRecord(), nil) + suite.expectAudienceResolution() + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, assertion, "app-1", "").Return(nil) + suite.mockStore.EXPECT().UpdateState(mock.Anything, "auth-req-1", CIBAStateDenied).Return(nil) + + svc := suite.serviceWithServerErrorReporting(false) + cibaErr := svc.HandleCallback(context.Background(), "auth-req-1", assertion) + + suite.Nil(cibaErr) +} + +// TestHandleCallback_Failure_UnsetToggleLeavesRequestPending verifies the default +func (suite *CIBAServiceTestSuite) TestHandleCallback_Failure_UnsetToggleLeavesRequestPending() { + assertion := buildErrorAssertion("auth-req-1", flowcm.FlowErrorTypeServer, "flow failed") + suite.mockStore.EXPECT().GetByID(mock.Anything, "auth-req-1").Return(suite.pendingRecord(), nil) + suite.expectAudienceResolution() + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, assertion, "app-1", "").Return(nil) + + // testhelpers.OAuthConfig() leaves SendServerErrorsToClient nil, which must default to suppressing. + suite.Require().Nil(testhelpers.OAuthConfig().OAuth.SendServerErrorsToClient) + cibaErr := suite.service.HandleCallback(context.Background(), "auth-req-1", assertion) + + suite.Nil(cibaErr) + suite.mockStore.AssertNotCalled(suite.T(), "UpdateState", mock.Anything, mock.Anything, mock.Anything) +} + +func (suite *CIBAServiceTestSuite) TestHandleCallback_Failure_InvalidSignature_NoStateChange() { + assertion := buildErrorAssertion("auth-req-1", flowcm.FlowErrorTypeEndUser, "") + suite.mockStore.EXPECT().GetByID(mock.Anything, "auth-req-1").Return(suite.pendingRecord(), nil) + suite.expectAudienceResolution() + suite.mockJWTService.EXPECT(). + VerifyJWT(mock.Anything, assertion, "app-1", "").Return(&jwt.ErrorInvalidTokenSignature) + + cibaErr := suite.service.HandleCallback(context.Background(), "auth-req-1", assertion) + + suite.NotNil(cibaErr) + suite.Equal(oauth2const.ErrorInvalidRequest, cibaErr.Code) + suite.mockStore.AssertNotCalled(suite.T(), "UpdateState", mock.Anything, mock.Anything, mock.Anything) +} + +func (suite *CIBAServiceTestSuite) TestHandleCallback_Failure_BindingMismatch_NoStateChange() { + assertion := buildErrorAssertion("other-req", flowcm.FlowErrorTypeEndUser, "") + suite.mockStore.EXPECT().GetByID(mock.Anything, "auth-req-1").Return(suite.pendingRecord(), nil) + suite.expectAudienceResolution() + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, assertion, "app-1", "").Return(nil) + + cibaErr := suite.service.HandleCallback(context.Background(), "auth-req-1", assertion) + + suite.NotNil(cibaErr) + suite.Equal(oauth2const.ErrorInvalidRequest, cibaErr.Code) + suite.mockStore.AssertNotCalled(suite.T(), "UpdateState", mock.Anything, mock.Anything, mock.Anything) +} + +func (suite *CIBAServiceTestSuite) TestHandleCallback_Failure_NotPending_NoStateChange() { + assertion := buildErrorAssertion("auth-req-1", flowcm.FlowErrorTypeEndUser, "") + record := suite.pendingRecord() + record.State = CIBAStateDenied + suite.mockStore.EXPECT().GetByID(mock.Anything, "auth-req-1").Return(record, nil) + + cibaErr := suite.service.HandleCallback(context.Background(), "auth-req-1", assertion) + + suite.NotNil(cibaErr) + suite.Equal(oauth2const.ErrorInvalidRequest, cibaErr.Code) + suite.mockStore.AssertNotCalled(suite.T(), "UpdateState", mock.Anything, mock.Anything, mock.Anything) +} + +func (suite *CIBAServiceTestSuite) TestHandleCallback_Failure_Expired_NoStateChange() { + assertion := buildErrorAssertion("auth-req-1", flowcm.FlowErrorTypeEndUser, "") + record := suite.pendingRecord() + record.ExpiryTime = time.Now().Add(-time.Minute) + suite.mockStore.EXPECT().GetByID(mock.Anything, "auth-req-1").Return(record, nil) + + cibaErr := suite.service.HandleCallback(context.Background(), "auth-req-1", assertion) + + suite.NotNil(cibaErr) + suite.Equal(oauth2const.ErrorExpiredToken, cibaErr.Code) + suite.mockStore.AssertNotCalled(suite.T(), "UpdateState", mock.Anything, mock.Anything, mock.Anything) +} + +func (suite *CIBAServiceTestSuite) TestHandleCallback_Failure_MissingParams() { + cibaErr := suite.service.HandleCallback(context.Background(), "", "assertion") + suite.NotNil(cibaErr) + suite.Equal(oauth2const.ErrorInvalidRequest, cibaErr.Code) + + cibaErr = suite.service.HandleCallback(context.Background(), "auth-req-1", "") + suite.NotNil(cibaErr) + suite.Equal(oauth2const.ErrorInvalidRequest, cibaErr.Code) +} + +func (suite *CIBAServiceTestSuite) TestHandleCallback_Failure_RequestNotFound() { + assertion := buildErrorAssertion("missing", flowcm.FlowErrorTypeEndUser, "") + suite.mockStore.EXPECT().GetByID(mock.Anything, "missing").Return(nil, ErrCIBARequestNotFound) + + cibaErr := suite.service.HandleCallback(context.Background(), "missing", assertion) + + suite.NotNil(cibaErr) + suite.Equal(oauth2const.ErrorInvalidRequest, cibaErr.Code) +} + +func (suite *CIBAServiceTestSuite) TestHandleCallback_Failure_UpdateStateFailure_ReturnsServerError() { + assertion := buildErrorAssertion("auth-req-1", flowcm.FlowErrorTypeEndUser, "") + suite.mockStore.EXPECT().GetByID(mock.Anything, "auth-req-1").Return(suite.pendingRecord(), nil) + suite.expectAudienceResolution() + suite.mockJWTService.EXPECT().VerifyJWT(mock.Anything, assertion, "app-1", "").Return(nil) + suite.mockStore.EXPECT(). + UpdateState(mock.Anything, "auth-req-1", CIBAStateDenied).Return(errors.New("db down")) + + cibaErr := suite.service.HandleCallback(context.Background(), "auth-req-1", assertion) + + suite.NotNil(cibaErr) + suite.Equal(oauth2const.ErrorServerError, cibaErr.Code) +} diff --git a/backend/internal/oauth/oauth2/granthandlers/ciba.go b/backend/internal/oauth/oauth2/granthandlers/ciba.go index 1589ad6d51..188258ad57 100644 --- a/backend/internal/oauth/oauth2/granthandlers/ciba.go +++ b/backend/internal/oauth/oauth2/granthandlers/ciba.go @@ -147,6 +147,11 @@ func (h *cibaGrantHandler) HandleGrant(ctx context.Context, tokenRequest *model. Error: constants.ErrorAccessDenied, ErrorDescription: "The user denied the authentication request", } + case ciba.CIBAStateFailed: + return nil, &model.ErrorResponse{ + Error: constants.ErrorServerError, + ErrorDescription: "Authentication could not be completed due to a server error", + } case ciba.CIBAStateConsumed: return nil, &model.ErrorResponse{ Error: constants.ErrorInvalidGrant, diff --git a/backend/internal/oauth/oauth2/granthandlers/ciba_test.go b/backend/internal/oauth/oauth2/granthandlers/ciba_test.go index f48fc2a36a..8fedf5c127 100644 --- a/backend/internal/oauth/oauth2/granthandlers/ciba_test.go +++ b/backend/internal/oauth/oauth2/granthandlers/ciba_test.go @@ -194,6 +194,18 @@ func (suite *CIBAGrantHandlerTestSuite) TestHandleGrant_Denied() { suite.Equal(constants.ErrorAccessDenied, errResp.Error) } +func (suite *CIBAGrantHandlerTestSuite) TestHandleGrant_Failed() { + // A server-side flow failure surfaces as server_error, which the token endpoint maps to HTTP 500. + record := suite.pendingRecord() + record.State = ciba.CIBAStateFailed + suite.mockCIBAService.EXPECT().GetByAuthReqID(mock.Anything, "auth-req-1").Return(record, nil) + + resp, errResp := suite.handler.HandleGrant(context.Background(), suite.tokenReq, suite.oauthApp) + suite.Nil(resp) + suite.NotNil(errResp) + suite.Equal(constants.ErrorServerError, errResp.Error) +} + func (suite *CIBAGrantHandlerTestSuite) TestHandleGrant_Consumed() { record := suite.pendingRecord() record.State = ciba.CIBAStateConsumed diff --git a/backend/internal/oauth/oauth2/utils/assertion.go b/backend/internal/oauth/oauth2/utils/assertion.go index 6aafe27501..493c30d0e2 100644 --- a/backend/internal/oauth/oauth2/utils/assertion.go +++ b/backend/internal/oauth/oauth2/utils/assertion.go @@ -8,6 +8,7 @@ import ( "fmt" "time" + flowcm "github.com/thunder-id/thunderid/internal/flow/common" oauth2const "github.com/thunder-id/thunderid/internal/oauth/oauth2/constants" "github.com/thunder-id/thunderid/internal/system/jose/jwt" sysutils "github.com/thunder-id/thunderid/internal/system/utils" @@ -23,6 +24,31 @@ type FlowAssertionClaims struct { AuthTime time.Time } +// FlowErrorAssertionClaims holds the claims of a flow error assertion, minted by the flow service +// when an OAuth-initiated flow terminates in failure. +type FlowErrorAssertionClaims struct { + AuthorizationRequestID string + ErrorType string + Description string +} + +// DecodeFlowErrorAssertionClaims decodes a flow error assertion JWT. Callers must verify the +// signature and the authorization request binding before acting on the claims. +func DecodeFlowErrorAssertionClaims(assertion string) (FlowErrorAssertionClaims, error) { + claims := FlowErrorAssertionClaims{} + + _, jwtPayload, err := jwt.DecodeJWT(assertion) + if err != nil { + return claims, fmt.Errorf("failed to decode the JWT token: %w", err) + } + + claims.AuthorizationRequestID, _ = jwtPayload[flowcm.ClaimAuthorizationRequestID].(string) + claims.ErrorType, _ = jwtPayload[flowcm.ClaimFlowErrorType].(string) + claims.Description, _ = jwtPayload[flowcm.ClaimFlowErrorDescription].(string) + + return claims, nil +} + // DecodeFlowAssertionClaims decodes the common flow assertion claims from a JWT string. // It extracts sub (user ID), aci (attribute cache ID), completed_auth_class (completed ACR), // and iat (authentication time). The raw JWT payload is also returned so callers can extract diff --git a/backend/internal/oauth/oauth2/utils/oauthutils.go b/backend/internal/oauth/oauth2/utils/oauthutils.go index 84bffab9e8..25980ac62d 100644 --- a/backend/internal/oauth/oauth2/utils/oauthutils.go +++ b/backend/internal/oauth/oauth2/utils/oauthutils.go @@ -10,6 +10,7 @@ import ( "encoding/json" "fmt" "regexp" + "strings" "github.com/thunder-id/thunderid/internal/oauth/oauth2/constants" "github.com/thunder-id/thunderid/internal/oauth/oauth2/model" @@ -28,25 +29,47 @@ func GetURIWithQueryParams(uri string, queryParams map[string]string) (string, e return utils.GetURIWithQueryParams(uri, queryParams) } +// allowedErrorParamChars matches the character set permitted for the error and error_description +// parameters: %x20-21 / %x23-5B / %x5D-7E. +var allowedErrorParamChars = regexp.MustCompile(`^[\x20-\x21\x23-\x5B\x5D-\x7E]*$`) + +// maxErrorDescriptionLength bounds the error description carried in a client redirect. +const maxErrorDescriptionLength = 256 + // validateErrorParams validates the error code and error description parameters. func validateErrorParams(err, desc string) error { - // Define a regex pattern for the allowed character set: %x20-21 / %x23-5B / %x5D-7E - allowedCharPattern := `^[\x20-\x21\x23-\x5B\x5D-\x7E]*$` - allowedCharRegex := regexp.MustCompile(allowedCharPattern) - // Validate the error code. - if err != "" && !allowedCharRegex.MatchString(err) { + if err != "" && !allowedErrorParamChars.MatchString(err) { return fmt.Errorf("invalid error code: %s", err) } // Validate the error description. - if desc != "" && !allowedCharRegex.MatchString(desc) { + if desc != "" && !allowedErrorParamChars.MatchString(desc) { return fmt.Errorf("invalid error description: %s", desc) } return nil } +// SanitizeErrorDescription drops characters the spec disallows in an error description and truncates +// the result, so a description sourced from a flow error cannot make the client redirect unbuildable. +// It returns "" when nothing usable remains, letting the caller fall back to its own message. +func SanitizeErrorDescription(desc string) string { + sanitized := strings.Map(func(r rune) rune { + if allowedErrorParamChars.MatchString(string(r)) { + return r + } + return -1 + }, desc) + + sanitized = strings.TrimSpace(sanitized) + if len(sanitized) > maxErrorDescriptionLength { + sanitized = strings.TrimSpace(sanitized[:maxErrorDescriptionLength]) + } + + return sanitized +} + const ( // OAuth2ClientIDLength specifies the byte length for OAuth client IDs (16 bytes = 128 bits) // This provides sufficient entropy while keeping the resulting base64 string reasonably short diff --git a/backend/internal/oauth/oauth2/utils/oauthutils_test.go b/backend/internal/oauth/oauth2/utils/oauthutils_test.go index 3478dec012..ec33e51e78 100644 --- a/backend/internal/oauth/oauth2/utils/oauthutils_test.go +++ b/backend/internal/oauth/oauth2/utils/oauthutils_test.go @@ -1815,3 +1815,43 @@ func (suite *OAuth2UtilsTestSuite) TestResolveEffectiveScopeClaims_DoesNotMutate suite.Equal([]string{"email", "email_verified"}, constants.StandardOIDCScopes["email"].Claims, "the shared StandardOIDCScopes constant must not be mutated via the returned map") } + +func (suite *OAuth2UtilsTestSuite) TestSanitizeErrorDescription() { + longDesc := strings.Repeat("a", maxErrorDescriptionLength+50) + + tests := []struct { + name string + input string + expected string + }{ + {"CleanDescriptionUnchanged", "User denied consent", "User denied consent"}, + {"DropsDoubleQuote", `Denied "email"`, "Denied email"}, + {"DropsBackslash", `back\slash`, "backslash"}, + {"DropsControlCharacters", "line one\nline two\ttab", "line oneline twotab"}, + {"DropsNonASCII", "café dénied", "caf dnied"}, + {"TrimsSurroundingSpace", " spaced ", "spaced"}, + {"EmptyStaysEmpty", "", ""}, + {"OnlyDisallowedBecomesEmpty", "\n\t\"\\", ""}, + {"TruncatesToLimit", longDesc, strings.Repeat("a", maxErrorDescriptionLength)}, + } + + for _, tt := range tests { + suite.Run(tt.name, func() { + assert.Equal(suite.T(), tt.expected, SanitizeErrorDescription(tt.input)) + }) + } +} + +func (suite *OAuth2UtilsTestSuite) TestSanitizeErrorDescriptionKeepsRedirectBuildable() { + // Whatever the flow reports, the sanitized result must always pass the spec charset check so the + // client redirect can still be constructed. + dirty := "Denied \"email\"\né back\\slash" + + uri, err := GetURIWithQueryParams("https://client.example.com/callback", map[string]string{ + constants.RequestParamError: constants.ErrorAccessDenied, + constants.RequestParamErrorDescription: SanitizeErrorDescription(dirty), + }) + + assert.NoError(suite.T(), err) + assert.Contains(suite.T(), uri, "error=access_denied") +} diff --git a/backend/internal/system/config/config_test.go b/backend/internal/system/config/config_test.go index 809c80d431..255e9c5b27 100644 --- a/backend/internal/system/config/config_test.go +++ b/backend/internal/system/config/config_test.go @@ -458,6 +458,36 @@ func (suite *ConfigTestSuite) TestLogConfigOverrideToZeroValues() { assert.Equal(suite.T(), boolPtr(true), base2.Log.Output.Console.Enabled, "nil override keeps the default") } +// TestOAuthSendServerErrorsToClientOverride verifies the presence-based (pointer) merge for +// the OAuth.SendServerErrorsToClient field: +func (suite *ConfigTestSuite) TestOAuthSendServerErrorsToClientOverride() { + base := &Config{} + base.OAuth.SendServerErrorsToClient = boolPtr(false) + + user := &Config{} + user.OAuth.SendServerErrorsToClient = boolPtr(true) + + mergeConfigs(base, user) + assert.Equal(suite.T(), boolPtr(true), base.OAuth.SendServerErrorsToClient, + "true must override the false default") + + // The reverse direction is what a plain bool could not express: a zero-valued user field is + // skipped by the merge, so only the pointer lets deployment.yaml turn the toggle back off. + base2 := &Config{} + base2.OAuth.SendServerErrorsToClient = boolPtr(true) + user2 := &Config{} + user2.OAuth.SendServerErrorsToClient = boolPtr(false) + mergeConfigs(base2, user2) + assert.Equal(suite.T(), boolPtr(false), base2.OAuth.SendServerErrorsToClient, + "explicit false must override a true base") + + base3 := &Config{} + base3.OAuth.SendServerErrorsToClient = boolPtr(false) + mergeConfigs(base3, &Config{}) + assert.Equal(suite.T(), boolPtr(false), base3.OAuth.SendServerErrorsToClient, + "nil override keeps the default") +} + func (suite *ConfigTestSuite) TestLoadConfigWithDefaults_ErrorCases() { tempDir := suite.T().TempDir() diff --git a/backend/internal/system/jose/jwt/service.go b/backend/internal/system/jose/jwt/service.go index 4c254f89c2..3174d82bc8 100644 --- a/backend/internal/system/jose/jwt/service.go +++ b/backend/internal/system/jose/jwt/service.go @@ -161,13 +161,15 @@ func (js *jwtService) GenerateJWT( } defaultClaims := map[string]interface{}{ - "sub": sub, "iss": tokenIssuer, "exp": expirationTime, "iat": iat.Unix(), "nbf": iat.Unix(), "jti": jti, } + if sub != "" { + defaultClaims["sub"] = sub + } payload := make(map[string]interface{}, len(claims)+len(defaultClaims)) maps.Copy(payload, claims) diff --git a/backend/internal/system/jose/jwt/service_test.go b/backend/internal/system/jose/jwt/service_test.go index 3ee2b49f86..d5f5abe040 100644 --- a/backend/internal/system/jose/jwt/service_test.go +++ b/backend/internal/system/jose/jwt/service_test.go @@ -594,7 +594,8 @@ func (suite *JWTServiceTestSuite) TestGenerateJWTScenarios() { err = json.Unmarshal(payloadBytes, &payload) assert.NoError(t, err) - assert.Equal(t, "", payload["sub"]) + _, hasSub := payload["sub"] + assert.False(t, hasSub, "sub claim should be omitted when the subject is empty") }, }, { diff --git a/backend/pkg/thunderidengine/config/config.go b/backend/pkg/thunderidengine/config/config.go index 7397a824cc..36c3114827 100644 --- a/backend/pkg/thunderidengine/config/config.go +++ b/backend/pkg/thunderidengine/config/config.go @@ -248,11 +248,21 @@ type OAuthConfig struct { AllowedResponseTypes []string `yaml:"allowed_response_types" json:"allowed_response_types"` // AllowedAuthMethods lists allowed client token endpoint auth methods AllowedAuthMethods []string `yaml:"allowed_auth_methods" json:"allowed_auth_methods"` + // SendServerErrorsToClient controls whether a flow failure that maps to the OAuth + // server_error code is reported to the client. Denials (access_denied) are always + // reported and are not affected. Nil means unset; the default lives in default.json. + SendServerErrorsToClient *bool `yaml:"send_server_errors_to_client" json:"send_server_errors_to_client"` TokenRevocation OAuthTokenRevocationConfig `yaml:"token_revocation" json:"token_revocation"` Logout LogoutConfig `yaml:"logout" json:"logout"` } +// SendServerErrorsToClientEnabled reports whether server errors reach the client, defaulting to +// false when unset so that a missing key does not disclose an internal failure to the client. +func (c OAuthConfig) SendServerErrorsToClientEnabled() bool { + return c.SendServerErrorsToClient != nil && *c.SendServerErrorsToClient +} + // OAuthTokenRevocationConfig holds the configuration details for the token revocation feature type OAuthTokenRevocationConfig struct { // Enabled controls whether the OAuth token revocation endpoint is active. It uses a pointer diff --git a/backend/pkg/thunderidengine/config/validate_test.go b/backend/pkg/thunderidengine/config/validate_test.go index bddbd259fe..fd4971883f 100644 --- a/backend/pkg/thunderidengine/config/validate_test.go +++ b/backend/pkg/thunderidengine/config/validate_test.go @@ -302,3 +302,26 @@ func (suite *ValidateTestSuite) TestCORSConfig_Validate() { assert.Error(t, cors.Validate(origins)) }) } + +// ----- OAuthConfig.SendServerErrorsToClientEnabled ----- + +// TestOAuthConfig_SendServerErrorsToClientEnabled tests the behavior of the SendServerErrorsToClientEnabled method. +func (suite *ValidateTestSuite) TestOAuthConfig_SendServerErrorsToClientEnabled() { + enabled, disabled := true, false + tests := []struct { + name string + value *bool + expected bool + }{ + {"UnsetDefaultsToSuppressing", nil, false}, + {"ExplicitTrue", &enabled, true}, + {"ExplicitFalse", &disabled, false}, + } + + for _, tt := range tests { + suite.T().Run(tt.name, func(t *testing.T) { + c := OAuthConfig{SendServerErrorsToClient: tt.value} + assert.Equal(t, tt.expected, c.SendServerErrorsToClientEnabled()) + }) + } +} diff --git a/backend/pkg/thunderidengine/engine.go b/backend/pkg/thunderidengine/engine.go index 344f6050c6..d8af590d6a 100644 --- a/backend/pkg/thunderidengine/engine.go +++ b/backend/pkg/thunderidengine/engine.go @@ -159,7 +159,7 @@ func New(mux *http.ServeMux, opts ...Option) *Engine { engineCtx.flowExecService, err = flowexec.Initialize(mux, engineCtx.flowProvider, engineCtx.actorProvider, engineCtx.execRegistry, engineCtx.interceptorRegistry, engineCtx.observabilitySvc, engineCtx.runtimeCryptoSvc, engineCtx.attestationProvider, engineCtx.graphBuilder, - engineCtx.runtimeStoreProvider, engineCtx.transactioner, nil, flowConfig) + engineCtx.jwtService, engineCtx.runtimeStoreProvider, engineCtx.transactioner, nil, flowConfig) if err != nil { logger.Fatal(ctx, "Failed to initialize flow execution service", log.Error(err)) } diff --git a/docs/content/deployment/configuration.mdx b/docs/content/deployment/configuration.mdx index 26298f5692..5c10657a5b 100644 --- a/docs/content/deployment/configuration.mdx +++ b/docs/content/deployment/configuration.mdx @@ -459,6 +459,7 @@ OAuth 2.0 and OpenID Connect settings. | `oauth.allowed_response_types` | `["code"]` | OAuth response types allowed during client registration | | `oauth.allowed_grant_types` | `["client_credentials", "authorization_code", "refresh_token", "urn:ietf:params:oauth:grant-type:token-exchange", "urn:openid:params:grant-type:ciba", "urn:ietf:params:oauth:grant-type:jwt-bearer"]` | OAuth grant types allowed during client registration | | `oauth.allow_wildcard_redirect_uri` | `false` | If `true`, allows wildcard patterns in registered redirect URIs: `*` and `**` in the path component, and `*` in the host component (label-internal, alphanumeric only). When `false`, only exact redirect URI matching is performed and registering a wildcard URI returns a `400 Bad Request` error. | +| `oauth.send_server_errors_to_client` | `false` | If `true`, an authentication flow failure that maps to the OAuth `server_error` code is reported to the client, as RFC 6749 section 4.1.2.1 requires. If `false`, the authorization code flow shows the error page instead of redirecting to the client, and CIBA leaves the request pending so the polling client times out. Denials (`access_denied`) are always reported to the client and are not affected by this setting. | :::note Enabling `oauth.allow_wildcard_redirect_uri` affects all applications in the deployment. See [Use Wildcard Redirect URIs](../../guides/applications/application-settings#use-wildcard-redirect-uris) for pattern syntax and matching rules. diff --git a/frontend/apps/gate/src/components/AcceptInvite/AcceptInviteBox.tsx b/frontend/apps/gate/src/components/AcceptInvite/AcceptInviteBox.tsx index d3eef1a296..422e306dd5 100644 --- a/frontend/apps/gate/src/components/AcceptInvite/AcceptInviteBox.tsx +++ b/frontend/apps/gate/src/components/AcceptInvite/AcceptInviteBox.tsx @@ -15,6 +15,7 @@ import RouteConfig from '../../configs/RouteConfig'; export interface FlowChangeResponse { flowStatus?: string; assertion?: string; + errorAssertion?: string; data?: {additionalData?: Record}; error?: { code?: string; @@ -44,17 +45,18 @@ export default function AcceptInviteBox(): JSX.Element { }; /** - * Posts the completed flow assertion to the callback endpoint. - * Requires authId (from URL params) and assertion (from flow completion). - * callbackType is optional — the backend defaults to authorization_code when absent. + * Posts a flow outcome to the callback endpoint: a completed flow's assertion (success) or a signed + * error assertion (terminal failure). Both go in the same field; the backend tells them apart from + * the assertion's own claims. Requires authId (from URL params). callbackType is optional; the + * backend defaults to authorization_code when absent. * * Response handling is generic: - * - redirect_uri present → redirect the browser (e.g. auth code flow) - * - redirect_uri absent → no redirect; the flow's completion components display the outcome + * - redirect_uri present: redirect the browser (e.g. auth code flow, or an error redirect) + * - redirect_uri absent: no redirect; the flow's completion components display the outcome */ const handleFlowCallback = async (authId: string, assertion: string, callbackType?: string): Promise => { try { - const body: Record = {authId, assertion}; + const body: Record = {assertion, authId}; if (callbackType) { body.type = callbackType; } @@ -104,25 +106,32 @@ export default function AcceptInviteBox(): JSX.Element { }} onFlowChange={(response: FlowChangeResponse) => { const messageKey: string | undefined = response?.error?.message?.key; - if (messageKey) { - const translated: string = t(messageKey); - if (translated !== messageKey) { - setFlowError(translated); + const translated: string | undefined = messageKey ? t(messageKey) : undefined; + + if (translated && translated !== messageKey) { + setFlowError(translated); + } else { + const fallback: string | undefined = + response?.error?.message?.defaultValue ?? response?.error?.description?.defaultValue; + setFlowError(fallback ?? null); + } - return; - } + // Relay the terminal flow outcome to the waiting OAuth request: the assertion on success, or + // the signed error assertion on failure (auth code: error redirect; CIBA: request denied). + // flowStatus selects which field to read, since both are relayed in the same request field. + let assertion: string | undefined; + + if (response.flowStatus === 'COMPLETE') { + assertion = response.assertion; + } else if (response.flowStatus === 'ERROR') { + assertion = response.errorAssertion; } - const fallback: string | undefined = - response?.error?.message?.defaultValue ?? response?.error?.description?.defaultValue; - setFlowError(fallback ?? null); - if (response.flowStatus === 'COMPLETE' && response.assertion) { + if (assertion) { const authId = searchParams.get('auth_req_id') ?? searchParams.get('authId'); - const {assertion} = response; - const callbackType = response.data?.additionalData?.callbackType; - if (authId && assertion) { - void handleFlowCallback(authId, assertion, callbackType); + if (authId) { + void handleFlowCallback(authId, assertion, response.data?.additionalData?.callbackType); } } }} diff --git a/frontend/apps/gate/src/components/AcceptInvite/__tests__/AcceptInviteBox.test.tsx b/frontend/apps/gate/src/components/AcceptInvite/__tests__/AcceptInviteBox.test.tsx index 0b837d3790..4cc95564cf 100644 --- a/frontend/apps/gate/src/components/AcceptInvite/__tests__/AcceptInviteBox.test.tsx +++ b/frontend/apps/gate/src/components/AcceptInvite/__tests__/AcceptInviteBox.test.tsx @@ -56,6 +56,16 @@ const render = (ui: React.ReactElement) => { return testRender({ui}); }; +// Mock react-i18next so a test can make a specific message key resolve to a translation, the way a +// deployment-supplied translation bundle does. Keys with no entry fall back to the caller's default +// value, matching the untranslated behavior the other tests rely on. +const {mockTranslations} = vi.hoisted(() => ({mockTranslations: {} as Record})); +vi.mock('react-i18next', () => ({ + useTranslation: () => ({ + t: (key: string, defaultValue?: string) => mockTranslations[key] ?? defaultValue ?? key, + }), +})); + // Mock useTemplateLiteralResolver vi.mock('@thunderid/hooks', () => ({ useTemplateLiteralResolver: () => ({ @@ -1625,5 +1635,106 @@ describe('AcceptInviteBox', () => { await new Promise((r) => setTimeout(r, 50)); expect(mockFetch).not.toHaveBeenCalled(); }); + + it('relays the errorAssertion in the shared assertion field when the flow fails', async () => { + const mockFetch = vi.fn().mockResolvedValue({ + ok: true, + json: () => Promise.resolve({}), + }); + vi.stubGlobal('fetch', mockFetch); + + render(); + + capturedOnFlowChange?.({ + flowStatus: 'ERROR', + errorAssertion: 'test-error-assertion', + data: {additionalData: {callbackType: 'urn:openid:params:grant-type:ciba'}}, + }); + + await waitFor(() => { + expect(mockFetch).toHaveBeenCalled(); + }); + + const callBody = JSON.parse((mockFetch.mock.calls[0][1] as {body: string}).body) as { + authId?: string; + assertion?: string; + errorAssertion?: string; + type?: string; + }; + expect(callBody.authId).toBe('ciba-req-123'); + // Success and failure assertions share one field; the backend tells them apart by their claims. + expect(callBody.assertion).toBe('test-error-assertion'); + expect(callBody.errorAssertion).toBeUndefined(); + expect(callBody.type).toBe('urn:openid:params:grant-type:ciba'); + }); + + it('relays the errorAssertion when the failure carries a translatable message key', async () => { + mockTranslations['error.flowexecservice.flow_failed'] = 'Localized flow error'; + const mockFetch = vi.fn().mockResolvedValue({ + ok: true, + json: () => Promise.resolve({}), + }); + vi.stubGlobal('fetch', mockFetch); + + render(); + + capturedOnFlowChange?.({ + flowStatus: 'ERROR', + errorAssertion: 'test-error-assertion', + error: { + code: 'FEE-60001', + message: {key: 'error.flowexecservice.flow_failed', defaultValue: 'Flow error'}, + }, + }); + + // The localized message is displayed, and the failure still reaches the waiting OAuth request. + expect(await screen.findByText('Localized flow error')).toBeInTheDocument(); + await waitFor(() => { + expect(mockFetch).toHaveBeenCalled(); + }); + + const callBody = JSON.parse((mockFetch.mock.calls[0][1] as {body: string}).body) as { + assertion?: string; + authId?: string; + }; + expect(callBody.authId).toBe('ciba-req-123'); + expect(callBody.assertion).toBe('test-error-assertion'); + + delete mockTranslations['error.flowexecservice.flow_failed']; + }); + + it('redirects when the failure callback returns a redirect_uri', async () => { + const mockFetch = vi.fn().mockResolvedValue({ + ok: true, + json: () => + Promise.resolve({ + redirect_uri: 'https://client.example.com/callback?error=access_denied&state=xyz', + }), + }); + vi.stubGlobal('fetch', mockFetch); + const assignSpy = vi.fn(); + Object.defineProperty(window, 'location', {value: {href: ''}, writable: true}); + Object.defineProperty(window.location, 'href', {set: assignSpy, configurable: true}); + + render(); + + capturedOnFlowChange?.({flowStatus: 'ERROR', errorAssertion: 'test-error-assertion'}); + + await waitFor(() => { + expect(assignSpy).toHaveBeenCalledWith('https://client.example.com/callback?error=access_denied&state=xyz'); + }); + }); + + it('does not call callback when the failed flow carries no errorAssertion', async () => { + const mockFetch = vi.fn(); + vi.stubGlobal('fetch', mockFetch); + + render(); + + capturedOnFlowChange?.({flowStatus: 'ERROR'}); + + await new Promise((r) => setTimeout(r, 50)); + expect(mockFetch).not.toHaveBeenCalled(); + }); }); }); diff --git a/install/helm/README.md b/install/helm/README.md index 908ec9f991..be418d1a10 100644 --- a/install/helm/README.md +++ b/install/helm/README.md @@ -504,6 +504,7 @@ Password fields are available in `configuration.database.config.postgres`, `conf | `configuration.oauth.refreshToken.validityPeriod` | Refresh token validity period in seconds | `86400` | | `configuration.oauth.authorizationCode.validityPeriod` | Authorization code validity period in seconds | `600` | | `configuration.oauth.authorizationRequest.validityPeriod` | How long the authorization request context stays valid while the user completes the login flow, in seconds | `3600` | +| `configuration.oauth.sendServerErrorsToClient` | Report an authentication flow failure that maps to the OAuth `server_error` code to the client | `false` | | `configuration.flow.maxVersionHistory` | Maximum flow version history to retain | `3` | | `configuration.flow.autoInferRegistration` | Enable auto-infer registration flow | `true` | | `configuration.passkey.allowedOrigins` | Passkey allowed origins | `[]` | diff --git a/install/helm/conf/deployment.yaml b/install/helm/conf/deployment.yaml index df27b1ff66..1f467264fa 100644 --- a/install/helm/conf/deployment.yaml +++ b/install/helm/conf/deployment.yaml @@ -240,6 +240,7 @@ oauth: dcr: enabled: {{ .Values.configuration.oauth.dcr.enabled }} insecure: {{ .Values.configuration.oauth.dcr.insecure }} + send_server_errors_to_client: {{ .Values.configuration.oauth.sendServerErrorsToClient }} allowed_auth_methods: {{- range .Values.configuration.oauth.allowedAuthMethods }} - {{ . | quote }} diff --git a/install/helm/values.yaml b/install/helm/values.yaml index 551e3f4be2..1bc27ea18d 100644 --- a/install/helm/values.yaml +++ b/install/helm/values.yaml @@ -412,6 +412,8 @@ configuration: dcr: enabled: true insecure: false + # Report an authentication flow failure that maps to the OAuth server_error code to the client. + sendServerErrorsToClient: false # Client token endpoint auth methods allowed during registration. allowedAuthMethods: - "client_secret_basic" diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 27cc70b09e..8c1269ce5e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -242,11 +242,11 @@ catalogs: specifier: 14.6.1 version: 14.6.1 '@thunderid/react': - specifier: 0.11.3 - version: 0.11.3 + specifier: 0.11.4 + version: 0.11.4 '@thunderid/react-router': - specifier: 0.10.2 - version: 0.10.2 + specifier: 0.10.4 + version: 0.10.4 '@types/lodash-es': specifier: 4.17.12 version: 4.17.12 @@ -502,7 +502,7 @@ importers: version: link:../frontend/packages/prettier-config '@thunderid/react': specifier: 'catalog:' - version: 0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) + version: 0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) '@thunderid/utils': specifier: workspace:^ version: link:../frontend/packages/utils @@ -695,10 +695,10 @@ importers: version: link:../../packages/logger '@thunderid/react': specifier: 'catalog:' - version: 0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) + version: 0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) '@thunderid/react-router': specifier: 'catalog:' - version: 0.10.2(@thunderid/react@0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react-router@8.3.0(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3) + version: 0.10.4(@thunderid/browser@0.11.4)(@thunderid/react@0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react-router@8.3.0(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3) '@thunderid/utils': specifier: workspace:^ version: link:../../packages/utils @@ -882,10 +882,10 @@ importers: version: link:../../packages/logger '@thunderid/react': specifier: 'catalog:' - version: 0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) + version: 0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) '@thunderid/react-router': specifier: 'catalog:' - version: 0.10.2(@thunderid/react@0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react-router@8.3.0(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3) + version: 0.10.4(@thunderid/browser@0.11.4)(@thunderid/react@0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react-router@8.3.0(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3) '@thunderid/utils': specifier: workspace:^ version: link:../../packages/utils @@ -1030,7 +1030,7 @@ importers: version: 5.90.5(react@19.2.3) '@thunderid/react': specifier: 'catalog:' - version: 0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) + version: 0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) '@wso2/oxygen-ui': specifier: 'catalog:' version: 0.13.0(@mui/system@7.3.11(@emotion/react@11.14.0(@types/react@19.2.14)(react@19.2.3))(@emotion/styled@11.14.1(@emotion/react@11.14.0(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(@wso2/oxygen-ui-icons-react@0.13.0(react@19.2.3))(dayjs@1.11.21)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) @@ -1130,7 +1130,7 @@ importers: version: 5.90.5(react@19.2.3) '@thunderid/react': specifier: 'catalog:' - version: 0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) + version: 0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) '@wso2/oxygen-ui': specifier: 'catalog:' version: 0.13.0(@mui/system@7.3.11(@emotion/react@11.14.0(@types/react@19.2.14)(react@19.2.3))(@emotion/styled@11.14.1(@emotion/react@11.14.0(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(@wso2/oxygen-ui-icons-react@0.13.0(react@19.2.3))(dayjs@1.11.21)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) @@ -1251,7 +1251,7 @@ importers: version: link:../prettier-config '@thunderid/react': specifier: 'catalog:' - version: 0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) + version: 0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) '@thunderid/test-utils': specifier: workspace:^ version: link:../test-utils @@ -1302,7 +1302,7 @@ importers: version: 5.90.5(react@19.2.3) '@thunderid/react': specifier: 'catalog:' - version: 0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) + version: 0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) '@wso2/oxygen-ui': specifier: 'catalog:' version: 0.13.0(@mui/system@7.3.11(@emotion/react@11.14.0(@types/react@19.2.14)(react@19.2.3))(@emotion/styled@11.14.1(@emotion/react@11.14.0(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(@wso2/oxygen-ui-icons-react@0.13.0(react@19.2.3))(dayjs@1.11.21)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) @@ -1402,7 +1402,7 @@ importers: version: 5.90.5(react@19.2.3) '@thunderid/react': specifier: 'catalog:' - version: 0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) + version: 0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) '@wso2/oxygen-ui': specifier: 'catalog:' version: 0.13.0(@mui/system@7.3.11(@emotion/react@11.14.0(@types/react@19.2.14)(react@19.2.3))(@emotion/styled@11.14.1(@emotion/react@11.14.0(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(@wso2/oxygen-ui-icons-react@0.13.0(react@19.2.3))(dayjs@1.11.21)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) @@ -1520,7 +1520,7 @@ importers: version: 5.90.5(react@19.2.3) '@thunderid/react': specifier: 'catalog:' - version: 0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) + version: 0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) '@wso2/oxygen-ui': specifier: 'catalog:' version: 0.13.0(@mui/system@7.3.11(@emotion/react@11.14.0(@types/react@19.2.14)(react@19.2.3))(@emotion/styled@11.14.1(@emotion/react@11.14.0(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(@wso2/oxygen-ui-icons-react@0.13.0(react@19.2.3))(dayjs@1.11.21)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) @@ -1635,7 +1635,7 @@ importers: version: 5.90.5(react@19.2.3) '@thunderid/react': specifier: 'catalog:' - version: 0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) + version: 0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) '@wso2/oxygen-ui': specifier: 'catalog:' version: 0.13.0(@mui/system@7.3.11(@emotion/react@11.14.0(@types/react@19.2.14)(react@19.2.3))(@emotion/styled@11.14.1(@emotion/react@11.14.0(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(@wso2/oxygen-ui-icons-react@0.13.0(react@19.2.3))(dayjs@1.11.21)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) @@ -1738,7 +1738,7 @@ importers: version: 5.90.5(react@19.2.3) '@thunderid/react': specifier: 'catalog:' - version: 0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) + version: 0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) '@wso2/oxygen-ui': specifier: 'catalog:' version: 0.13.0(@mui/system@7.3.11(@emotion/react@11.14.0(@types/react@19.2.14)(react@19.2.3))(@emotion/styled@11.14.1(@emotion/react@11.14.0(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(@wso2/oxygen-ui-icons-react@0.13.0(react@19.2.3))(dayjs@1.11.21)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) @@ -1856,7 +1856,7 @@ importers: version: 5.90.5(react@19.2.3) '@thunderid/react': specifier: 'catalog:' - version: 0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) + version: 0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) '@wso2/oxygen-ui': specifier: 'catalog:' version: 0.13.0(@mui/system@7.3.11(@emotion/react@11.14.0(@types/react@19.2.14)(react@19.2.3))(@emotion/styled@11.14.1(@emotion/react@11.14.0(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(@wso2/oxygen-ui-icons-react@0.13.0(react@19.2.3))(dayjs@1.11.21)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) @@ -1992,7 +1992,7 @@ importers: version: link:../prettier-config '@thunderid/react': specifier: 'catalog:' - version: 0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) + version: 0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) '@thunderid/test-utils': specifier: workspace:^ version: link:../test-utils @@ -2168,7 +2168,7 @@ importers: version: link:../logger '@thunderid/react': specifier: 'catalog:' - version: 0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) + version: 0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) '@thunderid/utils': specifier: workspace:^ version: link:../utils @@ -2429,7 +2429,7 @@ importers: version: 5.90.5(react@19.2.3) '@thunderid/react': specifier: 'catalog:' - version: 0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) + version: 0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) i18next: specifier: 25.6.0 version: 25.6.0(typescript@5.9.3) @@ -2761,7 +2761,7 @@ importers: dependencies: '@thunderid/react': specifier: 'catalog:' - version: 0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) + version: 0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) '@wso2/oxygen-ui': specifier: 'catalog:' version: 0.13.0(@mui/system@7.3.11(@emotion/react@11.14.0(@types/react@19.2.14)(react@19.2.3))(@emotion/styled@11.14.1(@emotion/react@11.14.0(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(react@19.2.3))(@types/react@19.2.14)(@wso2/oxygen-ui-icons-react@0.13.0(react@19.2.3))(dayjs@1.11.21)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) @@ -6684,15 +6684,22 @@ packages: '@thunderid/browser@0.11.3': resolution: {integrity: sha512-2k6iiAeu7ODSL/G/0xf46tdFQMyn+qg+aHW/HNDE8xIYXudWvXqOk/t8+JkSdQiQagFPomGc67LobF9ojHBqGQ==} + '@thunderid/browser@0.11.4': + resolution: {integrity: sha512-CKD0bVf6LTARGEs+IErNhX1FM2KPJkbML2zgEVJ2hVCwwVgK8GLSzrm8uA6nA0xOFZeHmemDUrlnmoKTchtaFg==} + '@thunderid/javascript@0.0.5': resolution: {integrity: sha512-lMjbHCsNgYWclUiPlN3CA6vjKSyjIBIxVGSnuJwiC9eVakVsWmRuxkO8KX6AWlA4SRHgpGLMnD4ZE5lpM+gi9g==} '@thunderid/javascript@0.11.2': resolution: {integrity: sha512-26k2Aq3B077BNPoE50JvjiFJVepV4dKFD6s80EwVf9BVtAm2GwVCkRDh9WtOgSxMLd7IGPR9yRPenDhRpN3YAQ==} - '@thunderid/react-router@0.10.2': - resolution: {integrity: sha512-0S7mv7dXGmSWEDJLnddaLjhCoR9Benvt4VNz+5gogER1HOXUwaPjEL+KWAAHwAlE5xt0+Hht+4DI1ANsNBu/ww==} + '@thunderid/javascript@0.11.3': + resolution: {integrity: sha512-/Qj7x6ySNDCCWonfwWU9YIhCU5VR8NeYm85MFxrn4A0kdJrN8Iswqs5wtI5j4oygKis4xjjDY9Xaw43x4jiDHQ==} + + '@thunderid/react-router@0.10.4': + resolution: {integrity: sha512-Onus9iMQW2nWy31bbX5XLbxJd69a3yxYT1vM5NScwABaae6En2iIQFYKgsXJNys/tUgVkm4pPiSqgKCNdu0z2Q==} peerDependencies: + '@thunderid/browser': '>=0.1.0' '@thunderid/react': '>=0.1.0' react: '>=16.8.0' react-router: '>=6.30.1' @@ -6704,8 +6711,8 @@ packages: react: '>=16.8.0' react-dom: '>=16.8.0' - '@thunderid/react@0.11.3': - resolution: {integrity: sha512-AATKSGoi/KRyieyvfplmbwH9XJ8FJ/VEInQSPKvpwzPo0QpBQpPaVO3b9M1bsLO45jZExiLzJ6KCEN5Ej31o/A==} + '@thunderid/react@0.11.4': + resolution: {integrity: sha512-V5Tg5aKQUcLP9+c8Son2VqWo1OFfZQXiqIsd8JoR3xkBY9CoBc84z8PaX99CmCdAzI8T3lVc5UzzE2JCBoM9Ag==} peerDependencies: '@types/react': '>=16.8.0' react: '>=16.8.0' @@ -18830,6 +18837,19 @@ snapshots: stream-browserify: 3.0.0 tslib: 2.8.1 + '@thunderid/browser@0.11.4': + dependencies: + '@thunderid/javascript': 0.11.3 + base64url: 3.0.1 + buffer: 6.0.3 + core-js: 3.42.0 + fast-sha256: 1.3.0 + jose: 6.2.3 + process: 0.11.10 + randombytes: 2.1.0 + stream-browserify: 3.0.0 + tslib: 2.8.1 + '@thunderid/javascript@0.0.5': dependencies: jose: 5.2.0 @@ -18840,9 +18860,15 @@ snapshots: jose: 6.2.3 tslib: 2.8.1 - '@thunderid/react-router@0.10.2(@thunderid/react@0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react-router@8.3.0(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3)': + '@thunderid/javascript@0.11.3': dependencies: - '@thunderid/react': 0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) + jose: 6.2.3 + tslib: 2.8.1 + + '@thunderid/react-router@0.10.4(@thunderid/browser@0.11.4)(@thunderid/react@0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react-router@8.3.0(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3)': + dependencies: + '@thunderid/browser': 0.11.4 + '@thunderid/react': 0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) react: 19.2.3 react-router: 8.3.0(react-dom@19.2.3(react@19.2.3))(react@19.2.3) tslib: 2.8.1 @@ -18860,11 +18886,11 @@ snapshots: transitivePeerDependencies: - supports-color - '@thunderid/react@0.11.3(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3)': + '@thunderid/react@0.11.4(@types/react@19.2.14)(react-dom@19.2.3(react@19.2.3))(react@19.2.3)': dependencies: '@emotion/css': 11.13.5 '@floating-ui/react': 0.27.12(react-dom@19.2.3(react@19.2.3))(react@19.2.3) - '@thunderid/browser': 0.11.3 + '@thunderid/browser': 0.11.4 '@types/react': 19.2.14 dompurify: 3.4.12 react: 19.2.3 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index c8d681ab99..49e888e528 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -36,8 +36,8 @@ catalog: '@testing-library/jest-dom': 6.9.1 '@testing-library/react': 16.3.0 '@testing-library/user-event': 14.6.1 - '@thunderid/react': 0.11.3 - '@thunderid/react-router': 0.10.2 + '@thunderid/react': 0.11.4 + '@thunderid/react-router': 0.10.4 '@types/lodash-es': 4.17.12 '@types/node': 24.7.2 '@types/react': 19.2.14 diff --git a/tests/integration/oauth/authz/authz_test.go b/tests/integration/oauth/authz/authz_test.go index f9fc386dd8..5525a81852 100644 --- a/tests/integration/oauth/authz/authz_test.go +++ b/tests/integration/oauth/authz/authz_test.go @@ -1602,6 +1602,141 @@ func (ts *AuthzTestSuite) TestAssertionBoundToAuthorizationRequest() { ts.NotEmpty(code, "Authorization code must be issued for the matching authId/assertion pair") } +// TestFlowFailurePropagatesToClientRedirect verifies the end-to-end failure path: a flow that +// terminates in ERROR mints a signed error assertion, and relaying that assertion to the callback +// produces an RFC 6749 error redirect to the client instead of leaving the request to expire. +// +// The trigger is an invalid challenge token. The ChallengeTokenInterceptor runs PRE_REQUEST on every +// flow, so this needs no special graph — and unlike a wrong password (which the credentials executor +// turns into a re-prompt, never a terminal failure) it reliably reaches FlowStatusError. +func (ts *AuthzTestSuite) TestFlowFailurePropagatesToClientRedirect() { + username := "flow_failure_user" + password := "testpass123" + + user := testutils.User{ + OUID: testOUID, + Type: "authz-test-person", + Attributes: json.RawMessage(`{ + "username": "flow_failure_user", + "password": "testpass123", + "email": "flow_failure_user@example.com", + "given_name": "Flow", + "family_name": "Failure" + }`), + } + userID, err := testutils.CreateUser(user) + ts.Require().NoError(err, "Failed to create test user") + defer func() { + if err := testutils.DeleteUser(userID); err != nil { + ts.T().Logf("Warning: Failed to delete test user: %v", err) + } + }() + + authID, errorAssertion := ts.runFlowToErrorAssertion(username, password, "failure_state_a") + + authzResponse, err := testutils.CompleteAuthorization(authID, errorAssertion) + ts.Require().NoError(err, "Callback should return a redirect (200), not an HTTP error") + + parsed, err := url.Parse(authzResponse.RedirectURI) + ts.Require().NoError(err, "Failed to parse client redirect URI") + ts.Equal("access_denied", parsed.Query().Get("error"), + "An end-user flow failure must reach the client as access_denied") + ts.Equal("failure_state_a", parsed.Query().Get("state"), + "The client's state must be echoed back on the error redirect") + ts.NotEmpty(parsed.Query().Get("iss"), "The error redirect must carry the issuer") + ts.Empty(parsed.Query().Get("code"), "No authorization code may be issued for a failed flow") + // The description comes from the flow's own error, not the fixed fallback message. + ts.Equal("The challenge token is missing or invalid", parsed.Query().Get("error_description"), + "The flow error description should be surfaced to the client") + + // Positive control: the failure path must not have broken the success path on the same fixtures. + successAuthID, assertion := ts.runFlowToAssertion(username, password, "failure_state_ok") + successResp, err := testutils.CompleteAuthorization(successAuthID, assertion) + ts.Require().NoError(err, "Legitimate callback should succeed") + code, err := testutils.ExtractAuthorizationCode(successResp.RedirectURI) + ts.Require().NoError(err, "Legitimate callback should issue an authorization code") + ts.NotEmpty(code, "Authorization code must still be issued after a failed flow") +} + +// TestErrorAssertionBoundToAuthorizationRequest verifies that an error assertion is non-transferable +// and, crucially, that rejecting one does not consume the authorization request it was aimed at. +// Verification runs before the request context is loaded precisely so a caller holding only a live +// authId cannot destroy a pending authorization. +func (ts *AuthzTestSuite) TestErrorAssertionBoundToAuthorizationRequest() { + username := "error_binding_user" + password := "testpass123" + + user := testutils.User{ + OUID: testOUID, + Type: "authz-test-person", + Attributes: json.RawMessage(`{ + "username": "error_binding_user", + "password": "testpass123", + "email": "error_binding_user@example.com", + "given_name": "Error", + "family_name": "Binding" + }`), + } + userID, err := testutils.CreateUser(user) + ts.Require().NoError(err, "Failed to create test user") + defer func() { + if err := testutils.DeleteUser(userID); err != nil { + ts.T().Logf("Warning: Failed to delete test user: %v", err) + } + }() + + // Flow A fails and yields an error assertion bound to authIDA. Flow B is a live, untouched request. + _, errorAssertionA := ts.runFlowToErrorAssertion(username, password, "error_binding_a") + authIDB, assertionB := ts.runFlowToAssertion(username, password, "error_binding_b") + + // Aim A's error assertion at B's authId: the binding claim names authIDA, so it must be rejected. + rejected, err := testutils.CompleteAuthorization(authIDB, errorAssertionA) + ts.Require().NoError(err, "Callback should return a redirect (200), not an HTTP error") + parsed, err := url.Parse(rejected.RedirectURI) + ts.Require().NoError(err, "Failed to parse redirect URI") + ts.Empty(parsed.Query().Get("code"), "A mismatched error assertion must not issue a code") + + // authIDB must have survived the rejection, so its own assertion still completes normally. + successResp, err := testutils.CompleteAuthorization(authIDB, assertionB) + ts.Require().NoError(err, "The untouched authorization request should still complete") + code, err := testutils.ExtractAuthorizationCode(successResp.RedirectURI) + ts.Require().NoError(err, + "A rejected error assertion must not consume the authorization request it targeted") + ts.NotEmpty(code, "Authorization code must be issued after the rejected error assertion") +} + +// runFlowToErrorAssertion drives an OAuth2-initiated flow to a terminal failure and returns the authId +// along with the signed error assertion minted for it. +func (ts *AuthzTestSuite) runFlowToErrorAssertion(username, password, state string) (string, string) { + resp, err := testutils.InitiateAuthorizationFlow(clientID, redirectURI, "code", "openid", state) + ts.Require().NoError(err, "Failed to initiate authorization flow") + defer resp.Body.Close() + + ts.Require().Equal(http.StatusFound, resp.StatusCode, "Expected redirect status") + + authID, executionID, err := testutils.ExtractAuthData(resp.Header.Get("Location")) + ts.Require().NoError(err, "Failed to extract auth data from redirect") + + _, err = testutils.ExecuteAuthenticationFlow(executionID, nil, "") + ts.Require().NoError(err, "Failed to initiate authentication flow") + + // Submitting a bad challenge token fails the PRE_REQUEST interceptor, which has no onFailure + // target, so the flow terminates in ERROR rather than re-prompting. + flowStep, err := testutils.ExecuteAuthenticationFlow(executionID, map[string]string{ + "username": username, + "password": password, + }, "action_001", "wrong-challenge-token") + ts.Require().NoError(err, "Flow step should be returned for an invalid challenge token") + ts.Require().Equal("ERROR", flowStep.FlowStatus, "Flow should terminate in ERROR") + ts.Require().NotNil(flowStep.Error, "Terminal failure should carry an error") + ts.Require().Equal("ICS-1002", flowStep.Error.Code, "Expected the invalid challenge token error") + ts.Require().NotEmpty(flowStep.ErrorAssertion, + "An OAuth-initiated flow failure must mint an error assertion") + ts.Require().Empty(flowStep.Assertion, "A failed flow must not produce an authentication assertion") + + return authID, flowStep.ErrorAssertion +} + // runFlowToAssertion initiates an OAuth2 authorize flow, drives the authentication flow to // completion, and returns the authId issued at initiation along with the resulting assertion. func (ts *AuthzTestSuite) runFlowToAssertion(username, password, state string) (string, string) { diff --git a/tests/integration/oauth/authz/flow_server_error_test.go b/tests/integration/oauth/authz/flow_server_error_test.go new file mode 100644 index 0000000000..3184814784 --- /dev/null +++ b/tests/integration/oauth/authz/flow_server_error_test.go @@ -0,0 +1,225 @@ +// Copyright 2026 The ThunderID Authors +// SPDX-License-Identifier: Apache-2.0 + +package authz + +import ( + "bytes" + "encoding/json" + "io" + "net/http" + "net/url" + "testing" + + "github.com/stretchr/testify/suite" + "github.com/thunder-id/thunderid/tests/integration/testutils" +) + +const ( + serverErrClientID = "authz_server_err_client_123" + serverErrClientSecret = "authz_server_err_secret_123" + serverErrRedirect = "https://localhost:3000" + // A port with nothing listening, so the notification send fails with a connection error. This + // stands in for any infrastructure dependency being unreachable mid-flow. + serverErrDeadSenderURL = "http://127.0.0.1:65533/send" +) + +// ServerErrorTestSuite covers the engine-failure channel: an infrastructure dependency that goes down +// mid-authentication surfaces as a server-side error rather than an in-band flowStatus=ERROR, so the +// error assertion travels in the 4xx/5xx body instead of the flow response. It must still reach the +// waiting OAuth request, as server_error rather than access_denied. +// +// The dependency broken here is the notification gateway, because SMSExecutor returns a bare Go error +// on any send failure in an AUTHENTICATION flow (sms_executor.go), which the task node converts to +// InternalServerError. That is the same path a userdb outage takes out of CredentialsAuthExecutor. +type ServerErrorTestSuite struct { + suite.Suite + ouID string + senderID string + flowID string + appID string + client *http.Client +} + +func TestServerErrorTestSuite(t *testing.T) { + suite.Run(t, new(ServerErrorTestSuite)) +} + +func (ts *ServerErrorTestSuite) SetupSuite() { + ts.client = testutils.GetHTTPClient() + + ouID, err := testutils.CreateOrganizationUnit(testutils.OrganizationUnit{ + Handle: "authz-server-err-ou", + Name: "Authz Server Error OU", + Description: "Organization unit for the engine-failure error assertion test", + Parent: nil, + }) + ts.Require().NoError(err, "Failed to create test organization unit") + ts.ouID = ouID + + senderID, err := testutils.CreateNotificationSender(testutils.NotificationSender{ + Name: "Unreachable Test Sender", + Description: "Sender pointed at a dead port to force a server-side send failure", + Provider: "custom", + Properties: []testutils.SenderProperty{ + {Name: "url", Value: serverErrDeadSenderURL}, + {Name: "http_method", Value: "POST"}, + {Name: "content_type", Value: "JSON"}, + }, + }) + ts.Require().NoError(err, "Failed to create notification sender") + ts.senderID = senderID + + flowID, err := testutils.CreateFlow(testutils.Flow{ + Name: "Authz Server Error Flow", + FlowType: "AUTHENTICATION", + Handle: "auth_flow_authz_server_error", + Nodes: []map[string]interface{}{ + {"id": "start", "type": "START", "onSuccess": "prompt_mobile"}, + { + "id": "prompt_mobile", + "type": "PROMPT", + "prompts": []map[string]interface{}{ + { + "inputs": []map[string]interface{}{ + { + "ref": "input_001", + "identifier": "mobile_number", + "type": "TEXT_INPUT", + "required": true, + }, + }, + "action": map[string]interface{}{ + "ref": "action_001", + "nextNode": "send_sms", + }, + }, + }, + }, + { + "id": "send_sms", + "type": "TASK_EXECUTION", + "properties": map[string]interface{}{ + "senderId": senderID, + "smsTemplate": "CIBA_NOTIFICATION", + }, + "executor": map[string]interface{}{"name": "SMSExecutor"}, + "onSuccess": "auth_assert", + }, + { + "id": "auth_assert", + "type": "TASK_EXECUTION", + "executor": map[string]interface{}{"name": "AuthAssertExecutor"}, + "onSuccess": "end", + }, + {"id": "end", "type": "END"}, + }, + }) + ts.Require().NoError(err, "Failed to create server-error flow") + ts.flowID = flowID + + ts.appID = ts.createServerErrorApplication(flowID) +} + +func (ts *ServerErrorTestSuite) TearDownSuite() { + if ts.appID != "" { + _ = testutils.DeleteApplication(ts.appID) + } + if ts.flowID != "" { + _ = testutils.DeleteFlow(ts.flowID) + } + if ts.senderID != "" { + _ = testutils.DeleteNotificationSender(ts.senderID) + } + if ts.ouID != "" { + if err := testutils.DeleteOrganizationUnit(ts.ouID); err != nil { + ts.T().Logf("Failed to delete test organization unit: %v", err) + } + } +} + +// TestServerErrorPropagatesToClientRedirect verifies that a server-side failure during authentication +// still reaches the client, as server_error. The assertion arrives in the 4xx/5xx error body rather +// than a flow response, which is a separate serialization path from the in-band ERROR channel. +func (ts *ServerErrorTestSuite) TestServerErrorPropagatesToClientRedirect() { + resp, err := testutils.InitiateAuthorizationFlow(serverErrClientID, serverErrRedirect, + "code", "openid", "server_err_state") + ts.Require().NoError(err, "Failed to initiate authorization flow") + defer resp.Body.Close() + + ts.Require().Equal(http.StatusFound, resp.StatusCode, "Expected redirect status") + + authID, executionID, err := testutils.ExtractAuthData(resp.Header.Get("Location")) + ts.Require().NoError(err, "Failed to extract auth data from redirect") + + initialStep, err := testutils.ExecuteAuthenticationFlow(executionID, nil, "") + ts.Require().NoError(err, "Failed to initiate authentication flow") + + // Submitting the recipient advances into the SMS node, whose gateway is unreachable. + status, errBody, err := testutils.ExecuteAuthenticationFlowExpectingError(executionID, + map[string]string{"mobile_number": "+1987654321"}, "action_001", initialStep.ChallengeToken) + ts.Require().NoError(err, "Failed to execute the flow") + ts.Require().GreaterOrEqual(status, http.StatusInternalServerError, + "An infrastructure failure should be reported as a 5xx, not an in-band flow response") + ts.Require().NotNil(errBody) + ts.Require().NotEmpty(errBody.ErrorAssertion, + "The server-error body must carry the signed error assertion") + + authzResponse, err := testutils.CompleteAuthorization(authID, errBody.ErrorAssertion) + ts.Require().NoError(err, "Callback should return a redirect (200), not an HTTP error") + + parsed, err := url.Parse(authzResponse.RedirectURI) + ts.Require().NoError(err, "Failed to parse client redirect URI") + ts.Equal("server_error", parsed.Query().Get("error"), + "A server-side failure must reach the client as server_error, not access_denied") + ts.Equal("server_err_state", parsed.Query().Get("state"), + "The client's state must be echoed back on the error redirect") + ts.NotEmpty(parsed.Query().Get("iss"), "The error redirect must carry the issuer") + ts.Empty(parsed.Query().Get("code"), "No authorization code may be issued for a failed flow") +} + +// createServerErrorApplication creates an OAuth application bound to the given authentication flow. +func (ts *ServerErrorTestSuite) createServerErrorApplication(authFlowID string) string { + app := map[string]interface{}{ + "name": "AuthzServerErrorApp", + "description": "Application for the server-error assertion test", + "ouId": ts.ouID, + "type": "browser", + "authFlowId": authFlowID, + "isRegistrationFlowEnabled": false, + "inboundAuthConfig": []map[string]interface{}{ + { + "type": "oauth2", + "config": map[string]interface{}{ + "clientId": serverErrClientID, + "clientSecret": serverErrClientSecret, + "redirectUris": []string{serverErrRedirect}, + "grantTypes": []string{"authorization_code"}, + "responseTypes": []string{"code"}, + "tokenEndpointAuthMethod": "client_secret_basic", + }, + }, + }, + } + + jsonData, err := json.Marshal(app) + ts.Require().NoError(err) + + req, err := http.NewRequest("POST", testutils.TestServerURL+"/applications", bytes.NewBuffer(jsonData)) + ts.Require().NoError(err) + req.Header.Set("Content-Type", "application/json") + + resp, err := ts.client.Do(req) + ts.Require().NoError(err) + defer resp.Body.Close() + + if resp.StatusCode != http.StatusCreated { + bodyBytes, _ := io.ReadAll(resp.Body) + ts.T().Fatalf("Failed to create application. Status: %d, Response: %s", + resp.StatusCode, string(bodyBytes)) + } + + var respData map[string]interface{} + ts.Require().NoError(json.NewDecoder(resp.Body).Decode(&respData)) + return respData["id"].(string) +} diff --git a/tests/integration/oauth/ciba/ciba_test.go b/tests/integration/oauth/ciba/ciba_test.go index 6130b07b6e..a15e2f7f26 100644 --- a/tests/integration/oauth/ciba/ciba_test.go +++ b/tests/integration/oauth/ciba/ciba_test.go @@ -43,8 +43,14 @@ const ( type CIBATestSuite struct { suite.Suite - ouID string - client *http.Client + ouID string + client *http.Client + mockServer *testutils.MockNotificationServer + senderID string + userTypeID string + userID string + flowID string + appID string } func TestCIBATestSuite(t *testing.T) { @@ -62,27 +68,11 @@ func (ts *CIBATestSuite) SetupSuite() { }) ts.Require().NoError(err, "Failed to create test organization unit") ts.ouID = ouID -} - -func (ts *CIBATestSuite) TearDownSuite() { - if ts.ouID != "" { - if err := testutils.DeleteOrganizationUnit(ts.ouID); err != nil { - ts.T().Logf("Failed to delete test organization unit: %v", err) - } - } -} -// TestCIBAGrantFlow exercises the full Client-Initiated Backchannel Authentication (CIBA) grant -// end to end: it initiates a backchannel request, recovers the server-initiated flow's executionId -// from an out-of-band notification, completes the authentication flow, drives the state machine -// (PENDING -> AUTHENTICATED -> CONSUMED) through the callback and token endpoints, and asserts the -// one-time-use enforcement backed by the runtime store's CompareFieldAndSwap primitive. -func (ts *CIBATestSuite) TestCIBAGrantFlow() { // Mock notification server captures the CIBA notification (which carries the invite link with // the executionId). It is a plain HTTP server; the sender below is pointed at it via the API. - mockServer := testutils.NewMockNotificationServer(cibaMockNotificationServerPort) - ts.Require().NoError(mockServer.Start(), "Failed to start mock notification server") - defer func() { _ = mockServer.Stop() }() + ts.mockServer = testutils.NewMockNotificationServer(cibaMockNotificationServerPort) + ts.Require().NoError(ts.mockServer.Start(), "Failed to start mock notification server") time.Sleep(100 * time.Millisecond) // A custom notification sender that POSTs rendered messages to the mock server. This is a DB @@ -92,13 +82,13 @@ func (ts *CIBATestSuite) TestCIBAGrantFlow() { Description: "Sender for CIBA integration test", Provider: "custom", Properties: []testutils.SenderProperty{ - {Name: "url", Value: mockServer.GetSendSMSURL()}, + {Name: "url", Value: ts.mockServer.GetSendSMSURL()}, {Name: "http_method", Value: "POST"}, {Name: "content_type", Value: "JSON"}, }, }) ts.Require().NoError(err, "Failed to create notification sender") - defer func() { _ = testutils.DeleteNotificationSender(senderID) }() + ts.senderID = senderID // User type + user. mobile_number is the recipient the SMS executor resolves from the // identified user; username/password back the credential confirmation step. @@ -113,7 +103,7 @@ func (ts *CIBATestSuite) TestCIBAGrantFlow() { }, }) ts.Require().NoError(err, "Failed to create CIBA test user type") - defer func() { _ = testutils.DeleteUserType(userTypeID) }() + ts.userTypeID = userTypeID userID, err := testutils.CreateUser(testutils.User{ OUID: ts.ouID, @@ -126,7 +116,7 @@ func (ts *CIBATestSuite) TestCIBAGrantFlow() { }`), }) ts.Require().NoError(err, "Failed to create CIBA test user") - defer func() { _ = testutils.DeleteUser(userID) }() + ts.userID = userID // CIBA authentication flow. bc-authorize runs this server-side with login_hint. The // IdentifyingExecutor resolves the user (login_hint -> username), the InviteExecutor mints a @@ -265,10 +255,52 @@ func (ts *CIBATestSuite) TestCIBAGrantFlow() { }, }) ts.Require().NoError(err, "Failed to create CIBA auth flow") - defer func() { _ = testutils.DeleteFlow(flowID) }() + ts.flowID = flowID + + ts.appID = ts.createCIBATestApplication(flowID) +} + +// SetupTest drops notifications captured by earlier tests, so that each test recovers the executionId +// of its own backchannel request from the shared mock server. +func (ts *CIBATestSuite) SetupTest() { + if ts.mockServer != nil { + ts.mockServer.ClearMessages() + } +} + +func (ts *CIBATestSuite) TearDownSuite() { + if ts.appID != "" { + _ = testutils.DeleteApplication(ts.appID) + } + if ts.flowID != "" { + _ = testutils.DeleteFlow(ts.flowID) + } + if ts.userID != "" { + _ = testutils.DeleteUser(ts.userID) + } + if ts.userTypeID != "" { + _ = testutils.DeleteUserType(ts.userTypeID) + } + if ts.senderID != "" { + _ = testutils.DeleteNotificationSender(ts.senderID) + } + if ts.mockServer != nil { + _ = ts.mockServer.Stop() + } + if ts.ouID != "" { + if err := testutils.DeleteOrganizationUnit(ts.ouID); err != nil { + ts.T().Logf("Failed to delete test organization unit: %v", err) + } + } +} - appID := ts.createCIBATestApplication(flowID) - defer func() { _ = testutils.DeleteApplication(appID) }() +// TestCIBAGrantFlow exercises the full Client-Initiated Backchannel Authentication (CIBA) grant +// end to end: it initiates a backchannel request, recovers the server-initiated flow's executionId +// from an out-of-band notification, completes the authentication flow, drives the state machine +// (PENDING -> AUTHENTICATED -> CONSUMED) through the callback and token endpoints, and asserts the +// one-time-use enforcement backed by the runtime store's CompareFieldAndSwap primitive. +func (ts *CIBATestSuite) TestCIBAGrantFlow() { + mockServer := ts.mockServer // Step 1: Backchannel authorization request. status, bcResp := ts.cibaBackchannelAuthorize(cibaTestUsername, "openid") @@ -331,7 +363,7 @@ func (ts *CIBATestSuite) TestCIBAGrantFlow() { claims, err := testutils.DecodeJWT(tokenRes.accessToken) ts.Require().NoError(err, "issued access token should be a decodable JWT") - ts.Require().Equal(userID, claims.Sub, "token subject should be the CIBA user") + ts.Require().Equal(ts.userID, claims.Sub, "token subject should be the CIBA user") // Step 7: A second poll is rejected — the request is CONSUMED (one-time use). reuse := ts.cibaPollToken(bcResp.AuthReqID) @@ -339,6 +371,64 @@ func (ts *CIBATestSuite) TestCIBAGrantFlow() { ts.Require().Equal("invalid_grant", reuse.errorCode, "a consumed request must not issue tokens again") } +// TestCIBAFlowFailureDeniesRequest verifies that a terminal flow failure is propagated to the polling +// client instead of leaving the request PENDING until it expires. The flow mints a signed error +// assertion, the gate relays it to the callback in the same field a success assertion uses, and the +// request transitions to DENIED so the next token poll returns access_denied. +// +// The trigger is a bogus inviteToken: InviteExecutor's verify mode fails, and verify_invite has no +// onFailure target, so the flow terminates in ERROR. +func (ts *CIBATestSuite) TestCIBAFlowFailureDeniesRequest() { + // Step 1: Backchannel authorization request. + status, bcResp := ts.cibaBackchannelAuthorize(cibaTestUsername, "openid") + ts.Require().Equal(http.StatusOK, status, "bc-authorize should succeed") + ts.Require().NotEmpty(bcResp.AuthReqID, "bc-authorize response should carry auth_req_id") + + // Step 2: Baseline — before the failure the client is told nothing but "keep waiting". + pending := ts.cibaPollToken(bcResp.AuthReqID) + ts.Require().Equal(http.StatusBadRequest, pending.statusCode) + ts.Require().Equal("authorization_pending", pending.errorCode) + + // Step 3: Recover the executionId from the notification captured for this request. + var executionID string + ts.Require().Eventually(func() bool { + msg := ts.mockServer.GetLastMessage() + if msg == nil { + return false + } + if extractCIBALinkParam(msg.Message, "auth_req_id") != bcResp.AuthReqID { + return false + } + executionID = extractCIBALinkParam(msg.Message, "executionId") + return executionID != "" + }, 5*time.Second, 100*time.Millisecond, "Expected CIBA notification carrying the executionId") + + // Step 4: Fail the flow at the invite-verify node. + flowStep, err := testutils.ExecuteAuthenticationFlow(executionID, + map[string]string{"inviteToken": "bogus-invite-token"}, "") + ts.Require().NoError(err, "Flow step should be returned for an invalid invite token") + ts.Require().Equal("ERROR", flowStep.FlowStatus, "Flow should terminate in ERROR") + ts.Require().NotEmpty(flowStep.ErrorAssertion, + "A CIBA-initiated flow failure must mint an error assertion") + ts.Require().Empty(flowStep.Assertion, "A failed flow must not produce an authentication assertion") + + // Step 5: Relay the error assertion. The callback op itself succeeds; the outcome lives in the + // request state, which is why this is a 200 and not an error status. + ts.Require().Equal(http.StatusOK, ts.cibaPostCallback(bcResp.AuthReqID, flowStep.ErrorAssertion), + "CIBA callback should accept the error assertion") + + // Step 6: The polling client now learns the outcome instead of hanging on authorization_pending. + denied := ts.cibaPollToken(bcResp.AuthReqID) + if denied.statusCode == http.StatusBadRequest && denied.errorCode == "slow_down" { + time.Sleep(cibaPollIntervalSeconds * time.Second) + denied = ts.cibaPollToken(bcResp.AuthReqID) + } + ts.Require().Equal(http.StatusBadRequest, denied.statusCode) + ts.Require().Equal("access_denied", denied.errorCode, + "An end-user flow failure must surface as access_denied, not authorization_pending") + ts.Require().Empty(denied.accessToken, "A denied request must not issue tokens") +} + // createCIBATestApplication creates an OAuth application that allows the CIBA grant and is bound to // the given authentication flow, returning its application ID. func (ts *CIBATestSuite) createCIBATestApplication(authFlowID string) string { diff --git a/tests/integration/resources/deployment.yaml b/tests/integration/resources/deployment.yaml index 49f75c53f0..f24a69a4fa 100644 --- a/tests/integration/resources/deployment.yaml +++ b/tests/integration/resources/deployment.yaml @@ -43,6 +43,7 @@ passkey: oauth: allow_wildcard_redirect_uri: true + send_server_errors_to_client: true auth_class: amrs: - PWD diff --git a/tests/integration/resources/scripts/setup-test-config.ps1 b/tests/integration/resources/scripts/setup-test-config.ps1 index 4661b19a55..096bdc4021 100644 --- a/tests/integration/resources/scripts/setup-test-config.ps1 +++ b/tests/integration/resources/scripts/setup-test-config.ps1 @@ -133,6 +133,7 @@ server_config: oauth: allow_wildcard_redirect_uri: true + send_server_errors_to_client: true auth_class: amrs: - PWD diff --git a/tests/integration/resources/scripts/setup-test-config.sh b/tests/integration/resources/scripts/setup-test-config.sh index 8eff10e7dd..3c40cf9d08 100644 --- a/tests/integration/resources/scripts/setup-test-config.sh +++ b/tests/integration/resources/scripts/setup-test-config.sh @@ -123,6 +123,7 @@ server_config: oauth: allow_wildcard_redirect_uri: true + send_server_errors_to_client: true auth_class: amrs: - PWD diff --git a/tests/integration/testutils/models.go b/tests/integration/testutils/models.go index 29fcc9864a..0a339b220d 100644 --- a/tests/integration/testutils/models.go +++ b/tests/integration/testutils/models.go @@ -262,15 +262,28 @@ type FlowAction struct { // FlowStep represents a single step in a flow execution type FlowStep struct { - ExecutionID string `json:"executionId"` - FlowStatus string `json:"flowStatus"` - Type string `json:"type"` - Data *FlowData `json:"data,omitempty"` - Assertion string `json:"assertion,omitempty"` + ExecutionID string `json:"executionId"` + FlowStatus string `json:"flowStatus"` + Type string `json:"type"` + Data *FlowData `json:"data,omitempty"` + Assertion string `json:"assertion,omitempty"` + // ErrorAssertion is the signed error assertion minted when an OAuth-initiated flow terminates in + // failure. It is relayed to /oauth2/auth/callback in the same field as a success assertion. + ErrorAssertion string `json:"errorAssertion,omitempty"` Error *FlowExecutionError `json:"error,omitempty"` ChallengeToken string `json:"challengeToken,omitempty"` } +// FlowErrorResponse is the body returned when flow execution fails at the engine level (4xx/5xx), +// which has no flow response to carry the assertion. Message and Description are i18n objects, so +// they are left raw; tests assert on the code and the assertion. +type FlowErrorResponse struct { + Code string `json:"code"` + Message json.RawMessage `json:"message"` + Description json.RawMessage `json:"description"` + ErrorAssertion string `json:"errorAssertion,omitempty"` +} + // Flow represents a flow definition type Flow struct { Name string `json:"name"` diff --git a/tests/integration/testutils/oauth2_utils.go b/tests/integration/testutils/oauth2_utils.go index 6d444864da..cc4ab0cef3 100644 --- a/tests/integration/testutils/oauth2_utils.go +++ b/tests/integration/testutils/oauth2_utils.go @@ -191,6 +191,58 @@ func ExecuteAuthenticationFlow(executionId string, inputs map[string]string, act return &flowStep, nil } +// ExecuteAuthenticationFlowExpectingError executes a flow step that is expected to fail at the engine +// level, and returns the HTTP status along with the parsed error body. ExecuteAuthenticationFlow +// collapses any non-200 into a Go error, which hides the errorAssertion carried in that body. +func ExecuteAuthenticationFlowExpectingError(executionId string, inputs map[string]string, + action string, challengeToken ...string) (int, *FlowErrorResponse, error) { + flowData := map[string]interface{}{ + "executionId": executionId, + } + + if len(inputs) > 0 { + flowData["inputs"] = inputs + } + if action != "" { + flowData["action"] = action + } + if len(challengeToken) > 0 && challengeToken[0] != "" { + flowData["challengeToken"] = challengeToken[0] + } + + flowJSON, err := json.Marshal(flowData) + if err != nil { + return 0, nil, fmt.Errorf("failed to marshal flow data: %w", err) + } + + req, err := http.NewRequest("POST", TestServerURL+"/flow/execute", bytes.NewBuffer(flowJSON)) + if err != nil { + return 0, nil, fmt.Errorf("failed to create flow request: %w", err) + } + req.Header.Set("Content-Type", "application/json") + + client := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + }, + } + + resp, err := client.Do(req) + if err != nil { + return 0, nil, fmt.Errorf("failed to execute flow: %w", err) + } + defer resp.Body.Close() + + bodyBytes, _ := io.ReadAll(resp.Body) + var errorResponse FlowErrorResponse + if err := json.Unmarshal(bodyBytes, &errorResponse); err != nil { + return resp.StatusCode, nil, fmt.Errorf("failed to decode flow error response %q: %w", + string(bodyBytes), err) + } + + return resp.StatusCode, &errorResponse, nil +} + // CompleteAuthorization completes the authorization using the assertion func CompleteAuthorization(authID, assertion string) (*AuthorizationResponse, error) { authzData := map[string]interface{}{