Skip to content

Commit 260656e

Browse files
Avery-DunnAvery-Dunn
authored
Merge pull request #811 from AzureAD/avdunn/claims-refresh-fix
Refresh tokens when request contains claims
2 parents 07d67b7 + b581622 commit 260656e

File tree

7 files changed

+270
-75
lines changed

7 files changed

+270
-75
lines changed

‎msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/AcquireTokenSilentIT.java‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ void acquireTokenSilent_ConfidentialClient_acquireTokenSilent(String environment
193193
}
194194

195195
@Test
196-
public void acquireTokenSilent_ConfidentialClient_acquireTokenSilentDifferentScopeThrowsException()
196+
void acquireTokenSilent_ConfidentialClient_acquireTokenSilentDifferentScopeThrowsException()
197197
throws Exception {
198198
cfg = new Config(AzureEnvironment.AZURE);
199199

@@ -344,6 +344,48 @@ void acquireTokenSilent_emptyScopeSet(String environment) throws Exception {
344344
assertEquals(result.accessToken(), silentResult.accessToken());
345345
}
346346

347+
@Test
348+
public void acquireTokenSilent_ClaimsForceRefresh() throws Exception {
349+
cfg = new Config(AzureEnvironment.AZURE);
350+
User user = labUserProvider.getDefaultUser(AzureEnvironment.AZURE);
351+
352+
Set<String> scopes = new HashSet<>();
353+
PublicClientApplication pca = PublicClientApplication.builder(
354+
user.getAppId()).
355+
authority(cfg.organizationsAuthority()).
356+
build();
357+
358+
IAuthenticationResult result = pca.acquireToken(UserNamePasswordParameters.
359+
builder(scopes,
360+
user.getUpn(),
361+
user.getPassword().toCharArray())
362+
.build())
363+
.get();
364+
365+
assertResultNotNull(result);
366+
367+
IAuthenticationResult silentResultWithoutClaims = pca.acquireTokenSilently(SilentParameters.
368+
builder(scopes, result.account())
369+
.build())
370+
.get();
371+
372+
assertResultNotNull(silentResultWithoutClaims);
373+
assertEquals(result.accessToken(), silentResultWithoutClaims.accessToken());
374+
375+
//If claims are added to a silent request, it should trigger the refresh flow and return a new token
376+
ClaimsRequest cr = new ClaimsRequest();
377+
cr.requestClaimInAccessToken("email", null);
378+
379+
IAuthenticationResult silentResultWithClaims = pca.acquireTokenSilently(SilentParameters.
380+
builder(scopes, result.account())
381+
.claims(cr)
382+
.build())
383+
.get();
384+
385+
assertResultNotNull(silentResultWithClaims);
386+
assertNotEquals(result.accessToken(), silentResultWithClaims.accessToken());
387+
}
388+
347389
private IConfidentialClientApplication getConfidentialClientApplications() throws Exception {
348390
String clientId = cfg.appProvider.getOboAppId();
349391
String password = cfg.appProvider.getOboAppPassword();

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

Lines changed: 82 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
class AcquireTokenSilentSupplier extends AuthenticationResultSupplier {
1414

1515
private SilentRequest silentRequest;
16+
protected static final int ACCESS_TOKEN_EXPIRE_BUFFER_IN_SEC = 5 * 60;
1617

1718
AcquireTokenSilentSupplier(AbstractApplicationBase clientApplication, SilentRequest silentRequest) {
1819
super(clientApplication, silentRequest);
@@ -22,6 +23,7 @@ class AcquireTokenSilentSupplier extends AuthenticationResultSupplier {
2223

2324
@Override
2425
AuthenticationResult execute() throws Exception {
26+
boolean shouldRefresh;
2527
Authority requestAuthority = silentRequest.requestAuthority();
2628
if (requestAuthority.authorityType != AuthorityType.B2C) {
2729
requestAuthority =
@@ -53,29 +55,9 @@ AuthenticationResult execute() throws Exception {
5355
clientApplication.serviceBundle().getServerSideTelemetry().incrementSilentSuccessfulCount();
5456
}
5557

56-
//Determine if the current token needs to be refreshed according to the refresh_in value
57-
long currTimeStampSec = new Date().getTime() / 1000;
58-
boolean afterRefreshOn = res.refreshOn() != null && res.refreshOn() > 0 &&
59-
res.refreshOn() < currTimeStampSec && res.expiresOn() >= currTimeStampSec;
60-
61-
if (silentRequest.parameters().forceRefresh() || afterRefreshOn || StringHelper.isBlank(res.accessToken())) {
62-
63-
//As of version 3 of the telemetry schema, there is a field for collecting data about why a token was refreshed,
64-
// so here we set the telemetry value based on the cause of the refresh
65-
if (silentRequest.parameters().forceRefresh()) {
66-
clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo(
67-
CacheTelemetry.REFRESH_FORCE_REFRESH.telemetryValue);
68-
} else if (afterRefreshOn) {
69-
clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo(
70-
CacheTelemetry.REFRESH_REFRESH_IN.telemetryValue);
71-
} else if (res.expiresOn() < currTimeStampSec) {
72-
clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo(
73-
CacheTelemetry.REFRESH_ACCESS_TOKEN_EXPIRED.telemetryValue);
74-
} else if (StringHelper.isBlank(res.accessToken())) {
75-
clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo(
76-
CacheTelemetry.REFRESH_NO_ACCESS_TOKEN.telemetryValue);
77-
}
58+
shouldRefresh = shouldRefresh(silentRequest.parameters(), res);
7859

60+
if (shouldRefresh || clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo() == CacheTelemetry.REFRESH_REFRESH_IN.telemetryValue) {
7961
if (!StringHelper.isBlank(res.refreshToken())) {
8062
//There are certain scenarios where the cached authority may differ from the client app's authority,
8163
// such as when a request is instance aware. Unless overridden by SilentParameters.authorityUrl, the
@@ -84,29 +66,7 @@ AuthenticationResult execute() throws Exception {
8466
requestAuthority = Authority.createAuthority(new URL(requestAuthority.authority().replace(requestAuthority.host(),
8567
res.account().environment())));
8668
}
87-
88-
RefreshTokenRequest refreshTokenRequest = new RefreshTokenRequest(
89-
RefreshTokenParameters.builder(silentRequest.parameters().scopes(), res.refreshToken()).build(),
90-
silentRequest.application(),
91-
silentRequest.requestContext(),
92-
silentRequest);
93-
94-
AcquireTokenByAuthorizationGrantSupplier acquireTokenByAuthorisationGrantSupplier =
95-
new AcquireTokenByAuthorizationGrantSupplier(clientApplication, refreshTokenRequest, requestAuthority);
96-
97-
try {
98-
res = acquireTokenByAuthorisationGrantSupplier.execute();
99-
100-
res.metadata().tokenSource(TokenSource.IDENTITY_PROVIDER);
101-
102-
log.info("Access token refreshed successfully.");
103-
} catch (MsalServiceException ex) {
104-
//If the token refresh attempt threw a MsalServiceException but the refresh attempt was done
105-
// only because of refreshOn, then simply return the existing cached token
106-
if (afterRefreshOn && !(silentRequest.parameters().forceRefresh() || StringHelper.isBlank(res.accessToken()))) {
107-
return res;
108-
} else throw ex;
109-
}
69+
res = makeRefreshRequest(res, requestAuthority);
11070
} else {
11171
res = null;
11272
}
@@ -120,4 +80,81 @@ AuthenticationResult execute() throws Exception {
12080

12181
return res;
12282
}
83+
84+
private AuthenticationResult makeRefreshRequest(AuthenticationResult cachedResult, Authority requestAuthority) throws Exception {
85+
RefreshTokenRequest refreshTokenRequest = new RefreshTokenRequest(
86+
RefreshTokenParameters.builder(silentRequest.parameters().scopes(), cachedResult.refreshToken()).build(),
87+
silentRequest.application(),
88+
silentRequest.requestContext(),
89+
silentRequest);
90+
91+
AcquireTokenByAuthorizationGrantSupplier acquireTokenByAuthorisationGrantSupplier =
92+
new AcquireTokenByAuthorizationGrantSupplier(clientApplication, refreshTokenRequest, requestAuthority);
93+
94+
try {
95+
AuthenticationResult refreshedResult = acquireTokenByAuthorisationGrantSupplier.execute();
96+
97+
refreshedResult.metadata().tokenSource(TokenSource.IDENTITY_PROVIDER);
98+
99+
log.info("Access token refreshed successfully.");
100+
return refreshedResult;
101+
} catch (MsalServiceException ex) {
102+
//If the token refresh attempt threw a MsalServiceException but the refresh attempt was done
103+
// only because of refreshOn, then simply return the existing cached token rather than throw an exception
104+
if (clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo() == CacheTelemetry.REFRESH_REFRESH_IN.telemetryValue) {
105+
return cachedResult;
106+
}
107+
throw ex;
108+
}
109+
}
110+
111+
//Handles any logic to determine if a token should be refreshed, based on the request parameters and the status of cached tokens
112+
private boolean shouldRefresh(SilentParameters parameters, AuthenticationResult cachedResult) {
113+
114+
//If forceRefresh is true, no reason to check any other option
115+
if (parameters.forceRefresh()) {
116+
setCacheTelemetry(CacheTelemetry.REFRESH_FORCE_REFRESH.telemetryValue);
117+
log.debug("Refreshing access token because forceRefresh parameter is true.");
118+
return true;
119+
}
120+
121+
//If the request contains claims then the token should be refreshed, to ensure that the returned token has the correct claims
122+
// Note: these are the types of claims found in (for example) a claims challenge, and do not include client capabilities
123+
if (parameters.claims() != null) {
124+
setCacheTelemetry(CacheTelemetry.REFRESH_FORCE_REFRESH.telemetryValue);
125+
log.debug("Refreshing access token because the claims parameter is not null.");
126+
return true;
127+
}
128+
129+
long currTimeStampSec = new Date().getTime() / 1000;
130+
131+
//If the access token is expired or within 5 minutes of becoming expired, refresh it
132+
if (!StringHelper.isBlank(cachedResult.accessToken()) && cachedResult.expiresOn() < (currTimeStampSec - ACCESS_TOKEN_EXPIRE_BUFFER_IN_SEC)) {
133+
setCacheTelemetry(CacheTelemetry.REFRESH_ACCESS_TOKEN_EXPIRED.telemetryValue);
134+
log.debug("Refreshing access token because it is expired.");
135+
return true;
136+
}
137+
138+
//Certain long-lived tokens will have a 'refresh on' time that indicates a refresh should be attempted long before the token would expire
139+
if (!StringHelper.isBlank(cachedResult.accessToken()) &&
140+
cachedResult.refreshOn() != null && cachedResult.refreshOn() > 0 &&
141+
cachedResult.refreshOn() < currTimeStampSec && cachedResult.expiresOn() >= (currTimeStampSec + ACCESS_TOKEN_EXPIRE_BUFFER_IN_SEC)){
142+
setCacheTelemetry(CacheTelemetry.REFRESH_REFRESH_IN.telemetryValue);
143+
log.debug("Attempting to refresh access token because it is after the refreshOn time.");
144+
return true;
145+
}
146+
147+
//If there is a refresh token but no access token, we should use the refresh token to get the access token
148+
if (StringHelper.isBlank(cachedResult.accessToken()) && !StringHelper.isBlank(cachedResult.refreshToken())) {
149+
setCacheTelemetry(CacheTelemetry.REFRESH_NO_ACCESS_TOKEN.telemetryValue);
150+
log.debug("Refreshing access token because it was missing from the cache.");
151+
return true;
152+
}
153+
154+
return false;
155+
}
156+
157+
private void setCacheTelemetry(int cacheInfoValue){
158+
clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo(cacheInfoValue);
159+
}
123160
}

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -474,13 +474,11 @@ private Optional<AccessTokenCacheEntity> getAccessTokenCacheEntity(
474474
Set<String> scopes,
475475
String clientId,
476476
Set<String> environmentAliases) {
477-
long currTimeStampSec = new Date().getTime() / 1000;
478477

479478
return accessTokens.values().stream().filter(
480479
accessToken ->
481480
accessToken.homeAccountId.equals(account.homeAccountId()) &&
482481
environmentAliases.contains(accessToken.environment) &&
483-
Long.parseLong(accessToken.expiresOn()) > currTimeStampSec + MIN_ACCESS_TOKEN_EXPIRE_IN_SEC &&
484482
accessToken.realm.equals(authority.tenant()) &&
485483
accessToken.clientId.equals(clientId) &&
486484
isMatchingScopes(accessToken, scopes)

‎msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/AcquireTokenSilentlyTest.java‎

Lines changed: 78 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,17 @@
55

66
import org.junit.jupiter.api.Test;
77
import org.junit.jupiter.api.TestInstance;
8+
89
import static org.junit.jupiter.api.Assertions.assertTrue;
910
import static org.junit.jupiter.api.Assertions.assertThrows;
10-
11+
import static org.junit.jupiter.api.Assertions.assertNotNull;
12+
import static org.junit.jupiter.api.Assertions.assertEquals;
13+
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
14+
15+
import java.io.IOException;
16+
import java.net.URISyntaxException;
17+
import java.nio.file.Files;
18+
import java.nio.file.Paths;
1119
import java.util.Collections;
1220
import java.util.concurrent.CompletableFuture;
1321
import java.util.concurrent.ExecutionException;
@@ -16,6 +24,9 @@
1624
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
1725
class AcquireTokenSilentlyTest {
1826

27+
Account basicAccount = new Account("home_account_id", "login.windows.net", "username", null);
28+
String cache = readResource("/AAD_cache_data/full_cache.json");
29+
1930
@Test
2031
void publicAppAcquireTokenSilently_emptyCache_MsalClientException() throws Throwable {
2132

@@ -29,7 +40,7 @@ void publicAppAcquireTokenSilently_emptyCache_MsalClientException() throws Throw
2940

3041
ExecutionException ex = assertThrows(ExecutionException.class, future::get);
3142

32-
assertTrue(ex.getCause() instanceof MsalClientException);
43+
assertInstanceOf(MsalClientException.class, ex.getCause());
3344
assertTrue(ex.getMessage().contains(AuthenticationErrorMessage.NO_TOKEN_IN_CACHE));
3445
}
3546

@@ -45,7 +56,71 @@ void confidentialAppAcquireTokenSilently_emptyCache_MsalClientException() throws
4556

4657
ExecutionException ex = assertThrows(ExecutionException.class, future::get);
4758

48-
assertTrue(ex.getCause() instanceof MsalClientException);
59+
assertInstanceOf(MsalClientException.class, ex.getCause());
4960
assertTrue(ex.getMessage().contains(AuthenticationErrorMessage.NO_TOKEN_IN_CACHE));
5061
}
62+
63+
@Test
64+
void publicAppAcquireTokenSilently_claimsSkipCache() throws Throwable {
65+
66+
PublicClientApplication application = PublicClientApplication.builder("client_id")
67+
.instanceDiscovery(false)
68+
.authority("https://some.authority.com/realm")
69+
.build();
70+
71+
application.tokenCache.deserialize(cache);
72+
73+
SilentParameters parameters = SilentParameters.builder(Collections.singleton("scopes"), basicAccount).build();
74+
75+
IAuthenticationResult result = application.acquireTokenSilently(parameters).get();
76+
77+
//Confirm cached dummy token returned from silent request
78+
assertNotNull(result);
79+
assertEquals("token", result.accessToken());
80+
81+
ClaimsRequest cr = new ClaimsRequest();
82+
cr.requestClaimInAccessToken("something", null);
83+
84+
parameters = SilentParameters.builder(Collections.singleton("scopes"), basicAccount).claims(cr).build();
85+
CompletableFuture<IAuthenticationResult> future = application.acquireTokenSilently(parameters);
86+
87+
//Confirm cached dummy token ignored when claims are part of request
88+
ExecutionException ex = assertThrows(ExecutionException.class, future::get);
89+
assertInstanceOf(MsalInteractionRequiredException.class, ex.getCause());
90+
}
91+
92+
@Test
93+
void confidentialAppAcquireTokenSilently_claimsSkipCache() throws Throwable {
94+
95+
ConfidentialClientApplication application = ConfidentialClientApplication
96+
.builder("client_id", ClientCredentialFactory.createFromSecret(TestConfiguration.AAD_CLIENT_DUMMYSECRET))
97+
.instanceDiscovery(false)
98+
.authority("https://some.authority.com/realm").build();
99+
100+
application.tokenCache.deserialize(cache);
101+
102+
SilentParameters parameters = SilentParameters.builder(Collections.singleton("scopes"), basicAccount).build();
103+
104+
IAuthenticationResult result = application.acquireTokenSilently(parameters).get();
105+
106+
assertNotNull(result);
107+
assertEquals("token", result.accessToken());
108+
109+
ClaimsRequest cr = new ClaimsRequest();
110+
cr.requestClaimInAccessToken("something", null);
111+
112+
parameters = SilentParameters.builder(Collections.singleton("scopes"), basicAccount).claims(cr).build();
113+
CompletableFuture<IAuthenticationResult> future = application.acquireTokenSilently(parameters);
114+
115+
ExecutionException ex = assertThrows(ExecutionException.class, future::get);
116+
assertInstanceOf(MsalInteractionRequiredException.class, ex.getCause());
117+
}
118+
119+
String readResource(String resource) {
120+
try {
121+
return new String(Files.readAllBytes(Paths.get(getClass().getResource(resource).toURI())));
122+
} catch (IOException | URISyntaxException e) {
123+
throw new RuntimeException(e);
124+
}
125+
}
51126
}

0 commit comments

Comments
 (0)