diff --git a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java index 1508746222..fbe37b482f 100755 --- a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java +++ b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java @@ -596,9 +596,8 @@ public class AuthenticationProcessor { } public void validateUser(UserModel authenticatedUser) { - if (authenticatedUser != null) { - if (!authenticatedUser.isEnabled()) throw new AuthException(Error.USER_DISABLED); - } + if (authenticatedUser == null) return; + if (!authenticatedUser.isEnabled()) throw new AuthException(Error.USER_DISABLED); if (realm.isBruteForceProtected()) { if (protector.isTemporarilyDisabled(session, realm, authenticatedUser.getUsername())) { throw new AuthException(Error.USER_TEMPORARILY_DISABLED); diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java index c9ca6dee28..ea95c14310 100755 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java @@ -592,121 +592,6 @@ public class AuthenticationManager { return null; } - public AuthenticationStatus authenticateForm(KeycloakSession session, ClientConnection clientConnection, RealmModel realm, MultivaluedMap formData) { - String username = formData.getFirst(FORM_USERNAME); - if (username == null) { - logger.debug("Username not provided"); - return AuthenticationStatus.INVALID_USER; - } - - if (realm.isBruteForceProtected()) { - if (protector.isTemporarilyDisabled(session, realm, username)) { - return AuthenticationStatus.ACCOUNT_TEMPORARILY_DISABLED; - } - } - - AuthenticationStatus status = authenticateInternal(session, realm, formData, username); - if (realm.isBruteForceProtected()) { - switch (status) { - case SUCCESS: - protector.successfulLogin(realm, username, clientConnection); - break; - case FAILED: - case MISSING_TOTP: - case MISSING_PASSWORD: - case INVALID_CREDENTIALS: - protector.failedLogin(realm, username, clientConnection); - break; - case INVALID_USER: - protector.invalidUser(realm, username, clientConnection); - break; - default: - break; - } - } - - return status; - } - - protected AuthenticationStatus authenticateInternal(KeycloakSession session, RealmModel realm, MultivaluedMap formData, String username) { - UserModel user = KeycloakModelUtils.findUserByNameOrEmail(session, realm, username); - - if (user == null) { - logger.debugv("User {0} not found", username); - return AuthenticationStatus.INVALID_USER; - } - - Set types = new HashSet(); - - for (RequiredCredentialModel credential : realm.getRequiredCredentials()) { - types.add(credential.getType()); - } - - if (types.contains(CredentialRepresentation.PASSWORD)) { - List credentials = new LinkedList(); - - String password = formData.getFirst(CredentialRepresentation.PASSWORD); - if (password != null) { - credentials.add(UserCredentialModel.password(password)); - } - - String passwordToken = formData.getFirst(CredentialRepresentation.PASSWORD_TOKEN); - if (passwordToken != null) { - credentials.add(UserCredentialModel.passwordToken(passwordToken)); - } - - String totp = formData.getFirst(CredentialRepresentation.TOTP); - if (totp != null) { - credentials.add(UserCredentialModel.totp(totp)); - } - - if ((password == null || password.isEmpty()) && (passwordToken == null || passwordToken.isEmpty())) { - logger.debug("Password not provided"); - return AuthenticationStatus.MISSING_PASSWORD; - } - - logger.debugv("validating password for user: {0}", username); - - if (!session.users().validCredentials(realm, user, credentials)) { - return AuthenticationStatus.INVALID_CREDENTIALS; - } - - if (!user.isEnabled()) { - return AuthenticationStatus.ACCOUNT_DISABLED; - } - - if (user.isTotp() && totp == null) { - return AuthenticationStatus.MISSING_TOTP; - } - - if (!user.getRequiredActions().isEmpty()) { - return AuthenticationStatus.ACTIONS_REQUIRED; - } else { - return AuthenticationStatus.SUCCESS; - } - } else if (types.contains(CredentialRepresentation.SECRET)) { - String secret = formData.getFirst(CredentialRepresentation.SECRET); - if (secret == null) { - logger.debug("Secret not provided"); - return AuthenticationStatus.MISSING_PASSWORD; - } - if (!session.users().validCredentials(realm, user, UserCredentialModel.secret(secret))) { - return AuthenticationStatus.INVALID_CREDENTIALS; - } - if (!user.isEnabled()) { - return AuthenticationStatus.ACCOUNT_DISABLED; - } - if (!user.getRequiredActions().isEmpty()) { - return AuthenticationStatus.ACTIONS_REQUIRED; - } else { - return AuthenticationStatus.SUCCESS; - } - } else { - logger.warn("Do not know how to authenticate user"); - return AuthenticationStatus.FAILED; - } - } - public enum AuthenticationStatus { SUCCESS, ACCOUNT_TEMPORARILY_DISABLED, ACCOUNT_DISABLED, ACTIONS_REQUIRED, INVALID_USER, INVALID_CREDENTIALS, MISSING_PASSWORD, MISSING_TOTP, FAILED } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTest.java index 1799a644f0..745a5a2e1d 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTest.java @@ -137,6 +137,21 @@ public class LoginTest { .assertEvent(); } + @Test + public void loginMissingPassword() { + loginPage.open(); + loginPage.missingPassword("login-test"); + + loginPage.assertCurrent(); + + Assert.assertEquals("Invalid username or password.", loginPage.getError()); + + events.expectLogin().user(userId).session((String) null).error("invalid_user_credentials") + .detail(Details.USERNAME, "login-test") + .removeDetail(Details.CONSENT) + .assertEvent(); + } + @Test public void loginInvalidPasswordDisabledUser() { keycloakRule.configure(new KeycloakRule.KeycloakSetup() { @@ -214,6 +229,20 @@ public class LoginTest { .assertEvent(); } + @Test + public void loginMissingUsername() { + loginPage.open(); + loginPage.missingUsername(); + + loginPage.assertCurrent(); + + Assert.assertEquals("Invalid username or password.", loginPage.getError()); + + events.expectLogin().user((String) null).session((String) null).error("user_not_found") + .removeDetail(Details.CONSENT) + .assertEvent(); + } + @Test public void loginSuccess() { loginPage.open(); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTotpTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTotpTest.java index 3ea0915d1a..03747150d3 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTotpTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTotpTest.java @@ -121,6 +121,25 @@ public class LoginTotpTest { .assertEvent(); } + @Test + public void loginWithMissingTotp() throws Exception { + loginPage.open(); + loginPage.login("test-user@localhost", "password"); + + loginTotpPage.assertCurrent(); + + loginTotpPage.login(null); + loginTotpPage.assertCurrent(); + Assert.assertEquals("Invalid authenticator code.", loginPage.getError()); + + //loginPage.assertCurrent(); // Invalid authenticator code. + //Assert.assertEquals("Invalid username or password.", loginPage.getError()); + + events.expectLogin().error("invalid_user_credentials").session((String) null) + .removeDetail(Details.CONSENT) + .assertEvent(); + } + @Test public void loginWithTotpSuccess() throws Exception { loginPage.open(); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/AuthenticationManagerTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/AuthenticationManagerTest.java deleted file mode 100755 index 11e1887cbd..0000000000 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/AuthenticationManagerTest.java +++ /dev/null @@ -1,288 +0,0 @@ -package org.keycloak.testsuite.model; - -import org.jboss.resteasy.specimpl.MultivaluedMapImpl; -import org.jboss.resteasy.spi.ResteasyProviderFactory; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.keycloak.ClientConnection; -import org.keycloak.jose.jws.JWSBuilder; -import org.keycloak.models.RealmModel; -import org.keycloak.models.UserCredentialModel; -import org.keycloak.models.UserModel; -import org.keycloak.models.UserModel.RequiredAction; -import org.keycloak.models.utils.KeycloakModelUtils; -import org.keycloak.models.utils.TimeBasedOTP; -import org.keycloak.representations.PasswordToken; -import org.keycloak.representations.idm.CredentialRepresentation; -import org.keycloak.services.managers.AuthenticationManager; -import org.keycloak.services.managers.AuthenticationManager.AuthenticationStatus; -import org.keycloak.services.managers.BruteForceProtector; -import org.keycloak.util.Time; - -import javax.ws.rs.core.MultivaluedMap; -import java.util.UUID; - -public class AuthenticationManagerTest extends AbstractModelTest { - - private AuthenticationManager am; - private MultivaluedMap formData; - private TimeBasedOTP otp; - private RealmModel realm; - private UserModel user; - private BruteForceProtector protector; - private ClientConnection dummyConnection = new ClientConnection() { - @Override - public String getRemoteAddr() { - return "127.0.0.1"; - - } - - @Override - public String getRemoteHost() { - return "localhost"; - - } - - @Override - public int getReportPort() { - return 8080; - } - }; - - @Test - public void authForm() { - AuthenticationStatus status = am.authenticateForm(session, dummyConnection, realm, formData); - Assert.assertEquals(AuthenticationStatus.SUCCESS, status); - } - - @Test - public void authFormInvalidPassword() { - formData.remove(CredentialRepresentation.PASSWORD); - formData.add(CredentialRepresentation.PASSWORD, "invalid"); - - AuthenticationStatus status = am.authenticateForm(session, dummyConnection, realm, formData); - Assert.assertEquals(AuthenticationStatus.INVALID_CREDENTIALS, status); - } - - @Test - public void authFormMissingUsername() { - formData.remove("username"); - - AuthenticationStatus status = am.authenticateForm(session, dummyConnection, realm, formData); - Assert.assertEquals(AuthenticationStatus.INVALID_USER, status); - } - - @Test - public void authFormMissingPassword() { - formData.remove(CredentialRepresentation.PASSWORD); - - AuthenticationStatus status = am.authenticateForm(session, dummyConnection, realm, formData); - Assert.assertEquals(AuthenticationStatus.MISSING_PASSWORD, status); - } - - @Test - public void authFormRequiredAction() { - realm.addRequiredCredential(CredentialRepresentation.TOTP); - user.addRequiredAction(RequiredAction.CONFIGURE_TOTP); - - AuthenticationStatus status = am.authenticateForm(session, dummyConnection, realm, formData); - Assert.assertEquals(AuthenticationStatus.ACTIONS_REQUIRED, status); - } - - @Test - public void authFormUserDisabled() { - user.setEnabled(false); - - AuthenticationStatus status = am.authenticateForm(session, dummyConnection, realm, formData); - Assert.assertEquals(AuthenticationStatus.ACCOUNT_DISABLED, status); - } - - @Test - public void authFormWithTotp() { - realm.addRequiredCredential(CredentialRepresentation.TOTP); - - String totpSecret = UUID.randomUUID().toString(); - - UserCredentialModel credential = new UserCredentialModel(); - credential.setType(CredentialRepresentation.TOTP); - credential.setValue(totpSecret); - - user.updateCredential(credential); - - user.setTotp(true); - - String token = otp.generate(totpSecret); - - formData.add(CredentialRepresentation.TOTP, token); - - AuthenticationStatus status = am.authenticateForm(session, dummyConnection, realm, formData); - Assert.assertEquals(AuthenticationStatus.SUCCESS, status); - } - - @Test - public void authFormWithTotpInvalidPassword() { - authFormWithTotp(); - - formData.remove(CredentialRepresentation.PASSWORD); - formData.add(CredentialRepresentation.PASSWORD, "invalid"); - - AuthenticationStatus status = am.authenticateForm(session, dummyConnection, realm, formData); - Assert.assertEquals(AuthenticationStatus.INVALID_CREDENTIALS, status); - } - - @Test - public void authFormWithTotpMissingPassword() { - authFormWithTotp(); - - formData.remove(CredentialRepresentation.PASSWORD); - - AuthenticationStatus status = am.authenticateForm(session, dummyConnection, realm, formData); - Assert.assertEquals(AuthenticationStatus.MISSING_PASSWORD, status); - } - - @Test - public void authFormWithTotpInvalidTotp() { - authFormWithTotp(); - - formData.remove(CredentialRepresentation.TOTP); - formData.add(CredentialRepresentation.TOTP, "invalid"); - - AuthenticationStatus status = am.authenticateForm(session, dummyConnection, realm, formData); - Assert.assertEquals(AuthenticationStatus.INVALID_CREDENTIALS, status); - } - - @Test - public void authFormWithTotpMissingTotp() { - authFormWithTotp(); - - formData.remove(CredentialRepresentation.TOTP); - - AuthenticationStatus status = am.authenticateForm(session, dummyConnection, realm, formData); - Assert.assertEquals(AuthenticationStatus.MISSING_TOTP, status); - } - - @Test - public void authFormWithTotpPasswordToken() { - realm.addRequiredCredential(CredentialRepresentation.TOTP); - - String totpSecret = UUID.randomUUID().toString(); - - UserCredentialModel credential = new UserCredentialModel(); - credential.setType(CredentialRepresentation.TOTP); - credential.setValue(totpSecret); - - user.updateCredential(credential); - - user.setTotp(true); - - String token = otp.generate(totpSecret); - - formData.add(CredentialRepresentation.TOTP, token); - formData.remove(CredentialRepresentation.PASSWORD); - - String passwordToken = new JWSBuilder().jsonContent(new PasswordToken(realm.getName(), user.getId())).rsa256(realm.getPrivateKey()); - formData.add(CredentialRepresentation.PASSWORD_TOKEN, passwordToken); - - AuthenticationStatus status = am.authenticateForm(session, dummyConnection, realm, formData); - Assert.assertEquals(AuthenticationStatus.SUCCESS, status); - } - - @Test - public void authFormWithTotpPasswordTokenInvalidKey() { - authFormWithTotpPasswordToken(); - - formData.remove(CredentialRepresentation.PASSWORD_TOKEN); - String passwordToken = new JWSBuilder().jsonContent(new PasswordToken(realm.getName(), user.getId())).rsa256(realm.getPrivateKey()); - formData.add(CredentialRepresentation.PASSWORD_TOKEN, passwordToken); - - KeycloakModelUtils.generateRealmKeys(realm); - - AuthenticationStatus status = am.authenticateForm(session, dummyConnection, realm, formData); - Assert.assertEquals(AuthenticationStatus.INVALID_CREDENTIALS, status); - } - - @Test - public void authFormWithTotpPasswordTokenInvalidRealm() { - authFormWithTotpPasswordToken(); - - formData.remove(CredentialRepresentation.PASSWORD_TOKEN); - String passwordToken = new JWSBuilder().jsonContent(new PasswordToken("invalid", user.getId())).rsa256(realm.getPrivateKey()); - formData.add(CredentialRepresentation.PASSWORD_TOKEN, passwordToken); - - AuthenticationStatus status = am.authenticateForm(session, dummyConnection, realm, formData); - Assert.assertEquals(AuthenticationStatus.INVALID_CREDENTIALS, status); - } - - @Test - public void authFormWithTotpPasswordTokenInvalidUser() { - authFormWithTotpPasswordToken(); - - formData.remove(CredentialRepresentation.PASSWORD_TOKEN); - String passwordToken = new JWSBuilder().jsonContent(new PasswordToken(realm.getName(), "invalid")).rsa256(realm.getPrivateKey()); - formData.add(CredentialRepresentation.PASSWORD_TOKEN, passwordToken); - - AuthenticationStatus status = am.authenticateForm(session, dummyConnection, realm, formData); - Assert.assertEquals(AuthenticationStatus.INVALID_CREDENTIALS, status); - } - - @Test - public void authFormWithTotpPasswordTokenExpired() throws InterruptedException { - int lifespan = realm.getAccessCodeLifespanUserAction(); - - try { - authFormWithTotpPasswordToken(); - - realm.setAccessCodeLifespanUserAction(1); - - formData.remove(CredentialRepresentation.PASSWORD_TOKEN); - String passwordToken = new JWSBuilder().jsonContent(new PasswordToken(realm.getName(), "invalid")).rsa256(realm.getPrivateKey()); - formData.add(CredentialRepresentation.PASSWORD_TOKEN, passwordToken); - - Time.setOffset(2); - - AuthenticationStatus status = am.authenticateForm(session, dummyConnection, realm, formData); - Assert.assertEquals(AuthenticationStatus.INVALID_CREDENTIALS, status); - - Time.setOffset(0); - } finally { - realm.setAccessCodeLifespanUserAction(lifespan); - } - } - - @Before - @Override - public void before() throws Exception { - super.before(); - - realm = realmManager.createRealm("TestAuth"); - realm.setAccessCodeLifespan(100); - realm.setAccessCodeLifespanUserAction(100); - realm.setEnabled(true); - realm.setName("TestAuth"); - - KeycloakModelUtils.generateRealmKeys(realm); - - realm.setAccessTokenLifespan(1000); - realm.addRequiredCredential(CredentialRepresentation.PASSWORD); - - protector = ResteasyProviderFactory.getContextData(BruteForceProtector.class); - am = new AuthenticationManager(protector); - - user = realmManager.getSession().users().addUser(realm, "test"); - user.setEnabled(true); - - UserCredentialModel credential = new UserCredentialModel(); - credential.setType(CredentialRepresentation.PASSWORD); - credential.setValue("password"); - - user.updateCredential(credential); - - formData = new MultivaluedMapImpl(); - formData.add("username", "test"); - formData.add(CredentialRepresentation.PASSWORD, "password"); - - otp = new TimeBasedOTP(); - } - -} diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/LoginPage.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/LoginPage.java old mode 100644 new mode 100755 index 2be207c5c8..2ea8b62ffe --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/LoginPage.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/LoginPage.java @@ -91,6 +91,19 @@ public class LoginPage extends AbstractPage { submitButton.click(); } + public void missingPassword(String username) { + usernameInput.clear(); + usernameInput.sendKeys(username); + passwordInput.clear(); + submitButton.click(); + + } + public void missingUsername() { + usernameInput.clear(); + submitButton.click(); + + } + public String getUsername() { return usernameInput.getAttribute("value"); } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/LoginTotpPage.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/LoginTotpPage.java old mode 100644 new mode 100755 index e1a934a61a..b725a169b9 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/LoginTotpPage.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/LoginTotpPage.java @@ -43,7 +43,8 @@ public class LoginTotpPage extends AbstractPage { private WebElement loginErrorMessage; public void login(String totp) { - totpInput.sendKeys(totp); + totpInput.clear(); + if (totp != null) totpInput.sendKeys(totp); submitButton.click(); }