Skip to content

Commit

Permalink
Merge pull request #1770 from AzureAD/msal-common-scopeset-update
Browse files Browse the repository at this point in the history 
[msal-common family][#3] Utilize ScopeSet across the library
  • Loading branch information
Prithvi Kanherkar committed Jun 18, 2020
2 parents c968ef7 + 8057856 commit a42591f
Show file tree
Hide file tree
Showing 36 changed files with 360 additions and 718 deletions.
62 changes: 21 additions & 41 deletions lib/msal-browser/src/app/PublicClientApplication.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {
CacheSchemaType,
AuthenticationResult,
SilentFlowRequest,
AccountEntity,
IAccount
} from "@azure/msal-common";
import { buildConfiguration, Configuration } from "../config/Configuration";
Expand Down Expand Up @@ -270,25 +269,7 @@ export class PublicClientApplication {
* @param {@link (AuthenticationParameters:type)}
*/
async loginRedirect(request: AuthorizationUrlRequest): Promise<void> {
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));
}

/**
Expand All @@ -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);
Expand All @@ -335,22 +316,7 @@ export class PublicClientApplication {
* @returns {Promise.<AuthenticationResult>} - 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<AuthenticationResult> {
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));
}

/**
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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(" ") : "";
Expand Down Expand Up @@ -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.
*/
Expand All @@ -601,7 +581,7 @@ export class PublicClientApplication {
if (this.interactionInProgress()) {
throw BrowserAuthError.createInteractionInProgressError();
}

return this.initializeRequest(request);
}

Expand Down
2 changes: 1 addition & 1 deletion lib/msal-browser/src/cache/BrowserStorage.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
2 changes: 1 addition & 1 deletion lib/msal-browser/src/utils/BrowserConstants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
39 changes: 23 additions & 16 deletions lib/msal-browser/test/app/PublicClientApplication.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
Expand Down Expand Up @@ -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
};
Expand Down Expand Up @@ -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);
});
Expand Down Expand Up @@ -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({
Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
});
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -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, <any>"loadFrameSync");
sinon.stub(SilentHandler.prototype, "monitorFrameForHash").resolves(TEST_HASHES.TEST_SUCCESS_CODE_HASH);
sinon.stub(SilentHandler.prototype, "handleCodeResponse").resolves(testTokenResponse);
Expand Down Expand Up @@ -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
};
Expand Down Expand Up @@ -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, <any>"silentTokenHelper").resolves(testTokenResponse);
sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({
challenge: TEST_CONFIG.TEST_CHALLENGE,
Expand Down
Loading

0 comments on commit a42591f

Please sign in to comment.