Skip to content

Commit ad1a86c

Browse files
authored
Merge pull request #1629 from AzureAD/ats-instrumentation
Extend acquireTokenSilent Instrumentation
2 parents c8c5ca2 + 76381f7 commit ad1a86c

File tree

2 files changed

+39
-15
lines changed

2 files changed

+39
-15
lines changed

lib/msal-core/src/UserAgentApplication.ts

+35-11
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,8 @@ export class UserAgentApplication {
630630
*
631631
*/
632632
acquireTokenSilent(userRequest: AuthenticationParameters): Promise<AuthResponse> {
633+
this.logger.verbose("AcquireTokenSilent has been called");
634+
633635
// validate the request
634636
const request = RequestUtils.validateRequest(userRequest, false, this.clientId, Constants.interactionTypeSilent);
635637
const apiEvent: ApiEvent = this.telemetryManager.createAndStartApiEvent(request.correlationId, API_EVENT_IDENTIFIER.AcquireTokenSilent);
@@ -641,21 +643,31 @@ export class UserAgentApplication {
641643
WindowUtils.blockReloadInHiddenIframes();
642644

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

645648
// if the developer passes an account, give that account the priority
646-
const account: Account = request.account || this.getAccount();
649+
let account: Account;
650+
if (request.account) {
651+
account = request.account;
652+
this.logger.verbose("Account set from request");
653+
} else {
654+
account = this.getAccount();
655+
this.logger.verbose("Account set from MSAL Cache");
656+
}
647657

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

651-
// if there is no account logged in and no login_hint/sid is passed in the request
661+
// 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
652662
if (!account && !(request.sid || request.loginHint) && StringUtils.isEmpty(adalIdToken) ) {
653663
this.logger.info("User login is required");
664+
// The promise rejects with a UserLoginRequiredError, which should be caught and user should be prompted to log in interactively
654665
return reject(ClientAuthError.createUserLoginRequiredError());
655666
}
656667

657668
// set the response type based on the current cache status / scopes set
658669
const responseType = this.getTokenType(account, request.scopes, true);
670+
this.logger.verbose(`Response type: ${responseType}`);
659671

660672
// create a serverAuthenticationRequest populating the `queryParameters` to be sent to the Server
661673
const serverAuthenticationRequest = new ServerRequestParameters(
@@ -668,23 +680,30 @@ export class UserAgentApplication {
668680
request.correlationId,
669681
);
670682

683+
this.logger.verbose("Finished building server authentication request");
684+
671685
// populate QueryParameters (sid/login_hint) and any other extraQueryParameters set by the developer
672686
if (ServerRequestParameters.isSSOParam(request) || account) {
673687
serverAuthenticationRequest.populateQueryParams(account, request, null, true);
688+
this.logger.verbose("Query parameters populated from existing SSO or account");
674689
}
675690
// if user didn't pass login_hint/sid and adal's idtoken is present, extract the login_hint from the adalIdToken
676691
else if (!account && !StringUtils.isEmpty(adalIdToken)) {
677692
// if adalIdToken exists, extract the SSO info from the same
678693
const adalIdTokenObject = TokenUtils.extractIdToken(adalIdToken);
679-
this.logger.verbose("ADAL's idToken exists. Extracting login information from ADAL's idToken ");
694+
this.logger.verbose("ADAL's idToken exists. Extracting login information from ADAL's idToken to populate query parameters");
680695
serverAuthenticationRequest.populateQueryParams(account, null, adalIdTokenObject, true);
681696
}
697+
else {
698+
this.logger.verbose("No additional query parameters added");
699+
}
682700

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

685703
let authErr: AuthError;
686704
let cacheResultResponse;
687705

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

696715
// resolve/reject based on cacheResult
697716
if (cacheResultResponse) {
698-
this.logger.info("Token is already in cache for scope:" + scope);
717+
this.logger.verbose("Token is already in cache for scope: " + scope);
699718
resolve(cacheResultResponse);
700719
return null;
701720
}
@@ -708,18 +727,20 @@ export class UserAgentApplication {
708727
else {
709728
let logMessage;
710729
if (userContainedClaims) {
711-
logMessage = "Skipped cache lookup since claims were given.";
730+
logMessage = "Skipped cache lookup since claims were given";
712731
} else if (request.forceRefresh) {
713732
logMessage = "Skipped cache lookup since request.forceRefresh option was set to true";
714733
} else {
715-
logMessage = "Token is not in cache for scope:" + scope;
734+
logMessage = "Token is not in cache for scope: " + scope;
716735
}
717736
this.logger.verbose(logMessage);
718737

719-
// 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.
738+
// 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.
720739
if (!serverAuthenticationRequest.authorityInstance) {
721740
serverAuthenticationRequest.authorityInstance = request.authority ? AuthorityFactory.CreateInstance(request.authority, this.config.auth.validateAuthority) : this.authorityInstance;
722741
}
742+
this.logger.verbosePii(`Authority instance: ${serverAuthenticationRequest.authority}`);
743+
723744
// cache miss
724745

725746
// start http event
@@ -729,6 +750,8 @@ export class UserAgentApplication {
729750
* refresh attempt with iframe
730751
* Already renewing for this scope, callback when we get the token.
731752
*/
753+
this.logger.verbose("Authority has been updated with endpoint discovery response");
754+
732755
if (window.activeRenewals[requestSignature]) {
733756
this.logger.verbose("Renew token for scope and authority: " + requestSignature + " is in progress. Registering callback");
734757
// Active renewals contains the state for each renewal.
@@ -740,23 +763,24 @@ export class UserAgentApplication {
740763
* App uses idToken to send to api endpoints
741764
* Default scope is tracked as clientId to store this token
742765
*/
743-
this.logger.verbose("renewing idToken");
766+
this.logger.verbose("Renewing idToken");
744767
this.silentLogin = true;
745768
this.renewIdToken(requestSignature, resolve, reject, account, serverAuthenticationRequest);
746769
} else {
747770
// renew access token
748-
this.logger.verbose("renewing accesstoken");
771+
this.logger.verbose("Renewing access token");
749772
this.renewToken(requestSignature, resolve, reject, account, serverAuthenticationRequest);
750773
}
751774
}
752775
}).catch((err) => {
753-
this.logger.error(err);
776+
this.logger.warning("Could not resolve endpoints");
754777
reject(ClientAuthError.createEndpointResolutionError(err.toString()));
755778
return null;
756779
});
757780
}
758781
})
759782
.then(res => {
783+
this.logger.verbose("Successfully acquired token");
760784
this.telemetryManager.stopAndFlushApiEvent(request.correlationId, apiEvent, true);
761785
return res;
762786
})

lib/msal-core/src/telemetry/TelemetryUtils.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,11 @@ export const hashPersonalIdentifier = (valueToHash: string) => {
4242
export const prependEventNamePrefix = (suffix: string): string => `${EVENT_NAME_PREFIX}${suffix || ""}`;
4343

4444
export const supportsBrowserPerformance = (): boolean => !!(
45-
typeof window !== "undefined" &&
45+
typeof window !== "undefined" &&
4646
"performance" in window &&
4747
window.performance.mark &&
4848
window.performance.measure
49-
);
49+
);
5050

5151
export const endBrowserPerformanceMeasurement = (measureName: string, startMark: string, endMark: string) => {
5252
if (supportsBrowserPerformance()) {
@@ -57,10 +57,10 @@ export const endBrowserPerformanceMeasurement = (measureName: string, startMark:
5757
window.performance.clearMarks(startMark);
5858
window.performance.clearMarks(endMark);
5959
}
60-
}
60+
};
6161

6262
export const startBrowserPerformanceMeasurement = (startMark: string) => {
6363
if (supportsBrowserPerformance()) {
6464
window.performance.mark(startMark);
6565
}
66-
}
66+
};

0 commit comments

Comments
 (0)