Skip to content

Commit 56e01f9

Browse files
sameeragsameerag
committed
Merge branch 'dev' into msal-node-silent-flow-client
2 parents 5753b70 + 8ec2586 commit 56e01f9

File tree

11 files changed

+92
-94
lines changed

11 files changed

+92
-94
lines changed

‎lib/msal-browser/src/app/PublicClientApplication.ts

Lines changed: 20 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,14 @@ export class PublicClientApplication {
102102
// Initialize default authority instance
103103
TrustedAuthority.setTrustedAuthoritiesFromConfig(this.config.auth.knownAuthorities, this.config.auth.cloudDiscoveryMetadata);
104104

105-
this.defaultAuthorityPromise = AuthorityFactory.createDiscoveredInstance(
106-
this.config.auth.authority,
107-
this.networkClient
108-
);
105+
this.defaultAuthorityPromise = AuthorityFactory.createDiscoveredInstance(this.config.auth.authority, this.networkClient);
106+
107+
const { location: { hash } } = window;
108+
const cachedHash = this.browserStorage.getItem(this.browserStorage.generateCacheKey(TemporaryCacheKeys.URL_HASH), CacheSchemaType.TEMPORARY) as string;
109+
if (StringUtils.isEmpty(hash) && StringUtils.isEmpty(cachedHash)) {
110+
// There is no hash - assume we are in clean state and clear any current request data.
111+
this.browserStorage.cleanRequest();
112+
}
109113
}
110114

