Getting all tests to pass

Prithvi Kanherkar · Prithvi Kanherkar · commit 37d94e5dc593 · 2019-09-12T18:11:46.000-07:00
diff --git a/lib/msal-core/src/ServerRequestParameters.ts b/lib/msal-core/src/ServerRequestParameters.ts
@@ -137,7 +137,7 @@ export class ServerRequestParameters {
      * @param request
      */
     private validatePromptParameter (prompt: string) {
-        if (!([PromptState.LOGIN, PromptState.SELECT_ACCOUNT, PromptState.CONSENT, PromptState.NONE].indexOf(prompt) >= 0)) {
+        if ([PromptState.LOGIN, PromptState.SELECT_ACCOUNT, PromptState.CONSENT, PromptState.NONE].indexOf(prompt) < 0) {
             throw ClientConfigurationError.createInvalidPromptError(prompt);
         }
     }
diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts
@@ -238,11 +238,7 @@ export class UserAgentApplication {
         this.acquireTokenInProgress = false;
 
         // cache keys msal - typescript throws an error if any value other than "localStorage" or "sessionStorage" is passed
-        try {
-            this.cacheStorage = new AuthCache(this.clientId, this.config.cache.cacheLocation, this.inCookie);
-        } catch (e) {
-            throw ClientConfigurationError.createInvalidCacheLocationConfigError(this.config.cache.cacheLocation);
-        }
+        this.cacheStorage = new AuthCache(this.clientId, this.config.cache.cacheLocation, this.inCookie);
 
         // Initialize window handling code
         window.openedWindows = [];
diff --git a/lib/msal-core/src/cache/AuthCache.ts b/lib/msal-core/src/cache/AuthCache.ts
@@ -50,7 +50,12 @@ export class AuthCache extends BrowserStorage {// Singleton
             JSON.parse(key);
             return key;
         } catch (e) {
-            return `${CacheKeys.PREFIX}.${this.clientId}.${key}`;
+            if (key.startsWith(`${CacheKeys.PREFIX}.${this.clientId}`)) {
+                return key;
+            } else {
+                return `${CacheKeys.PREFIX}.${this.clientId}.${key}`;
+            }
+            
         }
     }
 
@@ -109,23 +114,21 @@ export class AuthCache extends BrowserStorage {// Singleton
 
     removeAcquireTokenEntries(state?: string): void {
         const storage = window[this.cacheLocation];
-        if (storage) {
-            let key: string;
-            for (key in storage) {
-                if (storage.hasOwnProperty(key)) {
-                    if ((key.indexOf(CacheKeys.AUTHORITY) !== -1 || key.indexOf(CacheKeys.ACQUIRE_TOKEN_ACCOUNT) !== 1) && (!state || key.indexOf(state) !== -1)) {
-                        const splitKey = key.split(Constants.resourceDelimiter);
-                        let state;
-                        if (splitKey.length > 1) {
-                            state = splitKey[1];
-                        }
-                        if (state && !this.tokenRenewalInProgress(state)) {
-                            this.removeItem(key);
-                            this.removeItem(CacheKeys.RENEW_STATUS + state);
-                            this.removeItem(CacheKeys.STATE_LOGIN);
-                            this.removeItem(CacheKeys.STATE_ACQ_TOKEN);
-                            this.setItemCookie(key, "", -1);
-                        }
+        let key: string;
+        for (key in storage) {
+            if (storage.hasOwnProperty(key)) {
+                if ((key.indexOf(CacheKeys.AUTHORITY) !== -1 || key.indexOf(CacheKeys.ACQUIRE_TOKEN_ACCOUNT) !== 1) && (!state || key.indexOf(state) !== -1)) {
+                    const resourceDelimSplitKey = key.split(Constants.resourceDelimiter);
+                    let keyState;
+                    if (resourceDelimSplitKey.length > 1) {
+                        keyState = resourceDelimSplitKey[resourceDelimSplitKey.length-1];
+                    }
+                    if (keyState && !this.tokenRenewalInProgress(keyState)) {
+                        this.removeItem(key);
+                        this.removeItem(CacheKeys.RENEW_STATUS + state);
+                        this.removeItem(CacheKeys.STATE_LOGIN);
+                        this.removeItem(CacheKeys.STATE_ACQ_TOKEN);
+                        this.setItemCookie(key, "", -1);
                     }
                 }
             }
diff --git a/lib/msal-core/src/cache/BrowserStorage.ts b/lib/msal-core/src/cache/BrowserStorage.ts
@@ -3,7 +3,7 @@
  * Licensed under the MIT License.
  */
 
-import { Constants, CacheKeys } from "../utils/Constants";
+import { CacheKeys } from "../utils/Constants";
 import { CacheLocation } from "../Configuration";
 import { ClientConfigurationError } from "../error/ClientConfigurationError";
 import { AuthError } from "../error/AuthError";
@@ -13,22 +13,18 @@ import { AuthError } from "../error/AuthError";
  */
 export class BrowserStorage {// Singleton
 
-    private localStorageSupported: boolean;
-    private sessionStorageSupported: boolean;
     protected cacheLocation: CacheLocation;
 
     constructor(cacheLocation: CacheLocation) {
         if (!window) {
             throw AuthError.createNoWindowObjectError("Browser storage class could not find window object");
         }
 
-        this.cacheLocation = cacheLocation;
-        this.localStorageSupported = typeof window[this.cacheLocation] !== "undefined" && window[this.cacheLocation] != null;
-        this.sessionStorageSupported = typeof window[cacheLocation] !== "undefined" && window[cacheLocation] != null;
-
-        if (!this.localStorageSupported && !this.sessionStorageSupported) {
-            throw ClientConfigurationError.createNoStorageSupportedError();
+        const storageSupported = typeof window[cacheLocation] !== "undefined" && window[cacheLocation] != null;
+        if (!storageSupported) {
+            throw ClientConfigurationError.createStorageNotSupportedError(cacheLocation);
         }
+        this.cacheLocation = cacheLocation;
     }
 
     // add value to storage
diff --git a/lib/msal-core/src/error/ClientConfigurationError.ts b/lib/msal-core/src/error/ClientConfigurationError.ts
@@ -12,13 +12,9 @@ export const ClientConfigurationErrorMessage = {
         code: "no_config_set",
         desc: "Configuration has not been set. Please call the UserAgentApplication constructor with a valid Configuration object."
     },
-    invalidCacheLocation: {
-        code: "invalid_cache_location",
-        desc: "The cache location provided is not valid."
-    },
-    noStorageSupported: {
-        code: "browser_storage_not_supported",
-        desc: "localStorage and sessionStorage are not supported."
+    storageNotSupported: {
+        code: "storage_not_supported",
+        desc: "The value for the cacheLocation is not supported."
     },
     noRedirectCallbacksSet: {
         code: "no_redirect_callbacks",
@@ -100,14 +96,9 @@ export class ClientConfigurationError extends ClientAuthError {
             `${ClientConfigurationErrorMessage.configurationNotSet.desc}`);
     }
 
-    static createInvalidCacheLocationConfigError(givenCacheLocation: string): ClientConfigurationError {
-        return new ClientConfigurationError(ClientConfigurationErrorMessage.invalidCacheLocation.code,
-            `${ClientConfigurationErrorMessage.invalidCacheLocation.desc} Provided value: ${givenCacheLocation}. Possible values are: ${Constants.cacheLocationLocal}, ${Constants.cacheLocationSession}.`);
-    }
-
-    static createNoStorageSupportedError() : ClientConfigurationError {
-        return new ClientConfigurationError(ClientConfigurationErrorMessage.noStorageSupported.code,
-            ClientConfigurationErrorMessage.noStorageSupported.desc);
+    static createStorageNotSupportedError(givenCacheLocation: string) : ClientConfigurationError {
+        return new ClientConfigurationError(ClientConfigurationErrorMessage.storageNotSupported.code,
+            `${ClientConfigurationErrorMessage.storageNotSupported.desc} Given location: ${givenCacheLocation}`);
     }
 
     static createRedirectCallbacksNotSetError(): ClientConfigurationError {
diff --git a/lib/msal-core/src/utils/Constants.ts b/lib/msal-core/src/utils/Constants.ts
@@ -51,9 +51,6 @@ export class Constants {
     static get openidScope(): string { return "openid"; }
     static get profileScope(): string { return "profile"; }
 
-    static get cacheLocationLocal(): CacheLocation { return "localStorage"; }
-    static get cacheLocationSession(): CacheLocation { return "sessionStorage"; }
-
     static get interactionTypeRedirect(): InteractionType { return "redirectInteraction"; }
     static get interactionTypePopup(): InteractionType { return "popupInteraction"; }
 }
@@ -140,11 +137,11 @@ export type InteractionType = "redirectInteraction" | "popupInteraction";
  * internal partners too, hence the choice of generic "string" type instead of the "enum"
  * @hidden
  */
-export enum PromptState {
-    LOGIN = "login",
-    SELECT_ACCOUNT = "select_account",
-    CONSENT = "consent",
-    NONE = "none",
+export const PromptState = {
+    LOGIN: "login",
+    SELECT_ACCOUNT: "select_account",
+    CONSENT: "consent",
+    NONE: "none",
 };
 
 /**
diff --git a/lib/msal-core/test/Storage.localStorage.spec.ts b/lib/msal-core/test/Storage.localStorage.spec.ts
@@ -2,18 +2,21 @@ import { expect } from "chai";
 import sinon from "sinon";
 import { BrowserStorage } from "../src/cache/BrowserStorage";
 import { AuthCache } from "../src/cache/AuthCache";
-import { Constants } from "../src";
+import { Constants, AuthError } from "../src";
 import { CacheKeys } from "../src/utils/Constants";
 import { AccessTokenKey } from "../src/cache/AccessTokenKey";
 import { AccessTokenValue } from "../src/cache/AccessTokenValue";
 import { Account } from "../src/Account";
+import { AuthErrorMessage } from "../src/error/AuthError";
+import { ClientConfigurationErrorMessage, ClientConfigurationError } from "../src/error/ClientConfigurationError";
 
 describe("CacheStorage.ts Class - Local Storage", function () {
-    let TEST_KEY = "test_key";
-    let TEST_VALUE = "test value";
-    let TEST_ACCOUNT_ID = "1234";
-    let TEST_STATE = "state5678";
-    let TEST_STATE2 = "state9012";
+    const TEST_KEY = "test_key";
+    const TEST_VALUE = "test value";
+    const TEST_ACCOUNT_ID = "1234";
+    const TEST_STATE = "state5678";
+    const TEST_STATE2 = "state9012";
+    const LOCAL_STORAGE = "localStorage";
     let cacheStorage : BrowserStorage;
     let ACCESS_TOKEN_KEY : AccessTokenKey;
     let ACCESS_TOKEN_VALUE : AccessTokenValue;
@@ -59,29 +62,58 @@ describe("CacheStorage.ts Class - Local Storage", function () {
         });
 
         it("parses the cache location correctly", function () {
-            cacheStorage = new BrowserStorage("localStorage");
+            cacheStorage = new BrowserStorage(LOCAL_STORAGE);
             cacheStorage.setItem(TEST_KEY, TEST_VALUE);
             expect(window.localStorage.getItem(TEST_KEY)).to.be.eq(TEST_VALUE);
         });
 
         it("throws error if cache location is not supported", function () {
-            // Cannot test with current tooling - will need to take a look
-            // Possibly wrapple as an option here? https://github.com/mroderick/wrapple
-        });
-
-        it("uses previous storage instance if one already exists", function () {
-            let oldCacheStorage = new BrowserStorage(Constants.cacheLocationLocal);
-            cacheStorage = new BrowserStorage(Constants.cacheLocationSession);
-            expect(cacheStorage).to.deep.eq(oldCacheStorage);
+            sinon.stub(window, LOCAL_STORAGE).value(null);
+            console.log(window.localStorage);
+            let authErr;
+            try {
+                cacheStorage = new BrowserStorage(LOCAL_STORAGE);
+            } catch (e) {
+                authErr = e;
+            }
+            expect(authErr instanceof ClientConfigurationError).to.be.true;
+            expect(authErr instanceof Error).to.be.true;
+            expect(authErr.errorCode).to.equal(ClientConfigurationErrorMessage.storageNotSupported.code);
+            expect(authErr.errorMessage).to.include(ClientConfigurationErrorMessage.storageNotSupported.desc);
+            expect(authErr.message).to.include(ClientConfigurationErrorMessage.storageNotSupported.desc);
+            expect(authErr.errorMessage).to.include(LOCAL_STORAGE);
+            expect(authErr.message).to.include(LOCAL_STORAGE);
+            expect(authErr.name).to.equal("ClientConfigurationError");
+            expect(authErr.stack).to.include("Storage.localStorage.spec.ts");
+            sinon.restore();
         });
 
+        it("throws error if window object does not exist", function () {
+            let authErr;
+            const oldWindow = window;
+            window = null;
+            try {
+                cacheStorage = new BrowserStorage(LOCAL_STORAGE);
+            } catch (e) {
+                authErr = e;
+            }
+            expect(authErr instanceof AuthError).to.be.true;
+            expect(authErr instanceof Error).to.be.true;
+            expect(authErr.errorCode).to.equal(AuthErrorMessage.noWindowObjectError.code);
+            expect(authErr.errorMessage).to.include(AuthErrorMessage.noWindowObjectError.desc);
+            expect(authErr.message).to.include(AuthErrorMessage.noWindowObjectError.desc);
+            expect(authErr.name).to.equal("AuthError");
+            expect(authErr.stack).to.include("Storage.localStorage.spec.ts");
+            window = oldWindow;
+        })
     });
 
     describe("localStorage access functions", function () {
 
         beforeEach(function () {
-            cacheStorage = new BrowserStorage("localStorage");
+            cacheStorage = new BrowserStorage(LOCAL_STORAGE);
             setTestCacheItems();
+            document.cookie = "";
         });
 
         afterEach(function () {
@@ -118,13 +150,15 @@ describe("CacheStorage.ts Class - Local Storage", function () {
             cacheStorage.setItemCookie(CacheKeys.NONCE_IDTOKEN, idTokenNonceString);
             expect(document.cookie).to.include(CacheKeys.NONCE_IDTOKEN);
             expect(document.cookie).to.include(idTokenNonceString);
+            cacheStorage.clearItemCookie(CacheKeys.NONCE_IDTOKEN);
         });
 
         it("tests getItemCookie ", function () {
             let idTokenNonceString = "idTokenNonce";
             cacheStorage.setItemCookie(CacheKeys.NONCE_IDTOKEN, idTokenNonceString);
             let retrievedItem = cacheStorage.getItemCookie(CacheKeys.NONCE_IDTOKEN);
             expect(retrievedItem).to.include(idTokenNonceString);
+            cacheStorage.clearItemCookie(CacheKeys.NONCE_IDTOKEN);
         });
 
         it("tests getCookieExpirationTime", function () {
@@ -173,8 +207,9 @@ describe("CacheStorage.ts Class - Local Storage", function () {
         let msalCacheStorage: AuthCache;
 
         beforeEach(function () {
-            msalCacheStorage = new AuthCache(MSAL_CLIENT_ID,"localStorage", true);
+            msalCacheStorage = new AuthCache(MSAL_CLIENT_ID,LOCAL_STORAGE, true);
             setTestCacheItems();
+            document.cookie = "";
         });
 
         afterEach(function () {
@@ -217,8 +252,9 @@ describe("CacheStorage.ts Class - Local Storage", function () {
         it("removeAcquireTokenEntries removes any acquireToken or authorityKey entries in the cache", function () {
             let acquireTokenAccountKey = AuthCache.generateAcquireTokenAccountKey(TEST_ACCOUNT_ID, TEST_STATE);
             let authorityKey = AuthCache.generateAuthorityKey(TEST_STATE);
-            window.localStorage.setItem(acquireTokenAccountKey, JSON.stringify(ACCOUNT));
-            window.localStorage.setItem(authorityKey, validAuthority);
+
+            window.localStorage.setItem(`${CacheKeys.PREFIX}.${MSAL_CLIENT_ID}.${acquireTokenAccountKey}`, JSON.stringify(ACCOUNT));
+            window.localStorage.setItem(`${CacheKeys.PREFIX}.${MSAL_CLIENT_ID}.${authorityKey}`, validAuthority);
 
             expect(msalCacheStorage.getItem(acquireTokenAccountKey)).to.be.eq(JSON.stringify(ACCOUNT));
             expect(msalCacheStorage.getItem(authorityKey)).to.be.eq(validAuthority);
@@ -235,10 +271,10 @@ describe("CacheStorage.ts Class - Local Storage", function () {
 
             let acquireTokenAccountKey2 = AuthCache.generateAcquireTokenAccountKey(TEST_ACCOUNT_ID, TEST_STATE2);
             let authorityKey2 = AuthCache.generateAuthorityKey(TEST_STATE2);
-            window.localStorage.setItem(acquireTokenAccountKey, JSON.stringify(ACCOUNT));
-            window.localStorage.setItem(authorityKey, validAuthority);
-            window.localStorage.setItem(acquireTokenAccountKey2, JSON.stringify(ACCOUNT));
-            window.localStorage.setItem(authorityKey2, validAuthority);
+            window.localStorage.setItem(`${CacheKeys.PREFIX}.${MSAL_CLIENT_ID}.${acquireTokenAccountKey}`, JSON.stringify(ACCOUNT));
+            window.localStorage.setItem(`${CacheKeys.PREFIX}.${MSAL_CLIENT_ID}.${authorityKey}`, validAuthority);
+            window.localStorage.setItem(`${CacheKeys.PREFIX}.${MSAL_CLIENT_ID}.${acquireTokenAccountKey2}`, JSON.stringify(ACCOUNT));
+            window.localStorage.setItem(`${CacheKeys.PREFIX}.${MSAL_CLIENT_ID}.${authorityKey2}`, validAuthority);
 
             expect(msalCacheStorage.getItem(acquireTokenAccountKey)).to.be.eq(JSON.stringify(ACCOUNT));
             expect(msalCacheStorage.getItem(authorityKey)).to.be.eq(validAuthority);
@@ -263,11 +299,17 @@ describe("CacheStorage.ts Class - Local Storage", function () {
             let stateLoginString = "stateLogin";
             let loginRequestString = "loginRequest";
             let stateAcquireTokenString = "stateAcquireToken";
+            console.log(document.cookie);
             msalCacheStorage.setItemCookie(CacheKeys.NONCE_IDTOKEN, idTokenNonceString);
+            console.log(document.cookie);
             msalCacheStorage.setItemCookie(CacheKeys.STATE_LOGIN, stateLoginString);
+            console.log(document.cookie);
             msalCacheStorage.setItemCookie(CacheKeys.LOGIN_REQUEST, loginRequestString);
+            console.log(document.cookie);
             msalCacheStorage.setItemCookie(CacheKeys.STATE_ACQ_TOKEN, stateAcquireTokenString);
+            console.log(document.cookie);
             msalCacheStorage.clearMsalCookie();
+            console.log(document.cookie);
             expect(document.cookie).to.be.empty;
         });
 
diff --git a/lib/msal-core/test/Storage.sessionStorage.spec.ts b/lib/msal-core/test/Storage.sessionStorage.spec.ts
diff --git a/lib/msal-core/test/UserAgentApplication.spec.ts b/lib/msal-core/test/UserAgentApplication.spec.ts
diff --git a/lib/msal-core/test/error/ClientConfigurationError.spec.ts b/lib/msal-core/test/error/ClientConfigurationError.spec.ts

Original file line number	Diff line number	Diff line change
`@@ -137,7 +137,7 @@ export class ServerRequestParameters {`
`137`	`137`	`* @param request`
`138`	`138`	`*/`
`139`	`139`	`private validatePromptParameter (prompt: string) {`
`140`		`- if (!([PromptState.LOGIN, PromptState.SELECT_ACCOUNT, PromptState.CONSENT, PromptState.NONE].indexOf(prompt) >= 0)) {`
	`140`	`+ if ([PromptState.LOGIN, PromptState.SELECT_ACCOUNT, PromptState.CONSENT, PromptState.NONE].indexOf(prompt) < 0) {`
`141`	`141`	`throw ClientConfigurationError.createInvalidPromptError(prompt);`
`142`	`142`	`}`
`143`	`143`	`}`