From 038b2e6894896812176f9467fa786febe0d457a2 Mon Sep 17 00:00:00 2001 From: Elias Rieb Date: Wed, 28 Feb 2024 12:52:29 +0100 Subject: [PATCH 1/4] fix(auth): consider only entities of kind user when using findCatalogUser with filter query Signed-off-by: Elias Rieb --- .changeset/poor-scissors-carry.md | 6 ++++++ .../src/lib/resolvers/CatalogAuthResolverContext.ts | 11 ++++++++++- plugins/auth-node/src/types.ts | 2 +- 3 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 .changeset/poor-scissors-carry.md diff --git a/.changeset/poor-scissors-carry.md b/.changeset/poor-scissors-carry.md new file mode 100644 index 0000000000..fd0d6e10e3 --- /dev/null +++ b/.changeset/poor-scissors-carry.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-auth-backend': patch +'@backstage/plugin-auth-node': patch +--- + +Only consider entities of kind `User` when using `findCatalogUser` with a filter query, unless an explicit `kind` filter is provided. diff --git a/plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.ts b/plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.ts index a4bd2203ae..1f4b2766e8 100644 --- a/plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.ts +++ b/plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.ts @@ -123,8 +123,17 @@ export class CatalogAuthResolverContext implements AuthResolverContext { const res = await this.catalogApi.getEntities({ filter }, { token }); result = res.items; } else if ('filter' in query) { + const filter = [query.filter].flat().map(value => { + if (!('kind' in Object.keys(value).map(key => key.toLowerCase()))) { + return { + ...value, + kind: 'user', + }; + } + return value; + }); const res = await this.catalogApi.getEntities( - { filter: query.filter }, + { filter: filter }, { token }, ); result = res.items; diff --git a/plugins/auth-node/src/types.ts b/plugins/auth-node/src/types.ts index d4cb0ac48e..1ee9e9cd96 100644 --- a/plugins/auth-node/src/types.ts +++ b/plugins/auth-node/src/types.ts @@ -86,7 +86,7 @@ export type BackstageUserIdentity = { * If `annotations` are used, all annotations must be present and * match the provided value exactly. Only entities of kind `'User'` will be considered. * - * If `filter` are used they are passed on as they are to the `CatalogApi`. + * If `filter` are used, only entities of kind `'User'` will be considered unless it is explicitly specified differently in the filter. * * Regardless of the query method, the query must match exactly one entity * in the catalog, or an error will be thrown. From 5a146a15fcde94ec62cc169ce30bdb2255765c6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 12 Mar 2024 12:12:33 +0100 Subject: [PATCH 2/4] Update plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../src/lib/resolvers/CatalogAuthResolverContext.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.ts b/plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.ts index 1f4b2766e8..ba12543e1a 100644 --- a/plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.ts +++ b/plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.ts @@ -124,7 +124,7 @@ export class CatalogAuthResolverContext implements AuthResolverContext { result = res.items; } else if ('filter' in query) { const filter = [query.filter].flat().map(value => { - if (!('kind' in Object.keys(value).map(key => key.toLowerCase()))) { + if (!('kind' in Object.keys(value).map(key => key.toLocaleLowerCase('en-US')))) { return { ...value, kind: 'user', From 469c9c59e89807072b11ba9c4057d5eec9a4ab13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 12 Mar 2024 13:17:57 +0100 Subject: [PATCH 3/4] prettier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../src/lib/resolvers/CatalogAuthResolverContext.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.ts b/plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.ts index ba12543e1a..a5c09b800b 100644 --- a/plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.ts +++ b/plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.ts @@ -124,7 +124,12 @@ export class CatalogAuthResolverContext implements AuthResolverContext { result = res.items; } else if ('filter' in query) { const filter = [query.filter].flat().map(value => { - if (!('kind' in Object.keys(value).map(key => key.toLocaleLowerCase('en-US')))) { + if ( + !( + 'kind' in + Object.keys(value).map(key => key.toLocaleLowerCase('en-US')) + ) + ) { return { ...value, kind: 'user', From 002bc3bf74b1cf03906a852f79d2e928f82b3972 Mon Sep 17 00:00:00 2001 From: Elias Rieb Date: Tue, 12 Mar 2024 18:02:59 +0100 Subject: [PATCH 4/4] fix code and add test Signed-off-by: Elias Rieb --- .../CatalogAuthResolverContext.test.ts | 54 +++++++++++++++++++ .../resolvers/CatalogAuthResolverContext.ts | 5 +- 2 files changed, 56 insertions(+), 3 deletions(-) create mode 100644 plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.test.ts diff --git a/plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.test.ts b/plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.test.ts new file mode 100644 index 0000000000..d16ba0303a --- /dev/null +++ b/plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.test.ts @@ -0,0 +1,54 @@ +/* + * 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 { CatalogAuthResolverContext } from './CatalogAuthResolverContext'; +import { getVoidLogger } from '@backstage/backend-common'; +import { CatalogApi } from '@backstage/catalog-client'; +import { mockServices } from '@backstage/backend-test-utils'; +import { TokenIssuer } from '../../identity/types'; +import { DiscoveryService } from '@backstage/backend-plugin-api'; + +describe('CatalogAuthResolverContext', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + const mockCatalogApi = { + getEntities: jest.fn().mockResolvedValue({ items: [{}] }), + } as Partial>; + + it('adds kind to filter when missing', async () => { + const context = CatalogAuthResolverContext.create({ + logger: getVoidLogger(), + catalogApi: mockCatalogApi as CatalogApi, + tokenIssuer: {} as TokenIssuer, + tokenManager: mockServices.tokenManager(), + discovery: {} as DiscoveryService, + auth: mockServices.auth(), + httpAuth: mockServices.httpAuth(), + }); + + await context.findCatalogUser({ + filter: [{}, { kind: 'group' }, { KIND: 'USER' }], + }); + expect(mockCatalogApi.getEntities).toHaveBeenCalledWith( + { + filter: [{ kind: 'user' }, { kind: 'group' }, { KIND: 'USER' }], + }, + { token: 'mock-service-token:{"sub":"plugin:test","target":"catalog"}' }, + ); + }); +}); diff --git a/plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.ts b/plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.ts index a5c09b800b..4f9673ac79 100644 --- a/plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.ts +++ b/plugins/auth-backend/src/lib/resolvers/CatalogAuthResolverContext.ts @@ -125,9 +125,8 @@ export class CatalogAuthResolverContext implements AuthResolverContext { } else if ('filter' in query) { const filter = [query.filter].flat().map(value => { if ( - !( - 'kind' in - Object.keys(value).map(key => key.toLocaleLowerCase('en-US')) + !Object.keys(value).some( + key => key.toLocaleLowerCase('en-US') === 'kind', ) ) { return {