From 2f0519cba345407cfb63bf2773760893a1740240 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 14 May 2026 11:53:36 +0200 Subject: [PATCH 1/3] permission: cache user info and parallelize resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The permission backend previously resolved userInfo and minted a plugin request token sequentially for every authorize request with user credentials. On high-traffic endpoints this meant two serial internal HTTP round-trips per request, even when the same user made many requests in quick succession. This change: 1. Adds a 5-second TTL cache to DefaultUserInfoService so that repeated getUserInfo() calls for the same user return the cached result without an HTTP call to the auth backend. 2. Parallelises the getUserInfo() and getPluginRequestToken() calls in the permission backend's handleRequest via Promise.all, saving one sequential round-trip on cache misses. Signed-off-by: Fredrik Adelöw Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Fredrik Adelöw --- .changeset/permission-backend-drop-token.md | 5 + ...mission-node-optional-deprecated-fields.md | 11 ++ .changeset/permission-userinfo-cache.md | 5 + .../userInfo/CachedUserInfoService.test.ts | 181 ++++++++++++++++++ .../userInfo/CachedUserInfoService.ts | 90 +++++++++ .../userInfo/userInfoServiceFactory.ts | 14 +- .../src/service/router.test.ts | 11 -- .../permission-backend/src/service/router.ts | 10 - plugins/permission-node/package.json | 1 - plugins/permission-node/report.api.md | 4 - plugins/permission-node/src/policy/types.ts | 24 +-- yarn.lock | 1 - 12 files changed, 306 insertions(+), 51 deletions(-) create mode 100644 .changeset/permission-backend-drop-token.md create mode 100644 .changeset/permission-node-optional-deprecated-fields.md create mode 100644 .changeset/permission-userinfo-cache.md create mode 100644 packages/backend-defaults/src/entrypoints/userInfo/CachedUserInfoService.test.ts create mode 100644 packages/backend-defaults/src/entrypoints/userInfo/CachedUserInfoService.ts diff --git a/.changeset/permission-backend-drop-token.md b/.changeset/permission-backend-drop-token.md new file mode 100644 index 0000000000..b3fb317897 --- /dev/null +++ b/.changeset/permission-backend-drop-token.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-permission-backend': patch +--- + +The permission backend no longer populates the removed `token` and `identity` fields on `PolicyQueryUser`, and no longer calls `auth.getPluginRequestToken()` during policy evaluation. This removes one internal round-trip per authorize request. diff --git a/.changeset/permission-node-optional-deprecated-fields.md b/.changeset/permission-node-optional-deprecated-fields.md new file mode 100644 index 0000000000..b4d5fed41c --- /dev/null +++ b/.changeset/permission-node-optional-deprecated-fields.md @@ -0,0 +1,11 @@ +--- +'@backstage/plugin-permission-node': minor +--- + +**BREAKING**: Cleaned up the `PolicyQueryUser` type: + +- `token` — **Removed.** Was previously deprecated in favor of `credentials` with `coreServices.auth`. +- `expiresInSeconds` — **Removed.** Was previously deprecated. +- `identity` — **Removed.** Was previously deprecated in favor of `info`. +- `info` — **Deprecated.** Still required and populated for now; will be made optional and then removed in a future release. +- `credentials` — Unchanged. diff --git a/.changeset/permission-userinfo-cache.md b/.changeset/permission-userinfo-cache.md new file mode 100644 index 0000000000..6e82a62431 --- /dev/null +++ b/.changeset/permission-userinfo-cache.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-defaults': patch +--- + +Added a new `CachedUserInfoService` decorator that wraps `DefaultUserInfoService` with a 5-second TTL cache and in-flight request coalescing. The decorator is wired in via `userInfoServiceFactory` using a shared root-level cache. Repeated `getUserInfo()` calls for the same user token within the TTL window return the cached result without making an HTTP call to the auth backend. Note that custom `UserInfoService` implementations registered via their own factory will not benefit from this cache automatically. diff --git a/packages/backend-defaults/src/entrypoints/userInfo/CachedUserInfoService.test.ts b/packages/backend-defaults/src/entrypoints/userInfo/CachedUserInfoService.test.ts new file mode 100644 index 0000000000..c44062f127 --- /dev/null +++ b/packages/backend-defaults/src/entrypoints/userInfo/CachedUserInfoService.test.ts @@ -0,0 +1,181 @@ +/* + * Copyright 2026 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 { + BackstageUserInfo, + UserInfoService, +} from '@backstage/backend-plugin-api'; +import { mockCredentials } from '@backstage/backend-test-utils'; +import { CachedUserInfoService } from './CachedUserInfoService'; + +const aliceInfo: BackstageUserInfo = { + userEntityRef: 'user:default/alice', + ownershipEntityRefs: ['user:default/alice', 'group:default/team-a'], +}; + +describe('CachedUserInfoService', () => { + it('delegates to the underlying service on the first call', async () => { + const delegate: UserInfoService = { + getUserInfo: jest.fn().mockResolvedValue(aliceInfo), + }; + const service = new CachedUserInfoService(delegate); + + const result = await service.getUserInfo(mockCredentials.user()); + + expect(result).toEqual(aliceInfo); + expect(delegate.getUserInfo).toHaveBeenCalledTimes(1); + }); + + it('returns the cached result on subsequent calls within TTL', async () => { + const delegate: UserInfoService = { + getUserInfo: jest.fn().mockResolvedValue(aliceInfo), + }; + const service = new CachedUserInfoService(delegate); + const creds = mockCredentials.user(); + + await service.getUserInfo(creds); + await service.getUserInfo(creds); + await service.getUserInfo(creds); + + expect(delegate.getUserInfo).toHaveBeenCalledTimes(1); + }); + + it('coalesces concurrent in-flight requests for the same token', async () => { + const delegate: UserInfoService = { + getUserInfo: jest.fn().mockResolvedValue(aliceInfo), + }; + const service = new CachedUserInfoService(delegate); + const creds = mockCredentials.user(); + + const [r1, r2, r3] = await Promise.all([ + service.getUserInfo(creds), + service.getUserInfo(creds), + service.getUserInfo(creds), + ]); + + expect(r1).toEqual(aliceInfo); + expect(r2).toEqual(aliceInfo); + expect(r3).toEqual(aliceInfo); + expect(delegate.getUserInfo).toHaveBeenCalledTimes(1); + }); + + it('caches different tokens separately', async () => { + const delegate: UserInfoService = { + getUserInfo: jest.fn().mockResolvedValue(aliceInfo), + }; + const service = new CachedUserInfoService(delegate); + + await service.getUserInfo(mockCredentials.user('user:default/alice')); + await service.getUserInfo(mockCredentials.user('user:default/bob')); + + expect(delegate.getUserInfo).toHaveBeenCalledTimes(2); + }); + + it('re-fetches after the TTL expires', async () => { + const delegate: UserInfoService = { + getUserInfo: jest.fn().mockResolvedValue(aliceInfo), + }; + const service = new CachedUserInfoService(delegate, { ttlMs: 50 }); + const creds = mockCredentials.user(); + + await service.getUserInfo(creds); + expect(delegate.getUserInfo).toHaveBeenCalledTimes(1); + + await new Promise(resolve => setTimeout(resolve, 60)); + + await service.getUserInfo(creds); + expect(delegate.getUserInfo).toHaveBeenCalledTimes(2); + }); + + it('evicts the cache entry on rejection and retries on next call', async () => { + const delegate: UserInfoService = { + getUserInfo: jest + .fn() + .mockRejectedValueOnce(new Error('auth backend down')) + .mockResolvedValueOnce(aliceInfo), + }; + const service = new CachedUserInfoService(delegate); + const creds = mockCredentials.user(); + + await expect(service.getUserInfo(creds)).rejects.toThrow( + 'auth backend down', + ); + expect(delegate.getUserInfo).toHaveBeenCalledTimes(1); + + const result = await service.getUserInfo(creds); + expect(result).toEqual(aliceInfo); + expect(delegate.getUserInfo).toHaveBeenCalledTimes(2); + }); + + it('evicts eagerly so concurrent waiters see the rejection and the next call retries', async () => { + let rejectFirst: (error: Error) => void; + const firstCall = new Promise((_resolve, reject) => { + rejectFirst = reject; + }); + + const delegate: UserInfoService = { + getUserInfo: jest + .fn() + .mockReturnValueOnce(firstCall) + .mockResolvedValueOnce(aliceInfo), + }; + const service = new CachedUserInfoService(delegate); + const creds = mockCredentials.user(); + + const p1 = service.getUserInfo(creds); + const p2 = service.getUserInfo(creds); + + rejectFirst!(new Error('boom')); + + await expect(p1).rejects.toThrow('boom'); + await expect(p2).rejects.toThrow('boom'); + + const result = await service.getUserInfo(creds); + expect(result).toEqual(aliceInfo); + expect(delegate.getUserInfo).toHaveBeenCalledTimes(2); + }); + + it('delegates directly when credentials have no token', async () => { + const delegate: UserInfoService = { + getUserInfo: jest.fn().mockResolvedValue(aliceInfo), + }; + const service = new CachedUserInfoService(delegate); + + await service.getUserInfo(mockCredentials.none()); + await service.getUserInfo(mockCredentials.none()); + + expect(delegate.getUserInfo).toHaveBeenCalledTimes(2); + }); + + it('shares cache entries across instances when given the same map', async () => { + const delegate1: UserInfoService = { + getUserInfo: jest.fn().mockResolvedValue(aliceInfo), + }; + const delegate2: UserInfoService = { + getUserInfo: jest.fn().mockResolvedValue(aliceInfo), + }; + const entries = new Map(); + const service1 = new CachedUserInfoService(delegate1, { entries }); + const service2 = new CachedUserInfoService(delegate2, { entries }); + const creds = mockCredentials.user(); + + await service1.getUserInfo(creds); + await service2.getUserInfo(creds); + + expect(delegate1.getUserInfo).toHaveBeenCalledTimes(1); + expect(delegate2.getUserInfo).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/backend-defaults/src/entrypoints/userInfo/CachedUserInfoService.ts b/packages/backend-defaults/src/entrypoints/userInfo/CachedUserInfoService.ts new file mode 100644 index 0000000000..41d7480341 --- /dev/null +++ b/packages/backend-defaults/src/entrypoints/userInfo/CachedUserInfoService.ts @@ -0,0 +1,90 @@ +/* + * Copyright 2026 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 { + BackstageCredentials, + BackstageUserInfo, + UserInfoService, +} from '@backstage/backend-plugin-api'; +import { toInternalBackstageCredentials } from '../auth/helpers'; + +const DEFAULT_TTL_MS = 5_000; +const SWEEP_INTERVAL_MS = 30_000; + +export type UserInfoCacheEntry = { + promise: Promise; + expiresAt: number; +}; + +export class CachedUserInfoService implements UserInfoService { + readonly #delegate: UserInfoService; + readonly #entries: Map; + readonly #ttlMs: number; + #lastSweep: number = Date.now(); + + constructor( + delegate: UserInfoService, + options?: { + entries?: Map; + ttlMs?: number; + }, + ) { + this.#delegate = delegate; + this.#entries = options?.entries ?? new Map(); + this.#ttlMs = options?.ttlMs ?? DEFAULT_TTL_MS; + } + + async getUserInfo( + credentials: BackstageCredentials, + ): Promise { + const internalCredentials = toInternalBackstageCredentials(credentials); + const token = internalCredentials.token; + if (!token) { + return this.#delegate.getUserInfo(credentials); + } + + const now = Date.now(); + + const cached = this.#entries.get(token); + if (cached) { + if (cached.expiresAt > now) { + return cached.promise; + } + this.#entries.delete(token); + } + + if (now - this.#lastSweep > SWEEP_INTERVAL_MS) { + this.#lastSweep = now; + for (const [key, entry] of this.#entries) { + if (entry.expiresAt <= now) { + this.#entries.delete(key); + } + } + } + + const promise = this.#delegate.getUserInfo(credentials).catch(error => { + this.#entries.delete(token); + throw error; + }); + + this.#entries.set(token, { + promise, + expiresAt: now + this.#ttlMs, + }); + + return promise; + } +} diff --git a/packages/backend-defaults/src/entrypoints/userInfo/userInfoServiceFactory.ts b/packages/backend-defaults/src/entrypoints/userInfo/userInfoServiceFactory.ts index fe8c398bca..02b485b7b2 100644 --- a/packages/backend-defaults/src/entrypoints/userInfo/userInfoServiceFactory.ts +++ b/packages/backend-defaults/src/entrypoints/userInfo/userInfoServiceFactory.ts @@ -18,6 +18,10 @@ import { coreServices, createServiceFactory, } from '@backstage/backend-plugin-api'; +import { + CachedUserInfoService, + UserInfoCacheEntry, +} from './CachedUserInfoService'; import { DefaultUserInfoService } from './DefaultUserInfoService'; /** @@ -34,7 +38,13 @@ export const userInfoServiceFactory = createServiceFactory({ deps: { discovery: coreServices.discovery, }, - async factory({ discovery }) { - return new DefaultUserInfoService({ discovery }); + createRootContext() { + return new Map(); + }, + async factory({ discovery }, entries) { + return new CachedUserInfoService( + new DefaultUserInfoService({ discovery }), + { entries }, + ); }, }); diff --git a/plugins/permission-backend/src/service/router.test.ts b/plugins/permission-backend/src/service/router.test.ts index fe2b880d21..7246a449c2 100644 --- a/plugins/permission-backend/src/service/router.test.ts +++ b/plugins/permission-backend/src/service/router.test.ts @@ -270,17 +270,6 @@ describe('createRouter', () => { }, }, { - token: mockCredentials.service.token({ - onBehalfOf: mockCredentials.user(), - targetPluginId: 'catalog', - }), - identity: { - type: 'user', - userEntityRef: mockCredentials.user().principal.userEntityRef, - ownershipEntityRefs: [ - mockCredentials.user().principal.userEntityRef, - ], - }, info: { userEntityRef: mockCredentials.user().principal.userEntityRef, ownershipEntityRefs: [ diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index 92a2e40259..67004d3789 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -132,17 +132,7 @@ const handleRequest = async ( let user: PolicyQueryUser | undefined; if (auth.isPrincipal(credentials, 'user')) { const info = await userInfo.getUserInfo(credentials); - const { token } = await auth.getPluginRequestToken({ - onBehalfOf: credentials, - targetPluginId: 'catalog', // TODO: unknown at this point - }); user = { - identity: { - type: 'user', - userEntityRef: credentials.principal.userEntityRef, - ownershipEntityRefs: info.ownershipEntityRefs, - }, - token, credentials, info, }; diff --git a/plugins/permission-node/package.json b/plugins/permission-node/package.json index 342f131014..3501e1d0e3 100644 --- a/plugins/permission-node/package.json +++ b/plugins/permission-node/package.json @@ -59,7 +59,6 @@ "@backstage/backend-plugin-api": "workspace:^", "@backstage/config": "workspace:^", "@backstage/errors": "workspace:^", - "@backstage/plugin-auth-node": "workspace:^", "@backstage/plugin-permission-common": "workspace:^", "@types/express": "^4.17.6", "express": "^4.22.0", diff --git a/plugins/permission-node/report.api.md b/plugins/permission-node/report.api.md index 09452b969b..1ba1f514bd 100644 --- a/plugins/permission-node/report.api.md +++ b/plugins/permission-node/report.api.md @@ -10,7 +10,6 @@ import { AuthorizePermissionResponse } from '@backstage/plugin-permission-common import { AuthorizeResult } from '@backstage/plugin-permission-common'; import { AuthService } from '@backstage/backend-plugin-api'; import { BackstageCredentials } from '@backstage/backend-plugin-api'; -import { BackstageUserIdentity } from '@backstage/plugin-auth-node'; import { BackstageUserInfo } from '@backstage/backend-plugin-api'; import { ConditionalPolicyDecision } from '@backstage/plugin-permission-common'; import { Config } from '@backstage/config'; @@ -365,9 +364,6 @@ export type PolicyQuery = { // @public export type PolicyQueryUser = { - token: string; - expiresInSeconds?: number; - identity: BackstageUserIdentity; credentials: BackstageCredentials; info: BackstageUserInfo; }; diff --git a/plugins/permission-node/src/policy/types.ts b/plugins/permission-node/src/policy/types.ts index a7b8b75218..9c63c3534e 100644 --- a/plugins/permission-node/src/policy/types.ts +++ b/plugins/permission-node/src/policy/types.ts @@ -18,7 +18,6 @@ import { Permission, PolicyDecision, } from '@backstage/plugin-permission-common'; -import { BackstageUserIdentity } from '@backstage/plugin-auth-node'; import { BackstageCredentials, BackstageUserInfo, @@ -44,27 +43,6 @@ export type PolicyQuery = { * @public */ export type PolicyQueryUser = { - /** - * The token used to authenticate the user within Backstage. - * - * @deprecated User the `credentials` field in combination with `coreServices.auth` to generate a request token instead. - */ - token: string; - - /** - * The number of seconds until the token expires. If not set, it can be assumed that the token does not expire. - * - * @deprecated This field is deprecated and will be removed in a future release. - */ - expiresInSeconds?: number; - - /** - * A plaintext description of the identity that is encapsulated within the token. - * - * @deprecated Use the `info` field instead. - */ - identity: BackstageUserIdentity; - /** * The credentials of the user making the request. */ @@ -72,6 +50,8 @@ export type PolicyQueryUser = { /** * The information for the user making the request. + * + * @deprecated This field is deprecated and will be removed in a future release. */ info: BackstageUserInfo; }; diff --git a/yarn.lock b/yarn.lock index 224a8264ab..fc6b2b0cd2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6498,7 +6498,6 @@ __metadata: "@backstage/cli": "workspace:^" "@backstage/config": "workspace:^" "@backstage/errors": "workspace:^" - "@backstage/plugin-auth-node": "workspace:^" "@backstage/plugin-permission-common": "workspace:^" "@types/express": "npm:^4.17.6" "@types/supertest": "npm:^2.0.8" From 644adbd19abe7646c9ff94523b06308fc754f7fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 19 May 2026 14:09:58 +0200 Subject: [PATCH 2/3] Move sweep before cache lookup to simplify expired entry handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Fredrik Adelöw --- .../entrypoints/userInfo/CachedUserInfoService.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/backend-defaults/src/entrypoints/userInfo/CachedUserInfoService.ts b/packages/backend-defaults/src/entrypoints/userInfo/CachedUserInfoService.ts index 41d7480341..7d26e3af70 100644 --- a/packages/backend-defaults/src/entrypoints/userInfo/CachedUserInfoService.ts +++ b/packages/backend-defaults/src/entrypoints/userInfo/CachedUserInfoService.ts @@ -58,14 +58,6 @@ export class CachedUserInfoService implements UserInfoService { const now = Date.now(); - const cached = this.#entries.get(token); - if (cached) { - if (cached.expiresAt > now) { - return cached.promise; - } - this.#entries.delete(token); - } - if (now - this.#lastSweep > SWEEP_INTERVAL_MS) { this.#lastSweep = now; for (const [key, entry] of this.#entries) { @@ -75,6 +67,11 @@ export class CachedUserInfoService implements UserInfoService { } } + const cached = this.#entries.get(token); + if (cached && cached.expiresAt > now) { + return cached.promise; + } + const promise = this.#delegate.getUserInfo(credentials).catch(error => { this.#entries.delete(token); throw error; From a9f5560b492bc54c041df3342f0289a9eb97cc5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 19 May 2026 14:44:28 +0200 Subject: [PATCH 3/3] Address review feedback: use fake timers and type the shared cache map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Fredrik Adelöw --- .../userInfo/CachedUserInfoService.test.ts | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/packages/backend-defaults/src/entrypoints/userInfo/CachedUserInfoService.test.ts b/packages/backend-defaults/src/entrypoints/userInfo/CachedUserInfoService.test.ts index c44062f127..b846a79200 100644 --- a/packages/backend-defaults/src/entrypoints/userInfo/CachedUserInfoService.test.ts +++ b/packages/backend-defaults/src/entrypoints/userInfo/CachedUserInfoService.test.ts @@ -19,7 +19,10 @@ import { UserInfoService, } from '@backstage/backend-plugin-api'; import { mockCredentials } from '@backstage/backend-test-utils'; -import { CachedUserInfoService } from './CachedUserInfoService'; +import { + CachedUserInfoService, + UserInfoCacheEntry, +} from './CachedUserInfoService'; const aliceInfo: BackstageUserInfo = { userEntityRef: 'user:default/alice', @@ -85,19 +88,24 @@ describe('CachedUserInfoService', () => { }); it('re-fetches after the TTL expires', async () => { - const delegate: UserInfoService = { - getUserInfo: jest.fn().mockResolvedValue(aliceInfo), - }; - const service = new CachedUserInfoService(delegate, { ttlMs: 50 }); - const creds = mockCredentials.user(); + jest.useFakeTimers(); + try { + const delegate: UserInfoService = { + getUserInfo: jest.fn().mockResolvedValue(aliceInfo), + }; + const service = new CachedUserInfoService(delegate, { ttlMs: 50 }); + const creds = mockCredentials.user(); - await service.getUserInfo(creds); - expect(delegate.getUserInfo).toHaveBeenCalledTimes(1); + await service.getUserInfo(creds); + expect(delegate.getUserInfo).toHaveBeenCalledTimes(1); - await new Promise(resolve => setTimeout(resolve, 60)); + jest.advanceTimersByTime(60); - await service.getUserInfo(creds); - expect(delegate.getUserInfo).toHaveBeenCalledTimes(2); + await service.getUserInfo(creds); + expect(delegate.getUserInfo).toHaveBeenCalledTimes(2); + } finally { + jest.useRealTimers(); + } }); it('evicts the cache entry on rejection and retries on next call', async () => { @@ -167,7 +175,7 @@ describe('CachedUserInfoService', () => { const delegate2: UserInfoService = { getUserInfo: jest.fn().mockResolvedValue(aliceInfo), }; - const entries = new Map(); + const entries = new Map(); const service1 = new CachedUserInfoService(delegate1, { entries }); const service2 = new CachedUserInfoService(delegate2, { entries }); const creds = mockCredentials.user();