From ef10292e8cc8a073839139290c861b0c10fc18b0 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 13 Feb 2024 10:26:07 +0100 Subject: [PATCH 1/6] chore: removing this middleware Signed-off-by: blam --- plugins/badges-backend/src/service/router.ts | 45 ++++++-------------- 1 file changed, 12 insertions(+), 33 deletions(-) diff --git a/plugins/badges-backend/src/service/router.ts b/plugins/badges-backend/src/service/router.ts index 47bb8fcbe8..0223e2e84d 100644 --- a/plugins/badges-backend/src/service/router.ts +++ b/plugins/badges-backend/src/service/router.ts @@ -24,13 +24,12 @@ import { } from '@backstage/backend-common'; import { CatalogApi, CatalogClient } from '@backstage/catalog-client'; import { Config } from '@backstage/config'; -import { AuthenticationError, NotFoundError } from '@backstage/errors'; +import { NotFoundError } from '@backstage/errors'; import { BadgeBuilder, DefaultBadgeBuilder } from '../lib/BadgeBuilder'; import { BadgeContext, BadgeFactories } from '../types'; import { isNil } from 'lodash'; import { Logger } from 'winston'; import { IdentityApi } from '@backstage/plugin-auth-node'; -import { getBearerTokenFromAuthorizationHeader } from '@backstage/plugin-auth-node'; import { BadgesStore, DatabaseBadgesStore } from '../database/badgesStore'; import { createDefaultBadgeFactories } from '../badges'; @@ -215,39 +214,19 @@ async function obfuscatedRoute( res.status(200).send(data); }); - router.get( - '/entity/:namespace/:kind/:name/obfuscated', - function authenticate(req, _res, next) { - const token = - getBearerTokenFromAuthorizationHeader(req.headers.authorization) || - (req.cookies?.token as string | undefined); + router.get('/entity/:namespace/:kind/:name/obfuscated', async (req, res) => { + const { namespace, kind, name } = req.params; + const storedEntityUuid: { uuid: string } | undefined = + await store.getBadgeUuid(name, namespace, kind); - if (!token) { - throw new AuthenticationError('Unauthorized'); - } + if (isNil(storedEntityUuid)) { + throw new NotFoundError( + `No uuid found for entity "${namespace}/${kind}/${name}"`, + ); + } - try { - req.user = identity.getIdentity({ request: req }); - next(); - } catch (error) { - tokenManager.authenticate(token.toString()); - next(error); - } - }, - async (req, res) => { - const { namespace, kind, name } = req.params; - const storedEntityUuid: { uuid: string } | undefined = - await store.getBadgeUuid(name, namespace, kind); - - if (isNil(storedEntityUuid)) { - throw new NotFoundError( - `No uuid found for entity "${namespace}/${kind}/${name}"`, - ); - } - - return res.status(200).json(storedEntityUuid); - }, - ); + return res.status(200).json(storedEntityUuid); + }); router.use(errorHandler()); From 6991e5f37afd6940487dacde1e4d8f38538f7f3e Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 13 Feb 2024 10:26:51 +0100 Subject: [PATCH 2/6] chore: added changeset Signed-off-by: blam --- .changeset/curvy-icons-peel.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/curvy-icons-peel.md diff --git a/.changeset/curvy-icons-peel.md b/.changeset/curvy-icons-peel.md new file mode 100644 index 0000000000..589232766d --- /dev/null +++ b/.changeset/curvy-icons-peel.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-badges-backend': patch +--- + +Removing the authentication middleware as it's not used From c6a973ef1534a3cd1a8b421a33c5d03e2bf5273a Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 13 Feb 2024 10:27:37 +0100 Subject: [PATCH 3/6] chore: updated changeset wording Signed-off-by: blam --- .changeset/curvy-icons-peel.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/curvy-icons-peel.md b/.changeset/curvy-icons-peel.md index 589232766d..ccd8337bea 100644 --- a/.changeset/curvy-icons-peel.md +++ b/.changeset/curvy-icons-peel.md @@ -2,4 +2,4 @@ '@backstage/plugin-badges-backend': patch --- -Removing the authentication middleware as it's not used +Removing the authentication middleware from the obfuscated routes, as it doesn't do what it's supposed to do. From 750fca110d7179626444f02e4b66b3aa5fe8fe5e Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 13 Feb 2024 10:56:56 +0100 Subject: [PATCH 4/6] chore: fix the middleware Signed-off-by: blam --- .changeset/curvy-icons-peel.md | 2 +- plugins/badges-backend/src/service/router.ts | 56 +++++++++++++++----- 2 files changed, 45 insertions(+), 13 deletions(-) diff --git a/.changeset/curvy-icons-peel.md b/.changeset/curvy-icons-peel.md index ccd8337bea..bbadcb0a55 100644 --- a/.changeset/curvy-icons-peel.md +++ b/.changeset/curvy-icons-peel.md @@ -2,4 +2,4 @@ '@backstage/plugin-badges-backend': patch --- -Removing the authentication middleware from the obfuscated routes, as it doesn't do what it's supposed to do. +Updating the `authorization` middleware to call the Catalog to check that the requesting user has permission to see the Entity before generating the UUID. diff --git a/plugins/badges-backend/src/service/router.ts b/plugins/badges-backend/src/service/router.ts index 0223e2e84d..0c5e19d0e8 100644 --- a/plugins/badges-backend/src/service/router.ts +++ b/plugins/badges-backend/src/service/router.ts @@ -24,12 +24,13 @@ import { } from '@backstage/backend-common'; import { CatalogApi, CatalogClient } from '@backstage/catalog-client'; import { Config } from '@backstage/config'; -import { NotFoundError } from '@backstage/errors'; +import { AuthenticationError, NotFoundError } from '@backstage/errors'; import { BadgeBuilder, DefaultBadgeBuilder } from '../lib/BadgeBuilder'; import { BadgeContext, BadgeFactories } from '../types'; import { isNil } from 'lodash'; import { Logger } from 'winston'; import { IdentityApi } from '@backstage/plugin-auth-node'; +import { getBearerTokenFromAuthorizationHeader } from '@backstage/plugin-auth-node'; import { BadgesStore, DatabaseBadgesStore } from '../database/badgesStore'; import { createDefaultBadgeFactories } from '../badges'; @@ -214,19 +215,50 @@ async function obfuscatedRoute( res.status(200).send(data); }); - router.get('/entity/:namespace/:kind/:name/obfuscated', async (req, res) => { - const { namespace, kind, name } = req.params; - const storedEntityUuid: { uuid: string } | undefined = - await store.getBadgeUuid(name, namespace, kind); - - if (isNil(storedEntityUuid)) { - throw new NotFoundError( - `No uuid found for entity "${namespace}/${kind}/${name}"`, + router.get( + '/entity/:namespace/:kind/:name/obfuscated', + async function authenticate(req, _res, next) { + const token = getBearerTokenFromAuthorizationHeader( + req.headers.authorization, ); - } - return res.status(200).json(storedEntityUuid); - }); + const { kind, namespace, name } = req.params; + + // check that the user has the correct permissions + // to view the catalog entity by forwarding the token + const entity = await catalog.getEntityByRef( + { + kind, + namespace, + name, + }, + { token }, + ); + + if (!entity) { + next( + new NotFoundError( + `No ${kind} entity in ${namespace} named "${name}"`, + ), + ); + } else { + next(); + } + }, + async (req, res) => { + const { namespace, kind, name } = req.params; + const storedEntityUuid: { uuid: string } | undefined = + await store.getBadgeUuid(name, namespace, kind); + + if (isNil(storedEntityUuid)) { + throw new NotFoundError( + `No uuid found for entity "${namespace}/${kind}/${name}"`, + ); + } + + return res.status(200).json(storedEntityUuid); + }, + ); router.use(errorHandler()); From 59ce34dc4ddd04d19b03a76d89cbdb0e0b35dcf1 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 13 Feb 2024 11:38:00 +0100 Subject: [PATCH 5/6] chore: just throw it's cleaner Signed-off-by: blam --- plugins/badges-backend/src/service/router.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/plugins/badges-backend/src/service/router.ts b/plugins/badges-backend/src/service/router.ts index 0c5e19d0e8..e18f3667fb 100644 --- a/plugins/badges-backend/src/service/router.ts +++ b/plugins/badges-backend/src/service/router.ts @@ -24,7 +24,7 @@ import { } from '@backstage/backend-common'; import { CatalogApi, CatalogClient } from '@backstage/catalog-client'; import { Config } from '@backstage/config'; -import { AuthenticationError, NotFoundError } from '@backstage/errors'; +import { NotFoundError } from '@backstage/errors'; import { BadgeBuilder, DefaultBadgeBuilder } from '../lib/BadgeBuilder'; import { BadgeContext, BadgeFactories } from '../types'; import { isNil } from 'lodash'; @@ -236,10 +236,8 @@ async function obfuscatedRoute( ); if (!entity) { - next( - new NotFoundError( - `No ${kind} entity in ${namespace} named "${name}"`, - ), + throw new NotFoundError( + `No ${kind} entity in ${namespace} named "${name}"`, ); } else { next(); From 3d3c729728b6225dcf63eead67ae0d2e65c67e97 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 13 Feb 2024 13:53:14 +0100 Subject: [PATCH 6/6] chore: fixing mocking and tests Signed-off-by: blam --- .../src/service/router-obfuscated.test.ts | 23 +++++++++++++++---- plugins/badges-backend/src/service/router.ts | 5 ++-- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/plugins/badges-backend/src/service/router-obfuscated.test.ts b/plugins/badges-backend/src/service/router-obfuscated.test.ts index 49d0368c7f..fd4a525234 100644 --- a/plugins/badges-backend/src/service/router-obfuscated.test.ts +++ b/plugins/badges-backend/src/service/router-obfuscated.test.ts @@ -212,20 +212,30 @@ describe('createRouter', () => { }); describe('GET /entity/:namespace/:kind/:name/obfuscated', () => { - catalog.getEntityByRef.mockResolvedValueOnce(entity); - catalog.getEntities.mockResolvedValueOnce({ items: entities }); + beforeEach(() => { + catalog.getEntityByRef = jest.fn().mockResolvedValueOnce(entity); + catalog.getEntities = jest + .fn() + .mockResolvedValueOnce({ items: entities }); + }); + + it('returns obfuscated 404 if the user token does not return a catalog entity', async () => { + catalog.getEntityByRef = jest.fn().mockResolvedValue(undefined); - it('returns obfuscated 401 if no auth', async () => { const obfuscatedEntity = await request(app).get( '/entity/default/component/test/obfuscated', ); - expect(obfuscatedEntity.status).toEqual(401); + + expect(obfuscatedEntity.status).toEqual(404); }); it('returns obfuscated entity and badges', async () => { + catalog.getEntityByRef = jest.fn().mockResolvedValue(entity); + const obfuscatedEntity = await request(app) .get('/entity/default/component/test/obfuscated') .set('Authorization', 'Bearer fakeToken'); + expect(obfuscatedEntity.status).toEqual(200); expect(obfuscatedEntity.body.uuid).toMatch( new RegExp( @@ -233,6 +243,11 @@ describe('createRouter', () => { ), ); + expect(catalog.getEntityByRef).toHaveBeenCalledWith( + { namespace: 'default', kind: 'component', name: 'test' }, + { token: 'fakeToken' }, + ); + const uuid = obfuscatedEntity.body.uuid; const url = `/entity/${uuid}/test-badge?format=json`; let response = await request(app).get(url); diff --git a/plugins/badges-backend/src/service/router.ts b/plugins/badges-backend/src/service/router.ts index e18f3667fb..5a8fbb538a 100644 --- a/plugins/badges-backend/src/service/router.ts +++ b/plugins/badges-backend/src/service/router.ts @@ -60,7 +60,7 @@ export async function createRouter( ); const router = Router(); - const { config, logger, tokenManager, discovery, identity } = options; + const { config, logger, tokenManager, discovery } = options; const baseUrl = await discovery.getExternalBaseUrl('badges'); if (config.getOptionalBoolean('app.badges.obfuscate')) { @@ -72,7 +72,6 @@ export async function createRouter( logger, options, config, - identity, baseUrl, ); } @@ -94,7 +93,6 @@ async function obfuscatedRoute( logger: Logger, options: RouterOptions, config: Config, - identity: IdentityApi, baseUrl: string, ) { logger.info('Badges obfuscation is enabled'); @@ -130,6 +128,7 @@ async function obfuscatedRoute( }, token, ); + if (isNil(entity)) { throw new NotFoundError( `No ${kind} entity in ${namespace} named "${name}"`,