From d11767c861ecd6bd2a48d02f8b9ff957347c09c5 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 9 Apr 2024 13:31:50 +0200 Subject: [PATCH] core-app-api: replace cookie auth provider with new implementation in AppIdentityProxy Signed-off-by: Patrik Oldsberg --- packages/core-app-api/package.json | 1 - .../IdentityApi/AppIdentityProxy.ts | 41 ++++++-- .../src/app/AppAuthProvider.test.tsx | 99 ------------------- .../core-app-api/src/app/AppAuthProvider.tsx | 33 ------- packages/core-app-api/src/app/AppManager.tsx | 24 ++--- packages/core-app-api/src/app/AppRouter.tsx | 15 +-- yarn.lock | 1 - 7 files changed, 52 insertions(+), 162 deletions(-) delete mode 100644 packages/core-app-api/src/app/AppAuthProvider.test.tsx delete mode 100644 packages/core-app-api/src/app/AppAuthProvider.tsx diff --git a/packages/core-app-api/package.json b/packages/core-app-api/package.json index dcdb5a78f1..bb3566062c 100644 --- a/packages/core-app-api/package.json +++ b/packages/core-app-api/package.json @@ -44,7 +44,6 @@ "dependencies": { "@backstage/config": "workspace:^", "@backstage/core-plugin-api": "workspace:^", - "@backstage/plugin-auth-react": "workspace:^", "@backstage/types": "workspace:^", "@backstage/version-bridge": "workspace:^", "@types/prop-types": "^15.7.3", 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 5263d6490f..8db890ad47 100644 --- a/packages/core-app-api/src/apis/implementations/IdentityApi/AppIdentityProxy.ts +++ b/packages/core-app-api/src/apis/implementations/IdentityApi/AppIdentityProxy.ts @@ -18,7 +18,11 @@ import { IdentityApi, ProfileInfo, BackstageUserIdentity, + ErrorApi, + DiscoveryApi, + FetchApi, } from '@backstage/core-plugin-api'; +import { startCookieAuthRefresh } from './startCookieAuthRefresh'; function mkError(thing: string) { return new Error( @@ -50,7 +54,8 @@ export class AppIdentityProxy implements IdentityApi { private waitForTarget: Promise; private resolveTarget: (api: CompatibilityIdentityApi) => void = () => {}; private signOutTargetUrl = '/'; - #signOutCallback?: () => Promise; + + #cookieAuthSignOut?: () => void; constructor() { this.waitForTarget = new Promise(resolve => { @@ -68,10 +73,6 @@ export class AppIdentityProxy implements IdentityApi { this.resolveTarget(identityApi); } - setSignOutCallback(signOutCallback: () => Promise): void { - this.#signOutCallback = signOutCallback; - } - getUserId(): string { if (!this.target) { throw mkError('getUserId'); @@ -129,7 +130,35 @@ export class AppIdentityProxy implements IdentityApi { async signOut(): Promise { await this.waitForTarget.then(target => target.signOut()); - await this.#signOutCallback?.(); + + await this.#cookieAuthSignOut?.(); + window.location.href = this.signOutTargetUrl; } + + enableCookieAuth(ctx: { + errorApi: ErrorApi; + fetchApi: FetchApi; + discoveryApi: DiscoveryApi; + }) { + if (this.#cookieAuthSignOut) { + return; + } + + const stopRefresh = startCookieAuthRefresh(ctx); + + this.#cookieAuthSignOut = async () => { + stopRefresh(); + + // It is fine if we do NOT worry yet about deleting cookies for OTHER backends like techdocs + const appBaseUrl = await ctx.discoveryApi.getBaseUrl('app'); + try { + await ctx.fetchApi.fetch(`${appBaseUrl}/.backstage/auth/v1/cookie`, { + method: 'DELETE', + }); + } catch { + // Ignore the error for those who use static serving of the frontend + } + }; + } } diff --git a/packages/core-app-api/src/app/AppAuthProvider.test.tsx b/packages/core-app-api/src/app/AppAuthProvider.test.tsx deleted file mode 100644 index 2ee0c49f8d..0000000000 --- a/packages/core-app-api/src/app/AppAuthProvider.test.tsx +++ /dev/null @@ -1,99 +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 { render, screen, waitFor } from '@testing-library/react'; -import { discoveryApiRef, fetchApiRef } from '@backstage/core-plugin-api'; -import { AppAuthProvider } from './AppAuthProvider'; - -const now = 1710316886171; -const tenMinutesInMilliseconds = 10 * 60 * 1000; -const tenMinutesFromNowInMilliseconds = now + tenMinutesInMilliseconds; -const expiresAt = new Date(tenMinutesFromNowInMilliseconds).toISOString(); - -jest.mock('@backstage/core-plugin-api', () => { - return { - ...jest.requireActual('@backstage/core-plugin-api'), - useApp: jest.fn().mockReturnValue({ - getComponents: () => ({ Progress: () =>
}), - }), - useApi: jest.fn(ref => { - if (ref === discoveryApiRef) { - return { - getBaseUrl: jest.fn().mockResolvedValue('http://localhost:7000/app'), - }; - } - - if (ref === fetchApiRef) { - return { - fetch: jest.fn().mockResolvedValue({ - ok: true, - json: jest.fn().mockResolvedValue({ expiresAt }), - }), - }; - } - - // componentsApiRef - return { - getComponent: jest - .fn() - .mockReturnValue(() =>
), - }; - }), - }; -}); - -describe('AppAuthProvider', () => { - beforeEach(() => { - jest.clearAllMocks(); - jest.useFakeTimers({ now }); - }); - - afterEach(() => { - jest.useRealTimers(); - }); - - it('should render the children when app mode is undefined', async () => { - render(Test content); - await waitFor(() => - expect(screen.getByText('Test content')).toBeInTheDocument(), - ); - }); - - it('should render the children the app mode is public', async () => { - render( - <> - - Test content - , - ); - await waitFor(() => - expect(screen.getByText('Test content')).toBeInTheDocument(), - ); - }); - - it('should render the children wrapped in the CookieAuthRefreshProvider', async () => { - render( - <> - - Test content - , - ); - await waitFor(() => - expect(screen.getByText('Test content')).toBeInTheDocument(), - ); - }); -}); diff --git a/packages/core-app-api/src/app/AppAuthProvider.tsx b/packages/core-app-api/src/app/AppAuthProvider.tsx deleted file mode 100644 index 8834930ef6..0000000000 --- a/packages/core-app-api/src/app/AppAuthProvider.tsx +++ /dev/null @@ -1,33 +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, { ReactNode } from 'react'; -import { CookieAuthRefreshProvider } from '@backstage/plugin-auth-react'; -import { isProtectedApp } from './isProtectedApp'; - -export function AppAuthProvider(props: { children: ReactNode }): JSX.Element { - const { children } = props; - - if (isProtectedApp()) { - return ( - - {children} - - ); - } - - return <>{children}; -} diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx index bd51f0046b..64736470d2 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -44,6 +44,7 @@ import { FeatureFlag, fetchApiRef, discoveryApiRef, + errorApiRef, } from '@backstage/core-plugin-api'; import { AppLanguageApi, @@ -359,20 +360,21 @@ DEPRECATION WARNING: React Router Beta is deprecated and support for it will be const apis = this.getApiHolder(); - this.appIdentityProxy.setSignOutCallback(async () => { + if (isProtectedApp()) { + const errorApi = apis.get(errorApiRef); const fetchApi = apis.get(fetchApiRef); const discoveryApi = apis.get(discoveryApiRef); - if (!fetchApi || !discoveryApi || !isProtectedApp()) 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/auth/v1/cookie`, { - method: 'DELETE', - }); - } catch { - // Ignore the error for those who use static serving of the frontend + if (!errorApi || !fetchApi || !discoveryApi) { + throw new Error( + 'App is running in protected mode but missing required APIs', + ); } - }); + this.appIdentityProxy.enableCookieAuth({ + errorApi, + fetchApi, + discoveryApi, + }); + } return ( diff --git a/packages/core-app-api/src/app/AppRouter.tsx b/packages/core-app-api/src/app/AppRouter.tsx index 21704bc26b..7481f6662a 100644 --- a/packages/core-app-api/src/app/AppRouter.tsx +++ b/packages/core-app-api/src/app/AppRouter.tsx @@ -29,7 +29,6 @@ import { isReactRouterBeta } from './isReactRouterBeta'; import { RouteTracker } from '../routing/RouteTracker'; import { Route, Routes } from 'react-router-dom'; import { AppIdentityProxy } from '../apis/implementations/IdentityApi/AppIdentityProxy'; -import { AppAuthProvider } from './AppAuthProvider'; /** * Get the app base path from the configured app baseUrl. @@ -146,10 +145,7 @@ export function AppRouter(props: AppRouterProps) { - {props.children}} - /> + {props.children}} /> ); @@ -158,7 +154,7 @@ export function AppRouter(props: AppRouterProps) { return ( - {props.children} + {props.children} ); } @@ -172,10 +168,7 @@ export function AppRouter(props: AppRouterProps) { appIdentityProxy={appIdentityProxy} > - {props.children}} - /> + {props.children}} /> @@ -189,7 +182,7 @@ export function AppRouter(props: AppRouterProps) { component={SignInPageComponent} appIdentityProxy={appIdentityProxy} > - {props.children} + {props.children} ); diff --git a/yarn.lock b/yarn.lock index 0b90ddd2e9..6ce1ec54f3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3834,7 +3834,6 @@ __metadata: "@backstage/cli": "workspace:^" "@backstage/config": "workspace:^" "@backstage/core-plugin-api": "workspace:^" - "@backstage/plugin-auth-react": "workspace:^" "@backstage/test-utils": "workspace:^" "@backstage/types": "workspace:^" "@backstage/version-bridge": "workspace:^"