Skip to content

Commit

Permalink
chore PR review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@lbalmaceda
lbalmaceda committed Oct 30, 2020
1 parent 7cb620d commit ac08e60
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 21 deletions.
4 changes: 2 additions & 2 deletions auth0/src/main/java/com/auth0/android/authentication/storage/CredentialsManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public void getCredentials(@Nullable String scope, final int minTtl, @NonNull fi
return;
}
if (refreshToken == null) {
callback.onFailure(new CredentialsManagerException("Credentials have expired and no Refresh Token was available to renew them."));
callback.onFailure(new CredentialsManagerException("Credentials need to be renewed but no Refresh Token is available to renew them."));
return;
}

Expand Down Expand Up @@ -175,7 +175,7 @@ private boolean hasScopeChanged(@NonNull String storedScope, @Nullable String re
Arrays.sort(stored);
String[] required = requiredScope.split(" ");
Arrays.sort(required);
return stored != required;
return !Arrays.equals(stored, required);
}

private boolean willExpire(long expiresAt, long minTtl) {
Expand Down
39 changes: 20 additions & 19 deletions auth0/src/test/java/com/auth0/android/authentication/storage/CredentialsManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@
@Config(sdk = 21)
public class CredentialsManagerTest {

private static final long ONE_HOUR_SECONDS = 60 * 60;

@Mock
private AuthenticationAPIClient client;
@Mock
Expand Down Expand Up @@ -98,7 +100,7 @@ public Object answer(InvocationOnMock invocation) {

@Test
public void shouldSaveRefreshableCredentialsInStorage() {
long expirationTime = CredentialsMock.CURRENT_TIME_MS + 123456 * 1000;
long expirationTime = CredentialsMock.ONE_HOUR_AHEAD_MS;
Credentials credentials = new CredentialsMock("idToken", "accessToken", "type", "refreshToken", new Date(expirationTime), "scope");
prepareJwtDecoderMock(new Date(expirationTime));
manager.saveCredentials(credentials);
Expand All @@ -115,7 +117,7 @@ public void shouldSaveRefreshableCredentialsInStorage() {

@Test
public void shouldSaveRefreshableCredentialsUsingAccessTokenExpForCacheExpirationInStorage() {
long accessTokenExpirationTime = CredentialsMock.CURRENT_TIME_MS + 123456 * 1000;
long accessTokenExpirationTime = CredentialsMock.ONE_HOUR_AHEAD_MS;
Credentials credentials = new CredentialsMock(null, "accessToken", "type", "refreshToken", new Date(accessTokenExpirationTime), "scope");
prepareJwtDecoderMock(new Date(accessTokenExpirationTime));
manager.saveCredentials(credentials);
Expand All @@ -132,7 +134,7 @@ public void shouldSaveRefreshableCredentialsUsingAccessTokenExpForCacheExpiratio

@Test
public void shouldSaveRefreshableCredentialsUsingIdTokenExpForCacheExpirationInStorage() {
long accessTokenExpirationTime = CredentialsMock.CURRENT_TIME_MS + 123456 * 1000;
long accessTokenExpirationTime = CredentialsMock.CURRENT_TIME_MS + 5000 * 1000;
long idTokenExpirationTime = CredentialsMock.CURRENT_TIME_MS + 2000 * 1000;
Credentials credentials = new CredentialsMock("idToken", "accessToken", "type", "refreshToken", new Date(accessTokenExpirationTime), "scope");
prepareJwtDecoderMock(new Date(idTokenExpirationTime));
Expand All @@ -150,7 +152,7 @@ public void shouldSaveRefreshableCredentialsUsingIdTokenExpForCacheExpirationInS

@Test
public void shouldSaveNonRefreshableCredentialsInStorage() {
long expirationTime = CredentialsMock.CURRENT_TIME_MS + 123456 * 1000;
long expirationTime = CredentialsMock.ONE_HOUR_AHEAD_MS;
Credentials credentials = new CredentialsMock("idToken", "accessToken", "type", null, new Date(expirationTime), "scope");
prepareJwtDecoderMock(new Date(expirationTime));
manager.saveCredentials(credentials);
Expand Down Expand Up @@ -180,7 +182,6 @@ public void shouldThrowOnSetIfCredentialsDoesNotHaveExpiresAt() {
exception.expectMessage("Credentials must have a valid date of expiration and a valid access_token or id_token value.");

Date date = null;
//noinspection ConstantConditions
Credentials credentials = new CredentialsMock("idToken", "accessToken", "type", "refreshToken", date, "scope");
manager.saveCredentials(credentials);
}
Expand All @@ -206,7 +207,7 @@ public void shouldFailOnGetCredentialsWhenNoAccessTokenOrIdTokenWasSaved() {
when(storage.retrieveString("com.auth0.access_token")).thenReturn(null);
when(storage.retrieveString("com.auth0.refresh_token")).thenReturn("refreshToken");
when(storage.retrieveString("com.auth0.token_type")).thenReturn("type");
long expirationTime = CredentialsMock.CURRENT_TIME_MS + 123456L * 1000;
long expirationTime = CredentialsMock.ONE_HOUR_AHEAD_MS;
when(storage.retrieveLong("com.auth0.expires_at")).thenReturn(expirationTime);
when(storage.retrieveLong("com.auth0.cache_expires_at")).thenReturn(expirationTime);
when(storage.retrieveString("com.auth0.scope")).thenReturn("scope");
Expand Down Expand Up @@ -236,7 +237,7 @@ public void shouldFailOnGetCredentialsWhenExpiredAndNoRefreshTokenWasSaved() {
verify(callback).onFailure(exceptionCaptor.capture());
CredentialsManagerException exception = exceptionCaptor.getValue();
assertThat(exception, is(notNullValue()));
assertThat(exception.getMessage(), is("Credentials have expired and no Refresh Token was available to renew them."));
assertThat(exception.getMessage(), is("Credentials need to be renewed but no Refresh Token is available to renew them."));
}

@Test
Expand All @@ -246,7 +247,7 @@ public void shouldNotFailOnGetCredentialsWhenCacheExpiresAtNotSetButExpiresAtIsP
when(storage.retrieveString("com.auth0.id_token")).thenReturn("idToken");
when(storage.retrieveString("com.auth0.access_token")).thenReturn("accessToken");
when(storage.retrieveString("com.auth0.token_type")).thenReturn("type");
long expirationTime = CredentialsMock.CURRENT_TIME_MS + 123456L * 1000;
long expirationTime = CredentialsMock.ONE_HOUR_AHEAD_MS;
when(storage.retrieveLong("com.auth0.expires_at")).thenReturn(expirationTime);
when(storage.retrieveLong("com.auth0.cache_expires_at")).thenReturn(null);
when(storage.retrieveString("com.auth0.scope")).thenReturn("scope");
Expand All @@ -266,7 +267,7 @@ public void shouldGetNonExpiredCredentialsFromStorage() {
when(storage.retrieveString("com.auth0.access_token")).thenReturn("accessToken");
when(storage.retrieveString("com.auth0.refresh_token")).thenReturn("refreshToken");
when(storage.retrieveString("com.auth0.token_type")).thenReturn("type");
long expirationTime = CredentialsMock.CURRENT_TIME_MS + 123456L * 1000;
long expirationTime = CredentialsMock.ONE_HOUR_AHEAD_MS;
when(storage.retrieveLong("com.auth0.expires_at")).thenReturn(expirationTime);
when(storage.retrieveLong("com.auth0.cache_expires_at")).thenReturn(expirationTime);
when(storage.retrieveString("com.auth0.scope")).thenReturn("scope");
Expand All @@ -281,7 +282,7 @@ public void shouldGetNonExpiredCredentialsFromStorage() {
assertThat(retrievedCredentials.getRefreshToken(), is("refreshToken"));
assertThat(retrievedCredentials.getType(), is("type"));
assertThat(retrievedCredentials.getExpiresIn(), is(notNullValue()));
assertThat(retrievedCredentials.getExpiresIn().doubleValue(), CoreMatchers.is(closeTo(123456L, 1)));
assertThat(retrievedCredentials.getExpiresIn().doubleValue(), CoreMatchers.is(closeTo(ONE_HOUR_SECONDS, 1)));
assertThat(retrievedCredentials.getExpiresAt(), is(notNullValue()));
assertThat(retrievedCredentials.getExpiresAt().getTime(), is(expirationTime));
assertThat(retrievedCredentials.getScope(), is("scope"));
Expand All @@ -295,7 +296,7 @@ public void shouldGetNonExpiredCredentialsFromStorageWhenOnlyIdTokenIsAvailable(
when(storage.retrieveString("com.auth0.access_token")).thenReturn(null);
when(storage.retrieveString("com.auth0.refresh_token")).thenReturn("refreshToken");
when(storage.retrieveString("com.auth0.token_type")).thenReturn("type");
long expirationTime = CredentialsMock.CURRENT_TIME_MS + 123456L * 1000;
long expirationTime = CredentialsMock.ONE_HOUR_AHEAD_MS;
when(storage.retrieveLong("com.auth0.expires_at")).thenReturn(expirationTime);
when(storage.retrieveLong("com.auth0.cache_expires_at")).thenReturn(expirationTime);
when(storage.retrieveString("com.auth0.scope")).thenReturn("scope");
Expand All @@ -310,7 +311,7 @@ public void shouldGetNonExpiredCredentialsFromStorageWhenOnlyIdTokenIsAvailable(
assertThat(retrievedCredentials.getRefreshToken(), is("refreshToken"));
assertThat(retrievedCredentials.getType(), is("type"));
assertThat(retrievedCredentials.getExpiresIn(), is(notNullValue()));
assertThat(retrievedCredentials.getExpiresIn().doubleValue(), CoreMatchers.is(closeTo(123456L, 1)));
assertThat(retrievedCredentials.getExpiresIn().doubleValue(), CoreMatchers.is(closeTo(ONE_HOUR_SECONDS, 1)));
assertThat(retrievedCredentials.getExpiresAt(), is(notNullValue()));
assertThat(retrievedCredentials.getExpiresAt().getTime(), is(expirationTime));
assertThat(retrievedCredentials.getScope(), is("scope"));
Expand All @@ -324,7 +325,7 @@ public void shouldGetNonExpiredCredentialsFromStorageWhenOnlyAccessTokenIsAvaila
when(storage.retrieveString("com.auth0.access_token")).thenReturn("accessToken");
when(storage.retrieveString("com.auth0.refresh_token")).thenReturn("refreshToken");
when(storage.retrieveString("com.auth0.token_type")).thenReturn("type");
Long expirationTime = CredentialsMock.CURRENT_TIME_MS + 123456L * 1000;
Long expirationTime = CredentialsMock.ONE_HOUR_AHEAD_MS;
when(storage.retrieveLong("com.auth0.expires_at")).thenReturn(expirationTime);
when(storage.retrieveLong("com.auth0.cache_expires_at")).thenReturn(expirationTime);
when(storage.retrieveString("com.auth0.scope")).thenReturn("scope");
Expand All @@ -339,7 +340,7 @@ public void shouldGetNonExpiredCredentialsFromStorageWhenOnlyAccessTokenIsAvaila
assertThat(retrievedCredentials.getRefreshToken(), is("refreshToken"));
assertThat(retrievedCredentials.getType(), is("type"));
assertThat(retrievedCredentials.getExpiresIn(), is(notNullValue()));
assertThat(retrievedCredentials.getExpiresIn().doubleValue(), CoreMatchers.is(closeTo(123456L, 1)));
assertThat(retrievedCredentials.getExpiresIn().doubleValue(), CoreMatchers.is(closeTo(ONE_HOUR_SECONDS, 1)));
assertThat(retrievedCredentials.getExpiresAt(), is(notNullValue()));
assertThat(retrievedCredentials.getExpiresAt().getTime(), is(expirationTime));
assertThat(retrievedCredentials.getScope(), is("scope"));
Expand All @@ -351,13 +352,13 @@ public void shouldRenewCredentialsWhenScopeHasChanged() {
when(storage.retrieveString("com.auth0.access_token")).thenReturn("accessToken");
when(storage.retrieveString("com.auth0.refresh_token")).thenReturn("refreshToken");
when(storage.retrieveString("com.auth0.token_type")).thenReturn("type");
long expirationTime = CredentialsMock.CURRENT_TIME_MS + 123456L * 1000; // non expired credentials
long expirationTime = CredentialsMock.ONE_HOUR_AHEAD_MS; // non expired credentials
when(storage.retrieveLong("com.auth0.expires_at")).thenReturn(expirationTime);
when(storage.retrieveLong("com.auth0.cache_expires_at")).thenReturn(expirationTime);
when(storage.retrieveString("com.auth0.scope")).thenReturn("some new scope");
when(client.renewAuth("refreshToken")).thenReturn(request);

Date newDate = new Date(CredentialsMock.CURRENT_TIME_MS + 1234 * 1000);
Date newDate = new Date(CredentialsMock.ONE_HOUR_AHEAD_MS + ONE_HOUR_SECONDS * 1000);
JWT jwtMock = mock(JWT.class);
when(jwtMock.getExpiresAt()).thenReturn(newDate);
when(jwtDecoder.decode("newId")).thenReturn(jwtMock);
Expand Down Expand Up @@ -457,7 +458,7 @@ public void shouldGetAndSuccessfullyRenewExpiredCredentials() {
when(storage.retrieveString("com.auth0.scope")).thenReturn("scope");
when(client.renewAuth("refreshToken")).thenReturn(request);

Date newDate = new Date(CredentialsMock.CURRENT_TIME_MS + 1234 * 1000);
Date newDate = new Date(CredentialsMock.ONE_HOUR_AHEAD_MS);
JWT jwtMock = mock(JWT.class);
when(jwtMock.getExpiresAt()).thenReturn(newDate);
when(jwtDecoder.decode("newId")).thenReturn(jwtMock);
Expand Down Expand Up @@ -547,7 +548,7 @@ public void shouldGetAndSuccessfullyRenewExpiredCredentialsWithRefreshTokenRotat
when(storage.retrieveString("com.auth0.scope")).thenReturn("scope");
when(client.renewAuth("refreshToken")).thenReturn(request);

Date newDate = new Date(CredentialsMock.CURRENT_TIME_MS + 1234 * 1000);
Date newDate = new Date(CredentialsMock.ONE_HOUR_AHEAD_MS);
JWT jwtMock = mock(JWT.class);
when(jwtMock.getExpiresAt()).thenReturn(newDate);
when(jwtDecoder.decode("newId")).thenReturn(jwtMock);
Expand Down Expand Up @@ -629,7 +630,7 @@ public void shouldClearCredentials() {

@Test
public void shouldHaveCredentialsWhenTokenHasNotExpired() {
long expirationTime = CredentialsMock.CURRENT_TIME_MS + 123456L * 1000;
long expirationTime = CredentialsMock.ONE_HOUR_AHEAD_MS;
when(storage.retrieveLong("com.auth0.expires_at")).thenReturn(expirationTime);
when(storage.retrieveLong("com.auth0.cache_expires_at")).thenReturn(expirationTime);

Expand Down
1 change: 1 addition & 0 deletions auth0/src/test/java/com/auth0/android/result/CredentialsMock.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
public class CredentialsMock extends Credentials {

public static final long CURRENT_TIME_MS = calculateCurrentTime();
public static final long ONE_HOUR_AHEAD_MS = CURRENT_TIME_MS + 60 * 60 * 1000;

public CredentialsMock(@Nullable String idToken, @Nullable String accessToken, @Nullable String type, @Nullable String refreshToken, @Nullable Long expiresIn) {
super(idToken, accessToken, type, refreshToken, expiresIn);
Expand Down

0 comments on commit ac08e60

Please sign in to comment.