Skip to content

Commit 42eee6a

Browse files
neha-bhargavaneha-bhargava
committed
Address comments
1 parent 37a6a29 commit 42eee6a

File tree

2 files changed

+61
-66
lines changed

2 files changed

+61
-66
lines changed

‎msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByManagedIdentitySupplier.java‎

Lines changed: 55 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ class AcquireTokenByManagedIdentitySupplier extends AuthenticationResultSupplier
1313

1414
private static final Logger LOG = LoggerFactory.getLogger(AcquireTokenByManagedIdentitySupplier.class);
1515

16-
private static final int TWO_HOURS = 2*3600;
16+
private static final int TWO_HOURS = 2 * 3600;
1717

1818
private ManagedIdentityParameters managedIdentityParameters;
1919

@@ -39,60 +39,61 @@ AuthenticationResult execute() throws Exception {
3939

4040
CacheRefreshReason cacheRefreshReason = CacheRefreshReason.NOT_APPLICABLE;
4141

42-
if (!managedIdentityParameters.forceRefresh) {
43-
LOG.debug("ForceRefresh set to false. Attempting cache lookup");
44-
45-
try {
46-
Set<String> scopes = new HashSet<>();
47-
scopes.add(this.managedIdentityParameters.resource);
48-
SilentParameters parameters = SilentParameters
49-
.builder(scopes)
50-
.tenant(managedIdentityParameters.tenant())
51-
.build();
52-
53-
RequestContext context = new RequestContext(
54-
this.clientApplication,
55-
PublicApi.ACQUIRE_TOKEN_SILENTLY,
56-
parameters);
57-
58-
SilentRequest silentRequest = new SilentRequest(
59-
parameters,
60-
this.clientApplication,
61-
context,
62-
null);
63-
64-
AcquireTokenSilentSupplier supplier = new AcquireTokenSilentSupplier(
65-
this.clientApplication,
66-
silentRequest);
67-
68-
AuthenticationResult result = supplier.execute();
69-
cacheRefreshReason = SilentRequestHelper.NeedsRefresh(
70-
parameters,
71-
result,
72-
LOG);
73-
74-
// If the token does not need a refresh, return the cached token
75-
// Else refresh the token if it is either expired, proactively refreshable, or if the claims are passed.
76-
if (cacheRefreshReason == CacheRefreshReason.NOT_APPLICABLE) {
77-
LOG.debug("Returning token from cache");
78-
result.metadata().tokenSource(TokenSource.CACHE);
79-
return result;
80-
} else {
81-
LOG.debug(String.format("Refreshing access token. Cache refresh reason: %s", cacheRefreshReason));
82-
}
83-
} catch (MsalClientException ex) {
84-
if (ex.errorCode().equals(AuthenticationErrorCode.CACHE_MISS)) {
85-
LOG.debug(String.format("Cache lookup failed: %s", ex.getMessage()));
86-
return fetchNewAccessTokenAndSaveToCache(tokenRequestExecutor, cacheRefreshReason);
87-
} else {
88-
LOG.error(String.format("Error occurred while cache lookup: %s", ex.getMessage()));
89-
throw ex;
90-
}
91-
}
42+
if (managedIdentityParameters.forceRefresh) {
43+
LOG.debug("ForceRefresh set to true. Skipping cache lookup and attempting to acquire new token");
44+
return fetchNewAccessTokenAndSaveToCache(tokenRequestExecutor, CacheRefreshReason.FORCE_REFRESH);
9245
}
9346

94-
LOG.info("Skipped looking for an Access Token in the cache because forceRefresh was set. Or the token in the cache needs refresh");
95-
return fetchNewAccessTokenAndSaveToCache(tokenRequestExecutor, cacheRefreshReason);
47+
48+
LOG.debug("ForceRefresh set to false. Attempting cache lookup");
49+
try {
50+
Set<String> scopes = new HashSet<>();
51+
scopes.add(this.managedIdentityParameters.resource);
52+
SilentParameters parameters = SilentParameters
53+
.builder(scopes)
54+
.tenant(managedIdentityParameters.tenant())
55+
.build();
56+
57+
RequestContext context = new RequestContext(
58+
this.clientApplication,
59+
PublicApi.ACQUIRE_TOKEN_SILENTLY,
60+
parameters);
61+
62+
SilentRequest silentRequest = new SilentRequest(
63+
parameters,
64+
this.clientApplication,
65+
context,
66+
null);
67+
68+
AcquireTokenSilentSupplier supplier = new AcquireTokenSilentSupplier(
69+
this.clientApplication,
70+
silentRequest);
71+
72+
AuthenticationResult result = supplier.execute();
73+
cacheRefreshReason = SilentRequestHelper.getCacheRefreshReasonIfApplicable(
74+
parameters,
75+
result,
76+
LOG);
77+
78+
// If the token does not need a refresh, return the cached token
79+
// Else refresh the token if it is either expired, proactively refreshable, or if the claims are passed.
80+
if (cacheRefreshReason == CacheRefreshReason.NOT_APPLICABLE) {
81+
LOG.debug("Returning token from cache");
82+
result.metadata().tokenSource(TokenSource.CACHE);
83+
return result;
84+
} else {
85+
LOG.debug(String.format("Refreshing access token. Cache refresh reason: %s", cacheRefreshReason));
86+
return fetchNewAccessTokenAndSaveToCache(tokenRequestExecutor, cacheRefreshReason);
87+
}
88+
} catch (MsalClientException ex) {
89+
if (ex.errorCode().equals(AuthenticationErrorCode.CACHE_MISS)) {
90+
LOG.debug(String.format("Cache lookup failed: %s", ex.getMessage()));
91+
return fetchNewAccessTokenAndSaveToCache(tokenRequestExecutor, cacheRefreshReason);
92+
} else {
93+
LOG.error(String.format("Error occurred while cache lookup: %s", ex.getMessage()));
94+
throw ex;
95+
}
96+
}
9697
}
9798

9899
private AuthenticationResult fetchNewAccessTokenAndSaveToCache(TokenRequestExecutor tokenRequestExecutor, CacheRefreshReason cacheRefreshReason) throws Exception {
@@ -131,7 +132,7 @@ private AuthenticationResult createFromManagedIdentityResponse(ManagedIdentityRe
131132
.build();
132133
}
133134

134-
private long calculateRefreshOn(long expiresOn){
135+
private long calculateRefreshOn(long expiresOn) {
135136
long timestampSeconds = System.currentTimeMillis() / 1000;
136137
long expiresIn = expiresOn - timestampSeconds;
137138

‎msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/SilentRequestHelper.java‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,37 +14,31 @@ private SilentRequestHelper() {
1414
// Utility class
1515
}
1616

17-
static CacheRefreshReason NeedsRefresh(SilentParameters parameters, AuthenticationResult cachedResult, Logger log) {
18-
//If forceRefresh is true, no reason to check any other option
19-
if (parameters.forceRefresh()) {
20-
log.debug(String.format("Refreshing access token. Cache refresh reason: %s", CacheRefreshReason.FORCE_REFRESH));
21-
return CacheRefreshReason.FORCE_REFRESH;
22-
}
23-
24-
//If the request contains claims then the token should be refreshed, to ensure that the returned token has the correct claims
25-
// Note: these are the types of claims found in (for example) a claims challenge, and do not include client capabilities
17+
static CacheRefreshReason getCacheRefreshReasonIfApplicable(SilentParameters parameters, AuthenticationResult cachedResult, Logger log) {
18+
// If the request contains claims then the token should be refreshed, to ensure that the returned token has the correct claims
19+
// Note: these are the types of claims found in (for example) a claims challenge, and do not include client capabilities
2620
if (parameters.claims() != null) {
2721
log.debug(String.format("Refreshing access token. Cache refresh reason: %s", CacheRefreshReason.CLAIMS));
2822
return CacheRefreshReason.CLAIMS;
2923
}
3024

3125
long currTimeStampSec = new Date().getTime() / 1000;
3226

33-
//If the access token is expired or within 5 minutes of becoming expired, refresh it
27+
// If the access token is expired or within 5 minutes of becoming expired, refresh it
3428
if (!StringHelper.isBlank(cachedResult.accessToken()) && cachedResult.expiresOn() < (currTimeStampSec + ACCESS_TOKEN_EXPIRE_BUFFER_IN_SEC)) {
3529
log.debug(String.format("Refreshing access token. Cache refresh reason: %s", CacheRefreshReason.EXPIRED));
3630
return CacheRefreshReason.EXPIRED;
3731
}
3832

39-
//Certain long-lived tokens will have a 'refresh on' time that indicates a refresh should be attempted long before the token would expire
33+
// Certain long-lived tokens will have a 'refresh on' time that indicates a refresh should be attempted long before the token would expire
4034
if (!StringHelper.isBlank(cachedResult.accessToken()) &&
4135
cachedResult.refreshOn() != null && cachedResult.refreshOn() > 0 &&
4236
cachedResult.refreshOn() < currTimeStampSec && cachedResult.expiresOn() >= (currTimeStampSec + ACCESS_TOKEN_EXPIRE_BUFFER_IN_SEC)){
4337
log.debug(String.format("Refreshing access token. Cache refresh reason: %s", CacheRefreshReason.PROACTIVE_REFRESH));
4438
return CacheRefreshReason.PROACTIVE_REFRESH;
4539
}
4640

47-
//If there is a refresh token but no access token, we should use the refresh token to get the access token
41+
// If there is a refresh token but no access token, we should use the refresh token to get the access token
4842
if (StringHelper.isBlank(cachedResult.accessToken()) && !StringHelper.isBlank(cachedResult.refreshToken())) {
4943
log.debug(String.format("Refreshing access token. Cache refresh reason: %s", CacheRefreshReason.NO_CACHED_ACCESS_TOKEN));
5044
return CacheRefreshReason.NO_CACHED_ACCESS_TOKEN;

0 commit comments

Comments
 (0)