From 484e500f4944f1b6c45f829801e552f847c585d7 Mon Sep 17 00:00:00 2001 From: Shijun Wang <2675460+Speissi@users.noreply.github.com> Date: Wed, 30 Jul 2025 14:38:44 +0300 Subject: [PATCH] techdocs-backend: migrate CachedEntityLoader to use BackstageCredentials (#30652) add changeset and tests Signed-off-by: Shijun Wang --- .changeset/sad-cities-lay.md | 5 + .../src/service/CachedEntityLoader.test.ts | 122 ++++++++++++++++-- .../src/service/CachedEntityLoader.ts | 22 +++- .../techdocs-backend/src/service/router.ts | 13 +- 4 files changed, 138 insertions(+), 24 deletions(-) create mode 100644 .changeset/sad-cities-lay.md diff --git a/.changeset/sad-cities-lay.md b/.changeset/sad-cities-lay.md new file mode 100644 index 0000000000..141c093b46 --- /dev/null +++ b/.changeset/sad-cities-lay.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-techdocs-backend': patch +--- + +Updated CachedEntityLoader to use BackstageCredentials instead of raw tokens for cache key generation. It now uses principal-based identification (user entity ref for users, subject for services) instead of token-based keys, providing more consistent caching behavior. diff --git a/plugins/techdocs-backend/src/service/CachedEntityLoader.test.ts b/plugins/techdocs-backend/src/service/CachedEntityLoader.test.ts index 2eb6bf03f4..df5c038dce 100644 --- a/plugins/techdocs-backend/src/service/CachedEntityLoader.test.ts +++ b/plugins/techdocs-backend/src/service/CachedEntityLoader.test.ts @@ -18,9 +18,11 @@ import { CachedEntityLoader } from './CachedEntityLoader'; import { CompoundEntityRef } from '@backstage/catalog-model'; import { mockServices } from '@backstage/backend-test-utils'; import { catalogServiceMock } from '@backstage/plugin-catalog-node/testUtils'; +import { BackstageCredentials } from '@backstage/backend-plugin-api'; describe('CachedEntityLoader', () => { const cache = mockServices.cache.mock(); + const auth = mockServices.auth.mock(); const entityName: CompoundEntityRef = { kind: 'component', @@ -39,20 +41,37 @@ describe('CachedEntityLoader', () => { const token = 'test-token'; + const userCredentials: BackstageCredentials = { + $$type: '@backstage/BackstageCredentials', + principal: { + type: 'user', + userEntityRef: 'user:default/test-user', + }, + }; + + const pluginCredentials: BackstageCredentials = { + $$type: '@backstage/BackstageCredentials', + principal: { + type: 'plugin', + subject: 'plugin:test-plugin', + }, + }; + afterEach(() => { jest.resetAllMocks(); }); - it('writes entities to cache', async () => { + it('writes entities to cache for user credentials', async () => { cache.get.mockResolvedValue(undefined); const catalog = catalogServiceMock({ entities: [entity] }); + auth.isPrincipal.mockReturnValue(true); - const loader = new CachedEntityLoader({ catalog, cache }); - const result = await loader.load(entityName, token); + const loader = new CachedEntityLoader({ auth, catalog, cache }); + const result = await loader.load(userCredentials, entityName, token); expect(result).toEqual(entity); expect(cache.set).toHaveBeenCalledWith( - 'catalog:component:default/test:test-token', + 'catalog:component:default/test:user:default/test-user', entity, { ttl: 5000 }, ); @@ -62,9 +81,10 @@ describe('CachedEntityLoader', () => { const catalog = catalogServiceMock(); jest.spyOn(catalog, 'getEntityByRef'); cache.get.mockResolvedValue(entity); + auth.isPrincipal.mockReturnValue(true); - const loader = new CachedEntityLoader({ catalog, cache }); - const result = await loader.load(entityName, token); + const loader = new CachedEntityLoader({ auth, catalog, cache }); + const result = await loader.load(userCredentials, entityName, token); expect(result).toEqual(entity); expect(catalog.getEntityByRef).not.toHaveBeenCalled(); @@ -73,24 +93,26 @@ describe('CachedEntityLoader', () => { it('does not cache missing entities', async () => { const catalog = catalogServiceMock({ entities: [] }); cache.get.mockResolvedValue(undefined); + auth.isPrincipal.mockReturnValue(true); - const loader = new CachedEntityLoader({ catalog, cache }); - const result = await loader.load(entityName, token); + const loader = new CachedEntityLoader({ auth, catalog, cache }); + const result = await loader.load(userCredentials, entityName, token); expect(result).toBeUndefined(); expect(cache.set).not.toHaveBeenCalled(); }); - it('uses entity ref as cache key for anonymous users', async () => { + it('uses entity ref as cache key for service credentials', async () => { const catalog = catalogServiceMock({ entities: [entity] }); cache.get.mockResolvedValue(undefined); + auth.isPrincipal.mockReturnValueOnce(false).mockReturnValueOnce(true); - const loader = new CachedEntityLoader({ catalog, cache }); - const result = await loader.load(entityName, undefined); + const loader = new CachedEntityLoader({ auth, catalog, cache }); + const result = await loader.load(pluginCredentials, entityName, undefined); expect(result).toEqual(entity); expect(cache.set).toHaveBeenCalledWith( - 'catalog:component:default/test', + 'catalog:component:default/test:plugin:test-plugin', entity, { ttl: 5000, @@ -106,10 +128,82 @@ describe('CachedEntityLoader', () => { }), ); const catalog = catalogServiceMock({ entities: [entity] }); + auth.isPrincipal.mockReturnValue(true); - const loader = new CachedEntityLoader({ catalog, cache }); - const result = await loader.load(entityName, token); + const loader = new CachedEntityLoader({ auth, catalog, cache }); + const result = await loader.load(userCredentials, entityName, token); expect(result).toEqual(entity); }); + + it('creates different cache keys for different users', async () => { + const catalog = catalogServiceMock({ entities: [entity] }); + cache.get.mockResolvedValue(undefined); + auth.isPrincipal.mockReturnValue(true); + + const loader = new CachedEntityLoader({ auth, catalog, cache }); + + const anotherUserCredentials: BackstageCredentials = { + $$type: '@backstage/BackstageCredentials', + principal: { + type: 'user', + userEntityRef: 'user:default/another-user', + }, + }; + + await loader.load(userCredentials, entityName, token); + await loader.load(anotherUserCredentials, entityName, token); + + expect(cache.set).toHaveBeenCalledWith( + 'catalog:component:default/test:user:default/test-user', + entity, + { ttl: 5000 }, + ); + expect(cache.set).toHaveBeenCalledWith( + 'catalog:component:default/test:user:default/another-user', + entity, + { ttl: 5000 }, + ); + }); + + it('creates cache key with service subject for service credentials', async () => { + const catalog = catalogServiceMock({ entities: [entity] }); + cache.get.mockResolvedValue(undefined); + auth.isPrincipal.mockReturnValueOnce(false).mockReturnValueOnce(true); + + const loader = new CachedEntityLoader({ auth, catalog, cache }); + const result = await loader.load(pluginCredentials, entityName, token); + + expect(result).toEqual(entity); + expect(cache.set).toHaveBeenCalledWith( + 'catalog:component:default/test:plugin:test-plugin', + entity, + { ttl: 5000 }, + ); + expect(auth.isPrincipal).toHaveBeenCalledWith(pluginCredentials, 'user'); + expect(auth.isPrincipal).toHaveBeenCalledWith(pluginCredentials, 'service'); + }); + + it('handles credentials that are neither user nor service', async () => { + const catalog = catalogServiceMock({ entities: [entity] }); + cache.get.mockResolvedValue(undefined); + auth.isPrincipal.mockReturnValue(false); + + const unknownCredentials: BackstageCredentials = { + $$type: '@backstage/BackstageCredentials', + principal: { + type: 'unknown' as any, + }, + }; + + const loader = new CachedEntityLoader({ auth, catalog, cache }); + const result = await loader.load(unknownCredentials, entityName, token); + + expect(result).toEqual(entity); + expect(cache.set).toHaveBeenCalledWith( + 'catalog:component:default/test', + entity, + { ttl: 5000 }, + ); + }); }); diff --git a/plugins/techdocs-backend/src/service/CachedEntityLoader.ts b/plugins/techdocs-backend/src/service/CachedEntityLoader.ts index 7541509907..9f97ed2cd4 100644 --- a/plugins/techdocs-backend/src/service/CachedEntityLoader.ts +++ b/plugins/techdocs-backend/src/service/CachedEntityLoader.ts @@ -14,7 +14,11 @@ * limitations under the License. */ -import { CacheService } from '@backstage/backend-plugin-api'; +import { + AuthService, + BackstageCredentials, + CacheService, +} from '@backstage/backend-plugin-api'; import { CatalogApi } from '@backstage/catalog-client'; import { Entity, @@ -23,25 +27,29 @@ import { } from '@backstage/catalog-model'; export type CachedEntityLoaderOptions = { + auth: AuthService; catalog: CatalogApi; cache: CacheService; }; export class CachedEntityLoader { + private readonly auth: AuthService; private readonly catalog: CatalogApi; private readonly cache: CacheService; private readonly readTimeout = 1000; - constructor({ catalog, cache }: CachedEntityLoaderOptions) { + constructor({ auth, catalog, cache }: CachedEntityLoaderOptions) { + this.auth = auth; this.catalog = catalog; this.cache = cache; } async load( + credentials: BackstageCredentials, entityRef: CompoundEntityRef, token: string | undefined, ): Promise { - const cacheKey = this.getCacheKey(entityRef, token); + const cacheKey = this.getCacheKey(entityRef, credentials); let result = await this.getFromCache(cacheKey); if (result) { @@ -68,12 +76,14 @@ export class CachedEntityLoader { private getCacheKey( entityName: CompoundEntityRef, - token: string | undefined, + credentials: BackstageCredentials, ): string { const key = ['catalog', stringifyEntityRef(entityName)]; - if (token) { - key.push(token); + if (this.auth.isPrincipal(credentials, 'user')) { + key.push(credentials.principal.userEntityRef); + } else if (this.auth.isPrincipal(credentials, 'service')) { + key.push(credentials.principal.subject); } return key.join(':'); diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index bfaf072717..7ce26232e1 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -124,6 +124,7 @@ export async function createRouter( // Entities are cached to optimize the /static/docs request path, which can be called many times // when loading a single techdocs page. const entityLoader = new CachedEntityLoader({ + auth, catalog: catalogClient, cache: options.cache, }); @@ -158,7 +159,7 @@ export async function createRouter( }); // Verify that the related entity exists and the current user has permission to view it. - const entity = await entityLoader.load(entityName, token); + const entity = await entityLoader.load(credentials, entityName, token); if (!entity) { throw new NotFoundError( @@ -196,7 +197,7 @@ export async function createRouter( targetPluginId: 'catalog', }); - const entity = await entityLoader.load(entityName, token); + const entity = await entityLoader.load(credentials, entityName, token); if (!entity) { throw new NotFoundError( @@ -234,7 +235,11 @@ export async function createRouter( targetPluginId: 'catalog', }); - const entity = await entityLoader.load({ kind, namespace, name }, token); + const entity = await entityLoader.load( + credentials, + { kind, namespace, name }, + token, + ); if (!entity?.metadata?.uid) { throw new NotFoundError('Entity metadata UID missing'); @@ -305,7 +310,7 @@ export async function createRouter( targetPluginId: 'catalog', }); - const entity = await entityLoader.load(entityName, token); + const entity = await entityLoader.load(credentials, entityName, token); if (!entity) { throw new NotFoundError(