From 4dd28c0adf322f6a42e87e9e490b902832324b76 Mon Sep 17 00:00:00 2001 From: mposolda Date: Fri, 8 Jul 2016 11:04:08 +0200 Subject: [PATCH] KEYCLOAK-3221 Tokens should be invalidated if an attempt to reuse code is made --- .../keycloak/protocol/oidc/TokenManager.java | 5 ++ .../oidc/endpoints/TokenEndpoint.java | 14 ++++ .../org/keycloak/services/ServicesLogger.java | 4 ++ .../testsuite/util/UserInfoClientUtil.java | 65 +++++++++++++++++++ .../testsuite/oauth/AccessTokenTest.java | 63 +++++++++++++----- .../keycloak/testsuite/oidc/UserInfoTest.java | 45 ++++--------- 6 files changed, 147 insertions(+), 49 deletions(-) create mode 100644 testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/UserInfoClientUtil.java diff --git a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java index 0950d4d5b9..0cb4bbfc7f 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java +++ b/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; } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java index f7d68dc0d5..d84cf57932 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java +++ b/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 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) { diff --git a/services/src/main/java/org/keycloak/services/ServicesLogger.java b/services/src/main/java/org/keycloak/services/ServicesLogger.java index fb71a2e2ad..4536eefe0f 100644 --- a/services/src/main/java/org/keycloak/services/ServicesLogger.java +++ b/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); } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/UserInfoClientUtil.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/UserInfoClientUtil.java new file mode 100644 index 0000000000..3d6033d10e --- /dev/null +++ b/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 Marek Posolda + */ +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()); + } + +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java index effeeadbd4..88d7f3f2fb 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java +++ b/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"); - Assert.assertEquals(400, response.getStatusCode()); + // Check that tokenIntrospection can be invoked + 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); - 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(); - 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 diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/UserInfoTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/UserInfoTest.java index 7e91d96b56..994ce3ae6b 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/UserInfoTest.java +++ b/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()); - - 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()); + UserInfoClientUtil.testSuccessfulUserInfoResponse(response, "test-user@localhost", "test-user@localhost"); } }