Skip to content

Commit

Permalink
Updated based on PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
TrevorJoelHarris committed Apr 17, 2024
1 parent c3004d9 commit d7a5048
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"type": "patch",
"comment": "Updated `authention.authenticate` so that it only accepts https URLs.",
"comment": "Updated `authentication.authenticate` so that it only accepts https URLs.",
"packageName": "@microsoft/teams-js",
"email": "trharris@microsoft.com",
"dependentChangeType": "patch"
Expand Down
2 changes: 1 addition & 1 deletion packages/teams-js/src/public/authentication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ export namespace authentication {

function throwIfStringNotValidHttpsUrl(link: string): void {
if (!isValidHttpsURL(new URL(link))) {
throw new Error(`${ErrorCode.INVALID_ARGUMENTS}: url must be https`);
throw new Error(`${ErrorCode.INVALID_ARGUMENTS}: ${link} must be valid https URL`);
}
}

Expand Down
16 changes: 11 additions & 5 deletions packages/teams-js/test/public/authentication.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,9 @@ describe('Testing authentication capability', () => {
};

const promise = authentication.authenticate(authenticationParams);
await expect(promise).rejects.toThrowError(`${ErrorCode.INVALID_ARGUMENTS}: url must be https`);
await expect(promise).rejects.toThrowError(
`${ErrorCode.INVALID_ARGUMENTS}: ${authenticationParams.url} must be valid https URL`,
);
});

it(`authentication.authenticate it should successfully handle auth success in a non-web client in legacy flow from ${context} context`, (done) => {
Expand Down Expand Up @@ -432,7 +434,9 @@ describe('Testing authentication capability', () => {
};

const promise = authentication.authenticate(authenticationParams);
await expect(promise).rejects.toThrowError(`${ErrorCode.INVALID_ARGUMENTS}: url must be https`);
await expect(promise).rejects.toThrowError(
`${ErrorCode.INVALID_ARGUMENTS}: ${authenticationParams.url} must be valid https URL`,
);
});
it(`authentication.authenticate should open a client window in web client in legacy flow from ${context} context`, async () => {
expect.assertions(5);
Expand Down Expand Up @@ -941,16 +945,18 @@ describe('Testing authentication capability', () => {
await expect(promise).resolves.toEqual(mockResult);
});

it(`authentication.authenticate should throw an error if non-https URL passed in`, async () => {
it(`authentication.authenticate should throw an error on non-web platforms if non-https URL passed in`, async () => {
await utils.initializeWithContext(context, HostClientType.desktop);
const authenticationParams: authentication.AuthenticatePopUpParameters = {
url: 'http://someurl',
url: 'http://someurl/',
width: 100,
height: 200,
isExternal: true,
};
const promise = authentication.authenticate(authenticationParams);
await expect(promise).rejects.toThrowError(`${ErrorCode.INVALID_ARGUMENTS}: url must be https`);
await expect(promise).rejects.toThrowError(
`${ErrorCode.INVALID_ARGUMENTS}: ${authenticationParams.url} must be valid https URL`,
);
});

it(`authentication.authenticate should handle auth failure with parameters in a non-web client from ${context} context`, async () => {
Expand Down

0 comments on commit d7a5048

Please sign in to comment.