Merge pull request #1854 from AzureAD/extrascopes-append-error-2.0

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

Author: Prithvi Kanherkar

lib/msal-common/src/request/ScopeSet.ts

@@ -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());
     }
 
     /**
@@ -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);
         }

lib/msal-common/test/request/ScopeSet.spec.ts

@@ -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", () => {
@@ -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();