diff --git a/plugins/auth-node/src/oauth/CookieScopeManager.test.ts b/plugins/auth-node/src/oauth/CookieScopeManager.test.ts new file mode 100644 index 0000000000..82eeb902d8 --- /dev/null +++ b/plugins/auth-node/src/oauth/CookieScopeManager.test.ts @@ -0,0 +1,236 @@ +/* + * Copyright 2024 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 express from 'express'; +import { + OAuthAuthenticator, + OAuthAuthenticatorResult, + OAuthAuthenticatorScopeOptions, +} from './types'; +import { OAuthCookieManager } from './OAuthCookieManager'; +import { OAuthState } from './state'; +import { CookieScopeManager } from './CookieScopeManager'; + +function makeReq(scope?: string): express.Request { + return { + query: { scope }, + res: {}, + get: _name => 'https://example.com', + } as express.Request; +} + +describe('CookieScopeManager', () => { + it('should work with minimal config', async () => { + const manager = CookieScopeManager.create({ + authenticator: {} as OAuthAuthenticator, + cookieManager: {} as OAuthCookieManager, + }); + + await expect(manager.start(makeReq())).resolves.toEqual({ + scope: '', + }); + await expect(manager.start(makeReq('x'))).resolves.toEqual({ + scope: 'x', + }); + await expect(manager.start(makeReq('x,y'))).resolves.toEqual({ + scope: 'x y', + }); + + await expect( + manager.handleCallback(makeReq(), { + result: { session: { scope: 'x,y' } } as OAuthAuthenticatorResult, + state: {} as OAuthState, + }), + ).resolves.toEqual('x y'); + + await expect(manager.clear(makeReq())).resolves.toBe(undefined); + + const refresh = await manager.refresh(makeReq('x,y')); + expect(refresh.scope).toBe('x y'); + + await expect( + refresh.commit({ + session: { scope: 'y,z' }, + } as OAuthAuthenticatorResult), + ).resolves.toEqual('y z'); + }); + + it('should include additional scopes', async () => { + const manager = CookieScopeManager.create({ + additionalScopes: ['a', 'b'], + authenticator: {} as OAuthAuthenticator, + cookieManager: {} as OAuthCookieManager, + }); + + await expect(manager.start(makeReq())).resolves.toEqual({ + scope: 'a b', + }); + await expect(manager.start(makeReq('x'))).resolves.toEqual({ + scope: 'x a b', + }); + await expect(manager.start(makeReq('x,y'))).resolves.toEqual({ + scope: 'x y a b', + }); + + const refresh = await manager.refresh(makeReq('x|y')); + expect(refresh.scope).toBe('x y a b'); + + await expect( + refresh.commit({ + session: { scope: 'y,z a' }, + } as OAuthAuthenticatorResult), + ).resolves.toEqual('y z a'); + }); + + it('should persist scopes', async () => { + const setGrantedScopes = jest.fn(); + const removeGrantedScopes = jest.fn(); + const manager = CookieScopeManager.create({ + authenticator: { + scopes: { + persist: true, + } as OAuthAuthenticatorScopeOptions, + } as OAuthAuthenticator, + cookieManager: { + getGrantedScopes: () => 'g', + setGrantedScopes, + removeGrantedScopes, + } as unknown as OAuthCookieManager, + }); + + await expect(manager.start(makeReq())).resolves.toEqual({ + scope: 'g', + scopeState: { + scope: 'g', + }, + }); + await expect(manager.start(makeReq('x'))).resolves.toEqual({ + scope: 'x g', + scopeState: { + scope: 'x g', + }, + }); + + expect(setGrantedScopes).not.toHaveBeenCalled(); + await expect( + manager.handleCallback(makeReq(), { + // The state is prioritized even if scope is present in the result + result: { session: { scope: 'x,y' } } as OAuthAuthenticatorResult, + state: { scope: 'x g' } as OAuthState, + origin: 'https://other.example.com', + }), + ).resolves.toEqual('x g'); + expect(setGrantedScopes).toHaveBeenCalledWith( + expect.anything(), + 'x g', + 'https://other.example.com', + ); + setGrantedScopes.mockClear(); + + expect(removeGrantedScopes).not.toHaveBeenCalled(); + await expect(manager.clear(makeReq())).resolves.toBe(undefined); + expect(removeGrantedScopes).toHaveBeenCalledWith( + expect.anything(), + 'https://example.com', + ); + + const refresh = await manager.refresh(makeReq('x,y')); + expect(refresh.scope).toBe('x y g'); + + expect(setGrantedScopes).not.toHaveBeenCalled(); + await expect( + refresh.commit({ + session: { scope: 'y,z a' }, + } as OAuthAuthenticatorResult), + ).resolves.toEqual('x y g'); + expect(setGrantedScopes).toHaveBeenCalledWith( + expect.anything(), + 'x y g', + 'https://example.com', + ); + }); + + it('should use custom scope transform', async () => { + const manager = CookieScopeManager.create({ + additionalScopes: ['b'], + authenticator: { + scopes: { + persist: true, + required: ['a'], + transform: ({ required, additional, requested, granted }) => + new Set([ + ...[...requested].map(s => `requested-${s}`), + ...[...granted].map(s => `granted-${s}`), + ...[...required].map(s => `required-${s}`), + ...[...additional].map(s => `additional-${s}`), + ]), + } as OAuthAuthenticatorScopeOptions, + } as OAuthAuthenticator, + cookieManager: { + getGrantedScopes: _req => 'g', + setGrantedScopes: (_req, _scope, _origin) => {}, + } as OAuthCookieManager, + }); + + await expect(manager.start(makeReq())).resolves.toEqual({ + scope: 'granted-g required-a additional-b', + scopeState: { + scope: 'granted-g required-a additional-b', + }, + }); + await expect(manager.start(makeReq('x'))).resolves.toEqual({ + scope: 'requested-x granted-g required-a additional-b', + scopeState: { + scope: 'requested-x granted-g required-a additional-b', + }, + }); + + const refresh = await manager.refresh(makeReq('x,y')); + expect(refresh.scope).toBe( + 'requested-x requested-y granted-g required-a additional-b', + ); + + await expect( + refresh.commit({ + session: { scope: 'y,z a' }, + } as OAuthAuthenticatorResult), + ).resolves.toEqual( + 'requested-x requested-y granted-g required-a additional-b', + ); + }); + + it('should fail on invalid input', async () => { + const manager = CookieScopeManager.create({ + authenticator: { + scopes: { + persist: true, + } as OAuthAuthenticatorScopeOptions, + } as OAuthAuthenticator, + cookieManager: {} as OAuthCookieManager, + }); + + await expect( + manager.handleCallback(makeReq(), { + result: { session: { scope: 'x,y' } } as OAuthAuthenticatorResult, + state: {} as OAuthState, + }), + ).rejects.toThrow('No scope found in OAuth state'); + + await expect(manager.clear({} as express.Request)).rejects.toThrow( + 'No response object found in request', + ); + }); +}); diff --git a/plugins/auth-node/src/oauth/CookieScopeManager.ts b/plugins/auth-node/src/oauth/CookieScopeManager.ts index f75bf71db1..45f6e6238a 100644 --- a/plugins/auth-node/src/oauth/CookieScopeManager.ts +++ b/plugins/auth-node/src/oauth/CookieScopeManager.ts @@ -33,8 +33,8 @@ function reqRes(req: express.Request): express.Response { } const defaultTransform: Required['transform'] = - ({ required, additional, requested, granted }) => { - return new Set([...required, ...requested, ...additional, ...granted]); + ({ requested, granted, required, additional }) => { + return [...requested, ...granted, ...required, ...additional]; }; function splitScope(scope?: string): Iterable { @@ -42,12 +42,12 @@ function splitScope(scope?: string): Iterable { return []; } - return new Set(scope.split(/[\s|,]/).filter(Boolean)); + return scope.split(/[\s|,]/).filter(Boolean); } export class CookieScopeManager { static create(options: { - additionalScopes: string[]; + additionalScopes?: string[]; authenticator: OAuthAuthenticator; cookieManager: OAuthCookieManager; }) { @@ -59,13 +59,13 @@ export class CookieScopeManager { false; const transform = authenticator?.scopes?.transform ?? defaultTransform; - const additional = options.additionalScopes; + const additional = options.additionalScopes ?? []; const required = authenticator?.scopes?.required ?? []; return new CookieScopeManager( (requested, granted) => Array.from( - transform({ required, additional, requested, granted }), + new Set(transform({ required, additional, requested, granted })), ).join(' '), shouldPersistScopes ? options.cookieManager : undefined, ); @@ -83,8 +83,9 @@ export class CookieScopeManager { req: express.Request, ): Promise<{ scopeState?: Partial; scope: string }> { const requestScope = splitScope(req.query.scope?.toString()); + const grantedScope = splitScope(this.cookieManager?.getGrantedScopes(req)); - const scope = this.scopeTransform(requestScope, []); + const scope = this.scopeTransform(requestScope, grantedScope); if (this.cookieManager) { // If scopes are persisted then we pass them through the state so that we @@ -107,7 +108,7 @@ export class CookieScopeManager { ): Promise { // If we are not persisting scopes we can forward the scope from the result if (!this.cookieManager) { - return ctx.result.session.scope; + return Array.from(splitScope(ctx.result.session.scope)).join(' '); } const scope = ctx.state.scope; @@ -149,7 +150,7 @@ export class CookieScopeManager { return scope; } - return result.session.scope; + return Array.from(splitScope(result.session.scope)).join(' '); }, }; }