From d269af1b70960a2f7dfc2fb71002da2b209e37a8 Mon Sep 17 00:00:00 2001 From: mposolda Date: Tue, 6 Oct 2020 10:20:00 +0200 Subject: [PATCH] KEYCLOAK-15830 Remove authentication session after failed directGrant authentication --- .../protocol/oidc/endpoints/TokenEndpoint.java | 4 ++++ ...ourceOwnerPasswordCredentialsGrantTest.java | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+) 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 9e4576c09c..f7d5744499 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 @@ -637,12 +637,16 @@ public class TokenEndpoint { .setRequest(request); Response challenge = processor.authenticateOnly(); if (challenge != null) { + // Remove authentication session as "Resource Owner Password Credentials Grant" is single-request scoped authentication + new AuthenticationSessionManager(session).removeAuthenticationSession(realm, authSession, false); cors.build(httpResponse); return challenge; } processor.evaluateRequiredActionTriggers(); UserModel user = authSession.getAuthenticatedUser(); if (user.getRequiredActions() != null && user.getRequiredActions().size() > 0) { + // Remove authentication session as "Resource Owner Password Credentials Grant" is single-request scoped authentication + new AuthenticationSessionManager(session).removeAuthenticationSession(realm, authSession, false); event.error(Errors.RESOLVE_REQUIRED_ACTIONS); throw new CorsErrorResponseException(cors, OAuthErrorException.INVALID_GRANT, "Account is not fully set up", Response.Status.BAD_REQUEST); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java index a8d7e6e14d..c0d998908f 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java @@ -31,6 +31,7 @@ import org.keycloak.OAuth2Constants; import org.keycloak.OAuthErrorException; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.authentication.authenticators.client.ClientIdAndSecretAuthenticator; +import org.keycloak.connections.infinispan.InfinispanConnectionProvider; import org.keycloak.crypto.Algorithm; import org.keycloak.events.Details; import org.keycloak.events.Errors; @@ -152,7 +153,12 @@ public class ResourceOwnerPasswordCredentialsGrantTest extends AbstractKeycloakT @Test public void grantAccessTokenUsername() throws Exception { + int authSessionsBefore = getAuthenticationSessionsCount(); + grantAccessToken("direct-login", "resource-owner"); + + // Check that count of authSessions is same as before authentication (as authentication session was removed) + Assert.assertEquals(authSessionsBefore, getAuthenticationSessionsCount()); } @Test @@ -204,6 +210,8 @@ public class ResourceOwnerPasswordCredentialsGrantTest extends AbstractKeycloakT @Test public void grantAccessTokenInvalidTotp() throws Exception { + int authSessionsBefore = getAuthenticationSessionsCount(); + oauth.clientId("resource-owner"); OAuthClient.AccessTokenResponse response = oauth.doGrantAccessTokenRequest("secret", "direct-login-otp", "password", totp.generateTOTP("totpSecret2")); @@ -219,6 +227,9 @@ public class ResourceOwnerPasswordCredentialsGrantTest extends AbstractKeycloakT .error(Errors.INVALID_USER_CREDENTIALS) .user(userId2) .assertEvent(); + + // Check that count of authSessions is same as before authentication (as authentication session was removed) + Assert.assertEquals(authSessionsBefore, getAuthenticationSessionsCount()); } private void grantAccessToken(String login, String clientId) throws Exception { @@ -438,6 +449,7 @@ public class ResourceOwnerPasswordCredentialsGrantTest extends AbstractKeycloakT @Test public void grantAccessTokenVerifyEmail() throws Exception { + int authSessionsBefore = getAuthenticationSessionsCount(); RealmResource realmResource = adminClient.realm("test"); RealmManager.realm(realmResource).verifyEmail(true); @@ -462,6 +474,8 @@ public class ResourceOwnerPasswordCredentialsGrantTest extends AbstractKeycloakT RealmManager.realm(realmResource).verifyEmail(false); UserManager.realm(realmResource).username("test-user@localhost").removeRequiredAction(UserModel.RequiredAction.VERIFY_EMAIL.toString()); + // Check that count of authSessions is same as before authentication (as authentication session was removed) + Assert.assertEquals(authSessionsBefore, getAuthenticationSessionsCount()); } @Test @@ -646,4 +660,8 @@ public class ResourceOwnerPasswordCredentialsGrantTest extends AbstractKeycloakT assertEquals("Unsupported grant_type", response.getErrorDescription()); } } + + private int getAuthenticationSessionsCount() { + return testingClient.testing().cache(InfinispanConnectionProvider.AUTHENTICATION_SESSIONS_CACHE_NAME).size(); + } }