Skip to content
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.

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

[msal-browser] Remove default scopes from silent flow #1962

Merged
merged 11 commits into from
Jul 17, 2020
Merged
3 changes: 2 additions & 1 deletion lerna.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
"@azure/msal-browser",
"@azure/msal-angular",
"@azure/msal-node",
"vanilla-js-test-app"
"vanilla-js-test-app",
"vanilla-js-test-app-2.0"
],
"stream": true
}
Expand Down
10 changes: 5 additions & 5 deletions lib/msal-browser/src/app/PublicClientApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -591,10 +591,7 @@ export class PublicClientApplication implements IPublicClientApplication {

validatedRequest.correlationId = (request && request.correlationId) || this.browserCrypto.createNewGuid();

return {
...validatedRequest,
...this.setDefaultScopes(validatedRequest)
};
return validatedRequest;
}

/**
Expand Down Expand Up @@ -650,7 +647,10 @@ export class PublicClientApplication implements IPublicClientApplication {

this.browserStorage.updateCacheEntries(validatedRequest.state, validatedRequest.nonce, validatedRequest.authority);

return validatedRequest;
return {
...validatedRequest,
...this.setDefaultScopes(validatedRequest)
};
}

/**
Expand Down
58 changes: 34 additions & 24 deletions lib/msal-browser/test/app/PublicClientApplication.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const expect = chai.expect;
import sinon from "sinon";
import { PublicClientApplication } from "../../src/app/PublicClientApplication";
import { TEST_CONFIG, TEST_URIS, TEST_HASHES, TEST_TOKENS, TEST_DATA_CLIENT_INFO, TEST_TOKEN_LIFETIMES, RANDOM_TEST_GUID, DEFAULT_OPENID_CONFIG_RESPONSE, testNavUrl, testLogoutUrl, TEST_STATE_VALUES } from "../utils/StringConstants";
import { ServerError, Constants, AccountInfo, IdTokenClaims, PromptValue, AuthenticationResult, AuthorizationCodeRequest, AuthorizationUrlRequest, IdToken, PersistentCacheKeys, SilentFlowRequest, CacheSchemaType, TimeUtils, AuthorizationCodeClient, ResponseMode, SilentFlowClient, TrustedAuthority, EndSessionRequest, CloudDiscoveryMetadata } from "@azure/msal-common";
import { ServerError, Constants, AccountInfo, IdTokenClaims, PromptValue, AuthenticationResult, AuthorizationCodeRequest, AuthorizationUrlRequest, IdToken, PersistentCacheKeys, SilentFlowRequest, CacheSchemaType, TimeUtils, AuthorizationCodeClient, ResponseMode, SilentFlowClient, TrustedAuthority, EndSessionRequest, CloudDiscoveryMetadata, ProtocolUtils } from "@azure/msal-common";
import { BrowserConfigurationAuthError } from "../../src/error/BrowserConfigurationAuthError";
import { BrowserUtils } from "../../src/utils/BrowserUtils";
import { BrowserConstants, TemporaryCacheKeys } from "../../src/utils/BrowserConstants";
Expand Down Expand Up @@ -1019,19 +1019,30 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
const testTokenResponse: AuthenticationResult = {
uniqueId: testIdTokenClaims.oid,
tenantId: testIdTokenClaims.tid,
scopes: TEST_CONFIG.DEFAULT_SCOPES,
scopes: ["scope1"],
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(SilentFlowClient.prototype, "acquireToken").resolves(testTokenResponse);
sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID);
const silentATStub = sinon.stub(SilentFlowClient.prototype, "acquireToken").resolves(testTokenResponse);
const tokenRequest: SilentFlowRequest = {
scopes: ["scope1"],
account: testAccount
};
const expectedTokenRequest: SilentFlowRequest = {
...tokenRequest,
authority: `${Constants.DEFAULT_AUTHORITY}`,
correlationId: RANDOM_TEST_GUID
};
const tokenResp = await pca.acquireTokenSilent({
scopes: TEST_CONFIG.DEFAULT_SCOPES,
scopes: ["scope1"],
account: testAccount
});
expect(silentATStub.calledWith(expectedTokenRequest)).to.be.true;
expect(tokenResp).to.be.deep.eq(testTokenResponse);
});

Expand Down Expand Up @@ -1086,7 +1097,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
const testTokenResponse: AuthenticationResult = {
uniqueId: testIdTokenClaims.oid,
tenantId: testIdTokenClaims.tid,
scopes: TEST_CONFIG.DEFAULT_SCOPES,
scopes: [...TEST_CONFIG.DEFAULT_SCOPES, "User.Read"],
idToken: testServerTokenResponse.id_token,
idTokenClaims: testIdTokenClaims,
accessToken: testServerTokenResponse.access_token,
Expand All @@ -1100,30 +1111,29 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
challenge: TEST_CONFIG.TEST_CHALLENGE,
verifier: TEST_CONFIG.TEST_VERIFIER
});
sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID);
const urlRequest: AuthorizationUrlRequest = {
scopes: TEST_CONFIG.DEFAULT_SCOPES,
redirectUri: "",
prompt: "none",
state: RANDOM_TEST_GUID,
correlationId: RANDOM_TEST_GUID,
authority: `${Constants.DEFAULT_AUTHORITY}`,
nonce: RANDOM_TEST_GUID
};
const codeRequest: AuthorizationCodeRequest = {
...urlRequest,
redirectUri: urlRequest.redirectUri,
code: "",
codeVerifier: TEST_CONFIG.TEST_VERIFIER
};
sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID);
sinon.stub(ProtocolUtils, "setRequestState").returns(RANDOM_TEST_GUID);
const silentFlowRequest: SilentFlowRequest = {
scopes: TEST_CONFIG.DEFAULT_SCOPES,
scopes: ["User.Read"],
account: testAccount
};
const expectedRequest: AuthorizationUrlRequest = {
...silentFlowRequest,
scopes: ["User.Read", ...TEST_CONFIG.DEFAULT_SCOPES],
authority: `${Constants.DEFAULT_AUTHORITY}`,
correlationId: RANDOM_TEST_GUID,
prompt: "none",
redirectUri: TEST_URIS.TEST_REDIR_URI,
state: RANDOM_TEST_GUID,
nonce: RANDOM_TEST_GUID,
responseMode: ResponseMode.FRAGMENT,
codeChallenge: TEST_CONFIG.TEST_CHALLENGE,
codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD
};
const tokenResp = await pca.acquireTokenSilent(silentFlowRequest);

expect(tokenResp).to.be.deep.eq(testTokenResponse);
expect(createAcqTokenStub.calledOnce).to.be.true;
expect(createAcqTokenStub.calledWith(expectedRequest)).to.be.true;
expect(silentTokenHelperStub.calledWith(testNavUrl)).to.be.true;
});
});
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
},
"scripts": {
"postinstall": "node build/postinstall.js",
"build": "lerna run build",
"test": "lerna run test",
"build": "lerna run --concurrency 1 build",
"test": "lerna run --concurrency 1 test",
"test:coverage": "npm run clean:coverage && lerna run test:coverage",
"test:report": "nyc report | coveralls",
"test:e2e": "lerna run --concurrency 1 test:e2e",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ const msalConfig = {

// Add here scopes for id token to be used at MS Identity Platform endpoints.
const loginRequest = {
scopes: ["User.Read"],
redirectUri: "http://localhost:30662/"
scopes: ["User.Read"]
};

// Add here the endpoints for MS Graph API services you would like to use.
Expand All @@ -48,11 +47,9 @@ const graphConfig = {
// Add here scopes for access token to be used at MS Graph API endpoints.
const tokenRequest = {
scopes: ["Mail.Read"],
redirectUri: "http://localhost:30662/",
forceRefresh: false // Set this to "true" to skip a cached token and go to the server to get a new token
};

const silentRequest = {
scopes: ["openid", "profile", "User.Read", "Mail.Read"],
redirectUri: "http://localhost:30662/"
scopes: ["openid", "profile", "User.Read", "Mail.Read"]
};
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ const msalConfig = {

// Add here scopes for id token to be used at MS Identity Platform endpoints.
const loginRequest = {
scopes: ["User.Read"],
redirectUri: "http://localhost:30662/"
scopes: ["User.Read"]
};

// Add here the endpoints for MS Graph API services you would like to use.
Expand All @@ -48,11 +47,9 @@ const graphConfig = {
// Add here scopes for access token to be used at MS Graph API endpoints.
const tokenRequest = {
scopes: ["https://msidlab0.spoppe.com/User.Read"],
redirectUri: "http://localhost:30662/",
forceRefresh: false // Set this to "true" to skip a cached token and go to the server to get a new token
};

const silentRequest = {
scopes: ["openid", "profile", "User.Read", "Mail.Read"],
redirectUri: "http://localhost:30662/"
scopes: ["openid", "profile", "User.Read", "Mail.Read"]
};
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ const msalConfig = {

// Add here scopes for id token to be used at MS Identity Platform endpoints.
const loginRequest = {
scopes: ["User.Read"],
redirectUri: "http://localhost:30662/"
scopes: ["User.Read"]
};

// Add here the endpoints for MS Graph API services you would like to use.
Expand All @@ -48,11 +47,9 @@ const graphConfig = {
// Add here scopes for access token to be used at MS Graph API endpoints.
const tokenRequest = {
scopes: ["Mail.Read"],
redirectUri: "http://localhost:30662/",
forceRefresh: false // Set this to "true" to skip a cached token and go to the server to get a new token
};

const silentRequest = {
scopes: ["openid", "profile", "User.Read", "Mail.Read"],
redirectUri: "http://localhost:30662/"
scopes: ["openid", "profile", "User.Read", "Mail.Read"]
};
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ const msalConfig = {

// Add here scopes for id token to be used at MS Identity Platform endpoints.
const loginRequest = {
scopes: ["User.Read"],
redirectUri: "http://localhost:30662/"
scopes: ["User.Read"]
};

// Add here the endpoints for MS Graph API services you would like to use.
Expand All @@ -48,12 +47,10 @@ const graphConfig = {
// Add here scopes for access token to be used at MS Graph API endpoints.
const tokenRequest = {
scopes: ["Mail.Read", "openid", "profile"],
redirectUri: "http://localhost:30662/",
forceRefresh: false // Set this to "true" to skip a cached token and go to the server to get a new token
};

const silentRequest = {
scopes: ["openid", "profile", "User.Read", "Mail.Read"],
redirectUri: "http://localhost:30662/",
loginHint: "IDLAB@msidlab0.ccsctp.net"
};
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"resolveJsonModule": true
},
"files": [
"./test/*.ts"
"./e2eTests/*.ts"
],
"exclude": [
"node_modules"
Expand Down
2 changes: 1 addition & 1 deletion samples/msal-core-samples/VanillaJSTestApp/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"resolveJsonModule": true
},
"files": [
"./test/*.ts"
"./e2eTests/*.ts"
],
"exclude": [
"node_modules"
Expand Down