Revert "Idtoken time skew (#1)"

nisrulz · web-flow · commit 1437bde8955a · 2024-02-26T12:08:10.000+01:00
This reverts commit bc9ebba.
diff --git a/README.md b/README.md
@@ -619,7 +619,7 @@ AppAuthConfiguration appAuthConfig = new AppAuthConfiguration.Builder()
 
 ID Token validation was introduced in `0.8.0` but not all authorization servers or configurations support it correctly.
 
-- For testing environments [setSkipIssuerHttpsCheck](https://github.com/openid/AppAuth-Android/blob/master/library/java/net/openid/appauth/AppAuthConfiguration.java#L143) can be used to bypass the fact the issuer needs to be HTTPS.
+- For testing environments [setSkipIssuerHttpsCheck](https://github.com/openid/AppAuth-Android/blob/master/library/java/net/openid/appauth/AppAuthConfiguration.java#L129) can be used to bypass the fact the issuer needs to be HTTPS.
 
 ```java
 AppAuthConfiguration appAuthConfig = new AppAuthConfiguration.Builder()
@@ -635,22 +635,6 @@ AuthorizationRequest authRequest = authRequestBuilder
     .build();
 ```
 
-- To change the default allowed time skew of 10 minutes for the issue time, [setAllowedIssueTimeSkew](https://github.com/openid/AppAuth-Android/blob/master/library/java/net/openid/appauth/AppAuthConfiguration.java#L159) can be used.
-
-```java
-AppAuthConfiguration appAuthConfig = new AppAuthConfiguration.Builder()
-    .setAllowedIssueTimeSkew(THIRTY_MINUTES_IN_SECONDS)
-    .build()
-```
-
-- For testing environments [setSkipIssueTimeValidation](https://github.com/openid/AppAuth-Android/blob/master/library/java/net/openid/appauth/AppAuthConfiguration.java#L151) can be used to bypass the issue time validation.
-
-```java
-AppAuthConfiguration appAuthConfig = new AppAuthConfiguration.Builder()
-    .setSkipIssueTimeValidation(true)
-    .build()
-```
-
 ## Dynamic client registration
 
 AppAuth supports the
diff --git a/library/java/net/openid/appauth/AppAuthConfiguration.java b/library/java/net/openid/appauth/AppAuthConfiguration.java
@@ -42,21 +42,13 @@ public class AppAuthConfiguration {
 
     private final boolean mSkipIssuerHttpsCheck;
 
-    private final boolean mSkipIssueTimeValidation;
-
-    private final Long mAllowedIssueTimeSkew;
-
     private AppAuthConfiguration(
             @NonNull BrowserMatcher browserMatcher,
             @NonNull ConnectionBuilder connectionBuilder,
-            Boolean skipIssuerHttpsCheck,
-            Boolean skipIssueTimeValidation,
-            Long allowedIssueTimeSkew) {
+            Boolean skipIssuerHttpsCheck) {
         mBrowserMatcher = browserMatcher;
         mConnectionBuilder = connectionBuilder;
         mSkipIssuerHttpsCheck = skipIssuerHttpsCheck;
-        mSkipIssueTimeValidation = skipIssueTimeValidation;
-        mAllowedIssueTimeSkew = allowedIssueTimeSkew;
     }
 
     /**
@@ -84,22 +76,6 @@ public ConnectionBuilder getConnectionBuilder() {
      */
     public boolean getSkipIssuerHttpsCheck() { return mSkipIssuerHttpsCheck; }
 
-    /**
-     * Returns <code>true</code> if the ID token issue time validation is disables,
-     * otherwise <code>false</code>.
-     *
-     * @see Builder#setSkipIssueTimeValidation(Boolean)
-     */
-    public boolean getSkipIssueTimeValidation() { return mSkipIssueTimeValidation; }
-
-    /**
-     * Returns the time in seconds that the ID token issue time is allowed to be
-     * skewed.
-     *
-     * @see Builder#setAllowedIssueTimeSkew(Long)
-     */
-    public Long getAllowedIssueTimeSkew() { return mAllowedIssueTimeSkew; }
-
     /**
      * Creates {@link AppAuthConfiguration} instances.
      */
@@ -108,8 +84,6 @@ public static class Builder {
         private BrowserMatcher mBrowserMatcher = AnyBrowserMatcher.INSTANCE;
         private ConnectionBuilder mConnectionBuilder = DefaultConnectionBuilder.INSTANCE;
         private boolean mSkipIssuerHttpsCheck;
-        private boolean mSkipIssueTimeValidation;
-        private Long mAllowedIssueTimeSkew;
         private boolean mSkipNonceVerification;
 
         /**
@@ -145,22 +119,6 @@ public Builder setSkipIssuerHttpsCheck(Boolean skipIssuerHttpsCheck) {
             return this;
         }
 
-        /**
-         * Disables issue time validation for the id token.
-         */
-        public Builder setSkipIssueTimeValidation(Boolean skipIssueTimeValidation) {
-            mSkipIssueTimeValidation = skipIssueTimeValidation;
-            return this;
-        }
-
-        /**
-         * Sets the allowed time skew in seconds for id token issue time validation.
-         */
-        public Builder setAllowedIssueTimeSkew(Long allowedIssueTimeSkew) {
-            mAllowedIssueTimeSkew = allowedIssueTimeSkew;
-            return this;
-        }
-
         /**
          * Creates the instance from the configured properties.
          */
@@ -169,9 +127,7 @@ public AppAuthConfiguration build() {
             return new AppAuthConfiguration(
                 mBrowserMatcher,
                 mConnectionBuilder,
-                mSkipIssuerHttpsCheck,
-                mSkipIssueTimeValidation,
-                mAllowedIssueTimeSkew
+                mSkipIssuerHttpsCheck
             );
         }
 
diff --git a/library/java/net/openid/appauth/AuthorizationService.java b/library/java/net/openid/appauth/AuthorizationService.java
@@ -506,9 +506,7 @@ public void performTokenRequest(
                 mClientConfiguration.getConnectionBuilder(),
                 SystemClock.INSTANCE,
                 callback,
-                mClientConfiguration.getSkipIssuerHttpsCheck(),
-                mClientConfiguration.getSkipIssueTimeValidation(),
-                mClientConfiguration.getAllowedIssueTimeSkew())
+                mClientConfiguration.getSkipIssuerHttpsCheck())
                 .execute();
     }
 
@@ -587,8 +585,6 @@ private static class TokenRequestTask
         private TokenResponseCallback mCallback;
         private Clock mClock;
         private boolean mSkipIssuerHttpsCheck;
-        private boolean mSkipIssueTimeValidation;
-        private Long mAllowedIssueTimeSkew;
 
         private AuthorizationException mException;
 
@@ -597,17 +593,13 @@ private static class TokenRequestTask
                          @NonNull ConnectionBuilder connectionBuilder,
                          Clock clock,
                          TokenResponseCallback callback,
-                         Boolean skipIssuerHttpsCheck,
-                         Boolean skipissueTimeValidation,
-                         Long allowedIssueTimeSkew) {
+                         Boolean skipIssuerHttpsCheck) {
             mRequest = request;
             mClientAuthentication = clientAuthentication;
             mConnectionBuilder = connectionBuilder;
             mClock = clock;
             mCallback = callback;
             mSkipIssuerHttpsCheck = skipIssuerHttpsCheck;
-            mSkipIssueTimeValidation = skipissueTimeValidation;
-            mAllowedIssueTimeSkew = allowedIssueTimeSkew;
         }
 
         @Override
@@ -718,9 +710,7 @@ protected void onPostExecute(JSONObject json) {
                     idToken.validate(
                             mRequest,
                             mClock,
-                            mSkipIssuerHttpsCheck,
-                            mSkipIssueTimeValidation,
-                            mAllowedIssueTimeSkew
+                            mSkipIssuerHttpsCheck
                     );
                 } catch (AuthorizationException ex) {
                     mCallback.onTokenRequestCompleted(null, ex);
diff --git a/library/java/net/openid/appauth/IdToken.java b/library/java/net/openid/appauth/IdToken.java
@@ -204,14 +204,12 @@ static IdToken from(String token) throws JSONException, IdTokenException {
 
     @VisibleForTesting
     void validate(@NonNull TokenRequest tokenRequest, Clock clock) throws AuthorizationException {
-        validate(tokenRequest, clock, false, false, null);
+        validate(tokenRequest, clock, false);
     }
 
     void validate(@NonNull TokenRequest tokenRequest,
                   Clock clock,
-                  boolean skipIssuerHttpsCheck,
-                  boolean skipIssueTimeValidation,
-                  @Nullable Long allowedIssueTimeSkew) throws AuthorizationException {
+                  boolean skipIssuerHttpsCheck) throws AuthorizationException {
         // OpenID Connect Core Section 3.1.3.7. rule #1
         // Not enforced: AppAuth does not support JWT encryption.
 
@@ -278,16 +276,13 @@ void validate(@NonNull TokenRequest tokenRequest,
                 new IdTokenException("ID Token expired"));
         }
 
-
-        if (!skipIssueTimeValidation) {
-            // OpenID Connect Core Section 3.1.3.7. rule #10
-            // Validates that the issued at time is not more than the +/- configured allowed time skew,
-            // or +/- 10 minutes as a default, on the current time.
-            if (Math.abs(nowInSeconds - this.issuedAt) > (allowedIssueTimeSkew == null ? TEN_MINUTES_IN_SECONDS : allowedIssueTimeSkew)) {
-                throw AuthorizationException.fromTemplate(GeneralErrors.ID_TOKEN_VALIDATION_ERROR,
-                    new IdTokenException("Issued at time is more than 10 minutes "
-                        + "before or after the current time"));
-            }
+        // OpenID Connect Core Section 3.1.3.7. rule #10
+        // Validates that the issued at time is not more than +/- 10 minutes on the current
+        // time.
+        if (Math.abs(nowInSeconds - this.issuedAt) > TEN_MINUTES_IN_SECONDS) {
+            throw AuthorizationException.fromTemplate(GeneralErrors.ID_TOKEN_VALIDATION_ERROR,
+                new IdTokenException("Issued at time is more than 10 minutes "
+                    + "before or after the current time"));
         }
 
         // Only relevant for the authorization_code response type
diff --git a/library/javatests/net/openid/appauth/IdTokenTest.java b/library/javatests/net/openid/appauth/IdTokenTest.java
@@ -272,7 +272,7 @@ public void testValidate_shouldSkipNonHttpsIssuer()
             .setRedirectUri(TEST_APP_REDIRECT_URI)
             .build();
         Clock clock = SystemClock.INSTANCE;
-        idToken.validate(tokenRequest, clock, true, false, null);
+        idToken.validate(tokenRequest, clock, true);
     }
 
     @Test(expected = AuthorizationException.class)
@@ -464,60 +464,6 @@ public void testValidate_shouldFailOnIssuedAtOverTenMinutesAgo() throws Authoriz
         idToken.validate(tokenRequest, clock);
     }
 
-    @Test
-    public void testValidate_withSkipIssueTimeValidation() throws AuthorizationException {
-        Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000;
-        Long anHourInSeconds = (long) (60 * 60);
-        IdToken idToken = new IdToken(
-            TEST_ISSUER,
-            TEST_SUBJECT,
-            Collections.singletonList(TEST_CLIENT_ID),
-            nowInSeconds,
-            nowInSeconds - (anHourInSeconds * 2),
-            TEST_NONCE,
-            TEST_CLIENT_ID
-        );
-        TokenRequest tokenRequest = getAuthCodeExchangeRequestWithNonce();
-        Clock clock = SystemClock.INSTANCE;
-        idToken.validate(tokenRequest, clock, false, true, null);
-    }
-
-    @Test(expected = AuthorizationException.class)
-    public void testValidate_shouldFailOnIssuedAtOverConfiguredTimeSkew() throws AuthorizationException {
-        Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000;
-        Long anHourInSeconds = (long) (60 * 60);
-        IdToken idToken = new IdToken(
-            TEST_ISSUER,
-            TEST_SUBJECT,
-            Collections.singletonList(TEST_CLIENT_ID),
-            nowInSeconds,
-            nowInSeconds - anHourInSeconds - 1,
-            TEST_NONCE,
-            TEST_CLIENT_ID
-        );
-        TokenRequest tokenRequest = getAuthCodeExchangeRequestWithNonce();
-        Clock clock = SystemClock.INSTANCE;
-        idToken.validate(tokenRequest, clock, false, false, anHourInSeconds);
-    }
-
-    @Test
-    public void testValidate_withConfiguredTimeSkew() throws AuthorizationException {
-        Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000;
-        Long anHourInSeconds = (long) (60 * 60);
-        IdToken idToken = new IdToken(
-            TEST_ISSUER,
-            TEST_SUBJECT,
-            Collections.singletonList(TEST_CLIENT_ID),
-            nowInSeconds,
-            nowInSeconds - anHourInSeconds,
-            TEST_NONCE,
-            TEST_CLIENT_ID
-        );
-        TokenRequest tokenRequest = getAuthCodeExchangeRequestWithNonce();
-        Clock clock = SystemClock.INSTANCE;
-        idToken.validate(tokenRequest, clock, false, false, anHourInSeconds);
-    }
-
     @Test(expected = AuthorizationException.class)
     public void testValidate_shouldFailOnNonceMismatch() throws AuthorizationException {
         Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000;
