diff --git a/.changeset/rotten-cats-matter.md b/.changeset/rotten-cats-matter.md new file mode 100644 index 0000000000..2515db9991 --- /dev/null +++ b/.changeset/rotten-cats-matter.md @@ -0,0 +1,19 @@ +--- +'@backstage/core-app-api': minor +--- + +`OAuth2` now gets ID tokens from a session with the `openid` scope explicitly +requested. + +This should not be considered a breaking change, because spec-compliant OIDC +providers will already be returning ID tokens if and only if the `openid` scope +is granted. + +This change makes the dependence explicit, and removes the burden on +OAuth2-based providers which require an ID token (e.g. this is done by various +default [auth +handlers](https://backstage.io/docs/auth/identity-resolver/#authhandler)) to add +`openid` to their default scopes. _That_ could carry another indirect benefit: +by removing `openid` from the default scopes for a provider, grants for +resource-specific access tokens can avoid requesting excess ID token-related +scopes. 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 0bb40c23fc..63070120ef 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 @@ -48,9 +48,8 @@ describe('OAuth2', () => { expect(await oauth2.getAccessToken('my-scope my-scope2')).toBe( 'access-token', ); - expect(getSession).toHaveBeenCalledTimes(1); - expect(getSession.mock.calls[0][0].scopes).toEqual( - new Set(['my-scope', 'my-scope2']), + expect(getSession).toHaveBeenCalledWith( + expect.objectContaining({ scopes: new Set(['my-scope', 'my-scope2']) }), ); }); @@ -65,9 +64,10 @@ describe('OAuth2', () => { }); expect(await oauth2.getAccessToken('my-scope')).toBe('access-token'); - expect(getSession).toHaveBeenCalledTimes(1); - expect(getSession.mock.calls[0][0].scopes).toEqual( - new Set(['my-prefix/my-scope']), + expect(getSession).toHaveBeenCalledWith( + expect.objectContaining({ + scopes: new Set(['my-prefix/my-scope']), + }), ); }); @@ -82,7 +82,11 @@ describe('OAuth2', () => { }); expect(await oauth2.getIdToken()).toBe('id-token'); - expect(getSession).toHaveBeenCalledTimes(1); + expect(getSession).toHaveBeenCalledWith( + expect.objectContaining({ + scopes: new Set(['openid']), + }), + ); }); it('should get optional id token', async () => { @@ -96,7 +100,11 @@ describe('OAuth2', () => { }); expect(await oauth2.getIdToken({ optional: true })).toBe('id-token'); - expect(getSession).toHaveBeenCalledTimes(1); + expect(getSession).toHaveBeenCalledWith( + expect.objectContaining({ + scopes: new Set(['openid']), + }), + ); }); it('should share popup closed errors', async () => { 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 ad2d04cb56..6f88178415 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 @@ -153,7 +153,10 @@ export default class OAuth2 } async getIdToken(options: AuthRequestOptions = {}) { - const session = await this.sessionManager.getSession(options); + const session = await this.sessionManager.getSession({ + ...options, + scopes: new Set(['openid']), + }); return session?.providerInfo.idToken ?? ''; }