From 18619f793c949f70e13fed1266cfd4f73edc0753 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 18 Aug 2023 17:14:05 +0200 Subject: [PATCH] auth-backend: track backstage session expiration separately Signed-off-by: Patrik Oldsberg --- .changeset/fresh-candles-warn.md | 5 ++ .changeset/great-chairs-swim.md | 5 ++ .changeset/lovely-years-sniff.md | 5 ++ .changeset/popular-readers-allow.md | 5 ++ packages/core-app-api/api-report.md | 4 +- .../auth/oauth2/OAuth2.test.ts | 31 +++++++++++ .../implementations/auth/oauth2/OAuth2.ts | 55 +++++++++++++++---- .../apis/implementations/auth/oauth2/types.ts | 4 +- packages/core-plugin-api/api-report.md | 1 + .../src/apis/definitions/auth.ts | 5 ++ .../src/lib/legacy/adaptLegacyOAuthHandler.ts | 2 +- .../legacy/adaptLegacyOAuthSignInResolver.ts | 2 +- .../src/providers/microsoft/provider.test.ts | 29 +++++++--- plugins/auth-node/api-report.md | 3 +- .../prepareBackstageIdentityResponse.test.ts | 41 ++++++++++++++ .../prepareBackstageIdentityResponse.ts | 18 +++++- plugins/auth-node/src/oauth/types.ts | 2 +- plugins/auth-node/src/types.ts | 5 ++ 18 files changed, 193 insertions(+), 29 deletions(-) create mode 100644 .changeset/fresh-candles-warn.md create mode 100644 .changeset/great-chairs-swim.md create mode 100644 .changeset/lovely-years-sniff.md create mode 100644 .changeset/popular-readers-allow.md diff --git a/.changeset/fresh-candles-warn.md b/.changeset/fresh-candles-warn.md new file mode 100644 index 0000000000..49ca2e5402 --- /dev/null +++ b/.changeset/fresh-candles-warn.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-node': patch +--- + +The `BackstageIdentityResponse` interface now has an optional `expiresInSeconds` field that can be used to signal session expiration. The `prepareBackstageIdentityResponse` utility will now also read the expiration from the provided token, and include it in the response. diff --git a/.changeset/great-chairs-swim.md b/.changeset/great-chairs-swim.md new file mode 100644 index 0000000000..e6b8ab0882 --- /dev/null +++ b/.changeset/great-chairs-swim.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-app-api': minor +--- + +Fixed two bugs in how the `OAuth2Session` type represents the underlying data. The `expiresAt` and `backstageIdentity` are now both optional, since that's what they are in practice. This is not considered a breaking change since it was effectively a bug in the modelling of the state that this type represents, and the type was not used in any other external contract. diff --git a/.changeset/lovely-years-sniff.md b/.changeset/lovely-years-sniff.md new file mode 100644 index 0000000000..09b86f7112 --- /dev/null +++ b/.changeset/lovely-years-sniff.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-app-api': minor +--- + +The `OAuth` class which is used by all OAuth providers will now consider both the session expiration of both the Backstage identity as well as the upstream identity provider, and refresh the session with either of them is about to expire. diff --git a/.changeset/popular-readers-allow.md b/.changeset/popular-readers-allow.md new file mode 100644 index 0000000000..b89e0c264e --- /dev/null +++ b/.changeset/popular-readers-allow.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-plugin-api': minor +--- + +Added the optional `expiresAt` field that may now be part of a `BackstageIdentityResponse`. diff --git a/packages/core-app-api/api-report.md b/packages/core-app-api/api-report.md index 70e45dd108..8b344e748c 100644 --- a/packages/core-app-api/api-report.md +++ b/packages/core-app-api/api-report.md @@ -538,10 +538,10 @@ export type OAuth2Session = { idToken: string; accessToken: string; scopes: Set; - expiresAt: Date; + expiresAt?: Date; }; profile: ProfileInfo; - backstageIdentity: BackstageIdentityResponse; + backstageIdentity?: BackstageIdentityResponse; }; // @public diff --git a/packages/core-app-api/src/apis/implementations/auth/oauth2/OAuth2.test.ts b/packages/core-app-api/src/apis/implementations/auth/oauth2/OAuth2.test.ts index 6f941379a7..8ca818ebd8 100644 --- a/packages/core-app-api/src/apis/implementations/auth/oauth2/OAuth2.test.ts +++ b/packages/core-app-api/src/apis/implementations/auth/oauth2/OAuth2.test.ts @@ -76,6 +76,37 @@ describe('OAuth2', () => { ); }); + it('should forward backstage identity', async () => { + getSession = jest.fn().mockResolvedValue({ + providerInfo: { accessToken: 'access-token', expiresAt: theFuture }, + backstageIdentity: { + token: 'a.b.c', + expiresAt: theFuture, + identity: { + type: 'user', + userEntityRef: 'user:default/mock', + ownershipEntityRefs: [], + }, + }, + }); + const oauth2 = OAuth2.create({ + configApi: configApi, + scopeTransform: scopes => scopes.map(scope => `my-prefix/${scope}`), + oauthRequestApi: new MockOAuthApi(), + discoveryApi: UrlPatternDiscovery.compile('http://example.com'), + }); + + await expect(oauth2.getBackstageIdentity()).resolves.toEqual({ + token: 'a.b.c', + expiresAt: theFuture, + identity: { + type: 'user', + userEntityRef: 'user:default/mock', + ownershipEntityRefs: [], + }, + }); + }); + it('should get refreshed id token', async () => { getSession = jest.fn().mockResolvedValue({ providerInfo: { idToken: 'id-token', expiresAt: theFuture }, diff --git a/packages/core-app-api/src/apis/implementations/auth/oauth2/OAuth2.ts b/packages/core-app-api/src/apis/implementations/auth/oauth2/OAuth2.ts index fd40a9c8fb..6be7cfc89d 100644 --- a/packages/core-app-api/src/apis/implementations/auth/oauth2/OAuth2.ts +++ b/packages/core-app-api/src/apis/implementations/auth/oauth2/OAuth2.ts @@ -30,6 +30,7 @@ import { SessionState, SessionApi, BackstageIdentityApi, + BackstageUserIdentity, } from '@backstage/core-plugin-api'; import { Observable } from '@backstage/types'; import { OAuth2Session } from './types'; @@ -49,10 +50,14 @@ export type OAuth2Response = { accessToken: string; idToken: string; scope: string; - expiresInSeconds: number; + expiresInSeconds?: number; }; profile: ProfileInfo; - backstageIdentity: BackstageIdentityResponse; + backstageIdentity: { + token: string; + expiresInSeconds?: number; + identity: BackstageUserIdentity; + }; }; const DEFAULT_PROVIDER = { @@ -92,8 +97,11 @@ export default class OAuth2 environment, provider, oauthRequestApi: oauthRequestApi, - sessionTransform(res: OAuth2Response): OAuth2Session { - return { + sessionTransform({ + backstageIdentity, + ...res + }: OAuth2Response): OAuth2Session { + const session: OAuth2Session = { ...res, providerInfo: { idToken: res.providerInfo.idToken, @@ -102,11 +110,26 @@ export default class OAuth2 scopeTransform, res.providerInfo.scope, ), - expiresAt: new Date( - Date.now() + res.providerInfo.expiresInSeconds * 1000, - ), + expiresAt: res.providerInfo.expiresInSeconds + ? new Date(Date.now() + res.providerInfo.expiresInSeconds * 1000) + : undefined, }, }; + if (backstageIdentity) { + // TODO(Rugvip): This fallback can be removed a few releases after 1.18. It's there to avoid + // breaking deployments that update their frontend before updating their backend. + const expInSec = + backstageIdentity.expiresInSeconds ?? + res.providerInfo.expiresInSeconds; + session.backstageIdentity = { + token: backstageIdentity.token, + identity: backstageIdentity.identity, + expiresAt: expInSec + ? new Date(Date.now() + expInSec * 1000) + : undefined, + }; + } + return session; }, popupOptions, }); @@ -116,9 +139,21 @@ export default class OAuth2 defaultScopes: new Set(defaultScopes), sessionScopes: (session: OAuth2Session) => session.providerInfo.scopes, sessionShouldRefresh: (session: OAuth2Session) => { - const expiresInSec = - (session.providerInfo.expiresAt.getTime() - Date.now()) / 1000; - return expiresInSec < 60 * 5; + // TODO(Rugvip): Optimize to use separate checks for provider vs backstage session expiration + let min = Infinity; + if (session.providerInfo?.expiresAt) { + min = Math.min( + min, + (session.providerInfo.expiresAt.getTime() - Date.now()) / 1000, + ); + } + if (session.backstageIdentity?.expiresAt) { + min = Math.min( + min, + (session.backstageIdentity.expiresAt.getTime() - Date.now()) / 1000, + ); + } + return min < 60 * 5; }, }); diff --git a/packages/core-app-api/src/apis/implementations/auth/oauth2/types.ts b/packages/core-app-api/src/apis/implementations/auth/oauth2/types.ts index bd3bb95ba6..e51de63dbe 100644 --- a/packages/core-app-api/src/apis/implementations/auth/oauth2/types.ts +++ b/packages/core-app-api/src/apis/implementations/auth/oauth2/types.ts @@ -31,8 +31,8 @@ export type OAuth2Session = { idToken: string; accessToken: string; scopes: Set; - expiresAt: Date; + expiresAt?: Date; }; profile: ProfileInfo; - backstageIdentity: BackstageIdentityResponse; + backstageIdentity?: BackstageIdentityResponse; }; diff --git a/packages/core-plugin-api/api-report.md b/packages/core-plugin-api/api-report.md index 5ee7104d68..930f1f979d 100644 --- a/packages/core-plugin-api/api-report.md +++ b/packages/core-plugin-api/api-report.md @@ -214,6 +214,7 @@ export type BackstageIdentityApi = { // @public export type BackstageIdentityResponse = { token: string; + expiresAt?: Date; identity: BackstageUserIdentity; }; diff --git a/packages/core-plugin-api/src/apis/definitions/auth.ts b/packages/core-plugin-api/src/apis/definitions/auth.ts index 62d0d5c810..d9aec2ca5a 100644 --- a/packages/core-plugin-api/src/apis/definitions/auth.ts +++ b/packages/core-plugin-api/src/apis/definitions/auth.ts @@ -226,6 +226,11 @@ export type BackstageIdentityResponse = { */ token: string; + /** + * The time at which the token expires. If not set, it can be assumed that the token does not expire. + */ + expiresAt?: Date; + /** * Identity information derived from the token. */ diff --git a/plugins/auth-backend/src/lib/legacy/adaptLegacyOAuthHandler.ts b/plugins/auth-backend/src/lib/legacy/adaptLegacyOAuthHandler.ts index 37f10ff184..3b8eca0a95 100644 --- a/plugins/auth-backend/src/lib/legacy/adaptLegacyOAuthHandler.ts +++ b/plugins/auth-backend/src/lib/legacy/adaptLegacyOAuthHandler.ts @@ -37,7 +37,7 @@ export function adaptLegacyOAuthHandler( scope: result.session.scope, id_token: result.session.idToken, token_type: result.session.tokenType, - expires_in: result.session.expiresInSeconds, + expires_in: result.session.expiresInSeconds!, }, }, ctx, diff --git a/plugins/auth-backend/src/lib/legacy/adaptLegacyOAuthSignInResolver.ts b/plugins/auth-backend/src/lib/legacy/adaptLegacyOAuthSignInResolver.ts index e0318464ed..e671a62d52 100644 --- a/plugins/auth-backend/src/lib/legacy/adaptLegacyOAuthSignInResolver.ts +++ b/plugins/auth-backend/src/lib/legacy/adaptLegacyOAuthSignInResolver.ts @@ -39,7 +39,7 @@ export function adaptLegacyOAuthSignInResolver( scope: input.result.session.scope, id_token: input.result.session.idToken, token_type: input.result.session.tokenType, - expires_in: input.result.session.expiresInSeconds, + expires_in: input.result.session.expiresInSeconds!, }, }, }, diff --git a/plugins/auth-backend/src/providers/microsoft/provider.test.ts b/plugins/auth-backend/src/providers/microsoft/provider.test.ts index a1128ee6b9..7bbfd085d6 100644 --- a/plugins/auth-backend/src/providers/microsoft/provider.test.ts +++ b/plugins/auth-backend/src/providers/microsoft/provider.test.ts @@ -30,6 +30,10 @@ describe('MicrosoftAuthProvider', () => { const state = Buffer.from( `nonce=${encodeURIComponent(nonce)}&env=development`, ).toString('hex'); + const mockBackstageToken = `header.${Buffer.from( + JSON.stringify({ sub: 'user:default/mock' }), + 'utf8', + ).toString('base64')}.backstage`; const server = setupServer(); const microsoftApi = new FakeMicrosoftAPI(); @@ -64,10 +68,9 @@ describe('MicrosoftAuthProvider', () => { resolverContext: { issueToken: jest.fn(), findCatalogUser: jest.fn(), - signInWithCatalogUser: _ => - Promise.resolve({ - token: 'header.e30K.backstage', - }), + signInWithCatalogUser: async _ => ({ + token: mockBackstageToken, + }), } as AuthResolverContext, }) as AuthProviderRouteHandlers; @@ -209,8 +212,12 @@ describe('MicrosoftAuthProvider', () => { displayName: 'Conrad', }, backstageIdentity: { - token: 'header.e30K.backstage', - identity: { type: 'user', ownershipEntityRefs: [] }, + token: mockBackstageToken, + identity: { + type: 'user', + userEntityRef: 'user:default/mock', + ownershipEntityRefs: [], + }, }, }, }), @@ -330,8 +337,12 @@ describe('MicrosoftAuthProvider', () => { displayName: 'Conrad', }, backstageIdentity: { - token: 'header.e30K.backstage', - identity: { type: 'user', ownershipEntityRefs: [] }, + token: mockBackstageToken, + identity: { + type: 'user', + userEntityRef: 'user:default/mock', + ownershipEntityRefs: [], + }, }, }, }), @@ -430,7 +441,7 @@ describe('MicrosoftAuthProvider', () => { expect(response.json).toHaveBeenCalledWith( expect.objectContaining({ backstageIdentity: expect.objectContaining({ - token: 'header.e30K.backstage', + token: mockBackstageToken, }), }), ); diff --git a/plugins/auth-node/api-report.md b/plugins/auth-node/api-report.md index a7fc9b1612..bc17c328d4 100644 --- a/plugins/auth-node/api-report.md +++ b/plugins/auth-node/api-report.md @@ -100,6 +100,7 @@ export type AuthResolverContext = { // @public export interface BackstageIdentityResponse extends BackstageSignInResult { + expiresInSeconds?: number; identity: BackstageUserIdentity; } @@ -377,7 +378,7 @@ export interface OAuthSession { // (undocumented) accessToken: string; // (undocumented) - expiresInSeconds: number; + expiresInSeconds?: number; // (undocumented) idToken?: string; // (undocumented) diff --git a/plugins/auth-node/src/identity/prepareBackstageIdentityResponse.test.ts b/plugins/auth-node/src/identity/prepareBackstageIdentityResponse.test.ts index dac06f964e..d37224a11d 100644 --- a/plugins/auth-node/src/identity/prepareBackstageIdentityResponse.test.ts +++ b/plugins/auth-node/src/identity/prepareBackstageIdentityResponse.test.ts @@ -23,7 +23,37 @@ function mkToken(payload: unknown) { } describe('prepareBackstageIdentityResponse', () => { + afterEach(jest.resetAllMocks); + it('parses a complete token to determine the identity', () => { + jest.spyOn(Date, 'now').mockReturnValue(5000); + + const token = mkToken({ sub: 'k:ns/n', ent: ['k:ns/o'], exp: 1005 }); + expect( + prepareBackstageIdentityResponse({ + token, + }), + ).toEqual({ + token, + expiresInSeconds: 1000, + identity: { + type: 'user', + userEntityRef: 'k:ns/n', + ownershipEntityRefs: ['k:ns/o'], + }, + }); + }); + + it('should reject tokens without subject', () => { + const token = mkToken({}); + expect(() => + prepareBackstageIdentityResponse({ + token, + }), + ).toThrow('Identity response must return a token with subject claim'); + }); + + it('should treat expiration as optional', () => { const token = mkToken({ sub: 'k:ns/n', ent: ['k:ns/o'] }); expect( prepareBackstageIdentityResponse({ @@ -38,4 +68,15 @@ describe('prepareBackstageIdentityResponse', () => { }, }); }); + + it('should reject tokens with negative expiration', () => { + jest.spyOn(Date, 'now').mockReturnValue(5000); + + const token = mkToken({ sub: 'k:ns/n', ent: ['k:ns/o'], exp: 1 }); + expect(() => + prepareBackstageIdentityResponse({ + token, + }), + ).toThrow('Identity response must not return an expired token'); + }); }); diff --git a/plugins/auth-node/src/identity/prepareBackstageIdentityResponse.ts b/plugins/auth-node/src/identity/prepareBackstageIdentityResponse.ts index e2753d6648..33e9bafd0d 100644 --- a/plugins/auth-node/src/identity/prepareBackstageIdentityResponse.ts +++ b/plugins/auth-node/src/identity/prepareBackstageIdentityResponse.ts @@ -39,14 +39,28 @@ export function prepareBackstageIdentityResponse( throw new InputError(`Identity response must return a token`); } - const { sub, ent } = parseJwtPayload(result.token); + const { sub, ent = [], exp: expStr } = parseJwtPayload(result.token); + if (!sub) { + throw new InputError( + `Identity response must return a token with subject claim`, + ); + } + + const expAt = Number(expStr); + + // Default to 1 hour if no expiration is set, in particular to make testing simpler + const exp = expAt ? Math.round(expAt - Date.now() / 1000) : undefined; + if (exp && exp < 0) { + throw new InputError(`Identity response must not return an expired token`); + } return { ...result, + expiresInSeconds: exp, identity: { type: 'user', userEntityRef: sub, - ownershipEntityRefs: ent ?? [], + ownershipEntityRefs: ent, }, }; } diff --git a/plugins/auth-node/src/oauth/types.ts b/plugins/auth-node/src/oauth/types.ts index c552de394e..bba961d27d 100644 --- a/plugins/auth-node/src/oauth/types.ts +++ b/plugins/auth-node/src/oauth/types.ts @@ -24,7 +24,7 @@ export interface OAuthSession { tokenType: string; idToken?: string; scope: string; - expiresInSeconds: number; + expiresInSeconds?: number; refreshToken?: string; } diff --git a/plugins/auth-node/src/types.ts b/plugins/auth-node/src/types.ts index d52abf00fb..d4cb0ac48e 100644 --- a/plugins/auth-node/src/types.ts +++ b/plugins/auth-node/src/types.ts @@ -43,6 +43,11 @@ export interface BackstageSignInResult { * @public */ export interface BackstageIdentityResponse extends BackstageSignInResult { + /** + * The number of seconds until the token expires. If not set, it can be assumed that the token does not expire. + */ + expiresInSeconds?: number; + /** * A plaintext description of the identity that is encapsulated within the token. */