111115
// #region Redirect Flow
@@ -156,19 +160,19 @@ export class PublicClientApplication {
156160
*/
157161
private async handleRedirectResponse(): Promise<AuthenticationResult> {
158162
// Get current location hash from window or cache.
159-
const {location: {hash}} = window;
163+
const { location: { hash } } = window;
160164
const cachedHash = this.browserStorage.getItem(this.browserStorage.generateCacheKey(TemporaryCacheKeys.URL_HASH), CacheSchemaType.TEMPORARY) as string;
161165
const isResponseHash = UrlString.hashContainsKnownProperties(hash);
162166
const loginRequestUrl = this.browserStorage.getItem(this.browserStorage.generateCacheKey(TemporaryCacheKeys.ORIGIN_URI), CacheSchemaType.TEMPORARY) as string;
163167
const currentUrl = BrowserUtils.getCurrentUri();
164-
if (loginRequestUrl === currentUrl) {
168+
if (loginRequestUrl === currentUrl || !this.config.auth.navigateToLoginRequestUrl) {
165169
// We don't need to navigate - check for hash and prepare to process
166170
if (isResponseHash) {
167171
BrowserUtils.clearHash();
168172
return this.handleHash(hash);
169173
} else {
170174
// Loaded page with no valid hash - pass in the value retrieved from cache, or null/empty string
171-
return this.handleHash(`${cachedHash}`);
175+
return this.handleHash(cachedHash);
172176
}
173177
}
174178

@@ -184,39 +188,27 @@ export class PublicClientApplication {
184188
// Navigate to target url
185189
BrowserUtils.navigateWindow(loginRequestUrl, true);
186190
}
187-
return null;
188-
}
189-
190-
if (!isResponseHash) {
191-
// Loaded page with no valid hash - pass in the value retrieved from cache, or null/empty string
192-
return this.handleHash(cachedHash);
193-
}
194-
195-
if (!this.config.auth.navigateToLoginRequestUrl) {
196-
// We don't need to navigate - check for hash and prepare to process
197-
BrowserUtils.clearHash();
198-
return this.handleHash(hash);
199191
}
200192

201193
return null;
202194
}
203195

204196
/**
205-
* Checks if hash exists and handles in window. Otherwise, cancel any current requests and continue.
197+
* Checks if hash exists and handles in window.
206198
* @param responseHash
207199
* @param interactionHandler
208200
*/
209201
private async handleHash(responseHash: string): Promise<AuthenticationResult> {
202+
// There is no hash - return null.
203+
if (StringUtils.isEmpty(responseHash)) {
204+
return null;
205+
}
206+
207+
// Hash contains known properties - handle and return in callback
210208
const currentAuthority = this.browserStorage.getCachedAuthority();
211209
const authClient = await this.createAuthCodeClient(currentAuthority);
212210
const interactionHandler = new RedirectHandler(authClient, this.browserStorage);
213-
if (!StringUtils.isEmpty(responseHash)) {
214-
// Hash contains known properties - handle and return in callback
215-
return interactionHandler.handleCodeResponse(responseHash, this.browserCrypto);
216-
}
217-
// There is no hash - assume we are in clean state and clear any current request data.
218-
this.browserStorage.cleanRequest();
219-
return null;
211+
return interactionHandler.handleCodeResponse(responseHash, this.browserCrypto);
220212
}
221213

222214
/**
@@ -448,6 +440,7 @@ export class PublicClientApplication {
448440
// Handle response from hash string.
449441
return await silentHandler.handleCodeResponse(hash);
450442
} catch (e) {
443+
this.browserStorage.cleanRequest();
451444
throw e;
452445
}
453446
}

‎lib/msal-browser/test/app/PublicClientApplication.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
172172
expect(tokenResponse.idToken).to.be.eq(testTokenResponse.idToken);
173173
expect(tokenResponse.idTokenClaims).to.be.contain(testTokenResponse.idTokenClaims);
174174
expect(tokenResponse.accessToken).to.be.eq(testTokenResponse.accessToken);
175-
// expect(testTokenResponse.expiresOn.getMilliseconds() >= tokenResponse.expiresOn.getMilliseconds()).to.be.true;
175+
expect(testTokenResponse.expiresOn.getMilliseconds() >= tokenResponse.expiresOn.getMilliseconds()).to.be.true;
176176
expect(window.sessionStorage.length).to.be.eq(4);
177177
});
178178

@@ -273,7 +273,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
273273
expect(tokenResponse.idToken).to.be.eq(testTokenResponse.idToken);
274274
expect(tokenResponse.idTokenClaims).to.be.contain(testTokenResponse.idTokenClaims);
275275
expect(tokenResponse.accessToken).to.be.eq(testTokenResponse.accessToken);
276-
// expect(testTokenResponse.expiresOn.getMilliseconds() >= tokenResponse.expiresOn.getMilliseconds()).to.be.true;
276+
expect(testTokenResponse.expiresOn.getMilliseconds() >= tokenResponse.expiresOn.getMilliseconds()).to.be.true;
277277
expect(window.sessionStorage.length).to.be.eq(4);
278278
expect(window.location.hash).to.be.empty;
279279
});

‎lib/msal-common/src/cache/CacheManager.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ export abstract class CacheManager implements ICacheManager {
8282
* saves a cache record
8383
* @param cacheRecord
8484
*/
85-
saveCacheRecord(cacheRecord: CacheRecord, responseScopes?: ScopeSet): void {
85+
saveCacheRecord(cacheRecord: CacheRecord): void {
8686
if (!cacheRecord) {
8787
throw ClientAuthError.createNullOrUndefinedCacheRecord();
8888
}
@@ -96,7 +96,7 @@ export abstract class CacheManager implements ICacheManager {
9696
}
9797

9898
if (!!cacheRecord.accessToken) {
99-
this.saveAccessToken(cacheRecord.accessToken, responseScopes);
99+
this.saveAccessToken(cacheRecord.accessToken);
100100
}
101101

102102
if (!!cacheRecord.refreshToken) {
@@ -134,19 +134,20 @@ export abstract class CacheManager implements ICacheManager {
134134
* saves access token credential
135135
* @param credential
136136
*/
137-
private saveAccessToken(credential: AccessTokenEntity, responseScopes: ScopeSet): void {
137+
private saveAccessToken(credential: AccessTokenEntity): void {
138138
const currentTokenCache = this.getCredentialsFilteredBy({
139139
clientId: credential.clientId,
140140
credentialType: CredentialType.ACCESS_TOKEN,
141141
environment: credential.environment,
142142
homeAccountId: credential.homeAccountId,
143143
realm: credential.realm
144144
});
145+
const currentScopes = ScopeSet.fromString(credential.target);
145146
const currentAccessTokens: AccessTokenEntity[] = Object.values(currentTokenCache.accessTokens) as AccessTokenEntity[];
146147
if (currentAccessTokens) {
147148
currentAccessTokens.forEach((tokenEntity) => {
148149
const tokenScopeSet = ScopeSet.fromString(tokenEntity.target);
149-
if (tokenScopeSet.intersectingScopeSets(responseScopes)) {
150+
if (tokenScopeSet.intersectingScopeSets(currentScopes)) {
150151
this.removeCredential(tokenEntity);
151152
}
152153
});

‎lib/msal-common/src/cache/interface/ICacheManager.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import {
1111
CredentialFilter
1212
} from "../utils/CacheTypes";
1313
import { CacheRecord } from "../entities/CacheRecord";
14-
import { ScopeSet } from "../../request/ScopeSet";
1514
import { AccountEntity } from "../entities/AccountEntity";
1615
import { AccountInfo } from "../../account/AccountInfo";
1716

@@ -26,7 +25,7 @@ export interface ICacheManager {
2625
* saves a cache record
2726
* @param cacheRecord
2827
*/
29-
saveCacheRecord(cacheRecord: CacheRecord, responseScopes: ScopeSet): void;
28+
saveCacheRecord(cacheRecord: CacheRecord): void;
3029

3130
/**
3231
* Given account key retrieve an account

‎lib/msal-common/src/client/AuthorizationCodeClient.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ export class AuthorizationCodeClient extends BaseClient {
7070

7171
// Validate response. This function throws a server error if an error is returned by the server.
7272
responseHandler.validateTokenResponse(response.body);
73-
const tokenResponse = responseHandler.generateAuthenticationResult(response.body, this.authority, cachedNonce, cachedState);
73+
const tokenResponse = responseHandler.handleServerTokenResponse(response.body, this.authority, cachedNonce, cachedState);
7474

7575
return tokenResponse;
7676
}

‎lib/msal-common/src/client/RefreshTokenClient.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export class RefreshTokenClient extends BaseClient {
3434
);
3535

3636
responseHandler.validateTokenResponse(response.body);
37-
const tokenResponse = responseHandler.generateAuthenticationResult(
37+
const tokenResponse = responseHandler.handleServerTokenResponse(
3838
response.body,
3939
this.authority
4040
);

‎lib/msal-common/src/client/SilentFlowClient.ts

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { CredentialFilter, CredentialCache } from "../cache/utils/CacheTypes";
2121
import { AccountEntity } from "../cache/entities/AccountEntity";
2222
import { CredentialEntity } from "../cache/entities/CredentialEntity";
2323
import { ClientConfigurationError } from "../error/ClientConfigurationError";
24+
import { ResponseHandler } from "../response/ResponseHandler";
2425

2526
export class SilentFlowClient extends BaseClient {
2627

@@ -77,22 +78,13 @@ export class SilentFlowClient extends BaseClient {
7778

7879
const cachedIdToken = this.readIdTokenFromCache(homeAccountId, environment, cachedAccount.realm);
7980
const idTokenObj = new IdToken(cachedIdToken.secret, this.config.cryptoInterface);
80-
const cachedScopes = ScopeSet.fromString(cachedAccessToken.target);
81-
82-
// generate Authentication Result
83-
return {
84-
uniqueId: idTokenObj.claims.oid || idTokenObj.claims.sub,
85-
tenantId: idTokenObj.claims.tid,
86-
scopes: cachedScopes.asArray(),
87-
account: cachedAccount.getAccountInfo(),
88-
idToken: cachedIdToken.secret,
89-
idTokenClaims: idTokenObj.claims,
90-
accessToken: cachedAccessToken.secret,
91-
fromCache: true,
92-
expiresOn: new Date(cachedAccessToken.expiresOn),
93-
extExpiresOn: new Date(cachedAccessToken.extendedExpiresOn),
94-
familyId: null,
95-
};
81+
82+
return ResponseHandler.generateAuthenticationResult({
83+
account: cachedAccount,
84+
accessToken: cachedAccessToken,
85+
idToken: cachedIdToken,
86+
refreshToken: cachedRefreshToken
87+
}, idTokenObj, true);
9688
}
9789

9890
/**

‎lib/msal-common/src/response/ResponseHandler.ts

Lines changed: 51 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ export class ResponseHandler {
9898
* @param serverTokenResponse
9999
* @param authority
100100
*/
101-
generateAuthenticationResult(serverTokenResponse: ServerAuthorizationTokenResponse, authority: Authority, cachedNonce?: string, cachedState?: string): AuthenticationResult {
101+
handleServerTokenResponse(serverTokenResponse: ServerAuthorizationTokenResponse, authority: Authority, cachedNonce?: string, cachedState?: string): AuthenticationResult {
102102
// create an idToken object (not entity)
103103
const idTokenObj = new IdToken(serverTokenResponse.id_token, this.cryptoObj);
104104

@@ -116,43 +116,9 @@ export class ResponseHandler {
116116
}
117117

118118
const cacheRecord = this.generateCacheRecord(serverTokenResponse, idTokenObj, authority, requestStateObj && requestStateObj.libraryState);
119-
const responseScopes = ScopeSet.fromString(serverTokenResponse.scope);
120-
this.cacheStorage.saveCacheRecord(cacheRecord, responseScopes);
121-
122-
const authenticationResult: AuthenticationResult = {
123-
uniqueId: idTokenObj.claims.oid || idTokenObj.claims.sub,
124-
tenantId: idTokenObj.claims.tid,
125-
scopes: responseScopes.asArray(),
126-
account: cacheRecord.account.getAccountInfo(),
127-
idToken: idTokenObj.rawIdToken,
128-
idTokenClaims: idTokenObj.claims,
129-
accessToken: serverTokenResponse.access_token,
130-
fromCache: true,
131-
expiresOn: new Date(cacheRecord.accessToken.expiresOn),
132-
extExpiresOn: new Date(cacheRecord.accessToken.extendedExpiresOn),
133-
familyId: serverTokenResponse.foci || null,
134-
state: requestStateObj ? requestStateObj.userRequestState : ""
135-
};
119+
this.cacheStorage.saveCacheRecord(cacheRecord);
136120

137-
return authenticationResult;
138-
}
139-
140-
/**
141-
* Generate Account
142-
* @param serverTokenResponse
143-
* @param idToken
144-
* @param authority
145-
*/
146-
generateAccountEntity(serverTokenResponse: ServerAuthorizationTokenResponse, idToken: IdToken, authority: Authority): AccountEntity {
147-
const authorityType = authority.authorityType;
148-
149-
if (StringUtils.isEmpty(serverTokenResponse.client_info)) {
150-
throw ClientAuthError.createClientInfoEmptyError(serverTokenResponse.client_info);
151-
}
152-
153-
return (authorityType === AuthorityType.Adfs)?
154-
AccountEntity.createADFSAccount(authority, idToken):
155-
AccountEntity.createAccount(serverTokenResponse.client_info, authority, idToken, this.cryptoObj);
121+
return ResponseHandler.generateAuthenticationResult(cacheRecord, idTokenObj, false, requestStateObj ? requestStateObj.userRequestState : null);
156122
}
157123

158124
/**
@@ -161,7 +127,7 @@ export class ResponseHandler {
161127
* @param idTokenObj
162128
* @param authority
163129
*/
164-
generateCacheRecord(serverTokenResponse: ServerAuthorizationTokenResponse, idTokenObj: IdToken, authority: Authority, libraryState?: LibraryStateObject): CacheRecord {
130+
private generateCacheRecord(serverTokenResponse: ServerAuthorizationTokenResponse, idTokenObj: IdToken, authority: Authority, libraryState?: LibraryStateObject): CacheRecord {
165131
// Account
166132
const cachedAccount = this.generateAccountEntity(
167133
serverTokenResponse,
@@ -202,7 +168,7 @@ export class ResponseHandler {
202168
serverTokenResponse.access_token,
203169
this.clientId,
204170
idTokenObj.claims.tid,
205-
responseScopes.asArray().join(" "),
171+
responseScopes.printScopes(),
206172
tokenExpirationSeconds,
207173
extendedTokenExpirationSeconds
208174
);
@@ -218,4 +184,50 @@ export class ResponseHandler {
218184

219185
return new CacheRecord(cachedAccount, cachedIdToken, cachedAccessToken, cachedRefreshToken);
220186
}
187+
188+
/**
189+
* Generate Account
190+
* @param serverTokenResponse
191+
* @param idToken
192+
* @param authority
193+
*/
194+
private generateAccountEntity(serverTokenResponse: ServerAuthorizationTokenResponse, idToken: IdToken, authority: Authority): AccountEntity {
195+
const authorityType = authority.authorityType;
196+
197+
if (StringUtils.isEmpty(serverTokenResponse.client_info)) {
198+
throw ClientAuthError.createClientInfoEmptyError(serverTokenResponse.client_info);
199+
}
200+
201+
return (authorityType === AuthorityType.Adfs)?
202+
AccountEntity.createADFSAccount(authority, idToken):
203+
AccountEntity.createAccount(serverTokenResponse.client_info, authority, idToken, this.cryptoObj);
204+
}
205+
206+
/**
207+
* Creates an @AuthenticationResult from @CacheRecord , @IdToken , and a boolean that states whether or not the result is from cache.
208+
*
209+
* Optionally takes a state string that is set as-is in the response.
210+
*
211+
* @param cacheRecord
212+
* @param idTokenObj
213+
* @param fromTokenCache
214+
* @param stateString
215+
*/
216+
static generateAuthenticationResult(cacheRecord: CacheRecord, idTokenObj: IdToken, fromTokenCache: boolean, stateString?: string): AuthenticationResult {
217+
const responseScopes = ScopeSet.fromString(cacheRecord.accessToken.target);
218+
return {
219+
uniqueId: idTokenObj.claims.oid || idTokenObj.claims.sub,
220+
tenantId: idTokenObj.claims.tid,
221+
scopes: responseScopes.asArray(),
222+
account: cacheRecord.account.getAccountInfo(),
223+
idToken: idTokenObj.rawIdToken,
224+
idTokenClaims: idTokenObj.claims,
225+
accessToken: cacheRecord.accessToken.secret,
226+
fromCache: fromTokenCache,
227+
expiresOn: new Date(Number(cacheRecord.accessToken.expiresOn) * 1000),
228+
extExpiresOn: new Date(Number(cacheRecord.accessToken.extendedExpiresOn) * 1000),
229+
familyId: cacheRecord.refreshToken.familyId || null,
230+
state: stateString || ""
231+
};
232+
}
221233
}

‎lib/msal-common/src/url/UrlString.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ export class UrlString {
167167
const urlString = new UrlString(url);
168168
const parameters = urlString.getDeserializedHash<ServerAuthorizationCodeResponse>();
169169
return !!(
170+
parameters.code ||
170171
parameters.error_description ||
171172
parameters.error ||
172173
parameters.state

‎lib/msal-common/test/client/AuthorizationCodeClient.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,7 @@ describe("AuthorizationCodeClient unit tests", () => {
339339
const authenticationResult = await client.acquireToken(authCodeRequest, idTokenClaims.nonce, testState);
340340

341341
expect(authenticationResult.accessToken).to.deep.eq(AUTHENTICATION_RESULT.body.access_token);
342+
expect((Date.now() + (AUTHENTICATION_RESULT.body.expires_in * 1000)) >= authenticationResult.expiresOn.getMilliseconds()).to.be.true;
342343
expect(createTokenRequestBodySpy.calledWith(authCodeRequest)).to.be.ok;
343344

344345
expect(createTokenRequestBodySpy.returnValues[0]).to.contain(`${AADServerParamKeys.SCOPE}=${TEST_CONFIG.DEFAULT_GRAPH_SCOPE}%20${Constants.OPENID_SCOPE}%20${Constants.PROFILE_SCOPE}%20${Constants.OFFLINE_ACCESS_SCOPE}`);

‎samples/msal-browser-samples/VanillaJSTestApp2.0/app/ssoSilent/authConfig.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,12 @@ const graphConfig = {
2525
// Add here scopes for access token to be used at MS Graph API endpoints.
2626
const tokenRequest = {
2727
scopes: ["Mail.Read", "openid", "profile"],
28-
loginHint: "[email protected]",
2928
redirectUri: "http://localhost:30662/",
3029
forceRefresh: false // Set this to "true" to skip a cached token and go to the server to get a new token
3130
};
3231

3332
const silentRequest = {
3433
scopes: ["openid", "profile", "User.Read", "Mail.Read"],
3534
redirectUri: "http://localhost:30662/",
36-
loginHint: "[email protected]"
35+
loginHint: "[email protected]"
3736
};

0 commit comments

Comments
 (0)