Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[msal-common] Fix issue where extraScopesToConsent was not appending scopes correctly #1854

Merged
merged 2 commits into from
Jun 30, 2020
Merged

[msal-common] Fix issue where extraScopesToConsent was not appending scopes correctly #1854

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b8323fc
update appendScope to ensure we are using internal APIs and formattin…
pkanher617 Jun 29, 2020
70d6ac3
Merge branch 'dev' into extrascopes-append-error-2.0
Jun 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 3 additions & 4 deletions lib/msal-common/src/request/ScopeSet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,9 @@ export class ScopeSet {;
* @param newScope
*/
appendScope(newScope: string): void {
if (StringUtils.isEmpty(newScope)) {
throw ClientAuthError.createAppendEmptyScopeToSetError(newScope);
if (!StringUtils.isEmpty(newScope)) {
this.scopes.add(newScope.trim().toLowerCase());
}
this.scopes.add(newScope.trim().toLowerCase());
}

/**
Expand All @@ -87,7 +86,7 @@ export class ScopeSet {;
*/
appendScopes(newScopes: Array<string>): void {
try {
newScopes.forEach(newScope => this.scopes.add(newScope));
newScopes.forEach(newScope => this.appendScope(newScope));
} catch (e) {
throw ClientAuthError.createAppendScopeSetError(e);
}
Expand Down
24 changes: 17 additions & 7 deletions lib/msal-common/test/request/ScopeSet.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,19 +127,19 @@ describe("ScopeSet.ts", () => {
});

it("appendScope() does nothing if given scope is empty, null or undefined", () => {
const testScopes = [testScope];
const setAddSpy = sinon.spy(Set.prototype, "add");

expect(() => scopes.appendScope("")).to.throw(ClientAuthError);
expect(() => scopes.appendScope("")).to.throw(ClientAuthErrorMessage.appendEmptyScopeError.desc);
scopes.appendScope("");
expect(setAddSpy.called).to.be.false;
expect(scopes.asArray()).to.be.deep.eq(testScopes);

expect(() => scopes.appendScope(null)).to.throw(ClientAuthError);
expect(() => scopes.appendScope(null)).to.throw(ClientAuthErrorMessage.appendEmptyScopeError.desc);
scopes.appendScope(null);
expect(setAddSpy.called).to.be.false;
expect(scopes.asArray()).to.be.deep.eq(testScopes);

expect(() => scopes.appendScope(undefined)).to.throw(ClientAuthError);
expect(() => scopes.appendScope(undefined)).to.throw(ClientAuthErrorMessage.appendEmptyScopeError.desc);
scopes.appendScope(undefined);
expect(setAddSpy.called).to.be.false;
expect(scopes.asArray()).to.be.deep.eq(testScopes);
});

it("appendScopes() throws error if given array is null or undefined", () => {
Expand All @@ -164,6 +164,16 @@ describe("ScopeSet.ts", () => {
expect(scopes.asArray()).to.contain(testScope3);
});

it("appendScopes() trims and converts scopes to lower case before adding", () => {
const testScope2 = "TestScope2";
const expectedTestScope2 = "testscope2";
const testScope3 = " testscope3 ";
const expectedTestScope3 = "testscope3";
scopes.appendScopes([testScope2, testScope3]);
expect(scopes.asArray()).to.contain(expectedTestScope2);
expect(scopes.asArray()).to.contain(expectedTestScope3);
})

it("appendScopes() does not add duplicate scopes", () => {
const unchangedScopes = new ScopeSet([testScope, Constants.OFFLINE_ACCESS_SCOPE]);
const scopeArr = unchangedScopes.asArray();
Expand Down