Skip to content

[msal-common family][#3] Utilize ScopeSet across the library #1770

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jun 18, 2020
Merged

[msal-common family][#3] Utilize ScopeSet across the library #1770

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
34de2bb
Adding fromCache, nonce checks
pkanher617 Jun 10, 2020
38e5bb1
Refactored scopeset and utilized intersection and subset in unifiedCa…
pkanher617 Jun 10, 2020
59e7327
fixed scopes to always add defaults
pkanher617 Jun 10, 2020
0b4e4b7
Merge branch 'msal-node-cache-merge' into msal-common-scopeset-update
pkanher617 Jun 11, 2020
c28a1de
Update AccessTokenEntity.ts
pkanher617 Jun 11, 2020
cf8859d
Merge branch 'msal-node-cache-merge' into msal-common-scopeset-update
pkanher617 Jun 15, 2020
618e306
builds passing
pkanher617 Jun 15, 2020
eaa73ed
common tests passing, removed default scopes from msal-common
pkanher617 Jun 15, 2020
370e37c
fixed all tests
pkanher617 Jun 15, 2020
de739e6
Update sdl-tasks.yml
pkanher617 Jun 17, 2020
15cda15
Adding base request type
pkanher617 Jun 17, 2020
8057856
Merge branch 'dev' into msal-common-scopeset-update
pkanher617 Jun 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 21 additions & 41 deletions lib/msal-browser/src/app/PublicClientApplication.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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