paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 25 Packages Projects Releases Wiki Activity

change code to use search params instead of cookie

Browse Source 
Signed-off-by: Stephen Glass <stephen@stephen.glass>
This commit is contained in:
Stephen Glass
2024-10-01 23:12:10 -04:00
parent d964eb1a03
commit 4935d29d15
16 changed files with 34 additions and 388 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/violet-beds-promise.md
+1 -9
View File
@@ -1,14 +1,6 @@
---
'@backstage/core-components': patch
'@backstage/plugin-auth-backend': patch
'@backstage/plugin-auth-node': patch
'@backstage/plugin-auth-react': 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=true` query parameter. A cookie is also set in the redirect which contains the error message. The error can be fetched from any custom sign in page using the new `useSignInAuthError` hook.
```ts
import { useSignInAuthError } from '@backstage/plugin-auth-react';
const { error, checkAuthError } = useSignInAuthError();
```
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.
packages/core-components/package.json
-1
View File
@@ -57,7 +57,6 @@
"@backstage/config": "workspace:^",
"@backstage/core-plugin-api": "workspace:^",
"@backstage/errors": "workspace:^",
"@backstage/plugin-auth-react": "workspace:^",
"@backstage/theme": "workspace:^",
"@backstage/version-bridge": "workspace:^",
"@date-io/core": "^1.3.13",
packages/core-components/src/layout/SignInPage/SignInPage.tsx
+4 -13
View File
@@ -24,7 +24,7 @@ import { UserIdentity } from './UserIdentity';
import Button from '@material-ui/core/Button';
import Grid from '@material-ui/core/Grid';
import Typography from '@material-ui/core/Typography';
import React, { useEffect, useState } from 'react';
import React, { useState } from 'react';
import { useMountEffect } from '@react-hookz/web';
import { Progress } from '../../components/Progress';
import { Content } from '../Content/Content';
@@ -38,7 +38,6 @@ import { IdentityProviders, SignInProviderConfig } from './types';
import { coreComponentsTranslationRef } from '../../translation';
import { useTranslationRef } from '@backstage/core-plugin-api/alpha';
import { useSearchParams } from 'react-router-dom';
import { useSignInAuthError } from '@backstage/plugin-auth-react';
type MultiSignInPageProps = SignInPageProps & {
providers: IdentityProviders;
@@ -99,7 +98,6 @@ export const SingleSignInPage = ({
const classes = useStyles();
const authApi = useApi(provider.apiRef);
const configApi = useApi(configApiRef);
const { error: signInError, checkAuthError } = useSignInAuthError();
const { t } = useTranslationRef(coreComponentsTranslationRef);
const [error, setError] = useState<Error>();
@@ -112,7 +110,6 @@ export const SingleSignInPage = ({
// User was redirected back to sign in page with error from auth redirect flow
const [searchParams, _setSearchParams] = useSearchParams();
const errorParam = searchParams.get('error');
const hasErrorSearchParam = errorParam !== 'false' && errorParam !== null;
type LoginOpts = { checkExisting?: boolean; showPopup?: boolean };
const login = async ({ checkExisting, showPopup }: LoginOpts) => {
@@ -126,7 +123,7 @@ export const SingleSignInPage = ({
}
// If no session exists, show the sign-in page
if (!identityResponse && (showPopup || auto) && !hasErrorSearchParam) {
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);
@@ -161,18 +158,12 @@ export const SingleSignInPage = ({
};
useMountEffect(() => {
if (hasErrorSearchParam) {
checkAuthError();
if (errorParam) {
setError(new Error(decodeURIComponent(errorParam)));
}
login({ checkExisting: true });
});
useEffect(() => {
if (signInError) {
setError(signInError);
}
}, [signInError]);
return showLoginPage ? (
<Page themeId="home">
<Header title={configApi.getString('app.title')} />
packages/core-components/src/layout/SignInPage/providers.tsx
+15 -17
View File
@@ -14,13 +14,7 @@
* limitations under the License.
*/
import React, {
useLayoutEffect,
useState,
useMemo,
useCallback,
useEffect,
} from 'react';
import React, { useLayoutEffect, useState, useMemo, useCallback } from 'react';
import {
SignInPageProps,
useApi,
@@ -39,7 +33,9 @@ import { customProvider } from './customProvider';
import { IdentityApiSignOutProxy } from './IdentityApiSignOutProxy';
import { useSearchParams } from 'react-router-dom';
import { useMountEffect } from '@react-hookz/web';
import { useSignInAuthError } from '@backstage/plugin-auth-react';
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';
@@ -95,21 +91,23 @@ export const useSignInProviders = (
) => {
const errorApi = useApi(errorApiRef);
const apiHolder = useApiHolder();
const { error: signInError, checkAuthError } = useSignInAuthError();
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();
const errorParam = searchParams.get('error');
const hasErrorSearchParam = errorParam !== 'false' && errorParam !== null;
useMountEffect(() => hasErrorSearchParam && checkAuthError());
useEffect(() => {
if (signInError) {
errorApi.post(signInError);
useMountEffect(() => {
const errorParam = searchParams.get('error');
if (errorParam) {
errorApi.post(
new ForwardedError(t('signIn.loginFailed'), {
name: 'Error',
message: decodeURIComponent(errorParam),
}),
);
}
}, [errorApi, signInError]);
});
// This decorates the result with sign out logic from this hook
const handleWrappedResult = useCallback(
plugins/auth-backend/src/service/createCookieAuthErrorMiddleware.test.ts
-56
View File
@@ -1,56 +0,0 @@
/*
* Copyright 2024 The Backstage Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import express from 'express';
import request from 'supertest';
import cookieParser from 'cookie-parser';
import { createCookieAuthErrorMiddleware } from './createCookieAuthErrorMiddleware';
const AUTH_ERROR_COOKIE = 'auth-error';
describe('createCookieAuthErrorMiddleware', () => {
let app: express.Express;
beforeEach(() => {
app = express();
app.use(express.json());
app.use(express.urlencoded({ extended: true }));
app.use(cookieParser());
app.use(
createCookieAuthErrorMiddleware(
'http://localhost:3000',
'http://localhost:7000',
),
);
});
it('should return cookie content if error cookie exists', async () => {
const error = 'test';
const res = await request(app)
.get('/.backstage/error')
.set('Cookie', `${AUTH_ERROR_COOKIE}=${encodeURIComponent(error)}`);
expect(res.status).toBe(200);
expect(res.body).toEqual('test');
});
it('should return 404 if error cookie does not exist', async () => {
const res = await request(app).get('/.backstage/error');
expect(res.status).toBe(404);
});
});
plugins/auth-backend/src/service/createCookieAuthErrorMiddleware.ts
-52
View File
@@ -1,52 +0,0 @@
/*
* Copyright 2024 The Backstage Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import Router from 'express-promise-router';
const AUTH_ERROR_COOKIE = 'auth-error';
/**
* @public
* Creates a middleware that can be used to get auth errors in redirect flow
*/
export function createCookieAuthErrorMiddleware(
appUrl: string,
authUrl: string,
) {
const router = Router();
router.get('/.backstage/error', async (req, res) => {
const error = req.cookies[AUTH_ERROR_COOKIE];
if (error) {
const { hostname: domain, protocol } = new URL(authUrl);
const secure = protocol === 'https:';
const sameSite =
new URL(appUrl).hostname !== domain && secure ? 'none' : 'lax';
res.clearCookie(AUTH_ERROR_COOKIE, {
path: '/api/auth/.backstage/error',
domain,
sameSite,
secure,
});
res.status(200).json(error);
} else {
res.status(404).end();
}
});
return router;
}
plugins/auth-backend/src/service/router.ts
-3
View File
@@ -48,7 +48,6 @@ import { StaticTokenIssuer } from '../identity/StaticTokenIssuer';
import { StaticKeyStore } from '../identity/StaticKeyStore';
import { Config } from '@backstage/config';
import { bindProviderRouters, ProviderFactories } from '../providers/router';
import { createCookieAuthErrorMiddleware } from './createCookieAuthErrorMiddleware';
/** @public */
export interface RouterOptions {
@@ -170,8 +169,6 @@ export async function createRouter(
userInfoDatabaseHandler,
});
router.use(createCookieAuthErrorMiddleware(appUrl, authUrl));
// Gives a more helpful error message than a plain 404
router.use('/:provider/', req => {
const { provider } = req.params;
plugins/auth-node/src/oauth/createAuthErrorCookie.ts
-55
View File
@@ -1,55 +0,0 @@
/*
* Copyright 2024 The Backstage Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { Response } from 'express';
import { serializeError } from '@backstage/errors';
const ONE_MINUTE_MS = 60 * 1000;
const AUTH_ERROR_COOKIE = 'auth-error';
function configureAuthErrorCookie(apiUrl: string, appOrigin: string) {
const { hostname: domain, pathname: path, protocol } = new URL(apiUrl);
const secure = protocol === 'https:';
// For situations where the auth-backend is running on a
// different domain than the app, we set the SameSite attribute
// to 'none' to allow third-party access to the cookie, but
// only if it's in a secure context (https).
let sameSite: 'lax' | 'none' = 'lax';
if (new URL(appOrigin).hostname !== domain && secure) {
sameSite = 'none';
}
return { domain, path, secure, sameSite };
}
export function createAuthErrorCookie(
res: Response,
origin: string,
options: {
error: Error;
apiUrl: string;
},
) {
const { error, apiUrl } = options;
const jsonData = serializeError(error);
res.cookie(AUTH_ERROR_COOKIE, jsonData, {
maxAge: ONE_MINUTE_MS,
httpOnly: true,
...configureAuthErrorCookie(apiUrl, origin),
});
}
plugins/auth-node/src/oauth/createOAuthRouteHandlers.test.ts
+13 -7
View File
@@ -743,7 +743,7 @@ describe('createOAuthRouteHandlers', () => {
});
});
it('should set cookie and redirect on caught error', async () => {
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')
@@ -756,15 +756,21 @@ describe('createOAuthRouteHandlers', () => {
}),
});
// redirects on error with auth error cookie
// Check if it redirects on error
expect(res.status).toBe(302);
const setCookieHeader = res.header['set-cookie'];
expect(setCookieHeader).toBeDefined();
const authErrorCookie = setCookieHeader.find((cookie: string) =>
cookie.startsWith('auth-error='),
// 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(
encodeURIComponent('Auth response is missing cookie nonce'),
);
expect(authErrorCookie).toBeDefined();
});
});
});
plugins/auth-node/src/oauth/createOAuthRouteHandlers.ts
+1 -7
View File
@@ -42,7 +42,6 @@ import {
import { OAuthAuthenticator, OAuthAuthenticatorResult } from './types';
import { Config } from '@backstage/config';
import { CookieScopeManager } from './CookieScopeManager';
import { createAuthErrorCookie } from './createAuthErrorCookie';
/** @public */
export interface OAuthRouteHandlersOptions<TProfile> {
@@ -252,13 +251,8 @@ export function createOAuthRouteHandlers<TProfile>(
: new Error('Encountered invalid error'); // Being a bit safe and not forwarding the bad value
if (state?.flow === 'redirect' && state?.redirectUrl) {
createAuthErrorCookie(res, state?.redirectUrl, {
error: { name, message },
apiUrl: `${baseUrl}/.backstage/error`,
});
const redirectUrl = new URL(state.redirectUrl);
redirectUrl.searchParams.set('error', 'true');
redirectUrl.searchParams.set('error', encodeURIComponent(message));
// set the error in a cookie and redirect user back to sign in where the error can be rendered
res.redirect(redirectUrl.toString());
plugins/auth-react/api-report.md
-6
View File
@@ -36,10 +36,4 @@ export function useCookieAuthRefresh(options: { pluginId: string }):
expiresAt: string;
};
};
// @public
export function useSignInAuthError(): {
error: Error | undefined;
checkAuthError: () => void;
};
```
plugins/auth-react/src/hooks/index.ts
-1
View File
@@ -18,4 +18,3 @@
// which hooks are public API and should be exported from the package.
export * from './useCookieAuthRefresh';
export * from './useSignInAuthError';
plugins/auth-react/src/hooks/useSignInAuthError/index.tsx
-17
View File
@@ -1,17 +0,0 @@
/*
* Copyright 2024 The Backstage Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
export { useSignInAuthError } from './useSignInAuthError';
plugins/auth-react/src/hooks/useSignInAuthError/useSignInAuthError.test.tsx
-98
View File
@@ -1,98 +0,0 @@
/*
* Copyright 2024 The Backstage Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import React from 'react';
import { act, renderHook } from '@testing-library/react';
import { discoveryApiRef } from '@backstage/core-plugin-api';
import { TestApiProvider } from '@backstage/test-utils';
import { useSignInAuthError } from './useSignInAuthError';
import { serializeError } from '@backstage/errors';
describe('useSignInAuthError', () => {
const discoveryApiMock = {
getBaseUrl: jest.fn().mockResolvedValue('http://localhost:7000/api/auth'),
};
beforeEach(() => {
jest.clearAllMocks();
});
it('should return an error when the cookie returns an error object', async () => {
const errorObject = {
name: 'TestError',
message: 'This is a test error',
};
const serializedError = serializeError(errorObject);
global.fetch = jest.fn().mockResolvedValue({
ok: true,
json: jest.fn().mockResolvedValue(serializedError),
} as unknown as Response);
const { result } = renderHook(() => useSignInAuthError(), {
wrapper: ({ children }) => (
<TestApiProvider apis={[[discoveryApiRef, discoveryApiMock]]}>
{children}
</TestApiProvider>
),
});
await act(async () => {
result.current.checkAuthError();
});
expect(discoveryApiMock.getBaseUrl).toHaveBeenCalledWith('auth');
expect(fetch).toHaveBeenCalledWith(
'http://localhost:7000/api/auth/.backstage/error',
{
credentials: 'include',
},
);
expect(result.current.error).toBeInstanceOf(Error);
expect((result.current.error as Error).name).toEqual(errorObject.name);
expect((result.current.error as Error).message).toEqual(
errorObject.message,
);
});
it('should return undefined when the backend does not return an error object', async () => {
global.fetch = jest.fn().mockResolvedValue({
ok: true,
json: jest.fn().mockResolvedValue(undefined),
} as unknown as Response);
const { result } = renderHook(() => useSignInAuthError(), {
wrapper: ({ children }) => (
<TestApiProvider apis={[[discoveryApiRef, discoveryApiMock]]}>
{children}
</TestApiProvider>
),
});
await act(async () => {
result.current.checkAuthError();
});
expect(discoveryApiMock.getBaseUrl).toHaveBeenCalledWith('auth');
expect(fetch).toHaveBeenCalledWith(
'http://localhost:7000/api/auth/.backstage/error',
{
credentials: 'include',
},
);
expect(result.current.error).toBeUndefined();
});
});
plugins/auth-react/src/hooks/useSignInAuthError/useSignInAuthError.tsx
-45
View File
@@ -1,45 +0,0 @@
/*
* Copyright 2024 The Backstage Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { discoveryApiRef, useApi } from '@backstage/core-plugin-api';
import { useAsync } from '@react-hookz/web';
import { deserializeError } from '@backstage/errors';
/**
* @public
* A hook that will fetch any sign in auth error in redirect auth flow
*/
export function useSignInAuthError(): {
error: Error | undefined;
checkAuthError: () => void;
} {
const discoveryApi = useApi(discoveryApiRef);
const [state, { execute: checkAuthError }] = useAsync(async () => {
const baseUrl = await discoveryApi.getBaseUrl('auth');
// use native fetch instead of fetchApi because
// we are not signed in and are calling an unauthenticated endpoint
const response = await fetch(`${baseUrl}/.backstage/error`, {
credentials: 'include',
});
const data = await response.json();
return data ? deserializeError(data) : undefined;
});
return { error: state.result, checkAuthError };
}
yarn.lock
-1
View File
@@ -4287,7 +4287,6 @@ __metadata:
"@backstage/core-app-api": "workspace:^"
"@backstage/core-plugin-api": "workspace:^"
"@backstage/errors": "workspace:^"
"@backstage/plugin-auth-react": "workspace:^"
"@backstage/test-utils": "workspace:^"
"@backstage/theme": "workspace:^"
"@backstage/version-bridge": "workspace:^"