From 0cb6053699f0460bc827d29115f78ae7cd374b81 Mon Sep 17 00:00:00 2001 From: Martin Kanis Date: Fri, 12 Oct 2018 11:02:18 +0200 Subject: [PATCH] KEYCLOAK-8125 --- .../AbstractUsernameFormAuthenticator.java | 38 ++--- .../browser/OTPFormAuthenticator.java | 22 ++- .../challenge/BasicAuthAuthenticator.java | 38 +---- .../challenge/BasicAuthOTPAuthenticator.java | 20 ++- .../cli/CliUsernamePasswordAuthenticator.java | 37 +---- .../ConsoleUsernamePasswordAuthenticator.java | 24 +-- .../protocol/docker/DockerAuthenticator.java | 6 +- .../authenticator/HttpBasicAuthenticator.java | 25 ++- .../testsuite/forms/BruteForceTest.java | 82 ++++++++-- .../testsuite/forms/FlowOverrideTest.java | 145 ++++++++++++++++++ 10 files changed, 303 insertions(+), 134 deletions(-) diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java index 3448826a29..eac2558b0b 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java @@ -25,6 +25,7 @@ import org.keycloak.credential.CredentialInput; import org.keycloak.credential.hash.PasswordHashProvider; import org.keycloak.events.Details; import org.keycloak.events.Errors; +import org.keycloak.forms.login.LoginFormsProvider; import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.PasswordPolicy; import org.keycloak.models.UserCredentialModel; @@ -56,25 +57,19 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth } - protected Response invalidUser(AuthenticationFlowContext context) { - return context.form() - .setError(Messages.INVALID_USER) - .createLogin(); + protected Response challenge(AuthenticationFlowContext context, String error) { + LoginFormsProvider form = context.form(); + if (error != null) form.setError(error); + + return createLoginForm(form); } - protected Response disabledUser(AuthenticationFlowContext context) { - return context.form() - .setError(Messages.ACCOUNT_DISABLED).createLogin(); + protected Response createLoginForm(LoginFormsProvider form) { + return form.createLogin(); } - protected Response temporarilyDisabledUser(AuthenticationFlowContext context) { - return context.form() - .setError(Messages.INVALID_USER).createLogin(); - } - - protected Response invalidCredentials(AuthenticationFlowContext context) { - return context.form() - .setError(Messages.INVALID_USER).createLogin(); + protected String tempDisabledError() { + return Messages.INVALID_USER; } protected Response setDuplicateUserChallenge(AuthenticationFlowContext context, String eventError, String loginFormError, AuthenticationFlowError authenticatorError) { @@ -112,7 +107,7 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth if (user == null) { dummyHash(context); context.getEvent().error(Errors.USER_NOT_FOUND); - Response challengeResponse = invalidUser(context); + Response challengeResponse = challenge(context, Messages.INVALID_USER); context.failureChallenge(AuthenticationFlowError.INVALID_USER, challengeResponse); return true; } @@ -123,7 +118,7 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth if (!user.isEnabled()) { context.getEvent().user(user); context.getEvent().error(Errors.USER_DISABLED); - Response challengeResponse = disabledUser(context); + Response challengeResponse = challenge(context, Messages.ACCOUNT_DISABLED); // this is not a failure so don't call failureChallenge. //context.failureChallenge(AuthenticationFlowError.USER_DISABLED, challengeResponse); context.forceChallenge(challengeResponse); @@ -137,7 +132,7 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth String username = inputData.getFirst(AuthenticationManager.FORM_USERNAME); if (username == null) { context.getEvent().error(Errors.USER_NOT_FOUND); - Response challengeResponse = invalidUser(context); + Response challengeResponse = challenge(context, Messages.INVALID_USER); context.failureChallenge(AuthenticationFlowError.INVALID_USER, challengeResponse); return false; } @@ -200,19 +195,19 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth } else { context.getEvent().user(user); context.getEvent().error(Errors.INVALID_USER_CREDENTIALS); - Response challengeResponse = invalidCredentials(context); + Response challengeResponse = challenge(context, Messages.INVALID_USER); context.failureChallenge(AuthenticationFlowError.INVALID_CREDENTIALS, challengeResponse); context.clearUser(); return false; } } - private boolean isTemporarilyDisabledByBruteForce(AuthenticationFlowContext context, UserModel user) { + protected boolean isTemporarilyDisabledByBruteForce(AuthenticationFlowContext context, UserModel user) { if (context.getRealm().isBruteForceProtected()) { if (context.getProtector().isTemporarilyDisabled(context.getSession(), context.getRealm(), user)) { context.getEvent().user(user); context.getEvent().error(Errors.USER_TEMPORARILY_DISABLED); - Response challengeResponse = temporarilyDisabledUser(context); + Response challengeResponse = challenge(context, tempDisabledError()); // this is not a failure so don't call failureChallenge. //context.failureChallenge(AuthenticationFlowError.USER_TEMPORARILY_DISABLED, challengeResponse); context.forceChallenge(challengeResponse); @@ -221,5 +216,4 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth } return false; } - } diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java index 91266895db..14ecef5412 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java @@ -54,16 +54,23 @@ public class OTPFormAuthenticator extends AbstractUsernameFormAuthenticator impl context.resetFlow(); return; } + + UserModel userModel = context.getUser(); + if (!enabledUser(context, userModel)) { + // error in context is set in enabledUser/isTemporarilyDisabledByBruteForce + return; + } + String password = inputData.getFirst(CredentialRepresentation.TOTP); if (password == null) { Response challengeResponse = challenge(context, null); context.challenge(challengeResponse); return; } - boolean valid = context.getSession().userCredentialManager().isValid(context.getRealm(), context.getUser(), + boolean valid = context.getSession().userCredentialManager().isValid(context.getRealm(), userModel, UserCredentialModel.otp(context.getRealm().getOTPPolicy().getType(), password)); if (!valid) { - context.getEvent().user(context.getUser()) + context.getEvent().user(userModel) .error(Errors.INVALID_USER_CREDENTIALS); Response challengeResponse = challenge(context, Messages.INVALID_TOTP); context.failureChallenge(AuthenticationFlowError.INVALID_CREDENTIALS, challengeResponse); @@ -77,11 +84,14 @@ public class OTPFormAuthenticator extends AbstractUsernameFormAuthenticator impl return true; } - protected Response challenge(AuthenticationFlowContext context, String error) { - LoginFormsProvider forms = context.form(); - if (error != null) forms.setError(error); + @Override + protected String tempDisabledError() { + return Messages.INVALID_TOTP; + } - return forms.createLoginTotp(); + @Override + protected Response createLoginForm(LoginFormsProvider form) { + return form.createLoginTotp(); } @Override diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/challenge/BasicAuthAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/challenge/BasicAuthAuthenticator.java index 74e8760e54..57fa40e1fb 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/challenge/BasicAuthAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/challenge/BasicAuthAuthenticator.java @@ -49,14 +49,14 @@ public class BasicAuthAuthenticator extends AbstractUsernameFormAuthenticator im String authorizationHeader = getAuthorizationHeader(context); if (authorizationHeader == null) { - context.challenge(challengeResponse(context)); + context.challenge(challenge(context, null)); return; } String[] challenge = getChallenge(authorizationHeader); if (challenge == null) { - context.challenge(challengeResponse(context)); + context.challenge(challenge(context, null)); return; } @@ -64,9 +64,6 @@ public class BasicAuthAuthenticator extends AbstractUsernameFormAuthenticator im context.success(); return; } - - context.setUser(null); - context.challenge(challengeResponse(context)); } protected boolean onAuthenticate(AuthenticationFlowContext context, String[] challenge) { @@ -104,29 +101,14 @@ public class BasicAuthAuthenticator extends AbstractUsernameFormAuthenticator im return challenge; } - @Override - protected Response invalidUser(AuthenticationFlowContext context) { - return challengeResponse(context); - } - - @Override - protected Response disabledUser(AuthenticationFlowContext context) { - return challengeResponse(context); - } - - @Override - protected Response temporarilyDisabledUser(AuthenticationFlowContext context) { - return challengeResponse(context); - } - - @Override - protected Response invalidCredentials(AuthenticationFlowContext context) { - return challengeResponse(context); - } - @Override protected Response setDuplicateUserChallenge(AuthenticationFlowContext context, String eventError, String loginFormError, AuthenticationFlowError authenticatorError) { - return challengeResponse(context); + return challenge(context, null); + } + + @Override + protected Response challenge(AuthenticationFlowContext context, String error) { + return Response.status(401).header(HttpHeaders.WWW_AUTHENTICATE, getHeader(context)).build(); } @Override @@ -148,10 +130,6 @@ public class BasicAuthAuthenticator extends AbstractUsernameFormAuthenticator im } - private Response challengeResponse(AuthenticationFlowContext context) { - return Response.status(401).header(HttpHeaders.WWW_AUTHENTICATE, getHeader(context)).build(); - } - private String getHeader(AuthenticationFlowContext context) { return "Basic realm=\"" + context.getRealm().getName() + "\""; } diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/challenge/BasicAuthOTPAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/challenge/BasicAuthOTPAuthenticator.java index 99f8eb57ba..6ad463d4a5 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/challenge/BasicAuthOTPAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/challenge/BasicAuthOTPAuthenticator.java @@ -17,12 +17,19 @@ package org.keycloak.authentication.authenticators.challenge; import org.keycloak.authentication.AuthenticationFlowContext; +import org.keycloak.authentication.AuthenticationFlowError; import org.keycloak.authentication.Authenticator; +import org.keycloak.authentication.authenticators.browser.AbstractUsernameFormAuthenticator; +import org.keycloak.events.Details; +import org.keycloak.events.Errors; import org.keycloak.models.KeycloakSession; import org.keycloak.models.OTPPolicy; import org.keycloak.models.RealmModel; import org.keycloak.models.UserCredentialModel; import org.keycloak.models.UserModel; +import org.keycloak.services.messages.Messages; + +import javax.ws.rs.core.Response; /** * @author Bill Burke @@ -44,7 +51,7 @@ public class BasicAuthOTPAuthenticator extends BasicAuthAuthenticator implements password = password.substring(0, password.length() - otpLength); if (checkUsernameAndPassword(context, username, password)) { - String otp = password.substring(password.length() - otpLength); + String otp = challenge[1].substring(password.length(), challenge[1].length()); if (checkOtp(context, otp)) { return true; @@ -55,8 +62,17 @@ public class BasicAuthOTPAuthenticator extends BasicAuthAuthenticator implements } private boolean checkOtp(AuthenticationFlowContext context, String otp) { - return context.getSession().userCredentialManager().isValid(context.getRealm(), context.getUser(), + boolean valid = context.getSession().userCredentialManager().isValid(context.getRealm(), context.getUser(), UserCredentialModel.otp(context.getRealm().getOTPPolicy().getType(), otp)); + + if (!valid) { + context.getEvent().user(context.getUser()).error(Errors.INVALID_USER_CREDENTIALS); + Response challengeResponse = challenge(context, Messages.INVALID_TOTP); + context.failureChallenge(AuthenticationFlowError.INVALID_CREDENTIALS, challengeResponse); + return false; + } + + return true; } @Override diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/cli/CliUsernamePasswordAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/cli/CliUsernamePasswordAuthenticator.java index 0989d94ac7..f0f0bf0e2e 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/cli/CliUsernamePasswordAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/cli/CliUsernamePasswordAuthenticator.java @@ -66,45 +66,12 @@ public class CliUsernamePasswordAuthenticator extends AbstractUsernameFormAuthen } @Override - protected Response invalidUser(AuthenticationFlowContext context) { + protected Response challenge(AuthenticationFlowContext context, String error) { String header = getHeader(context); Response response = Response.status(401) .type(MediaType.TEXT_PLAIN_TYPE) .header(HttpHeaders.WWW_AUTHENTICATE, header) - .entity("\n" + context.form().getMessage(Messages.INVALID_USER) + "\n") - .build(); - return response; - } - - @Override - protected Response disabledUser(AuthenticationFlowContext context) { - String header = getHeader(context); - Response response = Response.status(401) - .type(MediaType.TEXT_PLAIN_TYPE) - .header(HttpHeaders.WWW_AUTHENTICATE, header) - .entity("\n" + context.form().getMessage(Messages.ACCOUNT_DISABLED) + "\n") - .build(); - return response; - } - - @Override - protected Response temporarilyDisabledUser(AuthenticationFlowContext context) { - String header = getHeader(context); - Response response = Response.status(401) - .type(MediaType.TEXT_PLAIN_TYPE) - .header(HttpHeaders.WWW_AUTHENTICATE, header) - .entity("\n" + context.form().getMessage(Messages.INVALID_USER) + "\n") - .build(); - return response; - } - - @Override - protected Response invalidCredentials(AuthenticationFlowContext context) { - String header = getHeader(context); - Response response = Response.status(401) - .type(MediaType.TEXT_PLAIN_TYPE) - .header(HttpHeaders.WWW_AUTHENTICATE, header) - .entity("\n" + context.form().getMessage(Messages.INVALID_USER) + "\n") + .entity("\n" + context.form().getMessage(error) + "\n") .build(); return response; } diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/console/ConsoleUsernamePasswordAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/console/ConsoleUsernamePasswordAuthenticator.java index 720e4e59fd..11b9118b6b 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/console/ConsoleUsernamePasswordAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/console/ConsoleUsernamePasswordAuthenticator.java @@ -22,7 +22,6 @@ import org.keycloak.authentication.authenticators.browser.AbstractUsernameFormAu import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; -import org.keycloak.services.messages.Messages; import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.core.Response; @@ -61,27 +60,8 @@ public class ConsoleUsernamePasswordAuthenticator extends AbstractUsernameFormAu } @Override - protected Response invalidUser(AuthenticationFlowContext context) { - Response response = challenge(context).message(Messages.INVALID_USER); - return response; - } - - @Override - protected Response disabledUser(AuthenticationFlowContext context) { - Response response = challenge(context).message(Messages.ACCOUNT_DISABLED); - return response; - } - - @Override - protected Response temporarilyDisabledUser(AuthenticationFlowContext context) { - Response response = challenge(context).message(Messages.INVALID_USER); - return response; - } - - @Override - protected Response invalidCredentials(AuthenticationFlowContext context) { - Response response = challenge(context).message(Messages.INVALID_USER); - return response; + protected Response challenge(AuthenticationFlowContext context, String error) { + return challenge(context).message(error); } @Override diff --git a/services/src/main/java/org/keycloak/protocol/docker/DockerAuthenticator.java b/services/src/main/java/org/keycloak/protocol/docker/DockerAuthenticator.java index b2c2b37886..cfeaab067b 100644 --- a/services/src/main/java/org/keycloak/protocol/docker/DockerAuthenticator.java +++ b/services/src/main/java/org/keycloak/protocol/docker/DockerAuthenticator.java @@ -37,13 +37,11 @@ public class DockerAuthenticator extends HttpBasicAuthenticator { } @Override - protected void userDisabledAction(AuthenticationFlowContext context, RealmModel realm, UserModel user) { + protected void userDisabledAction(AuthenticationFlowContext context, RealmModel realm, UserModel user, String eventError) { context.getEvent().user(user); - context.getEvent().error(Errors.USER_DISABLED); - + context.getEvent().error(eventError); final DockerError error = new DockerError("UNAUTHORIZED","Invalid username or password.", Collections.singletonList(new DockerAccess(context.getAuthenticationSession().getClientNote(DockerAuthV2Protocol.SCOPE_PARAM)))); - context.failure(AuthenticationFlowError.USER_DISABLED, new ResponseBuilderImpl() .status(Response.Status.UNAUTHORIZED) .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON) diff --git a/services/src/main/java/org/keycloak/protocol/saml/profile/ecp/authenticator/HttpBasicAuthenticator.java b/services/src/main/java/org/keycloak/protocol/saml/profile/ecp/authenticator/HttpBasicAuthenticator.java index 614d38b5da..d091b3f9ef 100644 --- a/services/src/main/java/org/keycloak/protocol/saml/profile/ecp/authenticator/HttpBasicAuthenticator.java +++ b/services/src/main/java/org/keycloak/protocol/saml/profile/ecp/authenticator/HttpBasicAuthenticator.java @@ -4,7 +4,9 @@ import org.jboss.resteasy.spi.HttpRequest; import org.keycloak.authentication.AuthenticationFlowContext; import org.keycloak.authentication.AuthenticationFlowError; import org.keycloak.authentication.Authenticator; +import org.keycloak.authentication.authenticators.browser.AbstractUsernameFormAuthenticator; import org.keycloak.common.util.Base64; +import org.keycloak.events.Details; import org.keycloak.events.Errors; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; @@ -34,15 +36,21 @@ public class HttpBasicAuthenticator implements Authenticator { final String username = usernameAndPassword[0]; final UserModel user = context.getSession().users().getUserByUsername(username, realm); + // to allow success/failure logging for brute force + context.getEvent().detail(Details.USERNAME, username); + context.getAuthenticationSession().setAuthNote(AbstractUsernameFormAuthenticator.ATTEMPTED_USERNAME, username); + if (user != null) { final String password = usernameAndPassword[1]; final boolean valid = context.getSession().userCredentialManager().isValid(realm, user, UserCredentialModel.password(password)); if (valid) { - if (user.isEnabled()) { + if (isTemporarilyDisabledByBruteForce(context, user)) { + userDisabledAction(context, realm, user, Errors.USER_TEMPORARILY_DISABLED); + } else if (user.isEnabled()) { userSuccessAction(context, user); } else { - userDisabledAction(context, realm, user); + userDisabledAction(context, realm, user, Errors.USER_DISABLED); } } else { notValidCredentialsAction(context, realm, user); @@ -58,8 +66,12 @@ public class HttpBasicAuthenticator implements Authenticator { context.success(); } - protected void userDisabledAction(AuthenticationFlowContext context, RealmModel realm, UserModel user) { - userSuccessAction(context, user); + protected void userDisabledAction(AuthenticationFlowContext context, RealmModel realm, UserModel user, String eventError) { + context.getEvent().user(user); + context.getEvent().error(eventError); + context.failure(AuthenticationFlowError.INVALID_USER, Response.status(Response.Status.UNAUTHORIZED) + .header(HttpHeaders.WWW_AUTHENTICATE, BASIC_PREFIX + "realm=\"" + realm.getName() + "\"") + .build()); } protected void nullUserAction(final AuthenticationFlowContext context, final RealmModel realm, final String user) { @@ -74,6 +86,11 @@ public class HttpBasicAuthenticator implements Authenticator { .build()); } + private boolean isTemporarilyDisabledByBruteForce(AuthenticationFlowContext context, UserModel user) { + return (context.getRealm().isBruteForceProtected()) + && (context.getProtector().isTemporarilyDisabled(context.getSession(), context.getRealm(), user)); + } + private String[] getUsernameAndPassword(final HttpHeaders httpHeaders) { final List authHeaders = httpHeaders.getRequestHeader(HttpHeaders.AUTHORIZATION); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java index d589f2a228..5ccb869c3c 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java @@ -376,12 +376,27 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { @Test public void testBrowserInvalidTotp() throws Exception { loginSuccess(); + loginInvalidPassword(); loginWithTotpFailure(); - loginWithTotpFailure(); - expectTemporarilyDisabled(); - expectTemporarilyDisabled("test-user@localhost", null, "invalid"); + continueLoginWithCorrectTotpExpectFailure(); + continueLoginWithInvalidTotp(); clearUserFailures(); - loginSuccess(); + continueLoginWithTotp(); + } + + @Test + public void testTotpGoingBack() throws Exception { + loginPage.open(); + loginPage.login("test-user@localhost", "password"); + + continueLoginWithInvalidTotp(); + loginTotpPage.cancel(); + loginPage.assertCurrent(); + loginPage.login("test-user@localhost", "password"); + continueLoginWithInvalidTotp(); + continueLoginWithCorrectTotpExpectFailure(); + clearUserFailures(); + continueLoginWithTotp(); } @Test @@ -389,10 +404,14 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { loginSuccess(); loginWithMissingTotp(); loginWithMissingTotp(); - expectTemporarilyDisabled(); - expectTemporarilyDisabled("test-user@localhost", null, "invalid"); - clearUserFailures(); - loginSuccess(); + continueLoginWithMissingTotp(); + continueLoginWithCorrectTotpExpectFailure(); + // wait to unlock + testingClient.testing().setTimeOffset(Collections.singletonMap("offset", String.valueOf(6))); + + continueLoginWithTotp(); + + testingClient.testing().setTimeOffset(Collections.singletonMap("offset", String.valueOf(0))); } @Test @@ -546,7 +565,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { } - public void loginWithTotpFailure() throws Exception { + public void loginWithTotpFailure() { loginPage.open(); loginPage.login("test-user@localhost", "password"); @@ -558,6 +577,51 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { events.clear(); } + public void continueLoginWithTotp() { + loginTotpPage.assertCurrent(); + + String totpSecret = totp.generateTOTP("totpSecret"); + loginTotpPage.login(totpSecret); + + Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); + + events.expectLogin().assertEvent(); + appPage.logout(); + events.clear(); + } + + public void continueLoginWithCorrectTotpExpectFailure() { + loginTotpPage.assertCurrent(); + + String totpSecret = totp.generateTOTP("totpSecret"); + loginTotpPage.login(totpSecret); + + loginTotpPage.assertCurrent(); + Assert.assertEquals("Invalid authenticator code.", loginTotpPage.getError()); + + events.clear(); + } + + public void continueLoginWithInvalidTotp() { + loginTotpPage.assertCurrent(); + + loginTotpPage.login("123456"); + + loginTotpPage.assertCurrent(); + Assert.assertEquals("Invalid authenticator code.", loginTotpPage.getError()); + events.clear(); + } + + public void continueLoginWithMissingTotp() { + loginTotpPage.assertCurrent(); + + loginTotpPage.login(null); + + loginTotpPage.assertCurrent(); + Assert.assertEquals("Invalid authenticator code.", loginTotpPage.getError()); + events.clear(); + } + public void loginWithMissingTotp() throws Exception { loginPage.open(); loginPage.login("test-user@localhost", "password"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/FlowOverrideTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/FlowOverrideTest.java index 036bdb316e..dd3613eedf 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/FlowOverrideTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/FlowOverrideTest.java @@ -29,14 +29,17 @@ import org.keycloak.OAuth2Constants; import org.keycloak.admin.client.resource.ClientsResource; import org.keycloak.admin.client.resource.UserResource; import org.keycloak.authentication.authenticators.browser.UsernamePasswordFormFactory; +import org.keycloak.authentication.authenticators.challenge.BasicAuthOTPAuthenticatorFactory; import org.keycloak.events.Details; import org.keycloak.models.AuthenticationExecutionModel; import org.keycloak.models.AuthenticationFlowBindings; import org.keycloak.models.AuthenticationFlowModel; import org.keycloak.models.ClientModel; import org.keycloak.models.RealmModel; +import org.keycloak.models.utils.TimeBasedOTP; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testsuite.AbstractTestRealmKeycloakTest; import org.keycloak.testsuite.AssertEvents; import org.keycloak.testsuite.authentication.PushButtonAuthenticatorFactory; @@ -44,6 +47,7 @@ import org.keycloak.testsuite.pages.AppPage; import org.keycloak.testsuite.pages.ErrorPage; import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.runonserver.RunOnServerDeployment; +import org.keycloak.testsuite.util.UserBuilder; import org.keycloak.util.BasicAuthHelper; import org.openqa.selenium.By; @@ -69,6 +73,7 @@ public class FlowOverrideTest extends AbstractTestRealmKeycloakTest { public static final String TEST_APP_DIRECT_OVERRIDE = "test-app-direct-override"; public static final String TEST_APP_FLOW = "test-app-flow"; public static final String TEST_APP_HTTP_CHALLENGE = "http-challenge-client"; + public static final String TEST_APP_HTTP_CHALLENGE_OTP = "http-challenge-otp-client"; @Rule public AssertEvents events = new AssertEvents(this); @@ -82,6 +87,8 @@ public class FlowOverrideTest extends AbstractTestRealmKeycloakTest { @Page protected ErrorPage errorPage; + private TimeBasedOTP totp = new TimeBasedOTP(); + @Override public void configureTestRealm(RealmRepresentation testRealm) { } @@ -181,6 +188,22 @@ public class FlowOverrideTest extends AbstractTestRealmKeycloakTest { realm.addAuthenticatorExecution(execution); + AuthenticationFlowModel challengeOTP = new AuthenticationFlowModel(); + challengeOTP.setAlias("challenge-override-flow"); + challengeOTP.setDescription("challenge grant based authentication"); + challengeOTP.setProviderId("basic-flow"); + challengeOTP.setTopLevel(true); + challengeOTP.setBuiltIn(true); + + realm.addAuthenticationFlow(challengeOTP); + + execution = new AuthenticationExecutionModel(); + execution.setParentFlow(challengeOTP.getId()); + execution.setRequirement(AuthenticationExecutionModel.Requirement.REQUIRED); + execution.setAuthenticator(BasicAuthOTPAuthenticatorFactory.PROVIDER_ID); + execution.setPriority(10); + realm.addAuthenticatorExecution(execution); + client = realm.addClient(TEST_APP_DIRECT_OVERRIDE); client.setSecret("password"); client.setBaseUrl("http://localhost:8180/auth/realms/master/app/auth"); @@ -203,6 +226,17 @@ public class FlowOverrideTest extends AbstractTestRealmKeycloakTest { client.setDirectAccessGrantsEnabled(true); client.setAuthenticationFlowBindingOverride(AuthenticationFlowBindings.DIRECT_GRANT_BINDING, realm.getFlowByAlias("http challenge").getId()); client.setAuthenticationFlowBindingOverride(AuthenticationFlowBindings.BROWSER_BINDING, realm.getFlowByAlias("http challenge").getId()); + + client = realm.addClient(TEST_APP_HTTP_CHALLENGE_OTP); + client.setSecret("password"); + client.setBaseUrl("http://localhost:8180/auth/realms/master/app/auth"); + client.setManagementUrl("http://localhost:8180/auth/realms/master/app/admin"); + client.setEnabled(true); + client.addRedirectUri("http://localhost:8180/auth/realms/master/app/auth/*"); + client.setPublicClient(true); + client.setDirectAccessGrantsEnabled(true); + client.setAuthenticationFlowBindingOverride(AuthenticationFlowBindings.DIRECT_GRANT_BINDING, realm.getFlowByAlias("challenge-override-flow").getId()); + client.setAuthenticationFlowBindingOverride(AuthenticationFlowBindings.BROWSER_BINDING, realm.getFlowByAlias("challenge-override-flow").getId()); }); } @@ -302,6 +336,7 @@ public class FlowOverrideTest extends AbstractTestRealmKeycloakTest { form.param(OAuth2Constants.GRANT_TYPE, OAuth2Constants.PASSWORD); form.param("username", "test-user@localhost"); form.param("password", "password"); + Response response = grantTarget.request() .header(HttpHeaders.AUTHORIZATION, header) .post(Entity.form(form)); @@ -363,6 +398,100 @@ public class FlowOverrideTest extends AbstractTestRealmKeycloakTest { events.clear(); } + @Test + public void testDirectGrantHttpChallengeOTP() { + UserRepresentation user = adminClient.realm("test").users().search("test-user@localhost").get(0); + UserRepresentation userUpdated = UserBuilder.edit(user).totpSecret("totpSecret").otpEnabled().build(); + adminClient.realm("test").users().get(user.getId()).update(userUpdated); + + setupBruteForce(); + + Client httpClient = javax.ws.rs.client.ClientBuilder.newClient(); + String grantUri = oauth.getResourceOwnerPasswordCredentialGrantUrl(); + WebTarget grantTarget = httpClient.target(grantUri); + + Form form = new Form(); + form.param(OAuth2Constants.GRANT_TYPE, OAuth2Constants.PASSWORD); + form.param(OAuth2Constants.CLIENT_ID, TEST_APP_HTTP_CHALLENGE_OTP); + + // correct password + totp + String totpCode = totp.generateTOTP("totpSecret"); + Response response = grantTarget.request() + .header(HttpHeaders.AUTHORIZATION, BasicAuthHelper.createHeader("test-user@localhost", "password" + totpCode)) + .post(Entity.form(form)); + assertEquals(200, response.getStatus()); + response.close(); + + // correct password + wrong totp 2x + response = grantTarget.request() + .header(HttpHeaders.AUTHORIZATION, BasicAuthHelper.createHeader("test-user@localhost", "password123456")) + .post(Entity.form(form)); + assertEquals(401, response.getStatus()); + response = grantTarget.request() + .header(HttpHeaders.AUTHORIZATION, BasicAuthHelper.createHeader("test-user@localhost", "password123456")) + .post(Entity.form(form)); + assertEquals(401, response.getStatus()); + + // correct password + totp but user is temporarily locked + totpCode = totp.generateTOTP("totpSecret"); + response = grantTarget.request() + .header(HttpHeaders.AUTHORIZATION, BasicAuthHelper.createHeader("test-user@localhost", "password" + totpCode)) + .post(Entity.form(form)); + assertEquals(401, response.getStatus()); + response.close(); + + clearBruteForce(); + adminClient.realm("test").users().get(user.getId()).removeTotp(); + } + + @Test + public void testDirectGrantHttpChallengeUserDisabled() { + setupBruteForce(); + + Client httpClient = javax.ws.rs.client.ClientBuilder.newClient(); + String grantUri = oauth.getResourceOwnerPasswordCredentialGrantUrl(); + WebTarget grantTarget = httpClient.target(grantUri); + + Form form = new Form(); + form.param(OAuth2Constants.GRANT_TYPE, OAuth2Constants.PASSWORD); + form.param(OAuth2Constants.CLIENT_ID, TEST_APP_HTTP_CHALLENGE); + + UserRepresentation user = adminClient.realm("test").users().search("test-user@localhost").get(0); + user.setEnabled(false); + adminClient.realm("test").users().get(user.getId()).update(user); + + // user disabled + Response response = grantTarget.request() + .header(HttpHeaders.AUTHORIZATION, BasicAuthHelper.createHeader("test-user@localhost", "password")) + .post(Entity.form(form)); + assertEquals(401, response.getStatus()); + assertEquals("Unauthorized", response.getStatusInfo().getReasonPhrase()); + response.close(); + + user.setEnabled(true); + adminClient.realm("test").users().get(user.getId()).update(user); + + // lock the user account + grantTarget.request() + .header(HttpHeaders.AUTHORIZATION, BasicAuthHelper.createHeader("test-user@localhost", "wrongpassword")) + .post(Entity.form(form)); + grantTarget.request() + .header(HttpHeaders.AUTHORIZATION, BasicAuthHelper.createHeader("test-user@localhost", "wrongpassword")) + .post(Entity.form(form)); + // user is temporarily disabled + response = grantTarget.request() + .header(HttpHeaders.AUTHORIZATION, BasicAuthHelper.createHeader("test-user@localhost", "password")) + .post(Entity.form(form)); + assertEquals(401, response.getStatus()); + assertEquals("Unauthorized", response.getStatusInfo().getReasonPhrase()); + response.close(); + + clearBruteForce(); + + httpClient.close(); + events.clear(); + } + @Test public void testClientOverrideFlowUsingBrowserHttpChallenge() { Client httpClient = javax.ws.rs.client.ClientBuilder.newClient(); @@ -446,4 +575,20 @@ public class FlowOverrideTest extends AbstractTestRealmKeycloakTest { } + private void setupBruteForce() { + RealmRepresentation testRealm = adminClient.realm("test").toRepresentation(); + testRealm.setBruteForceProtected(true); + testRealm.setFailureFactor(2); + testRealm.setMaxDeltaTimeSeconds(20); + testRealm.setMaxFailureWaitSeconds(100); + testRealm.setWaitIncrementSeconds(5); + adminClient.realm("test").update(testRealm); + } + + private void clearBruteForce() { + RealmRepresentation testRealm = adminClient.realm("test").toRepresentation(); + testRealm.setBruteForceProtected(false); + adminClient.realm("test").attackDetection().clearAllBruteForce(); + adminClient.realm("test").update(testRealm); + } }