diff --git a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts index 9d29e68277..ffd118884e 100644 --- a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts +++ b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts @@ -14,109 +14,31 @@ * limitations under the License. */ -import { UserEntity } from '@backstage/catalog-model'; -import { rest } from 'msw'; -import { setupServer } from 'msw/node'; +import { CatalogClient } from '@backstage/catalog-client'; import { CatalogIdentityClient } from './CatalogIdentityClient'; -import { PluginEndpointDiscovery } from '@backstage/backend-common'; -const server = setupServer(); -const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base'; -const discovery: PluginEndpointDiscovery = { - async getBaseUrl() { - return mockBaseUrl; - }, - async getExternalBaseUrl() { - return mockBaseUrl; - }, -}; +jest.mock('./CatalogClient'); +const MockedCatalogClient = CatalogClient as jest.Mock; describe('CatalogIdentityClient', () => { - let client: CatalogIdentityClient; + const token = 'fake-id-token'; - beforeAll(() => server.listen({ onUnhandledRequest: 'error' })); - afterAll(() => server.close()); - afterEach(() => server.resetHandlers()); + afterEach(() => jest.resetAllMocks()); - beforeEach(() => { - client = new CatalogIdentityClient({ discovery }); - }); + it('passes through the correct search params', async () => { + const client = new CatalogIdentityClient({ + catalogClient: new MockedCatalogClient(), + }); - describe('findUser', () => { - const defaultServiceResponse: UserEntity[] = [ - { - apiVersion: 'backstage.io/v1alpha1', - kind: 'User', - metadata: { - name: 'Test1', - namespace: 'test1', - annotations: { - key: 'value', - }, - }, - spec: { - memberOf: ['group1'], - }, + client.findUser(token, { annotations: { key: 'value' } }); + + const getEntities = MockedCatalogClient.mock.instances[0].getEntities; + expect(getEntities).toHaveBeenCalledWith(token, { + filter: { + kind: 'user', + 'metadata.annotations.key': 'value', }, - ]; - - beforeEach(() => { - server.use( - rest.get(`${mockBaseUrl}/entities`, (_, res, ctx) => { - return res(ctx.json(defaultServiceResponse)); - }), - ); - }); - - it('should entities from correct endpoint', async () => { - const response = await client.findUser({ annotations: { key: 'value' } }); - expect(response).toEqual(defaultServiceResponse[0]); - }); - - it('builds entity search filters properly', async () => { - expect.assertions(2); - - server.use( - rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { - expect(req.url.search).toBe( - '?filter=kind=user,metadata.annotations.key=value', - ); - return res(ctx.json(defaultServiceResponse)); - }), - ); - - const response = await client.findUser({ annotations: { key: 'value' } }); - - expect(response).toEqual(defaultServiceResponse[0]); - }); - - it('omits authorization header if not available', async () => { - expect.assertions(1); - - server.use( - rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { - expect(req.headers.has('authorization')).toBe(false); - return res(ctx.json([])); - }), - ); - - client.findUser({ annotations: { key: 'value' } }); - }); - - it('adds authorization header if available', async () => { - expect.assertions(1); - - server.use( - rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { - expect(req.headers.get('authorization')).toEqual('hello'); - return res(ctx.json([])); - }), - ); - - client.findUser( - { annotations: { key: 'value' } }, - { headers: { authorization: 'hello' } }, - ); }); + expect(getEntities).toHaveBeenCalledTimes(1); }); }); diff --git a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts index f4aa722b2d..0fad8122f4 100644 --- a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts +++ b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts @@ -14,12 +14,8 @@ * limitations under the License. */ -import fetch from 'cross-fetch'; -import { - ConflictError, - NotFoundError, - PluginEndpointDiscovery, -} from '@backstage/backend-common'; +import { ConflictError, NotFoundError } from '@backstage/backend-common'; +import { CatalogClient } from '@backstage/catalog-client'; import { UserEntity } from '@backstage/catalog-model'; type UserQuery = { @@ -30,10 +26,10 @@ type UserQuery = { * A catalog client tailored for reading out identity data from the catalog. */ export class CatalogIdentityClient { - private readonly discovery: PluginEndpointDiscovery; + private readonly catalogClient: CatalogClient; - constructor(options: { discovery: PluginEndpointDiscovery }) { - this.discovery = options.discovery; + constructor(options: { catalogClient: CatalogClient }) { + this.catalogClient = options.catalogClient; } /** @@ -42,8 +38,8 @@ export class CatalogIdentityClient { * Throws a NotFoundError or ConflictError if 0 or multiple users are found. */ async findUser( + token: string | undefined, query: UserQuery, - options?: { headers?: Record }, ): Promise { const filter: Record = { kind: 'user', @@ -51,44 +47,17 @@ export class CatalogIdentityClient { for (const [key, value] of Object.entries(query.annotations)) { filter[`metadata.annotations.${key}`] = value; } - const params: string[] = []; - const filterParts: string[] = []; - for (const [key, value] of Object.entries(filter)) { - for (const v of [value].flat()) { - filterParts.push(`${encodeURIComponent(key)}=${encodeURIComponent(v)}`); - } - } - if (filterParts.length) { - params.push(`filter=${filterParts.join(',')}`); - } - const queryPart = params.length ? `?${params.join('&')}` : ''; + const { items } = await this.catalogClient.getEntities(token, { filter }); - const url = `${await this.discovery.getBaseUrl( - 'catalog', - )}/entities${queryPart}`; - const response = await fetch(url, { - headers: { - ...options?.headers, - }, - }); - - if (!response.ok) { - const payload = await response.text(); - const message = `Request failed with ${response.status} ${response.statusText}, ${payload}`; - throw new Error(message); - } - - const entities: UserEntity[] = await response.json(); - - if (entities.length !== 1) { - if (entities.length > 1) { + if (items.length !== 1) { + if (items.length > 1) { throw new ConflictError('User lookup resulted in multiple matches'); } else { throw new NotFoundError('User not found'); } } - return entities[0] as UserEntity; + return items[0] as UserEntity; } } diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts index 10f3ae96f2..340ec46529 100644 --- a/plugins/auth-backend/src/providers/google/provider.ts +++ b/plugins/auth-backend/src/providers/google/provider.ts @@ -157,18 +157,11 @@ export class GoogleAuthProvider implements OAuthHandlers { const token = await this.tokenIssuer.issueToken({ claims: { sub: 'backstage.io/auth-backend' }, }); - const user = await this.identityClient.findUser( - { - annotations: { - 'google.com/email': profile.email, - }, + const user = await this.identityClient.findUser(token, { + annotations: { + 'google.com/email': profile.email, }, - { - headers: { - authorization: `Bearer ${token}`, - }, - }, - ); + }); return { ...response, @@ -194,7 +187,7 @@ export const createGoogleProvider: AuthProviderFactory = ({ config, logger, tokenIssuer, - discovery, + catalogClient, }) => OAuthEnvironmentHandler.mapConfig(config, envConfig => { const clientId = envConfig.getString('clientId'); @@ -207,7 +200,7 @@ export const createGoogleProvider: AuthProviderFactory = ({ callbackUrl, logger, tokenIssuer, - identityClient: new CatalogIdentityClient({ discovery }), + identityClient: new CatalogIdentityClient({ catalogClient }), }); return OAuthAdapter.fromConfig(globalConfig, provider, { diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index 8339258654..1ab90ac522 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -15,6 +15,7 @@ */ import { PluginEndpointDiscovery } from '@backstage/backend-common'; +import { CatalogClient } from '@backstage/catalog-client'; import { Config } from '@backstage/config'; import express from 'express'; import { Logger } from 'winston'; @@ -132,6 +133,7 @@ export type AuthProviderFactoryOptions = { tokenIssuer: TokenIssuer; discovery: PluginEndpointDiscovery; identityResolver?: ExperimentalIdentityResolver; + catalogClient: CatalogClient; }; export type AuthProviderFactory = ( diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 742a269b73..859c2a4846 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -27,6 +27,7 @@ import { PluginDatabaseManager, PluginEndpointDiscovery, } from '@backstage/backend-common'; +import { CatalogClient } from '@backstage/catalog-client'; import { Config } from '@backstage/config'; import { createOidcRouter, DatabaseKeyStore, TokenFactory } from '../identity'; import session from 'express-session'; @@ -65,6 +66,7 @@ export async function createRouter({ keyDurationSeconds, logger: logger.child({ component: 'token-factory' }), }); + const catalogClient = new CatalogClient({ discoveryApi: discovery }); const secret = config.getOptionalString('auth.session.secret'); if (secret) { @@ -101,6 +103,7 @@ export async function createRouter({ logger, tokenIssuer, discovery, + catalogClient, }); const r = Router();