diff --git a/change/@microsoft-teams-js-81b620f2-8af1-4450-89b0-20600bac843e.json b/change/@microsoft-teams-js-81b620f2-8af1-4450-89b0-20600bac843e.json new file mode 100644 index 0000000000..080bd8793a --- /dev/null +++ b/change/@microsoft-teams-js-81b620f2-8af1-4450-89b0-20600bac843e.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Updated internal app id validation", + "packageName": "@microsoft/teams-js", + "email": "36546967+AE-MS@users.noreply.github.com", + "dependentChangeType": "patch" +} diff --git a/packages/teams-js/src/internal/appIdValidation.ts b/packages/teams-js/src/internal/appIdValidation.ts index 028b993a03..d08ac734a2 100644 --- a/packages/teams-js/src/internal/appIdValidation.ts +++ b/packages/teams-js/src/internal/appIdValidation.ts @@ -1,3 +1,5 @@ +import { hasScriptTags } from './utils'; + /** * This function can be used to validate if a string is a "valid" app id. * Valid is a relative term, in this case. Truly valid app ids are UUIDs as documented in the schema: @@ -10,7 +12,7 @@ * @throws Error with a message describing the exact validation violation */ export function validateStringAsAppId(potentialAppId: string): void { - if (doesStringContainScriptTags(potentialAppId)) { + if (hasScriptTags(potentialAppId)) { throw new Error(`Potential app id (${potentialAppId}) is invalid; it contains script tags.`); } @@ -25,11 +27,6 @@ export function validateStringAsAppId(potentialAppId: string): void { } } -export function doesStringContainScriptTags(str: string): boolean { - const scriptRegex = /]*>[\s\S]*?<\/script[^>]*>/gi; - return scriptRegex.test(str); -} - export const minimumValidAppIdLength = 4; export const maximumValidAppIdLength = 256; diff --git a/packages/teams-js/src/internal/utils.ts b/packages/teams-js/src/internal/utils.ts index f3f460803b..2eb373e4c1 100644 --- a/packages/teams-js/src/internal/utils.ts +++ b/packages/teams-js/src/internal/utils.ts @@ -460,43 +460,19 @@ export function fullyQualifyUrlString(fullOrRelativePath: string): URL { } /** - * The hasScriptTags function first decodes any HTML entities in the input string using the decodeHTMLEntities function. - * It then tries to decode the result as a URI component. If the URI decoding fails (which would throw an error), it assumes that the input was not encoded and uses the original input. - * Next, it defines a regular expression scriptRegex that matches any string that starts with ')).toBe(true); - expect(doesStringContainScriptTags('')).toBe(true); - expect(doesStringContainScriptTags('')).toBe(true); - }); - - test('should return false for strings without script tags', () => { - expect(doesStringContainScriptTags('This is a test string')).toBe(false); - expect(doesStringContainScriptTags('8e6523aa-97f9-49ad-8614-75cae22f6597')).toBe(false); - expect(doesStringContainScriptTags('com.microsoft.teamspace.tab.youtube')).toBe(false); - expect(doesStringContainScriptTags('
This is a div
')).toBe(false); - expect(doesStringContainScriptTags('Link')).toBe(false); - }); - - test('should return true for strings with script tags containing newlines and spaces', () => { - expect(doesStringContainScriptTags('')).toBe(true); - expect(doesStringContainScriptTags('')).toBe(true); - }); - - test('should return false for empty string', () => { - expect(doesStringContainScriptTags('')).toBe(false); - }); - - test('should return true for strings with multiple script tags', () => { - expect(doesStringContainScriptTags('')).toBe( - true, - ); - }); -}); - // Since there are plenty of tests validating the individual validation functions, these tests are intentionally not as // comprehensive as those. It executes a few basic tests and also validates that the error messages thrown are as expected. describe('validateStringAsAppId', () => { @@ -98,15 +66,15 @@ describe('validateStringAsAppId', () => { }); test('should throw error with "script" in message for app id containing script tag', () => { - expect(() => validateStringAsAppId('')).toThrowError(/script/); + expect(() => validateStringAsAppId('')).toThrowError(/script/i); }); test('should throw error with "length" in message for app id too long or too short', () => { - expect(() => validateStringAsAppId('a')).toThrowError(/length/); - expect(() => validateStringAsAppId('a'.repeat(maximumValidAppIdLength))).toThrowError(/length/); + expect(() => validateStringAsAppId('a')).toThrowError(/length/i); + expect(() => validateStringAsAppId('a'.repeat(maximumValidAppIdLength))).toThrowError(/length/i); }); test('should throw error with "printable" in message for app id containing non-printable characters', () => { - expect(() => validateStringAsAppId('hello\u0080world')).toThrowError(/printable/); + expect(() => validateStringAsAppId('hello\u0080world')).toThrowError(/printable/i); }); }); diff --git a/packages/teams-js/test/internal/utils.spec.ts b/packages/teams-js/test/internal/utils.spec.ts index 740745de14..f36024e895 100644 --- a/packages/teams-js/test/internal/utils.spec.ts +++ b/packages/teams-js/test/internal/utils.spec.ts @@ -3,6 +3,7 @@ import { compareSDKVersions, createTeamsAppLink, getBase64StringFromBlob, + hasScriptTags, validateId, validateUrl, validateUuid, @@ -367,6 +368,105 @@ describe('utils', () => { }); }); + describe('hasScriptTags', () => { + test('detects plain opening ')).toBe(true); + }); + + test('detects HTML entity encoded opening tag', () => { + expect(hasScriptTags('')).toBe(true); + }); + + test('detects HTML entity encoded closing tag', () => { + expect(hasScriptTags('</script>')).toBe(true); + }); + + test('detects URI encoded closing tag', () => { + expect(hasScriptTags('%3C/script%3E')).toBe(true); + }); + + test('returns false for strings without ')).toBe(true); + }); + + test('returns false for empty string', () => { + expect(hasScriptTags('')).toBe(false); + }); + + test('detects multiple ')).toBe(true); + }); + + test('detects ')).toBe(true); + expect(hasScriptTags('')).toBe(true); + expect(hasScriptTags('')).toBe(true); + }); + + test('detects HTML entity encoded ')).toBe(true); + }); + + test('detects plain opening ')).toBe(true); + }); + + test('detects plain opening ')).toBe(true); + }); + + test('detects nested ')).toBe(true); + }); + + test('detects ')).toBe(true); + expect(hasScriptTags('')).toBe(true); + }); + + test('detects ')).toBe(true); + expect(hasScriptTags('<SCRIPT>alert("XSS")</SCRIPT>')).toBe(true); + expect(hasScriptTags('%3CSCRIPT%3Ealert("XSS")%3C/SCRIPT%3E')).toBe(true); + }); + + test('detects mixed casing ')).toBe(true); + expect(hasScriptTags('<sCRipT>alert("XSS")</sCRipT>')).toBe(true); + expect(hasScriptTags('%3CsCRipT%3Ealert("XSS")%3C/sCRipT%3E')).toBe(true); + }); + }); + describe('UUID class tests', () => { describe('validateUuid', () => { it('should throw error when id is undefined', async () => { diff --git a/packages/teams-js/test/private/externalAppAuthentication.spec.ts b/packages/teams-js/test/private/externalAppAuthentication.spec.ts index 0802b822ad..ccd2ebc89c 100644 --- a/packages/teams-js/test/private/externalAppAuthentication.spec.ts +++ b/packages/teams-js/test/private/externalAppAuthentication.spec.ts @@ -147,46 +147,43 @@ describe('externalAppAuthentication', () => { expect.assertions(1); await utils.initializeWithContext(frameContext); utils.setRuntimeConfig({ apiVersion: 2, supports: { externalAppAuthentication: {} } }); - const invalidAppId = 'invalidAppIdwith'; - try { - await externalAppAuthentication.authenticateAndResendRequest( - invalidAppId, - testAuthRequest, - testOriginalRequest, - ); - } catch (e) { - expect(e).toEqual(new Error('App id is not valid.')); - } - }); - it(`should throw error on invalid app ID if it contains non printabe ASCII characters with context - ${frameContext}`, async () => { + const invalidAppId = 'invalidAppIdWith'; + await expect( + async () => + await externalAppAuthentication.authenticateAndResendRequest( + invalidAppId, + testAuthRequest, + testOriginalRequest, + ), + ).rejects.toThrowError(/script/i); + }); + it(`should throw error on invalid app ID if it contains non printable ASCII characters with context - ${frameContext}`, async () => { expect.assertions(1); await utils.initializeWithContext(frameContext); utils.setRuntimeConfig({ apiVersion: 2, supports: { externalAppAuthentication: {} } }); const invalidAppId = 'appId\u0000'; - try { - await externalAppAuthentication.authenticateAndResendRequest( - invalidAppId, - testAuthRequest, - testOriginalRequest, - ); - } catch (e) { - expect(e).toEqual(new Error('App id is not valid.')); - } + await expect( + async () => + await externalAppAuthentication.authenticateAndResendRequest( + invalidAppId, + testAuthRequest, + testOriginalRequest, + ), + ).rejects.toThrowError(/characters/i); }); it(`should throw error on invalid app ID if its size exceeds 256 characters with context - ${frameContext}`, async () => { expect.assertions(1); await utils.initializeWithContext(frameContext); utils.setRuntimeConfig({ apiVersion: 2, supports: { externalAppAuthentication: {} } }); const invalidAppId = 'a'.repeat(257); - try { - await externalAppAuthentication.authenticateAndResendRequest( - invalidAppId, - testAuthRequest, - testOriginalRequest, - ); - } catch (e) { - expect(e).toEqual(new Error('App id is not valid.')); - } + await expect( + async () => + await externalAppAuthentication.authenticateAndResendRequest( + invalidAppId, + testAuthRequest, + testOriginalRequest, + ), + ).rejects.toThrowError(/length/i); }); it(`should throw error on original request info command ID exceeds max size with context - ${frameContext}`, async () => { expect.assertions(1); @@ -441,45 +438,42 @@ describe('externalAppAuthentication', () => { await utils.initializeWithContext(frameContext); utils.setRuntimeConfig({ apiVersion: 2, supports: { externalAppAuthentication: {} } }); const invalidAppId = 'invalidAppIdwith'; - try { - await externalAppAuthentication.authenticateWithSSOAndResendRequest( - invalidAppId, - testAuthRequest, - testOriginalRequest, - ); - } catch (e) { - expect(e).toEqual(new Error('App id is not valid.')); - } - }); - it(`should throw error on invalid app ID if it contains non printabe ASCII characters with context - ${frameContext}`, async () => { + await expect( + async () => + await externalAppAuthentication.authenticateWithSSOAndResendRequest( + invalidAppId, + testAuthRequest, + testOriginalRequest, + ), + ).rejects.toThrowError(/script/i); + }); + it(`should throw error on invalid app ID if it contains non printable ASCII characters with context - ${frameContext}`, async () => { expect.assertions(1); await utils.initializeWithContext(frameContext); utils.setRuntimeConfig({ apiVersion: 2, supports: { externalAppAuthentication: {} } }); const invalidAppId = 'appId\u0000'; - try { - await externalAppAuthentication.authenticateWithSSOAndResendRequest( - invalidAppId, - testAuthRequest, - testOriginalRequest, - ); - } catch (e) { - expect(e).toEqual(new Error('App id is not valid.')); - } + await expect( + async () => + await externalAppAuthentication.authenticateWithSSOAndResendRequest( + invalidAppId, + testAuthRequest, + testOriginalRequest, + ), + ).rejects.toThrowError(/characters/i); }); it(`should throw error on invalid app ID if if its size exceeds 256 characters with context - ${frameContext}`, async () => { expect.assertions(1); await utils.initializeWithContext(frameContext); utils.setRuntimeConfig({ apiVersion: 2, supports: { externalAppAuthentication: {} } }); const invalidAppId = 'a'.repeat(257); - try { - await externalAppAuthentication.authenticateWithSSOAndResendRequest( - invalidAppId, - testAuthRequest, - testOriginalRequest, - ); - } catch (e) { - expect(e).toEqual(new Error('App id is not valid.')); - } + await expect( + async () => + await externalAppAuthentication.authenticateWithSSOAndResendRequest( + invalidAppId, + testAuthRequest, + testOriginalRequest, + ), + ).rejects.toThrowError(/length/i); }); it(`should throw error on original request info command ID exceeds max size with context - ${frameContext}`, async () => { @@ -870,11 +864,11 @@ describe('externalAppAuthentication', () => { expect.assertions(1); await utils.initializeWithContext(frameContext); utils.setRuntimeConfig({ apiVersion: 2, supports: { externalAppAuthentication: {} } }); - const invalidsingInUrl = new URL('https://example.com?param='); + const invalidStringInUrl = new URL('https://example.com?param='); try { await externalAppAuthentication.authenticateWithPowerPlatformConnectorPlugins( titleId, - invalidsingInUrl, + invalidStringInUrl, testPPCWindowParameters, ); } catch (e) { @@ -886,11 +880,11 @@ describe('externalAppAuthentication', () => { await utils.initializeWithContext(frameContext); utils.setRuntimeConfig({ apiVersion: 2, supports: { externalAppAuthentication: {} } }); // eslint-disable-next-line @microsoft/sdl/no-insecure-url - const invalidsingInUrl = new URL('http://pishingsite.com'); + const invalidStringInUrl = new URL('http://adatum.com'); try { await externalAppAuthentication.authenticateWithPowerPlatformConnectorPlugins( titleId, - invalidsingInUrl, + invalidStringInUrl, testPPCWindowParameters, ); } catch (e) { @@ -901,7 +895,7 @@ describe('externalAppAuthentication', () => { expect.assertions(1); await utils.initializeWithContext(frameContext); utils.setRuntimeConfig({ apiVersion: 2, supports: { externalAppAuthentication: {} } }); - let dummyUrl = 'https://dummy.com?param='; + let dummyUrl = 'https://contoso.com?param='; while (dummyUrl.length < 2050) { dummyUrl += 'a'; } diff --git a/packages/teams-js/test/private/externalAppCardActions.spec.ts b/packages/teams-js/test/private/externalAppCardActions.spec.ts index 4e55838ce4..2fd8a7ea85 100644 --- a/packages/teams-js/test/private/externalAppCardActions.spec.ts +++ b/packages/teams-js/test/private/externalAppCardActions.spec.ts @@ -82,34 +82,28 @@ describe('externalAppCardActions', () => { expect.assertions(1); await utils.initializeWithContext(frameContext); utils.setRuntimeConfig({ apiVersion: 2, supports: { externalAppCardActions: {} } }); - const invalidAppId = 'invalidAppIdwith'; - try { - await externalAppCardActions.processActionSubmit(invalidAppId, testActionSubmitPayload); - } catch (e) { - expect(e).toEqual(new Error('App id is not valid.')); - } + const invalidAppId = 'invalidAppIdWith'; + await expect( + async () => await externalAppCardActions.processActionSubmit(invalidAppId, testActionSubmitPayload), + ).rejects.toThrowError(/script/i); }); - it(`should throw error on invalid app ID if it contains non printabe ASCII characters. context - ${frameContext}`, async () => { + it(`should throw error on invalid app ID if it contains non printable ASCII characters. context - ${frameContext}`, async () => { expect.assertions(1); await utils.initializeWithContext(frameContext); utils.setRuntimeConfig({ apiVersion: 2, supports: { externalAppCardActions: {} } }); const invalidAppId = 'appId\u0000'; - try { - await externalAppCardActions.processActionSubmit(invalidAppId, testActionSubmitPayload); - } catch (e) { - expect(e).toEqual(new Error('App id is not valid.')); - } + await expect( + async () => await externalAppCardActions.processActionSubmit(invalidAppId, testActionSubmitPayload), + ).rejects.toThrowError(/characters/i); }); it(`should throw error on invalid app ID if it its size exceeds 256 characters. context - ${frameContext}`, async () => { expect.assertions(1); await utils.initializeWithContext(frameContext); utils.setRuntimeConfig({ apiVersion: 2, supports: { externalAppCardActions: {} } }); const invalidAppId = 'a'.repeat(257); - try { - await externalAppCardActions.processActionSubmit(invalidAppId, testActionSubmitPayload); - } catch (e) { - expect(e).toEqual(new Error('App id is not valid.')); - } + await expect( + async () => await externalAppCardActions.processActionSubmit(invalidAppId, testActionSubmitPayload), + ).rejects.toThrowError(/length/i); }); } else { it(`should not allow calls from ${frameContext} context`, async () => { @@ -188,33 +182,27 @@ describe('externalAppCardActions', () => { await utils.initializeWithContext(frameContext); utils.setRuntimeConfig({ apiVersion: 2, supports: { externalAppCardActions: {} } }); const invalidAppId = 'invalidAppIdwith'; - try { - await externalAppCardActions.processActionOpenUrl(invalidAppId, testUrl); - } catch (e) { - expect(e).toEqual(new Error('App id is not valid.')); - } + await expect( + async () => await externalAppCardActions.processActionOpenUrl(invalidAppId, testUrl), + ).rejects.toThrowError(/script/i); }); - it(`should throw error on invalid app ID if it contains non printabe ASCII characters with context - ${frameContext}`, async () => { + it(`should throw error on invalid app ID if it contains non printable ASCII characters with context - ${frameContext}`, async () => { expect.assertions(1); await utils.initializeWithContext(frameContext); utils.setRuntimeConfig({ apiVersion: 2, supports: { externalAppCardActions: {} } }); const invalidAppId = 'appId\u0000'; - try { - await externalAppCardActions.processActionOpenUrl(invalidAppId, testUrl); - } catch (e) { - expect(e).toEqual(new Error('App id is not valid.')); - } + await expect( + async () => await externalAppCardActions.processActionOpenUrl(invalidAppId, testUrl), + ).rejects.toThrowError(/characters/i); }); it(`should throw error on invalid app ID if its size exceeds 256 characters with context - ${frameContext}`, async () => { expect.assertions(1); await utils.initializeWithContext(frameContext); utils.setRuntimeConfig({ apiVersion: 2, supports: { externalAppCardActions: {} } }); const invalidAppId = 'a'.repeat(257); - try { - await externalAppCardActions.processActionOpenUrl(invalidAppId, testUrl); - } catch (e) { - expect(e).toEqual(new Error('App id is not valid.')); - } + await expect( + async () => await externalAppCardActions.processActionOpenUrl(invalidAppId, testUrl), + ).rejects.toThrowError(/length/i); }); } else { it(`should not allow calls from ${frameContext} context`, async () => { diff --git a/packages/teams-js/test/private/externalAppCommands.spec.ts b/packages/teams-js/test/private/externalAppCommands.spec.ts index 0d90f94f3a..7f624fdc7b 100644 --- a/packages/teams-js/test/private/externalAppCommands.spec.ts +++ b/packages/teams-js/test/private/externalAppCommands.spec.ts @@ -97,34 +97,28 @@ describe('externalAppCommands', () => { expect.assertions(1); await utils.initializeWithContext(frameContext); utils.setRuntimeConfig({ apiVersion: 2, supports: { externalAppCommands: {} } }); - const invalidAppId = 'invalidAppIdwith'; - try { - await externalAppCommands.processActionCommand(invalidAppId, mockCommandId, mockExtractedParam); - } catch (e) { - expect(e).toEqual(new Error('App id is not valid.')); - } + const invalidAppId = 'invalidAppIdWith'; + await expect( + async () => await externalAppCommands.processActionCommand(invalidAppId, mockCommandId, mockExtractedParam), + ).rejects.toThrowError(/script/i); }); - it(`should throw error on invalid app ID if it contains non printabe ASCII characters with context - ${frameContext}`, async () => { + it(`should throw error on invalid app ID if it contains non printable ASCII characters with context - ${frameContext}`, async () => { expect.assertions(1); await utils.initializeWithContext(frameContext); utils.setRuntimeConfig({ apiVersion: 2, supports: { externalAppCommands: {} } }); const invalidAppId = 'appId\u0000'; - try { - await externalAppCommands.processActionCommand(invalidAppId, mockCommandId, mockExtractedParam); - } catch (e) { - expect(e).toEqual(new Error('App id is not valid.')); - } + await expect( + async () => await externalAppCommands.processActionCommand(invalidAppId, mockCommandId, mockExtractedParam), + ).rejects.toThrowError(/characters/i); }); it(`should throw error on invalid app ID if its size exceeds 256 characters with context - ${frameContext}`, async () => { expect.assertions(1); await utils.initializeWithContext(frameContext); utils.setRuntimeConfig({ apiVersion: 2, supports: { externalAppCommands: {} } }); const invalidAppId = 'a'.repeat(257); - try { - await externalAppCommands.processActionCommand(invalidAppId, mockCommandId, mockExtractedParam); - } catch (e) { - expect(e).toEqual(new Error('App id is not valid.')); - } + await expect( + async () => await externalAppCommands.processActionCommand(invalidAppId, mockCommandId, mockExtractedParam), + ).rejects.toThrowError(/length/i); }); } else { it(`should not allow calls from ${frameContext} context`, async () => { diff --git a/packages/teams-js/test/public/appId.spec.ts b/packages/teams-js/test/public/appId.spec.ts index 7e2b5f22ab..16bd2a9e50 100644 --- a/packages/teams-js/test/public/appId.spec.ts +++ b/packages/teams-js/test/public/appId.spec.ts @@ -10,15 +10,15 @@ describe('doesStringContainNonPrintableCharacters', () => { }); test('should throw error with "script" in message for app id containing script tag', () => { - expect(() => new AppId('')).toThrowError(/script/); + expect(() => new AppId('')).toThrowError(/script/i); }); test('should throw error with "length" in message for app id too long or too short', () => { - expect(() => new AppId('a')).toThrowError(/length/); - expect(() => new AppId('a'.repeat(maximumValidAppIdLength))).toThrowError(/length/); + expect(() => new AppId('a')).toThrowError(/length/i); + expect(() => new AppId('a'.repeat(maximumValidAppIdLength))).toThrowError(/length/i); }); test('should throw error with "printable" in message for app id containing non-printable characters', () => { - expect(() => new AppId('hello\u0080world')).toThrowError(/printable/); + expect(() => new AppId('hello\u0080world')).toThrowError(/printable/i); }); });