From 8ca1d848213b3be8fc876d87369115b96f9f8b2f Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Thu, 18 Apr 2024 15:28:57 +0000 Subject: [PATCH 1/7] fix: update UI code to forward bearer tokens properly --- plugins/backstage-plugin-coder/src/api.ts | 40 +++++++++++++++---- .../CoderProvider/CoderAuthProvider.tsx | 4 +- .../src/hooks/useCoderWorkspacesQuery.ts | 12 +++++- 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/api.ts b/plugins/backstage-plugin-coder/src/api.ts index 02dadbe4..fedf5107 100644 --- a/plugins/backstage-plugin-coder/src/api.ts +++ b/plugins/backstage-plugin-coder/src/api.ts @@ -9,6 +9,7 @@ import { WorkspaceAgentStatus, } from './typesConstants'; import { CoderAuth, assertValidCoderAuth } from './components/CoderProvider'; +import { IdentityApi } from '@backstage/core-plugin-api'; export const CODER_QUERY_KEY_PREFIX = 'coder-backstage-plugin'; @@ -19,9 +20,27 @@ export const ASSETS_ROUTE_PREFIX = PROXY_ROUTE_PREFIX; export const CODER_AUTH_HEADER_KEY = 'Coder-Session-Token'; export const REQUEST_TIMEOUT_MS = 20_000; -function getCoderApiRequestInit(authToken: string): RequestInit { +async function getBearerToken( + identity: IdentityApi, +): Promise { + const credentials = await identity.getCredentials(); + return credentials.token; +} + +function getCoderApiRequestInit( + authToken: string, + bearerToken: string | undefined, +): RequestInit { + const headers: HeadersInit = { + [CODER_AUTH_HEADER_KEY]: authToken, + }; + + if (bearerToken) { + headers.Authorization = `bearer ${bearerToken}`; + } + return { - headers: { [CODER_AUTH_HEADER_KEY]: authToken }, + headers, signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), }; } @@ -53,6 +72,7 @@ export class BackstageHttpError extends Error { type FetchInputs = Readonly<{ auth: CoderAuth; baseUrl: string; + identity: IdentityApi; }>; type WorkspacesFetchInputs = Readonly< @@ -64,7 +84,7 @@ type WorkspacesFetchInputs = Readonly< async function getWorkspaces( fetchInputs: WorkspacesFetchInputs, ): Promise { - const { baseUrl, coderQuery, auth } = fetchInputs; + const { baseUrl, coderQuery, auth, identity } = fetchInputs; assertValidCoderAuth(auth); const urlParams = new URLSearchParams({ @@ -72,9 +92,10 @@ async function getWorkspaces( limit: '0', }); + const bearerToken = await getBearerToken(identity); const response = await fetch( `${baseUrl}${API_ROUTE_PREFIX}/workspaces?${urlParams.toString()}`, - getCoderApiRequestInit(auth.token), + getCoderApiRequestInit(auth.token, bearerToken), ); if (!response.ok) { @@ -116,12 +137,13 @@ type BuildParamsFetchInputs = Readonly< >; async function getWorkspaceBuildParameters(inputs: BuildParamsFetchInputs) { - const { baseUrl, auth, workspaceBuildId } = inputs; + const { baseUrl, auth, workspaceBuildId, identity } = inputs; assertValidCoderAuth(auth); + const bearerToken = await getBearerToken(identity); const res = await fetch( `${baseUrl}${API_ROUTE_PREFIX}/workspacebuilds/${workspaceBuildId}/parameters`, - getCoderApiRequestInit(auth.token), + getCoderApiRequestInit(auth.token, bearerToken), ); if (!res.ok) { @@ -256,16 +278,18 @@ export function workspacesByRepo( type AuthValidationInputs = Readonly<{ baseUrl: string; authToken: string; + identity: IdentityApi; }>; async function isAuthValid(inputs: AuthValidationInputs): Promise { - const { baseUrl, authToken } = inputs; + const { baseUrl, authToken, identity } = inputs; + const bearerToken = await getBearerToken(identity); // In this case, the request doesn't actually matter. Just need to make any // kind of dummy request to validate the auth const response = await fetch( `${baseUrl}${API_ROUTE_PREFIX}/users/me`, - getCoderApiRequestInit(authToken), + getCoderApiRequestInit(authToken, bearerToken), ); if (response.status >= 400 && response.status !== 401) { diff --git a/plugins/backstage-plugin-coder/src/components/CoderProvider/CoderAuthProvider.tsx b/plugins/backstage-plugin-coder/src/components/CoderProvider/CoderAuthProvider.tsx index 3192198e..8dd9a741 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderProvider/CoderAuthProvider.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderProvider/CoderAuthProvider.tsx @@ -19,6 +19,7 @@ import { authValidation, } from '../../api'; import { useBackstageEndpoints } from '../../hooks/useBackstageEndpoints'; +import { identityApiRef, useApi } from '@backstage/core-plugin-api'; const TOKEN_STORAGE_KEY = 'coder-backstage-plugin/token'; @@ -98,6 +99,7 @@ export function useCoderAuth(): CoderAuth { type CoderAuthProviderProps = Readonly>; export const CoderAuthProvider = ({ children }: CoderAuthProviderProps) => { + const identity = useApi(identityApiRef); const { baseUrl } = useBackstageEndpoints(); const [isInsideGracePeriod, setIsInsideGracePeriod] = useState(true); @@ -108,7 +110,7 @@ export const CoderAuthProvider = ({ children }: CoderAuthProviderProps) => { const [readonlyInitialAuthToken] = useState(authToken); const authValidityQuery = useQuery({ - ...authValidation({ baseUrl, authToken }), + ...authValidation({ baseUrl, authToken, identity }), refetchOnWindowFocus: query => query.state.data !== false, }); diff --git a/plugins/backstage-plugin-coder/src/hooks/useCoderWorkspacesQuery.ts b/plugins/backstage-plugin-coder/src/hooks/useCoderWorkspacesQuery.ts index 22556fda..3517ad2b 100644 --- a/plugins/backstage-plugin-coder/src/hooks/useCoderWorkspacesQuery.ts +++ b/plugins/backstage-plugin-coder/src/hooks/useCoderWorkspacesQuery.ts @@ -4,6 +4,7 @@ import { workspaces, workspacesByRepo } from '../api'; import { useCoderAuth } from '../components/CoderProvider/CoderAuthProvider'; import { useBackstageEndpoints } from './useBackstageEndpoints'; import { CoderWorkspacesConfig } from './useCoderWorkspacesConfig'; +import { identityApiRef, useApi } from '@backstage/core-plugin-api'; type QueryInput = Readonly<{ coderQuery: string; @@ -15,12 +16,19 @@ export function useCoderWorkspacesQuery({ workspacesConfig, }: QueryInput) { const auth = useCoderAuth(); + const identity = useApi(identityApiRef); const { baseUrl } = useBackstageEndpoints(); const hasRepoData = workspacesConfig && workspacesConfig.repoUrl; const queryOptions = hasRepoData - ? workspacesByRepo({ coderQuery, auth, baseUrl, workspacesConfig }) - : workspaces({ coderQuery, auth, baseUrl }); + ? workspacesByRepo({ + coderQuery, + identity, + auth, + baseUrl, + workspacesConfig, + }) + : workspaces({ coderQuery, identity, auth, baseUrl }); return useQuery(queryOptions); } From 701189ff96ec342cf633c380a7d83b415bc394dc Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Thu, 18 Apr 2024 16:01:17 +0000 Subject: [PATCH 2/7] refactor: consolidate init setup logic --- plugins/backstage-plugin-coder/src/api.ts | 45 ++++++++++++++--------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/api.ts b/plugins/backstage-plugin-coder/src/api.ts index fedf5107..dc524b90 100644 --- a/plugins/backstage-plugin-coder/src/api.ts +++ b/plugins/backstage-plugin-coder/src/api.ts @@ -20,23 +20,34 @@ export const ASSETS_ROUTE_PREFIX = PROXY_ROUTE_PREFIX; export const CODER_AUTH_HEADER_KEY = 'Coder-Session-Token'; export const REQUEST_TIMEOUT_MS = 20_000; -async function getBearerToken( - identity: IdentityApi, -): Promise { - const credentials = await identity.getCredentials(); - return credentials.token; -} +// No idea why Backstage doesn't have a formal type for this built in +type UserCredentials = Readonly<{ + token?: string; +}>; -function getCoderApiRequestInit( +async function getCoderApiRequestInit( authToken: string, - bearerToken: string | undefined, -): RequestInit { + identity: IdentityApi, +): Promise { const headers: HeadersInit = { [CODER_AUTH_HEADER_KEY]: authToken, }; - if (bearerToken) { - headers.Authorization = `bearer ${bearerToken}`; + let credentials: UserCredentials; + try { + credentials = await identity.getCredentials(); + } catch (err) { + if (err instanceof Error) { + throw err; + } + + throw new Error( + 'Unable to parse user information from Backstage APIs. Please ensure that your Backstage deployment is integrated to use the built-in Identity API', + ); + } + + if (credentials.token) { + headers.Authorization = `bearer ${credentials.token}`; } return { @@ -92,10 +103,10 @@ async function getWorkspaces( limit: '0', }); - const bearerToken = await getBearerToken(identity); + const requestInit = await getCoderApiRequestInit(auth.token, identity); const response = await fetch( `${baseUrl}${API_ROUTE_PREFIX}/workspaces?${urlParams.toString()}`, - getCoderApiRequestInit(auth.token, bearerToken), + requestInit, ); if (!response.ok) { @@ -140,10 +151,10 @@ async function getWorkspaceBuildParameters(inputs: BuildParamsFetchInputs) { const { baseUrl, auth, workspaceBuildId, identity } = inputs; assertValidCoderAuth(auth); - const bearerToken = await getBearerToken(identity); + const requestInit = await getCoderApiRequestInit(auth.token, identity); const res = await fetch( `${baseUrl}${API_ROUTE_PREFIX}/workspacebuilds/${workspaceBuildId}/parameters`, - getCoderApiRequestInit(auth.token, bearerToken), + requestInit, ); if (!res.ok) { @@ -283,13 +294,13 @@ type AuthValidationInputs = Readonly<{ async function isAuthValid(inputs: AuthValidationInputs): Promise { const { baseUrl, authToken, identity } = inputs; - const bearerToken = await getBearerToken(identity); // In this case, the request doesn't actually matter. Just need to make any // kind of dummy request to validate the auth + const requestInit = await getCoderApiRequestInit(authToken, identity); const response = await fetch( `${baseUrl}${API_ROUTE_PREFIX}/users/me`, - getCoderApiRequestInit(authToken, bearerToken), + requestInit, ); if (response.status >= 400 && response.status !== 401) { From a59c04bef38e2ce87304296a21db9243352ef033 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Thu, 18 Apr 2024 16:01:42 +0000 Subject: [PATCH 3/7] fix: update error catching logic --- plugins/backstage-plugin-coder/src/api.ts | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/api.ts b/plugins/backstage-plugin-coder/src/api.ts index dc524b90..d11248eb 100644 --- a/plugins/backstage-plugin-coder/src/api.ts +++ b/plugins/backstage-plugin-coder/src/api.ts @@ -20,11 +20,6 @@ export const ASSETS_ROUTE_PREFIX = PROXY_ROUTE_PREFIX; export const CODER_AUTH_HEADER_KEY = 'Coder-Session-Token'; export const REQUEST_TIMEOUT_MS = 20_000; -// No idea why Backstage doesn't have a formal type for this built in -type UserCredentials = Readonly<{ - token?: string; -}>; - async function getCoderApiRequestInit( authToken: string, identity: IdentityApi, @@ -33,23 +28,21 @@ async function getCoderApiRequestInit( [CODER_AUTH_HEADER_KEY]: authToken, }; - let credentials: UserCredentials; try { - credentials = await identity.getCredentials(); + const credentials = await identity.getCredentials(); + if (credentials.token) { + headers.Authorization = `Bearer ${credentials.token}`; + } } catch (err) { if (err instanceof Error) { throw err; } throw new Error( - 'Unable to parse user information from Backstage APIs. Please ensure that your Backstage deployment is integrated to use the built-in Identity API', + "Unable to parse user information for Coder requests. Please ensure that your Backstage deployment is integrated to use Backstage's Identity API", ); } - if (credentials.token) { - headers.Authorization = `bearer ${credentials.token}`; - } - return { headers, signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), From 4067a50e1b0c8c890b171b272eca987410ad8938 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Thu, 18 Apr 2024 16:55:20 +0000 Subject: [PATCH 4/7] fix: add new mock to get current tests passing --- .../CoderProvider/CoderProvider.test.tsx | 8 +++++- .../src/testHelpers/mockBackstageData.ts | 28 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderProvider/CoderProvider.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderProvider/CoderProvider.test.tsx index 2a240a75..41e75bee 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderProvider/CoderProvider.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderProvider/CoderProvider.test.tsx @@ -3,7 +3,11 @@ import { renderHook } from '@testing-library/react'; import { act, waitFor } from '@testing-library/react'; import { TestApiProvider, wrapInTestApp } from '@backstage/test-utils'; -import { configApiRef, errorApiRef } from '@backstage/core-plugin-api'; +import { + configApiRef, + errorApiRef, + identityApiRef, +} from '@backstage/core-plugin-api'; import { CoderProvider } from './CoderProvider'; import { useCoderAppConfig } from './CoderAppConfigProvider'; @@ -12,6 +16,7 @@ import { type CoderAuth, useCoderAuth } from './CoderAuthProvider'; import { getMockConfigApi, getMockErrorApi, + getMockIdentityApi, mockAppConfig, mockCoderAuthToken, } from '../../testHelpers/mockBackstageData'; @@ -87,6 +92,7 @@ describe(`${CoderProvider.name}`, () => { diff --git a/plugins/backstage-plugin-coder/src/testHelpers/mockBackstageData.ts b/plugins/backstage-plugin-coder/src/testHelpers/mockBackstageData.ts index 049050cc..0febc8c0 100644 --- a/plugins/backstage-plugin-coder/src/testHelpers/mockBackstageData.ts +++ b/plugins/backstage-plugin-coder/src/testHelpers/mockBackstageData.ts @@ -17,6 +17,7 @@ import { import { ScmIntegrationsApi } from '@backstage/integration-react'; import { API_ROUTE_PREFIX, ASSETS_ROUTE_PREFIX } from '../api'; +import { IdentityApi } from '@backstage/core-plugin-api'; /** * This is the key that Backstage checks from the entity data to determine the @@ -207,6 +208,33 @@ export function getMockErrorApi() { return errorApi; } +export function getMockIdentityApi(): IdentityApi { + return { + signOut: async () => { + return void 'Not going to implement this'; + }, + getProfileInfo: async () => { + return { + displayName: 'Dobah', + email: 'i-love-my-dog-dobah@dog.ceo', + picture: undefined, + }; + }, + getBackstageIdentity: async () => { + return { + type: 'user', + userEntityRef: 'User:default/Dobah', + ownershipEntityRefs: [], + }; + }, + getCredentials: async () => { + return { + token: '', + }; + }, + }; +} + /** * Exposes a mock ScmIntegrationRegistry to be used with scmIntegrationsApiRef * for mocking out code that relies on source code data. From 24c27b4a7e3752bd652f6edc9e1b4b14668ab1c2 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Thu, 18 Apr 2024 16:59:49 +0000 Subject: [PATCH 5/7] fix: add mock bearer token --- .../src/testHelpers/mockBackstageData.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/backstage-plugin-coder/src/testHelpers/mockBackstageData.ts b/plugins/backstage-plugin-coder/src/testHelpers/mockBackstageData.ts index 0febc8c0..2e0fa6fe 100644 --- a/plugins/backstage-plugin-coder/src/testHelpers/mockBackstageData.ts +++ b/plugins/backstage-plugin-coder/src/testHelpers/mockBackstageData.ts @@ -58,6 +58,7 @@ export const mockBackstageProxyEndpoint = `${mockBackstageUrlRoot}${API_ROUTE_PR export const mockBackstageAssetsEndpoint = `${mockBackstageUrlRoot}${ASSETS_ROUTE_PREFIX}`; +export const mockBearerToken = 'This-is-an-opaque-value-by-design'; export const mockCoderAuthToken = 'ZG0HRy2gGN-mXljc1s5FqtE8WUJ4sUc5X'; export const mockYamlConfig = { @@ -229,7 +230,7 @@ export function getMockIdentityApi(): IdentityApi { }, getCredentials: async () => { return { - token: '', + token: mockBearerToken, }; }, }; From f0018ec792d90e22dc8d035624fee53f165ea001 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Thu, 18 Apr 2024 18:36:42 +0000 Subject: [PATCH 6/7] chore: add test middleware to verify bearer token behavior --- .../src/testHelpers/server.ts | 61 +++++++++++++++++-- .../src/testHelpers/setup.tsx | 11 +++- 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/testHelpers/server.ts b/plugins/backstage-plugin-coder/src/testHelpers/server.ts index 5602241d..04fbc6ce 100644 --- a/plugins/backstage-plugin-coder/src/testHelpers/server.ts +++ b/plugins/backstage-plugin-coder/src/testHelpers/server.ts @@ -1,5 +1,12 @@ /* eslint-disable @backstage/no-undeclared-imports -- For test helpers only */ -import { RestHandler, rest } from 'msw'; +import { + type DefaultBodyType, + type ResponseResolver, + type RestContext, + RestHandler, + rest, + RestRequest, +} from 'msw'; import { setupServer } from 'msw/node'; /* eslint-enable @backstage/no-undeclared-imports */ @@ -8,14 +15,60 @@ import { mockWorkspaceBuildParameters, } from './mockCoderAppData'; import { + mockBearerToken, mockCoderAuthToken, mockBackstageProxyEndpoint as root, } from './mockBackstageData'; import type { Workspace, WorkspacesResponse } from '../typesConstants'; import { CODER_AUTH_HEADER_KEY } from '../api'; +type RestResolver = ResponseResolver< + RestRequest, + RestContext, + TBody +>; + +export type RestResolverMiddleware = ( + resolver: RestResolver, +) => RestResolver; + +const defaultMiddleware = [ + function validateBearerToken(handler) { + return (req, res, ctx) => { + const tokenRe = /^Bearer (.+)$/; + const authHeader = req.headers.get('Authorization') ?? ''; + const [, bearerToken] = tokenRe.exec(authHeader) ?? []; + + if (bearerToken === mockBearerToken) { + return handler(req, res, ctx); + } + + return res(ctx.status(401)); + }; + }, +] as const satisfies readonly RestResolverMiddleware[]; + +export function wrapInDefaultMiddleware( + resolver: RestResolver, +): RestResolver { + return defaultMiddleware.reduceRight((currentResolver, middleware) => { + const recastMiddleware = + middleware as unknown as RestResolverMiddleware; + + return recastMiddleware(currentResolver); + }, resolver); +} + +function wrappedGet( + path: string, + resolver: RestResolver, +): RestHandler { + const wrapped = wrapInDefaultMiddleware(resolver); + return rest.get(path, wrapped); +} + const handlers: readonly RestHandler[] = [ - rest.get(`${root}/workspaces`, (req, res, ctx) => { + wrappedGet(`${root}/workspaces`, (req, res, ctx) => { const queryText = String(req.url.searchParams.get('q')); let returnedWorkspaces: Workspace[]; @@ -36,7 +89,7 @@ const handlers: readonly RestHandler[] = [ ); }), - rest.get( + wrappedGet( `${root}/workspacebuilds/:workspaceBuildId/parameters`, (req, res, ctx) => { const buildId = String(req.params.workspaceBuildId); @@ -51,7 +104,7 @@ const handlers: readonly RestHandler[] = [ ), // This is the dummy request used to verify a user's auth status - rest.get(`${root}/users/me`, (req, res, ctx) => { + wrappedGet(`${root}/users/me`, (req, res, ctx) => { const token = req.headers.get(CODER_AUTH_HEADER_KEY); if (token === mockCoderAuthToken) { return res(ctx.status(200)); diff --git a/plugins/backstage-plugin-coder/src/testHelpers/setup.tsx b/plugins/backstage-plugin-coder/src/testHelpers/setup.tsx index 92a23594..70afba5b 100644 --- a/plugins/backstage-plugin-coder/src/testHelpers/setup.tsx +++ b/plugins/backstage-plugin-coder/src/testHelpers/setup.tsx @@ -12,7 +12,11 @@ import { import React from 'react'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { scmIntegrationsApiRef } from '@backstage/integration-react'; -import { configApiRef, errorApiRef } from '@backstage/core-plugin-api'; +import { + configApiRef, + errorApiRef, + identityApiRef, +} from '@backstage/core-plugin-api'; import { EntityProvider } from '@backstage/plugin-catalog-react'; import { type CoderAuth, @@ -30,6 +34,7 @@ import { getMockConfigApi, mockAuthStates, BackstageEntity, + getMockIdentityApi, } from './mockBackstageData'; import { CoderErrorBoundary } from '../plugin'; @@ -159,6 +164,7 @@ export const renderHookAsCoderEntity = async < const mockErrorApi = getMockErrorApi(); const mockSourceControl = getMockSourceControl(); const mockConfigApi = getMockConfigApi(); + const mockIdentityApi = getMockIdentityApi(); const mockQueryClient = getMockQueryClient(); const renderHookValue = renderHook(hook, { @@ -168,6 +174,7 @@ export const renderHookAsCoderEntity = async < Date: Thu, 18 Apr 2024 18:58:00 +0000 Subject: [PATCH 7/7] refactor: update variable names for clarity --- plugins/backstage-plugin-coder/src/testHelpers/server.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/testHelpers/server.ts b/plugins/backstage-plugin-coder/src/testHelpers/server.ts index 04fbc6ce..99db7c1b 100644 --- a/plugins/backstage-plugin-coder/src/testHelpers/server.ts +++ b/plugins/backstage-plugin-coder/src/testHelpers/server.ts @@ -3,9 +3,9 @@ import { type DefaultBodyType, type ResponseResolver, type RestContext, - RestHandler, + type RestHandler, + type RestRequest, rest, - RestRequest, } from 'msw'; import { setupServer } from 'msw/node'; /* eslint-enable @backstage/no-undeclared-imports */ @@ -67,7 +67,7 @@ function wrappedGet( return rest.get(path, wrapped); } -const handlers: readonly RestHandler[] = [ +const mainTestHandlers: readonly RestHandler[] = [ wrappedGet(`${root}/workspaces`, (req, res, ctx) => { const queryText = String(req.url.searchParams.get('q')); @@ -114,4 +114,4 @@ const handlers: readonly RestHandler[] = [ }), ]; -export const server = setupServer(...handlers); +export const server = setupServer(...mainTestHandlers);