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

Extend acquireTokenSilent Instrumentation #1629

Merged
merged 14 commits into from
May 19, 2020
Merged
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 33 additions & 8 deletions lib/msal-core/src/UserAgentApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,7 @@ export class UserAgentApplication {
*
*/
acquireTokenSilent(userRequest: AuthenticationParameters): Promise<AuthResponse> {
this.logger.info("AcquireTokenSilent has been called");
jo-arroyo marked this conversation as resolved.
Show resolved Hide resolved
// validate the request
const request = RequestUtils.validateRequest(userRequest, false, this.clientId, Constants.interactionTypeSilent);
const apiEvent: ApiEvent = this.telemetryManager.createAndStartApiEvent(request.correlationId, API_EVENT_IDENTIFIER.AcquireTokenSilent);
Expand All @@ -641,21 +642,34 @@ export class UserAgentApplication {
WindowUtils.blockReloadInHiddenIframes();

const scope = request.scopes.join(" ").toLowerCase();
this.logger.verbosePii(`Serialized scopes: ${scope}`);

// if the developer passes an account, give that account the priority
const account: Account = request.account || this.getAccount();
let account: Account;
if (request.account) {
account = request.account;
this.logger.verbose("Account set from request");
} else {
account = this.getAccount();
this.logger.verbose("Account set from MSAL configuration");
jo-arroyo marked this conversation as resolved.
Show resolved Hide resolved
}

// extract if there is an adalIdToken stashed in the cache
// Extract adalIdToken if stashed in the cache to allow for seamless ADAL to MSAL migration
const adalIdToken = this.cacheStorage.getItem(Constants.adalIdToken);

// if there is no account logged in and no login_hint/sid is passed in the request
/**
* if there is no account logged in and no login_hint/sid is passed in the request
jo-arroyo marked this conversation as resolved.
Show resolved Hide resolved
* In the event of no account being passed in the config, no session id, and no pre-existing adalIdToken, user will need to log in
*/
if (!account && !(request.sid || request.loginHint) && StringUtils.isEmpty(adalIdToken) ) {
this.logger.info("User login is required");
// The promise rejects with a UserLoginRequiredError, which should be caught and user should be prompted to log in interactively
return reject(ClientAuthError.createUserLoginRequiredError());
}

// set the response type based on the current cache status / scopes set
const responseType = this.getTokenType(account, request.scopes, true);
jo-arroyo marked this conversation as resolved.
Show resolved Hide resolved
this.logger.verbose(`Response type: ${responseType}`);

// create a serverAuthenticationRequest populating the `queryParameters` to be sent to the Server
const serverAuthenticationRequest = new ServerRequestParameters(
Expand All @@ -668,6 +682,8 @@ export class UserAgentApplication {
request.correlationId,
);

this.logger.verbose("Finished building server authentication request");

// populate QueryParameters (sid/login_hint) and any other extraQueryParameters set by the developer
if (ServerRequestParameters.isSSOParam(request) || account) {
serverAuthenticationRequest.populateQueryParams(account, request, null, true);
jo-arroyo marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -680,11 +696,14 @@ export class UserAgentApplication {
serverAuthenticationRequest.populateQueryParams(account, null, adalIdTokenObject, true);
}

this.logger.verbose("Query parameters populated");
jo-arroyo marked this conversation as resolved.
Show resolved Hide resolved

const userContainedClaims = request.claimsRequest || serverAuthenticationRequest.claimsValue;

let authErr: AuthError;
let cacheResultResponse;

// If request.forceRefresh is set to true, force a request for a new token instead of getting it from the cache
if (!userContainedClaims && !request.forceRefresh) {
try {
cacheResultResponse = this.getCachedToken(serverAuthenticationRequest, account);
Expand All @@ -695,7 +714,7 @@ export class UserAgentApplication {

// resolve/reject based on cacheResult
if (cacheResultResponse) {
this.logger.info("Token is already in cache for scope:" + scope);
this.logger.info("Token is already in cache for scope: " + scope);
resolve(cacheResultResponse);
return null;
}
Expand All @@ -708,17 +727,20 @@ export class UserAgentApplication {
else {
let logMessage;
if (userContainedClaims) {
logMessage = "Skipped cache lookup since claims were given.";
logMessage = "Skipped cache lookup since claims were given";
} else if (request.forceRefresh) {
logMessage = "Skipped cache lookup since request.forceRefresh option was set to true";
} else {
logMessage = "Token is not in cache for scope:" + scope;
logMessage = "Token is not in cache for scope: " + scope;
jo-arroyo marked this conversation as resolved.
Show resolved Hide resolved
}
this.logger.verbose(logMessage);

// Cache result can return null if cache is empty. In that case, set authority to default value if no authority is passed to the api.
// Cache result can return null if cache is empty. In that case, set authority to default value if no authority is passed to the API.
if (!serverAuthenticationRequest.authorityInstance) {
serverAuthenticationRequest.authorityInstance = request.authority ? AuthorityFactory.CreateInstance(request.authority, this.config.auth.validateAuthority) : this.authorityInstance;
if (!request.authority){
this.logger.verbose("Using default authority since no authority was passed to API");
jo-arroyo marked this conversation as resolved.
Show resolved Hide resolved
}
}
// cache miss

Expand All @@ -729,6 +751,8 @@ export class UserAgentApplication {
* refresh attempt with iframe
* Already renewing for this scope, callback when we get the token.
*/
this.logger.verbose("The authority has been updated with endpoint discovery response");

if (window.activeRenewals[requestSignature]) {
this.logger.verbose("Renew token for scope and authority: " + requestSignature + " is in progress. Registering callback");
// Active renewals contains the state for each renewal.
Expand All @@ -745,7 +769,7 @@ export class UserAgentApplication {
this.renewIdToken(requestSignature, resolve, reject, account, serverAuthenticationRequest);
} else {
// renew access token
this.logger.verbose("renewing accesstoken");
this.logger.verbose("renewing access token");
jo-arroyo marked this conversation as resolved.
Show resolved Hide resolved
this.renewToken(requestSignature, resolve, reject, account, serverAuthenticationRequest);
}
}
Expand All @@ -757,6 +781,7 @@ export class UserAgentApplication {
}
})
.then(res => {
this.logger.verbose("Successfully acquired token");
this.telemetryManager.stopAndFlushApiEvent(request.correlationId, apiEvent, true);
return res;
})
Expand Down