From c9d328fca8b7b3650af34f1b2b33a4fd81751887 Mon Sep 17 00:00:00 2001 From: Konstantin Pershin Date: Fri, 28 Jul 2023 20:17:24 +0700 Subject: [PATCH 1/7] create way to add some additional params to redirect url --- src/client/authorization-code.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/client/authorization-code.ts b/src/client/authorization-code.ts index 618a33d..cde07f8 100644 --- a/src/client/authorization-code.ts +++ b/src/client/authorization-code.ts @@ -28,6 +28,14 @@ type GetAuthorizeUrlParams = { scope?: string[]; } +type GetAuthorizeUrlCustomParams = { + /** + * List of non-standard parameters that may need, if your auth server want to have some additional info in url + */ + [key: string]: string; +} + + type ValidateResponseResult = { /** @@ -56,7 +64,7 @@ export class OAuth2AuthorizationCodeClient { * Returns the URi that the user should open in a browser to initiate the * authorization_code flow. */ - async getAuthorizeUri(params: GetAuthorizeUrlParams): Promise { + async getAuthorizeUri(params: GetAuthorizeUrlParams, customParams: GetAuthorizeUrlCustomParams): Promise { const [ codeChallenge, @@ -72,6 +80,7 @@ export class OAuth2AuthorizationCodeClient { redirect_uri: params.redirectUri, code_challenge_method: codeChallenge?.[0], code_challenge: codeChallenge?.[1], + ...customParams }; if (params.state) { query.state = params.state; From cd5e8264388867e9f73b38e0d87bb8a2a8ddf79b Mon Sep 17 00:00:00 2001 From: Konstantin Pershin Date: Fri, 28 Jul 2023 20:32:00 +0700 Subject: [PATCH 2/7] update doc --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 05f4975..66ee04f 100644 --- a/README.md +++ b/README.md @@ -173,6 +173,10 @@ document.location = await client.authorizationCode.getAuthorizeUri({ scope: ['scope1', 'scope2'], +},{ + // optional list of custom parameters to add redirect URL + customParam1: "customParam1value1", + customParam2: "customParam1value2" }); ``` From b55e7be40ead1a9ec71d0dd1404c99d64bf7d990 Mon Sep 17 00:00:00 2001 From: Konstantin Pershin Date: Fri, 28 Jul 2023 21:24:41 +0700 Subject: [PATCH 3/7] update method signature --- src/client/authorization-code.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/authorization-code.ts b/src/client/authorization-code.ts index cde07f8..8d27ccf 100644 --- a/src/client/authorization-code.ts +++ b/src/client/authorization-code.ts @@ -64,7 +64,7 @@ export class OAuth2AuthorizationCodeClient { * Returns the URi that the user should open in a browser to initiate the * authorization_code flow. */ - async getAuthorizeUri(params: GetAuthorizeUrlParams, customParams: GetAuthorizeUrlCustomParams): Promise { + async getAuthorizeUri(params: GetAuthorizeUrlParams, customParams?: GetAuthorizeUrlCustomParams): Promise { const [ codeChallenge, From 299c9712e0ea010c1dd9a243ef9b1ced01f33c75 Mon Sep 17 00:00:00 2001 From: Konstantin Pershin Date: Mon, 31 Jul 2023 19:44:50 +0700 Subject: [PATCH 4/7] fixes after review. Add tests and params checking, change signatures due naming convention. --- README.md | 8 ++-- src/client/authorization-code.ts | 19 +++++---- src/messages.ts | 1 + test/authorization-code.ts | 69 ++++++++++++++++++++++++++++++++ 4 files changed, 86 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 66ee04f..d0c8436 100644 --- a/README.md +++ b/README.md @@ -173,10 +173,10 @@ document.location = await client.authorizationCode.getAuthorizeUri({ scope: ['scope1', 'scope2'], -},{ - // optional list of custom parameters to add redirect URL - customParam1: "customParam1value1", - customParam2: "customParam1value2" + // Optional parameters that may be need for 3rd party auth servers + extraParams: { + foo: 'bar' + } }); ``` diff --git a/src/client/authorization-code.ts b/src/client/authorization-code.ts index 8d27ccf..59772c2 100644 --- a/src/client/authorization-code.ts +++ b/src/client/authorization-code.ts @@ -26,16 +26,13 @@ type GetAuthorizeUrlParams = { * List of scopes. */ scope?: string[]; -} -type GetAuthorizeUrlCustomParams = { /** * List of non-standard parameters that may need, if your auth server want to have some additional info in url */ - [key: string]: string; + extraParams?: Record } - type ValidateResponseResult = { /** @@ -64,7 +61,7 @@ export class OAuth2AuthorizationCodeClient { * Returns the URi that the user should open in a browser to initiate the * authorization_code flow. */ - async getAuthorizeUri(params: GetAuthorizeUrlParams, customParams?: GetAuthorizeUrlCustomParams): Promise { + async getAuthorizeUri(params: GetAuthorizeUrlParams): Promise { const [ codeChallenge, @@ -74,13 +71,12 @@ export class OAuth2AuthorizationCodeClient { this.client.getEndpoint('authorizationEndpoint') ]); - const query: AuthorizationQueryParams = { + let query: AuthorizationQueryParams = { client_id: this.client.settings.clientId, response_type: 'code', redirect_uri: params.redirectUri, code_challenge_method: codeChallenge?.[0], code_challenge: codeChallenge?.[1], - ...customParams }; if (params.state) { query.state = params.state; @@ -89,6 +85,15 @@ export class OAuth2AuthorizationCodeClient { query.scope = params.scope.join(' '); } + const disallowed = Object.keys(query) + + if (params?.extraParams && Object.keys(params.extraParams).filter((key) => disallowed.includes(key)).length > 0) { + throw new Error(`The following extraParams are disallowed: '${disallowed.join("', '")}'`); + } + + query = {...query, ...params?.extraParams} + + return authorizationEndpoint + '?' + generateQueryString(query); } diff --git a/src/messages.ts b/src/messages.ts index ee04295..3d4636c 100644 --- a/src/messages.ts +++ b/src/messages.ts @@ -47,6 +47,7 @@ export type AuthorizationQueryParams = { scope?: string; code_challenge_method?: 'plain' | 'S256'; code_challenge?: string; + [key: string]: string | undefined; } /** diff --git a/test/authorization-code.ts b/test/authorization-code.ts index ddb7808..3dec6f6 100644 --- a/test/authorization-code.ts +++ b/test/authorization-code.ts @@ -35,6 +35,75 @@ describe('authorization-code', () => { }) ).to.equal(server.url + '/authorize?' + params.toString()); + }); + it('should support extraparams', async() => { + + const server = testServer(); + const client = new OAuth2Client({ + server: server.url, + authorizationEndpoint: '/authorize', + clientId: 'test-client-id', + }); + + const redirectUri = 'http://my-app.example/redirect'; + + const params = new URLSearchParams({ + client_id: 'test-client-id', + response_type: 'code', + redirect_uri: redirectUri, + scope: 'a b', + foo: 'bar', + }); + + expect( + await client.authorizationCode.getAuthorizeUri({ + redirectUri, + scope: ['a', 'b'], + extraParams: { + foo: 'bar' + } + }) + ).to.equal(server.url + '/authorize?' + params.toString()); + + }); + it('should throw error when user rewrote params by extraparams', async() => { + + const server = testServer(); + const client = new OAuth2Client({ + server: server.url, + authorizationEndpoint: '/authorize', + clientId: 'test-client-id', + }); + + const redirectUri = 'http://my-app.example/redirect'; + + const params = { + redirectUri, + scope: ['a', 'b'], + state: 'some-state' + + }; + + const extraParams = { + foo: 'bar', + scope: 'accidentally rewrote core parameter' + } + + try { + await client.authorizationCode.getAuthorizeUri({ + ...params, + extraParams + }) + } catch (error: any) { + expect(error.message).to.equal( + 'The following extraParams are disallowed: \'client_id\', \'response_type\', \'redirect_uri\', ' + + '\'code_challenge_method\', \'code_challenge\', \'state\', \'scope\'' + ); + return; + } + + expect.fail('Should have thrown'); + }); it('should support PKCE', async() => { From 6afc87480e04251099c2ec20fe1a0ae527158191 Mon Sep 17 00:00:00 2001 From: Evert Pot Date: Wed, 2 Aug 2023 22:36:13 -0400 Subject: [PATCH 5/7] Remove extraParams from example It's not a core use-case, and it's nice to keep the readme a bit minimal. --- README.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/README.md b/README.md index d0c8436..05f4975 100644 --- a/README.md +++ b/README.md @@ -173,10 +173,6 @@ document.location = await client.authorizationCode.getAuthorizeUri({ scope: ['scope1', 'scope2'], - // Optional parameters that may be need for 3rd party auth servers - extraParams: { - foo: 'bar' - } }); ``` From 94cc641a2222ec2b6dd2eb71105a10d95cf1a8c0 Mon Sep 17 00:00:00 2001 From: Evert Pot Date: Wed, 2 Aug 2023 22:37:52 -0400 Subject: [PATCH 6/7] JSDoc tweak --- src/client/authorization-code.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/authorization-code.ts b/src/client/authorization-code.ts index 59772c2..e34d69e 100644 --- a/src/client/authorization-code.ts +++ b/src/client/authorization-code.ts @@ -28,7 +28,7 @@ type GetAuthorizeUrlParams = { scope?: string[]; /** - * List of non-standard parameters that may need, if your auth server want to have some additional info in url + * Any parameters listed here will be added to the query string for the authorization server endpoint. */ extraParams?: Record } From 338ecfa9fe25c3a9ddebc6e58ef3f9ebd51e9bef Mon Sep 17 00:00:00 2001 From: Konstantin Pershin Date: Thu, 3 Aug 2023 15:49:39 +0700 Subject: [PATCH 7/7] eslint fixes --- src/client/authorization-code.ts | 6 +++--- test/authorization-code.ts | 20 ++++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/client/authorization-code.ts b/src/client/authorization-code.ts index e34d69e..ada05dd 100644 --- a/src/client/authorization-code.ts +++ b/src/client/authorization-code.ts @@ -30,7 +30,7 @@ type GetAuthorizeUrlParams = { /** * Any parameters listed here will be added to the query string for the authorization server endpoint. */ - extraParams?: Record + extraParams?: Record; } type ValidateResponseResult = { @@ -85,13 +85,13 @@ export class OAuth2AuthorizationCodeClient { query.scope = params.scope.join(' '); } - const disallowed = Object.keys(query) + const disallowed = Object.keys(query); if (params?.extraParams && Object.keys(params.extraParams).filter((key) => disallowed.includes(key)).length > 0) { throw new Error(`The following extraParams are disallowed: '${disallowed.join("', '")}'`); } - query = {...query, ...params?.extraParams} + query = {...query, ...params?.extraParams}; return authorizationEndpoint + '?' + generateQueryString(query); diff --git a/test/authorization-code.ts b/test/authorization-code.ts index 3dec6f6..4477e60 100644 --- a/test/authorization-code.ts +++ b/test/authorization-code.ts @@ -56,13 +56,13 @@ describe('authorization-code', () => { }); expect( - await client.authorizationCode.getAuthorizeUri({ - redirectUri, - scope: ['a', 'b'], - extraParams: { - foo: 'bar' - } - }) + await client.authorizationCode.getAuthorizeUri({ + redirectUri, + scope: ['a', 'b'], + extraParams: { + foo: 'bar' + } + }) ).to.equal(server.url + '/authorize?' + params.toString()); }); @@ -87,16 +87,16 @@ describe('authorization-code', () => { const extraParams = { foo: 'bar', scope: 'accidentally rewrote core parameter' - } + }; try { await client.authorizationCode.getAuthorizeUri({ ...params, extraParams - }) + }); } catch (error: any) { expect(error.message).to.equal( - 'The following extraParams are disallowed: \'client_id\', \'response_type\', \'redirect_uri\', ' + + 'The following extraParams are disallowed: \'client_id\', \'response_type\', \'redirect_uri\', ' + '\'code_challenge_method\', \'code_challenge\', \'state\', \'scope\'' ); return;