diff --git a/.changeset/violet-beds-promise.md b/.changeset/violet-beds-promise.md new file mode 100644 index 0000000000..b5369d4c56 --- /dev/null +++ b/.changeset/violet-beds-promise.md @@ -0,0 +1,6 @@ +--- +'@backstage/core-components': patch +'@backstage/plugin-auth-node': patch +--- + +Fix authentication error handling using redirect flow via `enableExperimentalRedirectFlow` config. If an error is caught during authentication, the user is redirected back to app origin with `error` query parameter containing the error message. diff --git a/packages/core-components/src/layout/SignInPage/SignInPage.tsx b/packages/core-components/src/layout/SignInPage/SignInPage.tsx index fb8f804d5e..c3139f9316 100644 --- a/packages/core-components/src/layout/SignInPage/SignInPage.tsx +++ b/packages/core-components/src/layout/SignInPage/SignInPage.tsx @@ -37,6 +37,7 @@ import { GridItem, useStyles } from './styles'; import { IdentityProviders, SignInProviderConfig } from './types'; import { coreComponentsTranslationRef } from '../../translation'; import { useTranslationRef } from '@backstage/core-plugin-api/alpha'; +import { useSearchParams } from 'react-router-dom'; type MultiSignInPageProps = SignInPageProps & { providers: IdentityProviders; @@ -114,6 +115,10 @@ export const SingleSignInPage = ({ // displayed for a split second when the user is already logged-in. const [showLoginPage, setShowLoginPage] = useState(false); + // User was redirected back to sign in page with error from auth redirect flow + const [searchParams, _setSearchParams] = useSearchParams(); + const errorParam = searchParams.get('error'); + type LoginOpts = { checkExisting?: boolean; showPopup?: boolean }; const login = async ({ checkExisting, showPopup }: LoginOpts) => { try { @@ -126,7 +131,7 @@ export const SingleSignInPage = ({ } // If no session exists, show the sign-in page - if (!identityResponse && (showPopup || auto)) { + if (!identityResponse && (showPopup || auto) && !errorParam) { // Unless auto is set to true, this step should not happen. // When user intentionally clicks the Sign In button, autoShowPopup is set to true setShowLoginPage(true); @@ -160,7 +165,12 @@ export const SingleSignInPage = ({ } }; - useMountEffect(() => login({ checkExisting: true })); + useMountEffect(() => { + if (errorParam) { + setError(new Error(errorParam)); + } + login({ checkExisting: true }); + }); return showLoginPage ? ( diff --git a/packages/core-components/src/layout/SignInPage/providers.tsx b/packages/core-components/src/layout/SignInPage/providers.tsx index e456a6948b..c1c3b744c1 100644 --- a/packages/core-components/src/layout/SignInPage/providers.tsx +++ b/packages/core-components/src/layout/SignInPage/providers.tsx @@ -31,6 +31,11 @@ import { commonProvider } from './commonProvider'; import { guestProvider } from './guestProvider'; import { customProvider } from './customProvider'; import { IdentityApiSignOutProxy } from './IdentityApiSignOutProxy'; +import { useSearchParams } from 'react-router-dom'; +import { useMountEffect } from '@react-hookz/web'; +import { ForwardedError } from '@backstage/errors'; +import { coreComponentsTranslationRef } from '../../translation'; +import { useTranslationRef } from '@backstage/core-plugin-api/alpha'; const PROVIDER_STORAGE_KEY = '@backstage/core:SignInPage:provider'; @@ -88,6 +93,19 @@ export const useSignInProviders = ( const apiHolder = useApiHolder(); const [loading, setLoading] = useState(true); + const { t } = useTranslationRef(coreComponentsTranslationRef); + // User was redirected back to sign in page with error from auth redirect flow + const [searchParams, _setSearchParams] = useSearchParams(); + + useMountEffect(() => { + const errorParam = searchParams.get('error'); + if (errorParam) { + errorApi.post( + new ForwardedError(t('signIn.loginFailed'), new Error(errorParam)), + ); + } + }); + // This decorates the result with sign out logic from this hook const handleWrappedResult = useCallback( (identityApi: IdentityApi) => { diff --git a/plugins/auth-node/src/oauth/createOAuthRouteHandlers.test.ts b/plugins/auth-node/src/oauth/createOAuthRouteHandlers.test.ts index 7da5519655..edf816cbee 100644 --- a/plugins/auth-node/src/oauth/createOAuthRouteHandlers.test.ts +++ b/plugins/auth-node/src/oauth/createOAuthRouteHandlers.test.ts @@ -742,5 +742,33 @@ describe('createOAuthRouteHandlers', () => { }, }); }); + + it('should set error search param and redirect on caught error', async () => { + const app = wrapInApp(createOAuthRouteHandlers(baseConfig)); + const res = await request(app) + .get('/my-provider/handler/frame') + .query({ + state: encodeOAuthState({ + env: 'development', + nonce: '123', + flow: 'redirect', + redirectUrl: 'http://localhost:3000', + }), + }); + + // Check if it redirects on error + expect(res.status).toBe(302); + + // Extract the redirect URL from the location header + const redirectLocation = res.header.location; + expect(redirectLocation).toBeDefined(); + + // Create a URL object from the redirect location + const redirectUrl = new URL(redirectLocation); + + // Verify that the 'error' search param is set with the encoded error message + const errorMessage = redirectUrl.searchParams.get('error'); + expect(errorMessage).toBe('Auth response is missing cookie nonce'); + }); }); }); diff --git a/plugins/auth-node/src/oauth/createOAuthRouteHandlers.ts b/plugins/auth-node/src/oauth/createOAuthRouteHandlers.ts index 82bfa2f02c..53ae4101d3 100644 --- a/plugins/auth-node/src/oauth/createOAuthRouteHandlers.ts +++ b/plugins/auth-node/src/oauth/createOAuthRouteHandlers.ts @@ -164,9 +164,10 @@ export function createOAuthRouteHandlers( res: express.Response, ): Promise { let origin = defaultAppOrigin; + let state; try { - const state = decodeOAuthState(req.query.state?.toString() ?? ''); + state = decodeOAuthState(req.query.state?.toString() ?? ''); if (state.origin) { try { @@ -248,11 +249,20 @@ export function createOAuthRouteHandlers( const { name, message } = isError(error) ? error : new Error('Encountered invalid error'); // Being a bit safe and not forwarding the bad value - // post error message back to popup if failure - sendWebMessageResponse(res, origin, { - type: 'authorization_response', - error: { name, message }, - }); + + if (state?.flow === 'redirect' && state?.redirectUrl) { + const redirectUrl = new URL(state.redirectUrl); + redirectUrl.searchParams.set('error', message); + + // set the error in a cookie and redirect user back to sign in where the error can be rendered + res.redirect(redirectUrl.toString()); + } else { + // post error message back to popup if failure + sendWebMessageResponse(res, origin, { + type: 'authorization_response', + error: { name, message }, + }); + } } },