From 318789ec38ca6194df63af7e0868f49f2d87b2db Mon Sep 17 00:00:00 2001 From: Bryan Frimin Date: Fri, 24 Jul 2026 21:32:56 +0000 Subject: [PATCH] Cover magic-link and SAML failures on auth error page Browser auth callbacks still returned JSON or used one-off pages for several refusal reasons. Route OIDC email verification, magic-link, and SAML ACS failures through /auth/error with stable error codes so users always see an explanation. Signed-off-by: Cursor Agent Co-authored-by: Bryan FRIMIN --- .../src/pages/iam/auth/AuthErrorPage.tsx | 70 +++++++++++++++ .../iam/auth/MagicLinkAlreadyUsedPage.tsx | 45 ---------- .../pages/iam/auth/MagicLinkExpiredPage.tsx | 45 ---------- apps/console/src/routes.tsx | 8 -- pkg/server/api/connect/v1/auth_error.go | 49 +++++++++++ pkg/server/api/connect/v1/auth_error_test.go | 87 +++++++++++++++++++ pkg/server/api/connect/v1/oidc_handler.go | 17 ++-- pkg/server/api/connect/v1/saml_handler.go | 45 ++-------- 8 files changed, 223 insertions(+), 143 deletions(-) delete mode 100644 apps/console/src/pages/iam/auth/MagicLinkAlreadyUsedPage.tsx delete mode 100644 apps/console/src/pages/iam/auth/MagicLinkExpiredPage.tsx diff --git a/apps/console/src/pages/iam/auth/AuthErrorPage.tsx b/apps/console/src/pages/iam/auth/AuthErrorPage.tsx index 7636d6751..6a4a93e06 100644 --- a/apps/console/src/pages/iam/auth/AuthErrorPage.tsx +++ b/apps/console/src/pages/iam/auth/AuthErrorPage.tsx @@ -39,6 +39,76 @@ function useAuthErrorContent(code: string | null): AuthErrorContent { "Personal Google and Microsoft accounts cannot be used to sign in. Please use your work or school account instead.", ), }; + case "email_not_verified": + return { + title: __("Email not verified"), + description: __( + "Your email address is not verified with the identity provider. Please verify it, then try signing in again.", + ), + }; + case "magic_link_expired": + return { + title: __("Link Expired"), + description: __( + "This magic link has expired. Magic links are only valid for 15 minutes. Please request a new one.", + ), + }; + case "magic_link_already_used": + return { + title: __("Link Already Used"), + description: __( + "This magic link has already been used. Please request a new one.", + ), + }; + case "magic_link_invalid": + return { + title: __("Invalid link"), + description: __( + "This magic link is invalid. Please request a new one.", + ), + }; + case "saml_disabled": + return { + title: __("SSO unavailable"), + description: __( + "Single sign-on is disabled for this organization. Please contact your administrator.", + ), + }; + case "saml_configuration_not_found": + return { + title: __("SSO configuration not found"), + description: __( + "This single sign-on configuration could not be found. Please contact your administrator.", + ), + }; + case "saml_email_domain_mismatch": + return { + title: __("Email domain not allowed"), + description: __( + "Your email domain is not allowed for this organization's single sign-on. Please use an account from the configured domain.", + ), + }; + case "saml_auto_signup_disabled": + return { + title: __("Account not found"), + description: __( + "No account exists for this email, and automatic signup is disabled. Please contact your administrator.", + ), + }; + case "saml_user_inactive": + return { + title: __("Account inactive"), + description: __( + "Your account is inactive. Please contact your administrator.", + ), + }; + case "saml_subject_already_in_use": + return { + title: __("Account already linked"), + description: __( + "This single sign-on identity is already linked to another account. Please contact your administrator.", + ), + }; default: return { title: __("Authentication failed"), diff --git a/apps/console/src/pages/iam/auth/MagicLinkAlreadyUsedPage.tsx b/apps/console/src/pages/iam/auth/MagicLinkAlreadyUsedPage.tsx deleted file mode 100644 index 10f81b405..000000000 --- a/apps/console/src/pages/iam/auth/MagicLinkAlreadyUsedPage.tsx +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright (c) 2026 Probo Inc . -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -import { usePageTitle } from "@probo/hooks"; -import { useTranslate } from "@probo/i18n"; -import { Button } from "@probo/ui"; - -export default function MagicLinkAlreadyUsedPage() { - const { __ } = useTranslate(); - - usePageTitle(__("Link Already Used")); - - return ( -
-
-

{__("Link Already Used")}

-

- {__( - "This magic link has already been used. Please request a new one.", - )} -

-
- -
- ); -} diff --git a/apps/console/src/pages/iam/auth/MagicLinkExpiredPage.tsx b/apps/console/src/pages/iam/auth/MagicLinkExpiredPage.tsx deleted file mode 100644 index 1920a8d0e..000000000 --- a/apps/console/src/pages/iam/auth/MagicLinkExpiredPage.tsx +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright (c) 2026 Probo Inc . -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -import { usePageTitle } from "@probo/hooks"; -import { useTranslate } from "@probo/i18n"; -import { Button } from "@probo/ui"; - -export default function MagicLinkExpiredPage() { - const { __ } = useTranslate(); - - usePageTitle(__("Link Expired")); - - return ( -
-
-

{__("Link Expired")}

-

- {__( - "This magic link has expired. Magic links are only valid for 15 minutes. Please request a new one.", - )} -

-
- -
- ); -} diff --git a/apps/console/src/routes.tsx b/apps/console/src/routes.tsx index 5c55d583b..54ce82248 100644 --- a/apps/console/src/routes.tsx +++ b/apps/console/src/routes.tsx @@ -119,14 +119,6 @@ const routes = [ () => import("./pages/iam/auth/ConsentPageLoader"), ), }, - { - path: "magic-link-expired", - Component: lazy(() => import("./pages/iam/auth/MagicLinkExpiredPage")), - }, - { - path: "magic-link-already-used", - Component: lazy(() => import("./pages/iam/auth/MagicLinkAlreadyUsedPage")), - }, { path: "error", Component: lazy(() => import("./pages/iam/auth/AuthErrorPage")), diff --git a/pkg/server/api/connect/v1/auth_error.go b/pkg/server/api/connect/v1/auth_error.go index 3e2647589..b744ed0e8 100644 --- a/pkg/server/api/connect/v1/auth_error.go +++ b/pkg/server/api/connect/v1/auth_error.go @@ -21,13 +21,26 @@ package connect_v1 import ( + "errors" "net/http" "net/url" + + "go.probo.inc/probo/pkg/iam/saml" ) const ( authErrorPersonalAccountNotAllowed = "personal_account_not_allowed" + authErrorEmailNotVerified = "email_not_verified" authErrorAuthenticationFailed = "authentication_failed" + authErrorMagicLinkExpired = "magic_link_expired" + authErrorMagicLinkAlreadyUsed = "magic_link_already_used" + authErrorMagicLinkInvalid = "magic_link_invalid" + authErrorSAMLDisabled = "saml_disabled" + authErrorSAMLConfigurationNotFound = "saml_configuration_not_found" + authErrorSAMLEmailDomainMismatch = "saml_email_domain_mismatch" + authErrorSAMLAutoSignupDisabled = "saml_auto_signup_disabled" + authErrorSAMLUserInactive = "saml_user_inactive" + authErrorSAMLSubjectAlreadyInUse = "saml_subject_already_in_use" ) func redirectAuthError(w http.ResponseWriter, r *http.Request, code string) { @@ -41,3 +54,39 @@ func redirectAuthError(w http.ResponseWriter, r *http.Request, code string) { http.Redirect(w, r, redirectURL.String(), http.StatusFound) } + +func authErrorCodeFromSAML(err error) (string, bool) { + if _, ok := errors.AsType[*saml.ErrSAMLDisabled](err); ok { + return authErrorSAMLDisabled, true + } + + if _, ok := errors.AsType[*saml.ErrSAMLConfigurationNotFound](err); ok { + return authErrorSAMLConfigurationNotFound, true + } + + if _, ok := errors.AsType[*saml.ErrEmailDomainMismatch](err); ok { + return authErrorSAMLEmailDomainMismatch, true + } + + if _, ok := errors.AsType[*saml.ErrSAMLAutoSignupDisabled](err); ok { + return authErrorSAMLAutoSignupDisabled, true + } + + if _, ok := errors.AsType[*saml.ErrUserInactive](err); ok { + return authErrorSAMLUserInactive, true + } + + if _, ok := errors.AsType[*saml.ErrSAMLSubjectAlreadyInUse](err); ok { + return authErrorSAMLSubjectAlreadyInUse, true + } + + if _, ok := errors.AsType[*saml.ErrInvalidAssertion](err); ok { + return authErrorAuthenticationFailed, true + } + + if _, ok := errors.AsType[*saml.ErrReplayAttackDetected](err); ok { + return authErrorAuthenticationFailed, true + } + + return "", false +} diff --git a/pkg/server/api/connect/v1/auth_error_test.go b/pkg/server/api/connect/v1/auth_error_test.go index 3329f286b..4e574e5e2 100644 --- a/pkg/server/api/connect/v1/auth_error_test.go +++ b/pkg/server/api/connect/v1/auth_error_test.go @@ -21,12 +21,17 @@ package connect_v1 import ( + "errors" "net/http" "net/http/httptest" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.probo.inc/probo/pkg/coredata" + "go.probo.inc/probo/pkg/gid" + "go.probo.inc/probo/pkg/iam/saml" + "go.probo.inc/probo/pkg/mail" ) func TestRedirectAuthError(t *testing.T) { @@ -43,3 +48,85 @@ func TestRedirectAuthError(t *testing.T) { assert.Equal(t, "/auth/error", location.Path) assert.Equal(t, authErrorPersonalAccountNotAllowed, location.Query().Get("error")) } + +func TestAuthErrorCodeFromSAML(t *testing.T) { + t.Parallel() + + configID := gid.New(gid.TenantID(1), coredata.SAMLConfigurationEntityType) + email, err := mail.ParseAddr("user@example.com") + require.NoError(t, err) + + tests := []struct { + name string + err error + code string + ok bool + }{ + { + name: "disabled", + err: saml.NewSAMLDisabledError(), + code: authErrorSAMLDisabled, + ok: true, + }, + { + name: "configuration not found", + err: saml.NewSAMLConfigurationNotFoundError(configID), + code: authErrorSAMLConfigurationNotFound, + ok: true, + }, + { + name: "email domain mismatch", + err: saml.NewEmailDomainMismatchError(email, "acme.com"), + code: authErrorSAMLEmailDomainMismatch, + ok: true, + }, + { + name: "auto signup disabled", + err: saml.NewSAMLAutoSignupDisabledError(configID), + code: authErrorSAMLAutoSignupDisabled, + ok: true, + }, + { + name: "user inactive", + err: saml.NewUserInactiveError(configID), + code: authErrorSAMLUserInactive, + ok: true, + }, + { + name: "subject already in use", + err: saml.NewSAMLSubjectAlreadyInUseError("assertion-1"), + code: authErrorSAMLSubjectAlreadyInUse, + ok: true, + }, + { + name: "invalid assertion maps to generic failure", + err: saml.NewInvalidAssertionError("assertion-1", errors.New("bad signature")), + code: authErrorAuthenticationFailed, + ok: true, + }, + { + name: "replay maps to generic failure", + err: saml.NewReplayAttackDetectedError("assertion-1"), + code: authErrorAuthenticationFailed, + ok: true, + }, + { + name: "unknown error", + err: errors.New("boom"), + ok: false, + }, + } + + for _, tt := range tests { + t.Run( + tt.name, + func(t *testing.T) { + t.Parallel() + + code, ok := authErrorCodeFromSAML(tt.err) + assert.Equal(t, tt.ok, ok) + assert.Equal(t, tt.code, code) + }, + ) + } +} diff --git a/pkg/server/api/connect/v1/oidc_handler.go b/pkg/server/api/connect/v1/oidc_handler.go index 74eec08a9..3670cc43c 100644 --- a/pkg/server/api/connect/v1/oidc_handler.go +++ b/pkg/server/api/connect/v1/oidc_handler.go @@ -138,6 +138,13 @@ func (h *OIDCHandler) CallbackHandler(w http.ResponseWriter, r *http.Request) { return } + if _, ok := errors.AsType[*oidc.ErrEmailNotVerified](err); ok { + h.logger.WarnCtx(ctx, "OIDC login rejected: email not verified") + redirectAuthError(w, r, authErrorEmailNotVerified) + + return + } + h.logger.ErrorCtx(ctx, "cannot handle OIDC callback", log.Error(err)) redirectAuthError(w, r, authErrorAuthenticationFailed) @@ -301,29 +308,29 @@ func (h *MagicLinkHandler) VerifyHandler(w http.ResponseWriter, r *http.Request) token := r.URL.Query().Get("token") if token == "" { - httpserver.RenderError(w, http.StatusBadRequest, errors.New("missing token")) + redirectAuthError(w, r, authErrorMagicLinkInvalid) return } identity, session, continueURL, err := h.iam.AuthService.OpenSessionWithMagicLink(ctx, token) if err != nil { if _, ok := errors.AsType[*iam.ErrExpiredToken](err); ok { - http.Redirect(w, r, "/auth/magic-link-expired", http.StatusFound) + redirectAuthError(w, r, authErrorMagicLinkExpired) return } if _, ok := errors.AsType[*iam.ErrTokenAlreadyUsed](err); ok { - http.Redirect(w, r, "/auth/magic-link-already-used", http.StatusFound) + redirectAuthError(w, r, authErrorMagicLinkAlreadyUsed) return } if _, ok := errors.AsType[*iam.ErrInvalidToken](err); ok { - httpserver.RenderError(w, http.StatusBadRequest, errors.New("invalid token")) + redirectAuthError(w, r, authErrorMagicLinkInvalid) return } h.logger.ErrorCtx(ctx, "cannot open session with magic link", log.Error(err)) - httpserver.RenderError(w, http.StatusInternalServerError, errors.New("internal server error")) + redirectAuthError(w, r, authErrorAuthenticationFailed) return } diff --git a/pkg/server/api/connect/v1/saml_handler.go b/pkg/server/api/connect/v1/saml_handler.go index 05a06d91c..844494952 100644 --- a/pkg/server/api/connect/v1/saml_handler.go +++ b/pkg/server/api/connect/v1/saml_handler.go @@ -32,7 +32,6 @@ import ( "go.probo.inc/probo/pkg/baseurl" "go.probo.inc/probo/pkg/gid" "go.probo.inc/probo/pkg/iam" - "go.probo.inc/probo/pkg/iam/saml" "go.probo.inc/probo/pkg/saferedirect" "go.probo.inc/probo/pkg/securecookie" "go.probo.inc/probo/pkg/server/api/authn" @@ -61,49 +60,15 @@ func (h *SAMLHandler) renderInternalServerError(w http.ResponseWriter) { } func (h *SAMLHandler) renderAssertionError(w http.ResponseWriter, r *http.Request, err error) { - if isClientSAMLError(err) { - httpserver.RenderError(w, http.StatusUnauthorized, err) + if code, ok := authErrorCodeFromSAML(err); ok { + h.logger.WarnCtx(r.Context(), "SAML login rejected", log.Error(err), log.String("error_code", code)) + redirectAuthError(w, r, code) + return } h.logger.ErrorCtx(r.Context(), "cannot handle SAML assertion", log.Error(err)) - httpserver.RenderError(w, http.StatusUnauthorized, errors.New("authentication failed")) -} - -func isClientSAMLError(err error) bool { - if _, ok := errors.AsType[*saml.ErrSAMLConfigurationNotFound](err); ok { - return true - } - - if _, ok := errors.AsType[*saml.ErrSAMLDisabled](err); ok { - return true - } - - if _, ok := errors.AsType[*saml.ErrInvalidAssertion](err); ok { - return true - } - - if _, ok := errors.AsType[*saml.ErrReplayAttackDetected](err); ok { - return true - } - - if _, ok := errors.AsType[*saml.ErrEmailDomainMismatch](err); ok { - return true - } - - if _, ok := errors.AsType[*saml.ErrSAMLAutoSignupDisabled](err); ok { - return true - } - - if _, ok := errors.AsType[*saml.ErrUserInactive](err); ok { - return true - } - - if _, ok := errors.AsType[*saml.ErrSAMLSubjectAlreadyInUse](err); ok { - return true - } - - return false + redirectAuthError(w, r, authErrorAuthenticationFailed) } func (h *SAMLHandler) MetadataHandler(w http.ResponseWriter, r *http.Request) {