diff --git a/lib/msal-browser/src/app/PublicClientApplication.ts b/lib/msal-browser/src/app/PublicClientApplication.ts index 9a338baf9b..57703e52e5 100644 --- a/lib/msal-browser/src/app/PublicClientApplication.ts +++ b/lib/msal-browser/src/app/PublicClientApplication.ts @@ -24,7 +24,6 @@ import { CacheSchemaType, AuthenticationResult, SilentFlowRequest, - AccountEntity, IAccount } from "@azure/msal-common"; import { buildConfiguration, Configuration } from "../config/Configuration"; @@ -270,25 +269,7 @@ export class PublicClientApplication { * @param {@link (AuthenticationParameters:type)} */ async loginRedirect(request: AuthorizationUrlRequest): Promise { - try { - // Preflight request - const validRequest: AuthorizationUrlRequest = this.preflightRequest(request); - - // Create auth code request and generate PKCE params - const authCodeRequest: AuthorizationCodeRequest = await this.generateAuthorizationCodeRequest(validRequest); - - // Create redirect interaction handler. - const interactionHandler = new RedirectHandler(this.authModule, this.browserStorage); - - // Create login url. - const navigateUrl = await this.authModule.createLoginUrl(validRequest); - - // Show the UI once the url has been created. Response will come back in the hash, which will be handled in the handleRedirectCallback function. - interactionHandler.initiateAuthRequest(navigateUrl, authCodeRequest, this.browserCrypto); - } catch (e) { - this.browserStorage.cleanRequest(); - throw e; - } + return this.acquireTokenRedirect(this.generateLoginRequest(request)); } /** @@ -313,7 +294,7 @@ export class PublicClientApplication { const interactionHandler = new RedirectHandler(this.authModule, this.browserStorage); // Create acquire token url. - const navigateUrl = await this.authModule.createAcquireTokenUrl(validRequest); + const navigateUrl = await this.authModule.createUrl(validRequest); // Show the UI once the url has been created. Response will come back in the hash, which will be handled in the handleRedirectCallback function. interactionHandler.initiateAuthRequest(navigateUrl, authCodeRequest, this.browserCrypto); @@ -335,22 +316,7 @@ export class PublicClientApplication { * @returns {Promise.} - a promise that is fulfilled when this function has completed, or rejected if an error was raised. Returns the {@link AuthResponse} object */ async loginPopup(request: AuthorizationUrlRequest): Promise { - try { - // Preflight request - const validRequest: AuthorizationUrlRequest = this.preflightRequest(request); - - // Create auth code request and generate PKCE params - const authCodeRequest: AuthorizationCodeRequest = await this.generateAuthorizationCodeRequest(validRequest); - - // Create login url, which will by default append the client id scope to the call. - const navigateUrl = await this.authModule.createLoginUrl(validRequest); - - // Acquire token with popup - return await this.popupTokenHelper(navigateUrl, authCodeRequest); - } catch (e) { - this.browserStorage.cleanRequest(); - throw e; - } + return this.acquireTokenPopup(this.generateLoginRequest(request)); } /** @@ -369,7 +335,7 @@ export class PublicClientApplication { const authCodeRequest: AuthorizationCodeRequest = await this.generateAuthorizationCodeRequest(validRequest); // Create acquire token url. - const navigateUrl = await this.authModule.createAcquireTokenUrl(validRequest); + const navigateUrl = await this.authModule.createUrl(validRequest); // Acquire token with popup return await this.popupTokenHelper(navigateUrl, authCodeRequest); @@ -441,7 +407,7 @@ export class PublicClientApplication { const scopeString = silentRequest.scopes ? silentRequest.scopes.join(" ") : ""; // Create authorize request url - const navigateUrl = await this.authModule.createLoginUrl(silentRequest); + const navigateUrl = await this.authModule.createUrl(silentRequest); return this.silentTokenHelper(navigateUrl, authCodeRequest, scopeString); } @@ -480,7 +446,7 @@ export class PublicClientApplication { const authCodeRequest: AuthorizationCodeRequest = await this.generateAuthorizationCodeRequest(silentAuthUrlRequest); // Create authorize request url - const navigateUrl = await this.authModule.createAcquireTokenUrl(silentAuthUrlRequest); + const navigateUrl = await this.authModule.createUrl(silentAuthUrlRequest); // Get scopeString for iframe ID const scopeString = silentRequest.scopes ? silentRequest.scopes.join(" ") : ""; @@ -590,6 +556,20 @@ export class PublicClientApplication { return (this.browserStorage.getItem(this.browserStorage.generateCacheKey(BrowserConstants.INTERACTION_STATUS_KEY), CacheSchemaType.TEMPORARY) as string) === BrowserConstants.INTERACTION_IN_PROGRESS_VALUE; } + /** + * Generates a request that will contain the openid and profile scopes. + * @param request + */ + private generateLoginRequest(request: AuthorizationUrlRequest): AuthorizationUrlRequest { + const loginRequest = { ...request }; + if (!loginRequest.scopes) { + loginRequest.scopes = [Constants.OPENID_SCOPE, Constants.PROFILE_SCOPE]; + } else { + loginRequest.scopes.push(Constants.OPENID_SCOPE, Constants.PROFILE_SCOPE); + } + return loginRequest; + } + /** * Helper to validate app environment before making a request. */ @@ -601,7 +581,7 @@ export class PublicClientApplication { if (this.interactionInProgress()) { throw BrowserAuthError.createInteractionInProgressError(); } - + return this.initializeRequest(request); } diff --git a/lib/msal-browser/src/cache/BrowserStorage.ts b/lib/msal-browser/src/cache/BrowserStorage.ts index be3336b810..9028cb0a3d 100644 --- a/lib/msal-browser/src/cache/BrowserStorage.ts +++ b/lib/msal-browser/src/cache/BrowserStorage.ts @@ -2,7 +2,7 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the MIT License. */ -import { ICacheStorage, Constants, PersistentCacheKeys, StringUtils, AuthorizationCodeRequest, ICrypto, CacheSchemaType, AccountEntity, IdTokenEntity, CacheHelper, CredentialType, Credential, AccessTokenEntity, RefreshTokenEntity, AppMetadataEntity } from "@azure/msal-common"; +import { ICacheStorage, Constants, PersistentCacheKeys, StringUtils, AuthorizationCodeRequest, ICrypto, CacheSchemaType, AccountEntity, IdTokenEntity, CacheHelper, CredentialType, AccessTokenEntity, RefreshTokenEntity, AppMetadataEntity } from "@azure/msal-common"; import { CacheOptions } from "../config/Configuration"; import { BrowserAuthError } from "../error/BrowserAuthError"; import { BrowserConfigurationAuthError } from "../error/BrowserConfigurationAuthError"; diff --git a/lib/msal-browser/src/utils/BrowserConstants.ts b/lib/msal-browser/src/utils/BrowserConstants.ts index 9fcfa477ab..d6178254fc 100644 --- a/lib/msal-browser/src/utils/BrowserConstants.ts +++ b/lib/msal-browser/src/utils/BrowserConstants.ts @@ -43,7 +43,7 @@ export enum TemporaryCacheKeys { ACQUIRE_TOKEN_ACCOUNT = "acquireToken.account", SESSION_STATE = "session.state", REQUEST_STATE = "request.state", - NONCE_IDTOKEN = "nonce.idtoken", + NONCE_IDTOKEN = "nonce.id_token", ORIGIN_URI = "request.origin", RENEW_STATUS = "token.renew.status", URL_HASH = "urlHash", diff --git a/lib/msal-browser/test/app/PublicClientApplication.spec.ts b/lib/msal-browser/test/app/PublicClientApplication.spec.ts index 862f61bcb6..981dcfc72c 100644 --- a/lib/msal-browser/test/app/PublicClientApplication.spec.ts +++ b/lib/msal-browser/test/app/PublicClientApplication.spec.ts @@ -166,6 +166,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { idToken: testServerTokenResponse.body.id_token, idTokenClaims: testIdTokenClaims, accessToken: testServerTokenResponse.body.access_token, + fromCache: false, expiresOn: new Date(Date.now() + (testServerTokenResponse.body.expires_in * 1000)), account: testAccount }; @@ -266,6 +267,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { idToken: testServerTokenResponse.body.id_token, idTokenClaims: testIdTokenClaims, accessToken: testServerTokenResponse.body.access_token, + fromCache: false, expiresOn: new Date(Date.now() + (testServerTokenResponse.body.expires_in * 1000)), account: testAccount }; @@ -317,7 +319,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); const loginRequest: AuthorizationUrlRequest = { redirectUri: TEST_URIS.TEST_REDIR_URI, - scopes: ["user.read", TEST_CONFIG.MSAL_CLIENT_ID] + scopes: ["user.read"] }; pca.loginRedirect(loginRequest); }); @@ -346,7 +348,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { it("Caches token request correctly", async () => { const tokenRequest: AuthorizationUrlRequest = { redirectUri: TEST_URIS.TEST_REDIR_URI, - scopes: [TEST_CONFIG.MSAL_CLIENT_ID], + scopes: [], correlationId: RANDOM_TEST_GUID }; sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ @@ -362,7 +364,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { const browserCrypto = new CryptoOps(); await pca.loginRedirect(tokenRequest); const cachedRequest: AuthorizationCodeRequest = JSON.parse(browserCrypto.base64Decode(browserStorage.getItem(browserStorage.generateCacheKey(TemporaryCacheKeys.REQUEST_PARAMS), CacheSchemaType.TEMPORARY) as string)); - expect(cachedRequest.scopes).to.be.deep.eq([TEST_CONFIG.MSAL_CLIENT_ID]); + expect(cachedRequest.scopes).to.be.deep.eq(TEST_CONFIG.DEFAULT_SCOPES); expect(cachedRequest.codeVerifier).to.be.deep.eq(TEST_CONFIG.TEST_VERIFIER); expect(cachedRequest.authority).to.be.deep.eq(`${Constants.DEFAULT_AUTHORITY}/`); expect(cachedRequest.correlationId).to.be.deep.eq(RANDOM_TEST_GUID); @@ -379,7 +381,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); const loginUrlErr = "loginUrlErr"; - sinon.stub(SPAClient.prototype, "createLoginUrl").throws(new BrowserAuthError(loginUrlErr)); + sinon.stub(SPAClient.prototype, "createUrl").throws(new BrowserAuthError(loginUrlErr)); await expect(pca.loginRedirect(emptyRequest)).to.be.rejectedWith(loginUrlErr); await expect(pca.loginRedirect(emptyRequest)).to.be.rejectedWith(BrowserAuthError); expect(browserStorage.getKeys()).to.be.empty; @@ -400,7 +402,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { sinon.stub(IdToken, "extractIdToken").returns(idTokenClaims); const browserStorage: BrowserStorage = new BrowserStorage(TEST_CONFIG.MSAL_CLIENT_ID, cacheConfig); browserStorage.setItem(PersistentCacheKeys.ADAL_ID_TOKEN, TEST_TOKENS.IDTOKEN_V1, CacheSchemaType.TEMPORARY); - const loginUrlSpy = sinon.spy(SPAClient.prototype, "createLoginUrl"); + const loginUrlSpy = sinon.spy(SPAClient.prototype, "createUrl"); sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER @@ -443,7 +445,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { sinon.stub(IdToken, "extractIdToken").returns(idTokenClaims); const browserStorage: BrowserStorage = new BrowserStorage(TEST_CONFIG.MSAL_CLIENT_ID, cacheConfig); browserStorage.setItem(PersistentCacheKeys.ADAL_ID_TOKEN, TEST_TOKENS.IDTOKEN_V1, CacheSchemaType.TEMPORARY); - const loginUrlSpy = sinon.spy(SPAClient.prototype, "createLoginUrl"); + const loginUrlSpy = sinon.spy(SPAClient.prototype, "createUrl"); sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER @@ -493,7 +495,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); const loginRequest: AuthorizationUrlRequest = { redirectUri: TEST_URIS.TEST_REDIR_URI, - scopes: ["user.read", TEST_CONFIG.MSAL_CLIENT_ID] + scopes: ["user.read", "openid", "profile"] }; pca.acquireTokenRedirect(loginRequest); }); @@ -558,7 +560,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); const loginUrlErr = "loginUrlErr"; - sinon.stub(SPAClient.prototype, "createAcquireTokenUrl").throws(new BrowserAuthError(loginUrlErr)); + sinon.stub(SPAClient.prototype, "createUrl").throws(new BrowserAuthError(loginUrlErr)); await expect(pca.acquireTokenRedirect(emptyRequest)).to.be.rejectedWith(loginUrlErr); await expect(pca.acquireTokenRedirect(emptyRequest)).to.be.rejectedWith(BrowserAuthError); expect(browserStorage.getKeys()).to.be.empty; @@ -580,7 +582,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { sinon.stub(IdToken, "extractIdToken").returns(idTokenClaims); const browserStorage: BrowserStorage = new BrowserStorage(TEST_CONFIG.MSAL_CLIENT_ID, cacheConfig); browserStorage.setItem(PersistentCacheKeys.ADAL_ID_TOKEN, TEST_TOKENS.IDTOKEN_V1, CacheSchemaType.TEMPORARY); - const acquireTokenUrlSpy = sinon.spy(SPAClient.prototype, "createAcquireTokenUrl"); + const acquireTokenUrlSpy = sinon.spy(SPAClient.prototype, "createUrl"); sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER @@ -623,7 +625,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { sinon.stub(IdToken, "extractIdToken").returns(idTokenClaims); const browserStorage: BrowserStorage = new BrowserStorage(TEST_CONFIG.MSAL_CLIENT_ID, cacheConfig); browserStorage.setItem(PersistentCacheKeys.ADAL_ID_TOKEN, TEST_TOKENS.IDTOKEN_V1, CacheSchemaType.TEMPORARY); - const acquireTokenUrlSpy = sinon.spy(SPAClient.prototype, "createAcquireTokenUrl"); + const acquireTokenUrlSpy = sinon.spy(SPAClient.prototype, "createUrl"); sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER @@ -697,10 +699,11 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { idToken: testServerTokenResponse.id_token, idTokenClaims: testIdTokenClaims, accessToken: testServerTokenResponse.access_token, + fromCache: false, expiresOn: new Date(Date.now() + (testServerTokenResponse.expires_in * 1000)), account: testAccount }; - sinon.stub(SPAClient.prototype, "createLoginUrl").resolves(testNavUrl); + sinon.stub(SPAClient.prototype, "createUrl").resolves(testNavUrl); sinon.stub(PopupHandler.prototype, "initiateAuthRequest").callsFake((requestUrl: string): Window => { expect(requestUrl).to.be.eq(testNavUrl); return window; @@ -718,7 +721,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { it("catches error and cleans cache before rethrowing", async () => { const testError = "Error in creating a login url"; - sinon.stub(SPAClient.prototype, "createLoginUrl").resolves(testNavUrl); + sinon.stub(SPAClient.prototype, "createUrl").resolves(testNavUrl); sinon.stub(PopupHandler.prototype, "initiateAuthRequest").throws(testError); sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ challenge: TEST_CONFIG.TEST_CHALLENGE, @@ -781,10 +784,11 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { idToken: testServerTokenResponse.id_token, idTokenClaims: testIdTokenClaims, accessToken: testServerTokenResponse.access_token, + fromCache: false, expiresOn: new Date(Date.now() + (testServerTokenResponse.expires_in * 1000)), account: testAccount }; - sinon.stub(SPAClient.prototype, "createAcquireTokenUrl").resolves(testNavUrl); + sinon.stub(SPAClient.prototype, "createUrl").resolves(testNavUrl); sinon.stub(PopupHandler.prototype, "initiateAuthRequest").callsFake((requestUrl: string): Window => { expect(requestUrl).to.be.eq(testNavUrl); return window; @@ -805,7 +809,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { it("catches error and cleans cache before rethrowing", async () => { const testError = "Error in creating a login url"; - sinon.stub(SPAClient.prototype, "createAcquireTokenUrl").resolves(testNavUrl); + sinon.stub(SPAClient.prototype, "createUrl").resolves(testNavUrl); sinon.stub(PopupHandler.prototype, "initiateAuthRequest").throws(testError); sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ challenge: TEST_CONFIG.TEST_CHALLENGE, @@ -883,10 +887,11 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { idToken: testServerTokenResponse.id_token, idTokenClaims: testIdTokenClaims, accessToken: testServerTokenResponse.access_token, + fromCache: false, expiresOn: new Date(Date.now() + (testServerTokenResponse.expires_in * 1000)), account: testAccount }; - sinon.stub(SPAClient.prototype, "createLoginUrl").resolves(testNavUrl); + sinon.stub(SPAClient.prototype, "createUrl").resolves(testNavUrl); const loadFrameSyncSpy = sinon.spy(SilentHandler.prototype, "loadFrameSync"); sinon.stub(SilentHandler.prototype, "monitorFrameForHash").resolves(TEST_HASHES.TEST_SUCCESS_CODE_HASH); sinon.stub(SilentHandler.prototype, "handleCodeResponse").resolves(testTokenResponse); @@ -940,6 +945,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { idToken: testServerTokenResponse.id_token, idTokenClaims: testIdTokenClaims, accessToken: testServerTokenResponse.access_token, + fromCache: false, expiresOn: new Date(Date.now() + (testServerTokenResponse.expires_in * 1000)), account: testAccount }; @@ -1006,10 +1012,11 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { idToken: testServerTokenResponse.id_token, idTokenClaims: testIdTokenClaims, accessToken: testServerTokenResponse.access_token, + fromCache: false, expiresOn: new Date(Date.now() + (testServerTokenResponse.expires_in * 1000)), account: testAccount }; - const createAcqTokenStub = sinon.stub(SPAClient.prototype, "createAcquireTokenUrl").resolves(testNavUrl); + const createAcqTokenStub = sinon.stub(SPAClient.prototype, "createUrl").resolves(testNavUrl); const silentTokenHelperStub = sinon.stub(pca, "silentTokenHelper").resolves(testTokenResponse); sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ challenge: TEST_CONFIG.TEST_CHALLENGE, diff --git a/lib/msal-browser/test/utils/StringConstants.ts b/lib/msal-browser/test/utils/StringConstants.ts index 7656beb45b..4cf9d6fd8a 100644 --- a/lib/msal-browser/test/utils/StringConstants.ts +++ b/lib/msal-browser/test/utils/StringConstants.ts @@ -29,7 +29,7 @@ export const TEST_CONFIG = { TEST_VERIFIER: "Y5LnOOlAWK0kt370Bjm0ZcrW9Sc2pMXR1slip9TFZXoyUV8Y8lCn0WHXyyQ1QcTnALMbrUAj85dC7WIe6gYqc8o8jsHCezP3xiUNB143A5IfwtSfO6Kb8oy7pNqcT9vN", TEST_CHALLENGE: "JsjesZmxJwehdhNY9kvyr0QOeSMEvryY_EHZo3BKrqg", TOKEN_TYPE_BEARER: "Bearer", - DEFAULT_SCOPES: ["openid", "profile", "offline_access"] + DEFAULT_SCOPES: ["openid", "profile"] }; // Test Tokens @@ -151,5 +151,5 @@ export const ALTERNATE_OPENID_CONFIG_RESPONSE = { } }; -export const testNavUrl = `https://login.microsoftonline.com/common/oauth2/v2.0/authorize?response_type=code&client_id=${TEST_CONFIG.MSAL_CLIENT_ID}&scope=user.read%20openid%20profile%20offline_access&redirect_uri=https%3A%2F%2Flocalhost%3A8081%2Findex.html&client-request-id=${RANDOM_TEST_GUID}&code_challenge=JsjesZmxJwehdhNY9kvyr0QOeSMEvryY_EHZo3BKrqg&code_challenge_method=S256&state=${RANDOM_TEST_GUID}&nonce=${RANDOM_TEST_GUID}&client_info=1&x-client-SKU=msal.js.browser&x-client-VER=2.0.0-beta.2&x-client-OS=&x-client-CPU=&response_mode=fragment`; +export const testNavUrl = `https://login.microsoftonline.com/common/oauth2/v2.0/authorize?response_type=code&client_id=${TEST_CONFIG.MSAL_CLIENT_ID}&scope=user.read%20openid%20profile&redirect_uri=https%3A%2F%2Flocalhost%3A8081%2Findex.html&client-request-id=${RANDOM_TEST_GUID}&code_challenge=JsjesZmxJwehdhNY9kvyr0QOeSMEvryY_EHZo3BKrqg&code_challenge_method=S256&state=${RANDOM_TEST_GUID}&nonce=${RANDOM_TEST_GUID}&client_info=1&x-client-SKU=msal.js.browser&x-client-VER=2.0.0-beta.2&x-client-OS=&x-client-CPU=&response_mode=fragment`; export const testLogoutUrl = `https://login.microsoftonline.com/common/oauth2/v2.0/logout?post_logout_redirect_uri=${encodeURIComponent(TEST_URIS.TEST_REDIR_URI)}`; diff --git a/lib/msal-common/src/cache/UnifiedCacheManager.ts b/lib/msal-common/src/cache/UnifiedCacheManager.ts index f233f60f7d..a78c0c1822 100644 --- a/lib/msal-common/src/cache/UnifiedCacheManager.ts +++ b/lib/msal-common/src/cache/UnifiedCacheManager.ts @@ -23,14 +23,17 @@ import { StringUtils } from "../utils/StringUtils"; import { IdTokenEntity } from "./entities/IdTokenEntity"; import { AccessTokenEntity } from "./entities/AccessTokenEntity"; import { RefreshTokenEntity } from "./entities/RefreshTokenEntity"; +import { ScopeSet } from "../request/ScopeSet"; export class UnifiedCacheManager implements ICacheManager { // Storage interface private cacheStorage: ICacheStorage; private inMemory: boolean; + private clientId: string; - constructor(cacheImpl: ICacheStorage, storeInMemory: boolean) { + constructor(cacheImpl: ICacheStorage, clientId: string, storeInMemory: boolean) { this.cacheStorage = cacheImpl; + this.clientId = clientId; this.inMemory = storeInMemory; } @@ -63,11 +66,10 @@ export class UnifiedCacheManager implements ICacheManager { * saves a cache record * @param cacheRecord */ - saveCacheRecord(cacheRecord: CacheRecord): void { + saveCacheRecord(cacheRecord: CacheRecord, responseScopes: ScopeSet): void { this.saveAccount(cacheRecord.account); this.saveCredential(cacheRecord.idToken); - // TODO: Check for scope intersection and delete accessToken with intersecting scopes - this.saveCredential(cacheRecord.accessToken); + this.saveAccessToken(cacheRecord.accessToken, responseScopes); this.saveCredential(cacheRecord.refreshToken); } @@ -99,6 +101,30 @@ export class UnifiedCacheManager implements ICacheManager { ); } + /** + * saves access token credential + * @param credential + */ + saveAccessToken(credential: AccessTokenEntity, responseScopes: ScopeSet): void { + const currentTokenCache = this.getCredentialsFilteredBy({ + clientId: credential.clientId, + credentialType: CredentialType.ACCESS_TOKEN, + environment: credential.environment, + homeAccountId: credential.homeAccountId, + realm: credential.realm + }); + const currentAccessTokens: AccessTokenEntity[] = Object.values(currentTokenCache.accessTokens) as AccessTokenEntity[]; + if (currentAccessTokens) { + currentAccessTokens.forEach((tokenEntity) => { + const tokenScopeSet = ScopeSet.fromString(tokenEntity.target); + if (tokenScopeSet.intersectingScopeSets(responseScopes)) { + this.removeCredential(tokenEntity); + } + }); + } + this.saveCredential(credential); + } + /** * Given account key retrieve an account * @param key @@ -232,10 +258,7 @@ export class UnifiedCacheManager implements ICacheManager { let matches: boolean = true; // don't parse any non-credential type cache entities const credType = CacheHelper.getCredentialType(cacheKey); - if ( - credType === - Constants.NOT_DEFINED - ) { + if (credType === Constants.NOT_DEFINED) { return; } @@ -270,11 +293,8 @@ export class UnifiedCacheManager implements ICacheManager { } // idTokens do not have "target", target specific refreshTokens do exist for some types of authentication - if ( - !StringUtils.isEmpty(target) && - credType === - CredentialType.ACCESS_TOKEN - ) { + // TODO: Add case for target specific refresh tokens + if (!StringUtils.isEmpty(target) && credType === CredentialType.ACCESS_TOKEN) { matches = matches && CacheHelper.matchTarget(entity, target); } diff --git a/lib/msal-common/src/cache/interface/ICacheStorage.ts b/lib/msal-common/src/cache/interface/ICacheStorage.ts index 6bf103e2a3..9afaf429c8 100644 --- a/lib/msal-common/src/cache/interface/ICacheStorage.ts +++ b/lib/msal-common/src/cache/interface/ICacheStorage.ts @@ -57,4 +57,3 @@ export interface ICacheStorage { */ clear(): void; } - diff --git a/lib/msal-common/src/cache/utils/CacheHelper.ts b/lib/msal-common/src/cache/utils/CacheHelper.ts index cb3df0ec29..d308af6315 100644 --- a/lib/msal-common/src/cache/utils/CacheHelper.ts +++ b/lib/msal-common/src/cache/utils/CacheHelper.ts @@ -13,6 +13,7 @@ import { import { IAccount } from "../../account/IAccount"; import { AccountEntity } from "../entities/AccountEntity"; import { Credential } from "../entities/Credential"; +import { ScopeSet } from "../../request/ScopeSet"; export class CacheHelper { /** @@ -89,7 +90,7 @@ export class CacheHelper { /** * - * @param key + * @param entity * @param credentialType */ static matchCredentialType(entity: Credential, credentialType: string): boolean { @@ -98,7 +99,7 @@ export class CacheHelper { /** * - * @param key + * @param entity * @param clientId */ static matchClientId(entity: Credential, clientId: string): boolean { @@ -107,7 +108,7 @@ export class CacheHelper { /** * - * @param key + * @param entity * @param realm */ static matchRealm(entity: AccountEntity | Credential, realm: string): boolean { @@ -115,29 +116,14 @@ export class CacheHelper { } /** - * - * @param key + * Returns true if the target scopes are a subset of the current entity's scopes, false otherwise. + * @param entity * @param target */ static matchTarget(entity: Credential, target: string): boolean { - return CacheHelper.targetsSubset(entity.target, target); - } - - /** - * returns a boolean if the sets of scopes intersect (scopes are stored as "target" in cache) - * @param target - * @param credentialTarget - */ - static targetsSubset(credentialTarget: string, target: string): boolean { - const targetSet = new Set(target.split(" ")); - const credentialTargetSet = new Set(credentialTarget.split(" ")); - - let isSubset = true; - targetSet.forEach((key) => { - isSubset = isSubset && credentialTargetSet.has(key); - }); - - return isSubset; + const entityScopeSet: ScopeSet = ScopeSet.fromString(entity.target); + const requestTargetScopeSet: ScopeSet = ScopeSet.fromString(target); + return entityScopeSet.containsScopeSet(requestTargetScopeSet); } /** diff --git a/lib/msal-common/src/client/AuthorizationCodeClient.ts b/lib/msal-common/src/client/AuthorizationCodeClient.ts index f632a677d7..2ec6707d79 100644 --- a/lib/msal-common/src/client/AuthorizationCodeClient.ts +++ b/lib/msal-common/src/client/AuthorizationCodeClient.ts @@ -60,7 +60,7 @@ export class AuthorizationCodeClient extends BaseClient { ); responseHandler.validateTokenResponse(response.body); - const tokenResponse = await responseHandler.generateAuthenticationResult( + const tokenResponse = responseHandler.generateAuthenticationResult( response.body, this.defaultAuthority ); @@ -94,10 +94,7 @@ export class AuthorizationCodeClient extends BaseClient { // validate the redirectUri (to be a non null value) parameterBuilder.addRedirectUri(request.redirectUri); - const scopeSet = new ScopeSet( - request.scopes || [], - this.config.authOptions.clientId, - false); + const scopeSet = new ScopeSet(request.scopes || []); parameterBuilder.addScopes(scopeSet); // add code: user set, not validated @@ -123,9 +120,7 @@ export class AuthorizationCodeClient extends BaseClient { parameterBuilder.addClientId(this.config.authOptions.clientId); - const scopeSet = new ScopeSet(request.scopes || [], - this.config.authOptions.clientId, - false); + const scopeSet = new ScopeSet(request.scopes || []); if (request.extraScopesToConsent) { scopeSet.appendScopes(request.extraScopesToConsent); } diff --git a/lib/msal-common/src/client/BaseClient.ts b/lib/msal-common/src/client/BaseClient.ts index 99a09e0808..ae9df5727f 100644 --- a/lib/msal-common/src/client/BaseClient.ts +++ b/lib/msal-common/src/client/BaseClient.ts @@ -63,6 +63,7 @@ export abstract class BaseClient { // Initialize serialized cache manager this.unifiedCacheManager = new UnifiedCacheManager( this.cacheStorage, + this.config.authOptions.clientId, this.config.systemOptions.storeInMemory ); diff --git a/lib/msal-common/src/client/DeviceCodeClient.ts b/lib/msal-common/src/client/DeviceCodeClient.ts index f7d5e77813..2463a09bcf 100644 --- a/lib/msal-common/src/client/DeviceCodeClient.ts +++ b/lib/msal-common/src/client/DeviceCodeClient.ts @@ -96,9 +96,7 @@ export class DeviceCodeClient extends BaseClient { const parameterBuilder: RequestParameterBuilder = new RequestParameterBuilder(); - const scopeSet = new ScopeSet(request.scopes || [], - this.config.authOptions.clientId, - false); + const scopeSet = new ScopeSet(request.scopes || []); parameterBuilder.addScopes(scopeSet); parameterBuilder.addClientId(this.config.authOptions.clientId); @@ -169,9 +167,7 @@ export class DeviceCodeClient extends BaseClient { const requestParameters: RequestParameterBuilder = new RequestParameterBuilder(); - const scopeSet = new ScopeSet(request.scopes || [], - this.config.authOptions.clientId, - false); + const scopeSet = new ScopeSet(request.scopes || []); requestParameters.addScopes(scopeSet); requestParameters.addClientId(this.config.authOptions.clientId); requestParameters.addGrantType(GrantType.DEVICE_CODE_GRANT); diff --git a/lib/msal-common/src/client/RefreshTokenClient.ts b/lib/msal-common/src/client/RefreshTokenClient.ts index 8eab52e498..9e35f96a79 100644 --- a/lib/msal-common/src/client/RefreshTokenClient.ts +++ b/lib/msal-common/src/client/RefreshTokenClient.ts @@ -34,7 +34,7 @@ export class RefreshTokenClient extends BaseClient { ); responseHandler.validateTokenResponse(response.body); - const tokenResponse = await responseHandler.generateAuthenticationResult( + const tokenResponse = responseHandler.generateAuthenticationResult( response.body, this.defaultAuthority ); @@ -54,12 +54,11 @@ export class RefreshTokenClient extends BaseClient { private createTokenRequestBody(request: RefreshTokenRequest): string { const parameterBuilder = new RequestParameterBuilder(); - const scopeSet = new ScopeSet(request.scopes || [], - this.config.authOptions.clientId, - false); + const scopeSet = new ScopeSet(request.scopes || []); parameterBuilder.addScopes(scopeSet); parameterBuilder.addClientId(this.config.authOptions.clientId); parameterBuilder.addGrantType(GrantType.REFRESH_TOKEN_GRANT); + parameterBuilder.addClientInfo(); parameterBuilder.addRefreshToken(request.refreshToken); return parameterBuilder.createQueryString(); diff --git a/lib/msal-common/src/client/SPAClient.ts b/lib/msal-common/src/client/SPAClient.ts index 06a5eb560f..1c890cf4f7 100644 --- a/lib/msal-common/src/client/SPAClient.ts +++ b/lib/msal-common/src/client/SPAClient.ts @@ -50,31 +50,14 @@ export class SPAClient extends BaseClient { ); } - /** - * Creates a url for logging in a user. This will by default append the client id to the list of scopes, - * allowing you to retrieve an id token in the subsequent code exchange. Also performs validation of the request parameters. - * Including any SSO parameters (account, sid, login_hint) will short circuit the authentication and allow you to retrieve a code without interaction. - * @param request - */ - async createLoginUrl(request: AuthorizationUrlRequest): Promise { - return this.createUrl(request, true); - } - /** * Creates a url for logging in a user. Also performs validation of the request parameters. * Including any SSO parameters (account, sid, login_hint) will short circuit the authentication and allow you to retrieve a code without interaction. * @param request */ - async createAcquireTokenUrl(request: AuthorizationUrlRequest): Promise { - return this.createUrl(request, false); - } - - /** - * Helper function which creates URL. If isLoginCall is true, MSAL appends client id scope to retrieve id token from the service. - * @param request - * @param isLoginCall - */ - private async createUrl(request: AuthorizationUrlRequest, isLoginCall: boolean): Promise { + async createUrl( + request: AuthorizationUrlRequest + ): Promise { // Initialize authority or use default, and perform discovery endpoint check. const acquireTokenAuthority = request && request.authority @@ -98,24 +81,19 @@ export class SPAClient extends BaseClient { } const queryString = await this.createUrlRequestParamString( - request, - isLoginCall + request ); return `${acquireTokenAuthority.authorizationEndpoint}?${queryString}`; } - private async createUrlRequestParamString(request: AuthorizationUrlRequest, isLoginCall: boolean): Promise { + private async createUrlRequestParamString(request: AuthorizationUrlRequest): Promise { const parameterBuilder = new RequestParameterBuilder(); parameterBuilder.addResponseTypeCode(); // Client ID parameterBuilder.addClientId(this.config.authOptions.clientId); - const scopeSet = new ScopeSet( - (request && request.scopes) || [], - this.config.authOptions.clientId, - !isLoginCall - ); + const scopeSet = new ScopeSet((request && request.scopes) || []); if (request.extraScopesToConsent) { scopeSet.appendScopes(request && request.extraScopesToConsent); @@ -198,7 +176,7 @@ export class SPAClient extends BaseClient { codeRequest.redirectUri || this.getRedirectUri() ); - const scopeSet = new ScopeSet(codeRequest.scopes || [], this.config.authOptions.clientId, true); + const scopeSet = new ScopeSet(codeRequest.scopes || []); parameterBuilder.addScopes(scopeSet); // add code: set by user, not validated @@ -215,7 +193,7 @@ export class SPAClient extends BaseClient { // User helper to retrieve token response. // Need to await function call before return to catch any thrown errors. // if errors are thrown asynchronously in return statement, they are caught by caller of this function instead. - return await this.getTokenResponse(tokenEndpoint, parameterBuilder, acquireTokenAuthority, userState, cachedNonce); + return await this.getTokenResponse(tokenEndpoint, parameterBuilder, acquireTokenAuthority, cachedNonce, userState); } /** @@ -235,7 +213,7 @@ export class SPAClient extends BaseClient { } // Get account object for this request. - const requestScopes = new ScopeSet(request.scopes || [], this.config.authOptions.clientId, true); + const requestScopes = new ScopeSet(request.scopes || []); // Get current cached tokens const cachedAccount = this.unifiedCacheManager.getAccount(CacheHelper.generateAccountCacheKey(request.account)); @@ -250,25 +228,7 @@ export class SPAClient extends BaseClient { } // Check if refresh is forced, or if tokens are expired. If neither are true, return a token response with the found token entry. - if (!request.forceRefresh && this.isTokenExpired(cachedAccessToken.expiresOn)) { - const cachedIdToken = this.fetchIdToken(homeAccountId, env, cachedAccount.realm); - const idTokenObj = new IdToken(cachedIdToken.secret, this.cryptoUtils); - - const cachedScopes = ScopeSet.fromString(cachedAccessToken.target, this.config.authOptions.clientId, true); - return { - uniqueId: idTokenObj.claims.oid || idTokenObj.claims.sub, - tenantId: idTokenObj.claims.tid, - scopes: cachedScopes.asArray(), - idToken: idTokenObj.rawIdToken, - idTokenClaims: idTokenObj.claims, - accessToken: cachedAccessToken.secret, - account: CacheHelper.toIAccount(cachedAccount), - expiresOn: new Date(cachedAccessToken.expiresOn), - extExpiresOn: new Date(cachedAccessToken.extendedExpiresOn), - familyId: null, - state: "" - }; - } else { + if (request.forceRefresh || this.isTokenExpired(cachedAccessToken.expiresOn)) { if (!cachedRefreshToken) { throw ClientAuthError.createNoTokensFoundError(); } @@ -297,6 +257,25 @@ export class SPAClient extends BaseClient { authority: acquireTokenAuthority.canonicalAuthority }; return this.renewToken(refreshTokenRequest, acquireTokenAuthority, tokenEndpoint); + } else { + const cachedIdToken = this.fetchIdToken(homeAccountId, env, cachedAccount.realm); + const idTokenObj = new IdToken(cachedIdToken.secret, this.cryptoUtils); + + const cachedScopes = ScopeSet.fromString(cachedAccessToken.target); + return { + uniqueId: idTokenObj.claims.oid || idTokenObj.claims.sub, + tenantId: idTokenObj.claims.tid, + scopes: cachedScopes.asArray(), + idToken: idTokenObj.rawIdToken, + idTokenClaims: idTokenObj.claims, + accessToken: cachedAccessToken.secret, + fromCache: true, + account: CacheHelper.toIAccount(cachedAccount), + expiresOn: new Date(cachedAccessToken.expiresOn), + extExpiresOn: new Date(cachedAccessToken.extendedExpiresOn), + familyId: null, + state: "" + }; } } @@ -442,7 +421,7 @@ export class SPAClient extends BaseClient { * @param tokenRequest * @param codeResponse */ - private async getTokenResponse(tokenEndpoint: string, parameterBuilder: RequestParameterBuilder, authority: Authority, userState: string, cachedNonce?: string): Promise { + private async getTokenResponse(tokenEndpoint: string, parameterBuilder: RequestParameterBuilder, authority: Authority, cachedNonce?: string, userState?: string): Promise { // Perform token request. const acquiredTokenResponse = await this.networkClient.sendPostRequestAsync< ServerAuthorizationTokenResponse @@ -456,10 +435,7 @@ export class SPAClient extends BaseClient { // Validate response. This function throws a server error if an error is returned by the server. responseHandler.validateTokenResponse(acquiredTokenResponse.body); // Return token response with given parameters - const tokenResponse = responseHandler.generateAuthenticationResult( - acquiredTokenResponse.body, - authority - ); + const tokenResponse = responseHandler.generateAuthenticationResult(acquiredTokenResponse.body, authority, cachedNonce); tokenResponse.state = userState; return tokenResponse; @@ -479,21 +455,19 @@ export class SPAClient extends BaseClient { parameterBuilder.addRedirectUri(this.getRedirectUri()); - const scopeSet = new ScopeSet( - refreshTokenRequest.scopes || [], - this.config.authOptions.clientId, - true - ); + const scopeSet = new ScopeSet(refreshTokenRequest.scopes || []); parameterBuilder.addScopes(scopeSet); parameterBuilder.addRefreshToken(refreshTokenRequest.refreshToken); parameterBuilder.addGrantType(GrantType.REFRESH_TOKEN_GRANT); + parameterBuilder.addClientInfo(); + // User helper to retrieve token response. // Need to await function call before return to catch any thrown errors. // if errors are thrown asynchronously in return statement, they are caught by caller of this function instead. - return await this.getTokenResponse(tokenEndpoint, parameterBuilder, authority, ""); + return await this.getTokenResponse(tokenEndpoint, parameterBuilder, authority); } // #endregion diff --git a/lib/msal-common/src/client/SilentFlowClient.ts b/lib/msal-common/src/client/SilentFlowClient.ts index d8891e2389..6b486c1b23 100644 --- a/lib/msal-common/src/client/SilentFlowClient.ts +++ b/lib/msal-common/src/client/SilentFlowClient.ts @@ -37,7 +37,7 @@ export class SilentFlowClient extends BaseClient { throw ClientAuthError.createNoAccountInSilentRequestError(); } - const requestScopes = new ScopeSet(request.scopes || [], this.config.authOptions.clientId, true); + const requestScopes = new ScopeSet(request.scopes || []); // fetch account const accountKey: string = CacheHelper.generateAccountCacheKey(request.account); const cachedAccount = this.unifiedCacheManager.getAccount(accountKey); @@ -77,6 +77,7 @@ export class SilentFlowClient extends BaseClient { idToken: cachedIdToken.secret, idTokenClaims: idTokenObj.claims, accessToken: cachedAccessToken.secret, + fromCache: true, expiresOn: new Date(cachedAccessToken.expiresOn), extExpiresOn: new Date(cachedAccessToken.extendedExpiresOn), familyId: null, diff --git a/lib/msal-common/src/index.ts b/lib/msal-common/src/index.ts index f3c7dc448d..c469c28cbd 100644 --- a/lib/msal-common/src/index.ts +++ b/lib/msal-common/src/index.ts @@ -38,12 +38,13 @@ export { UrlString } from "./url/UrlString"; // Crypto Interface export { ICrypto, PkceCodes } from "./crypto/ICrypto"; // Request and Response +export { BaseAuthRequest } from "./request/BaseAuthRequest"; export { AuthorizationUrlRequest } from "./request/AuthorizationUrlRequest"; export { AuthorizationCodeRequest } from "./request/AuthorizationCodeRequest"; export { RefreshTokenRequest } from "./request/RefreshTokenRequest"; export { SilentFlowRequest } from "./request/SilentFlowRequest"; -export { AuthenticationResult } from "./response/AuthenticationResult"; export { DeviceCodeRequest } from "./request/DeviceCodeRequest"; +export { AuthenticationResult } from "./response/AuthenticationResult"; // Logger Callback export { ILoggerCallback, LogLevel, Logger } from "./logger/Logger"; // Errors @@ -54,7 +55,7 @@ export { ClientAuthError, ClientAuthErrorMessage } from "./error/ClientAuthError export { ClientConfigurationError, ClientConfigurationErrorMessage } from "./error/ClientConfigurationError"; // Constants and Utils export { - Constants, PromptValue, PersistentCacheKeys, Prompt, ResponseMode, CacheSchemaType, CredentialType + Constants, PromptValue, PersistentCacheKeys, ResponseMode, CacheSchemaType, CredentialType } from "./utils/Constants"; export { StringUtils } from "./utils/StringUtils"; export { StringDict } from "./utils/MsalTypes"; diff --git a/lib/msal-common/src/request/AuthorizationCodeRequest.ts b/lib/msal-common/src/request/AuthorizationCodeRequest.ts index ee647234de..f8d87c5d87 100644 --- a/lib/msal-common/src/request/AuthorizationCodeRequest.ts +++ b/lib/msal-common/src/request/AuthorizationCodeRequest.ts @@ -3,6 +3,8 @@ * Licensed under the MIT License. */ +import { BaseAuthRequest } from "./BaseAuthRequest"; + /** * @type AuthorizationCodeRequest: Request object passed by user to acquire a token from the server exchanging a valid authorization code * (second leg of OAuth2.0 Authorization Code flow) @@ -27,11 +29,9 @@ * correlationId: Unique GUID set per request to trace a request end-to-end for telemetry purposes * */ -export type AuthorizationCodeRequest = { - scopes: Array; +export type AuthorizationCodeRequest = BaseAuthRequest & { redirectUri: string; code: string; - authority?: string; codeVerifier?: string; correlationId?: string; }; diff --git a/lib/msal-common/src/request/AuthorizationUrlRequest.ts b/lib/msal-common/src/request/AuthorizationUrlRequest.ts index eb56232ed6..290cde0a0c 100644 --- a/lib/msal-common/src/request/AuthorizationUrlRequest.ts +++ b/lib/msal-common/src/request/AuthorizationUrlRequest.ts @@ -5,35 +5,24 @@ import { ResponseMode } from "../utils/Constants"; import { StringDict } from "../utils/MsalTypes"; +import { BaseAuthRequest } from "./BaseAuthRequest"; /** * @type AuthorizationCodeUrlRequest: Request object passed by user to retrieve a Code from the * server (first leg of authorization code grant flow) */ -export type AuthorizationUrlRequest = { +export type AuthorizationUrlRequest = BaseAuthRequest & { /** * The redirect URI where authentication responses can be received by your application. It * must exactly match one of the redirect URIs registered in the Azure portal. */ redirectUri: string; - /** - * Scopes the application is requesting access to. - */ - scopes: Array; - /** * Scopes for a different resource when the user needs consent upfront */ extraScopesToConsent?: Array; - /** - * Url of the authority which the application acquires tokens from. Defaults to - * https://login.microsoftonline.com/common. If using the same authority for all request, authority should set - * on client application object and not request, to avoid resolving authority endpoints multiple times. - */ - authority?: string; - /** * Specifies the method that should be used to send the authentication result to your app. * Can be query, form_post, or fragment. If no value is passed in, it defaults to query. diff --git a/lib/msal-common/src/request/BaseAuthRequest.ts b/lib/msal-common/src/request/BaseAuthRequest.ts new file mode 100644 index 0000000000..ae6197a0f3 --- /dev/null +++ b/lib/msal-common/src/request/BaseAuthRequest.ts @@ -0,0 +1,18 @@ +/* + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +export type BaseAuthRequest = { + /** + * Scopes the application is requesting access to. + */ + scopes: Array; + + /** + * Url of the authority which the application acquires tokens from. Defaults to + * https://login.microsoftonline.com/common. If using the same authority for all request, authority should set + * on client application object and not request, to avoid resolving authority endpoints multiple times. + */ + authority?: string; +}; diff --git a/lib/msal-common/src/request/DeviceCodeRequest.ts b/lib/msal-common/src/request/DeviceCodeRequest.ts index aa50ae1b6f..8c39b244a2 100644 --- a/lib/msal-common/src/request/DeviceCodeRequest.ts +++ b/lib/msal-common/src/request/DeviceCodeRequest.ts @@ -4,11 +4,12 @@ */ import { DeviceCodeResponse } from "../response/DeviceCodeResponse"; +import { BaseAuthRequest } from "./BaseAuthRequest"; /** * Parameters for Oauth2 device code flow. */ -export type DeviceCodeRequest = { +export type DeviceCodeRequest = BaseAuthRequest & { /** * Callback containing device code response. Message should be shown to end user. End user can then navigate to the verification_uri, @@ -16,11 +17,6 @@ export type DeviceCodeRequest = { */ deviceCodeCallback: (response: DeviceCodeResponse) => void; - /** - * Scopes to which the application is requesting access to. - */ - scopes: Array; - /** * Boolean to cancel polling of device code endpoint. * @@ -28,11 +24,4 @@ export type DeviceCodeRequest = { * specified in the device code response (usually 15 minutes). To stop polling and cancel the request, set cancel=true; */ cancel?: boolean; - - /** - * Url of the authority which the application acquires tokens from. Defaults to - * https://login.microsoftonline.com/common. If using the same authority for all request, authority should set - * on client application object and not request, to avoid resolving authority endpoints multiple times. - */ - authority?: string; }; diff --git a/lib/msal-common/src/request/RefreshTokenRequest.ts b/lib/msal-common/src/request/RefreshTokenRequest.ts index 3d38d63b29..7fe746a935 100644 --- a/lib/msal-common/src/request/RefreshTokenRequest.ts +++ b/lib/msal-common/src/request/RefreshTokenRequest.ts @@ -3,6 +3,8 @@ * Licensed under the MIT License. */ +import { BaseAuthRequest } from "./BaseAuthRequest"; + /** * @type RefreshTokenRequest * @@ -10,8 +12,6 @@ * authority: URL of the authority, the security token service (STS) from which MSAL will acquire tokens. * refreshToken: A refresh token returned from a previous request to the Identity provider. */ -export class RefreshTokenRequest { - scopes: Array; +export type RefreshTokenRequest = BaseAuthRequest & { refreshToken: string; - authority?: string; -} +}; diff --git a/lib/msal-common/src/request/ScopeSet.ts b/lib/msal-common/src/request/ScopeSet.ts index 41d76a2123..af35c949b0 100644 --- a/lib/msal-common/src/request/ScopeSet.ts +++ b/lib/msal-common/src/request/ScopeSet.ts @@ -12,39 +12,19 @@ import { ClientAuthError } from "../error/ClientAuthError"; * The ScopeSet class creates a set of scopes. Scopes are case-insensitive, unique values, so the Set object in JS makes * the most sense to implement for this class. All scopes are trimmed and converted to lower case strings to ensure uniqueness of strings. */ -export class ScopeSet { - // Client ID of application - private clientId: string; +export class ScopeSet {; // Scopes as a Set of strings private scopes: Set; - // Original scopes passed to constructor. Usually used for caching or telemetry. - private originalScopes: Set; - // Boolean denoting whether scopes are required. Usually used for validation. - private scopesRequired: boolean; - - constructor( - inputScopes: Array, - clientId: string, - scopesRequired: boolean, - ) { - // lower case need for replaceDefaultScopes() because ADFS clientids don't have to be GUIDS. - this.clientId = clientId.toLowerCase(); - this.scopesRequired = scopesRequired; + constructor(inputScopes: Array) { // Filter empty string and null/undefined array items - const filteredInput = inputScopes && StringUtils.removeEmptyStringsFromArray(inputScopes); + const scopeArr = inputScopes ? StringUtils.trimAndConvertArrayEntriesToLowerCase([...inputScopes]) : []; + const filteredInput = scopeArr ? StringUtils.removeEmptyStringsFromArray(scopeArr) : []; // Validate and filter scopes (validate function throws if validation fails) this.validateInputScopes(filteredInput); - const scopeArr = filteredInput? StringUtils.trimAndConvertArrayEntriesToLowerCase([...filteredInput]): []; - this.scopes = new Set(scopeArr); - if (!this.scopesRequired) { - this.appendScope(this.clientId); - } - this.originalScopes = new Set(this.scopes); - - this.replaceDefaultScopes(); + this.scopes = new Set(filteredInput); } /** @@ -53,26 +33,10 @@ export class ScopeSet { * @param appClientId * @param scopesRequired */ - static fromString( - inputScopeString: string, - appClientId: string, - scopesRequired: boolean - ): ScopeSet { + static fromString(inputScopeString: string): ScopeSet { inputScopeString = inputScopeString || ""; const inputScopes: Array = inputScopeString.split(" "); - return new ScopeSet(inputScopes, appClientId, scopesRequired); - } - - /** - * Replace client id with the default scopes used for token acquisition. - */ - private replaceDefaultScopes(): void { - if (this.scopes.has(this.clientId)) { - this.removeScope(this.clientId); - this.appendScope(Constants.OPENID_SCOPE); - this.appendScope(Constants.PROFILE_SCOPE); - } - this.appendScope(Constants.OFFLINE_ACCESS_SCOPE); + return new ScopeSet(inputScopes); } /** @@ -81,20 +45,9 @@ export class ScopeSet { * @param {boolean} scopesRequired - Boolean indicating whether the scopes array is required or not */ private validateInputScopes(inputScopes: Array): void { - if (this.scopesRequired) { - // Check if scopes are required but not given or is an empty array - if (!inputScopes || inputScopes.length < 1) { - throw ClientConfigurationError.createEmptyScopesArrayError( - inputScopes - ); - } - } - - // Check that scopes is an array object - if (!Array.isArray(inputScopes)) { - throw ClientConfigurationError.createScopesNonArrayError( - inputScopes - ); + // Check if scopes are required but not given or is an empty array + if (!inputScopes || inputScopes.length < 1) { + throw ClientConfigurationError.createEmptyScopesArrayError(inputScopes); } } @@ -114,10 +67,8 @@ export class ScopeSet { if (!scopeSet) { return false; } - return ( - this.scopes.size >= scopeSet.scopes.size && - scopeSet.asArray().every(scope => this.containsScope(scope)) - ); + + return (this.scopes.size >= scopeSet.scopes.size && scopeSet.asArray().every(scope => this.containsScope(scope))); } /** @@ -126,9 +77,7 @@ export class ScopeSet { */ appendScope(newScope: string): void { if (StringUtils.isEmpty(newScope)) { - throw ClientAuthError.createAppendEmptyScopeToSetError( - newScope - ); + throw ClientAuthError.createAppendEmptyScopeToSetError(newScope); } this.scopes.add(newScope.trim().toLowerCase()); } @@ -139,12 +88,7 @@ export class ScopeSet { */ appendScopes(newScopes: Array): void { try { - const newScopeSet = new ScopeSet( - newScopes, - this.clientId, - this.scopesRequired - ); - this.scopes = this.unionScopeSets(newScopeSet); + newScopes.forEach(newScope => this.scopes.add(newScope)); } catch (e) { throw ClientAuthError.createAppendScopeSetError(e); } @@ -156,9 +100,7 @@ export class ScopeSet { */ removeScope(scope: string): void { if (StringUtils.isEmpty(scope)) { - throw ClientAuthError.createRemoveEmptyScopeFromSetError( - scope - ); + throw ClientAuthError.createRemoveEmptyScopeFromSetError(scope); } this.scopes.delete(scope.trim().toLowerCase()); } @@ -169,14 +111,9 @@ export class ScopeSet { */ unionScopeSets(otherScopes: ScopeSet): Set { if (!otherScopes) { - throw ClientAuthError.createEmptyInputScopeSetError( - otherScopes - ); + throw ClientAuthError.createEmptyInputScopeSetError(otherScopes); } - return new Set([ - ...otherScopes.asArray(), - ...Array.from(this.scopes) - ]); + return new Set([...otherScopes.asArray(), ...Array.from(this.scopes)]); } /** @@ -185,17 +122,15 @@ export class ScopeSet { */ intersectingScopeSets(otherScopes: ScopeSet): boolean { if (!otherScopes) { - throw ClientAuthError.createEmptyInputScopeSetError( - otherScopes - ); + throw ClientAuthError.createEmptyInputScopeSetError(otherScopes); } const unionScopes = this.unionScopeSets(otherScopes); // Do not allow offline_access to be the only intersecting scope - const sizeOtherScopes = otherScopes.containsScope(Constants.OFFLINE_ACCESS_SCOPE)? otherScopes.getScopeCount() - 1 : otherScopes.getScopeCount(); - const sizeThisScopes = this.containsScope(Constants.OFFLINE_ACCESS_SCOPE)? this.getScopeCount() - 1: this.getScopeCount(); - const sizeUnionScopes = unionScopes.has(Constants.OFFLINE_ACCESS_SCOPE)? unionScopes.size - 1: unionScopes.size; + const sizeOtherScopes = otherScopes.getScopeCount(); + const sizeThisScopes = this.getScopeCount(); + const sizeUnionScopes = unionScopes.size; return sizeUnionScopes < (sizeThisScopes + sizeOtherScopes); } @@ -206,17 +141,6 @@ export class ScopeSet { return this.scopes.size; } - /** - * Returns true if the set of original scopes only contained client-id - */ - isLoginScopeSet(): boolean { - const hasLoginScopes = - this.originalScopes.has(this.clientId) || - this.originalScopes.has(Constants.OPENID_SCOPE) || - this.originalScopes.has(Constants.PROFILE_SCOPE); - return this.originalScopes && hasLoginScopes; - } - /** * Returns the scopes as an array of string values */ @@ -224,13 +148,6 @@ export class ScopeSet { return Array.from(this.scopes); } - /** - * Returns the original scopes as an array (no extra scopes to consent) - */ - getOriginalScopesAsArray(): Array { - return Array.from(this.originalScopes); - } - /** * Prints scopes into a space-delimited string */ diff --git a/lib/msal-common/src/request/SilentFlowRequest.ts b/lib/msal-common/src/request/SilentFlowRequest.ts index b1a70d3b6d..04c43846f5 100644 --- a/lib/msal-common/src/request/SilentFlowRequest.ts +++ b/lib/msal-common/src/request/SilentFlowRequest.ts @@ -4,6 +4,7 @@ */ import { IAccount } from "../account/IAccount"; +import { BaseAuthRequest } from "./BaseAuthRequest"; /** * SilentFlow parameters passed by the user to retrieve credentials silently @@ -13,9 +14,7 @@ import { IAccount } from "../account/IAccount"; * - forceRefresh: Forces silent requests to make network calls if true * - correlationId: GUID set by the user to trace the request */ -export type SilentFlowRequest = { - scopes: Array; - authority?: string; +export type SilentFlowRequest = BaseAuthRequest & { account: IAccount; forceRefresh?: boolean; correlationId?: string; diff --git a/lib/msal-common/src/response/AuthenticationResult.ts b/lib/msal-common/src/response/AuthenticationResult.ts index 4fe4f86f90..17d22928f4 100644 --- a/lib/msal-common/src/response/AuthenticationResult.ts +++ b/lib/msal-common/src/response/AuthenticationResult.ts @@ -17,6 +17,7 @@ export class AuthenticationResult { idToken: string; idTokenClaims: StringDict; accessToken: string; + fromCache: boolean; expiresOn: Date; extExpiresOn?: Date; state?: string; diff --git a/lib/msal-common/src/response/ResponseHandler.ts b/lib/msal-common/src/response/ResponseHandler.ts index a0f4dd4946..eae8ae2f35 100644 --- a/lib/msal-common/src/response/ResponseHandler.ts +++ b/lib/msal-common/src/response/ResponseHandler.ts @@ -104,16 +104,22 @@ export class ResponseHandler { * @param serverTokenResponse * @param authority */ - generateAuthenticationResult(serverTokenResponse: ServerAuthorizationTokenResponse, authority: Authority): AuthenticationResult { + generateAuthenticationResult(serverTokenResponse: ServerAuthorizationTokenResponse, authority: Authority, cachedNonce?: string): AuthenticationResult { // create an idToken object (not entity) const idTokenObj = new IdToken(serverTokenResponse.id_token, this.cryptoObj); + // token nonce check (TODO: Add a warning if no nonce is given?) + if (!StringUtils.isEmpty(cachedNonce)) { + if (idTokenObj.claims.nonce !== cachedNonce) { + throw ClientAuthError.createNonceMismatchError(); + } + } + // save the response tokens const cacheRecord = this.generateCacheRecord(serverTokenResponse, idTokenObj, authority); - this.uCacheManager.saveCacheRecord(cacheRecord); - - const responseScopes = ScopeSet.fromString(serverTokenResponse.scope, this.clientId, true); + const responseScopes = ScopeSet.fromString(serverTokenResponse.scope); + this.uCacheManager.saveCacheRecord(cacheRecord, responseScopes); const authenticationResult: AuthenticationResult = { uniqueId: idTokenObj.claims.oid || idTokenObj.claims.sub, @@ -123,6 +129,7 @@ export class ResponseHandler { idToken: idTokenObj.rawIdToken, idTokenClaims: idTokenObj.claims, accessToken: serverTokenResponse.access_token, + fromCache: true, expiresOn: new Date(cacheRecord.accessToken.expiresOn), extExpiresOn: new Date(cacheRecord.accessToken.extendedExpiresOn), familyId: serverTokenResponse.foci || null, @@ -181,7 +188,7 @@ export class ResponseHandler { ); // AccessToken - const responseScopes = ScopeSet.fromString(serverTokenResponse.scope, this.clientId, true); + const responseScopes = ScopeSet.fromString(serverTokenResponse.scope); // Expiration calculation const expiresInSeconds = TimeUtils.nowSeconds() + serverTokenResponse.expires_in; const extendedExpiresInSeconds = expiresInSeconds + serverTokenResponse.ext_expires_in; diff --git a/lib/msal-common/src/utils/Constants.ts b/lib/msal-common/src/utils/Constants.ts index 25d0481034..f4f1df5823 100644 --- a/lib/msal-common/src/utils/Constants.ts +++ b/lib/msal-common/src/utils/Constants.ts @@ -152,17 +152,6 @@ export const PromptValue = { NONE: "none", }; -/** - * // TODO: Have only one Prompr constant - * Allowed values for prompt - */ -export enum Prompt { - LOGIN = "login", - NONE = "none", - CONSENT = "consent", - SELECT_ACCOUNT = "select_account" -} - /** * SSO Types - generated to populate hints */ diff --git a/lib/msal-common/test/cache/UnifiedCacheManager.spec.ts b/lib/msal-common/test/cache/UnifiedCacheManager.spec.ts index 8ca43cf6fa..14afb11440 100644 --- a/lib/msal-common/test/cache/UnifiedCacheManager.spec.ts +++ b/lib/msal-common/test/cache/UnifiedCacheManager.spec.ts @@ -10,6 +10,7 @@ import { CacheSchemaType, CacheHelper, CredentialType } from "../../src"; import { IdTokenEntity } from "../../src/cache/entities/IdTokenEntity"; import { RefreshTokenEntity } from "../../src/cache/entities/RefreshTokenEntity"; import { AppMetadataEntity } from "../../src/cache/entities/AppMetadataEntity"; +import { TEST_CONFIG } from "../utils/StringConstants"; const cacheJson = require("./serialize/cache.json"); @@ -222,7 +223,7 @@ describe("UnifiedCacheManager test cases", () => { }); it("initCache", () => { - let unifiedCacheManager = new UnifiedCacheManager(storageInterface, true); + let unifiedCacheManager = new UnifiedCacheManager(storageInterface, TEST_CONFIG.MSAL_CLIENT_ID, true); // create mock AccessToken const atOne = mockCache.createMockATOne(); @@ -248,7 +249,7 @@ describe("UnifiedCacheManager test cases", () => { clientInfo: "eyJ1aWQiOiJzb21lVWlkIiwgInV0aWQiOiJzb21lVXRpZCJ9", }); - let unifiedCacheManager = new UnifiedCacheManager(storageInterface, true); + let unifiedCacheManager = new UnifiedCacheManager(storageInterface, TEST_CONFIG.MSAL_CLIENT_ID, true); const accountKey = ac.generateAccountKey(); const cache = storageInterface.getCache() as InMemoryCache; @@ -271,7 +272,7 @@ describe("UnifiedCacheManager test cases", () => { extendedExpiresOn: "4600", }); - let unifiedCacheManager = new UnifiedCacheManager(storageInterface, true); + let unifiedCacheManager = new UnifiedCacheManager(storageInterface, TEST_CONFIG.MSAL_CLIENT_ID, true); const atKey = at.generateCredentialKey(); unifiedCacheManager.saveCredential(at); const cache = storageInterface.getCache() as InMemoryCache; @@ -279,19 +280,19 @@ describe("UnifiedCacheManager test cases", () => { }); it("getAccount", () => { - let unifiedCacheManager = new UnifiedCacheManager(storageInterface, true); + let unifiedCacheManager = new UnifiedCacheManager(storageInterface, TEST_CONFIG.MSAL_CLIENT_ID, true); expect(unifiedCacheManager.getAccount("someuid.someutid-login.microsoftonline.com-microsoft").homeAccountId).to.eql("someUid.someUtid"); }); it("getCredential", () => { - let unifiedCacheManager = new UnifiedCacheManager(storageInterface, true); + let unifiedCacheManager = new UnifiedCacheManager(storageInterface, TEST_CONFIG.MSAL_CLIENT_ID, true); expect(unifiedCacheManager.getCredential("someuid.someutid-login.microsoftonline.com-accesstoken-mock_client_id-microsoft-scope6 scope7").homeAccountId).to.eql("someUid.someUtid"); }); it("getAccountsFilteredBy", () => { - let unifiedCacheManager = new UnifiedCacheManager(storageInterface, true); + let unifiedCacheManager = new UnifiedCacheManager(storageInterface, TEST_CONFIG.MSAL_CLIENT_ID, true); const filterOne: AccountFilter = { homeAccountId: "uid.utid" }; let accounts = unifiedCacheManager.getAccountsFilteredBy(filterOne); @@ -303,7 +304,7 @@ describe("UnifiedCacheManager test cases", () => { }); it("getCredentials", () => { - let unifiedCacheManager = new UnifiedCacheManager(storageInterface, true); + let unifiedCacheManager = new UnifiedCacheManager(storageInterface, TEST_CONFIG.MSAL_CLIENT_ID, true); // filter by homeAccountId const filterOne: CredentialFilter = { homeAccountId: "uid.utid" }; @@ -325,7 +326,7 @@ describe("UnifiedCacheManager test cases", () => { }); it("removeAccount", () => { - let unifiedCacheManager = new UnifiedCacheManager(storageInterface, true); + let unifiedCacheManager = new UnifiedCacheManager(storageInterface, TEST_CONFIG.MSAL_CLIENT_ID, true); let ac = new AccountEntity(); Object.assign(ac, { @@ -345,7 +346,7 @@ describe("UnifiedCacheManager test cases", () => { }); it("removeCredential", () => { - let unifiedCacheManager = new UnifiedCacheManager(storageInterface, true); + let unifiedCacheManager = new UnifiedCacheManager(storageInterface, TEST_CONFIG.MSAL_CLIENT_ID, true); let at = new AccessTokenEntity(); Object.assign(at, { diff --git a/lib/msal-common/test/client/AuthorizationCodeClient.spec.ts b/lib/msal-common/test/client/AuthorizationCodeClient.spec.ts index a2a3f900b8..1ed7d820fd 100644 --- a/lib/msal-common/test/client/AuthorizationCodeClient.spec.ts +++ b/lib/msal-common/test/client/AuthorizationCodeClient.spec.ts @@ -17,7 +17,7 @@ import { } from "../utils/StringConstants"; import { ClientConfiguration } from "../../src/config/ClientConfiguration"; import { BaseClient } from "../../src/client/BaseClient"; -import { AADServerParamKeys, PromptValue, ResponseMode, SSOTypes, Prompt } from "../../src/utils/Constants"; +import { AADServerParamKeys, PromptValue, ResponseMode, SSOTypes } from "../../src/utils/Constants"; import { ClientTestUtils } from "./ClientTestUtils"; import { B2cAuthority } from "../../src/authority/B2cAuthority"; @@ -34,7 +34,6 @@ describe("AuthorizationCodeClient unit tests", () => { describe("Constructor", () => { it("creates a AuthorizationCodeClient", async () => { - sinon.stub(Authority.prototype, "discoverEndpoints").resolves(DEFAULT_OPENID_CONFIG_RESPONSE); const config: ClientConfiguration = await ClientTestUtils.createTestClientConfiguration(); const client = new AuthorizationCodeClient(config); @@ -47,36 +46,13 @@ describe("AuthorizationCodeClient unit tests", () => { describe("Authorization url creation", () => { it("Creates an authorization url with default parameters", async () => { - sinon.stub(Authority.prototype, "discoverEndpoints").resolves(DEFAULT_OPENID_CONFIG_RESPONSE); const config: ClientConfiguration = await ClientTestUtils.createTestClientConfiguration(); const client = new AuthorizationCodeClient(config); const authCodeUrlRequest: AuthorizationUrlRequest = { redirectUri: TEST_URIS.TEST_REDIRECT_URI_LOCALHOST, - scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE, - codeChallenge: TEST_CONFIG.TEST_CHALLENGE, - codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD - }; - const loginUrl = await client.getAuthCodeUrl(authCodeUrlRequest); - expect(loginUrl).to.contain(Constants.DEFAULT_AUTHORITY); - expect(loginUrl).to.contain(DEFAULT_OPENID_CONFIG_RESPONSE.body.authorization_endpoint.replace("{tenant}", "common")); - expect(loginUrl).to.contain(`${AADServerParamKeys.SCOPE}=${TEST_CONFIG.DEFAULT_GRAPH_SCOPE}%20${Constants.OPENID_SCOPE}%20${Constants.PROFILE_SCOPE}%20${Constants.OFFLINE_ACCESS_SCOPE}`); - expect(loginUrl).to.contain(`${AADServerParamKeys.RESPONSE_TYPE}=${Constants.CODE_RESPONSE_TYPE}`); - expect(loginUrl).to.contain(`${AADServerParamKeys.CLIENT_ID}=${TEST_CONFIG.MSAL_CLIENT_ID}`); - expect(loginUrl).to.contain(`${AADServerParamKeys.REDIRECT_URI}=${encodeURIComponent(TEST_URIS.TEST_REDIRECT_URI_LOCALHOST)}`); - expect(loginUrl).to.contain(`${AADServerParamKeys.RESPONSE_MODE}=${encodeURIComponent(ResponseMode.QUERY)}`); - }); - - it("Creates an authorization url passing in a default scope", async () => { - - sinon.stub(Authority.prototype, "discoverEndpoints").resolves(DEFAULT_OPENID_CONFIG_RESPONSE); - const config: ClientConfiguration = await ClientTestUtils.createTestClientConfiguration(); - const client = new AuthorizationCodeClient(config); - - const authCodeUrlRequest: AuthorizationUrlRequest = { - redirectUri: TEST_URIS.TEST_REDIRECT_URI_LOCALHOST, - scopes: [Constants.OPENID_SCOPE], + scopes: TEST_CONFIG.DEFAULT_SCOPES, codeChallenge: TEST_CONFIG.TEST_CHALLENGE, codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD }; @@ -91,7 +67,6 @@ describe("AuthorizationCodeClient unit tests", () => { }); it("Creates an authorization url passing in optional parameters", async () => { - // Override with alternate authority openid_config sinon.stub(Authority.prototype, "discoverEndpoints").resolves(ALTERNATE_OPENID_CONFIG_RESPONSE); @@ -100,13 +75,13 @@ describe("AuthorizationCodeClient unit tests", () => { const authCodeUrlRequest: AuthorizationUrlRequest = { redirectUri: TEST_URIS.TEST_REDIRECT_URI_LOCALHOST, - scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE, + scopes: [...TEST_CONFIG.DEFAULT_GRAPH_SCOPE, ...TEST_CONFIG.DEFAULT_SCOPES], authority: TEST_CONFIG.alternateValidAuthority, responseMode: ResponseMode.FORM_POST, codeChallenge: TEST_CONFIG.TEST_CHALLENGE, codeChallengeMethod: TEST_CONFIG.CODE_CHALLENGE_METHOD, state: TEST_CONFIG.STATE, - prompt: Prompt.SELECT_ACCOUNT, + prompt: PromptValue.SELECT_ACCOUNT, loginHint: TEST_CONFIG.LOGIN_HINT, domainHint: TEST_CONFIG.DOMAIN_HINT, claims: TEST_CONFIG.CLAIMS, @@ -141,7 +116,7 @@ describe("AuthorizationCodeClient unit tests", () => { const config: ClientConfiguration = await ClientTestUtils.createTestClientConfiguration(); const client = new AuthorizationCodeClient(config); const authCodeRequest: AuthorizationCodeRequest = { - scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE, + scopes: [...TEST_CONFIG.DEFAULT_GRAPH_SCOPE, ...TEST_CONFIG.DEFAULT_SCOPES], redirectUri: TEST_URIS.TEST_REDIRECT_URI_LOCALHOST, code: TEST_TOKENS.AUTHORIZATION_CODE, codeVerifier: TEST_CONFIG.TEST_VERIFIER diff --git a/lib/msal-common/test/client/DeviceCodeClient.spec.ts b/lib/msal-common/test/client/DeviceCodeClient.spec.ts index b39901e74f..0551bd5aac 100644 --- a/lib/msal-common/test/client/DeviceCodeClient.spec.ts +++ b/lib/msal-common/test/client/DeviceCodeClient.spec.ts @@ -53,7 +53,7 @@ describe("DeviceCodeClient unit tests", async () => { let deviceCodeResponse = null; const request: DeviceCodeRequest = { - scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE, + scopes: [...TEST_CONFIG.DEFAULT_GRAPH_SCOPE, ...TEST_CONFIG.DEFAULT_SCOPES], deviceCodeCallback: (response) => deviceCodeResponse = response }; @@ -87,8 +87,7 @@ describe("DeviceCodeClient unit tests", async () => { let deviceCodeResponse = null; const request: DeviceCodeRequest = { - - scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE, + scopes: [...TEST_CONFIG.DEFAULT_GRAPH_SCOPE, ...TEST_CONFIG.DEFAULT_SCOPES], deviceCodeCallback: (response) => deviceCodeResponse = response }; @@ -110,7 +109,7 @@ describe("DeviceCodeClient unit tests", async () => { let deviceCodeResponse = null; const request: DeviceCodeRequest = { - scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE, + scopes: [...TEST_CONFIG.DEFAULT_GRAPH_SCOPE, ...TEST_CONFIG.DEFAULT_SCOPES], deviceCodeCallback: (response) => deviceCodeResponse = response, }; @@ -126,7 +125,7 @@ describe("DeviceCodeClient unit tests", async () => { let deviceCodeResponse = null; const request: DeviceCodeRequest = { - scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE, + scopes: [...TEST_CONFIG.DEFAULT_GRAPH_SCOPE, ...TEST_CONFIG.DEFAULT_SCOPES], deviceCodeCallback: (response) => deviceCodeResponse = response, }; diff --git a/lib/msal-common/test/client/RefreshTokenClient.spec.ts b/lib/msal-common/test/client/RefreshTokenClient.spec.ts index b53ee33f4e..a9571111b7 100644 --- a/lib/msal-common/test/client/RefreshTokenClient.spec.ts +++ b/lib/msal-common/test/client/RefreshTokenClient.spec.ts @@ -68,7 +68,7 @@ describe("RefreshTokenClient unit tests", () => { environment: "login.windows.net", username: idTokenClaims.preferred_username }; - const expectedScopes = [Constants.OPENID_SCOPE, Constants.PROFILE_SCOPE, TEST_CONFIG.DEFAULT_GRAPH_SCOPE[0], "email", Constants.OFFLINE_ACCESS_SCOPE]; + const expectedScopes = [Constants.OPENID_SCOPE, Constants.PROFILE_SCOPE, TEST_CONFIG.DEFAULT_GRAPH_SCOPE[0], "email"]; expect(authResult.uniqueId).to.deep.eq(idTokenClaims.oid); expect(authResult.tenantId).to.deep.eq(idTokenClaims.tid); expect(authResult.scopes).to.deep.eq(expectedScopes); @@ -80,7 +80,7 @@ describe("RefreshTokenClient unit tests", () => { expect(createTokenRequestBodySpy.calledWith(refreshTokenRequest)).to.be.true; - expect(createTokenRequestBodySpy.returnValues[0]).to.contain(`${TEST_CONFIG.DEFAULT_GRAPH_SCOPE}%20${Constants.OPENID_SCOPE}%20${Constants.PROFILE_SCOPE}%20${Constants.OFFLINE_ACCESS_SCOPE}`); + expect(createTokenRequestBodySpy.returnValues[0]).to.contain(`${TEST_CONFIG.DEFAULT_GRAPH_SCOPE[0]}`); expect(createTokenRequestBodySpy.returnValues[0]).to.contain(`${AADServerParamKeys.CLIENT_ID}=${TEST_CONFIG.MSAL_CLIENT_ID}`); expect(createTokenRequestBodySpy.returnValues[0]).to.contain(`${AADServerParamKeys.REFRESH_TOKEN}=${TEST_TOKENS.REFRESH_TOKEN}`); expect(createTokenRequestBodySpy.returnValues[0]).to.contain(`${AADServerParamKeys.GRANT_TYPE}=${GrantType.REFRESH_TOKEN_GRANT}`); diff --git a/lib/msal-common/test/client/SPAClient.spec.ts b/lib/msal-common/test/client/SPAClient.spec.ts index ef0239ce3e..523dfc68cf 100644 --- a/lib/msal-common/test/client/SPAClient.spec.ts +++ b/lib/msal-common/test/client/SPAClient.spec.ts @@ -151,14 +151,14 @@ describe("SPAClient.ts Class Unit Tests", () => { store = {}; }); - it("Creates a login URL with default scopes", async () => { + it("Creates a login URL", async () => { const emptyRequest: AuthorizationUrlRequest = { redirectUri: TEST_URIS.TEST_REDIR_URI, - scopes: [], + scopes: TEST_CONFIG.DEFAULT_SCOPES, codeChallenge: TEST_CONFIG.TEST_CHALLENGE, codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD }; - const loginUrl = await Client.createLoginUrl(emptyRequest); + const loginUrl = await Client.createUrl(emptyRequest); expect(loginUrl).to.contain(Constants.DEFAULT_AUTHORITY); expect(loginUrl).to.contain(DEFAULT_OPENID_CONFIG_RESPONSE.body.authorization_endpoint.replace("{tenant}", "common")); expect(loginUrl).to.contain(`${AADServerParamKeys.SCOPE}=${Constants.OPENID_SCOPE}%20${Constants.PROFILE_SCOPE}%20${Constants.OFFLINE_ACCESS_SCOPE}`); @@ -178,8 +178,8 @@ describe("SPAClient.ts Class Unit Tests", () => { codeChallenge: TEST_CONFIG.TEST_CHALLENGE, codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD }; - const loginUrl = await Client.createLoginUrl(loginRequest); - expect(loginUrl).to.contain(`${AADServerParamKeys.SCOPE}=${encodeURIComponent(`${testScope1} ${testScope2} ${Constants.OPENID_SCOPE} ${Constants.PROFILE_SCOPE} ${Constants.OFFLINE_ACCESS_SCOPE}`)}`); + const loginUrl = await Client.createUrl(loginRequest); + expect(loginUrl).to.contain(`${AADServerParamKeys.SCOPE}=${encodeURIComponent(`${testScope1} ${testScope2}`)}`); }); it("Uses authority if given in request", async () => { @@ -187,12 +187,12 @@ describe("SPAClient.ts Class Unit Tests", () => { sinon.stub(Authority.prototype, "discoverEndpoints").resolves(ALTERNATE_OPENID_CONFIG_RESPONSE); const loginRequest: AuthorizationUrlRequest = { redirectUri: TEST_URIS.TEST_REDIR_URI, - scopes: [], + scopes: TEST_CONFIG.DEFAULT_SCOPES, authority: `${TEST_URIS.ALTERNATE_INSTANCE}/common`, codeChallenge: TEST_CONFIG.TEST_CHALLENGE, codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD }; - const loginUrl = await Client.createLoginUrl(loginRequest); + const loginUrl = await Client.createUrl(loginRequest); expect(loginUrl).to.contain(TEST_URIS.ALTERNATE_INSTANCE); expect(loginUrl).to.contain(ALTERNATE_OPENID_CONFIG_RESPONSE.body.authorization_endpoint); expect(loginUrl).to.contain(`${AADServerParamKeys.SCOPE}=${encodeURIComponent(`${Constants.OPENID_SCOPE} ${Constants.PROFILE_SCOPE} ${Constants.OFFLINE_ACCESS_SCOPE}`)}`); @@ -211,122 +211,27 @@ describe("SPAClient.ts Class Unit Tests", () => { codeChallenge: TEST_CONFIG.TEST_CHALLENGE, codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD }; - await expect(Client.createLoginUrl(emptyRequest)).to.be.rejectedWith(`${ClientAuthErrorMessage.endpointResolutionError.desc} Detail: ${exceptionString}`); - }); - }); - - describe("Acquire Token Url Creation", () => { - let Client: SPAClient; - beforeEach(() => { - sinon.stub(Authority.prototype, "discoverEndpoints").resolves(DEFAULT_OPENID_CONFIG_RESPONSE); - Client = new SPAClient(defaultAuthConfig); - }); - - afterEach(() => { - sinon.restore(); - store = {}; - }); - - it("Creates a acquire token URL with scopes from given token request", async () => { - const testScope1 = "testscope1"; - const testScope2 = "testscope2"; - const tokenRequest: AuthorizationUrlRequest = { - redirectUri: TEST_URIS.TEST_REDIR_URI, - scopes: [testScope1, testScope2], - codeChallenge: TEST_CONFIG.TEST_CHALLENGE, - codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD - }; - const acquireTokenUrl = await Client.createAcquireTokenUrl(tokenRequest); - expect(acquireTokenUrl).to.contain(Constants.DEFAULT_AUTHORITY); - expect(acquireTokenUrl).to.contain(DEFAULT_OPENID_CONFIG_RESPONSE.body.authorization_endpoint.replace("{tenant}", "common")); - expect(acquireTokenUrl).to.contain(`${AADServerParamKeys.SCOPE}=${encodeURIComponent(`${testScope1} ${testScope2} ${Constants.OFFLINE_ACCESS_SCOPE}`)}`); - expect(acquireTokenUrl).to.contain(`${AADServerParamKeys.RESPONSE_TYPE}=${Constants.CODE_RESPONSE_TYPE}`); - expect(acquireTokenUrl).to.contain(`${AADServerParamKeys.CLIENT_ID}=${TEST_CONFIG.MSAL_CLIENT_ID}`); - expect(acquireTokenUrl).to.contain(`${AADServerParamKeys.REDIRECT_URI}=${encodeURIComponent(TEST_URIS.TEST_REDIR_URI)}`); - }); - - it("Creates a acquire token URL with replaced client id scope", async () => { - const tokenRequest: AuthorizationUrlRequest = { - redirectUri: TEST_URIS.TEST_REDIR_URI, - scopes: [TEST_CONFIG.MSAL_CLIENT_ID], - codeChallenge: TEST_CONFIG.TEST_CHALLENGE, - codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD - }; - const acquireTokenUrl = await Client.createAcquireTokenUrl(tokenRequest); - expect(acquireTokenUrl).to.contain(Constants.DEFAULT_AUTHORITY); - expect(acquireTokenUrl).to.contain(DEFAULT_OPENID_CONFIG_RESPONSE.body.authorization_endpoint.replace("{tenant}", "common")); - expect(acquireTokenUrl).to.contain(`${AADServerParamKeys.SCOPE}=${encodeURIComponent(`${Constants.OPENID_SCOPE} ${Constants.PROFILE_SCOPE} ${Constants.OFFLINE_ACCESS_SCOPE}`)}`); - expect(acquireTokenUrl).to.contain(`${AADServerParamKeys.RESPONSE_TYPE}=${Constants.CODE_RESPONSE_TYPE}`); - expect(acquireTokenUrl).to.contain(`${AADServerParamKeys.CLIENT_ID}=${TEST_CONFIG.MSAL_CLIENT_ID}`); - expect(acquireTokenUrl).to.contain(`${AADServerParamKeys.REDIRECT_URI}=${encodeURIComponent(TEST_URIS.TEST_REDIR_URI)}`); + await expect(Client.createUrl(emptyRequest)).to.be.rejectedWith(`${ClientAuthErrorMessage.endpointResolutionError.desc} Detail: ${exceptionString}`); }); - it("Throws error if no scopes are passed to createAcquireTokenUrl", async () => { + it("Throws error if no scopes are passed to createUrl", async () => { const emptyRequest: AuthorizationUrlRequest = { redirectUri: TEST_URIS.TEST_REDIR_URI, scopes: null, codeChallenge: TEST_CONFIG.TEST_CHALLENGE, codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD }; - await expect(Client.createAcquireTokenUrl(emptyRequest)).to.be.rejectedWith(ClientConfigurationErrorMessage.emptyScopesError.desc); + await expect(Client.createUrl(emptyRequest)).to.be.rejectedWith(ClientConfigurationErrorMessage.emptyScopesError.desc); }); - it("Throws error if empty scopes are passed to createAcquireTokenUrl", async () => { + it("Throws error if empty scopes are passed to createUrl", async () => { const emptyRequest: AuthorizationUrlRequest = { redirectUri: TEST_URIS.TEST_REDIR_URI, scopes: [], codeChallenge: TEST_CONFIG.TEST_CHALLENGE, codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD }; - await expect(Client.createAcquireTokenUrl(emptyRequest)).to.be.rejectedWith(ClientConfigurationErrorMessage.emptyScopesError.desc); - }); - - it("Uses authority if given in request", async () => { - sinon.restore(); - sinon.stub(Authority.prototype, "discoverEndpoints").resolves(ALTERNATE_OPENID_CONFIG_RESPONSE); - const tokenRequest: AuthorizationUrlRequest = { - authority: `${TEST_URIS.ALTERNATE_INSTANCE}/common`, - redirectUri: TEST_URIS.TEST_REDIR_URI, - scopes: [TEST_CONFIG.MSAL_CLIENT_ID], - codeChallenge: TEST_CONFIG.TEST_CHALLENGE, - codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD - }; - const acquireTokenUrl = await Client.createAcquireTokenUrl(tokenRequest); - expect(acquireTokenUrl).to.contain(TEST_URIS.ALTERNATE_INSTANCE); - expect(acquireTokenUrl).to.contain(ALTERNATE_OPENID_CONFIG_RESPONSE.body.authorization_endpoint); - expect(acquireTokenUrl).to.contain(`${AADServerParamKeys.SCOPE}=${encodeURIComponent(`${Constants.OPENID_SCOPE} ${Constants.PROFILE_SCOPE} ${Constants.OFFLINE_ACCESS_SCOPE}`)}`); - expect(acquireTokenUrl).to.contain(`${AADServerParamKeys.RESPONSE_TYPE}=${Constants.CODE_RESPONSE_TYPE}`); - expect(acquireTokenUrl).to.contain(`${AADServerParamKeys.CLIENT_ID}=${TEST_CONFIG.MSAL_CLIENT_ID}`); - expect(acquireTokenUrl).to.contain(`${AADServerParamKeys.REDIRECT_URI}=${encodeURIComponent(TEST_URIS.TEST_REDIR_URI)}`); - }); - - it("Throws endpoint discovery error if resolveEndpointsAsync fails", async () => { - sinon.restore(); - const exceptionString = "Could not make a network request."; - sinon.stub(Authority.prototype, "resolveEndpointsAsync").throwsException(exceptionString); - const tokenRequest: AuthorizationUrlRequest = { - redirectUri: TEST_URIS.TEST_REDIR_URI, - scopes: [TEST_CONFIG.MSAL_CLIENT_ID], - codeChallenge: TEST_CONFIG.TEST_CHALLENGE, - codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD - }; - await expect(Client.createAcquireTokenUrl(tokenRequest)).to.be.rejectedWith(`${ClientAuthErrorMessage.endpointResolutionError.desc} Detail: ${exceptionString}`); - }); - - it("Cleans cache before error is thrown", async () => { - const guidCreationErr = "GUID can't be created."; - const tokenRequest: AuthorizationUrlRequest = { - redirectUri: TEST_URIS.TEST_REDIR_URI, - scopes: [TEST_CONFIG.MSAL_CLIENT_ID], - codeChallenge: TEST_CONFIG.TEST_CHALLENGE, - codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD - }; - defaultAuthConfig.cryptoInterface.createNewGuid = (): string => { - throw AuthError.createUnexpectedError(guidCreationErr); - }; - Client = new SPAClient(defaultAuthConfig); - await expect(Client.createAcquireTokenUrl(tokenRequest)).to.be.rejectedWith(guidCreationErr); - expect(defaultAuthConfig.storageInterface.getKeys()).to.be.empty; + await expect(Client.createUrl(emptyRequest)).to.be.rejectedWith(ClientConfigurationErrorMessage.emptyScopesError.desc); }); }); diff --git a/lib/msal-common/test/client/SilentFlowClient.spec.ts b/lib/msal-common/test/client/SilentFlowClient.spec.ts index 579571e93b..cb44dc8048 100644 --- a/lib/msal-common/test/client/SilentFlowClient.spec.ts +++ b/lib/msal-common/test/client/SilentFlowClient.spec.ts @@ -111,7 +111,7 @@ describe("SilentFlowClient unit tests", () => { }; const authResult: AuthenticationResult = await client.acquireToken(silentFlowRequest); - const expectedScopes = [Constants.OPENID_SCOPE, Constants.PROFILE_SCOPE, TEST_CONFIG.DEFAULT_GRAPH_SCOPE[0], "email", Constants.OFFLINE_ACCESS_SCOPE]; + const expectedScopes = [Constants.OPENID_SCOPE, Constants.PROFILE_SCOPE, TEST_CONFIG.DEFAULT_GRAPH_SCOPE[0], "email"]; expect(authResult.uniqueId).to.deep.eq(idTokenClaims.oid); expect(authResult.tenantId).to.deep.eq(idTokenClaims.tid); expect(authResult.scopes).to.deep.eq(expectedScopes); @@ -121,7 +121,7 @@ describe("SilentFlowClient unit tests", () => { expect(authResult.accessToken).to.deep.eq(AUTHENTICATION_RESULT.body.access_token); expect(authResult.state).to.be.undefined; - expect(createTokenRequestBodySpy.returnValues[0]).to.contain(`${TEST_CONFIG.DEFAULT_GRAPH_SCOPE}%20${Constants.OPENID_SCOPE}%20${Constants.PROFILE_SCOPE}%20${Constants.OFFLINE_ACCESS_SCOPE}`); + expect(createTokenRequestBodySpy.returnValues[0]).to.contain(`${TEST_CONFIG.DEFAULT_GRAPH_SCOPE[0]}`); expect(createTokenRequestBodySpy.returnValues[0]).to.contain(`${AADServerParamKeys.CLIENT_ID}=${TEST_CONFIG.MSAL_CLIENT_ID}`); expect(createTokenRequestBodySpy.returnValues[0]).to.contain(`${AADServerParamKeys.REFRESH_TOKEN}=${TEST_TOKENS.REFRESH_TOKEN}`); expect(createTokenRequestBodySpy.returnValues[0]).to.contain(`${AADServerParamKeys.GRANT_TYPE}=${GrantType.REFRESH_TOKEN_GRANT}`); diff --git a/lib/msal-common/test/request/ScopeSet.spec.ts b/lib/msal-common/test/request/ScopeSet.spec.ts index c73bd5f848..7727241d60 100644 --- a/lib/msal-common/test/request/ScopeSet.spec.ts +++ b/lib/msal-common/test/request/ScopeSet.spec.ts @@ -9,40 +9,11 @@ describe("ScopeSet.ts", () => { describe("Constructor and scope validation", () => { it("Throws error if scopes are null or empty and required", () => { - expect(() => new ScopeSet(null, TEST_CONFIG.MSAL_CLIENT_ID, true)).to.throw(ClientConfigurationErrorMessage.emptyScopesError.desc); - expect(() => new ScopeSet(null, TEST_CONFIG.MSAL_CLIENT_ID, true)).to.throw(ClientConfigurationError); + expect(() => new ScopeSet(null)).to.throw(ClientConfigurationErrorMessage.emptyScopesError.desc); + expect(() => new ScopeSet(null)).to.throw(ClientConfigurationError); - expect(() => new ScopeSet([], TEST_CONFIG.MSAL_CLIENT_ID, true)).to.throw(ClientConfigurationErrorMessage.emptyScopesError.desc); - expect(() => new ScopeSet([], TEST_CONFIG.MSAL_CLIENT_ID, true)).to.throw(ClientConfigurationError); - }); - - it("Throws error if scopes are not array object", () => { - expect(() => new ScopeSet(undefined, TEST_CONFIG.MSAL_CLIENT_ID, false)).to.throw(ClientConfigurationErrorMessage.nonArrayScopesError.desc); - expect(() => new ScopeSet(undefined, TEST_CONFIG.MSAL_CLIENT_ID, false)).to.throw(ClientConfigurationError); - }); - - it("Does not throw error if scopes are empty but not required", () => { - expect(() => new ScopeSet([], TEST_CONFIG.MSAL_CLIENT_ID, false)).to.not.throw(ClientConfigurationErrorMessage.emptyScopesError.desc); - expect(() => new ScopeSet([], TEST_CONFIG.MSAL_CLIENT_ID, false)).to.not.throw(ClientConfigurationError); - }); - - it("Appends client id to scopes and replaces it by default if scopes are not required", () => { - const scopeSet = new ScopeSet([], TEST_CONFIG.MSAL_CLIENT_ID, false); - expect(scopeSet.getOriginalScopesAsArray()).to.deep.eq([TEST_CONFIG.MSAL_CLIENT_ID]); - expect(scopeSet.asArray()).to.deep.eq(TEST_CONFIG.DEFAULT_SCOPES); - }); - - it("Replaces client id if scopes are required and client id is provided", () => { - const scopeSet = new ScopeSet([TEST_CONFIG.MSAL_CLIENT_ID], TEST_CONFIG.MSAL_CLIENT_ID, true); - expect(scopeSet.getOriginalScopesAsArray()).to.deep.eq([TEST_CONFIG.MSAL_CLIENT_ID]); - expect(scopeSet.asArray()).to.deep.eq(TEST_CONFIG.DEFAULT_SCOPES); - }); - - it("Does not append client id when scopes are required", () => { - const testScope = "testscope"; - const scopeSet = new ScopeSet([testScope], TEST_CONFIG.MSAL_CLIENT_ID, true); - expect(scopeSet.getOriginalScopesAsArray()).to.deep.eq([testScope]); - expect(scopeSet.asArray()).to.deep.eq([testScope, Constants.OFFLINE_ACCESS_SCOPE]); + expect(() => new ScopeSet([])).to.throw(ClientConfigurationErrorMessage.emptyScopesError.desc); + expect(() => new ScopeSet([])).to.throw(ClientConfigurationError); }); it("Converts array string values to lower case", () => { @@ -52,9 +23,8 @@ describe("ScopeSet.ts", () => { const lowerCaseScope2 = "testscope2"; const testScope3 = "TESTSCOPE3"; const lowerCaseScope3 = "testscope3"; - const scopeSet = new ScopeSet([testScope1, testScope2, testScope3], TEST_CONFIG.MSAL_CLIENT_ID, true); - expect(scopeSet.getOriginalScopesAsArray()).to.deep.eq([lowerCaseScope1, lowerCaseScope2, lowerCaseScope3]); - expect(scopeSet.asArray()).to.deep.eq([lowerCaseScope1, lowerCaseScope2, lowerCaseScope3, Constants.OFFLINE_ACCESS_SCOPE]); + const scopeSet = new ScopeSet([testScope1, testScope2, testScope3]); + expect(scopeSet.asArray()).to.deep.eq([lowerCaseScope1, lowerCaseScope2, lowerCaseScope3]); }); it("Removes duplicates from input scope array", () => { @@ -64,56 +34,22 @@ describe("ScopeSet.ts", () => { const testScope4 = "testscope"; const testScope5 = "testscope"; const lowerCaseScope = "testscope"; - const scopeSet = new ScopeSet([testScope1, testScope2, testScope3, testScope4, testScope5], TEST_CONFIG.MSAL_CLIENT_ID, true); - expect(scopeSet.getOriginalScopesAsArray()).to.deep.eq([lowerCaseScope]); - expect(scopeSet.asArray()).to.deep.eq([lowerCaseScope, Constants.OFFLINE_ACCESS_SCOPE]); + const scopeSet = new ScopeSet([testScope1, testScope2, testScope3, testScope4, testScope5]); + expect(scopeSet.asArray()).to.deep.eq([lowerCaseScope]); }); }); describe("fromString Constructor", () => { it("Throws error if scopeString is empty, null or undefined if scopes are required", () => { - expect(() => ScopeSet.fromString("", TEST_CONFIG.MSAL_CLIENT_ID, true)).to.throw(ClientConfigurationErrorMessage.emptyScopesError.desc); - expect(() => ScopeSet.fromString("", TEST_CONFIG.MSAL_CLIENT_ID, true)).to.throw(ClientConfigurationError); + expect(() => ScopeSet.fromString("")).to.throw(ClientConfigurationErrorMessage.emptyScopesError.desc); + expect(() => ScopeSet.fromString("")).to.throw(ClientConfigurationError); - expect(() => ScopeSet.fromString(null, TEST_CONFIG.MSAL_CLIENT_ID, true)).to.throw(ClientConfigurationErrorMessage.emptyScopesError.desc); - expect(() => ScopeSet.fromString(null, TEST_CONFIG.MSAL_CLIENT_ID, true)).to.throw(ClientConfigurationError); + expect(() => ScopeSet.fromString(null)).to.throw(ClientConfigurationErrorMessage.emptyScopesError.desc); + expect(() => ScopeSet.fromString(null)).to.throw(ClientConfigurationError); - expect(() => ScopeSet.fromString(undefined, TEST_CONFIG.MSAL_CLIENT_ID, true)).to.throw(ClientConfigurationErrorMessage.emptyScopesError.desc); - expect(() => ScopeSet.fromString(undefined, TEST_CONFIG.MSAL_CLIENT_ID, true)).to.throw(ClientConfigurationError); - }); - - it("Creates a default ScopeSet to send to the service if scopes are not required and empty, null or undefined", () => { - expect(() => ScopeSet.fromString("", TEST_CONFIG.MSAL_CLIENT_ID, false)).to.not.throw(ClientConfigurationErrorMessage.emptyScopesError.desc); - expect(() => ScopeSet.fromString("", TEST_CONFIG.MSAL_CLIENT_ID, false)).to.not.throw(ClientConfigurationError); - const scopeSet1 = ScopeSet.fromString("", TEST_CONFIG.MSAL_CLIENT_ID, false); - expect(scopeSet1.getOriginalScopesAsArray()).to.deep.eq([TEST_CONFIG.MSAL_CLIENT_ID]); - expect(scopeSet1.asArray()).to.deep.eq(TEST_CONFIG.DEFAULT_SCOPES); - - expect(() => ScopeSet.fromString(null, TEST_CONFIG.MSAL_CLIENT_ID, false)).to.not.throw(ClientConfigurationErrorMessage.emptyScopesError.desc); - expect(() => ScopeSet.fromString(null, TEST_CONFIG.MSAL_CLIENT_ID, false)).to.not.throw(ClientConfigurationError); - const scopeSet2 = ScopeSet.fromString(null, TEST_CONFIG.MSAL_CLIENT_ID, false); - expect(scopeSet2.getOriginalScopesAsArray()).to.deep.eq([TEST_CONFIG.MSAL_CLIENT_ID]); - expect(scopeSet2.asArray()).to.deep.eq(TEST_CONFIG.DEFAULT_SCOPES); - - expect(() => ScopeSet.fromString(undefined, TEST_CONFIG.MSAL_CLIENT_ID, false)).to.not.throw(ClientConfigurationErrorMessage.emptyScopesError.desc); - expect(() => ScopeSet.fromString(undefined, TEST_CONFIG.MSAL_CLIENT_ID, false)).to.not.throw(ClientConfigurationError); - const scopeSet3 = ScopeSet.fromString(undefined, TEST_CONFIG.MSAL_CLIENT_ID, false); - expect(scopeSet3.getOriginalScopesAsArray()).to.deep.eq([TEST_CONFIG.MSAL_CLIENT_ID]); - expect(scopeSet3.asArray()).to.deep.eq(TEST_CONFIG.DEFAULT_SCOPES); - }); - - it("Replaces client id if scopes are required and client id is provided", () => { - const scopeSet = ScopeSet.fromString(TEST_CONFIG.MSAL_CLIENT_ID, TEST_CONFIG.MSAL_CLIENT_ID, true); - expect(scopeSet.getOriginalScopesAsArray()).to.deep.eq([TEST_CONFIG.MSAL_CLIENT_ID]); - expect(scopeSet.asArray()).to.deep.eq(TEST_CONFIG.DEFAULT_SCOPES); - }); - - it("Does not append client id when scopes are required", () => { - const testScope = "testscope"; - const scopeSet = ScopeSet.fromString(testScope, TEST_CONFIG.MSAL_CLIENT_ID, true); - expect(scopeSet.getOriginalScopesAsArray()).to.deep.eq([testScope]); - expect(scopeSet.asArray()).to.deep.eq([testScope, Constants.OFFLINE_ACCESS_SCOPE]); + expect(() => ScopeSet.fromString(undefined)).to.throw(ClientConfigurationErrorMessage.emptyScopesError.desc); + expect(() => ScopeSet.fromString(undefined)).to.throw(ClientConfigurationError); }); it("Trims and converts array string values to lower case", () => { @@ -123,9 +59,8 @@ describe("ScopeSet.ts", () => { const lowerCaseScope2 = "testscope2"; const testScope3 = " TESTSCOPE3 "; const lowerCaseScope3 = "testscope3"; - const scopeSet = ScopeSet.fromString(`${testScope1} ${testScope2} ${testScope3}`, TEST_CONFIG.MSAL_CLIENT_ID, true); - expect(scopeSet.getOriginalScopesAsArray()).to.deep.eq([lowerCaseScope1, lowerCaseScope2, lowerCaseScope3]); - expect(scopeSet.asArray()).to.deep.eq([lowerCaseScope1, lowerCaseScope2, lowerCaseScope3, Constants.OFFLINE_ACCESS_SCOPE]); + const scopeSet = ScopeSet.fromString(`${testScope1} ${testScope2} ${testScope3}`); + expect(scopeSet.asArray()).to.deep.eq([lowerCaseScope1, lowerCaseScope2, lowerCaseScope3]); }); it("Removes duplicates from input scope array", () => { @@ -135,21 +70,18 @@ describe("ScopeSet.ts", () => { const testScope4 = "testscope"; const testScope5 = "testscope"; const lowerCaseScope = "testscope"; - const scopeSet = ScopeSet.fromString(`${testScope1} ${testScope2} ${testScope3} ${testScope4} ${testScope5}`, TEST_CONFIG.MSAL_CLIENT_ID, true); - expect(scopeSet.getOriginalScopesAsArray()).to.deep.eq([lowerCaseScope]); - expect(scopeSet.asArray()).to.deep.eq([lowerCaseScope, Constants.OFFLINE_ACCESS_SCOPE]); + const scopeSet = ScopeSet.fromString(`${testScope1} ${testScope2} ${testScope3} ${testScope4} ${testScope5}`); + expect(scopeSet.asArray()).to.deep.eq([lowerCaseScope]); }); }); describe("Set functions", () => { - let requiredScopeSet: ScopeSet; - let nonRequiredScopeSet: ScopeSet; + let scopes: ScopeSet; let testScope: string; beforeEach(() => { testScope = "testscope"; - requiredScopeSet = new ScopeSet([testScope], TEST_CONFIG.MSAL_CLIENT_ID, true); - nonRequiredScopeSet = new ScopeSet([testScope], TEST_CONFIG.MSAL_CLIENT_ID, false); + scopes = new ScopeSet([testScope]); }); afterEach(() => { @@ -157,191 +89,161 @@ describe("ScopeSet.ts", () => { }); it("containsScope() checks if a given scope is present in the set of scopes", () => { - expect(requiredScopeSet.containsScope(Constants.OPENID_SCOPE)).to.be.false; - expect(requiredScopeSet.containsScope("notinset")).to.be.false; - - expect(nonRequiredScopeSet.containsScope(Constants.OPENID_SCOPE)).to.be.true; - expect(nonRequiredScopeSet.containsScope(testScope)).to.be.true; - expect(nonRequiredScopeSet.containsScope("notinset")).to.be.false; + expect(scopes.containsScope(Constants.OPENID_SCOPE)).to.be.false; + expect(scopes.containsScope("notinset")).to.be.false; + expect(scopes.containsScope(testScope)).to.be.true; }); it("containsScope() returns false if null or empty scope is passed to it", () => { - expect(requiredScopeSet.containsScope("")).to.be.false; - expect(requiredScopeSet.containsScope(null)).to.be.false; - expect(requiredScopeSet.containsScope(undefined)).to.be.false; + expect(scopes.containsScope("")).to.be.false; + expect(scopes.containsScope(null)).to.be.false; + expect(scopes.containsScope(undefined)).to.be.false; }); it("containsScopeSet() checks if a given ScopeSet is fully contained in another - returns false otherwise", () => { - expect(nonRequiredScopeSet.containsScopeSet(requiredScopeSet)).to.be.true; + const biggerSet = new ScopeSet([testScope, "testScope2", "testScope3"]); + expect(biggerSet.containsScopeSet(scopes)).to.be.true; - const scopeSet = new ScopeSet(["testScope2"], TEST_CONFIG.MSAL_CLIENT_ID, true); - expect(nonRequiredScopeSet.containsScopeSet(scopeSet)).to.be.false; + const alternateSet = new ScopeSet(["testScope2"]); + expect(scopes.containsScopeSet(alternateSet)).to.be.false; }); it("containsScopeSet() returns false if given ScopeSet is null or undefined", () => { - expect(nonRequiredScopeSet.containsScope(null)).to.be.false; - expect(nonRequiredScopeSet.containsScope(undefined)).to.be.false; + expect(scopes.containsScopeSet(null)).to.be.false; + expect(scopes.containsScopeSet(undefined)).to.be.false; }); it("appendScope() adds scope to set after trimming and converting to lower case", () => { - expect(nonRequiredScopeSet.asArray()).to.contain(testScope); - nonRequiredScopeSet.appendScope(" testScope2 "); - expect(nonRequiredScopeSet.asArray()).to.contain("testscope2"); + expect(scopes.asArray()).to.contain(testScope); + scopes.appendScope(" testScope2 "); + expect(scopes.asArray()).to.contain("testscope2"); }); it("appendScope() does not add duplicates to ScopeSet", () => { - const scopeArr = nonRequiredScopeSet.asArray(); - nonRequiredScopeSet.appendScope(testScope); - const newScopeArr = nonRequiredScopeSet.asArray(); + const scopeArr = scopes.asArray(); + scopes.appendScope(testScope); + const newScopeArr = scopes.asArray(); expect(newScopeArr).to.be.deep.eq(scopeArr); }); it("appendScope() does nothing if given scope is empty, null or undefined", () => { const setAddSpy = sinon.spy(Set.prototype, "add"); - expect(() => nonRequiredScopeSet.appendScope("")).to.throw(ClientAuthErrorMessage.appendEmptyScopeError.desc); + expect(() => scopes.appendScope("")).to.throw(ClientAuthError); + expect(() => scopes.appendScope("")).to.throw(ClientAuthErrorMessage.appendEmptyScopeError.desc); expect(setAddSpy.called).to.be.false; - expect(() => nonRequiredScopeSet.appendScope(null)).to.throw(ClientAuthErrorMessage.appendEmptyScopeError.desc); + expect(() => scopes.appendScope(null)).to.throw(ClientAuthError); + expect(() => scopes.appendScope(null)).to.throw(ClientAuthErrorMessage.appendEmptyScopeError.desc); expect(setAddSpy.called).to.be.false; - expect(() => nonRequiredScopeSet.appendScope(undefined)).to.throw(ClientAuthErrorMessage.appendEmptyScopeError.desc); + expect(() => scopes.appendScope(undefined)).to.throw(ClientAuthError); + expect(() => scopes.appendScope(undefined)).to.throw(ClientAuthErrorMessage.appendEmptyScopeError.desc); expect(setAddSpy.called).to.be.false; }); - it("appendScopes() throws error if given array is null or empty", () => { - const setUnionSpy = sinon.spy(ScopeSet.prototype, "unionScopeSets"); - expect(() => requiredScopeSet.appendScopes(null)).to.throw(ClientAuthErrorMessage.appendScopeSetError.desc); - expect(setUnionSpy.called).to.be.false; - - expect(() => requiredScopeSet.appendScopes(undefined)).to.throw(ClientAuthErrorMessage.appendScopeSetError.desc); - expect(setUnionSpy.called).to.be.false; - - expect(() => requiredScopeSet.appendScopes([])).to.throw(ClientAuthErrorMessage.appendScopeSetError.desc); - expect(setUnionSpy.called).to.be.false; + it("appendScopes() throws error if given array is null or undefined", () => { + expect(() => scopes.appendScopes(null)).to.throw(ClientAuthError); + expect(() => scopes.appendScopes(null)).to.throw(ClientAuthErrorMessage.appendScopeSetError.desc); - expect(() => nonRequiredScopeSet.appendScopes(null)).to.throw(ClientAuthErrorMessage.appendScopeSetError.desc); - expect(setUnionSpy.called).to.be.false; - - expect(() => nonRequiredScopeSet.appendScopes(undefined)).to.throw(ClientAuthErrorMessage.appendScopeSetError.desc); - expect(setUnionSpy.called).to.be.false; + expect(() => scopes.appendScopes(undefined)).to.throw(ClientAuthError); + expect(() => scopes.appendScopes(undefined)).to.throw(ClientAuthErrorMessage.appendScopeSetError.desc); }); it("appendScopes() does not change ScopeSet if given array is empty", () => { - const setUnionSpy = sinon.spy(ScopeSet.prototype, "unionScopeSets"); - const scopeArr = nonRequiredScopeSet.asArray(); - nonRequiredScopeSet.appendScopes([]); - for (let i = 0; i < scopeArr.length; i++) { - expect(nonRequiredScopeSet.asArray()).to.contain(scopeArr[i]); - } - expect(setUnionSpy.calledOnce).to.be.true; + const scopeArr = scopes.asArray(); + scopes.appendScopes([]); + expect(scopes.asArray()).to.be.deep.eq(scopeArr); }); it("appendScopes() adds multiple scopes to ScopeSet", () => { const testScope2 = "testscope2"; const testScope3 = "testscope3"; - requiredScopeSet.appendScopes([testScope2, testScope3]); - expect(requiredScopeSet.asArray()).to.contain(testScope2); - expect(requiredScopeSet.asArray()).to.contain(testScope3); + scopes.appendScopes([testScope2, testScope3]); + expect(scopes.asArray()).to.contain(testScope2); + expect(scopes.asArray()).to.contain(testScope3); }); it("appendScopes() does not add duplicate scopes", () => { - const scopeArr = requiredScopeSet.asArray(); - requiredScopeSet.appendScopes([testScope, Constants.OFFLINE_ACCESS_SCOPE]); - expect(requiredScopeSet.asArray()).to.be.deep.eq(scopeArr); + const unchangedScopes = new ScopeSet([testScope, Constants.OFFLINE_ACCESS_SCOPE]); + const scopeArr = unchangedScopes.asArray(); + unchangedScopes.appendScopes([testScope, Constants.OFFLINE_ACCESS_SCOPE]); + expect(unchangedScopes.asArray()).to.be.deep.eq(scopeArr); }); it("removeScopes() throws error if scope is null, undefined or empty", () => { - expect(() => requiredScopeSet.removeScope(null)).to.throw(ClientAuthErrorMessage.removeEmptyScopeError.desc); - expect(() => requiredScopeSet.removeScope(null)).to.throw(ClientAuthError); + expect(() => scopes.removeScope(null)).to.throw(ClientAuthErrorMessage.removeEmptyScopeError.desc); + expect(() => scopes.removeScope(null)).to.throw(ClientAuthError); - expect(() => requiredScopeSet.removeScope(undefined)).to.throw(ClientAuthErrorMessage.removeEmptyScopeError.desc); - expect(() => requiredScopeSet.removeScope(undefined)).to.throw(ClientAuthError); + expect(() => scopes.removeScope(undefined)).to.throw(ClientAuthErrorMessage.removeEmptyScopeError.desc); + expect(() => scopes.removeScope(undefined)).to.throw(ClientAuthError); - expect(() => requiredScopeSet.removeScope("")).to.throw(ClientAuthErrorMessage.removeEmptyScopeError.desc); - expect(() => requiredScopeSet.removeScope("")).to.throw(ClientAuthError); + expect(() => scopes.removeScope("")).to.throw(ClientAuthErrorMessage.removeEmptyScopeError.desc); + expect(() => scopes.removeScope("")).to.throw(ClientAuthError); }); it("removeScopes() correctly removes scopes", () => { - const scopeArr = requiredScopeSet.asArray(); - requiredScopeSet.removeScope("testScope2"); - expect(requiredScopeSet.asArray()).to.be.deep.eq(scopeArr); - requiredScopeSet.removeScope(testScope); - expect(requiredScopeSet.asArray()).to.be.deep.eq([Constants.OFFLINE_ACCESS_SCOPE]); + const scopeArr = scopes.asArray(); + scopes.removeScope("testScope2"); + expect(scopes.asArray()).to.be.deep.eq(scopeArr); + scopes.removeScope(testScope); + expect(scopes.asArray()).to.be.empty; }); it("unionScopeSets() throws error if input is null or undefined", () => { - expect(() => requiredScopeSet.unionScopeSets(null)).to.throw(ClientAuthErrorMessage.emptyInputScopeSetError.desc); - expect(() => requiredScopeSet.unionScopeSets(null)).to.throw(ClientAuthError); + expect(() => scopes.unionScopeSets(null)).to.throw(ClientAuthErrorMessage.emptyInputScopeSetError.desc); + expect(() => scopes.unionScopeSets(null)).to.throw(ClientAuthError); - expect(() => requiredScopeSet.unionScopeSets(undefined)).to.throw(ClientAuthErrorMessage.emptyInputScopeSetError.desc); - expect(() => requiredScopeSet.unionScopeSets(undefined)).to.throw(ClientAuthError); + expect(() => scopes.unionScopeSets(undefined)).to.throw(ClientAuthErrorMessage.emptyInputScopeSetError.desc); + expect(() => scopes.unionScopeSets(undefined)).to.throw(ClientAuthError); }); it("unionScopeSets() combines multiple sets and returns new Set of scopes", () => { const testScope2 = "testScope2"; const testScope3 = "testScope3"; - const newScopeSet = new ScopeSet([testScope2, testScope3], TEST_CONFIG.MSAL_CLIENT_ID, true); + const newScopeSet = new ScopeSet([testScope2, testScope3]); newScopeSet.removeScope(Constants.OFFLINE_ACCESS_SCOPE); - const unionSet = newScopeSet.unionScopeSets(requiredScopeSet); + const unionSet = newScopeSet.unionScopeSets(scopes); const unionArray = Array.from(unionSet); expect(unionSet instanceof Set).to.be.true; - expect(unionSet.size).to.be.eq(newScopeSet.getScopeCount() + requiredScopeSet.getScopeCount()); + expect(unionSet.size).to.be.eq(newScopeSet.getScopeCount() + scopes.getScopeCount()); for(let i = 0; i < unionArray.length; i++) { - expect(newScopeSet.containsScope(unionArray[i]) || requiredScopeSet.containsScope(unionArray[i])).to.be.true; + expect(newScopeSet.containsScope(unionArray[i]) || scopes.containsScope(unionArray[i])).to.be.true; } }); it("intersectingScopeSets() throws error if input is null or undefined", () => { - expect(() => requiredScopeSet.intersectingScopeSets(null)).to.throw(ClientAuthErrorMessage.emptyInputScopeSetError.desc); - expect(() => requiredScopeSet.intersectingScopeSets(null)).to.throw(ClientAuthError); + expect(() => scopes.intersectingScopeSets(null)).to.throw(ClientAuthErrorMessage.emptyInputScopeSetError.desc); + expect(() => scopes.intersectingScopeSets(null)).to.throw(ClientAuthError); - expect(() => requiredScopeSet.intersectingScopeSets(undefined)).to.throw(ClientAuthErrorMessage.emptyInputScopeSetError.desc); - expect(() => requiredScopeSet.intersectingScopeSets(undefined)).to.throw(ClientAuthError); + expect(() => scopes.intersectingScopeSets(undefined)).to.throw(ClientAuthErrorMessage.emptyInputScopeSetError.desc); + expect(() => scopes.intersectingScopeSets(undefined)).to.throw(ClientAuthError); }); it("intersectingScopeSets() returns true if ScopeSets have one or more scopes in common", () => { const testScope2 = "testScope2"; - const newScopeSet = new ScopeSet([testScope, testScope2], TEST_CONFIG.MSAL_CLIENT_ID, true); - expect(newScopeSet.intersectingScopeSets(requiredScopeSet)).to.be.true; + const newScopeSet = new ScopeSet([testScope, testScope2]); + expect(newScopeSet.intersectingScopeSets(scopes)).to.be.true; }); it("intersectingScopeSets() returns false if ScopeSets have no scopes in common", () => { const testScope2 = "testScope2"; const testScope3 = "testScope3"; - const newScopeSet = new ScopeSet([testScope2, testScope3], TEST_CONFIG.MSAL_CLIENT_ID, true); + const newScopeSet = new ScopeSet([testScope2, testScope3]); newScopeSet.removeScope(Constants.OFFLINE_ACCESS_SCOPE); - expect(newScopeSet.intersectingScopeSets(requiredScopeSet)).to.be.false; - }); - - it("intersectingScopeSets() returns false if ScopeSets have no scopes in common other than offline_access", () => { - const testScope2 = "testScope2"; - const newScopeSet = new ScopeSet([testScope2], TEST_CONFIG.MSAL_CLIENT_ID, true); - - expect(newScopeSet.asArray()).to.contain("offline_access"); - expect(requiredScopeSet.asArray()).to.contain("offline_access"); - expect(newScopeSet.intersectingScopeSets(requiredScopeSet)).to.be.false; + expect(newScopeSet.intersectingScopeSets(scopes)).to.be.false; }); it("getScopeCount() correctly returns the size of the ScopeSet", () => { - expect(requiredScopeSet.getScopeCount()).to.be.eq(2); - expect(nonRequiredScopeSet.getScopeCount()).to.be.eq(4); - - requiredScopeSet.removeScope(Constants.OFFLINE_ACCESS_SCOPE); - expect(requiredScopeSet.getScopeCount()).to.be.eq(1); - - nonRequiredScopeSet.removeScope(Constants.OFFLINE_ACCESS_SCOPE); - expect(nonRequiredScopeSet.getScopeCount()).to.be.eq(3); - }); + expect(scopes.getScopeCount()).to.be.eq(1); - it("isLoginScopeSet() returns true if client id, openid or profile scope are present in the set of scopes", () => { - expect(nonRequiredScopeSet.isLoginScopeSet()).to.be.true; - }); + const twoScopes = new ScopeSet(["1", "2"]); + expect(twoScopes.getScopeCount()).to.be.eq(2); - it("isLoginScopeSet() returns false if original scopes do not have any scopes pertaining to a login call", () => { - expect(requiredScopeSet.isLoginScopeSet()).to.be.false; + const threeScopes = new ScopeSet(["1", "2", "3"]) + expect(threeScopes.getScopeCount()).to.be.eq(3); }); }); @@ -352,8 +254,8 @@ describe("ScopeSet.ts", () => { let testScope: string; beforeEach(() => { testScope = "testscope"; - requiredScopeSet = new ScopeSet([testScope], TEST_CONFIG.MSAL_CLIENT_ID, true); - nonRequiredScopeSet = new ScopeSet([testScope], TEST_CONFIG.MSAL_CLIENT_ID, false); + requiredScopeSet = new ScopeSet([testScope]); + nonRequiredScopeSet = new ScopeSet([testScope]); }); afterEach(() => { @@ -369,17 +271,6 @@ describe("ScopeSet.ts", () => { } }); - it("getOriginalScopesAsArray() returns the original scopes as an array", () => { - const originalScopeArr = nonRequiredScopeSet.getOriginalScopesAsArray(); - const scopeArr = nonRequiredScopeSet.asArray(); - expect(scopeArr).to.contain(Constants.OPENID_SCOPE); - expect(scopeArr).to.contain(Constants.PROFILE_SCOPE); - expect(scopeArr).to.contain(Constants.OFFLINE_ACCESS_SCOPE); - expect(scopeArr).to.contain(testScope); - expect(originalScopeArr).to.contain(testScope); - expect(originalScopeArr).to.contain(TEST_CONFIG.MSAL_CLIENT_ID); - }); - it("printScopes() prints space-delimited string of scopes", () => { const scopeArr = nonRequiredScopeSet.asArray(); expect(nonRequiredScopeSet.printScopes()).to.be.eq(scopeArr.join(" ")); diff --git a/lib/msal-common/test/server/RequestParameterBuilder.spec.ts b/lib/msal-common/test/server/RequestParameterBuilder.spec.ts index 11d537cacd..c3b2d57765 100644 --- a/lib/msal-common/test/server/RequestParameterBuilder.spec.ts +++ b/lib/msal-common/test/server/RequestParameterBuilder.spec.ts @@ -21,7 +21,7 @@ describe("RequestParameterBuilder unit tests", () => { const requestParameterBuilder = new RequestParameterBuilder(); requestParameterBuilder.addResponseTypeCode(); requestParameterBuilder.addResponseMode(ResponseMode.FORM_POST); - requestParameterBuilder.addScopes(new ScopeSet(TEST_CONFIG.DEFAULT_SCOPES, TEST_CONFIG.MSAL_CLIENT_ID, false)); + requestParameterBuilder.addScopes(new ScopeSet(TEST_CONFIG.DEFAULT_SCOPES, TEST_CONFIG.MSAL_CLIENT_ID)); requestParameterBuilder.addClientId(TEST_CONFIG.MSAL_CLIENT_ID); requestParameterBuilder.addRedirectUri(TEST_URIS.TEST_REDIRECT_URI_LOCALHOST); requestParameterBuilder.addDomainHint(TEST_CONFIG.DOMAIN_HINT); diff --git a/lib/msal-node/src/client/ClientApplication.ts b/lib/msal-node/src/client/ClientApplication.ts index dddd3e24ab..2de076051f 100644 --- a/lib/msal-node/src/client/ClientApplication.ts +++ b/lib/msal-node/src/client/ClientApplication.ts @@ -19,6 +19,7 @@ import { JsonCache, Serializer, InMemoryCache, + BaseAuthRequest, } from '@azure/msal-common'; import { Configuration, buildAppConfiguration } from '../config/Configuration'; import { CryptoProvider } from '../crypto/CryptoProvider'; @@ -64,7 +65,7 @@ export abstract class ClientApplication { const authorizationCodeClient = new AuthorizationCodeClient( authClientConfig ); - return authorizationCodeClient.getAuthCodeUrl(request); + return authorizationCodeClient.getAuthCodeUrl(this.initializeRequestScopes(request) as AuthorizationUrlRequest); } /** @@ -77,16 +78,14 @@ export abstract class ClientApplication { * * @param request */ - async acquireTokenByCode( - request: AuthorizationCodeRequest - ): Promise { + async acquireTokenByCode(request: AuthorizationCodeRequest): Promise { const authClientConfig = await this.buildOauthClientConfiguration( request.authority ); const authorizationCodeClient = new AuthorizationCodeClient( authClientConfig ); - return authorizationCodeClient.acquireToken(request); + return authorizationCodeClient.acquireToken(this.initializeRequestScopes(request) as AuthorizationCodeRequest); } /** @@ -97,21 +96,17 @@ export abstract class ClientApplication { * handle the caching and refreshing of tokens automatically. * @param request */ - async acquireTokenByRefreshToken( - request: RefreshTokenRequest - ): Promise { + async acquireTokenByRefreshToken(request: RefreshTokenRequest): Promise { const refreshTokenClientConfig = await this.buildOauthClientConfiguration( request.authority ); const refreshTokenClient = new RefreshTokenClient( refreshTokenClientConfig ); - return refreshTokenClient.acquireToken(request); + return refreshTokenClient.acquireToken(this.initializeRequestScopes(request) as RefreshTokenRequest); } - protected async buildOauthClientConfiguration( - authority?: string - ): Promise { + protected async buildOauthClientConfiguration(authority?: string): Promise { // using null assertion operator as we ensure that all config values have default values in buildConfiguration() return { authOptions: { @@ -137,20 +132,31 @@ export abstract class ClientApplication { }; } + /** + * Generates a request with the default scopes. + * @param authRequest + */ + protected initializeRequestScopes(authRequest: BaseAuthRequest): BaseAuthRequest { + const request: BaseAuthRequest = { ...authRequest }; + if (!request.scopes) { + request.scopes = [Constants.OPENID_SCOPE, Constants.PROFILE_SCOPE, Constants.OFFLINE_ACCESS_SCOPE]; + } else { + request.scopes.push(Constants.OPENID_SCOPE, Constants.PROFILE_SCOPE, Constants.OFFLINE_ACCESS_SCOPE); + } + return request; + } + /** * Create authority instance. If authority not passed in request, default to authority set on the application * object. If no authority set in application object, then default to common authority. * @param authorityString */ - private async createAuthority( - authorityString?: string - ): Promise { + private async createAuthority(authorityString?: string): Promise { const authority: Authority = authorityString ? AuthorityFactory.createInstance( - authorityString, - this.config.system!.networkClient! - ) - : this.authority; + authorityString, + this.config.system!.networkClient! + ) : this.authority; if (authority.discoveryComplete()) { return authority; diff --git a/lib/msal-node/src/client/PublicClientApplication.ts b/lib/msal-node/src/client/PublicClientApplication.ts index 875f6a97fe..fa43d1e608 100644 --- a/lib/msal-node/src/client/PublicClientApplication.ts +++ b/lib/msal-node/src/client/PublicClientApplication.ts @@ -47,13 +47,11 @@ export class PublicClientApplication extends ClientApplication { * until the end-user completes input of credentials. * @param request */ - public async acquireTokenByDeviceCode( - request: DeviceCodeRequest - ): Promise { + public async acquireTokenByDeviceCode(request: DeviceCodeRequest): Promise { const deviceCodeConfig = await this.buildOauthClientConfiguration( request.authority ); const deviceCodeClient = new DeviceCodeClient(deviceCodeConfig); - return deviceCodeClient.acquireToken(request); + return deviceCodeClient.acquireToken(this.initializeRequestScopes(request) as DeviceCodeRequest); } } diff --git a/samples/VanillaJSTestApp2.0/app/default/auth.js b/samples/VanillaJSTestApp2.0/app/default/auth.js index 007d15e870..cb5d5641f6 100644 --- a/samples/VanillaJSTestApp2.0/app/default/auth.js +++ b/samples/VanillaJSTestApp2.0/app/default/auth.js @@ -22,7 +22,6 @@ myMSALObj.handleRedirectPromise().then(handleResponse).catch(err => { function handleResponse(resp) { if (resp !== null) { - console.log(resp); showWelcomeMessage(resp.account); } else { // need to call getAccount here?