feat: remove cookie on sign out
Signed-off-by: Camila Belo <camilaibs@gmail.com>
This commit is contained in:
committed by
Patrik Oldsberg
parent
641a068514
commit
a1950ad5e6
+4
@@ -254,6 +254,10 @@ class DefaultHttpAuthService implements HttpAuthService {
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
removeUserCookie(res: Response): void {
|
||||
res.clearCookie(BACKSTAGE_AUTH_COOKIE);
|
||||
}
|
||||
}
|
||||
|
||||
/** @public */
|
||||
|
||||
+8
-2
@@ -59,6 +59,8 @@ export const httpRouterServiceFactory = createServiceFactory(
|
||||
rootHttpRouter,
|
||||
lifecycle,
|
||||
}) {
|
||||
let hasRegistedCookieAuthRefreshMiddleware = false;
|
||||
|
||||
if (options?.getPath) {
|
||||
logger.warn(
|
||||
`DEPRECATION WARNING: The 'getPath' option for HttpRouterService is deprecated. The ability to reconfigure the '/api/' path prefix for plugins will be removed in the future.`,
|
||||
@@ -85,8 +87,12 @@ export const httpRouterServiceFactory = createServiceFactory(
|
||||
},
|
||||
addAuthPolicy(policy: HttpRouterServiceAuthPolicy): void {
|
||||
credentialsBarrier.addAuthPolicy(policy);
|
||||
if (policy.allow === 'user-cookie') {
|
||||
// TODO: Make sure this is only added once
|
||||
if (
|
||||
policy.allow === 'user-cookie' &&
|
||||
!hasRegistedCookieAuthRefreshMiddleware
|
||||
) {
|
||||
// Only add the cookie refresh middleware once
|
||||
hasRegistedCookieAuthRefreshMiddleware = true;
|
||||
router.use(createCookieAuthRefreshMiddleware({ httpAuth }));
|
||||
}
|
||||
},
|
||||
|
||||
@@ -259,6 +259,10 @@ class HttpAuthCompat implements HttpAuthService {
|
||||
async issueUserCookie(_res: Response): Promise<{ expiresAt: Date }> {
|
||||
return { expiresAt: new Date(Date.now() + 3600_000) };
|
||||
}
|
||||
|
||||
removeUserCookie(res: Response): void {
|
||||
res.clearCookie('backstage-auth');
|
||||
}
|
||||
}
|
||||
|
||||
export class UserInfoCompat implements UserInfoService {
|
||||
|
||||
@@ -330,6 +330,8 @@ export interface HttpAuthService {
|
||||
): Promise<{
|
||||
expiresAt: Date;
|
||||
}>;
|
||||
// (undocumented)
|
||||
removeUserCookie(res: Response_2): void;
|
||||
}
|
||||
|
||||
// @public (undocumented)
|
||||
|
||||
@@ -37,4 +37,6 @@ export interface HttpAuthService {
|
||||
credentials?: BackstageCredentials<BackstageUserPrincipal>;
|
||||
},
|
||||
): Promise<{ expiresAt: Date }>;
|
||||
|
||||
removeUserCookie(res: Response): void;
|
||||
}
|
||||
|
||||
@@ -140,4 +140,8 @@ export class MockHttpAuthService implements HttpAuthService {
|
||||
|
||||
return { expiresAt: new Date(Date.now() + 3600_000) };
|
||||
}
|
||||
|
||||
removeUserCookie(res: Response): void {
|
||||
res.clearCookie(MOCK_AUTH_COOKIE);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -275,6 +275,7 @@ export namespace mockServices {
|
||||
export const mock = simpleMock(coreServices.httpAuth, () => ({
|
||||
credentials: jest.fn(),
|
||||
issueUserCookie: jest.fn(),
|
||||
removeUserCookie: jest.fn(),
|
||||
}));
|
||||
}
|
||||
|
||||
|
||||
@@ -50,6 +50,7 @@ export class AppIdentityProxy implements IdentityApi {
|
||||
private waitForTarget: Promise<CompatibilityIdentityApi>;
|
||||
private resolveTarget: (api: CompatibilityIdentityApi) => void = () => {};
|
||||
private signOutTargetUrl = '/';
|
||||
#signOutCallback?: () => Promise<void>;
|
||||
|
||||
constructor() {
|
||||
this.waitForTarget = new Promise<CompatibilityIdentityApi>(resolve => {
|
||||
@@ -67,6 +68,10 @@ export class AppIdentityProxy implements IdentityApi {
|
||||
this.resolveTarget(identityApi);
|
||||
}
|
||||
|
||||
setSignOutCallback(signOutCallback: () => Promise<void>): void {
|
||||
this.#signOutCallback = signOutCallback;
|
||||
}
|
||||
|
||||
getUserId(): string {
|
||||
if (!this.target) {
|
||||
throw mkError('getUserId');
|
||||
@@ -124,6 +129,7 @@ export class AppIdentityProxy implements IdentityApi {
|
||||
|
||||
async signOut(): Promise<void> {
|
||||
await this.waitForTarget.then(target => target.signOut());
|
||||
await this.#signOutCallback?.();
|
||||
window.location.href = this.signOutTargetUrl;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -20,7 +20,8 @@ import {
|
||||
renderWithEffects,
|
||||
withLogCollector,
|
||||
} from '@backstage/test-utils';
|
||||
import { render, screen } from '@testing-library/react';
|
||||
import { render, screen, waitFor } from '@testing-library/react';
|
||||
import userEvent from '@testing-library/user-event';
|
||||
import React, { PropsWithChildren, ReactNode } from 'react';
|
||||
import { BrowserRouter, Navigate, Route, Routes } from 'react-router-dom';
|
||||
import {
|
||||
@@ -36,7 +37,11 @@ import {
|
||||
analyticsApiRef,
|
||||
useApi,
|
||||
errorApiRef,
|
||||
fetchApiRef,
|
||||
discoveryApiRef,
|
||||
identityApiRef,
|
||||
} from '@backstage/core-plugin-api';
|
||||
import { AppRouter } from './AppRouter';
|
||||
import { AppManager } from './AppManager';
|
||||
import { AppComponents, AppIcons } from './types';
|
||||
import { FeatureFlagged } from '../routing/FeatureFlagged';
|
||||
@@ -817,4 +822,51 @@ describe('Integration Test', () => {
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it('should clear app cookie when the user logs out', async () => {
|
||||
const fetchApiMock = { fetch: jest.fn().mockResolvedValue({ ok: true }) };
|
||||
const discoveryApiMock = {
|
||||
getBaseUrl: jest.fn().mockResolvedValue('http://localhost:7007/app'),
|
||||
};
|
||||
const app = new AppManager({
|
||||
icons,
|
||||
themes,
|
||||
components,
|
||||
configLoader: async () => [],
|
||||
defaultApis: [
|
||||
noopErrorApi,
|
||||
createApiFactory({
|
||||
api: fetchApiRef,
|
||||
deps: {},
|
||||
factory: () => fetchApiMock,
|
||||
}),
|
||||
createApiFactory({
|
||||
api: discoveryApiRef,
|
||||
deps: {},
|
||||
factory: () => discoveryApiMock,
|
||||
}),
|
||||
],
|
||||
});
|
||||
|
||||
const SignOutButton = () => {
|
||||
const identityApi = useApi(identityApiRef);
|
||||
return <button onClick={() => identityApi.signOut()}>Sign Out</button>;
|
||||
};
|
||||
|
||||
const Root = app.createRoot(
|
||||
<AppRouter>
|
||||
<SignOutButton />
|
||||
</AppRouter>,
|
||||
);
|
||||
await renderWithEffects(<Root />);
|
||||
|
||||
await userEvent.click(screen.getByText('Sign Out'));
|
||||
|
||||
await waitFor(() =>
|
||||
expect(fetchApiMock.fetch).toHaveBeenCalledWith(
|
||||
'http://localhost:7007/app/.backstage/v1-cookie',
|
||||
{ method: 'DELETE' },
|
||||
),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -42,6 +42,8 @@ import {
|
||||
identityApiRef,
|
||||
BackstagePlugin,
|
||||
FeatureFlag,
|
||||
fetchApiRef,
|
||||
discoveryApiRef,
|
||||
} from '@backstage/core-plugin-api';
|
||||
import {
|
||||
AppLanguageApi,
|
||||
@@ -354,8 +356,25 @@ DEPRECATION WARNING: React Router Beta is deprecated and support for it will be
|
||||
|
||||
const { ThemeProvider = AppThemeProvider, Progress } = this.components;
|
||||
|
||||
const apis = this.getApiHolder();
|
||||
|
||||
this.appIdentityProxy.setSignOutCallback(async () => {
|
||||
const fetchApi = apis.get(fetchApiRef);
|
||||
const discoveryApi = apis.get(discoveryApiRef);
|
||||
if (!fetchApi || !discoveryApi) return;
|
||||
// It is fine if we do NOT worry yet about deleting cookies for OTHER backends like techdocs
|
||||
const appBaseUrl = await discoveryApi.getBaseUrl('app');
|
||||
try {
|
||||
await fetchApi.fetch(`${appBaseUrl}/.backstage/v1-cookie`, {
|
||||
method: 'DELETE',
|
||||
});
|
||||
} catch {
|
||||
// Ignore the error for those who use static serving of the frontend
|
||||
}
|
||||
});
|
||||
|
||||
return (
|
||||
<ApiProvider apis={this.getApiHolder()}>
|
||||
<ApiProvider apis={apis}>
|
||||
<AppContextProvider appContext={appContext}>
|
||||
<ThemeProvider>
|
||||
<RoutingProvider
|
||||
|
||||
@@ -3,9 +3,11 @@
|
||||
> Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/).
|
||||
|
||||
```ts
|
||||
import { AuthService } from '@backstage/backend-plugin-api';
|
||||
import { Config } from '@backstage/config';
|
||||
import { ConfigSchema } from '@backstage/config-loader';
|
||||
import express from 'express';
|
||||
import { HttpAuthService } from '@backstage/backend-plugin-api';
|
||||
import { Logger } from 'winston';
|
||||
import { PluginDatabaseManager } from '@backstage/backend-common';
|
||||
|
||||
@@ -16,10 +18,14 @@ export function createRouter(options: RouterOptions): Promise<express.Router>;
|
||||
export interface RouterOptions {
|
||||
appPackageName: string;
|
||||
// (undocumented)
|
||||
auth?: AuthService;
|
||||
// (undocumented)
|
||||
config: Config;
|
||||
database?: PluginDatabaseManager;
|
||||
disableConfigInjection?: boolean;
|
||||
// (undocumented)
|
||||
httpAuth?: HttpAuthService;
|
||||
// (undocumented)
|
||||
logger: Logger;
|
||||
schema?: ConfigSchema;
|
||||
staticFallbackHandler?: express.Handler;
|
||||
|
||||
@@ -39,6 +39,7 @@
|
||||
"@backstage/errors": "workspace:^",
|
||||
"@backstage/types": "workspace:^",
|
||||
"@types/express": "*",
|
||||
"@types/express-serve-static-core": "^4.17.5",
|
||||
"@types/passport": "^1.0.3",
|
||||
"express": "^4.17.1",
|
||||
"jose": "^5.0.0",
|
||||
|
||||
@@ -0,0 +1,48 @@
|
||||
/*
|
||||
* 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 { mockCredentials, mockServices } from '@backstage/backend-test-utils';
|
||||
import { createCookieAuthRefreshMiddleware } from './createCookieAuthRefreshMiddleware';
|
||||
|
||||
describe('createCookieAuthRefreshMiddleware', () => {
|
||||
let app: express.Express;
|
||||
|
||||
beforeAll(async () => {
|
||||
const httpAuth = mockServices.httpAuth();
|
||||
const router = createCookieAuthRefreshMiddleware({ httpAuth });
|
||||
app = express().use(router);
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
jest.resetAllMocks();
|
||||
});
|
||||
|
||||
it('should issue the user cookie', async () => {
|
||||
const response = await request(app).get('/.backstage/v1-cookie');
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.header['set-cookie'][0]).toMatch(
|
||||
`backstage-auth=${mockCredentials.limitedUser.token()}`,
|
||||
);
|
||||
});
|
||||
|
||||
it('should remove the user cookie', async () => {
|
||||
const response = await request(app).delete('/.backstage/v1-cookie');
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.header['set-cookie'][0]).toMatch('backstage-auth=');
|
||||
});
|
||||
});
|
||||
@@ -17,6 +17,8 @@
|
||||
import { HttpAuthService } from '@backstage/backend-plugin-api';
|
||||
import { Router } from 'express';
|
||||
|
||||
const WELL_KNOWN_COOKIE_PATH_V1 = '/.backstage/v1-cookie';
|
||||
|
||||
/**
|
||||
* @public
|
||||
* Creates a middleware that can be used to refresh the cookie for the user.
|
||||
@@ -28,10 +30,16 @@ export function createCookieAuthRefreshMiddleware(options: {
|
||||
const router = Router();
|
||||
|
||||
// Endpoint that sets the cookie for the user
|
||||
router.get('/.backstage/v1-cookie', async (_, res) => {
|
||||
router.get(WELL_KNOWN_COOKIE_PATH_V1, async (_, res) => {
|
||||
const { expiresAt } = await httpAuth.issueUserCookie(res);
|
||||
res.json({ expiresAt: expiresAt.toISOString() });
|
||||
});
|
||||
|
||||
// Endpoint that removes the cookie for the user
|
||||
router.delete(WELL_KNOWN_COOKIE_PATH_V1, async (_, res) => {
|
||||
httpAuth.removeUserCookie(res);
|
||||
res.send(200);
|
||||
});
|
||||
|
||||
return router;
|
||||
}
|
||||
|
||||
@@ -5086,6 +5086,7 @@ __metadata:
|
||||
"@backstage/errors": "workspace:^"
|
||||
"@backstage/types": "workspace:^"
|
||||
"@types/express": "*"
|
||||
"@types/express-serve-static-core": ^4.17.5
|
||||
"@types/passport": ^1.0.3
|
||||
cookie-parser: ^1.4.6
|
||||
express: ^4.17.1
|
||||
@@ -5118,7 +5119,6 @@ __metadata:
|
||||
"@testing-library/react": ^14.0.0
|
||||
"@testing-library/user-event": ^14.0.0
|
||||
"@types/react": ^16.13.1 || ^17.0.0 || ^18.0.0
|
||||
msw: ^1.0.0
|
||||
peerDependencies:
|
||||
react: ^16.13.1 || ^17.0.0 || ^18.0.0
|
||||
languageName: unknown
|
||||
|
||||
Reference in New Issue
Block a user