From 6fa4051ef019551ae93b4b3eca41382650fa5d8a Mon Sep 17 00:00:00 2001 From: Carlos Coronado Date: Sun, 20 Aug 2023 22:16:09 -0400 Subject: [PATCH 1/2] change endorsements type from array to set --- .../src/auth/endorsementsValidator.ts | 11 +++-------- .../src/auth/jwtTokenExtractor.ts | 8 +++++--- .../tests/auth/endorsementsValidator.test.js | 12 ++++++------ 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/libraries/botframework-connector/src/auth/endorsementsValidator.ts b/libraries/botframework-connector/src/auth/endorsementsValidator.ts index 12967738e6..64f6cc4bad 100644 --- a/libraries/botframework-connector/src/auth/endorsementsValidator.ts +++ b/libraries/botframework-connector/src/auth/endorsementsValidator.ts @@ -25,14 +25,14 @@ export class EndorsementsValidator { * some specific channels. That list is the endorsement list, and is validated here against the channelId. * @returns {boolean} True is the channelId is found in the Endorsement set. False if the channelId is not found. */ - static validate(channelId: string, endorsements: string[]): boolean { + static validate(channelId: string, endorsements: Set): boolean { // If the Activity came in and doesn't have a Channel ID then it's making no // assertions as to who endorses it. This means it should pass. if (channelId === null || channelId.trim() === '') { return true; } - if (endorsements === null) { + if (!endorsements) { throw new AuthenticationError('endorsements required', StatusCodes.UNAUTHORIZED); } @@ -47,11 +47,6 @@ export class EndorsementsValidator { // Does the set of endorsements match the channelId that was passed in? - // ToDo: Consider moving this to a HashSet instead of a string - // array, to make lookups O(1) instead of O(N). To give a sense - // of scope, tokens from WebChat have about 10 endorsements, and - // tokens coming from Teams have about 20. - - return endorsements.some((value: string) => value === channelId); + return endorsements.has(channelId); } } diff --git a/libraries/botframework-connector/src/auth/jwtTokenExtractor.ts b/libraries/botframework-connector/src/auth/jwtTokenExtractor.ts index fd2f1f71bf..bef637a1e0 100644 --- a/libraries/botframework-connector/src/auth/jwtTokenExtractor.ts +++ b/libraries/botframework-connector/src/auth/jwtTokenExtractor.ts @@ -156,12 +156,14 @@ export class JwtTokenExtractor { } // enforce endorsements in openIdMetadadata if there is any endorsements associated with the key - const endorsements = metadata.endorsements; - if (Array.isArray(endorsements) && endorsements.length !== 0) { + const endorsements = new Set(metadata.endorsements); + if (endorsements.size !== 0) { const isEndorsed = EndorsementsValidator.validate(channelId, endorsements); if (!isEndorsed) { throw new AuthenticationError( - `Could not validate endorsement for key: ${keyId} with endorsements: ${endorsements.join(',')}`, + `Could not validate endorsement for key: ${keyId} with endorsements: ${[...endorsements].join( + ',' + )}`, StatusCodes.UNAUTHORIZED ); } diff --git a/libraries/botframework-connector/tests/auth/endorsementsValidator.test.js b/libraries/botframework-connector/tests/auth/endorsementsValidator.test.js index 622a7825ca..d156b10205 100644 --- a/libraries/botframework-connector/tests/auth/endorsementsValidator.test.js +++ b/libraries/botframework-connector/tests/auth/endorsementsValidator.test.js @@ -6,7 +6,7 @@ const { EndorsementsValidator } = require('../..'); describe('EndorsementsValidator', function () { it('with null channelId should pass', function () { - assert(EndorsementsValidator.validate(null, [])); + assert(EndorsementsValidator.validate(null, new Set([]))); }); it('with null endorsements should throw', function () { @@ -14,22 +14,22 @@ describe('EndorsementsValidator', function () { }); it('with unendorsed channelId should fail', function () { - assert(!EndorsementsValidator.validate('channelOne', [])); + assert(!EndorsementsValidator.validate('channelOne', new Set([]))); }); it('with mismatched endorsements should fail', function () { - assert(!EndorsementsValidator.validate('right', ['wrong'])); + assert(!EndorsementsValidator.validate('right', new Set(['wrong']))); }); it('with endorsed channelId should pass', function () { - assert(EndorsementsValidator.validate('right', ['right'])); + assert(EndorsementsValidator.validate('right', new Set(['right']))); }); it('with endorsed channelId and many endorsements should pass', function () { - assert(EndorsementsValidator.validate('right', ['wrong', 'right'])); + assert(EndorsementsValidator.validate('right', new Set(['wrong', 'right']))); }); it('with empty channelId should pass', function () { - assert(EndorsementsValidator.validate('', ['wrong', 'right'])); + assert(EndorsementsValidator.validate('', new Set(['wrong', 'right']))); }); }); From b3ed292d7920a7b8102168de37664f40f1fc030e Mon Sep 17 00:00:00 2001 From: Carlos Coronado Date: Sun, 14 Jan 2024 19:01:35 -0400 Subject: [PATCH 2/2] refactor: create a set with endorsements values to validate channelId --- .../src/auth/endorsementsValidator.ts | 6 +++--- .../src/auth/jwtTokenExtractor.ts | 8 +++----- .../tests/auth/endorsementsValidator.test.js | 12 ++++++------ 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/libraries/botframework-connector/src/auth/endorsementsValidator.ts b/libraries/botframework-connector/src/auth/endorsementsValidator.ts index 64f6cc4bad..4b7ee6ca3f 100644 --- a/libraries/botframework-connector/src/auth/endorsementsValidator.ts +++ b/libraries/botframework-connector/src/auth/endorsementsValidator.ts @@ -25,14 +25,14 @@ export class EndorsementsValidator { * some specific channels. That list is the endorsement list, and is validated here against the channelId. * @returns {boolean} True is the channelId is found in the Endorsement set. False if the channelId is not found. */ - static validate(channelId: string, endorsements: Set): boolean { + static validate(channelId: string, endorsements: string[]): boolean { // If the Activity came in and doesn't have a Channel ID then it's making no // assertions as to who endorses it. This means it should pass. if (channelId === null || channelId.trim() === '') { return true; } - if (!endorsements) { + if (endorsements === null) { throw new AuthenticationError('endorsements required', StatusCodes.UNAUTHORIZED); } @@ -47,6 +47,6 @@ export class EndorsementsValidator { // Does the set of endorsements match the channelId that was passed in? - return endorsements.has(channelId); + return new Set(endorsements).has(channelId); } } diff --git a/libraries/botframework-connector/src/auth/jwtTokenExtractor.ts b/libraries/botframework-connector/src/auth/jwtTokenExtractor.ts index bef637a1e0..fd2f1f71bf 100644 --- a/libraries/botframework-connector/src/auth/jwtTokenExtractor.ts +++ b/libraries/botframework-connector/src/auth/jwtTokenExtractor.ts @@ -156,14 +156,12 @@ export class JwtTokenExtractor { } // enforce endorsements in openIdMetadadata if there is any endorsements associated with the key - const endorsements = new Set(metadata.endorsements); - if (endorsements.size !== 0) { + const endorsements = metadata.endorsements; + if (Array.isArray(endorsements) && endorsements.length !== 0) { const isEndorsed = EndorsementsValidator.validate(channelId, endorsements); if (!isEndorsed) { throw new AuthenticationError( - `Could not validate endorsement for key: ${keyId} with endorsements: ${[...endorsements].join( - ',' - )}`, + `Could not validate endorsement for key: ${keyId} with endorsements: ${endorsements.join(',')}`, StatusCodes.UNAUTHORIZED ); } diff --git a/libraries/botframework-connector/tests/auth/endorsementsValidator.test.js b/libraries/botframework-connector/tests/auth/endorsementsValidator.test.js index d156b10205..622a7825ca 100644 --- a/libraries/botframework-connector/tests/auth/endorsementsValidator.test.js +++ b/libraries/botframework-connector/tests/auth/endorsementsValidator.test.js @@ -6,7 +6,7 @@ const { EndorsementsValidator } = require('../..'); describe('EndorsementsValidator', function () { it('with null channelId should pass', function () { - assert(EndorsementsValidator.validate(null, new Set([]))); + assert(EndorsementsValidator.validate(null, [])); }); it('with null endorsements should throw', function () { @@ -14,22 +14,22 @@ describe('EndorsementsValidator', function () { }); it('with unendorsed channelId should fail', function () { - assert(!EndorsementsValidator.validate('channelOne', new Set([]))); + assert(!EndorsementsValidator.validate('channelOne', [])); }); it('with mismatched endorsements should fail', function () { - assert(!EndorsementsValidator.validate('right', new Set(['wrong']))); + assert(!EndorsementsValidator.validate('right', ['wrong'])); }); it('with endorsed channelId should pass', function () { - assert(EndorsementsValidator.validate('right', new Set(['right']))); + assert(EndorsementsValidator.validate('right', ['right'])); }); it('with endorsed channelId and many endorsements should pass', function () { - assert(EndorsementsValidator.validate('right', new Set(['wrong', 'right']))); + assert(EndorsementsValidator.validate('right', ['wrong', 'right'])); }); it('with empty channelId should pass', function () { - assert(EndorsementsValidator.validate('', new Set(['wrong', 'right']))); + assert(EndorsementsValidator.validate('', ['wrong', 'right'])); }); });