diff --git a/.changeset/revert-dcr-token-expiration.md b/.changeset/revert-dcr-token-expiration.md new file mode 100644 index 0000000000..2e7c854d46 --- /dev/null +++ b/.changeset/revert-dcr-token-expiration.md @@ -0,0 +1,13 @@ +--- +'@backstage/plugin-auth-backend': patch +--- + +Removed the `auth.experimentalDynamicClientRegistration.tokenExpiration` config option. DCR tokens now use the default 1 hour expiration. + +If you need longer-lived access, use refresh tokens via the `offline_access` scope instead. DCR clients should already have the `offline_access` scope available. Enable refresh tokens by setting: + +```yaml +auth: + experimentalRefreshToken: + enabled: true +``` diff --git a/plugins/auth-backend/config.d.ts b/plugins/auth-backend/config.d.ts index e3e0c46b2f..9326394a2a 100644 --- a/plugins/auth-backend/config.d.ts +++ b/plugins/auth-backend/config.d.ts @@ -95,7 +95,6 @@ export interface Config { /** * The backstage token expiration. - * Defaults to 1 hour (3600s). Maximum allowed is 24 hours. */ backstageTokenExpiration?: HumanDuration | string; @@ -150,12 +149,6 @@ export interface Config { * dynamic client registration. Defaults to '[*]' which allows any redirect URI. */ allowedRedirectUriPatterns?: string[]; - - /** - * The expiration time for the client registration access tokens. - * Defaults to 1 hour (3600s). Maximum allowed is 24 hours. - */ - tokenExpiration?: HumanDuration | string; }; /** diff --git a/plugins/auth-backend/src/service/OidcRouter.ts b/plugins/auth-backend/src/service/OidcRouter.ts index 2a651f47d3..746036806c 100644 --- a/plugins/auth-backend/src/service/OidcRouter.ts +++ b/plugins/auth-backend/src/service/OidcRouter.ts @@ -27,7 +27,6 @@ import { UserInfoDatabase } from '../database/UserInfoDatabase'; import { OidcDatabase } from '../database/OidcDatabase'; import { OfflineAccessService } from './OfflineAccessService'; import { json } from 'express'; -import { readDcrTokenExpiration } from './readTokenExpiration'; import { z } from 'zod'; import { fromZodError } from 'zod-validation-error'; import { OidcError } from './OidcError'; @@ -397,8 +396,6 @@ export class OidcRouter { client_secret: bodyClientSecret, } = validateRequest(tokenRequestBodySchema, req.body); - const expiresIn = readDcrTokenExpiration(this.config); - try { // Handle authorization_code grant type if (grantType === 'authorization_code') { @@ -415,7 +412,6 @@ export class OidcRouter { redirectUri, codeVerifier, grantType, - expiresIn, }); return res.json({ diff --git a/plugins/auth-backend/src/service/OidcService.test.ts b/plugins/auth-backend/src/service/OidcService.test.ts index 903e352471..78c5d88830 100644 --- a/plugins/auth-backend/src/service/OidcService.test.ts +++ b/plugins/auth-backend/src/service/OidcService.test.ts @@ -712,7 +712,6 @@ describe('OidcService', () => { code, redirectUri: 'https://example.com/callback', grantType: 'authorization_code', - expiresIn: 3600, }); expect(tokenResult).toEqual({ @@ -724,46 +723,6 @@ describe('OidcService', () => { }); }); - it('should exchange valid code for tokens with custom expiration', async () => { - const { service, mocks } = await createOidcService({ databaseId }); - const mockToken = 'mock-jwt-token'; - mocks.tokenIssuer.issueToken.mockResolvedValue({ token: mockToken }); - - const client = await service.registerClient({ - clientName: 'Test Client', - redirectUris: ['https://example.com/callback'], - }); - - const authSession = await service.createAuthorizationSession({ - clientId: client.clientId, - redirectUri: 'https://example.com/callback', - responseType: 'code', - scope: 'openid', - }); - - const authResult = await service.approveAuthorizationSession({ - sessionId: authSession.id, - userEntityRef: 'user:default/test', - }); - - const code = new URL(authResult.redirectUrl).searchParams.get('code')!; - - const tokenResult = await service.exchangeCodeForToken({ - code, - redirectUri: 'https://example.com/callback', - grantType: 'authorization_code', - expiresIn: 6000, - }); - - expect(tokenResult).toEqual({ - accessToken: mockToken, - tokenType: 'Bearer', - expiresIn: 6000, - idToken: mockToken, - scope: 'openid', - }); - }); - it('should throw error for invalid grant type', async () => { const { service } = await createOidcService({ databaseId }); @@ -772,7 +731,6 @@ describe('OidcService', () => { code: 'test-code', redirectUri: 'https://example.com/callback', grantType: 'client_credentials', - expiresIn: 3600, }), ).rejects.toThrow('Unsupported grant type'); }); @@ -813,7 +771,6 @@ describe('OidcService', () => { redirectUri: 'https://example.com/callback', grantType: 'authorization_code', codeVerifier, - expiresIn: 3600, }); expect(tokenResult.accessToken).toBe(mockToken); @@ -849,7 +806,6 @@ describe('OidcService', () => { redirectUri: 'https://example.com/callback', grantType: 'authorization_code', codeVerifier: 'invalid-verifier', - expiresIn: 3600, }), ).rejects.toThrow('Invalid code verifier'); }); @@ -1205,7 +1161,6 @@ describe('OidcService', () => { redirectUri: 'http://localhost:8080/callback', grantType: 'authorization_code', codeVerifier: pkceCodeVerifier, - expiresIn: 3600, }); expect(tokenResult).toEqual({ @@ -1259,7 +1214,6 @@ describe('OidcService', () => { redirectUri: 'http://localhost:8080/callback', grantType: 'authorization_code', codeVerifier, - expiresIn: 3600, }); expect(tokenResult.accessToken).toBe(mockToken); diff --git a/plugins/auth-backend/src/service/OidcService.ts b/plugins/auth-backend/src/service/OidcService.ts index 675dbcfd55..a48a38bfb2 100644 --- a/plugins/auth-backend/src/service/OidcService.ts +++ b/plugins/auth-backend/src/service/OidcService.ts @@ -27,7 +27,6 @@ import { OidcDatabase } from '../database/OidcDatabase'; import { DateTime } from 'luxon'; import matcher from 'matcher'; import { OfflineAccessService } from './OfflineAccessService'; -import { readDcrTokenExpiration } from './readTokenExpiration'; import { validateCimdUrl, fetchCimdMetadata } from './CimdClient'; export class OidcService { @@ -439,9 +438,8 @@ export class OidcService { redirectUri: string; codeVerifier?: string; grantType: string; - expiresIn: number; }) { - const { code, redirectUri, codeVerifier, grantType, expiresIn } = params; + const { code, redirectUri, codeVerifier, grantType } = params; if (grantType !== 'authorization_code') { throw new InputError('Unsupported grant type'); @@ -520,7 +518,7 @@ export class OidcService { return { accessToken: token, tokenType: 'Bearer', - expiresIn: expiresIn, + expiresIn: 3600, idToken: token, scope: session.scope || 'openid', refreshToken, @@ -547,12 +545,10 @@ export class OidcService { clientId: params.clientId, }); - const expiresIn = readDcrTokenExpiration(this.config); - return { accessToken, tokenType: 'Bearer', - expiresIn, + expiresIn: 3600, refreshToken, }; } diff --git a/plugins/auth-backend/src/service/readTokenExpiration.test.ts b/plugins/auth-backend/src/service/readTokenExpiration.test.ts index 69b6bbf848..56e711a944 100644 --- a/plugins/auth-backend/src/service/readTokenExpiration.test.ts +++ b/plugins/auth-backend/src/service/readTokenExpiration.test.ts @@ -15,10 +15,7 @@ */ import { ConfigReader } from '@backstage/config'; -import { - readBackstageTokenExpiration, - readTokenExpiration, -} from './readTokenExpiration.ts'; +import { readBackstageTokenExpiration } from './readTokenExpiration'; describe('Test for default backstage token expiry time', () => { it('Will return default backstage session expiration', () => { @@ -77,57 +74,4 @@ describe('Test for default backstage token expiry time', () => { }); expect(readBackstageTokenExpiration(config)).toBe(86400); }); - - it('will return expiration from custom key', () => { - const config = new ConfigReader({ - app: { - baseUrl: 'http://example.com/extra-path', - }, - custom: { - tokenExp: { minutes: 20 }, - }, - }); - expect(readTokenExpiration(config, { configKey: 'custom.tokenExp' })).toBe( - 1200, - ); - }); - - it('will return custom default expiration', () => { - const config = new ConfigReader({}); - expect( - readTokenExpiration(config, { - configKey: 'auth.backstageTokenExpiration', - defaultExpiration: 1234, - }), - ).toBe(1234); - }); - - it('will return custom min/max expiration', () => { - const config = new ConfigReader({ - auth: { - backstageTokenExpiration: { minutes: 20 }, - }, - }); - expect( - readTokenExpiration(config, { - configKey: 'auth.backstageTokenExpiration', - minExpiration: 2000, - maxExpiration: 3000, - }), - ).toBe(2000); - expect( - readTokenExpiration(config, { - configKey: 'auth.backstageTokenExpiration', - minExpiration: 1000, - maxExpiration: 1100, - }), - ).toBe(1100); - expect( - readTokenExpiration(config, { - configKey: 'auth.backstageTokenExpiration', - minExpiration: 1000, - maxExpiration: 2000, - }), - ).toBe(1200); - }); }); diff --git a/plugins/auth-backend/src/service/readTokenExpiration.ts b/plugins/auth-backend/src/service/readTokenExpiration.ts index 3094d2c054..c687ecdfc5 100644 --- a/plugins/auth-backend/src/service/readTokenExpiration.ts +++ b/plugins/auth-backend/src/service/readTokenExpiration.ts @@ -23,46 +23,22 @@ const TOKEN_EXP_MIN_S = 600; const TOKEN_EXP_MAX_S = 86400; export function readBackstageTokenExpiration(config: RootConfigService) { - return readTokenExpiration(config, { - configKey: 'auth.backstageTokenExpiration', - }); -} + const processingIntervalKey = 'auth.backstageTokenExpiration'; -export function readDcrTokenExpiration(config: RootConfigService) { - return readTokenExpiration(config, { - configKey: 'auth.experimentalDynamicClientRegistration.tokenExpiration', - }); -} - -export function readTokenExpiration( - config: RootConfigService, - options: { - configKey: string; - maxExpiration?: number; - minExpiration?: number; - defaultExpiration?: number; - }, -): number { - const { - configKey, - maxExpiration = TOKEN_EXP_MAX_S, - minExpiration = TOKEN_EXP_MIN_S, - defaultExpiration = TOKEN_EXP_DEFAULT_S, - } = options ?? {}; - if (!config.has(configKey)) { - return defaultExpiration; + if (!config.has(processingIntervalKey)) { + return TOKEN_EXP_DEFAULT_S; } const duration = readDurationFromConfig(config, { - key: configKey, + key: processingIntervalKey, }); const durationS = Math.round(durationToMilliseconds(duration) / 1000); - if (durationS < minExpiration) { - return minExpiration; - } else if (durationS > maxExpiration) { - return maxExpiration; + if (durationS < TOKEN_EXP_MIN_S) { + return TOKEN_EXP_MIN_S; + } else if (durationS > TOKEN_EXP_MAX_S) { + return TOKEN_EXP_MAX_S; } return durationS; } diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index ec5b303710..8a8118c43e 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -35,10 +35,8 @@ import session from 'express-session'; import connectSessionKnex from 'connect-session-knex'; import passport from 'passport'; import { AuthDatabase } from '../database/AuthDatabase'; -import { - readBackstageTokenExpiration, - readDcrTokenExpiration, -} from './readTokenExpiration.ts'; +import { readBackstageTokenExpiration } from './readTokenExpiration'; +import { TokenIssuer } from '../identity/types'; import { StaticTokenIssuer } from '../identity/StaticTokenIssuer'; import { StaticKeyStore } from '../identity/StaticKeyStore'; import { bindProviderRouters, ProviderFactories } from '../providers/router'; @@ -95,37 +93,29 @@ export async function createRouter( ? ['ent'] : []; - const createTokenIssuer = (opts: { - logger: LoggerService; - expirationSeconds: number; - }) => { - if (keyStore instanceof StaticKeyStore) { - return new StaticTokenIssuer( - { - logger: opts.logger, - issuer: authUrl, - sessionExpirationSeconds: opts.expirationSeconds, - omitClaimsFromToken, - }, - keyStore as StaticKeyStore, - ); - } - return new TokenFactory({ + let tokenIssuer: TokenIssuer; + if (keyStore instanceof StaticKeyStore) { + tokenIssuer = new StaticTokenIssuer( + { + logger: logger.child({ component: 'token-factory' }), + issuer: authUrl, + sessionExpirationSeconds: backstageTokenExpiration, + omitClaimsFromToken, + }, + keyStore as StaticKeyStore, + ); + } else { + tokenIssuer = new TokenFactory({ issuer: authUrl, keyStore, - keyDurationSeconds: opts.expirationSeconds, - logger: opts.logger, + keyDurationSeconds: backstageTokenExpiration, + logger: logger.child({ component: 'token-factory' }), algorithm: tokenFactoryAlgorithm ?? config.getOptionalString('auth.identityTokenAlgorithm'), omitClaimsFromToken, }); - }; - - const tokenIssuer = createTokenIssuer({ - logger: logger.child({ component: 'token-factory' }), - expirationSeconds: backstageTokenExpiration, - }); + } const secret = config.getOptionalString('auth.session.secret'); if (secret) { @@ -163,18 +153,11 @@ export async function createRouter( userInfo, }); - const dcrTokenExpiration = readDcrTokenExpiration(config); - - const oidcTokenIssuer = createTokenIssuer({ - logger: logger.child({ component: 'oidc-token-factory' }), - expirationSeconds: dcrTokenExpiration, - }); - const oidc = await OidcDatabase.create({ database }); const oidcRouter = OidcRouter.create({ auth: options.auth, - tokenIssuer: oidcTokenIssuer, + tokenIssuer, baseUrl: authUrl, appUrl, userInfo,