Skip to content

Commit

Permalink
Merge branch 'main' into baljesingh/TestAppGroupedMode
Browse files Browse the repository at this point in the history
  • Loading branch information
KangxuanYe authored Sep 25, 2024
2 parents 955c189 + 60eb045 commit 789b243
Show file tree
Hide file tree
Showing 15 changed files with 262 additions and 215 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Updated internal app id validation",
"packageName": "@microsoft/teams-js",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Added logging for current teamsjs instance and timestamps",
"packageName": "@microsoft/teams-js",
"email": "[email protected]",
"dependentChangeType": "patch"
}
9 changes: 3 additions & 6 deletions packages/teams-js/src/internal/appIdValidation.ts
Original file line number Diff line number Diff line change
@@ -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:
Expand All @@ -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.`);
}

Expand All @@ -25,11 +27,6 @@ export function validateStringAsAppId(potentialAppId: string): void {
}
}

export function doesStringContainScriptTags(str: string): boolean {
const scriptRegex = /<script[^>]*>[\s\S]*?<\/script[^>]*>/gi;
return scriptRegex.test(str);
}

export const minimumValidAppIdLength = 4;
export const maximumValidAppIdLength = 256;

Expand Down
12 changes: 12 additions & 0 deletions packages/teams-js/src/internal/telemetry.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
import { debug as registerLogger, Debugger } from 'debug';

import { UUID } from './uuidObject';

// Each teamsjs instance gets a unique identifier that will be prepended to every log statement
export const teamsJsInstanceIdentifier = new UUID();

// Every log statement will get prepended with the teamsJsInstanceIdentifier and a timestamp
const originalFormatArgsFunction = registerLogger.formatArgs;
registerLogger.formatArgs = function (args) {
args[0] = `(${new Date().toISOString()}): ${args[0]} [${teamsJsInstanceIdentifier.toString()}]`;
originalFormatArgsFunction.call(this, args);
};

const topLevelLogger = registerLogger('teamsJs');

/**
Expand Down
44 changes: 10 additions & 34 deletions packages/teams-js/src/internal/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 <script (followed by any characters), then has any characters (including newlines),
* and ends with </script> (preceded by any characters).
* Finally, it uses the test method to check if the decoded input matches this regular expression. The function returns true if a match is found and false otherwise.
* @param input URL converted to string to pattern match
* Detects if there are any script tags in a given string, even if they are Uri encoded or encoded as HTML entities.
* @param input string to test for script tags
* @returns true if the input string contains a script tag, false otherwise
*/
function hasScriptTags(input: string): boolean {
let decodedInput;
try {
const decodedHTMLInput = decodeHTMLEntities(input);
decodedInput = decodeURIComponent(decodedHTMLInput);
} catch (e) {
// input was not encoded, use it as is
decodedInput = input;
}
const scriptRegex = /<script[^>]*>[\s\S]*?<\/script[^>]*>/gi;
return scriptRegex.test(decodedInput);
}
export function hasScriptTags(input: string): boolean {
const openingScriptTagRegex = /<script[^>]*>|&lt;script[^&]*&gt;|%3Cscript[^%]*%3E/gi;
const closingScriptTagRegex = /<\/script[^>]*>|&lt;\/script[^&]*&gt;|%3C\/script[^%]*%3E/gi;

/**
* The decodeHTMLEntities function replaces HTML entities in the input string with their corresponding characters.
*/
function decodeHTMLEntities(input: string): string {
const entityMap = new Map<string, string>([
['&lt;', '<'],
['&gt;', '>'],
['&amp;', '&'],
['&quot;', '"'],
['&#39;', "'"],
['&#x2F;', '/'],
]);
entityMap.forEach((value, key) => {
input = input.replace(new RegExp(key, 'gi'), value);
});
return input;
const openingOrClosingScriptTagRegex = new RegExp(
`${openingScriptTagRegex.source}|${closingScriptTagRegex.source}`,
'gi',
);
return openingOrClosingScriptTagRegex.test(input);
}

function isIdLengthValid(id: string): boolean {
Expand Down
14 changes: 8 additions & 6 deletions packages/teams-js/src/private/externalAppAuthentication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { sendMessageToParentAsync } from '../internal/communication';
import { ensureInitialized } from '../internal/internalAPIs';
import { ApiName, ApiVersionNumber, getApiVersionTag } from '../internal/telemetry';
import { validateId, validateUrl } from '../internal/utils';
import { AppId } from '../public';
import { errorNotSupportedOnPlatform, FrameContexts } from '../public/constants';
import { runtime } from '../public/runtime';

Expand Down Expand Up @@ -351,7 +352,7 @@ export namespace externalAppAuthentication {
if (!isSupported()) {
throw errorNotSupportedOnPlatform;
}
validateId(appId, new Error('App id is not valid.'));
const typeSafeAppId: AppId = new AppId(appId);
validateOriginalRequestInfo(originalRequestInfo);

// Ask the parent window to open an authentication window with the parameters provided by the caller.
Expand All @@ -362,7 +363,7 @@ export namespace externalAppAuthentication {
),
'externalAppAuthentication.authenticateAndResendRequest',
[
appId,
typeSafeAppId.toString(),
originalRequestInfo,
authenticateParameters.url.href,
authenticateParameters.width,
Expand Down Expand Up @@ -395,14 +396,15 @@ export namespace externalAppAuthentication {
if (!isSupported()) {
throw errorNotSupportedOnPlatform;
}
validateId(appId, new Error('App id is not valid.'));
const typeSafeAppId: AppId = new AppId(appId);

return sendMessageToParentAsync(
getApiVersionTag(
externalAppAuthenticationTelemetryVersionNumber,
ApiName.ExternalAppAuthentication_AuthenticateWithSSO,
),
'externalAppAuthentication.authenticateWithSSO',
[appId, authTokenRequest.claims, authTokenRequest.silent],
[typeSafeAppId.toString(), authTokenRequest.claims, authTokenRequest.silent],
).then(([wasSuccessful, error]: [boolean, InvokeError]) => {
if (!wasSuccessful) {
throw error;
Expand Down Expand Up @@ -431,7 +433,7 @@ export namespace externalAppAuthentication {
if (!isSupported()) {
throw errorNotSupportedOnPlatform;
}
validateId(appId, new Error('App id is not valid.'));
const typeSafeAppId: AppId = new AppId(appId);

validateOriginalRequestInfo(originalRequestInfo);

Expand All @@ -441,7 +443,7 @@ export namespace externalAppAuthentication {
ApiName.ExternalAppAuthentication_AuthenticateWithSSOAndResendRequest,
),
'externalAppAuthentication.authenticateWithSSOAndResendRequest',
[appId, originalRequestInfo, authTokenRequest.claims, authTokenRequest.silent],
[typeSafeAppId.toString(), originalRequestInfo, authTokenRequest.claims, authTokenRequest.silent],
).then(([wasSuccessful, response]: [boolean, IInvokeResponse | InvokeErrorWrapper]) => {
if (wasSuccessful && response.responseType != null) {
return response as IInvokeResponse;
Expand Down
10 changes: 5 additions & 5 deletions packages/teams-js/src/private/externalAppCardActions.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { sendMessageToParentAsync } from '../internal/communication';
import { ensureInitialized } from '../internal/internalAPIs';
import { ApiName, ApiVersionNumber, getApiVersionTag } from '../internal/telemetry';
import { validateId } from '../internal/utils';
import { AppId } from '../public';
import { errorNotSupportedOnPlatform, FrameContexts } from '../public/constants';
import { runtime } from '../public/runtime';
import { ExternalAppErrorCode } from './constants';
Expand Down Expand Up @@ -96,15 +96,15 @@ export namespace externalAppCardActions {
if (!isSupported()) {
throw errorNotSupportedOnPlatform;
}
validateId(appId, new Error('App id is not valid.'));
const typeSafeAppId: AppId = new AppId(appId);

return sendMessageToParentAsync<[boolean, ActionSubmitError]>(
getApiVersionTag(
externalAppCardActionsTelemetryVersionNumber,
ApiName.ExternalAppCardActions_ProcessActionSubmit,
),
'externalAppCardActions.processActionSubmit',
[appId, actionSubmitPayload],
[typeSafeAppId.toString(), actionSubmitPayload],
).then(([wasSuccessful, error]: [boolean, ActionSubmitError]) => {
if (!wasSuccessful) {
throw error;
Expand Down Expand Up @@ -135,14 +135,14 @@ export namespace externalAppCardActions {
if (!isSupported()) {
throw errorNotSupportedOnPlatform;
}
validateId(appId, new Error('App id is not valid.'));
const typeSafeAppId: AppId = new AppId(appId);
return sendMessageToParentAsync<[ActionOpenUrlError, ActionOpenUrlType]>(
getApiVersionTag(
externalAppCardActionsTelemetryVersionNumber,
ApiName.ExternalAppCardActions_ProcessActionOpenUrl,
),
'externalAppCardActions.processActionOpenUrl',
[appId, url.href, fromElement],
[typeSafeAppId.toString(), url.href, fromElement],
).then(([error, response]: [ActionOpenUrlError, ActionOpenUrlType]) => {
if (error) {
throw error;
Expand Down
6 changes: 3 additions & 3 deletions packages/teams-js/src/private/externalAppCommands.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { sendMessageToParentAsync } from '../internal/communication';
import { ensureInitialized } from '../internal/internalAPIs';
import { ApiName, ApiVersionNumber, getApiVersionTag } from '../internal/telemetry';
import { validateId } from '../internal/utils';
import { AppId } from '../public';
import { errorNotSupportedOnPlatform, FrameContexts } from '../public/constants';
import { runtime } from '../public/runtime';
import { ExternalAppErrorCode } from './constants';
Expand Down Expand Up @@ -135,12 +135,12 @@ export namespace externalAppCommands {
if (!isSupported()) {
throw errorNotSupportedOnPlatform;
}
validateId(appId, new Error('App id is not valid.'));
const typeSafeAppId: AppId = new AppId(appId);

const [error, response] = await sendMessageToParentAsync<[ActionCommandError, IActionCommandResponse]>(
getApiVersionTag(externalAppCommandsTelemetryVersionNumber, ApiName.ExternalAppCommands_ProcessActionCommands),
ApiName.ExternalAppCommands_ProcessActionCommands,
[appId, commandId, extractedParameters],
[typeSafeAppId.toString(), commandId, extractedParameters],
);
if (error) {
throw error;
Expand Down
16 changes: 9 additions & 7 deletions packages/teams-js/src/public/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -750,17 +750,19 @@ export namespace app {
const scriptUsageWarning =
'Today, teamsjs can only be used from a single script or you may see undefined behavior. This log line is used to help detect cases where teamsjs is loaded multiple times -- it is always written. The presence of the log itself does not indicate a multi-load situation, but multiples of these log lines will. If you would like to use teamjs from more than one script at the same time, please open an issue at https://github.com/OfficeDev/microsoft-teams-library-js/issues';
if (!currentScriptSrc || currentScriptSrc.length === 0) {
appLogger(
'teamsjs version %s is being used from a script tag embedded directly in your html. %s',
version,
scriptUsageWarning,
);
appLogger('teamsjs is being used from a script tag embedded directly in your html. %s', scriptUsageWarning);
} else {
appLogger('teamsjs version %s is being used from %s. %s', version, currentScriptSrc, scriptUsageWarning);
appLogger('teamsjs is being used from %s. %s', currentScriptSrc, scriptUsageWarning);
}
}

// This is called right away to make sure that we capture which script is being executed correctly
// This is called right away to make sure that we capture which script is being executed and important stats about the current teamsjs instance
appLogger(
'teamsjs instance is version %s, starting at %s UTC (%s local)',
version,
new Date().toISOString(),
new Date().toLocaleString(),
);
logWhereTeamsJsIsBeingUsed();

/**
Expand Down
40 changes: 4 additions & 36 deletions packages/teams-js/test/internal/appIdValidation.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {
doesStringContainNonPrintableCharacters,
doesStringContainScriptTags,
isStringWithinAppIdLengthLimits,
maximumValidAppIdLength,
minimumValidAppIdLength,
Expand Down Expand Up @@ -58,37 +57,6 @@ describe('isStringWithinAppIdLengthLimits', () => {
});
});

describe('doesStringContainScriptTags', () => {
test('should return true for strings containing script tags', () => {
expect(doesStringContainScriptTags('<script>alert("Hello")</script>')).toBe(true);
expect(doesStringContainScriptTags('<script src="example.js"></script>')).toBe(true);
expect(doesStringContainScriptTags('<script type="text/javascript">console.log("test")</script>')).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('<div>This is a div</div>')).toBe(false);
expect(doesStringContainScriptTags('<a href="example.com">Link</a>')).toBe(false);
});

test('should return true for strings with script tags containing newlines and spaces', () => {
expect(doesStringContainScriptTags('<script>\nalert("Hello")\n</script>')).toBe(true);
expect(doesStringContainScriptTags('<script> \n console.log("test") \n </script>')).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('<script>alert("Hello")</script><script>console.log("test")</script>')).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', () => {
Expand All @@ -98,15 +66,15 @@ describe('validateStringAsAppId', () => {
});

test('should throw error with "script" in message for app id containing script tag', () => {
expect(() => validateStringAsAppId('<script>alert("Hello")</script>')).toThrowError(/script/);
expect(() => validateStringAsAppId('<script>alert("Hello")</script>')).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);
});
});
Loading

0 comments on commit 789b243

Please sign in to comment.