diff --git a/packages/backend-app-api/src/services/implementations/httpAuth/httpAuthServiceFactory.ts b/packages/backend-app-api/src/services/implementations/httpAuth/httpAuthServiceFactory.ts index 06e65a1413..807e31513e 100644 --- a/packages/backend-app-api/src/services/implementations/httpAuth/httpAuthServiceFactory.ts +++ b/packages/backend-app-api/src/services/implementations/httpAuth/httpAuthServiceFactory.ts @@ -254,6 +254,10 @@ class DefaultHttpAuthService implements HttpAuthService { throw error; } } + + removeUserCookie(res: Response): void { + res.clearCookie(BACKSTAGE_AUTH_COOKIE); + } } /** @public */ diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.ts b/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.ts index fc5abeb43c..5f32eb8d00 100644 --- a/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.ts +++ b/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.ts @@ -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 })); } }, diff --git a/packages/backend-common/src/auth/createLegacyAuthAdapters.ts b/packages/backend-common/src/auth/createLegacyAuthAdapters.ts index 0ba23661ef..7718f74ee8 100644 --- a/packages/backend-common/src/auth/createLegacyAuthAdapters.ts +++ b/packages/backend-common/src/auth/createLegacyAuthAdapters.ts @@ -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 { diff --git a/packages/backend-plugin-api/api-report.md b/packages/backend-plugin-api/api-report.md index 21a3bf10f8..d91f4a2450 100644 --- a/packages/backend-plugin-api/api-report.md +++ b/packages/backend-plugin-api/api-report.md @@ -330,6 +330,8 @@ export interface HttpAuthService { ): Promise<{ expiresAt: Date; }>; + // (undocumented) + removeUserCookie(res: Response_2): void; } // @public (undocumented) diff --git a/packages/backend-plugin-api/src/services/definitions/HttpAuthService.ts b/packages/backend-plugin-api/src/services/definitions/HttpAuthService.ts index 637109d1f0..eb6ba944f5 100644 --- a/packages/backend-plugin-api/src/services/definitions/HttpAuthService.ts +++ b/packages/backend-plugin-api/src/services/definitions/HttpAuthService.ts @@ -37,4 +37,6 @@ export interface HttpAuthService { credentials?: BackstageCredentials; }, ): Promise<{ expiresAt: Date }>; + + removeUserCookie(res: Response): void; } diff --git a/packages/backend-test-utils/src/next/services/MockHttpAuthService.ts b/packages/backend-test-utils/src/next/services/MockHttpAuthService.ts index 9a69620473..8934def5f0 100644 --- a/packages/backend-test-utils/src/next/services/MockHttpAuthService.ts +++ b/packages/backend-test-utils/src/next/services/MockHttpAuthService.ts @@ -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); + } } diff --git a/packages/backend-test-utils/src/next/services/mockServices.ts b/packages/backend-test-utils/src/next/services/mockServices.ts index a858bf7dbf..00f1aa901a 100644 --- a/packages/backend-test-utils/src/next/services/mockServices.ts +++ b/packages/backend-test-utils/src/next/services/mockServices.ts @@ -275,6 +275,7 @@ export namespace mockServices { export const mock = simpleMock(coreServices.httpAuth, () => ({ credentials: jest.fn(), issueUserCookie: jest.fn(), + removeUserCookie: jest.fn(), })); } diff --git a/packages/core-app-api/src/apis/implementations/IdentityApi/AppIdentityProxy.ts b/packages/core-app-api/src/apis/implementations/IdentityApi/AppIdentityProxy.ts index 4a51444879..5263d6490f 100644 --- a/packages/core-app-api/src/apis/implementations/IdentityApi/AppIdentityProxy.ts +++ b/packages/core-app-api/src/apis/implementations/IdentityApi/AppIdentityProxy.ts @@ -50,6 +50,7 @@ export class AppIdentityProxy implements IdentityApi { private waitForTarget: Promise; private resolveTarget: (api: CompatibilityIdentityApi) => void = () => {}; private signOutTargetUrl = '/'; + #signOutCallback?: () => Promise; constructor() { this.waitForTarget = new Promise(resolve => { @@ -67,6 +68,10 @@ export class AppIdentityProxy implements IdentityApi { this.resolveTarget(identityApi); } + setSignOutCallback(signOutCallback: () => Promise): void { + this.#signOutCallback = signOutCallback; + } + getUserId(): string { if (!this.target) { throw mkError('getUserId'); @@ -124,6 +129,7 @@ export class AppIdentityProxy implements IdentityApi { async signOut(): Promise { await this.waitForTarget.then(target => target.signOut()); + await this.#signOutCallback?.(); window.location.href = this.signOutTargetUrl; } } diff --git a/packages/core-app-api/src/app/AppManager.test.tsx b/packages/core-app-api/src/app/AppManager.test.tsx index fe8d2fa91a..9f81d0b62e 100644 --- a/packages/core-app-api/src/app/AppManager.test.tsx +++ b/packages/core-app-api/src/app/AppManager.test.tsx @@ -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 ; + }; + + const Root = app.createRoot( + + + , + ); + await renderWithEffects(); + + await userEvent.click(screen.getByText('Sign Out')); + + await waitFor(() => + expect(fetchApiMock.fetch).toHaveBeenCalledWith( + 'http://localhost:7007/app/.backstage/v1-cookie', + { method: 'DELETE' }, + ), + ); + }); }); diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx index 3843fdfaee..cb2577dd4d 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -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 ( - + 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; 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; diff --git a/plugins/auth-node/package.json b/plugins/auth-node/package.json index 49d9dd87b9..77c8bcb9cb 100644 --- a/plugins/auth-node/package.json +++ b/plugins/auth-node/package.json @@ -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", diff --git a/plugins/auth-node/src/identity/createCookieAuthRefreshMiddleware.test.ts b/plugins/auth-node/src/identity/createCookieAuthRefreshMiddleware.test.ts new file mode 100644 index 0000000000..65426b4ca5 --- /dev/null +++ b/plugins/auth-node/src/identity/createCookieAuthRefreshMiddleware.test.ts @@ -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='); + }); +}); diff --git a/plugins/auth-node/src/identity/createCookieAuthRefreshMiddleware.ts b/plugins/auth-node/src/identity/createCookieAuthRefreshMiddleware.ts index 85ff5b86d7..efb58345a1 100644 --- a/plugins/auth-node/src/identity/createCookieAuthRefreshMiddleware.ts +++ b/plugins/auth-node/src/identity/createCookieAuthRefreshMiddleware.ts @@ -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; } diff --git a/yarn.lock b/yarn.lock index ffc26aff2b..2155432223 100644 --- a/yarn.lock +++ b/yarn.lock @@ -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