From d6f030a05a25e900adf48f4546e45ef35c2feead Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Fri, 3 Oct 2014 12:10:26 +0200 Subject: [PATCH] KEYCLOAK-662 NPE when using direct grant API with email as username --- .../protocol/oidc/OpenIDConnectService.java | 3 +- ...urceOwnerPasswordCredentialsGrantTest.java | 52 +++++++++++++++++-- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/services/src/main/java/org/keycloak/protocol/oidc/OpenIDConnectService.java b/services/src/main/java/org/keycloak/protocol/oidc/OpenIDConnectService.java index 7ee021b9a7..b0574e9b6c 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/OpenIDConnectService.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/OpenIDConnectService.java @@ -28,6 +28,7 @@ import org.keycloak.models.OAuthClientModel; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.UserSessionModel; +import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.representations.AccessToken; import org.keycloak.representations.AccessTokenResponse; import org.keycloak.representations.RefreshToken; @@ -276,7 +277,7 @@ public class OpenIDConnectService { } event.detail(Details.USERNAME, username); - UserModel user = session.users().getUserByUsername(username, realm); + UserModel user = KeycloakModelUtils.findUserByNameOrEmail(session, realm, username); if (user != null) event.user(user); ClientModel client = authorizeClient(authorizationHeader, form, event); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java index f59c8ef811..f172dec304 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java @@ -8,6 +8,8 @@ import org.keycloak.events.Details; import org.keycloak.events.Errors; import org.keycloak.models.ApplicationModel; import org.keycloak.models.RealmModel; +import org.keycloak.models.UserCredentialModel; +import org.keycloak.models.UserModel; import org.keycloak.representations.AccessToken; import org.keycloak.representations.RefreshToken; import org.keycloak.services.managers.RealmManager; @@ -32,6 +34,14 @@ public class ResourceOwnerPasswordCredentialsGrantTest { ApplicationModel app = appRealm.addApplication("resource-owner"); app.setSecret("secret"); appRealm.setPasswordCredentialGrantAllowed(true); + + UserModel user = session.users().addUser(appRealm, "direct-login"); + user.setEmail("direct-login@localhost"); + user.setEnabled(true); + + userId = user.getId(); + + session.users().updateCredential(appRealm, user, UserCredentialModel.password("password")); } }); @@ -47,11 +57,22 @@ public class ResourceOwnerPasswordCredentialsGrantTest { @WebResource protected OAuthClient oauth; + private static String userId; + @Test - public void grantAccessToken() throws Exception { + public void grantAccessTokenUsername() throws Exception { + grantAccessToken("direct-login"); + } + + @Test + public void grantAccessTokenEmail() throws Exception { + grantAccessToken("direct-login@localhost"); + } + + private void grantAccessToken(String login) throws Exception { oauth.clientId("resource-owner"); - OAuthClient.AccessTokenResponse response = oauth.doGrantAccessTokenRequest("secret", "test-user@localhost", "password"); + OAuthClient.AccessTokenResponse response = oauth.doGrantAccessTokenRequest("secret", login, "password"); assertEquals(200, response.getStatusCode()); @@ -60,11 +81,13 @@ public class ResourceOwnerPasswordCredentialsGrantTest { events.expectLogin() .client("resource-owner") + .user(userId) .session(accessToken.getSessionState()) .detail(Details.AUTH_METHOD, "oauth_credentials") .detail(Details.RESPONSE_TYPE, "token") .detail(Details.TOKEN_ID, accessToken.getId()) .detail(Details.REFRESH_TOKEN_ID, refreshToken.getId()) + .detail(Details.USERNAME, login) .removeDetail(Details.CODE_ID) .removeDetail(Details.REDIRECT_URI) .assertEvent(); @@ -79,7 +102,7 @@ public class ResourceOwnerPasswordCredentialsGrantTest { assertEquals(accessToken.getSessionState(), refreshedAccessToken.getSessionState()); assertEquals(accessToken.getSessionState(), refreshedRefreshToken.getSessionState()); - events.expectRefresh(refreshToken.getId(), refreshToken.getSessionState()).client("resource-owner").assertEvent(); + events.expectRefresh(refreshToken.getId(), refreshToken.getSessionState()).user(userId).client("resource-owner").assertEvent(); } @Test @@ -187,4 +210,27 @@ public class ResourceOwnerPasswordCredentialsGrantTest { .assertEvent(); } + @Test + public void grantAccessTokenUserNotFound() throws Exception { + oauth.clientId("resource-owner"); + + OAuthClient.AccessTokenResponse response = oauth.doGrantAccessTokenRequest("secret", "invalid", "invalid"); + + assertEquals(400, response.getStatusCode()); + + assertEquals("invalid_grant", response.getError()); + + events.expectLogin() + .client("resource-owner") + .user((String) null) + .session((String) null) + .detail(Details.AUTH_METHOD, "oauth_credentials") + .detail(Details.RESPONSE_TYPE, "token") + .detail(Details.USERNAME, "invalid") + .removeDetail(Details.CODE_ID) + .removeDetail(Details.REDIRECT_URI) + .error(Errors.INVALID_USER_CREDENTIALS) + .assertEvent(); + } + }