chore PR comments

auth0 · Nov 4, 2020 · 1643e38 · 1643e38
1 parent 92b8d65
commit 1643e38
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 17 deletions.
diff --git a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java
@@ -220,7 +220,7 @@ public void getCredentials(@NonNull BaseCallback<Credentials, CredentialsManager
      * @param callback the callback to receive the result in.
      */
     public void getCredentials(@Nullable String scope, int minTtl, @NonNull BaseCallback<Credentials, CredentialsManagerException> callback) {
-        if (!hasValidCredentials()) {
+        if (!hasValidCredentials(minTtl)) {
             callback.onFailure(new CredentialsManagerException("No Credentials were previously set."));
             return;
         }
@@ -369,11 +369,11 @@ 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) {
-        if (minTtl == 0 && (expiresAt == null || expiresAt == 0)) {
+    private boolean willExpire(@Nullable Long expiresAt, long minTtl) {
+        if (expiresAt == null || expiresAt <= 0) {
             //expiresAt (access token) only considered if it has a positive value, to avoid logging out users
             return false;
         }
@@ -385,7 +385,7 @@ private boolean hasExpired(long expiresAt) {
         return expiresAt <= getCurrentTimeInMillis();
     }
 
-    private long calculateCacheExpiresAt(Credentials credentials) {
+    private long calculateCacheExpiresAt(@NonNull Credentials credentials) {
         long expiresAt = credentials.getExpiresAt().getTime();
 
         if (credentials.getIdToken() != null) {

diff --git a/.../src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.java b/.../src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.java
@@ -459,7 +459,7 @@ public void shouldRenewCredentialsWithMinTtl() {
         when(jwtDecoder.decode("newId")).thenReturn(jwtMock);
         when(client.renewAuth("refreshToken")).thenReturn(request);
 
-        manager.getCredentials(null, 60, callback);
+        manager.getCredentials(null, 60, callback); // minTTL of 1 minute
         verify(request, never()).addParameter(eq("scope"), anyString());
         verify(request).start(requestCallbackCaptor.capture());
 
@@ -510,11 +510,11 @@ public void shouldGetAndFailToRenewExpiredCredentialsWhenReceivedTokenHasLowerTt
         when(jwtDecoder.decode("newId")).thenReturn(jwtMock);
         when(client.renewAuth("refreshToken")).thenReturn(request);
 
-        manager.getCredentials(null, 60, callback);
+        manager.getCredentials(null, 60, callback); // minTTL of 1 minute
         verify(request, never()).addParameter(eq("scope"), anyString());
         verify(request).start(requestCallbackCaptor.capture());
 
-        // Trigger success
+        // Trigger failure
         Credentials expectedCredentials = new Credentials("newId", "newAccess", "newType", "refreshToken", newDate, "newScope");
         String expectedJson = gson.toJson(expectedCredentials);
         when(crypto.encrypt(expectedJson.getBytes())).thenReturn(expectedJson.getBytes());
@@ -542,7 +542,7 @@ public void shouldRenewCredentialsWhenScopeHasChanged() {
         when(jwtDecoder.decode("newId")).thenReturn(jwtMock);
         when(client.renewAuth("refreshToken")).thenReturn(request);
 
-        manager.getCredentials("different scope", 0, callback);
+        manager.getCredentials("different scope", 0, callback);  // minTTL of 0 seconds (default)
         verify(request).addParameter(eq("scope"), eq("different scope"));
         verify(request).start(requestCallbackCaptor.capture());
 
@@ -593,16 +593,15 @@ public void shouldRenewExpiredCredentialsWhenScopeHasChanged() {
         when(jwtDecoder.decode("newId")).thenReturn(jwtMock);
         when(client.renewAuth("refreshToken")).thenReturn(request);
 
-        manager.getCredentials(callback);
-        verify(request, never()).addParameter(eq("scope"), anyString());
+        manager.getCredentials("different scope", 0, callback);  // minTTL of 0 seconds (default)
+        verify(request).addParameter(eq("scope"), eq("different scope"));
         verify(request).start(requestCallbackCaptor.capture());
 
         // Trigger success
-        Credentials renewedCredentials = new Credentials("newId", "newAccess", "newType", null, newDate, "newScope");
-        Credentials expectedCredentials = new Credentials("newId", "newAccess", "newType", "refreshToken", newDate, "newScope");
+        Credentials expectedCredentials = new Credentials("newId", "newAccess", "newType", "refreshToken", newDate, "different scope");
         String expectedJson = gson.toJson(expectedCredentials);
         when(crypto.encrypt(expectedJson.getBytes())).thenReturn(expectedJson.getBytes());
-        requestCallbackCaptor.getValue().onSuccess(renewedCredentials);
+        requestCallbackCaptor.getValue().onSuccess(expectedCredentials);
         verify(callback).onSuccess(credentialsCaptor.capture());
         verify(storage).store(eq("com.auth0.credentials"), stringCaptor.capture());
         verify(storage).store("com.auth0.credentials_expires_at", newDate.getTime());
@@ -618,7 +617,7 @@ public void shouldRenewExpiredCredentialsWhenScopeHasChanged() {
         assertThat(retrievedCredentials.getType(), is("newType"));
         assertThat(retrievedCredentials.getRefreshToken(), is("refreshToken"));
         assertThat(retrievedCredentials.getExpiresAt(), is(newDate));
-        assertThat(retrievedCredentials.getScope(), is("newScope"));
+        assertThat(retrievedCredentials.getScope(), is("different scope"));
 
         // Verify the credentials are property stored
         String encodedJson = stringCaptor.getValue();
@@ -631,7 +630,7 @@ public void shouldRenewExpiredCredentialsWhenScopeHasChanged() {
         assertThat(renewedStoredCredentials.getType(), is("newType"));
         assertThat(renewedStoredCredentials.getExpiresAt(), is(notNullValue()));
         assertThat(renewedStoredCredentials.getExpiresAt().getTime(), is(newDate.getTime()));
-        assertThat(renewedStoredCredentials.getScope(), is("newScope"));
+        assertThat(renewedStoredCredentials.getScope(), is("different scope"));
     }
 
     @Test
@@ -792,7 +791,7 @@ public void shouldClearCredentials() {
      */
 
     @Test
-    public void shouldHaveValidCredentialsDuringMigrationWhenAccessTokenExpiresAtValueWasNotSaved() {
+    public void shouldPreventLoggingOutUsersWhenAccessTokenExpiresAtWasNotSaved() {
         long cacheExpiresAt = CredentialsMock.ONE_HOUR_AHEAD_MS;
         when(storage.retrieveLong("com.auth0.credentials_expires_at")).thenReturn(cacheExpiresAt);
         when(storage.retrieveLong("com.auth0.credentials_access_token_expires_at")).thenReturn(null);