KEYCLOAK-3221 Tokens should be invalidated if an attempt to reuse code is made

services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java

@@ -199,6 +199,11 @@ public class TokenManager {
             return false;
         }
 
+        ClientSessionModel clientSession = session.sessions().getClientSession(realm, token.getClientSession());
+        if (clientSession == null) {
+            return false;
+        }
+
         return true;
     }
 

services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java

@@ -68,6 +68,9 @@ import java.util.Map;
  */
 public class TokenEndpoint {
 
+    // Flag if code was already exchanged for token
+    private static final String CODE_EXCHANGED = "CODE_EXCHANGED";
+
     private static final ServicesLogger logger = ServicesLogger.ROOT_LOGGER;
     private MultivaluedMap<String, String> formParams;
     private ClientModel client;
@@ -215,12 +218,23 @@ public class TokenEndpoint {
 
         ClientSessionModel clientSession = accessCode.getClientSession();
         event.detail(Details.CODE_ID, clientSession.getId());
+
+        String codeExchanged = clientSession.getNote(CODE_EXCHANGED);
+        if (codeExchanged != null && Boolean.parseBoolean(codeExchanged)) {
+            logger.codeUsedAlready(code);
+            session.sessions().removeClientSession(realm, clientSession);
+
+            event.error(Errors.INVALID_CODE);
+            throw new ErrorResponseException("invalid_grant", "Code used already", Response.Status.BAD_REQUEST);
+        }
+
         if (!accessCode.isValid(ClientSessionModel.Action.CODE_TO_TOKEN.name(), ClientSessionCode.ActionType.CLIENT)) {
             event.error(Errors.INVALID_CODE);
             throw new ErrorResponseException("invalid_grant", "Code is expired", Response.Status.BAD_REQUEST);
         }
 
         accessCode.setAction(null);
+        clientSession.setNote(CODE_EXCHANGED, "true");
         UserSessionModel userSession = clientSession.getUserSession();
 
         if (userSession == null) {

services/src/main/java/org/keycloak/services/ServicesLogger.java

@@ -402,4 +402,8 @@ public interface ServicesLogger extends BasicLogger {
     @LogMessage(level = ERROR)
     @Message(id=90, value="Failed to close ProviderSession")
     void failedToCloseProviderSession(@Cause Throwable t);
+
+    @LogMessage(level = WARN)
+    @Message(id=91, value="Attempt to re-use code '%s'")
+    void codeUsedAlready(String code);
 }

testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/UserInfoClientUtil.java

@@ -0,0 +1,65 @@
+/*
+ * Copyright 2016 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * 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 org.keycloak.testsuite.util;
+
+import java.net.URI;
+
+import javax.ws.rs.client.Client;
+import javax.ws.rs.client.WebTarget;
+import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.core.UriBuilder;
+
+import org.junit.Assert;
+import org.keycloak.protocol.oidc.OIDCLoginProtocolService;
+import org.keycloak.representations.UserInfo;
+
+/**
+ * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
+ */
+public class UserInfoClientUtil {
+
+    public static Response executeUserInfoRequest_getMethod(Client client, String accessToken) {
+        WebTarget userInfoTarget = getUserInfoWebTarget(client);
+
+        return userInfoTarget.request()
+                .header(HttpHeaders.AUTHORIZATION, "bearer " + accessToken)
+                .get();
+    }
+
+    public static WebTarget getUserInfoWebTarget(Client client) {
+        UriBuilder builder = UriBuilder.fromUri(OAuthClient.AUTH_SERVER_ROOT);
+        UriBuilder uriBuilder = OIDCLoginProtocolService.tokenServiceBaseUrl(builder);
+        URI userInfoUri = uriBuilder.path(OIDCLoginProtocolService.class, "issueUserInfo").build("test");
+        return client.target(userInfoUri);
+    }
+
+    public static void testSuccessfulUserInfoResponse(Response response, String expectedUsername, String expectedEmail) {
+        Assert.assertEquals(Response.Status.OK.getStatusCode(), response.getStatus());
+
+        UserInfo userInfo = response.readEntity(UserInfo.class);
+
+        response.close();
+
+        Assert.assertNotNull(userInfo);
+        Assert.assertNotNull(userInfo.getSubject());
+        Assert.assertEquals(expectedEmail, userInfo.getEmail());
+        Assert.assertEquals(expectedUsername, userInfo.getPreferredUsername());
+    }
+
+}

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java

@@ -16,6 +16,8 @@
  */
 package org.keycloak.testsuite.oauth;
 
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import org.apache.http.NameValuePair;
 import org.apache.http.client.entity.UrlEncodedFormEntity;
 import org.apache.http.client.methods.HttpPost;
@@ -32,10 +34,8 @@ import org.keycloak.admin.client.resource.ClientTemplateResource;
 import org.keycloak.admin.client.resource.RealmResource;
 import org.keycloak.admin.client.resource.UserResource;
 import org.keycloak.common.enums.SslRequired;
-import org.keycloak.common.util.PemUtils;
 import org.keycloak.events.Details;
 import org.keycloak.events.Errors;
-import org.keycloak.jose.jwk.JWKBuilder;
 import org.keycloak.jose.jws.JWSHeader;
 import org.keycloak.jose.jws.JWSInput;
 import org.keycloak.jose.jws.JWSInputException;
@@ -62,6 +62,7 @@ import org.keycloak.testsuite.util.OAuthClient;
 import org.keycloak.testsuite.util.RealmManager;
 import org.keycloak.testsuite.util.RoleBuilder;
 import org.keycloak.testsuite.util.UserBuilder;
+import org.keycloak.testsuite.util.UserInfoClientUtil;
 import org.keycloak.testsuite.util.UserManager;
 import org.keycloak.util.BasicAuthHelper;
 
@@ -72,6 +73,8 @@ import javax.ws.rs.core.Form;
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriBuilder;
+
+import java.io.IOException;
 import java.net.URI;
 import java.util.Arrays;
 import java.util.LinkedList;
@@ -320,7 +323,7 @@ public class AccessTokenTest extends AbstractKeycloakTest {
     }
 
     @Test
-    public void accessTokenCodeUsed() {
+    public void accessTokenCodeUsed() throws IOException {
         oauth.doLogin("test-user@localhost", "password");
 
         EventRepresentation loginEvent = events.expectLogin().assertEvent();
@@ -331,23 +334,53 @@ public class AccessTokenTest extends AbstractKeycloakTest {
         String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
         OAuthClient.AccessTokenResponse response = oauth.doAccessTokenRequest(code, "password");
         Assert.assertEquals(200, response.getStatusCode());
+        String accessToken = response.getAccessToken();
 
-        events.clear();
+        Client jaxrsClient = javax.ws.rs.client.ClientBuilder.newClient();
+        try {
+            // Check that userInfo can be invoked
+            Response userInfoResponse = UserInfoClientUtil.executeUserInfoRequest_getMethod(jaxrsClient, accessToken);
+            UserInfoClientUtil.testSuccessfulUserInfoResponse(userInfoResponse, "test-user@localhost", "test-user@localhost");
 
-        response = oauth.doAccessTokenRequest(code, "password");
+            // Check that tokenIntrospection can be invoked
-        Assert.assertEquals(400, response.getStatusCode());
+            String introspectionResponse = oauth.introspectAccessTokenWithClientCredential("test-app", "password", accessToken);
+            ObjectMapper objectMapper = new ObjectMapper();
+            JsonNode jsonNode = objectMapper.readTree(introspectionResponse);
+            Assert.assertEquals(true, jsonNode.get("active").asBoolean());
+            Assert.assertEquals("test-user@localhost", jsonNode.get("email").asText());
 
-        AssertEvents.ExpectedEvent expectedEvent = events.expectCodeToToken(codeId, null);
+            events.clear();
-        expectedEvent.error("invalid_code")
-                .removeDetail(Details.TOKEN_ID)
-                .removeDetail(Details.REFRESH_TOKEN_ID)
-                .removeDetail(Details.REFRESH_TOKEN_TYPE)
-                .user((String) null);
-        expectedEvent.assertEvent();
 
-        events.clear();
+            // Repeating attempt to exchange code should be refused and invalidate previous clientSession
+            response = oauth.doAccessTokenRequest(code, "password");
+            Assert.assertEquals(400, response.getStatusCode());
 
-        RealmManager.realm(adminClient.realm("test")).accessCodeLifeSpan(60);
+            AssertEvents.ExpectedEvent expectedEvent = events.expectCodeToToken(codeId, null);
+            expectedEvent.error("invalid_code")
+                    .removeDetail(Details.TOKEN_ID)
+                    .removeDetail(Details.REFRESH_TOKEN_ID)
+                    .removeDetail(Details.REFRESH_TOKEN_TYPE)
+                    .user((String) null);
+            expectedEvent.assertEvent();
+
+            // Check that userInfo can't be invoked with invalidated accessToken
+            userInfoResponse = UserInfoClientUtil.executeUserInfoRequest_getMethod(jaxrsClient, accessToken);
+            assertEquals(Response.Status.FORBIDDEN.getStatusCode(), userInfoResponse.getStatus());
+            userInfoResponse.close();
+
+            // Check that tokenIntrospection can't be invoked with invalidated accessToken
+            introspectionResponse = oauth.introspectAccessTokenWithClientCredential("test-app", "password", accessToken);
+            objectMapper = new ObjectMapper();
+            jsonNode = objectMapper.readTree(introspectionResponse);
+            Assert.assertEquals(false, jsonNode.get("active").asBoolean());
+            Assert.assertNull(jsonNode.get("email"));
+
+            events.clear();
+
+            RealmManager.realm(adminClient.realm("test")).accessCodeLifeSpan(60);
+        } finally {
+            jaxrsClient.close();
+        }
     }
 
     @Test

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/UserInfoTest.java

@@ -28,6 +28,7 @@ import org.keycloak.testsuite.AbstractKeycloakTest;
 import org.keycloak.testsuite.AssertEvents;
 import org.keycloak.testsuite.util.ClientManager;
 import org.keycloak.testsuite.util.RealmBuilder;
+import org.keycloak.testsuite.util.UserInfoClientUtil;
 import org.keycloak.util.BasicAuthHelper;
 
 import javax.ws.rs.client.Client;
@@ -75,12 +76,12 @@ public class UserInfoTest extends AbstractKeycloakTest {
     }
 
     @Test
-    public void testSuccess_getMethod_bearer() throws Exception {
+    public void testSuccess_getMethod_header() throws Exception {
         Client client = ClientBuilder.newClient();
 
         try {
             AccessTokenResponse accessTokenResponse = executeGrantAccessTokenRequest(client);
-            Response response = executeUserInfoRequest_getMethod(client, accessTokenResponse.getToken());
+            Response response = UserInfoClientUtil.executeUserInfoRequest_getMethod(client, accessTokenResponse.getToken());
 
             testSuccessfulUserInfoResponse(response);
 
@@ -90,13 +91,13 @@ public class UserInfoTest extends AbstractKeycloakTest {
     }
 
     @Test
-    public void testSuccess_postMethod_bearer() throws Exception {
+    public void testSuccess_postMethod_header() throws Exception {
         Client client = ClientBuilder.newClient();
 
         try {
             AccessTokenResponse accessTokenResponse = executeGrantAccessTokenRequest(client);
 
-            WebTarget userInfoTarget = getUserInfoWebTarget(client);
+            WebTarget userInfoTarget = UserInfoClientUtil.getUserInfoWebTarget(client);
             Response response = userInfoTarget.request()
                     .header(HttpHeaders.AUTHORIZATION, "bearer " + accessTokenResponse.getToken())
                     .post(Entity.form(new Form()));
@@ -118,7 +119,7 @@ public class UserInfoTest extends AbstractKeycloakTest {
             Form form = new Form();
             form.param("access_token", accessTokenResponse.getToken());
 
-            WebTarget userInfoTarget = getUserInfoWebTarget(client);
+            WebTarget userInfoTarget = UserInfoClientUtil.getUserInfoWebTarget(client);
             Response response = userInfoTarget.request()
                     .post(Entity.form(form));
 
@@ -130,13 +131,13 @@ public class UserInfoTest extends AbstractKeycloakTest {
     }
 
     @Test
-    public void testSuccess_postMethod_bearer_textEntity() throws Exception {
+    public void testSuccess_postMethod_header_textEntity() throws Exception {
         Client client = ClientBuilder.newClient();
 
         try {
             AccessTokenResponse accessTokenResponse = executeGrantAccessTokenRequest(client);
 
-            WebTarget userInfoTarget = getUserInfoWebTarget(client);
+            WebTarget userInfoTarget = UserInfoClientUtil.getUserInfoWebTarget(client);
             Response response = userInfoTarget.request()
                     .header(HttpHeaders.AUTHORIZATION, "bearer " + accessTokenResponse.getToken())
                     .post(Entity.text(""));
@@ -157,7 +158,7 @@ public class UserInfoTest extends AbstractKeycloakTest {
 
             testingClient.testing().removeUserSessions("test");
 
-            Response response = executeUserInfoRequest_getMethod(client, accessTokenResponse.getToken());
+            Response response = UserInfoClientUtil.executeUserInfoRequest_getMethod(client, accessTokenResponse.getToken());
 
             assertEquals(Status.FORBIDDEN.getStatusCode(), response.getStatus());
 
@@ -173,7 +174,7 @@ public class UserInfoTest extends AbstractKeycloakTest {
         Client client = ClientBuilder.newClient();
 
         try {
-            Response response = executeUserInfoRequest_getMethod(client, "bad");
+            Response response = UserInfoClientUtil.executeUserInfoRequest_getMethod(client, "bad");
 
             response.close();
 
@@ -208,31 +209,7 @@ public class UserInfoTest extends AbstractKeycloakTest {
         return accessTokenResponse;
     }
 
-    private Response executeUserInfoRequest_getMethod(Client client, String accessToken) {
-        WebTarget userInfoTarget = getUserInfoWebTarget(client);
-
-        return userInfoTarget.request()
-                .header(HttpHeaders.AUTHORIZATION, "bearer " + accessToken)
-                .get();
-    }
-
-    private WebTarget getUserInfoWebTarget(Client client) {
-        UriBuilder builder = UriBuilder.fromUri(AUTH_SERVER_ROOT);
-        UriBuilder uriBuilder = OIDCLoginProtocolService.tokenServiceBaseUrl(builder);
-        URI userInfoUri = uriBuilder.path(OIDCLoginProtocolService.class, "issueUserInfo").build("test");
-        return client.target(userInfoUri);
-    }
-
     private void testSuccessfulUserInfoResponse(Response response) {
-        assertEquals(Status.OK.getStatusCode(), response.getStatus());
+        UserInfoClientUtil.testSuccessfulUserInfoResponse(response, "test-user@localhost", "test-user@localhost");
-
-        UserInfo userInfo = response.readEntity(UserInfo.class);
-
-        response.close();
-
-        assertNotNull(userInfo);
-        assertNotNull(userInfo.getSubject());
-        assertEquals("test-user@localhost", userInfo.getEmail());
-        assertEquals("test-user@localhost", userInfo.getPreferredUsername());
     }
 }