KEYCLOAK-9635 Add AccessTokenHash to IDToken for OIDC Auth Code flow

Revised tests
johngrimes · May 27, 2020 · e825ec2 · e825ec2
1 parent 5a337d0
commit e825ec2
Show file tree

Hide file tree

Showing 7 changed files with 56 additions and 22 deletions.
diff --git a/...illian/tests/base/src/test/java/org/keycloak/testsuite/AbstractTestRealmKeycloakTest.java b/...illian/tests/base/src/test/java/org/keycloak/testsuite/AbstractTestRealmKeycloakTest.java
@@ -93,21 +93,29 @@ public void deleteCookies() {
 
 
     protected IDToken sendTokenRequestAndGetIDToken(EventRepresentation loginEvent) {
+
+        OAuthClient.AccessTokenResponse response = sendTokenRequestAndGetResponse(loginEvent);
+        return oauth.verifyIDToken(response.getIdToken());
+    }
+
+    protected OAuthClient.AccessTokenResponse sendTokenRequestAndGetResponse(EventRepresentation loginEvent) {
+
         String sessionId = loginEvent.getSessionId();
         String codeId = loginEvent.getDetails().get(Details.CODE_ID);
 
         String code = new OAuthClient.AuthorizationEndpointResponse(oauth).getCode();
         OAuthClient.AccessTokenResponse response = oauth.doAccessTokenRequest(code, "password");
 
         Assert.assertEquals(200, response.getStatusCode());
-        IDToken idToken = oauth.verifyIDToken(response.getIdToken());
+
 
         Field eventsField = Reflections.findDeclaredField(this.getClass(), "events");
         if (eventsField != null) {
             AssertEvents events = Reflections.getFieldValue(eventsField, this, AssertEvents.class);
             events.expectCodeToToken(codeId, sessionId).assertEvent();
         }
-        return idToken;
+
+        return response;
     }
 
     /** KEYCLOAK-12065 Inherit Client Connection from parent session **/

diff --git a/...ts/base/src/test/java/org/keycloak/testsuite/oidc/flows/AbstractOIDCResponseTypeTest.java b/...ts/base/src/test/java/org/keycloak/testsuite/oidc/flows/AbstractOIDCResponseTypeTest.java
@@ -28,6 +28,7 @@
 import org.keycloak.events.Errors;
 import org.keycloak.jose.jws.JWSHeader;
 import org.keycloak.jose.jws.JWSInput;
+import org.keycloak.jose.jws.crypto.HashUtils;
 import org.keycloak.representations.IDToken;
 import org.keycloak.representations.idm.EventRepresentation;
 import org.keycloak.representations.idm.RealmRepresentation;
@@ -253,11 +254,6 @@ private void oidcFlow(String expectedAccessAlg, String expectedIdTokenAlg) throw
         for (IDToken idt : idTokens) {
             Assert.assertEquals("abcdef123456", idt.getNonce());
             Assert.assertEquals(authzResponse.getSessionState(), idt.getSessionState());
-            // see KEYCLOAK-9635
-            if (authzResponse.getCode() != null && authzResponse.getAccessToken() != null) {
-                // we have an IDToken that was obtained via auth code flow alongside an AccessToken
-                Assert.assertNotNull("claim at_hash should be present in IDToken for OIDC auth code flow requests", idt.getAccessTokenHash());
-            }
         }
     }
 
@@ -302,4 +298,29 @@ private void setIdTokenSignatureAlgorithm(String idTokenSigAlgName) {
     protected String getIdTokenSignatureAlgorithm() {
         return this.idTokenSigAlgName;
     }
+
+    /**
+     *  Validate "at_hash" claim in IDToken.
+     *  see KEYCLOAK-9635
+     * @param accessTokenHash
+     * @param accessToken
+     */
+    protected void assertValidAccessTokenHash(String accessTokenHash, String accessToken) {
+
+        Assert.assertNotNull(accessTokenHash);
+        Assert.assertNotNull(accessToken);
+        assertEquals(accessTokenHash, HashUtils.oidcHash(getIdTokenSignatureAlgorithm(), accessToken));
+    }
+
+    /**
+     * Validate  "c_hash" claim in IDToken.
+     * @param codeHash
+     * @param code
+     */
+    protected void assertValidCodeHash(String codeHash, String code) {
+
+        Assert.assertNotNull(codeHash);
+        Assert.assertNotNull(code);
+        Assert.assertEquals(codeHash, HashUtils.oidcHash(getIdTokenSignatureAlgorithm(), code));
+    }
 }
diff --git a/...s/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCBasicResponseTypeCodeTest.java b/...s/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCBasicResponseTypeCodeTest.java
@@ -57,9 +57,13 @@ protected List<IDToken> testAuthzResponseAndRetrieveIDTokens(OAuthClient.Authori
         Assert.assertNull(authzResponse.getAccessToken());
         Assert.assertNull(authzResponse.getIdToken());
 
-        IDToken idToken = sendTokenRequestAndGetIDToken(loginEvent);
+        OAuthClient.AccessTokenResponse authzResponse2 = sendTokenRequestAndGetResponse(loginEvent);
+        IDToken idToken2 = oauth.verifyIDToken(authzResponse2.getIdToken());
 
-        return Collections.singletonList(idToken);
+        // Validate "at_hash"
+        assertValidAccessTokenHash(idToken2.getAccessTokenHash(), authzResponse2.getAccessToken());
+
+        return Collections.singletonList(idToken2);
     }
 
 

