From 3b767f19c97ecfa436f3e578121d69d3207890c9 Mon Sep 17 00:00:00 2001 From: Nicolas Arnold Date: Fri, 1 Oct 2021 09:39:55 +0100 Subject: [PATCH] Adopt extra field for OAuth state Currently, the OAuth state is very limited. It only accepts three field: * env * nonce * origin This does not give the user much flexibility when passing in other fields to the state. Origin is set based on the window location. Env determined by the running environment of backstage. Nonce, randomly generated every time. If a user wanted to verify other fields in the state, they would be unable to do so. For example, let's say you have a GitHub app that serves multiple installations. In order for this to work you need a middle service between github and backstage. This service needs to programaticaly determine where to redirect the requests to (GitHub apps only allow one redirect url). Your intermediate service requires you to redirect to other paths on backstage based on the type of request the Github ap p receives. By adding in the `extraState` to the Github Provider Options, this can now be achieved. You can set the field to `{'redirect_url': '/some/path/to/redirect/to'}` to complete the OAuth flow. Although this is a very specific use case, I believe this will be useful across all the providers. Signed-off-by: Nicolas Arnold --- .changeset/lucky-books-worry.md | 5 ++++ plugins/auth-backend/api-report.md | 5 +++- .../src/lib/oauth/helpers.test.ts | 26 ++++++++++++++++--- plugins/auth-backend/src/lib/oauth/types.ts | 2 +- .../src/providers/github/provider.ts | 13 +++++++++- 5 files changed, 44 insertions(+), 7 deletions(-) create mode 100644 .changeset/lucky-books-worry.md diff --git a/.changeset/lucky-books-worry.md b/.changeset/lucky-books-worry.md new file mode 100644 index 0000000000..f91dccb3f9 --- /dev/null +++ b/.changeset/lucky-books-worry.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-backend': patch +--- + +Allow extra field in OAuth state parameter diff --git a/plugins/auth-backend/api-report.md b/plugins/auth-backend/api-report.md index 572f182082..25cd839b1d 100644 --- a/plugins/auth-backend/api-report.md +++ b/plugins/auth-backend/api-report.md @@ -290,6 +290,9 @@ export type GithubOAuthResult = { // @public (undocumented) export type GithubProviderOptions = { authHandler?: AuthHandler; + extraState?: { + [key: string]: string; + }; signIn?: { resolver?: SignInResolver; }; @@ -488,7 +491,7 @@ export type OAuthStartRequest = express.Request<{}> & { export type OAuthState = { nonce: string; env: string; - origin?: string; + [key: string]: string; }; // Warning: (ae-missing-release-tag) "oktaEmailSignInResolver" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) diff --git a/plugins/auth-backend/src/lib/oauth/helpers.test.ts b/plugins/auth-backend/src/lib/oauth/helpers.test.ts index 8af1d2f2ad..bd667dca73 100644 --- a/plugins/auth-backend/src/lib/oauth/helpers.test.ts +++ b/plugins/auth-backend/src/lib/oauth/helpers.test.ts @@ -20,16 +20,31 @@ import { verifyNonce, encodeState, readState } from './helpers'; describe('OAuthProvider Utils', () => { describe('encodeState', () => { it('should serialized values', () => { + const state = { + nonce: '123', + env: 'development', + }; + + const encoded = encodeState(state); + expect(encoded).toBe( + Buffer.from('nonce=123&env=development').toString('hex'), + ); + + expect(readState(encoded)).toEqual(state); + }); + + it('should serialized values with extra values', () => { const state = { nonce: '123', env: 'development', origin: 'https://example.com', + redirect_url: 'https://someurl.com/foo/bar', }; const encoded = encodeState(state); expect(encoded).toBe( Buffer.from( - 'nonce=123&env=development&origin=https%3A%2F%2Fexample.com', + 'nonce=123&env=development&origin=https%3A%2F%2Fexample.com&redirect_url=https%3A%2F%2Fsomeurl.com%2Ffoo%2Fbar', ).toString('hex'), ); @@ -37,9 +52,12 @@ describe('OAuthProvider Utils', () => { }); it('should not include undefined values', () => { - const state = { nonce: '123', env: 'development', origin: undefined }; - - const encoded = encodeState(state); + const state = { + nonce: '123', + env: 'development', + }; + // @ts-ignore + const encoded = encodeState({ test: undefined, ...state }); expect(encoded).toBe( Buffer.from('nonce=123&env=development').toString('hex'), ); diff --git a/plugins/auth-backend/src/lib/oauth/types.ts b/plugins/auth-backend/src/lib/oauth/types.ts index 7912dd16a5..775c5e35d1 100644 --- a/plugins/auth-backend/src/lib/oauth/types.ts +++ b/plugins/auth-backend/src/lib/oauth/types.ts @@ -77,7 +77,7 @@ export type OAuthState = { */ nonce: string; env: string; - origin?: string; + [key: string]: string; }; export type OAuthStartRequest = express.Request<{}> & { diff --git a/plugins/auth-backend/src/providers/github/provider.ts b/plugins/auth-backend/src/providers/github/provider.ts index 82d231edfa..12603e8230 100644 --- a/plugins/auth-backend/src/providers/github/provider.ts +++ b/plugins/auth-backend/src/providers/github/provider.ts @@ -64,6 +64,7 @@ export type GithubAuthProviderOptions = OAuthProviderOptions & { tokenUrl?: string; userProfileUrl?: string; authorizationUrl?: string; + extraState?: { [key: string]: string }; signInResolver?: SignInResolver; authHandler: AuthHandler; tokenIssuer: TokenIssuer; @@ -78,12 +79,14 @@ export class GithubAuthProvider implements OAuthHandlers { private readonly tokenIssuer: TokenIssuer; private readonly catalogIdentityClient: CatalogIdentityClient; private readonly logger: Logger; + private readonly extraState: { [key: string]: string }; constructor(options: GithubAuthProviderOptions) { this.signInResolver = options.signInResolver; this.authHandler = options.authHandler; this.tokenIssuer = options.tokenIssuer; this.catalogIdentityClient = options.catalogIdentityClient; + this.extraState = options.extraState || {}; this.logger = options.logger; this._strategy = new GithubStrategy( { @@ -109,7 +112,7 @@ export class GithubAuthProvider implements OAuthHandlers { async start(req: OAuthStartRequest): Promise { return await executeRedirectStrategy(req, this._strategy, { scope: req.scope, - state: encodeState(req.state), + state: encodeState({ ...this.extraState, ...req.state }), }); } @@ -200,6 +203,11 @@ export type GithubProviderOptions = { */ authHandler?: AuthHandler; + /** + * The extra state you would like to pass into the OAuth state + */ + extraState?: { [key: string]: string }; + /** * Configure sign-in for this provider, without it the provider can not be used to sign users in. */ @@ -263,6 +271,8 @@ export const createGithubProvider = ( logger, }); + const extraState = options?.extraState ? options.extraState : undefined; + const provider = new GithubAuthProvider({ clientId, clientSecret, @@ -274,6 +284,7 @@ export const createGithubProvider = ( authHandler, tokenIssuer, catalogIdentityClient, + extraState, logger, });