Skip to content

Commit 1437bde

Browse files
authored
Revert "Idtoken time skew (#1)"
This reverts commit bc9ebba.
1 parent bc9ebba commit 1437bde

File tree

5 files changed

+16
-145
lines changed

5 files changed

+16
-145
lines changed

README.md

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -619,7 +619,7 @@ AppAuthConfiguration appAuthConfig = new AppAuthConfiguration.Builder()
619619

620620
ID Token validation was introduced in `0.8.0` but not all authorization servers or configurations support it correctly.
621621

622-
- 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.
622+
- 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.
623623

624624
```java
625625
AppAuthConfiguration appAuthConfig = new AppAuthConfiguration.Builder()
@@ -635,22 +635,6 @@ AuthorizationRequest authRequest = authRequestBuilder
635635
.build();
636636
```
637637

638-
- 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.
639-
640-
```java
641-
AppAuthConfiguration appAuthConfig = new AppAuthConfiguration.Builder()
642-
.setAllowedIssueTimeSkew(THIRTY_MINUTES_IN_SECONDS)
643-
.build()
644-
```
645-
646-
- 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.
647-
648-
```java
649-
AppAuthConfiguration appAuthConfig = new AppAuthConfiguration.Builder()
650-
.setSkipIssueTimeValidation(true)
651-
.build()
652-
```
653-
654638
## Dynamic client registration
655639

656640
AppAuth supports the

library/java/net/openid/appauth/AppAuthConfiguration.java

Lines changed: 2 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -42,21 +42,13 @@ public class AppAuthConfiguration {
4242

4343
private final boolean mSkipIssuerHttpsCheck;
4444

45-
private final boolean mSkipIssueTimeValidation;
46-
47-
private final Long mAllowedIssueTimeSkew;
48-
4945
private AppAuthConfiguration(
5046
@NonNull BrowserMatcher browserMatcher,
5147
@NonNull ConnectionBuilder connectionBuilder,
52-
Boolean skipIssuerHttpsCheck,
53-
Boolean skipIssueTimeValidation,
54-
Long allowedIssueTimeSkew) {
48+
Boolean skipIssuerHttpsCheck) {
5549
mBrowserMatcher = browserMatcher;
5650
mConnectionBuilder = connectionBuilder;
5751
mSkipIssuerHttpsCheck = skipIssuerHttpsCheck;
58-
mSkipIssueTimeValidation = skipIssueTimeValidation;
59-
mAllowedIssueTimeSkew = allowedIssueTimeSkew;
6052
}
6153

6254
/**
@@ -84,22 +76,6 @@ public ConnectionBuilder getConnectionBuilder() {
8476
*/
8577
public boolean getSkipIssuerHttpsCheck() { return mSkipIssuerHttpsCheck; }
8678

87-
/**
88-
* Returns <code>true</code> if the ID token issue time validation is disables,
89-
* otherwise <code>false</code>.
90-
*
91-
* @see Builder#setSkipIssueTimeValidation(Boolean)
92-
*/
93-
public boolean getSkipIssueTimeValidation() { return mSkipIssueTimeValidation; }
94-
95-
/**
96-
* Returns the time in seconds that the ID token issue time is allowed to be
97-
* skewed.
98-
*
99-
* @see Builder#setAllowedIssueTimeSkew(Long)
100-
*/
101-
public Long getAllowedIssueTimeSkew() { return mAllowedIssueTimeSkew; }
102-
10379
/**
10480
* Creates {@link AppAuthConfiguration} instances.
10581
*/
@@ -108,8 +84,6 @@ public static class Builder {
10884
private BrowserMatcher mBrowserMatcher = AnyBrowserMatcher.INSTANCE;
10985
private ConnectionBuilder mConnectionBuilder = DefaultConnectionBuilder.INSTANCE;
11086
private boolean mSkipIssuerHttpsCheck;
111-
private boolean mSkipIssueTimeValidation;
112-
private Long mAllowedIssueTimeSkew;
11387
private boolean mSkipNonceVerification;
11488

11589
/**
@@ -145,22 +119,6 @@ public Builder setSkipIssuerHttpsCheck(Boolean skipIssuerHttpsCheck) {
145119
return this;
146120
}
147121

148-
/**
149-
* Disables issue time validation for the id token.
150-
*/
151-
public Builder setSkipIssueTimeValidation(Boolean skipIssueTimeValidation) {
152-
mSkipIssueTimeValidation = skipIssueTimeValidation;
153-
return this;
154-
}
155-
156-
/**
157-
* Sets the allowed time skew in seconds for id token issue time validation.
158-
*/
159-
public Builder setAllowedIssueTimeSkew(Long allowedIssueTimeSkew) {
160-
mAllowedIssueTimeSkew = allowedIssueTimeSkew;
161-
return this;
162-
}
163-
164122
/**
165123
* Creates the instance from the configured properties.
166124
*/
@@ -169,9 +127,7 @@ public AppAuthConfiguration build() {
169127
return new AppAuthConfiguration(
170128
mBrowserMatcher,
171129
mConnectionBuilder,
172-
mSkipIssuerHttpsCheck,
173-
mSkipIssueTimeValidation,
174-
mAllowedIssueTimeSkew
130+
mSkipIssuerHttpsCheck
175131
);
176132
}
177133

library/java/net/openid/appauth/AuthorizationService.java

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -506,9 +506,7 @@ public void performTokenRequest(
506506
mClientConfiguration.getConnectionBuilder(),
507507
SystemClock.INSTANCE,
508508
callback,
509-
mClientConfiguration.getSkipIssuerHttpsCheck(),
510-
mClientConfiguration.getSkipIssueTimeValidation(),
511-
mClientConfiguration.getAllowedIssueTimeSkew())
509+
mClientConfiguration.getSkipIssuerHttpsCheck())
512510
.execute();
513511
}
514512