diff --git a/...rc/test/java/org/keycloak/testsuite/oidc/flows/OIDCHybridResponseTypeCodeIDTokenTest.java b/...rc/test/java/org/keycloak/testsuite/oidc/flows/OIDCHybridResponseTypeCodeIDTokenTest.java
@@ -60,11 +60,11 @@ protected List<IDToken> testAuthzResponseAndRetrieveIDTokens(OAuthClient.Authori
         String idTokenStr = authzResponse.getIdToken();
         IDToken idToken = oauth.verifyIDToken(idTokenStr);
 
-        // Validate "c_hash"
+        // Validate "at_hash"
         Assert.assertNull(idToken.getAccessTokenHash());
-        Assert.assertNotNull(idToken.getCodeHash());
 
-        Assert.assertEquals(idToken.getCodeHash(), HashUtils.oidcHash(getIdTokenSignatureAlgorithm(), authzResponse.getCode()));
+        // Validate "c_hash"
+        assertValidCodeHash(idToken.getCodeHash(), authzResponse.getCode());
 
         // Financial API - Part 2: Read and Write API Security Profile
         // http://openid.net/specs/openid-financial-api-part-2.html#authorization-server

diff --git a/...st/java/org/keycloak/testsuite/oidc/flows/OIDCHybridResponseTypeCodeIDTokenTokenTest.java b/...st/java/org/keycloak/testsuite/oidc/flows/OIDCHybridResponseTypeCodeIDTokenTokenTest.java
@@ -60,13 +60,11 @@ protected List<IDToken> testAuthzResponseAndRetrieveIDTokens(OAuthClient.Authori
         String idTokenStr = authzResponse.getIdToken();
         IDToken idToken = oauth.verifyIDToken(idTokenStr);
 
-        // Validate "at_hash" and "c_hash"
-        Assert.assertNotNull(idToken.getAccessTokenHash());
+        // Validate "at_hash"
+        assertValidAccessTokenHash(idToken.getAccessTokenHash(), authzResponse.getAccessToken());
 
-        Assert.assertEquals(idToken.getAccessTokenHash(), HashUtils.oidcHash(getIdTokenSignatureAlgorithm(), authzResponse.getAccessToken()));
-        Assert.assertNotNull(idToken.getCodeHash());
-
-        Assert.assertEquals(idToken.getCodeHash(), HashUtils.oidcHash(getIdTokenSignatureAlgorithm(), authzResponse.getCode()));
+        // Validate "c_hash"
+        assertValidCodeHash(idToken.getCodeHash(), authzResponse.getCode());
 
         // Financial API - Part 2: Read and Write API Security Profile
         // http://openid.net/specs/openid-financial-api-part-2.html#authorization-server
@@ -81,7 +79,6 @@ protected List<IDToken> testAuthzResponseAndRetrieveIDTokens(OAuthClient.Authori
         return Arrays.asList(idToken, idToken2);
     }
 
-
     @Test
     public void nonceNotUsedErrorExpected() {
         super.validateNonceNotUsedErrorExpected();

diff --git a/.../src/test/java/org/keycloak/testsuite/oidc/flows/OIDCHybridResponseTypeCodeTokenTest.java b/.../src/test/java/org/keycloak/testsuite/oidc/flows/OIDCHybridResponseTypeCodeTokenTest.java
@@ -58,7 +58,11 @@ protected List<IDToken> testAuthzResponseAndRetrieveIDTokens(OAuthClient.Authori
         Assert.assertNull(authzResponse.getIdToken());
 
         // IDToken exchanged for the code
-        IDToken idToken2 = sendTokenRequestAndGetIDToken(loginEvent);
+        OAuthClient.AccessTokenResponse authzResponse2 = sendTokenRequestAndGetResponse(loginEvent);
+        IDToken idToken2 = oauth.verifyIDToken(authzResponse2.getIdToken());
+
+        // Validate "at_hash"
+        assertValidAccessTokenHash(idToken2.getAccessTokenHash(), authzResponse2.getAccessToken());
 
         return Collections.singletonList(idToken2);
     }

diff --git a/...test/java/org/keycloak/testsuite/oidc/flows/OIDCImplicitResponseTypeIDTokenTokenTest.java b/...test/java/org/keycloak/testsuite/oidc/flows/OIDCImplicitResponseTypeIDTokenTokenTest.java
@@ -60,9 +60,9 @@ protected List<IDToken> testAuthzResponseAndRetrieveIDTokens(OAuthClient.Authori
         IDToken idToken = oauth.verifyIDToken(idTokenStr);
 
         // Validate "at_hash"
-        Assert.assertNotNull(idToken.getAccessTokenHash());
+        assertValidAccessTokenHash(idToken.getAccessTokenHash(), authzResponse.getAccessToken());
 
-        Assert.assertEquals(idToken.getAccessTokenHash(), HashUtils.oidcHash(getIdTokenSignatureAlgorithm(), authzResponse.getAccessToken()));
+        // Validate "c_hash"
         Assert.assertNull(idToken.getCodeHash());
 
         return Collections.singletonList(idToken);