From c8e51ac568c5a7bdc85fd17c82cf69df85462d29 Mon Sep 17 00:00:00 2001 From: daniel Date: Fri, 3 Jul 2026 01:38:18 -0300 Subject: [PATCH 1/3] feat(token-handler): distinguish public and confidential clients #81 --- index.d.ts | 1 + lib/handlers/token-handler.js | 20 ++-- lib/model.js | 1 + .../handlers/token-handler_test.js | 104 ++++++++++++++++-- test/unit/handlers/token-handler_test.js | 29 ++++- 5 files changed, 135 insertions(+), 20 deletions(-) diff --git a/index.d.ts b/index.d.ts index 74e554f..27fd515 100644 --- a/index.d.ts +++ b/index.d.ts @@ -406,6 +406,7 @@ declare namespace OAuth2Server { grants: string | string[]; accessTokenLifetime?: number; refreshTokenLifetime?: number; + type?: 'public' | 'confidential'; [key: string]: any; } diff --git a/lib/handlers/token-handler.js b/lib/handlers/token-handler.js index c6ebebb..2b76cc1 100644 --- a/lib/handlers/token-handler.js +++ b/lib/handlers/token-handler.js @@ -122,10 +122,6 @@ class TokenHandler { throw new InvalidRequestError('Missing parameter: `client_id`'); } - if (this.isClientAuthenticationRequired(grantType) && !credentials.clientSecret && !isPkce) { - throw new InvalidRequestError('Missing parameter: `client_secret`'); - } - if (!isFormat.vschar(credentials.clientId)) { throw new InvalidRequestError('Invalid parameter: `client_id`'); } @@ -135,7 +131,7 @@ class TokenHandler { } try { - const client = await this.model.getClient(credentials.clientId, credentials.clientSecret); + const client = await this.model.getClient(credentials.clientId, credentials.clientSecret ?? null); if (!client) { throw new InvalidClientError('Invalid client: client is invalid'); @@ -149,6 +145,10 @@ class TokenHandler { throw new ServerError('Server error: `grants` must be an array'); } + if (this.isClientAuthenticationRequired(grantType, client) && !credentials.clientSecret && !isPkce) { + throw new InvalidClientError('Invalid client: client is invalid'); + } + return client; } catch (e) { // Include the "WWW-Authenticate" response header field if the client @@ -195,10 +195,8 @@ class TokenHandler { } } - if (!this.isClientAuthenticationRequired(grantType)) { - if (request.body.client_id) { - return { clientId: request.body.client_id }; - } + if (request.body.client_id) { + return { clientId: request.body.client_id }; } throw new InvalidClientError('Invalid client: cannot retrieve client credentials'); @@ -305,7 +303,9 @@ class TokenHandler { /** * Given a grant type, check if client authentication is required */ - isClientAuthenticationRequired(grantType) { + isClientAuthenticationRequired(grantType, client) { + if (client && client.type === 'public') return false; + if (Object.keys(this.requireClientAuthentication).length > 0) { return typeof this.requireClientAuthentication[grantType] !== 'undefined' ? this.requireClientAuthentication[grantType] diff --git a/lib/model.js b/lib/model.js index cbf4e84..8958e4c 100644 --- a/lib/model.js +++ b/lib/model.js @@ -46,6 +46,7 @@ const ServerError = require('./errors/server-error'); * @property grants {string[]} Grant types allowed for the client. * @property accessTokenLifetime {number} Client-specific lifetime of generated access tokens in seconds. * @property refreshTokenLifetime {number} Client-specific lifetime of generated refresh tokens in seconds. + * @property type {string} The client type: `public` or `confidential`. Defaults to 'confidential' when not set. */ /** diff --git a/test/integration/handlers/token-handler_test.js b/test/integration/handlers/token-handler_test.js index 010a929..0dc6efb 100644 --- a/test/integration/handlers/token-handler_test.js +++ b/test/integration/handlers/token-handler_test.js @@ -766,6 +766,98 @@ describe('TokenHandler integration', function () { .catch(should.fail); }); + describe('with a public client and no `client_secret`', function () { + it('should return the client without requiring a secret', function () { + const client = { id: 'foo', grants: ['authorization_code'], type: 'public' }; + const model = Model.from({ + getClient: function () { + return client; + }, + saveToken: function () {}, + }); + const handler = new TokenHandler({ + accessTokenLifetime: 120, + model: model, + refreshTokenLifetime: 120, + }); + const request = new Request({ + body: { client_id: 'foo', grant_type: 'authorization_code' }, + headers: {}, + method: {}, + query: {}, + }); + + return handler + .getClient(request) + .then(function (data) { + data.should.equal(client); + }) + .catch(should.fail); + }); + }); + + describe('with a confidential client (explicit type) and no `client_secret`', function () { + it('should throw an error', function () { + const client = { id: 'foo', grants: ['authorization_code'], type: 'confidential' }; + const model = Model.from({ + getClient: function () { + return client; + }, + saveToken: function () {}, + }); + const handler = new TokenHandler({ + accessTokenLifetime: 120, + model: model, + refreshTokenLifetime: 120, + }); + const request = new Request({ + body: { client_id: 'foo', grant_type: 'authorization_code' }, + headers: {}, + method: {}, + query: {}, + }); + + return handler + .getClient(request) + .then(should.fail) + .catch(function (e) { + e.should.be.an.instanceOf(InvalidClientError); + e.message.should.equal('Invalid client: client is invalid'); + }); + }); + }); + + describe('with a confidential client (no type, default) and no `client_secret`', function () { + it('should throw an error', function () { + const client = { id: 'foo', grants: ['password'] }; + const model = Model.from({ + getClient: function () { + return client; + }, + saveToken: function () {}, + }); + const handler = new TokenHandler({ + accessTokenLifetime: 120, + model: model, + refreshTokenLifetime: 120, + }); + const request = new Request({ + body: { client_id: 'foo', grant_type: 'password' }, + headers: {}, + method: {}, + query: {}, + }); + + return handler + .getClient(request) + .then(should.fail) + .catch(function (e) { + e.should.be.an.instanceOf(InvalidClientError); + e.message.should.equal('Invalid client: client is invalid'); + }); + }); + }); + describe('with `password` grant type and `requireClientAuthentication` is false', function () { it('should return a client ', function () { const client = { id: 12345, grants: [] }; @@ -909,7 +1001,7 @@ describe('TokenHandler integration', function () { } }); - it('should throw an error if `client_secret` is missing', async function () { + it('should return credentials with only `clientId` when `client_secret` is missing', function () { const model = Model.from({ getClient: function () {}, saveToken: function () {}, @@ -926,14 +1018,8 @@ describe('TokenHandler integration', function () { query: {}, }); - try { - await handler.getClientCredentials(request); - - should.fail(); - } catch (e) { - e.should.be.an.instanceOf(InvalidClientError); - e.message.should.equal('Invalid client: cannot retrieve client credentials'); - } + const credentials = handler.getClientCredentials(request); + credentials.should.eql({ clientId: 'foo' }); }); describe('with `client_id` and grant type is `password` and `requireClientAuthentication` is false', function () { diff --git a/test/unit/handlers/token-handler_test.js b/test/unit/handlers/token-handler_test.js index 89b30fb..7f75f61 100644 --- a/test/unit/handlers/token-handler_test.js +++ b/test/unit/handlers/token-handler_test.js @@ -16,7 +16,7 @@ const should = require('chai').should(); describe('TokenHandler', function () { describe('getClient()', function () { - it('should call `model.getClient()`', function () { + it('should call `model.getClient()` with the provided secret', function () { const model = Model.from({ getClient: sinon.stub().returns({ grants: ['password'] }), saveToken: function () {}, @@ -44,5 +44,32 @@ describe('TokenHandler', function () { }) .catch(should.fail); }); + + it('should call `model.getClient()` with no secret is provided (public client)', function () { + const model = Model.from({ + getClient: sinon.stub().returns({ grants: ['authorization_code'], type: 'public' }), + saveToken: function () {}, + }); + const handler = new TokenHandler({ + accessTokenLifetime: 120, + model: model, + refreshTokenLifetime: 120, + }); + const request = new Request({ + body: { client_id: 'foo', grant_type: 'authorization_code' }, + headers: {}, + method: {}, + query: {}, + }); + + return handler + .getClient(request) + .then(function () { + model.getClient.callCount.should.equal(1); + model.getClient.firstCall.args[0].should.equal('foo'); + should.equal(model.getClient.firstCall.args[1], null); + }) + .catch(should.fail); + }); }); }); From a0447b6bfe093884161ce53f8d1adbb79625a001 Mon Sep 17 00:00:00 2001 From: Daniel Schiramm <17592852+dsschiramm@users.noreply.github.com> Date: Fri, 3 Jul 2026 23:30:17 -0300 Subject: [PATCH 2/3] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- lib/handlers/token-handler.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/handlers/token-handler.js b/lib/handlers/token-handler.js index be510fb..5816cee 100644 --- a/lib/handlers/token-handler.js +++ b/lib/handlers/token-handler.js @@ -132,7 +132,7 @@ class TokenHandler { } try { - const client = await this.model.getClient(credentials.clientId, credentials.clientSecret ?? null); + const client = await this.model.getClient(credentials.clientId, credentials.clientSecret || null); if (!client) { throw new InvalidClientError('Invalid client: client is invalid'); From e0b27c6a4a161bc7cd2dcd9de5bd560f1b07985d Mon Sep 17 00:00:00 2001 From: daniel Date: Fri, 3 Jul 2026 23:37:14 -0300 Subject: [PATCH 3/3] refactor(token-handler): remove redundant PKCE check for client_id retrieval and rename test description --- lib/handlers/token-handler.js | 6 ------ test/unit/handlers/token-handler_test.js | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/handlers/token-handler.js b/lib/handlers/token-handler.js index 5816cee..180dd13 100644 --- a/lib/handlers/token-handler.js +++ b/lib/handlers/token-handler.js @@ -190,12 +190,6 @@ class TokenHandler { }; } - if (pkce.isPKCERequest({ grantType, codeVerifier })) { - if (request.body.client_id) { - return { clientId: request.body.client_id }; - } - } - if (request.body.client_id) { return { clientId: request.body.client_id }; } diff --git a/test/unit/handlers/token-handler_test.js b/test/unit/handlers/token-handler_test.js index 7f75f61..d9f4303 100644 --- a/test/unit/handlers/token-handler_test.js +++ b/test/unit/handlers/token-handler_test.js @@ -45,7 +45,7 @@ describe('TokenHandler', function () { .catch(should.fail); }); - it('should call `model.getClient()` with no secret is provided (public client)', function () { + it('should call `model.getClient()` when no client secret is provided (public client)', function () { const model = Model.from({ getClient: sinon.stub().returns({ grants: ['authorization_code'], type: 'public' }), saveToken: function () {},