refactor: more review refinements

Signed-off-by: Camila Belo <camilaibs@gmail.com>
This commit is contained in:
Camila Belo
2024-04-05 09:23:49 +02:00
committed by Patrik Oldsberg
parent ffd71105a6
commit b01e709ead
26 changed files with 73 additions and 78 deletions
+2 -2
View File
@@ -45,7 +45,7 @@ With that, Backstage's cli and backend will detect public entry point and serve
createApiFactory,
discoveryApiRef,
} from '@backstage/core-plugin-api';
import { CookieAuthRootRedirect } from '@backstage/plugin-auth-react';
import { CookieAuthRedirect } from '@backstage/plugin-auth-react';
import { providers } from '../src/identityProviders';
import { AuthProxyDiscoveryApi } from '../src/AuthProxyDiscoveryApi';
@@ -79,7 +79,7 @@ With that, Backstage's cli and backend will detect public entry point and serve
<OAuthRequestDialog />
<AppRouter>
{/* This is a special component that does the magic to redirect users to access the home page of your authenticated application version */}
<CookieAuthRootRedirect />
<CookieAuthRedirect />
</AppRouter>
</>,
);
@@ -21,7 +21,7 @@ import {
OAuthRequestDialog,
SignInPage,
} from '@backstage/core-components';
import { CookieAuthRootRedirect } from '@backstage/plugin-auth-react';
import { CookieAuthRedirect } from '@backstage/plugin-auth-react';
import React from 'react';
import ReactDOM from 'react-dom/client';
import { providers } from '../src/identityProviders';
@@ -59,7 +59,7 @@ const App = app.createRoot(
<AlertDisplay transientTimeoutMs={2500} />
<OAuthRequestDialog />
<AppRouter>
<CookieAuthRootRedirect />
<CookieAuthRedirect />
</AppRouter>
</>,
);
@@ -21,10 +21,10 @@ import {
createServiceFactory,
HttpRouterServiceAuthPolicy,
} from '@backstage/backend-plugin-api';
import { createCookieAuthRefreshMiddleware } from '@backstage/plugin-auth-node';
import { createLifecycleMiddleware } from './createLifecycleMiddleware';
import { createCredentialsBarrier } from './createCredentialsBarrier';
import { createAuthIntegrationRouter } from './createAuthIntegrationRouter';
import { createCookieAuthRefreshMiddleware } from './createCookieAuthRefreshMiddleware';
/**
* @public
@@ -59,8 +59,6 @@ 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.`,
@@ -80,6 +78,7 @@ export const httpRouterServiceFactory = createServiceFactory(
router.use(createLifecycleMiddleware({ lifecycle }));
router.use(createAuthIntegrationRouter({ auth }));
router.use(credentialsBarrier.middleware);
router.use(createCookieAuthRefreshMiddleware({ auth, httpAuth }));
return {
use(handler: Handler): void {
@@ -87,14 +86,6 @@ export const httpRouterServiceFactory = createServiceFactory(
},
addAuthPolicy(policy: HttpRouterServiceAuthPolicy): void {
credentialsBarrier.addAuthPolicy(policy);
if (
policy.allow === 'user-cookie' &&
!hasRegistedCookieAuthRefreshMiddleware
) {
// Only add the cookie refresh middleware once
hasRegistedCookieAuthRefreshMiddleware = true;
router.use(createCookieAuthRefreshMiddleware({ auth, httpAuth }));
}
},
};
},
@@ -824,10 +824,23 @@ describe('Integration Test', () => {
});
it('should clear app cookie when the user logs out', async () => {
const fetchApiMock = { fetch: jest.fn().mockResolvedValue({ ok: true }) };
const meta = global.document.createElement('meta');
meta.name = 'backstage-app-mode';
meta.content = 'protected';
global.document.head.appendChild(meta);
const fetchApiMock = {
fetch: jest.fn().mockResolvedValue({
ok: true,
json: jest.fn().mockResolvedValue({
expiresAt: new Date(Date.now() + 10 * 60 * 1000).toISOString(),
}),
}),
};
const discoveryApiMock = {
getBaseUrl: jest.fn().mockResolvedValue('http://localhost:7007/app'),
};
const app = new AppManager({
icons,
themes,
@@ -855,6 +868,7 @@ describe('Integration Test', () => {
const Root = app.createRoot(
<AppRouter>
<meta name="backstage-app-mode" content="protected" />
<SignOutButton />
</AppRouter>,
);
@@ -868,5 +882,7 @@ describe('Integration Test', () => {
{ method: 'DELETE' },
),
);
global.document.head.removeChild(meta);
});
});
+3 -2
View File
@@ -88,6 +88,7 @@ import { AppRouter, getBasePath } from './AppRouter';
import { AppLanguageSelector } from '../apis/implementations/AppLanguageApi';
import { I18nextTranslationApi } from '../apis/implementations/TranslationApi';
import { overrideBaseUrlConfigs } from './overrideBaseUrlConfigs';
import { isProtectedApp } from '@backstage/plugin-auth-react';
type CompatiblePlugin =
| BackstagePlugin
@@ -361,11 +362,11 @@ DEPRECATION WARNING: React Router Beta is deprecated and support for it will be
this.appIdentityProxy.setSignOutCallback(async () => {
const fetchApi = apis.get(fetchApiRef);
const discoveryApi = apis.get(discoveryApiRef);
if (!fetchApi || !discoveryApi) return;
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/v1-cookie`, {
await fetchApi.fetch(`${appBaseUrl}/.backstage/auth/v1/cookie`, {
method: 'DELETE',
});
} catch {
+5 -5
View File
@@ -29,7 +29,7 @@ import { isReactRouterBeta } from './isReactRouterBeta';
import { RouteTracker } from '../routing/RouteTracker';
import { Route, Routes } from 'react-router-dom';
import { AppIdentityProxy } from '../apis/implementations/IdentityApi/AppIdentityProxy';
import { AppMode } from '@backstage/plugin-auth-react';
import { AppAuthProvider } from '@backstage/plugin-auth-react';
/**
* Get the app base path from the configured app baseUrl.
@@ -148,7 +148,7 @@ export function AppRouter(props: AppRouterProps) {
<Routes>
<Route
path={mountPath}
element={<AppMode>{props.children}</AppMode>}
element={<AppAuthProvider>{props.children}</AppAuthProvider>}
/>
</Routes>
</RouterComponent>
@@ -158,7 +158,7 @@ export function AppRouter(props: AppRouterProps) {
return (
<RouterComponent basename={basePath}>
<RouteTracker routeObjects={routeObjects} />
<AppMode>{props.children}</AppMode>
<AppAuthProvider>{props.children}</AppAuthProvider>
</RouterComponent>
);
}
@@ -174,7 +174,7 @@ export function AppRouter(props: AppRouterProps) {
<Routes>
<Route
path={mountPath}
element={<AppMode>{props.children}</AppMode>}
element={<AppAuthProvider>{props.children}</AppAuthProvider>}
/>
</Routes>
</SignInPageWrapper>
@@ -189,7 +189,7 @@ export function AppRouter(props: AppRouterProps) {
component={SignInPageComponent}
appIdentityProxy={appIdentityProxy}
>
<AppMode>{props.children}</AppMode>
<AppAuthProvider>{props.children}</AppAuthProvider>
</SignInPageWrapper>
</RouterComponent>
);
@@ -42,7 +42,7 @@ import { AppIdentityProxy } from '../../../core-app-api/src/apis/implementations
import { BrowserRouter } from 'react-router-dom';
import { RouteTracker } from '../routing/RouteTracker';
import { getBasePath } from '../routing/getBasePath';
import { AppMode } from '@backstage/plugin-auth-react';
import { AppAuthProvider } from '@backstage/plugin-auth-react';
export const AppRoot = createExtension({
namespace: 'app',
@@ -191,7 +191,7 @@ export function AppRouter(props: AppRouterProps) {
return (
<RouterComponent>
<RouteTracker routeObjects={routeObjects} />
<AppMode>{children}</AppMode>
<AppAuthProvider>{children}</AppAuthProvider>
</RouterComponent>
);
}
@@ -203,7 +203,7 @@ export function AppRouter(props: AppRouterProps) {
component={SignInPageComponent}
appIdentityProxy={appIdentityProxy}
>
<AppMode>{children}</AppMode>
<AppAuthProvider>{children}</AppAuthProvider>
</SignInPageWrapper>
</RouterComponent>
);
@@ -62,9 +62,9 @@ describe('appPlugin', () => {
fetch(`http://localhost:${server.port()}/api/app/derp.html`).then(res =>
res.text(),
),
).resolves.toMatch('winning');
).resolves.toBe('winning');
await expect(
fetch(`http://localhost:${server.port()}`).then(res => res.text()),
).resolves.toMatch('winning');
).resolves.toBe('winning');
});
});
@@ -40,7 +40,6 @@ import {
import { ConfigSchema } from '@backstage/config-loader';
import { AuthService, HttpAuthService } from '@backstage/backend-plugin-api';
import { AuthenticationError } from '@backstage/errors';
import { createCookieAuthRefreshMiddleware } from '@backstage/plugin-auth-node';
// express uses mime v1 while we only have types for mime v2
type Mime = { lookup(arg0: string): string };
@@ -174,8 +173,6 @@ export async function createRouter(
if (enablePublicEntryPoint && auth && httpAuth) {
const publicRouter = Router();
publicRouter.use(createCookieAuthRefreshMiddleware({ auth, httpAuth }));
publicRouter.use(async (req, _res, next) => {
const credentials = await httpAuth.credentials(req, {
allow: ['user', 'service', 'none'],
-9
View File
@@ -3,7 +3,6 @@
> 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 { BackstageIdentityResponse as BackstageIdentityResponse_2 } from '@backstage/plugin-auth-node';
import { BackstageSignInResult as BackstageSignInResult_2 } from '@backstage/plugin-auth-node';
import { Config } from '@backstage/config';
@@ -11,7 +10,6 @@ import { Entity } from '@backstage/catalog-model';
import { EntityFilterQuery } from '@backstage/catalog-client';
import express from 'express';
import { ExtensionPoint } from '@backstage/backend-plugin-api';
import { HttpAuthService } from '@backstage/backend-plugin-api';
import { JsonObject } from '@backstage/types';
import { JsonValue } from '@backstage/types';
import { LoggerService } from '@backstage/backend-plugin-api';
@@ -19,7 +17,6 @@ import { PluginEndpointDiscovery } from '@backstage/backend-common';
import { Profile } from 'passport';
import { Request as Request_2 } from 'express';
import { Response as Response_2 } from 'express';
import { Router } from 'express-serve-static-core';
import { Strategy } from 'passport';
import { ZodSchema } from 'zod';
import { ZodTypeDef } from 'zod';
@@ -151,12 +148,6 @@ export type CookieConfigurer = (ctx: {
sameSite?: 'none' | 'lax' | 'strict';
};
// @public
export function createCookieAuthRefreshMiddleware(options: {
auth: AuthService;
httpAuth: HttpAuthService;
}): Router;
// @public (undocumented)
export function createOAuthAuthenticator<TContext, TProfile>(
authenticator: OAuthAuthenticator<TContext, TProfile>,
-1
View File
@@ -39,7 +39,6 @@
"@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",
-1
View File
@@ -22,4 +22,3 @@ export {
} from './DefaultIdentityClient';
export { IdentityClient } from './IdentityClient';
export type { IdentityApi, IdentityApiGetIdentityRequest } from './IdentityApi';
export { createCookieAuthRefreshMiddleware } from './createCookieAuthRefreshMiddleware';
+5 -2
View File
@@ -7,7 +7,10 @@ import { default as React_2 } from 'react';
import { ReactNode } from 'react';
// @public
export function AppMode(props: { children: ReactNode }): JSX.Element;
export function AppAuthProvider(props: { children: ReactNode }): JSX.Element;
// @public
export function CookieAuthRedirect(): React_2.JSX.Element | null;
// @public
export function CookieAuthRefreshProvider(
@@ -22,7 +25,7 @@ export type CookieAuthRefreshProviderProps = {
};
// @public
export function CookieAuthRootRedirect(): React_2.JSX.Element | null;
export function isProtectedApp(): boolean;
// @public
export function useCookieAuthRefresh(options: {
@@ -16,9 +16,9 @@
import React from 'react';
import { render, screen, waitFor } from '@testing-library/react';
import { AppMode } from './AppMode';
import { discoveryApiRef, fetchApiRef } from '@backstage/core-plugin-api';
import { componentsApiRef } from '@backstage/frontend-plugin-api';
import { discoveryApiRef, fetchApiRef } from '@backstage/core-plugin-api';
import { AppAuthProvider } from './AppAuthProvider';
const now = 1710316886171;
const tenMinutesInMilliseconds = 10 * 60 * 1000;
@@ -60,7 +60,7 @@ jest.mock('@backstage/core-plugin-api', () => {
};
});
describe('AppMode', () => {
describe('AppAuthProvider', () => {
beforeEach(() => {
jest.clearAllMocks();
jest.useFakeTimers({ now });
@@ -71,7 +71,7 @@ describe('AppMode', () => {
});
it('should render the children when app mode is undefined', async () => {
render(<AppMode>Test content</AppMode>);
render(<AppAuthProvider>Test content</AppAuthProvider>);
await waitFor(() =>
expect(screen.getByText('Test content')).toBeInTheDocument(),
);
@@ -81,7 +81,7 @@ describe('AppMode', () => {
render(
<>
<meta name="backstage-app-mode" content="public" />
<AppMode>Test content</AppMode>
<AppAuthProvider>Test content</AppAuthProvider>
</>,
);
await waitFor(() =>
@@ -93,7 +93,7 @@ describe('AppMode', () => {
render(
<>
<meta name="backstage-app-mode" content="protected" />
<AppMode>Test content</AppMode>
<AppAuthProvider>Test content</AppAuthProvider>
</>,
);
await waitFor(() =>
@@ -14,29 +14,18 @@
* limitations under the License.
*/
import React, { ReactNode, useEffect, useState } from 'react';
import React, { ReactNode } from 'react';
import { CookieAuthRefreshProvider } from '@backstage/plugin-auth-react';
import { CompatAppProgress } from '../CompatAppProgress';
import { isProtectedApp } from './isProtectedApp';
/**
* @public
* A provider that will protect the app when running in protected experimental mode.
*/
export function AppMode(props: { children: ReactNode }): JSX.Element {
export function AppAuthProvider(props: { children: ReactNode }): JSX.Element {
const { children } = props;
const [appMode, setAppMode] = useState<string | null>(null);
useEffect(() => {
const element = document.querySelector('meta[name="backstage-app-mode"]');
setAppMode(element?.getAttribute('content') ?? 'public');
}, [setAppMode]);
if (!appMode) {
return <CompatAppProgress />;
}
if (appMode === 'protected') {
if (isProtectedApp()) {
return (
<CookieAuthRefreshProvider pluginId="app">
{children}
@@ -14,4 +14,5 @@
* limitations under the License.
*/
export { CookieAuthRootRedirect } from './CookieAuthRootRedirect';
export { isProtectedApp } from './isProtectedApp';
export { AppAuthProvider } from './AppAuthProvider';
@@ -14,4 +14,12 @@
* limitations under the License.
*/
export { CompatAppProgress } from './CompatAppProgress';
/**
* @public
* An utility function that detects whether the app is operating in protected mode.
*/
export function isProtectedApp() {
const element = document.querySelector('meta[name="backstage-app-mode"]');
const appMode = element?.getAttribute('content') ?? 'public';
return appMode === 'protected';
}
@@ -18,9 +18,9 @@ import React from 'react';
import { screen, waitFor } from '@testing-library/react';
import { TestApiProvider, renderInTestApp } from '@backstage/test-utils';
import { identityApiRef } from '@backstage/core-plugin-api';
import { CookieAuthRootRedirect } from './CookieAuthRootRedirect';
import { CookieAuthRedirect } from './CookieAuthRedirect';
describe('CookieAuthRootRedirect', () => {
describe('CookieAuthRedirect', () => {
const identityApiMock = { getCredentials: jest.fn() };
beforeEach(() => {
@@ -32,7 +32,7 @@ describe('CookieAuthRootRedirect', () => {
await renderInTestApp(
<TestApiProvider apis={[[identityApiRef, identityApiMock]]}>
<CookieAuthRootRedirect />
<CookieAuthRedirect />
</TestApiProvider>,
);
@@ -50,7 +50,7 @@ describe('CookieAuthRootRedirect', () => {
await renderInTestApp(
<TestApiProvider apis={[[identityApiRef, identityApiMock]]}>
<CookieAuthRootRedirect />
<CookieAuthRedirect />
</TestApiProvider>,
);
@@ -20,9 +20,9 @@ import { useAsync, useMountEffect } from '@react-hookz/web';
/**
* @public
* A component that redirects to the root of the app after a successful sign-in.
* A component that redirects to the page an user was trying to access before sign-in.
*/
export function CookieAuthRootRedirect() {
export function CookieAuthRedirect() {
const identityApi = useApi(identityApiRef);
const [state, actions] = useAsync(async () => {
@@ -14,4 +14,4 @@
* limitations under the License.
*/
export { AppMode } from './AppMode';
export { CookieAuthRedirect } from './CookieAuthRedirect';
@@ -18,7 +18,7 @@ import React, { ReactNode } from 'react';
import { ErrorPanel } from '@backstage/core-components';
import { Button } from '@material-ui/core';
import { useCookieAuthRefresh } from '../../hooks';
import { CompatAppProgress } from '../CompatAppProgress/CompatAppProgress';
import { CompatAppProgress } from './CompatAppProgress';
/**
* @public
@@ -45,6 +45,7 @@ export function CookieAuthRefreshProvider(
const result = useCookieAuthRefresh(options);
if (result.status === 'loading') {
// This component should be compatible with both old and new frontend systems
return <CompatAppProgress />;
}
+2 -2
View File
@@ -17,6 +17,6 @@
// The index file in ./components/ is typically responsible for selecting
// which components are public API and should be exported from the package.
export * from './CookieAuthRootRedirect';
export * from './AppAuthProvider';
export * from './CookieAuthRedirect';
export * from './CookieAuthRefreshProvider';
export * from './AppMode';
-1
View File
@@ -5088,7 +5088,6 @@ __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