From 06a3d1c1ce70d4dca5bc2a6b4a756c79c1e1d4e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98yvind=20Robertsen?= Date: Mon, 23 Jul 2018 17:02:30 +0200 Subject: [PATCH 01/13] fix: move google() to top of repositories list This fixes an issue with initial project sync / assemble where gradle would not find com.android.support:support-fragment:27.1.1. --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index ea28481d..a67bbeb3 100644 --- a/build.gradle +++ b/build.gradle @@ -15,8 +15,8 @@ buildscript { subprojects { repositories { - jcenter() google() + jcenter() maven { url 'https://jitpack.io' } } } From ad64a89b941bbff84a4de7aa9e8975dbdc998718 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98yvind=20Robertsen?= Date: Mon, 23 Jul 2018 18:06:41 +0200 Subject: [PATCH 02/13] feat: Add nonce to AuthorizationRequest --- .../openid/appauth/AuthorizationRequest.java | 26 +++++++++++++++++++ .../appauth/AuthorizationRequestTest.java | 6 +++++ .../appauth/AuthorizationServiceTest.java | 14 +++++++++- .../net/openid/appauth/TestValues.java | 1 + 4 files changed, 46 insertions(+), 1 deletion(-) diff --git a/library/java/net/openid/appauth/AuthorizationRequest.java b/library/java/net/openid/appauth/AuthorizationRequest.java index fe403f5b..ce1b26e1 100644 --- a/library/java/net/openid/appauth/AuthorizationRequest.java +++ b/library/java/net/openid/appauth/AuthorizationRequest.java @@ -436,6 +436,19 @@ public static final class ResponseMode { @Nullable public final String state; + /** + * String value used to associate a Client session with an ID Token, and to mitigate replay + * attacks. The value is passed through unmodified from the Authentication Request to the ID + * Token. If this value is not explicitly set, this library will automatically add nonce and + * perform appropriate validation of the ID Token. It is recommended that the default + * implementation of this parameter be used wherever possible. + * + * @see "OpenID Connect Core 1.0, Section 3.1.2.1 + * " + */ + @Nullable + public final String nonce; + /** * The proof key for code exchange. This is an opaque value used to associate an authorization * request with a subsequent code exchange, in order to prevent any eavesdropping party from @@ -542,6 +555,9 @@ public static final class Builder { @Nullable private String mState; + @Nullable + private String mNonce; + @Nullable private String mCodeVerifier; @@ -570,6 +586,7 @@ public Builder( setResponseType(responseType); setRedirectUri(redirectUri); setState(AuthorizationRequest.generateRandomState()); + setNonce(AuthorizationRequest.generateRandomState()); setCodeVerifier(CodeVerifierUtil.generateRandomCodeVerifier()); } @@ -763,6 +780,12 @@ public Builder setState(@Nullable String state) { return this; } + @NonNull + public Builder setNonce(@Nullable String nonce) { + mNonce = checkNullOrNotEmpty(nonce, "state cannot be empty if defined"); + return this; + } + /** * Specifies the code verifier to use for this authorization request. The default challenge * method (typically {@link #CODE_CHALLENGE_METHOD_S256}) implemented by @@ -877,6 +900,7 @@ public AuthorizationRequest build() { mPrompt, mScope, mState, + mNonce, mCodeVerifier, mCodeVerifierChallenge, mCodeVerifierChallengeMethod, @@ -895,6 +919,7 @@ private AuthorizationRequest( @Nullable String prompt, @Nullable String scope, @Nullable String state, + @Nullable String nonce, @Nullable String codeVerifier, @Nullable String codeVerifierChallenge, @Nullable String codeVerifierChallengeMethod, @@ -913,6 +938,7 @@ private AuthorizationRequest( this.prompt = prompt; this.scope = scope; this.state = state; + this.nonce = nonce; this.codeVerifier = codeVerifier; this.codeVerifierChallenge = codeVerifierChallenge; this.codeVerifierChallengeMethod = codeVerifierChallengeMethod; diff --git a/library/javatests/net/openid/appauth/AuthorizationRequestTest.java b/library/javatests/net/openid/appauth/AuthorizationRequestTest.java index 12d59205..0b8abdf0 100644 --- a/library/javatests/net/openid/appauth/AuthorizationRequestTest.java +++ b/library/javatests/net/openid/appauth/AuthorizationRequestTest.java @@ -432,6 +432,12 @@ public void testState_autoGenerated() { assertThat(request.state).isNotEmpty(); } + @Test + public void testNonce_autoGenerated() { + AuthorizationRequest request = mRequestBuilder.build(); + assertThat(request.nonce).isNotEmpty(); + } + /* ******************************* additionalParams *******************************************/ @Test(expected = IllegalArgumentException.class) diff --git a/library/javatests/net/openid/appauth/AuthorizationServiceTest.java b/library/javatests/net/openid/appauth/AuthorizationServiceTest.java index bfda6b87..d845d7ac 100644 --- a/library/javatests/net/openid/appauth/AuthorizationServiceTest.java +++ b/library/javatests/net/openid/appauth/AuthorizationServiceTest.java @@ -63,6 +63,7 @@ import static net.openid.appauth.TestValues.TEST_CLIENT_SECRET; import static net.openid.appauth.TestValues.TEST_CLIENT_SECRET_EXPIRES_AT; import static net.openid.appauth.TestValues.TEST_ID_TOKEN; +import static net.openid.appauth.TestValues.TEST_NONCE; import static net.openid.appauth.TestValues.TEST_REFRESH_TOKEN; import static net.openid.appauth.TestValues.TEST_STATE; import static net.openid.appauth.TestValues.getTestAuthCodeExchangeRequest; @@ -160,7 +161,18 @@ public void testAuthorizationRequest_withSpecifiedState() throws Exception { } @Test - public void testAuthorizationRequest_withDefaultRandomState() throws Exception { + public void testAuthorizationRequest_withSpecifiedNonce() throws Exception { + AuthorizationRequest request = getTestAuthRequestBuilder() + .setNonce(TEST_NONCE) + .build(); + mService.performAuthorizationRequest(request, mPendingIntent); + Intent intent = captureAuthRequestIntent(); + assertRequestIntent(intent, null); + assertEquals(request.toUri().toString(), intent.getData().toString()); + } + + @Test + public void testAuthorizationRequest_withDefaultRandomStateAndNonce() throws Exception { AuthorizationRequest request = getTestAuthRequestBuilder().build(); mService.performAuthorizationRequest(request, mPendingIntent); Intent intent = captureAuthRequestIntent(); diff --git a/library/javatests/net/openid/appauth/TestValues.java b/library/javatests/net/openid/appauth/TestValues.java index c888e6ad..5805d027 100644 --- a/library/javatests/net/openid/appauth/TestValues.java +++ b/library/javatests/net/openid/appauth/TestValues.java @@ -25,6 +25,7 @@ class TestValues { public static final String TEST_CLIENT_ID = "test_client_id"; public static final String TEST_STATE = "$TAT3"; + public static final String TEST_NONCE = "NONC3"; public static final String TEST_APP_SCHEME = "com.test.app"; public static final Uri TEST_APP_REDIRECT_URI = Uri.parse(TEST_APP_SCHEME + ":/oidc_callback"); public static final String TEST_SCOPE = "openid email"; From c73985aad0c1eadd67ad843fe022cd32b118a3f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98yvind=20Robertsen?= Date: Tue, 24 Jul 2018 11:01:43 +0200 Subject: [PATCH 03/13] feat: include nonce when serializing AuthorizationRequests --- .../openid/appauth/AuthorizationRequest.java | 7 ++++++ .../appauth/AuthorizationRequestTest.java | 23 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/library/java/net/openid/appauth/AuthorizationRequest.java b/library/java/net/openid/appauth/AuthorizationRequest.java index ce1b26e1..9eb8135a 100644 --- a/library/java/net/openid/appauth/AuthorizationRequest.java +++ b/library/java/net/openid/appauth/AuthorizationRequest.java @@ -302,6 +302,9 @@ public static final class ResponseMode { @VisibleForTesting static final String PARAM_STATE = "state"; + @VisibleForTesting + static final String PARAM_NONCE = "nonce"; + private static final Set BUILT_IN_PARAMS = builtInParams( PARAM_CLIENT_ID, PARAM_CODE_CHALLENGE, @@ -324,6 +327,7 @@ public static final class ResponseMode { private static final String KEY_REDIRECT_URI = "redirectUri"; private static final String KEY_SCOPE = "scope"; private static final String KEY_STATE = "state"; + private static final String KEY_NONCE = "nonce"; private static final String KEY_CODE_VERIFIER = "codeVerifier"; private static final String KEY_CODE_VERIFIER_CHALLENGE = "codeVerifierChallenge"; private static final String KEY_CODE_VERIFIER_CHALLENGE_METHOD = "codeVerifierChallengeMethod"; @@ -978,6 +982,7 @@ public Uri toUri() { UriUtil.appendQueryParameterIfNotNull(uriBuilder, PARAM_LOGIN_HINT, loginHint); UriUtil.appendQueryParameterIfNotNull(uriBuilder, PARAM_PROMPT, prompt); UriUtil.appendQueryParameterIfNotNull(uriBuilder, PARAM_STATE, state); + UriUtil.appendQueryParameterIfNotNull(uriBuilder, PARAM_NONCE, nonce); UriUtil.appendQueryParameterIfNotNull(uriBuilder, PARAM_SCOPE, scope); UriUtil.appendQueryParameterIfNotNull(uriBuilder, PARAM_RESPONSE_MODE, responseMode); @@ -1009,6 +1014,7 @@ public JSONObject jsonSerialize() { JsonUtil.putIfNotNull(json, KEY_SCOPE, scope); JsonUtil.putIfNotNull(json, KEY_PROMPT, prompt); JsonUtil.putIfNotNull(json, KEY_STATE, state); + JsonUtil.putIfNotNull(json, KEY_NONCE, nonce); JsonUtil.putIfNotNull(json, KEY_CODE_VERIFIER, codeVerifier); JsonUtil.putIfNotNull(json, KEY_CODE_VERIFIER_CHALLENGE, codeVerifierChallenge); JsonUtil.putIfNotNull(json, KEY_CODE_VERIFIER_CHALLENGE_METHOD, @@ -1046,6 +1052,7 @@ public static AuthorizationRequest jsonDeserialize(@NonNull JSONObject json) .setLoginHint(JsonUtil.getStringIfDefined(json, KEY_LOGIN_HINT)) .setPrompt(JsonUtil.getStringIfDefined(json, KEY_PROMPT)) .setState(JsonUtil.getStringIfDefined(json, KEY_STATE)) + .setNonce(JsonUtil.getStringIfDefined(json, KEY_NONCE)) .setCodeVerifier( JsonUtil.getStringIfDefined(json, KEY_CODE_VERIFIER), JsonUtil.getStringIfDefined(json, KEY_CODE_VERIFIER_CHALLENGE), diff --git a/library/javatests/net/openid/appauth/AuthorizationRequestTest.java b/library/javatests/net/openid/appauth/AuthorizationRequestTest.java index 0b8abdf0..9e85a289 100644 --- a/library/javatests/net/openid/appauth/AuthorizationRequestTest.java +++ b/library/javatests/net/openid/appauth/AuthorizationRequestTest.java @@ -17,6 +17,7 @@ import static net.openid.appauth.TestValues.TEST_APP_REDIRECT_URI; import static net.openid.appauth.TestValues.TEST_CLIENT_ID; import static net.openid.appauth.TestValues.TEST_EMAIL_ADDRESS; +import static net.openid.appauth.TestValues.TEST_NONCE; import static net.openid.appauth.TestValues.TEST_STATE; import static net.openid.appauth.TestValues.getTestServiceConfig; import static org.assertj.core.api.Assertions.assertThat; @@ -459,6 +460,7 @@ public void testToUri() throws Exception { AuthorizationRequest.PARAM_RESPONSE_TYPE, AuthorizationRequest.PARAM_REDIRECT_URI, AuthorizationRequest.PARAM_STATE, + AuthorizationRequest.PARAM_NONCE, AuthorizationRequest.PARAM_CODE_CHALLENGE, AuthorizationRequest.PARAM_CODE_CHALLENGE_METHOD))); @@ -470,6 +472,8 @@ public void testToUri() throws Exception { .isEqualTo(TEST_APP_REDIRECT_URI.toString()); assertThat(uri.getQueryParameter(AuthorizationRequest.PARAM_STATE)) .isEqualTo(request.state); + assertThat(uri.getQueryParameter(AuthorizationRequest.PARAM_NONCE)) + .isEqualTo(request.nonce); assertThat(uri.getQueryParameter(AuthorizationRequest.PARAM_CODE_CHALLENGE)) .isEqualTo(request.codeVerifierChallenge); assertThat(uri.getQueryParameter(AuthorizationRequest.PARAM_CODE_CHALLENGE_METHOD)) @@ -565,6 +569,25 @@ public void testToUri_noStateParam() throws Exception { .doesNotContain(AuthorizationRequest.PARAM_STATE); } + @Test + public void testToUri_nonceParam() { + Uri uri = mRequestBuilder + .setNonce(TEST_NONCE) + .build() + .toUri(); + assertThat(uri.getQueryParameterNames()) + .contains(AuthorizationRequest.PARAM_NONCE); + assertThat(uri.getQueryParameter(AuthorizationRequest.PARAM_NONCE)) + .isEqualTo(TEST_NONCE); + } + + @Test + public void testToUri_noNonceParam() throws Exception { + AuthorizationRequest req = mRequestBuilder.setNonce(null).build(); + assertThat(req.toUri().getQueryParameterNames()) + .doesNotContain(AuthorizationRequest.PARAM_NONCE); + } + @Test public void testToUri_additionalParams() throws Exception { Map additionalParams = new HashMap<>(); From 9f117d4aa9fbfb2bab0755ce4d2462aae7b27b22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98yvind=20Robertsen?= Date: Wed, 25 Jul 2018 11:03:54 +0200 Subject: [PATCH 04/13] feat: parse and validate ID Token on TokenResponse Validate ID Tokens (if any) on TokenResponses according to http://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation --- .../appauth/AuthorizationException.java | 12 +++ .../openid/appauth/AuthorizationResponse.java | 1 + .../openid/appauth/AuthorizationService.java | 97 +++++++++++++++++++ library/java/net/openid/appauth/IDToken.java | 89 +++++++++++++++++ .../java/net/openid/appauth/TokenRequest.java | 31 +++++- .../appauth/AuthorizationRequestTest.java | 7 ++ .../AuthorizationServiceDiscoveryTest.java | 6 +- .../net/openid/appauth/IDTokenTest.java | 92 ++++++++++++++++++ .../net/openid/appauth/TestValues.java | 40 ++++++-- 9 files changed, 366 insertions(+), 9 deletions(-) create mode 100644 library/java/net/openid/appauth/IDToken.java create mode 100644 library/javatests/net/openid/appauth/IDTokenTest.java diff --git a/library/java/net/openid/appauth/AuthorizationException.java b/library/java/net/openid/appauth/AuthorizationException.java index e84d3fc5..b513c5b4 100644 --- a/library/java/net/openid/appauth/AuthorizationException.java +++ b/library/java/net/openid/appauth/AuthorizationException.java @@ -200,6 +200,18 @@ public static final class GeneralErrors { */ public static final AuthorizationException INVALID_REGISTRATION_RESPONSE = generalEx(7, "Invalid registration response"); + + /** + * ID token could not be parsed + */ + public static final AuthorizationException ID_TOKEN_PARSING_ERROR = + generalEx(8, "Unable to parse ID Token"); + + /** + * ID token is invalid + */ + public static final AuthorizationException ID_TOKEN_VALIDATION_ERROR = + generalEx(9, "Invalid ID Token"); } /** diff --git a/library/java/net/openid/appauth/AuthorizationResponse.java b/library/java/net/openid/appauth/AuthorizationResponse.java index 464d1735..84aa6bd0 100644 --- a/library/java/net/openid/appauth/AuthorizationResponse.java +++ b/library/java/net/openid/appauth/AuthorizationResponse.java @@ -466,6 +466,7 @@ public TokenRequest createTokenExchangeRequest( .setCodeVerifier(request.codeVerifier) .setAuthorizationCode(authorizationCode) .setAdditionalParameters(additionalExchangeParameters) + .setNonce(request.nonce) .build(); } diff --git a/library/java/net/openid/appauth/AuthorizationService.java b/library/java/net/openid/appauth/AuthorizationService.java index 5b9d90fd..52127545 100644 --- a/library/java/net/openid/appauth/AuthorizationService.java +++ b/library/java/net/openid/appauth/AuthorizationService.java @@ -14,6 +14,7 @@ package net.openid.appauth; +import static net.openid.appauth.Preconditions.checkNotEmpty; import static net.openid.appauth.Preconditions.checkNotNull; import android.annotation.TargetApi; @@ -315,6 +316,7 @@ public void performTokenRequest( request, clientAuthentication, mClientConfiguration.getConnectionBuilder(), + SystemClock.INSTANCE, callback) .execute(); } @@ -390,16 +392,19 @@ private static class TokenRequestTask private ClientAuthentication mClientAuthentication; private final ConnectionBuilder mConnectionBuilder; private TokenResponseCallback mCallback; + private Clock mClock; private AuthorizationException mException; TokenRequestTask(TokenRequest request, @NonNull ClientAuthentication clientAuthentication, @NonNull ConnectionBuilder connectionBuilder, + Clock clock, TokenResponseCallback callback) { mRequest = request; mClientAuthentication = clientAuthentication; mConnectionBuilder = connectionBuilder; + mClock = clock; mCallback = callback; } @@ -495,6 +500,98 @@ protected void onPostExecute(JSONObject json) { return; } + if (response.idToken != null) { + IDToken idToken; + try { + idToken = IDToken.from(response.idToken); + } catch (IDToken.IDTokenException | JSONException ex) { + mCallback.onTokenRequestCompleted(null, + AuthorizationException.fromTemplate(GeneralErrors.ID_TOKEN_PARSING_ERROR, ex)); + return; + } + + // OpenID Connect Core Section 3.1.3.7. rule #1 + // Not supported: AppAuth does not support JWT encryption. + + // OpenID Connect Core Section 3.1.3.7. rule #2 + // Validates that the issuer in the ID Token matches that of the discovery document. + + AuthorizationServiceDiscovery discoveryDoc = mRequest.configuration.discoveryDoc; + if (discoveryDoc == null) { + mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( + GeneralErrors.ID_TOKEN_VALIDATION_ERROR, + new NullPointerException("Missing discovery document"))); + return; + } + + String expectedIssuer = discoveryDoc.getIssuer(); + if (!idToken.issuer.equals(expectedIssuer)) { + mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( + GeneralErrors.ID_TOKEN_VALIDATION_ERROR, + new IDToken.IDTokenException("Issuer mismatch"))); + return; + } + + // OpenID Connect Core Section 3.1.3.7. rule #3 + // Validates that the audience of the ID Token matches the client ID. + String clientId = mRequest.clientId; + if (!idToken.audience.contains(clientId)) { + mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( + GeneralErrors.ID_TOKEN_VALIDATION_ERROR, + new IDToken.IDTokenException("Audience mismatch"))); + return; + } + + // OpenID Connect Core Section 3.1.3.7. rules #4 & #5 + // Not supported. + + // OpenID Connect Core Section 3.1.3.7. rule #6 + // As noted above, AppAuth only supports the code flow which results in direct communication + // of the ID Token from the Token Endpoint to the Client, and we are exercising the option to + // use TSL server validation instead of checking the token signature. Users may additionally + // check the token signature should they wish. + + // OpenID Connect Core Section 3.1.3.7. rules #7 & #8 + // Not applicable. See rule #6. + + // OpenID Connect Core Section 3.1.3.7. rule #9 + // Validates that the current time is before the expiry time. + if (mClock.getCurrentTimeMillis() > idToken.expiration) { + mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( + GeneralErrors.ID_TOKEN_VALIDATION_ERROR, + new IDToken.IDTokenException("ID Token expired"))); + return; + } + + // 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. + Long tenMinutesMillis = (long) (10 * 60 * 1000); + if (Math.abs(mClock.getCurrentTimeMillis() - idToken.issuedAt) > tenMinutesMillis) { + mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( + GeneralErrors.ID_TOKEN_VALIDATION_ERROR, + new IDToken.IDTokenException( + "Issued at time is more than 10 minutes before or after the current time"))); + return; + } + + // Only relevant for the authorization_code response type + if (GrantTypeValues.AUTHORIZATION_CODE.equals(mRequest.grantType)) { + // OpenID Connect Core Section 3.1.3.7. rule #11 + // Validates the nonce. + String expectedNonce = mRequest.nonce; + if(expectedNonce != null && (idToken.nonce != null && idToken.nonce.equals(expectedNonce))) { + mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( + GeneralErrors.ID_TOKEN_VALIDATION_ERROR, + new IDToken.IDTokenException("Nonce mismatch"))); + return; + } + } + // OpenID Connect Core Section 3.1.3.7. rules #12 + // ACR is not directly supported by AppAuth. + + // OpenID Connect Core Section 3.1.3.7. rules #12 + // max_age is not directly supported by AppAuth. + } Logger.debug("Token exchange with %s completed", mRequest.configuration.tokenEndpoint); mCallback.onTokenRequestCompleted(response, null); diff --git a/library/java/net/openid/appauth/IDToken.java b/library/java/net/openid/appauth/IDToken.java new file mode 100644 index 00000000..54864942 --- /dev/null +++ b/library/java/net/openid/appauth/IDToken.java @@ -0,0 +1,89 @@ +package net.openid.appauth; + +import android.support.annotation.NonNull; +import android.support.annotation.Nullable; +import android.util.Base64; + +import org.json.JSONException; +import org.json.JSONObject; + +import java.util.ArrayList; +import java.util.Date; +import java.util.List; + +class IDToken { + + private static final String KEY_ISSUER = "iss"; + private static final String KEY_SUBJECT = "sub"; + private static final String KEY_AUDIENCE = "aud"; + private static final String KEY_EXPIRATION = "exp"; + private static final String KEY_ISSUED_AT = "iat"; + private static final String KEY_NONCE = "nonce"; + + final String issuer; + final String subject; + final List audience; + final Long expiration; + final Long issuedAt; + final String nonce; + + IDToken( + @NonNull String issuer, + @NonNull String subject, + @NonNull List audience, + @NonNull Long expiration, + @NonNull Long issuedAt, + @Nullable String nonce) { + this.issuer = issuer; + this.subject = subject; + this.audience = audience; + this.expiration = expiration; + this.issuedAt = issuedAt; + this.nonce = nonce; + } + + static JSONObject parseJWTSection(String section) throws JSONException { + byte[] decodedSection = Base64.decode(section,Base64.URL_SAFE); + String jsonString = new String(decodedSection); + return new JSONObject(jsonString); + } + + static IDToken from(String token) throws JSONException, IDTokenException { + String[] sections = token.split("\\."); + + if (sections.length <= 1) { + throw new IDTokenException("ID token must have both header and claims section"); + } + + parseJWTSection(sections[0]); + JSONObject claims = parseJWTSection(sections[1]); + + String issuer = JsonUtil.getString(claims, KEY_ISSUER); + String subject = JsonUtil.getString(claims, KEY_SUBJECT); + List audience; + try { + audience = JsonUtil.getStringList(claims, KEY_AUDIENCE); + } catch (JSONException jsonEx) { + audience = new ArrayList<>(); + audience.add(JsonUtil.getString(claims, KEY_AUDIENCE)); + } + Long expiration = claims.getLong(KEY_EXPIRATION); + Long issuedAt = claims.getLong(KEY_ISSUED_AT); + String nonce = JsonUtil.getStringIfDefined(claims, KEY_NONCE); + + return new IDToken( + issuer, + subject, + audience, + expiration, + issuedAt, + nonce + ); + } + + static class IDTokenException extends Exception { + IDTokenException(String message) { + super(message); + } + } +} diff --git a/library/java/net/openid/appauth/TokenRequest.java b/library/java/net/openid/appauth/TokenRequest.java index 9af513ed..e86fcb0b 100644 --- a/library/java/net/openid/appauth/TokenRequest.java +++ b/library/java/net/openid/appauth/TokenRequest.java @@ -51,6 +51,8 @@ public class TokenRequest { @VisibleForTesting static final String KEY_CLIENT_ID = "clientId"; @VisibleForTesting + static final String KEY_NONCE = "nonce"; + @VisibleForTesting static final String KEY_GRANT_TYPE = "grantType"; @VisibleForTesting static final String KEY_REDIRECT_URI = "redirectUri"; @@ -125,6 +127,12 @@ public class TokenRequest { @NonNull public final AuthorizationServiceConfiguration configuration; + /** + * The (optional) nonce associated with the current session. + */ + @Nullable + public final String nonce; + /** * The client identifier. * @@ -214,6 +222,9 @@ public static final class Builder { @NonNull private String mClientId; + @Nullable + private String mNonce; + @Nullable private String mGrantType; @@ -265,6 +276,19 @@ public Builder setClientId(@NonNull String clientId) { return this; } + /** + * Specifies the (optional) nonce for the current session. + */ + @NonNull + public Builder setNonce(@Nullable String nonce) { + if (TextUtils.isEmpty(nonce)) { + mNonce = null; + } else { + this.mNonce = nonce; + } + return this; + } + /** * Specifies the grant type for the request, which must not be null or empty. */ @@ -428,6 +452,7 @@ public TokenRequest build() { return new TokenRequest( mConfiguration, mClientId, + mNonce, grantType, mRedirectUri, mScope, @@ -453,6 +478,7 @@ private String inferGrantType() { private TokenRequest( @NonNull AuthorizationServiceConfiguration configuration, @NonNull String clientId, + @Nullable String nonce, @NonNull String grantType, @Nullable Uri redirectUri, @Nullable String scope, @@ -462,6 +488,7 @@ private TokenRequest( @NonNull Map additionalParameters) { this.configuration = configuration; this.clientId = clientId; + this.nonce = nonce; this.grantType = grantType; this.redirectUri = redirectUri; this.scope = scope; @@ -517,6 +544,7 @@ public JSONObject jsonSerialize() { JSONObject json = new JSONObject(); JsonUtil.put(json, KEY_CONFIGURATION, configuration.toJson()); JsonUtil.put(json, KEY_CLIENT_ID, clientId); + JsonUtil.putIfNotNull(json, KEY_NONCE, nonce); JsonUtil.put(json, KEY_GRANT_TYPE, grantType); JsonUtil.putIfNotNull(json, KEY_REDIRECT_URI, redirectUri); JsonUtil.putIfNotNull(json, KEY_SCOPE, scope); @@ -553,7 +581,8 @@ public static TokenRequest jsonDeserialize(JSONObject json) throws JSONException .setGrantType(JsonUtil.getString(json, KEY_GRANT_TYPE)) .setRefreshToken(JsonUtil.getStringIfDefined(json, KEY_REFRESH_TOKEN)) .setAuthorizationCode(JsonUtil.getStringIfDefined(json, KEY_AUTHORIZATION_CODE)) - .setAdditionalParameters(JsonUtil.getStringMap(json, KEY_ADDITIONAL_PARAMETERS)); + .setAdditionalParameters(JsonUtil.getStringMap(json, KEY_ADDITIONAL_PARAMETERS)) + .setNonce(JsonUtil.getStringIfDefined(json, KEY_NONCE)); if (json.has(KEY_SCOPE)) { builder.setScopes(AsciiStringListUtil.stringToSet(JsonUtil.getString(json, KEY_SCOPE))); diff --git a/library/javatests/net/openid/appauth/AuthorizationRequestTest.java b/library/javatests/net/openid/appauth/AuthorizationRequestTest.java index 9e85a289..b45e7698 100644 --- a/library/javatests/net/openid/appauth/AuthorizationRequestTest.java +++ b/library/javatests/net/openid/appauth/AuthorizationRequestTest.java @@ -685,6 +685,13 @@ public void testJsonSerialize_state() throws Exception { assertThat(copy.state).isEqualTo(TEST_STATE); } + @Test + public void testJsonSerialize_nonce() throws Exception { + AuthorizationRequest copy = serializeDeserialize( + mRequestBuilder.setNonce(TEST_NONCE).build()); + assertThat(copy.nonce).isEqualTo(TEST_NONCE); + } + @Test public void testJsonSerialize_additionalParams() throws Exception { AuthorizationRequest copy = serializeDeserialize( diff --git a/library/javatests/net/openid/appauth/AuthorizationServiceDiscoveryTest.java b/library/javatests/net/openid/appauth/AuthorizationServiceDiscoveryTest.java index ee6a84d8..df4b8999 100644 --- a/library/javatests/net/openid/appauth/AuthorizationServiceDiscoveryTest.java +++ b/library/javatests/net/openid/appauth/AuthorizationServiceDiscoveryTest.java @@ -33,10 +33,11 @@ @Config(constants = BuildConfig.class, sdk=16) public class AuthorizationServiceDiscoveryTest { // ToDo: add more tests for remaining getters - private static final String TEST_ISSUER = "test_issuer"; + static final String TEST_ISSUER = "test_issuer"; private static final String TEST_AUTHORIZATION_ENDPOINT = "http://test.openid.com/o/oauth/auth"; private static final String TEST_TOKEN_ENDPOINT = "http://test.openid.com/o/oauth/token"; private static final String TEST_USERINFO_ENDPOINT = "http://test.openid.com/o/oauth/userinfo"; + private static final String TEST_REGISTRATION_ENDPOINT = "http://test.openid.com/o/oauth/register"; private static final String TEST_JWKS_URI = "http://test.openid.com/o/oauth/jwks"; private static final List TEST_RESPONSE_TYPE_SUPPORTED = Arrays.asList("code", "token"); private static final List TEST_SUBJECT_TYPES_SUPPORTED = Arrays.asList("public"); @@ -46,11 +47,12 @@ public class AuthorizationServiceDiscoveryTest { = Arrays.asList("client_secret_post", "client_secret_basic"); private static final List TEST_CLAIMS_SUPPORTED = Arrays.asList("aud", "exp"); - private static final String TEST_JSON = "{\n" + static final String TEST_JSON = "{\n" + " \"issuer\": \"" + TEST_ISSUER + "\",\n" + " \"authorization_endpoint\": \"" + TEST_AUTHORIZATION_ENDPOINT + "\",\n" + " \"token_endpoint\": \"" + TEST_TOKEN_ENDPOINT + "\",\n" + " \"userinfo_endpoint\": \"" + TEST_USERINFO_ENDPOINT + "\",\n" + + " \"registration_endpoint\": \"" + TEST_REGISTRATION_ENDPOINT + "\",\n" + " \"jwks_uri\": \"" + TEST_JWKS_URI + "\",\n" + " \"response_types_supported\": " + toJson(TEST_RESPONSE_TYPE_SUPPORTED) + ",\n" + " \"subject_types_supported\": " + toJson(TEST_SUBJECT_TYPES_SUPPORTED) + ",\n" diff --git a/library/javatests/net/openid/appauth/IDTokenTest.java b/library/javatests/net/openid/appauth/IDTokenTest.java new file mode 100644 index 00000000..fadc73df --- /dev/null +++ b/library/javatests/net/openid/appauth/IDTokenTest.java @@ -0,0 +1,92 @@ +package net.openid.appauth; + +import android.support.annotation.NonNull; +import android.support.annotation.Nullable; +import android.util.Base64; + +import org.json.JSONException; +import org.json.JSONObject; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; + +@RunWith(RobolectricTestRunner.class) +@Config(constants = BuildConfig.class, sdk=16) +public class IDTokenTest { + + static final String TEST_ID_TOKEN = "eyJhbGciOiJSUzI1NiIsImtpZCI6IjFlOWdkazcifQ.ewogIml" + + "zcyI6ICJodHRwOi8vc2VydmVyLmV4YW1wbGUuY29tIiwKICJzdWIiOiAiMjQ4Mjg5NzYxMDAxIiwKICJhdWQiOiAiczZ" + + "CaGRSa3F0MyIsCiAibm9uY2UiOiAibi0wUzZfV3pBMk1qIiwKICJleHAiOiAxMzExMjgxOTcwLAogImlhdCI6IDEzMTE" + + "yODA5NzAKfQ.ggW8hZ1EuVLuxNuuIJKX_V8a_OMXzR0EHR9R6jgdqrOOF4daGU96Sr_P6qJp6IcmD3HP99Obi1PRs-cw" + + "h3LO-p146waJ8IhehcwL7F09JdijmBqkvPeB2T9CJ NqeGpe-gccMg4vfKjkM8FcGvnzZUN4_KSP0aAp1tOJ1zZwgjxq" + + "GByKHiOtX7TpdQyHE5lcMiKPXfEIQILVq0pc_E2DzL7emopWoaoZTF_m0_N0YzFC6g6EJbOEoRoSK5hoDalrcvRYLSrQ" + + "AZZKflyuVCyixEoV9GfNQC3_osjzw2PAithfubEEBLuVVk4XUVrWOLrLl0nx7RkKU8NXNHq-rvKMzqg"; + static final String TEST_SUBJECT = "SUBJ3CT"; + + + @Test + public void testFrom() throws Exception { + IDToken idToken = IDToken.from(TEST_ID_TOKEN); + } + + @Test(expected = IDToken.IDTokenException.class) + public void testFrom_shouldFailOnMissingSection() throws IDToken.IDTokenException, JSONException { + IDToken.from("header."); + } + + @Test(expected = JSONException.class) + public void testFrom_shouldFailOnMalformedInput() throws IDToken.IDTokenException, JSONException { + IDToken.from("header.claims"); + } + + @Test(expected = JSONException.class) + public void testFrom_shouldFailOnMissingIssuer() throws IDToken.IDTokenException, JSONException { + + IDToken.from("header.claims"); + } + + private static String base64UrlNoPaddingEncode(byte[] data) { + return Base64.encodeToString(data, Base64.URL_SAFE | Base64.NO_PADDING | Base64.NO_WRAP); + } + + static String getUnsignedIdToken( + @Nullable String issuer, + @Nullable String subject, + @Nullable String audience, + @Nullable String nonce) { + Long now = SystemClock.INSTANCE.getCurrentTimeMillis(); + Long tenMinutesInMillis = (long) (10 * 60 * 1000); + return getUnsignedIdToken( + issuer, + subject, + audience, + now + tenMinutesInMillis, + now, + nonce); + } + + static String getUnsignedIdToken( + @Nullable String issuer, + @Nullable String subject, + @Nullable String audience, + @Nullable Long expiration, + @Nullable Long issuedAt, + @Nullable String nonce) { + JSONObject header = new JSONObject(); + JsonUtil.put(header, "typ", "JWT"); + + JSONObject claims = new JSONObject(); + JsonUtil.putIfNotNull(claims, "iss", issuer); + JsonUtil.putIfNotNull(claims, "sub", subject); + JsonUtil.putIfNotNull(claims, "aud", audience); + JsonUtil.putIfNotNull(claims, "exp", expiration != null ? String.valueOf(expiration) : null); + JsonUtil.putIfNotNull(claims, "iat", issuedAt != null ? String.valueOf(issuedAt) : null); + JsonUtil.putIfNotNull(claims, "nonce", nonce); + + + String encodedHeader = base64UrlNoPaddingEncode(header.toString().getBytes()); + String encodedClaims = base64UrlNoPaddingEncode(claims.toString().getBytes()); + return encodedHeader + "." + encodedClaims; + } +} diff --git a/library/javatests/net/openid/appauth/TestValues.java b/library/javatests/net/openid/appauth/TestValues.java index 5805d027..63dbbef5 100644 --- a/library/javatests/net/openid/appauth/TestValues.java +++ b/library/javatests/net/openid/appauth/TestValues.java @@ -16,8 +16,14 @@ import android.net.Uri; +import org.json.JSONException; +import org.json.JSONObject; + import java.util.Arrays; +import static net.openid.appauth.AuthorizationServiceDiscoveryTest.TEST_ISSUER; +import static net.openid.appauth.IDTokenTest.TEST_SUBJECT; + /** * Contains common test values which are useful across all tests. */ @@ -40,7 +46,12 @@ class TestValues { public static final String TEST_AUTH_CODE = "zxcvbnmjk"; public static final String TEST_ACCESS_TOKEN = "aaabbbccc"; public static final Long TEST_ACCESS_TOKEN_EXPIRATION_TIME = 120000L; // two minutes - public static final String TEST_ID_TOKEN = "abc.def.ghi"; + public static final String TEST_ID_TOKEN = IDTokenTest.getUnsignedIdToken( + TEST_ISSUER, + TEST_SUBJECT, + TEST_CLIENT_ID, + null + ); public static final String TEST_REFRESH_TOKEN = "asdfghjkl"; public static final Long TEST_CLIENT_SECRET_EXPIRES_AT = 78L; @@ -48,11 +59,17 @@ class TestValues { public static final String TEST_EMAIL_ADDRESS = "test@example.com"; + public static AuthorizationServiceDiscovery getTestDiscoveryDocument() { + try { + return new AuthorizationServiceDiscovery( + new JSONObject(AuthorizationServiceDiscoveryTest.TEST_JSON)); + } catch (JSONException | AuthorizationServiceDiscovery.MissingArgumentException ex) { + throw new RuntimeException("Unable to create test authorization service discover document", ex); + } + } + public static AuthorizationServiceConfiguration getTestServiceConfig() { - return new AuthorizationServiceConfiguration( - TEST_IDP_AUTH_ENDPOINT, - TEST_IDP_TOKEN_ENDPOINT, - TEST_IDP_REGISTRATION_ENDPOINT); + return new AuthorizationServiceConfiguration(getTestDiscoveryDocument()); } public static AuthorizationRequest.Builder getMinimalAuthRequestBuilder(String responseType) { @@ -70,7 +87,9 @@ public static AuthorizationRequest.Builder getTestAuthRequestBuilder() { } public static AuthorizationRequest getTestAuthRequest() { - return getTestAuthRequestBuilder().build(); + return getTestAuthRequestBuilder(). + setNonce(null). + build(); } public static AuthorizationResponse.Builder getTestAuthResponseBuilder() { @@ -130,4 +149,13 @@ public static RegistrationResponse getTestRegistrationResponse() { .setClientSecretExpiresAt(TEST_CLIENT_SECRET_EXPIRES_AT) .build(); } + + public static String getTestIdTokenWithNonce(String nonce) { + return IDTokenTest.getUnsignedIdToken( + TEST_ISSUER, + TEST_SUBJECT, + TEST_CLIENT_ID, + nonce + ); + } } From a8503d27e6f015a80caa13e29b5459a2db532d65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98yvind=20Robertsen?= Date: Wed, 25 Jul 2018 13:24:14 +0200 Subject: [PATCH 05/13] fix: bugs in ID Token validation Validate exp and iat claims based on seconds. Invert nonce equality check. --- .../java/net/openid/appauth/AuthorizationService.java | 9 +++++---- library/javatests/net/openid/appauth/IDTokenTest.java | 8 ++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/library/java/net/openid/appauth/AuthorizationService.java b/library/java/net/openid/appauth/AuthorizationService.java index 52127545..be0b7983 100644 --- a/library/java/net/openid/appauth/AuthorizationService.java +++ b/library/java/net/openid/appauth/AuthorizationService.java @@ -556,7 +556,8 @@ protected void onPostExecute(JSONObject json) { // OpenID Connect Core Section 3.1.3.7. rule #9 // Validates that the current time is before the expiry time. - if (mClock.getCurrentTimeMillis() > idToken.expiration) { + Long nowInSeconds = mClock.getCurrentTimeMillis() / 1000; + if (nowInSeconds > idToken.expiration) { mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( GeneralErrors.ID_TOKEN_VALIDATION_ERROR, new IDToken.IDTokenException("ID Token expired"))); @@ -565,8 +566,8 @@ protected void onPostExecute(JSONObject json) { // 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. - Long tenMinutesMillis = (long) (10 * 60 * 1000); - if (Math.abs(mClock.getCurrentTimeMillis() - idToken.issuedAt) > tenMinutesMillis) { + Long tenMinutesInSeconds = (long) (10 * 60); + if (Math.abs(nowInSeconds - idToken.issuedAt) > tenMinutesInSeconds) { mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( GeneralErrors.ID_TOKEN_VALIDATION_ERROR, new IDToken.IDTokenException( @@ -579,7 +580,7 @@ protected void onPostExecute(JSONObject json) { // OpenID Connect Core Section 3.1.3.7. rule #11 // Validates the nonce. String expectedNonce = mRequest.nonce; - if(expectedNonce != null && (idToken.nonce != null && idToken.nonce.equals(expectedNonce))) { + if(expectedNonce != null && (idToken.nonce != null && !idToken.nonce.equals(expectedNonce))) { mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( GeneralErrors.ID_TOKEN_VALIDATION_ERROR, new IDToken.IDTokenException("Nonce mismatch"))); diff --git a/library/javatests/net/openid/appauth/IDTokenTest.java b/library/javatests/net/openid/appauth/IDTokenTest.java index fadc73df..2b3abde7 100644 --- a/library/javatests/net/openid/appauth/IDTokenTest.java +++ b/library/javatests/net/openid/appauth/IDTokenTest.java @@ -55,14 +55,14 @@ static String getUnsignedIdToken( @Nullable String subject, @Nullable String audience, @Nullable String nonce) { - Long now = SystemClock.INSTANCE.getCurrentTimeMillis(); - Long tenMinutesInMillis = (long) (10 * 60 * 1000); + Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000; + Long tenMinutesInSeconds = (long) (10 * 60); return getUnsignedIdToken( issuer, subject, audience, - now + tenMinutesInMillis, - now, + nowInSeconds + tenMinutesInSeconds, + nowInSeconds, nonce); } From fbb274b27fd737546e8f1ef39219831d6cd641c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98yvind=20Robertsen?= Date: Wed, 25 Jul 2018 15:03:29 +0200 Subject: [PATCH 06/13] fix: lint errors --- .../appauth/AuthorizationException.java | 4 +- .../openid/appauth/AuthorizationService.java | 65 +++++++++++-------- .../appauth/{IDToken.java => IdToken.java} | 58 ++++++++++------- .../{IDTokenTest.java => IdTokenTest.java} | 20 +++--- .../net/openid/appauth/TestValues.java | 6 +- 5 files changed, 88 insertions(+), 65 deletions(-) rename library/java/net/openid/appauth/{IDToken.java => IdToken.java} (55%) rename library/javatests/net/openid/appauth/{IDTokenTest.java => IdTokenTest.java} (83%) diff --git a/library/java/net/openid/appauth/AuthorizationException.java b/library/java/net/openid/appauth/AuthorizationException.java index b513c5b4..caba45be 100644 --- a/library/java/net/openid/appauth/AuthorizationException.java +++ b/library/java/net/openid/appauth/AuthorizationException.java @@ -205,13 +205,13 @@ public static final class GeneralErrors { * ID token could not be parsed */ public static final AuthorizationException ID_TOKEN_PARSING_ERROR = - generalEx(8, "Unable to parse ID Token"); + generalEx(8, "Unable to parse ID Token"); /** * ID token is invalid */ public static final AuthorizationException ID_TOKEN_VALIDATION_ERROR = - generalEx(9, "Invalid ID Token"); + generalEx(9, "Invalid ID Token"); } /** diff --git a/library/java/net/openid/appauth/AuthorizationService.java b/library/java/net/openid/appauth/AuthorizationService.java index be0b7983..97e1abd5 100644 --- a/library/java/net/openid/appauth/AuthorizationService.java +++ b/library/java/net/openid/appauth/AuthorizationService.java @@ -14,7 +14,6 @@ package net.openid.appauth; -import static net.openid.appauth.Preconditions.checkNotEmpty; import static net.openid.appauth.Preconditions.checkNotNull; import android.annotation.TargetApi; @@ -35,6 +34,7 @@ import net.openid.appauth.AuthorizationException.RegistrationRequestErrors; import net.openid.appauth.AuthorizationException.TokenRequestErrors; +import net.openid.appauth.IdToken.IdTokenException; import net.openid.appauth.browser.BrowserDescriptor; import net.openid.appauth.browser.BrowserSelector; import net.openid.appauth.browser.CustomTabManager; @@ -388,6 +388,10 @@ private Intent prepareAuthorizationRequestIntent( private static class TokenRequestTask extends AsyncTask { + + private static final Long MILLIS_PER_SECOND = 1000L; + private static final Long TEN_MINUTES_IN_SECONDS = 600L; + private TokenRequest mRequest; private ClientAuthentication mClientAuthentication; private final ConnectionBuilder mConnectionBuilder; @@ -501,12 +505,14 @@ protected void onPostExecute(JSONObject json) { } if (response.idToken != null) { - IDToken idToken; + IdToken idToken; try { - idToken = IDToken.from(response.idToken); - } catch (IDToken.IDTokenException | JSONException ex) { + idToken = IdToken.from(response.idToken); + } catch (IdTokenException | JSONException ex) { mCallback.onTokenRequestCompleted(null, - AuthorizationException.fromTemplate(GeneralErrors.ID_TOKEN_PARSING_ERROR, ex)); + AuthorizationException.fromTemplate( + GeneralErrors.ID_TOKEN_PARSING_ERROR, + ex)); return; } @@ -519,16 +525,16 @@ protected void onPostExecute(JSONObject json) { AuthorizationServiceDiscovery discoveryDoc = mRequest.configuration.discoveryDoc; if (discoveryDoc == null) { mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( - GeneralErrors.ID_TOKEN_VALIDATION_ERROR, - new NullPointerException("Missing discovery document"))); + GeneralErrors.ID_TOKEN_VALIDATION_ERROR, + new NullPointerException("Missing discovery document"))); return; } String expectedIssuer = discoveryDoc.getIssuer(); if (!idToken.issuer.equals(expectedIssuer)) { mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( - GeneralErrors.ID_TOKEN_VALIDATION_ERROR, - new IDToken.IDTokenException("Issuer mismatch"))); + GeneralErrors.ID_TOKEN_VALIDATION_ERROR, + new IdTokenException("Issuer mismatch"))); return; } @@ -537,8 +543,8 @@ protected void onPostExecute(JSONObject json) { String clientId = mRequest.clientId; if (!idToken.audience.contains(clientId)) { mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( - GeneralErrors.ID_TOKEN_VALIDATION_ERROR, - new IDToken.IDTokenException("Audience mismatch"))); + GeneralErrors.ID_TOKEN_VALIDATION_ERROR, + new IdTokenException("Audience mismatch"))); return; } @@ -546,32 +552,32 @@ protected void onPostExecute(JSONObject json) { // Not supported. // OpenID Connect Core Section 3.1.3.7. rule #6 - // As noted above, AppAuth only supports the code flow which results in direct communication - // of the ID Token from the Token Endpoint to the Client, and we are exercising the option to - // use TSL server validation instead of checking the token signature. Users may additionally - // check the token signature should they wish. + // As noted above, AppAuth only supports the code flow which results in direct + // communication of the ID Token from the Token Endpoint to the Client, and we are + // exercising the option to use TSL server validation instead of checking the token + // signature. Users may additionally check the token signature should they wish. // OpenID Connect Core Section 3.1.3.7. rules #7 & #8 // Not applicable. See rule #6. // OpenID Connect Core Section 3.1.3.7. rule #9 // Validates that the current time is before the expiry time. - Long nowInSeconds = mClock.getCurrentTimeMillis() / 1000; + Long nowInSeconds = mClock.getCurrentTimeMillis() / MILLIS_PER_SECOND; if (nowInSeconds > idToken.expiration) { mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( - GeneralErrors.ID_TOKEN_VALIDATION_ERROR, - new IDToken.IDTokenException("ID Token expired"))); + GeneralErrors.ID_TOKEN_VALIDATION_ERROR, + new IdTokenException("ID Token expired"))); return; } // 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. - Long tenMinutesInSeconds = (long) (10 * 60); - if (Math.abs(nowInSeconds - idToken.issuedAt) > tenMinutesInSeconds) { + // Validates that the issued at time is not more than +/- 10 minutes on the current + // time. + if (Math.abs(nowInSeconds - idToken.issuedAt) > TEN_MINUTES_IN_SECONDS) { mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( - GeneralErrors.ID_TOKEN_VALIDATION_ERROR, - new IDToken.IDTokenException( - "Issued at time is more than 10 minutes before or after the current time"))); + GeneralErrors.ID_TOKEN_VALIDATION_ERROR, + new IdTokenException("Issued at time is more than 10 minutes " + + "before or after the current time"))); return; } @@ -580,10 +586,10 @@ protected void onPostExecute(JSONObject json) { // OpenID Connect Core Section 3.1.3.7. rule #11 // Validates the nonce. String expectedNonce = mRequest.nonce; - if(expectedNonce != null && (idToken.nonce != null && !idToken.nonce.equals(expectedNonce))) { + if (verifyNonce(idToken.nonce, expectedNonce)) { mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( - GeneralErrors.ID_TOKEN_VALIDATION_ERROR, - new IDToken.IDTokenException("Nonce mismatch"))); + GeneralErrors.ID_TOKEN_VALIDATION_ERROR, + new IdTokenException("Nonce mismatch"))); return; } } @@ -598,6 +604,11 @@ protected void onPostExecute(JSONObject json) { mCallback.onTokenRequestCompleted(response, null); } + private boolean verifyNonce(String idTokenNonce, String expectedNonce) { + return expectedNonce != null + && (idTokenNonce != null && !idTokenNonce.equals(expectedNonce)); + } + /** * GitHub will only return a spec-compliant response if JSON is explicitly defined * as an acceptable response type. As this is essentially harmless for all other diff --git a/library/java/net/openid/appauth/IDToken.java b/library/java/net/openid/appauth/IdToken.java similarity index 55% rename from library/java/net/openid/appauth/IDToken.java rename to library/java/net/openid/appauth/IdToken.java index 54864942..8276234b 100644 --- a/library/java/net/openid/appauth/IDToken.java +++ b/library/java/net/openid/appauth/IdToken.java @@ -1,3 +1,17 @@ +/* + * Copyright 2015 The AppAuth for Android Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the + * License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing permissions and + * limitations under the License. + */ + package net.openid.appauth; import android.support.annotation.NonNull; @@ -8,10 +22,9 @@ import org.json.JSONObject; import java.util.ArrayList; -import java.util.Date; import java.util.List; -class IDToken { +class IdToken { private static final String KEY_ISSUER = "iss"; private static final String KEY_SUBJECT = "sub"; @@ -20,20 +33,19 @@ class IDToken { private static final String KEY_ISSUED_AT = "iat"; private static final String KEY_NONCE = "nonce"; - final String issuer; - final String subject; - final List audience; - final Long expiration; - final Long issuedAt; - final String nonce; + public final String issuer; + public final String subject; + public final List audience; + public final Long expiration; + public final Long issuedAt; + public final String nonce; - IDToken( - @NonNull String issuer, - @NonNull String subject, - @NonNull List audience, - @NonNull Long expiration, - @NonNull Long issuedAt, - @Nullable String nonce) { + IdToken(@NonNull String issuer, + @NonNull String subject, + @NonNull List audience, + @NonNull Long expiration, + @NonNull Long issuedAt, + @Nullable String nonce) { this.issuer = issuer; this.subject = subject; this.audience = audience; @@ -42,21 +54,21 @@ class IDToken { this.nonce = nonce; } - static JSONObject parseJWTSection(String section) throws JSONException { + private static JSONObject parseJwtSection(String section) throws JSONException { byte[] decodedSection = Base64.decode(section,Base64.URL_SAFE); String jsonString = new String(decodedSection); return new JSONObject(jsonString); } - static IDToken from(String token) throws JSONException, IDTokenException { + static IdToken from(String token) throws JSONException, IdTokenException { String[] sections = token.split("\\."); if (sections.length <= 1) { - throw new IDTokenException("ID token must have both header and claims section"); + throw new IdTokenException("ID token must have both header and claims section"); } - parseJWTSection(sections[0]); - JSONObject claims = parseJWTSection(sections[1]); + parseJwtSection(sections[0]); + JSONObject claims = parseJwtSection(sections[1]); String issuer = JsonUtil.getString(claims, KEY_ISSUER); String subject = JsonUtil.getString(claims, KEY_SUBJECT); @@ -71,7 +83,7 @@ static IDToken from(String token) throws JSONException, IDTokenException { Long issuedAt = claims.getLong(KEY_ISSUED_AT); String nonce = JsonUtil.getStringIfDefined(claims, KEY_NONCE); - return new IDToken( + return new IdToken( issuer, subject, audience, @@ -81,8 +93,8 @@ static IDToken from(String token) throws JSONException, IDTokenException { ); } - static class IDTokenException extends Exception { - IDTokenException(String message) { + static class IdTokenException extends Exception { + IdTokenException(String message) { super(message); } } diff --git a/library/javatests/net/openid/appauth/IDTokenTest.java b/library/javatests/net/openid/appauth/IdTokenTest.java similarity index 83% rename from library/javatests/net/openid/appauth/IDTokenTest.java rename to library/javatests/net/openid/appauth/IdTokenTest.java index 2b3abde7..52a11d60 100644 --- a/library/javatests/net/openid/appauth/IDTokenTest.java +++ b/library/javatests/net/openid/appauth/IdTokenTest.java @@ -1,9 +1,9 @@ package net.openid.appauth; -import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.util.Base64; +import net.openid.appauth.IdToken.IdTokenException; import org.json.JSONException; import org.json.JSONObject; import org.junit.Test; @@ -13,7 +13,7 @@ @RunWith(RobolectricTestRunner.class) @Config(constants = BuildConfig.class, sdk=16) -public class IDTokenTest { +public class IdTokenTest { static final String TEST_ID_TOKEN = "eyJhbGciOiJSUzI1NiIsImtpZCI6IjFlOWdkazcifQ.ewogIml" + "zcyI6ICJodHRwOi8vc2VydmVyLmV4YW1wbGUuY29tIiwKICJzdWIiOiAiMjQ4Mjg5NzYxMDAxIiwKICJhdWQiOiAiczZ" @@ -27,23 +27,23 @@ public class IDTokenTest { @Test public void testFrom() throws Exception { - IDToken idToken = IDToken.from(TEST_ID_TOKEN); + IdToken idToken = IdToken.from(TEST_ID_TOKEN); } - @Test(expected = IDToken.IDTokenException.class) - public void testFrom_shouldFailOnMissingSection() throws IDToken.IDTokenException, JSONException { - IDToken.from("header."); + @Test(expected = IdTokenException.class) + public void testFrom_shouldFailOnMissingSection() throws IdTokenException, JSONException { + IdToken.from("header."); } @Test(expected = JSONException.class) - public void testFrom_shouldFailOnMalformedInput() throws IDToken.IDTokenException, JSONException { - IDToken.from("header.claims"); + public void testFrom_shouldFailOnMalformedInput() throws IdTokenException, JSONException { + IdToken.from("header.claims"); } @Test(expected = JSONException.class) - public void testFrom_shouldFailOnMissingIssuer() throws IDToken.IDTokenException, JSONException { + public void testFrom_shouldFailOnMissingIssuer() throws IdTokenException, JSONException { - IDToken.from("header.claims"); + IdToken.from("header.claims"); } private static String base64UrlNoPaddingEncode(byte[] data) { diff --git a/library/javatests/net/openid/appauth/TestValues.java b/library/javatests/net/openid/appauth/TestValues.java index 63dbbef5..86445308 100644 --- a/library/javatests/net/openid/appauth/TestValues.java +++ b/library/javatests/net/openid/appauth/TestValues.java @@ -22,7 +22,7 @@ import java.util.Arrays; import static net.openid.appauth.AuthorizationServiceDiscoveryTest.TEST_ISSUER; -import static net.openid.appauth.IDTokenTest.TEST_SUBJECT; +import static net.openid.appauth.IdTokenTest.TEST_SUBJECT; /** * Contains common test values which are useful across all tests. @@ -46,7 +46,7 @@ class TestValues { public static final String TEST_AUTH_CODE = "zxcvbnmjk"; public static final String TEST_ACCESS_TOKEN = "aaabbbccc"; public static final Long TEST_ACCESS_TOKEN_EXPIRATION_TIME = 120000L; // two minutes - public static final String TEST_ID_TOKEN = IDTokenTest.getUnsignedIdToken( + public static final String TEST_ID_TOKEN = IdTokenTest.getUnsignedIdToken( TEST_ISSUER, TEST_SUBJECT, TEST_CLIENT_ID, @@ -151,7 +151,7 @@ public static RegistrationResponse getTestRegistrationResponse() { } public static String getTestIdTokenWithNonce(String nonce) { - return IDTokenTest.getUnsignedIdToken( + return IdTokenTest.getUnsignedIdToken( TEST_ISSUER, TEST_SUBJECT, TEST_CLIENT_ID, From 90e1ac4bed929e4a97dec84960e874aa75c484c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98yvind=20Robertsen?= Date: Wed, 25 Jul 2018 15:39:12 +0200 Subject: [PATCH 07/13] test: add test for token request with nonce checking --- .../appauth/AuthorizationServiceTest.java | 62 ++++++++++++++----- 1 file changed, 48 insertions(+), 14 deletions(-) diff --git a/library/javatests/net/openid/appauth/AuthorizationServiceTest.java b/library/javatests/net/openid/appauth/AuthorizationServiceTest.java index d845d7ac..1a98e5ff 100644 --- a/library/javatests/net/openid/appauth/AuthorizationServiceTest.java +++ b/library/javatests/net/openid/appauth/AuthorizationServiceTest.java @@ -67,7 +67,9 @@ import static net.openid.appauth.TestValues.TEST_REFRESH_TOKEN; import static net.openid.appauth.TestValues.TEST_STATE; import static net.openid.appauth.TestValues.getTestAuthCodeExchangeRequest; +import static net.openid.appauth.TestValues.getTestAuthCodeExchangeRequestBuilder; import static net.openid.appauth.TestValues.getTestAuthRequestBuilder; +import static net.openid.appauth.TestValues.getTestIdTokenWithNonce; import static net.openid.appauth.TestValues.getTestRegistrationRequest; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; @@ -90,14 +92,6 @@ public class AuthorizationServiceTest { private static final int TEST_EXPIRES_IN = 3600; private static final String TEST_BROWSER_PACKAGE = "com.browser.test"; - private static final String AUTH_CODE_EXCHANGE_RESPONSE_JSON = "{\n" - + " \"refresh_token\": \"" + TEST_REFRESH_TOKEN + "\",\n" - + " \"access_token\": \"" + TEST_ACCESS_TOKEN + "\",\n" - + " \"expires_in\": \"" + TEST_EXPIRES_IN + "\",\n" - + " \"id_token\": \"" + TEST_ID_TOKEN + "\",\n" - + " \"token_type\": \"" + AuthorizationResponse.TOKEN_TYPE_BEARER + "\"\n" - + "}"; - private static final String REGISTRATION_RESPONSE_JSON = "{\n" + " \"client_id\": \"" + TEST_CLIENT_ID + "\",\n" + " \"client_secret\": \"" + TEST_CLIENT_SECRET + "\",\n" @@ -234,7 +228,7 @@ public void testGetAuthorizationRequestIntent_withCustomTabs_preservesTabSetting @Test public void testTokenRequest() throws Exception { - InputStream is = new ByteArrayInputStream(AUTH_CODE_EXCHANGE_RESPONSE_JSON.getBytes()); + InputStream is = new ByteArrayInputStream(getAuthCodeExchangeResponseJson().getBytes()); when(mHttpConnection.getInputStream()).thenReturn(is); when(mHttpConnection.getRequestProperty("Accept")).thenReturn(null); when(mHttpConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_OK); @@ -257,9 +251,25 @@ public void testTokenRequest() throws Exception { assertThat(params).containsEntry(TokenRequest.PARAM_CLIENT_ID, request.clientId); } + @Test + public void testTokenRequest_withNonceValidation() throws Exception { + String idToken = getTestIdTokenWithNonce(TEST_NONCE); + InputStream is = new ByteArrayInputStream( + getAuthCodeExchangeResponseJson(idToken).getBytes()); + when(mHttpConnection.getInputStream()).thenReturn(is); + when(mHttpConnection.getRequestProperty("Accept")).thenReturn(null); + when(mHttpConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_OK); + TokenRequest request = getTestAuthCodeExchangeRequestBuilder() + .setNonce(TEST_NONCE) + .build(); + mService.performTokenRequest(request, mAuthCallback); + mAuthCallback.waitForCallback(); + assertTokenResponse(mAuthCallback.response, request, idToken); + } + @Test public void testTokenRequest_clientSecretBasicAuth() throws Exception { - InputStream is = new ByteArrayInputStream(AUTH_CODE_EXCHANGE_RESPONSE_JSON.getBytes()); + InputStream is = new ByteArrayInputStream(getAuthCodeExchangeResponseJson().getBytes()); when(mHttpConnection.getInputStream()).thenReturn(is); when(mHttpConnection.getRequestProperty("Accept")).thenReturn(null); when(mHttpConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_OK); @@ -280,7 +290,7 @@ public void testTokenRequest_clientSecretBasicAuth() throws Exception { @Test public void testTokenRequest_leaveExistingAcceptUntouched() throws Exception { - InputStream is = new ByteArrayInputStream(AUTH_CODE_EXCHANGE_RESPONSE_JSON.getBytes()); + InputStream is = new ByteArrayInputStream(getAuthCodeExchangeResponseJson().getBytes()); // emulate some content types having already been set as an Accept value when(mHttpConnection.getRequestProperty("Accept")) @@ -298,7 +308,7 @@ public void testTokenRequest_leaveExistingAcceptUntouched() throws Exception { @Test public void testTokenRequest_withBasicAuth() throws Exception { ClientSecretBasic csb = new ClientSecretBasic(TEST_CLIENT_SECRET); - InputStream is = new ByteArrayInputStream(AUTH_CODE_EXCHANGE_RESPONSE_JSON.getBytes()); + InputStream is = new ByteArrayInputStream(getAuthCodeExchangeResponseJson().getBytes()); when(mHttpConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_OK); when(mHttpConnection.getInputStream()).thenReturn(is); TokenRequest request = getTestAuthCodeExchangeRequest(); @@ -314,7 +324,7 @@ public void testTokenRequest_withBasicAuth() throws Exception { @Test public void testTokenRequest_withPostAuth() throws Exception { ClientSecretPost csp = new ClientSecretPost(TEST_CLIENT_SECRET); - InputStream is = new ByteArrayInputStream(AUTH_CODE_EXCHANGE_RESPONSE_JSON.getBytes()); + InputStream is = new ByteArrayInputStream(getAuthCodeExchangeResponseJson().getBytes()); when(mHttpConnection.getInputStream()).thenReturn(is); when(mHttpConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_OK); TokenRequest request = getTestAuthCodeExchangeRequest(); @@ -420,12 +430,19 @@ private Intent captureAuthRequestIntent() { } private void assertTokenResponse(TokenResponse response, TokenRequest expectedRequest) { + assertTokenResponse(response, expectedRequest, TEST_ID_TOKEN); + } + + private void assertTokenResponse( + TokenResponse response, + TokenRequest expectedRequest, + String idToken) { assertNotNull(response); assertEquals(expectedRequest, response.request); assertEquals(TEST_ACCESS_TOKEN, response.accessToken); assertEquals(TEST_REFRESH_TOKEN, response.refreshToken); assertEquals(AuthorizationResponse.TOKEN_TYPE_BEARER, response.tokenType); - assertEquals(TEST_ID_TOKEN, response.idToken); + assertEquals(idToken, response.idToken); } private void assertInvalidGrant(AuthorizationException error) { @@ -532,4 +549,21 @@ public boolean matches(Intent intent) { static Intent serviceIntentEq() { return argThat(new CustomTabsServiceMatcher()); } + + String getAuthCodeExchangeResponseJson() { + return getAuthCodeExchangeResponseJson(null); + } + + String getAuthCodeExchangeResponseJson(@Nullable String idToken) { + if (idToken == null) { + idToken = TEST_ID_TOKEN; + } + return "{\n" + + " \"refresh_token\": \"" + TEST_REFRESH_TOKEN + "\",\n" + + " \"access_token\": \"" + TEST_ACCESS_TOKEN + "\",\n" + + " \"expires_in\": \"" + TEST_EXPIRES_IN + "\",\n" + + " \"id_token\": \"" + idToken + "\",\n" + + " \"token_type\": \"" + AuthorizationResponse.TOKEN_TYPE_BEARER + "\"\n" + + "}"; + } } From 063e51ceac3a8f622c1e09b499605f8ab55b31fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98yvind=20Robertsen?= Date: Wed, 8 Aug 2018 15:39:14 +0200 Subject: [PATCH 08/13] chore: minor changes to comments according to PR feedback --- .../net/openid/appauth/AuthorizationException.java | 4 ++-- .../net/openid/appauth/AuthorizationRequest.java | 10 ++++++++++ .../net/openid/appauth/AuthorizationService.java | 12 ++++++------ library/java/net/openid/appauth/IdToken.java | 13 ++++++++++++- 4 files changed, 30 insertions(+), 9 deletions(-) diff --git a/library/java/net/openid/appauth/AuthorizationException.java b/library/java/net/openid/appauth/AuthorizationException.java index caba45be..77738e33 100644 --- a/library/java/net/openid/appauth/AuthorizationException.java +++ b/library/java/net/openid/appauth/AuthorizationException.java @@ -202,13 +202,13 @@ public static final class GeneralErrors { generalEx(7, "Invalid registration response"); /** - * ID token could not be parsed + * Indicates that a received ID token could not be parsed */ public static final AuthorizationException ID_TOKEN_PARSING_ERROR = generalEx(8, "Unable to parse ID Token"); /** - * ID token is invalid + * Indicates that a received ID token is invalid */ public static final AuthorizationException ID_TOKEN_VALIDATION_ERROR = generalEx(9, "Invalid ID Token"); diff --git a/library/java/net/openid/appauth/AuthorizationRequest.java b/library/java/net/openid/appauth/AuthorizationRequest.java index 9eb8135a..0f133eb8 100644 --- a/library/java/net/openid/appauth/AuthorizationRequest.java +++ b/library/java/net/openid/appauth/AuthorizationRequest.java @@ -784,6 +784,16 @@ public Builder setState(@Nullable String state) { return this; } + /** + * Specifies the String value used to associate a Client session with an ID Token, and to + * mitigate replay attacks. The value is passed through unmodified from the Authentication + * Request to the ID Token. If this value is not explicitly set, this library will + * automatically add nonce and perform appropriate validation of the ID Token. It is + * recommended that the default implementation of this parameter be used wherever possible. + * + * @see "OpenID Connect Core 1.0, Section 3.1.2.1 + * " + */ @NonNull public Builder setNonce(@Nullable String nonce) { mNonce = checkNullOrNotEmpty(nonce, "state cannot be empty if defined"); diff --git a/library/java/net/openid/appauth/AuthorizationService.java b/library/java/net/openid/appauth/AuthorizationService.java index 97e1abd5..71ca8275 100644 --- a/library/java/net/openid/appauth/AuthorizationService.java +++ b/library/java/net/openid/appauth/AuthorizationService.java @@ -1,5 +1,5 @@ /* - * Copyright 2015 The AppAuth for Android Authors. All Rights Reserved. + * Copyright 2018 The AppAuth for Android Authors. All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at @@ -517,7 +517,7 @@ protected void onPostExecute(JSONObject json) { } // OpenID Connect Core Section 3.1.3.7. rule #1 - // Not supported: AppAuth does not support JWT encryption. + // Not enforced: AppAuth does not support JWT encryption. // OpenID Connect Core Section 3.1.3.7. rule #2 // Validates that the issuer in the ID Token matches that of the discovery document. @@ -549,16 +549,16 @@ protected void onPostExecute(JSONObject json) { } // OpenID Connect Core Section 3.1.3.7. rules #4 & #5 - // Not supported. + // Not enforced. // OpenID Connect Core Section 3.1.3.7. rule #6 // As noted above, AppAuth only supports the code flow which results in direct // communication of the ID Token from the Token Endpoint to the Client, and we are - // exercising the option to use TSL server validation instead of checking the token + // exercising the option to use TLS server validation instead of checking the token // signature. Users may additionally check the token signature should they wish. // OpenID Connect Core Section 3.1.3.7. rules #7 & #8 - // Not applicable. See rule #6. + // Not enforced. See rule #6. // OpenID Connect Core Section 3.1.3.7. rule #9 // Validates that the current time is before the expiry time. @@ -586,7 +586,7 @@ protected void onPostExecute(JSONObject json) { // OpenID Connect Core Section 3.1.3.7. rule #11 // Validates the nonce. String expectedNonce = mRequest.nonce; - if (verifyNonce(idToken.nonce, expectedNonce)) { + if (TextUtils.equals(idToken.nonce, expectedNonce)) { mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( GeneralErrors.ID_TOKEN_VALIDATION_ERROR, new IdTokenException("Nonce mismatch"))); diff --git a/library/java/net/openid/appauth/IdToken.java b/library/java/net/openid/appauth/IdToken.java index 8276234b..d27c9b60 100644 --- a/library/java/net/openid/appauth/IdToken.java +++ b/library/java/net/openid/appauth/IdToken.java @@ -1,5 +1,5 @@ /* - * Copyright 2015 The AppAuth for Android Authors. All Rights Reserved. + * Copyright 2018 The AppAuth for Android Authors. All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at @@ -24,6 +24,16 @@ import java.util.ArrayList; import java.util.List; +/** + * An OpenID Connect ID Token. Contains claims about the authentication of an End-User by an + * Authorization Server. Supports parsing ID Tokens from JWT Compact Serializations and validation + * according to the OpenID Connect specification. + * + * @see "OpenID Connect Core ID Token, Section 2 + * " + * @see "OpenID Connect Core ID Token Validation, Section 3.1.3.7 + * " + */ class IdToken { private static final String KEY_ISSUER = "iss"; @@ -67,6 +77,7 @@ static IdToken from(String token) throws JSONException, IdTokenException { throw new IdTokenException("ID token must have both header and claims section"); } + // We ignore header contents, but parse it to check that it is structurally valid JSON parseJwtSection(sections[0]); JSONObject claims = parseJwtSection(sections[1]); From 9d9cf74ef0474556dc62ef73b2a179bdd0a1e9c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98yvind=20Robertsen?= Date: Wed, 8 Aug 2018 16:20:22 +0200 Subject: [PATCH 09/13] fix: correct test for nonce validation, improve test for ID Token parsing --- .../openid/appauth/AuthorizationService.java | 2 +- .../net/openid/appauth/IdTokenTest.java | 25 +++++++++++++------ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/library/java/net/openid/appauth/AuthorizationService.java b/library/java/net/openid/appauth/AuthorizationService.java index 71ca8275..3df70bfe 100644 --- a/library/java/net/openid/appauth/AuthorizationService.java +++ b/library/java/net/openid/appauth/AuthorizationService.java @@ -586,7 +586,7 @@ protected void onPostExecute(JSONObject json) { // OpenID Connect Core Section 3.1.3.7. rule #11 // Validates the nonce. String expectedNonce = mRequest.nonce; - if (TextUtils.equals(idToken.nonce, expectedNonce)) { + if (!TextUtils.equals(idToken.nonce, expectedNonce)) { mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( GeneralErrors.ID_TOKEN_VALIDATION_ERROR, new IdTokenException("Nonce mismatch"))); diff --git a/library/javatests/net/openid/appauth/IdTokenTest.java b/library/javatests/net/openid/appauth/IdTokenTest.java index 52a11d60..3996238f 100644 --- a/library/javatests/net/openid/appauth/IdTokenTest.java +++ b/library/javatests/net/openid/appauth/IdTokenTest.java @@ -11,23 +11,32 @@ import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; +import static net.openid.appauth.TestValues.TEST_NONCE; +import static org.hamcrest.Matchers.contains; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; + @RunWith(RobolectricTestRunner.class) @Config(constants = BuildConfig.class, sdk=16) public class IdTokenTest { - static final String TEST_ID_TOKEN = "eyJhbGciOiJSUzI1NiIsImtpZCI6IjFlOWdkazcifQ.ewogIml" - + "zcyI6ICJodHRwOi8vc2VydmVyLmV4YW1wbGUuY29tIiwKICJzdWIiOiAiMjQ4Mjg5NzYxMDAxIiwKICJhdWQiOiAiczZ" - + "CaGRSa3F0MyIsCiAibm9uY2UiOiAibi0wUzZfV3pBMk1qIiwKICJleHAiOiAxMzExMjgxOTcwLAogImlhdCI6IDEzMTE" - + "yODA5NzAKfQ.ggW8hZ1EuVLuxNuuIJKX_V8a_OMXzR0EHR9R6jgdqrOOF4daGU96Sr_P6qJp6IcmD3HP99Obi1PRs-cw" - + "h3LO-p146waJ8IhehcwL7F09JdijmBqkvPeB2T9CJ NqeGpe-gccMg4vfKjkM8FcGvnzZUN4_KSP0aAp1tOJ1zZwgjxq" - + "GByKHiOtX7TpdQyHE5lcMiKPXfEIQILVq0pc_E2DzL7emopWoaoZTF_m0_N0YzFC6g6EJbOEoRoSK5hoDalrcvRYLSrQ" - + "AZZKflyuVCyixEoV9GfNQC3_osjzw2PAithfubEEBLuVVk4XUVrWOLrLl0nx7RkKU8NXNHq-rvKMzqg"; + static final String TEST_ISSUER = "https://test.issuer"; static final String TEST_SUBJECT = "SUBJ3CT"; + static final String TEST_AUDIENCE = "AUDI3NCE"; @Test public void testFrom() throws Exception { - IdToken idToken = IdToken.from(TEST_ID_TOKEN); + String serializedIdToken = getUnsignedIdToken( + TEST_ISSUER, + TEST_SUBJECT, + TEST_AUDIENCE, + TEST_NONCE); + IdToken idToken = IdToken.from(serializedIdToken); + assertEquals(idToken.issuer, TEST_ISSUER); + assertEquals(idToken.subject, TEST_SUBJECT); + assertThat(idToken.audience, contains(TEST_AUDIENCE)); + assertEquals(idToken.nonce, TEST_NONCE); } @Test(expected = IdTokenException.class) From 91dc40394ec139cff8106c787e0fb070adc76af4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98yvind=20Robertsen?= Date: Wed, 8 Aug 2018 17:21:07 +0200 Subject: [PATCH 10/13] test: better coverage for ID Token parsing --- .../net/openid/appauth/IdTokenTest.java | 126 +++++++++++++++++- 1 file changed, 119 insertions(+), 7 deletions(-) diff --git a/library/javatests/net/openid/appauth/IdTokenTest.java b/library/javatests/net/openid/appauth/IdTokenTest.java index 3996238f..03bd8e91 100644 --- a/library/javatests/net/openid/appauth/IdTokenTest.java +++ b/library/javatests/net/openid/appauth/IdTokenTest.java @@ -3,7 +3,10 @@ import android.support.annotation.Nullable; import android.util.Base64; +import java.util.Arrays; +import java.util.List; import net.openid.appauth.IdToken.IdTokenException; +import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; import org.junit.Test; @@ -27,16 +30,33 @@ public class IdTokenTest { @Test public void testFrom() throws Exception { - String serializedIdToken = getUnsignedIdToken( + String testToken = getUnsignedIdToken( TEST_ISSUER, TEST_SUBJECT, TEST_AUDIENCE, - TEST_NONCE); - IdToken idToken = IdToken.from(serializedIdToken); - assertEquals(idToken.issuer, TEST_ISSUER); - assertEquals(idToken.subject, TEST_SUBJECT); + TEST_NONCE + ); + IdToken idToken = IdToken.from(testToken); + assertEquals(TEST_ISSUER, idToken.issuer); + assertEquals(TEST_SUBJECT, idToken.subject); assertThat(idToken.audience, contains(TEST_AUDIENCE)); - assertEquals(idToken.nonce, TEST_NONCE); + assertEquals(TEST_NONCE, idToken.nonce); + } + + @Test + public void testFrom_shouldParseAudienceList() throws Exception { + List audienceList = Arrays.asList(TEST_AUDIENCE, "AUDI3NCE2"); + String testToken = getUnsignedIdTokenWithAudienceList( + TEST_ISSUER, + TEST_SUBJECT, + audienceList, + TEST_NONCE + ); + IdToken idToken = IdToken.from(testToken); + assertEquals(TEST_ISSUER, idToken.issuer); + assertEquals(TEST_SUBJECT, idToken.subject); + assertEquals(audienceList, idToken.audience); + assertEquals(TEST_NONCE, idToken.nonce); } @Test(expected = IdTokenException.class) @@ -51,14 +71,82 @@ public void testFrom_shouldFailOnMalformedInput() throws IdTokenException, JSONE @Test(expected = JSONException.class) public void testFrom_shouldFailOnMissingIssuer() throws IdTokenException, JSONException { + String testToken = getUnsignedIdToken( + null, + TEST_SUBJECT, + TEST_AUDIENCE, + TEST_NONCE + ); + IdToken.from(testToken); + } - IdToken.from("header.claims"); + @Test(expected = JSONException.class) + public void testFrom_shouldFailOnMissingSubject() throws IdTokenException, JSONException { + String testToken = getUnsignedIdToken( + TEST_ISSUER, + null, + TEST_AUDIENCE, + TEST_NONCE + ); + IdToken.from(testToken); + } + + @Test(expected = JSONException.class) + public void testFrom_shouldFailOnMissingAudience() throws IdTokenException, JSONException { + String testToken = getUnsignedIdToken( + TEST_ISSUER, + TEST_SUBJECT, + null, + TEST_NONCE + ); + IdToken.from(testToken); + } + + @Test(expected = JSONException.class) + public void testFrom_shouldFailOnMissingExpiration() throws IdTokenException, JSONException { + String testToken = getUnsignedIdToken( + TEST_ISSUER, + TEST_SUBJECT, + TEST_AUDIENCE, + null, + 0L, + TEST_NONCE + ); + IdToken.from(testToken); } + @Test(expected = JSONException.class) + public void testFrom_shouldFailOnMissingIssuedAt() throws IdTokenException, JSONException { + String testToken = getUnsignedIdToken( + TEST_ISSUER, + TEST_SUBJECT, + TEST_AUDIENCE, + 0L, + null, + TEST_NONCE + ); + IdToken.from(testToken); + } private static String base64UrlNoPaddingEncode(byte[] data) { return Base64.encodeToString(data, Base64.URL_SAFE | Base64.NO_PADDING | Base64.NO_WRAP); } + static String getUnsignedIdTokenWithAudienceList( + @Nullable String issuer, + @Nullable String subject, + @Nullable List audience, + @Nullable String nonce) { + Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000; + Long tenMinutesInSeconds = (long) (10 * 60); + return getUnsignedIdToken( + issuer, + subject, + audience, + nowInSeconds + tenMinutesInSeconds, + nowInSeconds, + nonce); + } + static String getUnsignedIdToken( @Nullable String issuer, @Nullable String subject, @@ -75,6 +163,30 @@ static String getUnsignedIdToken( nonce); } + static String getUnsignedIdToken( + @Nullable String issuer, + @Nullable String subject, + @Nullable List audience, + @Nullable Long expiration, + @Nullable Long issuedAt, + @Nullable String nonce) { + JSONObject header = new JSONObject(); + JsonUtil.put(header, "typ", "JWT"); + + JSONObject claims = new JSONObject(); + JsonUtil.putIfNotNull(claims, "iss", issuer); + JsonUtil.putIfNotNull(claims, "sub", subject); + JsonUtil.put(claims, "aud", new JSONArray(audience)); + JsonUtil.putIfNotNull(claims, "exp", expiration != null ? String.valueOf(expiration) : null); + JsonUtil.putIfNotNull(claims, "iat", issuedAt != null ? String.valueOf(issuedAt) : null); + JsonUtil.putIfNotNull(claims, "nonce", nonce); + + + String encodedHeader = base64UrlNoPaddingEncode(header.toString().getBytes()); + String encodedClaims = base64UrlNoPaddingEncode(claims.toString().getBytes()); + return encodedHeader + "." + encodedClaims; + } + static String getUnsignedIdToken( @Nullable String issuer, @Nullable String subject, From 169e3d652f04f9fbf98d098aaf0937ed79d67556 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98yvind=20Robertsen?= Date: Wed, 8 Aug 2018 17:23:05 +0200 Subject: [PATCH 11/13] chore: remove unused verifyNonce method --- library/java/net/openid/appauth/AuthorizationService.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/library/java/net/openid/appauth/AuthorizationService.java b/library/java/net/openid/appauth/AuthorizationService.java index 3df70bfe..2ae419c4 100644 --- a/library/java/net/openid/appauth/AuthorizationService.java +++ b/library/java/net/openid/appauth/AuthorizationService.java @@ -604,11 +604,6 @@ protected void onPostExecute(JSONObject json) { mCallback.onTokenRequestCompleted(response, null); } - private boolean verifyNonce(String idTokenNonce, String expectedNonce) { - return expectedNonce != null - && (idTokenNonce != null && !idTokenNonce.equals(expectedNonce)); - } - /** * GitHub will only return a spec-compliant response if JSON is explicitly defined * as an acceptable response type. As this is essentially harmless for all other From 8c2fa9d41c135ecd6f83d647473ae6e527149eb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98yvind=20Robertsen?= Date: Thu, 9 Aug 2018 14:53:26 +0200 Subject: [PATCH 12/13] kaizen: move validation logic to IdToken class Improve test coverage for ID Token validation --- .../openid/appauth/AuthorizationService.java | 87 +----- library/java/net/openid/appauth/IdToken.java | 97 +++++++ .../AuthorizationServiceDiscoveryTest.java | 65 ++--- .../net/openid/appauth/IdTokenTest.java | 269 +++++++++++++++++- .../net/openid/appauth/TestValues.java | 40 ++- 5 files changed, 438 insertions(+), 120 deletions(-) diff --git a/library/java/net/openid/appauth/AuthorizationService.java b/library/java/net/openid/appauth/AuthorizationService.java index 2ae419c4..cb59b2da 100644 --- a/library/java/net/openid/appauth/AuthorizationService.java +++ b/library/java/net/openid/appauth/AuthorizationService.java @@ -389,9 +389,6 @@ private Intent prepareAuthorizationRequestIntent( private static class TokenRequestTask extends AsyncTask { - private static final Long MILLIS_PER_SECOND = 1000L; - private static final Long TEN_MINUTES_IN_SECONDS = 600L; - private TokenRequest mRequest; private ClientAuthentication mClientAuthentication; private final ConnectionBuilder mConnectionBuilder; @@ -516,88 +513,12 @@ protected void onPostExecute(JSONObject json) { return; } - // OpenID Connect Core Section 3.1.3.7. rule #1 - // Not enforced: AppAuth does not support JWT encryption. - - // OpenID Connect Core Section 3.1.3.7. rule #2 - // Validates that the issuer in the ID Token matches that of the discovery document. - - AuthorizationServiceDiscovery discoveryDoc = mRequest.configuration.discoveryDoc; - if (discoveryDoc == null) { - mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( - GeneralErrors.ID_TOKEN_VALIDATION_ERROR, - new NullPointerException("Missing discovery document"))); - return; - } - - String expectedIssuer = discoveryDoc.getIssuer(); - if (!idToken.issuer.equals(expectedIssuer)) { - mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( - GeneralErrors.ID_TOKEN_VALIDATION_ERROR, - new IdTokenException("Issuer mismatch"))); - return; - } - - // OpenID Connect Core Section 3.1.3.7. rule #3 - // Validates that the audience of the ID Token matches the client ID. - String clientId = mRequest.clientId; - if (!idToken.audience.contains(clientId)) { - mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( - GeneralErrors.ID_TOKEN_VALIDATION_ERROR, - new IdTokenException("Audience mismatch"))); - return; - } - - // OpenID Connect Core Section 3.1.3.7. rules #4 & #5 - // Not enforced. - - // OpenID Connect Core Section 3.1.3.7. rule #6 - // As noted above, AppAuth only supports the code flow which results in direct - // communication of the ID Token from the Token Endpoint to the Client, and we are - // exercising the option to use TLS server validation instead of checking the token - // signature. Users may additionally check the token signature should they wish. - - // OpenID Connect Core Section 3.1.3.7. rules #7 & #8 - // Not enforced. See rule #6. - - // OpenID Connect Core Section 3.1.3.7. rule #9 - // Validates that the current time is before the expiry time. - Long nowInSeconds = mClock.getCurrentTimeMillis() / MILLIS_PER_SECOND; - if (nowInSeconds > idToken.expiration) { - mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( - GeneralErrors.ID_TOKEN_VALIDATION_ERROR, - new IdTokenException("ID Token expired"))); - return; - } - - // 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 - idToken.issuedAt) > TEN_MINUTES_IN_SECONDS) { - mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( - GeneralErrors.ID_TOKEN_VALIDATION_ERROR, - new IdTokenException("Issued at time is more than 10 minutes " - + "before or after the current time"))); + try { + idToken.validate(mRequest, mClock); + } catch (AuthorizationException ex) { + mCallback.onTokenRequestCompleted(null, ex); return; } - - // Only relevant for the authorization_code response type - if (GrantTypeValues.AUTHORIZATION_CODE.equals(mRequest.grantType)) { - // OpenID Connect Core Section 3.1.3.7. rule #11 - // Validates the nonce. - String expectedNonce = mRequest.nonce; - if (!TextUtils.equals(idToken.nonce, expectedNonce)) { - mCallback.onTokenRequestCompleted(null, AuthorizationException.fromTemplate( - GeneralErrors.ID_TOKEN_VALIDATION_ERROR, - new IdTokenException("Nonce mismatch"))); - return; - } - } - // OpenID Connect Core Section 3.1.3.7. rules #12 - // ACR is not directly supported by AppAuth. - - // OpenID Connect Core Section 3.1.3.7. rules #12 - // max_age is not directly supported by AppAuth. } Logger.debug("Token exchange with %s completed", mRequest.configuration.tokenEndpoint); diff --git a/library/java/net/openid/appauth/IdToken.java b/library/java/net/openid/appauth/IdToken.java index d27c9b60..ef9740b3 100644 --- a/library/java/net/openid/appauth/IdToken.java +++ b/library/java/net/openid/appauth/IdToken.java @@ -14,10 +14,13 @@ package net.openid.appauth; +import android.net.Uri; import android.support.annotation.NonNull; import android.support.annotation.Nullable; +import android.text.TextUtils; import android.util.Base64; +import net.openid.appauth.AuthorizationException.GeneralErrors; import org.json.JSONException; import org.json.JSONObject; @@ -42,6 +45,8 @@ class IdToken { private static final String KEY_EXPIRATION = "exp"; private static final String KEY_ISSUED_AT = "iat"; private static final String KEY_NONCE = "nonce"; + private static final Long MILLIS_PER_SECOND = 1000L; + private static final Long TEN_MINUTES_IN_SECONDS = 600L; public final String issuer; public final String subject; @@ -104,6 +109,98 @@ static IdToken from(String token) throws JSONException, IdTokenException { ); } + void validate(@NonNull TokenRequest tokenRequest, Clock clock) throws AuthorizationException { + // OpenID Connect Core Section 3.1.3.7. rule #1 + // Not enforced: AppAuth does not support JWT encryption. + + // OpenID Connect Core Section 3.1.3.7. rule #2 + // Validates that the issuer in the ID Token matches that of the discovery document. + AuthorizationServiceDiscovery discoveryDoc = tokenRequest.configuration.discoveryDoc; + if (discoveryDoc != null) { + String expectedIssuer = discoveryDoc.getIssuer(); + if (!this.issuer.equals(expectedIssuer)) { + throw AuthorizationException.fromTemplate(GeneralErrors.ID_TOKEN_VALIDATION_ERROR, + new IdTokenException("Issuer mismatch")); + } + + // OpenID Connect Core Section 2. + // The iss value is a case sensitive URL using the https scheme that contains scheme, + // host, and optionally, port number and path components and no query or fragment + // components. + Uri issuerUri = Uri.parse(this.issuer); + + if (!issuerUri.getScheme().equals("https")) { + throw AuthorizationException.fromTemplate(GeneralErrors.ID_TOKEN_VALIDATION_ERROR, + new IdTokenException("Issuer must be an https URL")); + } + + if (TextUtils.isEmpty(issuerUri.getHost())) { + throw AuthorizationException.fromTemplate(GeneralErrors.ID_TOKEN_VALIDATION_ERROR, + new IdTokenException("Issuer host can not be empty")); + } + + if (issuerUri.getFragment() != null || issuerUri.getQueryParameterNames().size() > 0) { + throw AuthorizationException.fromTemplate(GeneralErrors.ID_TOKEN_VALIDATION_ERROR, + new IdTokenException( + "Issuer URL should not containt query parameters or fragment components")); + } + } + + + // OpenID Connect Core Section 3.1.3.7. rule #3 + // Validates that the audience of the ID Token matches the client ID. + String clientId = tokenRequest.clientId; + if (!this.audience.contains(clientId)) { + throw AuthorizationException.fromTemplate(GeneralErrors.ID_TOKEN_VALIDATION_ERROR, + new IdTokenException("Audience mismatch")); + } + + // OpenID Connect Core Section 3.1.3.7. rules #4 & #5 + // Not enforced. + + // OpenID Connect Core Section 3.1.3.7. rule #6 + // As noted above, AppAuth only supports the code flow which results in direct + // communication of the ID Token from the Token Endpoint to the Client, and we are + // exercising the option to use TLS server validation instead of checking the token + // signature. Users may additionally check the token signature should they wish. + + // OpenID Connect Core Section 3.1.3.7. rules #7 & #8 + // Not enforced. See rule #6. + + // OpenID Connect Core Section 3.1.3.7. rule #9 + // Validates that the current time is before the expiry time. + Long nowInSeconds = clock.getCurrentTimeMillis() / MILLIS_PER_SECOND; + if (nowInSeconds > this.expiration) { + throw AuthorizationException.fromTemplate(GeneralErrors.ID_TOKEN_VALIDATION_ERROR, + new IdTokenException("ID Token expired")); + } + + // 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 + if (GrantTypeValues.AUTHORIZATION_CODE.equals(tokenRequest.grantType)) { + // OpenID Connect Core Section 3.1.3.7. rule #11 + // Validates the nonce. + String expectedNonce = tokenRequest.nonce; + if (!TextUtils.equals(this.nonce, expectedNonce)) { + throw AuthorizationException.fromTemplate(GeneralErrors.ID_TOKEN_VALIDATION_ERROR, + new IdTokenException("Nonce mismatch")); + } + } + // OpenID Connect Core Section 3.1.3.7. rules #12 + // ACR is not directly supported by AppAuth. + + // OpenID Connect Core Section 3.1.3.7. rules #12 + // max_age is not directly supported by AppAuth. + } + static class IdTokenException extends Exception { IdTokenException(String message) { super(message); diff --git a/library/javatests/net/openid/appauth/AuthorizationServiceDiscoveryTest.java b/library/javatests/net/openid/appauth/AuthorizationServiceDiscoveryTest.java index df4b8999..4441e064 100644 --- a/library/javatests/net/openid/appauth/AuthorizationServiceDiscoveryTest.java +++ b/library/javatests/net/openid/appauth/AuthorizationServiceDiscoveryTest.java @@ -14,6 +14,8 @@ package net.openid.appauth; +import static net.openid.appauth.TestValues.TEST_ISSUER; +import static net.openid.appauth.TestValues.getDiscoveryDocumentJson; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -21,7 +23,6 @@ import java.util.Arrays; import java.util.List; -import org.json.JSONArray; import org.json.JSONObject; import org.junit.Before; import org.junit.Test; @@ -33,36 +34,33 @@ @Config(constants = BuildConfig.class, sdk=16) public class AuthorizationServiceDiscoveryTest { // ToDo: add more tests for remaining getters - static final String TEST_ISSUER = "test_issuer"; - private static final String TEST_AUTHORIZATION_ENDPOINT = "http://test.openid.com/o/oauth/auth"; - private static final String TEST_TOKEN_ENDPOINT = "http://test.openid.com/o/oauth/token"; - private static final String TEST_USERINFO_ENDPOINT = "http://test.openid.com/o/oauth/userinfo"; - private static final String TEST_REGISTRATION_ENDPOINT = "http://test.openid.com/o/oauth/register"; - private static final String TEST_JWKS_URI = "http://test.openid.com/o/oauth/jwks"; - private static final List TEST_RESPONSE_TYPE_SUPPORTED = Arrays.asList("code", "token"); - private static final List TEST_SUBJECT_TYPES_SUPPORTED = Arrays.asList("public"); - private static final List TEST_ID_TOKEN_SIGNING_ALG_VALUES = Arrays.asList("RS256"); - private static final List TEST_SCOPES_SUPPORTED = Arrays.asList("openid", "profile"); - private static final List TEST_TOKEN_ENDPOINT_AUTH_METHODS - = Arrays.asList("client_secret_post", "client_secret_basic"); - private static final List TEST_CLAIMS_SUPPORTED = Arrays.asList("aud", "exp"); - - static final String TEST_JSON = "{\n" - + " \"issuer\": \"" + TEST_ISSUER + "\",\n" - + " \"authorization_endpoint\": \"" + TEST_AUTHORIZATION_ENDPOINT + "\",\n" - + " \"token_endpoint\": \"" + TEST_TOKEN_ENDPOINT + "\",\n" - + " \"userinfo_endpoint\": \"" + TEST_USERINFO_ENDPOINT + "\",\n" - + " \"registration_endpoint\": \"" + TEST_REGISTRATION_ENDPOINT + "\",\n" - + " \"jwks_uri\": \"" + TEST_JWKS_URI + "\",\n" - + " \"response_types_supported\": " + toJson(TEST_RESPONSE_TYPE_SUPPORTED) + ",\n" - + " \"subject_types_supported\": " + toJson(TEST_SUBJECT_TYPES_SUPPORTED) + ",\n" - + " \"id_token_signing_alg_values_supported\": " - + toJson(TEST_ID_TOKEN_SIGNING_ALG_VALUES) + ",\n" - + " \"scopes_supported\": " + toJson(TEST_SCOPES_SUPPORTED) + ",\n" - + " \"token_endpoint_auth_methods_supported\": " - + toJson(TEST_TOKEN_ENDPOINT_AUTH_METHODS) + ",\n" - + " \"claims_supported\": " + toJson(TEST_CLAIMS_SUPPORTED) + "\n" - + "}"; + static final String TEST_AUTHORIZATION_ENDPOINT = "http://test.openid.com/o/oauth/auth"; + static final String TEST_TOKEN_ENDPOINT = "http://test.openid.com/o/oauth/token"; + static final String TEST_USERINFO_ENDPOINT = "http://test.openid.com/o/oauth/userinfo"; + static final String TEST_REGISTRATION_ENDPOINT = "http://test.openid.com/o/oauth/register"; + static final String TEST_JWKS_URI = "http://test.openid.com/o/oauth/jwks"; + static final List TEST_RESPONSE_TYPES_SUPPORTED = Arrays.asList("code", "token"); + static final List TEST_SUBJECT_TYPES_SUPPORTED = Arrays.asList("public"); + static final List TEST_ID_TOKEN_SIGNING_ALG_VALUES = Arrays.asList("RS256"); + static final List TEST_SCOPES_SUPPORTED = Arrays.asList("openid", "profile"); + static final List TEST_TOKEN_ENDPOINT_AUTH_METHODS + = Arrays.asList("client_secret_post", "client_secret_basic"); + static final List TEST_CLAIMS_SUPPORTED = Arrays.asList("aud", "exp"); + + static final String TEST_JSON = getDiscoveryDocumentJson( + TEST_ISSUER, + TEST_AUTHORIZATION_ENDPOINT, + TEST_TOKEN_ENDPOINT, + TEST_USERINFO_ENDPOINT, + TEST_REGISTRATION_ENDPOINT, + TEST_JWKS_URI, + TEST_RESPONSE_TYPES_SUPPORTED, + TEST_SUBJECT_TYPES_SUPPORTED, + TEST_ID_TOKEN_SIGNING_ALG_VALUES, + TEST_SCOPES_SUPPORTED, + TEST_TOKEN_ENDPOINT_AUTH_METHODS, + TEST_CLAIMS_SUPPORTED + ); JSONObject mJson; AuthorizationServiceDiscovery mDiscovery; @@ -196,7 +194,7 @@ public void testGetJwksUri() { @Test public void testGetResponseTypeSupported() { - assertEquals(TEST_RESPONSE_TYPE_SUPPORTED, mDiscovery.getResponseTypesSupported()); + assertEquals(TEST_RESPONSE_TYPES_SUPPORTED, mDiscovery.getResponseTypesSupported()); } @Test @@ -226,7 +224,4 @@ public void testGetClaimsSupported() { assertEquals(TEST_CLAIMS_SUPPORTED, mDiscovery.getClaimsSupported()); } - private static String toJson(List strings) { - return new JSONArray(strings).toString(); - } } diff --git a/library/javatests/net/openid/appauth/IdTokenTest.java b/library/javatests/net/openid/appauth/IdTokenTest.java index 03bd8e91..3579bbb7 100644 --- a/library/javatests/net/openid/appauth/IdTokenTest.java +++ b/library/javatests/net/openid/appauth/IdTokenTest.java @@ -4,7 +4,9 @@ import android.util.Base64; import java.util.Arrays; +import java.util.Collections; import java.util.List; +import net.openid.appauth.AuthorizationServiceDiscovery.MissingArgumentException; import net.openid.appauth.IdToken.IdTokenException; import org.json.JSONArray; import org.json.JSONException; @@ -14,7 +16,25 @@ import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; +import static net.openid.appauth.AuthorizationServiceDiscoveryTest.TEST_AUTHORIZATION_ENDPOINT; +import static net.openid.appauth.AuthorizationServiceDiscoveryTest.TEST_CLAIMS_SUPPORTED; +import static net.openid.appauth.AuthorizationServiceDiscoveryTest.TEST_ID_TOKEN_SIGNING_ALG_VALUES; +import static net.openid.appauth.AuthorizationServiceDiscoveryTest.TEST_JWKS_URI; +import static net.openid.appauth.AuthorizationServiceDiscoveryTest.TEST_REGISTRATION_ENDPOINT; +import static net.openid.appauth.AuthorizationServiceDiscoveryTest.TEST_RESPONSE_TYPES_SUPPORTED; +import static net.openid.appauth.AuthorizationServiceDiscoveryTest.TEST_SCOPES_SUPPORTED; +import static net.openid.appauth.AuthorizationServiceDiscoveryTest.TEST_SUBJECT_TYPES_SUPPORTED; +import static net.openid.appauth.AuthorizationServiceDiscoveryTest.TEST_TOKEN_ENDPOINT; +import static net.openid.appauth.AuthorizationServiceDiscoveryTest.TEST_TOKEN_ENDPOINT_AUTH_METHODS; +import static net.openid.appauth.AuthorizationServiceDiscoveryTest.TEST_USERINFO_ENDPOINT; +import static net.openid.appauth.TestValues.TEST_APP_REDIRECT_URI; +import static net.openid.appauth.TestValues.TEST_AUTH_CODE; +import static net.openid.appauth.TestValues.TEST_CLIENT_ID; +import static net.openid.appauth.TestValues.TEST_CODE_VERIFIER; +import static net.openid.appauth.TestValues.TEST_ISSUER; import static net.openid.appauth.TestValues.TEST_NONCE; +import static net.openid.appauth.TestValues.getDiscoveryDocumentJson; +import static net.openid.appauth.TestValues.getTestAuthCodeExchangeRequestBuilder; import static org.hamcrest.Matchers.contains; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; @@ -23,7 +43,6 @@ @Config(constants = BuildConfig.class, sdk=16) public class IdTokenTest { - static final String TEST_ISSUER = "https://test.issuer"; static final String TEST_SUBJECT = "SUBJ3CT"; static final String TEST_AUDIENCE = "AUDI3NCE"; @@ -127,10 +146,258 @@ public void testFrom_shouldFailOnMissingIssuedAt() throws IdTokenException, JSON ); IdToken.from(testToken); } + + @Test + public void testValidate() throws AuthorizationException { + IdToken idToken = getValidIdToken(); + TokenRequest tokenRequest = getAuthCodeExchangeRequestWithNonce(); + Clock clock = SystemClock.INSTANCE; + idToken.validate(tokenRequest, clock); + } + + @Test(expected = AuthorizationException.class) + public void testValidate_shouldFailOnIssuerMismatch() throws AuthorizationException { + Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000; + Long tenMinutesInSeconds = (long) (10 * 60); + IdToken idToken = new IdToken( + "https://other.issuer", + TEST_SUBJECT, + Collections.singletonList(TEST_CLIENT_ID), + nowInSeconds + tenMinutesInSeconds, + nowInSeconds, + TEST_NONCE + ); + TokenRequest tokenRequest = getAuthCodeExchangeRequestWithNonce(); + Clock clock = SystemClock.INSTANCE; + idToken.validate(tokenRequest, clock); + } + + @Test(expected = AuthorizationException.class) + public void testValidate_shouldFailOnNonHttpsIssuer() + throws AuthorizationException, JSONException, MissingArgumentException { + Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000; + Long tenMinutesInSeconds = (long) (10 * 60); + IdToken idToken = new IdToken( + "http://other.issuer", + TEST_SUBJECT, + Collections.singletonList(TEST_CLIENT_ID), + nowInSeconds + tenMinutesInSeconds, + nowInSeconds, + TEST_NONCE + ); + + String serviceDocJsonWithOtherIssuer = getDiscoveryDocJsonWithIssuer("http://other.issuer"); + AuthorizationServiceDiscovery discoveryDoc = new AuthorizationServiceDiscovery( + new JSONObject(serviceDocJsonWithOtherIssuer)); + AuthorizationServiceConfiguration serviceConfiguration = + new AuthorizationServiceConfiguration(discoveryDoc); + TokenRequest tokenRequest = new TokenRequest.Builder(serviceConfiguration, TEST_CLIENT_ID) + .setAuthorizationCode(TEST_AUTH_CODE) + .setCodeVerifier(TEST_CODE_VERIFIER) + .setGrantType(GrantTypeValues.AUTHORIZATION_CODE) + .setRedirectUri(TEST_APP_REDIRECT_URI) + .build(); + Clock clock = SystemClock.INSTANCE; + idToken.validate(tokenRequest, clock); + } + + @Test(expected = AuthorizationException.class) + public void testValidate_shouldFailOnIssuerMissingHost() + throws AuthorizationException, JSONException, MissingArgumentException { + Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000; + Long tenMinutesInSeconds = (long) (10 * 60); + IdToken idToken = new IdToken( + "https://", + TEST_SUBJECT, + Collections.singletonList(TEST_CLIENT_ID), + nowInSeconds + tenMinutesInSeconds, + nowInSeconds, + TEST_NONCE + ); + + String serviceDocJsonWithIssuerMissingHost = getDiscoveryDocJsonWithIssuer("https://"); + AuthorizationServiceDiscovery discoveryDoc = new AuthorizationServiceDiscovery( + new JSONObject(serviceDocJsonWithIssuerMissingHost)); + AuthorizationServiceConfiguration serviceConfiguration = + new AuthorizationServiceConfiguration(discoveryDoc); + TokenRequest tokenRequest = new TokenRequest.Builder(serviceConfiguration, TEST_CLIENT_ID) + .setAuthorizationCode(TEST_AUTH_CODE) + .setCodeVerifier(TEST_CODE_VERIFIER) + .setGrantType(GrantTypeValues.AUTHORIZATION_CODE) + .setRedirectUri(TEST_APP_REDIRECT_URI) + .build(); + Clock clock = SystemClock.INSTANCE; + idToken.validate(tokenRequest, clock); + } + + @Test(expected = AuthorizationException.class) + public void testValidate_shouldFailOnIssuerWithQueryParam() + throws AuthorizationException, JSONException, MissingArgumentException { + Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000; + Long tenMinutesInSeconds = (long) (10 * 60); + IdToken idToken = new IdToken( + "https://some.issuer?param=value", + TEST_SUBJECT, + Collections.singletonList(TEST_CLIENT_ID), + nowInSeconds + tenMinutesInSeconds, + nowInSeconds, + TEST_NONCE + ); + + String serviceDocJsonWithIssuerMissingHost = getDiscoveryDocJsonWithIssuer( + "https://some.issuer?param=value"); + AuthorizationServiceDiscovery discoveryDoc = new AuthorizationServiceDiscovery( + new JSONObject(serviceDocJsonWithIssuerMissingHost)); + AuthorizationServiceConfiguration serviceConfiguration = + new AuthorizationServiceConfiguration(discoveryDoc); + TokenRequest tokenRequest = new TokenRequest.Builder(serviceConfiguration, TEST_CLIENT_ID) + .setAuthorizationCode(TEST_AUTH_CODE) + .setCodeVerifier(TEST_CODE_VERIFIER) + .setGrantType(GrantTypeValues.AUTHORIZATION_CODE) + .setRedirectUri(TEST_APP_REDIRECT_URI) + .build(); + Clock clock = SystemClock.INSTANCE; + idToken.validate(tokenRequest, clock); + } + + @Test(expected = AuthorizationException.class) + public void testValidate_shouldFailOnIssuerWithFragment() + throws AuthorizationException, JSONException, MissingArgumentException { + Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000; + Long tenMinutesInSeconds = (long) (10 * 60); + IdToken idToken = new IdToken( + "https://some.issuer/#/fragment", + TEST_SUBJECT, + Collections.singletonList(TEST_CLIENT_ID), + nowInSeconds + tenMinutesInSeconds, + nowInSeconds, + TEST_NONCE + ); + + String serviceDocJsonWithIssuerMissingHost = getDiscoveryDocJsonWithIssuer( + "https://some.issuer/#/fragment"); + AuthorizationServiceDiscovery discoveryDoc = new AuthorizationServiceDiscovery( + new JSONObject(serviceDocJsonWithIssuerMissingHost)); + AuthorizationServiceConfiguration serviceConfiguration = + new AuthorizationServiceConfiguration(discoveryDoc); + TokenRequest tokenRequest = new TokenRequest.Builder(serviceConfiguration, TEST_CLIENT_ID) + .setAuthorizationCode(TEST_AUTH_CODE) + .setCodeVerifier(TEST_CODE_VERIFIER) + .setGrantType(GrantTypeValues.AUTHORIZATION_CODE) + .setRedirectUri(TEST_APP_REDIRECT_URI) + .build(); + Clock clock = SystemClock.INSTANCE; + idToken.validate(tokenRequest, clock); + } + + @Test(expected = AuthorizationException.class) + public void testValidate_shouldFailOnAudienceMismatch() throws AuthorizationException { + Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000; + Long tenMinutesInSeconds = (long) (10 * 60); + IdToken idToken = new IdToken( + TEST_ISSUER, + TEST_SUBJECT, + Collections.singletonList("some_other_audience"), + nowInSeconds + tenMinutesInSeconds, + nowInSeconds, + TEST_NONCE + ); + TokenRequest tokenRequest = getAuthCodeExchangeRequestWithNonce(); + Clock clock = SystemClock.INSTANCE; + idToken.validate(tokenRequest, clock); + } + + @Test(expected = AuthorizationException.class) + public void testValidate_shouldFailOnExpiredToken() throws AuthorizationException { + Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000; + Long tenMinutesInSeconds = (long) (10 * 60); + IdToken idToken = new IdToken( + TEST_ISSUER, + TEST_SUBJECT, + Collections.singletonList(TEST_CLIENT_ID), + nowInSeconds - tenMinutesInSeconds, + nowInSeconds, + TEST_NONCE + ); + TokenRequest tokenRequest = getAuthCodeExchangeRequestWithNonce(); + Clock clock = SystemClock.INSTANCE; + idToken.validate(tokenRequest, clock); + } + + @Test(expected = AuthorizationException.class) + public void testValidate_shouldFailOnIssuedAtOverTenMinutesAgo() throws AuthorizationException { + Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000; + Long tenMinutesInSeconds = (long) (10 * 60); + IdToken idToken = new IdToken( + TEST_ISSUER, + TEST_SUBJECT, + Collections.singletonList(TEST_CLIENT_ID), + nowInSeconds + tenMinutesInSeconds, + nowInSeconds - (tenMinutesInSeconds * 2), + TEST_NONCE + ); + TokenRequest tokenRequest = getAuthCodeExchangeRequestWithNonce(); + Clock clock = SystemClock.INSTANCE; + idToken.validate(tokenRequest, clock); + } + + @Test(expected = AuthorizationException.class) + public void testValidate_shouldFailOnNonceMismatch() throws AuthorizationException { + Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000; + Long tenMinutesInSeconds = (long) (10 * 60); + IdToken idToken = new IdToken( + TEST_ISSUER, + TEST_SUBJECT, + Collections.singletonList(TEST_CLIENT_ID), + nowInSeconds + tenMinutesInSeconds, + nowInSeconds, + "some_other_nonce" + ); + TokenRequest tokenRequest = getAuthCodeExchangeRequestWithNonce(); + Clock clock = SystemClock.INSTANCE; + idToken.validate(tokenRequest, clock); + } + private static String base64UrlNoPaddingEncode(byte[] data) { return Base64.encodeToString(data, Base64.URL_SAFE | Base64.NO_PADDING | Base64.NO_WRAP); } + private String getDiscoveryDocJsonWithIssuer(String issuer) { + return getDiscoveryDocumentJson( + issuer, + TEST_AUTHORIZATION_ENDPOINT, + TEST_TOKEN_ENDPOINT, + TEST_USERINFO_ENDPOINT, + TEST_REGISTRATION_ENDPOINT, + TEST_JWKS_URI, + TEST_RESPONSE_TYPES_SUPPORTED, + TEST_SUBJECT_TYPES_SUPPORTED, + TEST_ID_TOKEN_SIGNING_ALG_VALUES, + TEST_SCOPES_SUPPORTED, + TEST_TOKEN_ENDPOINT_AUTH_METHODS, + TEST_CLAIMS_SUPPORTED + ); + } + + private TokenRequest getAuthCodeExchangeRequestWithNonce() { + return getTestAuthCodeExchangeRequestBuilder() + .setNonce(TEST_NONCE) + .build(); + } + + private static IdToken getValidIdToken() { + Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000; + Long tenMinutesInSeconds = (long) (10 * 60); + return new IdToken( + TEST_ISSUER, + TEST_SUBJECT, + Collections.singletonList(TEST_CLIENT_ID), + nowInSeconds + tenMinutesInSeconds, + nowInSeconds, + TEST_NONCE + ); + } + static String getUnsignedIdTokenWithAudienceList( @Nullable String issuer, @Nullable String subject, diff --git a/library/javatests/net/openid/appauth/TestValues.java b/library/javatests/net/openid/appauth/TestValues.java index 86445308..8e8b10f7 100644 --- a/library/javatests/net/openid/appauth/TestValues.java +++ b/library/javatests/net/openid/appauth/TestValues.java @@ -16,12 +16,13 @@ import android.net.Uri; +import java.util.List; +import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; import java.util.Arrays; -import static net.openid.appauth.AuthorizationServiceDiscoveryTest.TEST_ISSUER; import static net.openid.appauth.IdTokenTest.TEST_SUBJECT; /** @@ -46,6 +47,7 @@ class TestValues { public static final String TEST_AUTH_CODE = "zxcvbnmjk"; public static final String TEST_ACCESS_TOKEN = "aaabbbccc"; public static final Long TEST_ACCESS_TOKEN_EXPIRATION_TIME = 120000L; // two minutes + public static final String TEST_ISSUER = "https://test.issuer"; public static final String TEST_ID_TOKEN = IdTokenTest.getUnsignedIdToken( TEST_ISSUER, TEST_SUBJECT, @@ -59,6 +61,42 @@ class TestValues { public static final String TEST_EMAIL_ADDRESS = "test@example.com"; + private static String toJson(List strings) { + return new JSONArray(strings).toString(); + } + + static String getDiscoveryDocumentJson( + String issuer, + String authorizationEndpoint, + String tokenEndpoint, + String userInfoEndpoint, + String registrationEndpoint, + String jwksUri, + List responseTypesSupported, + List subjectTypesSupported, + List idTokenSigningAlgValues, + List scopesSupported, + List tokenEndpointAuthMethods, + List claimsSupported + ) { + return "{\n" + + " \"issuer\": \"" + issuer + "\",\n" + + " \"authorization_endpoint\": \"" + authorizationEndpoint + "\",\n" + + " \"token_endpoint\": \"" + tokenEndpoint + "\",\n" + + " \"userinfo_endpoint\": \"" + userInfoEndpoint + "\",\n" + + " \"registration_endpoint\": \"" + registrationEndpoint + "\",\n" + + " \"jwks_uri\": \"" + jwksUri + "\",\n" + + " \"response_types_supported\": " + toJson(responseTypesSupported) + ",\n" + + " \"subject_types_supported\": " + toJson(subjectTypesSupported) + ",\n" + + " \"id_token_signing_alg_values_supported\": " + + toJson(idTokenSigningAlgValues) + ",\n" + + " \"scopes_supported\": " + toJson(scopesSupported) + ",\n" + + " \"token_endpoint_auth_methods_supported\": " + + toJson(tokenEndpointAuthMethods) + ",\n" + + " \"claims_supported\": " + toJson(claimsSupported) + "\n" + + "}"; + } + public static AuthorizationServiceDiscovery getTestDiscoveryDocument() { try { return new AuthorizationServiceDiscovery( From 78a5d406b37292498cd6f3cd3da8884d87f0bf92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98yvind=20Robertsen?= Date: Thu, 9 Aug 2018 20:23:18 +0200 Subject: [PATCH 13/13] Revert unintentional copyright year bump --- library/java/net/openid/appauth/AuthorizationService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/java/net/openid/appauth/AuthorizationService.java b/library/java/net/openid/appauth/AuthorizationService.java index cb59b2da..87282ea6 100644 --- a/library/java/net/openid/appauth/AuthorizationService.java +++ b/library/java/net/openid/appauth/AuthorizationService.java @@ -1,5 +1,5 @@ /* - * Copyright 2018 The AppAuth for Android Authors. All Rights Reserved. + * Copyright 2015 The AppAuth for Android Authors. All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at