@@ -587,8 +585,6 @@ private static class TokenRequestTask
587585
private TokenResponseCallback mCallback;
588586
private Clock mClock;
589587
private boolean mSkipIssuerHttpsCheck;
590-
private boolean mSkipIssueTimeValidation;
591-
private Long mAllowedIssueTimeSkew;
592588

593589
private AuthorizationException mException;
594590

@@ -597,17 +593,13 @@ private static class TokenRequestTask
597593
@NonNull ConnectionBuilder connectionBuilder,
598594
Clock clock,
599595
TokenResponseCallback callback,
600-
Boolean skipIssuerHttpsCheck,
601-
Boolean skipissueTimeValidation,
602-
Long allowedIssueTimeSkew) {
596+
Boolean skipIssuerHttpsCheck) {
603597
mRequest = request;
604598
mClientAuthentication = clientAuthentication;
605599
mConnectionBuilder = connectionBuilder;
606600
mClock = clock;
607601
mCallback = callback;
608602
mSkipIssuerHttpsCheck = skipIssuerHttpsCheck;
609-
mSkipIssueTimeValidation = skipissueTimeValidation;
610-
mAllowedIssueTimeSkew = allowedIssueTimeSkew;
611603
}
612604

613605
@Override
@@ -718,9 +710,7 @@ protected void onPostExecute(JSONObject json) {
718710
idToken.validate(
719711
mRequest,
720712
mClock,
721-
mSkipIssuerHttpsCheck,
722-
mSkipIssueTimeValidation,
723-
mAllowedIssueTimeSkew
713+
mSkipIssuerHttpsCheck
724714
);
725715
} catch (AuthorizationException ex) {
726716
mCallback.onTokenRequestCompleted(null, ex);

library/java/net/openid/appauth/IdToken.java

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -204,14 +204,12 @@ static IdToken from(String token) throws JSONException, IdTokenException {
204204

205205
@VisibleForTesting
206206
void validate(@NonNull TokenRequest tokenRequest, Clock clock) throws AuthorizationException {
207-
validate(tokenRequest, clock, false, false, null);
207+
validate(tokenRequest, clock, false);
208208
}
209209

210210
void validate(@NonNull TokenRequest tokenRequest,
211211
Clock clock,
212-
boolean skipIssuerHttpsCheck,
213-
boolean skipIssueTimeValidation,
214-
@Nullable Long allowedIssueTimeSkew) throws AuthorizationException {
212+
boolean skipIssuerHttpsCheck) throws AuthorizationException {
215213
// OpenID Connect Core Section 3.1.3.7. rule #1
216214
// Not enforced: AppAuth does not support JWT encryption.
217215

@@ -278,16 +276,13 @@ void validate(@NonNull TokenRequest tokenRequest,
278276
new IdTokenException("ID Token expired"));
279277
}
280278

281-
282-
if (!skipIssueTimeValidation) {
283-
// OpenID Connect Core Section 3.1.3.7. rule #10
284-
// Validates that the issued at time is not more than the +/- configured allowed time skew,
285-
// or +/- 10 minutes as a default, on the current time.
286-
if (Math.abs(nowInSeconds - this.issuedAt) > (allowedIssueTimeSkew == null ? TEN_MINUTES_IN_SECONDS : allowedIssueTimeSkew)) {
287-
throw AuthorizationException.fromTemplate(GeneralErrors.ID_TOKEN_VALIDATION_ERROR,
288-
new IdTokenException("Issued at time is more than 10 minutes "
289-
+ "before or after the current time"));
290-
}
279+
// OpenID Connect Core Section 3.1.3.7. rule #10
280+
// Validates that the issued at time is not more than +/- 10 minutes on the current
281+
// time.
282+
if (Math.abs(nowInSeconds - this.issuedAt) > TEN_MINUTES_IN_SECONDS) {
283+
throw AuthorizationException.fromTemplate(GeneralErrors.ID_TOKEN_VALIDATION_ERROR,
284+
new IdTokenException("Issued at time is more than 10 minutes "
285+
+ "before or after the current time"));
291286
}
292287

293288
// Only relevant for the authorization_code response type

library/javatests/net/openid/appauth/IdTokenTest.java

Lines changed: 1 addition & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ public void testValidate_shouldSkipNonHttpsIssuer()
272272
.setRedirectUri(TEST_APP_REDIRECT_URI)
273273
.build();
274274
Clock clock = SystemClock.INSTANCE;
275-
idToken.validate(tokenRequest, clock, true, false, null);
275+
idToken.validate(tokenRequest, clock, true);
276276
}
277277

278278
@Test(expected = AuthorizationException.class)
@@ -464,60 +464,6 @@ public void testValidate_shouldFailOnIssuedAtOverTenMinutesAgo() throws Authoriz
464464
idToken.validate(tokenRequest, clock);
465465
}
466466

467-
@Test
468-
public void testValidate_withSkipIssueTimeValidation() throws AuthorizationException {
469-
Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000;
470-
Long anHourInSeconds = (long) (60 * 60);
471-
IdToken idToken = new IdToken(
472-
TEST_ISSUER,
473-
TEST_SUBJECT,
474-
Collections.singletonList(TEST_CLIENT_ID),
475-
nowInSeconds,
476-
nowInSeconds - (anHourInSeconds * 2),
477-
TEST_NONCE,
478-
TEST_CLIENT_ID
479-
);
480-
TokenRequest tokenRequest = getAuthCodeExchangeRequestWithNonce();
481-
Clock clock = SystemClock.INSTANCE;
482-
idToken.validate(tokenRequest, clock, false, true, null);
483-
}
484-
485-
@Test(expected = AuthorizationException.class)
486-
public void testValidate_shouldFailOnIssuedAtOverConfiguredTimeSkew() throws AuthorizationException {
487-
Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000;
488-
Long anHourInSeconds = (long) (60 * 60);
489-
IdToken idToken = new IdToken(
490-
TEST_ISSUER,
491-
TEST_SUBJECT,
492-
Collections.singletonList(TEST_CLIENT_ID),
493-
nowInSeconds,
494-
nowInSeconds - anHourInSeconds - 1,
495-
TEST_NONCE,
496-
TEST_CLIENT_ID
497-
);
498-
TokenRequest tokenRequest = getAuthCodeExchangeRequestWithNonce();
499-
Clock clock = SystemClock.INSTANCE;
500-
idToken.validate(tokenRequest, clock, false, false, anHourInSeconds);
501-
}
502-
503-
@Test
504-
public void testValidate_withConfiguredTimeSkew() throws AuthorizationException {
505-
Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000;
506-
Long anHourInSeconds = (long) (60 * 60);
507-
IdToken idToken = new IdToken(
508-
TEST_ISSUER,
509-
TEST_SUBJECT,
510-
Collections.singletonList(TEST_CLIENT_ID),
511-
nowInSeconds,
512-
nowInSeconds - anHourInSeconds,
513-
TEST_NONCE,
514-
TEST_CLIENT_ID
515-
);
516-
TokenRequest tokenRequest = getAuthCodeExchangeRequestWithNonce();
517-
Clock clock = SystemClock.INSTANCE;
518-
idToken.validate(tokenRequest, clock, false, false, anHourInSeconds);
519-
}
520-
521467
@Test(expected = AuthorizationException.class)
522468
public void testValidate_shouldFailOnNonceMismatch() throws AuthorizationException {
523469
Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000;

0 commit comments

Comments
 (0)