Prevent empty usernames and allow restarting the login

Closes #42837
Closes #42409

Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>
This commit is contained in:
Pedro Igor 2025-09-23 07:29:59 -03:00
parent 41b64c91aa
commit 1948e5baf3
5 changed files with 103 additions and 27 deletions

View File

@ -39,6 +39,7 @@ import org.keycloak.WebAuthnConstants;
import org.keycloak.authentication.AuthenticationFlowContext;
import org.keycloak.authentication.AuthenticationFlowError;
import org.keycloak.authentication.FlowStatus;
import org.keycloak.authentication.authenticators.browser.AbstractUsernameFormAuthenticator;
import org.keycloak.authentication.authenticators.browser.IdentityProviderAuthenticator;
import org.keycloak.authentication.authenticators.browser.WebAuthnConditionalUIAuthenticator;
import org.keycloak.email.freemarker.beans.ProfileBean;
@ -63,7 +64,9 @@ import org.keycloak.organization.forms.login.freemarker.model.OrganizationAwareR
import org.keycloak.organization.protocol.mappers.oidc.OrganizationScope;
import org.keycloak.organization.utils.Organizations;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.services.messages.Messages;
import org.keycloak.sessions.AuthenticationSessionModel;
import org.keycloak.utils.StringUtil;
public class OrganizationAuthenticator extends IdentityProviderAuthenticator {
@ -82,7 +85,7 @@ public class OrganizationAuthenticator extends IdentityProviderAuthenticator {
OrganizationProvider provider = getOrganizationProvider();
if (!isEnabledAndOrganizationsPresent(provider)) {
context.attempted();
attempted(context);
return;
}
@ -104,12 +107,16 @@ public class OrganizationAuthenticator extends IdentityProviderAuthenticator {
// make sure the organization is set to the auth session to remember it when processing subsequent requests
AuthenticationSessionModel authSession = context.getAuthenticationSession();
authSession.setAuthNote(OrganizationModel.ORGANIZATION_ATTRIBUTE, organization.getId());
action(context);
action(context, false);
}
}
@Override
public void action(AuthenticationFlowContext context) {
action(context, true);
}
private void action(AuthenticationFlowContext context, boolean formSubmitted) {
HttpRequest request = context.getHttpRequest();
MultivaluedMap<String, String> parameters = request.getDecodedFormParameters();
@ -124,8 +131,18 @@ public class OrganizationAuthenticator extends IdentityProviderAuthenticator {
}
String username = parameters.getFirst(UserModel.USERNAME);
UserModel user = context.getUser();
if (formSubmitted && user == null && StringUtil.isBlank(username)) {
initialChallenge(context, form -> {
form.addError(new FormMessage(UserModel.USERNAME, Messages.INVALID_USERNAME));
return form.createLoginUsername();
});
return;
}
RealmModel realm = context.getRealm();
UserModel user = resolveUser(context, username);
user = resolveUser(context, username);
String domain = getEmailDomain(username);
OrganizationModel organization = resolveOrganization(user, domain);
@ -140,7 +157,7 @@ public class OrganizationAuthenticator extends IdentityProviderAuthenticator {
clearAuthenticationSession(context);
// request does not map to any organization, go to the next step/sub-flow
context.attempted();
attempted(context, username);
return;
}
@ -173,7 +190,7 @@ public class OrganizationAuthenticator extends IdentityProviderAuthenticator {
// if re-authenticating in the scope of an organization
context.success();
} else {
context.attempted();
attempted(context);
}
}
@ -351,6 +368,10 @@ public class OrganizationAuthenticator extends IdentityProviderAuthenticator {
}
private void initialChallenge(AuthenticationFlowContext context) {
initialChallenge(context, null);
}
private void initialChallenge(AuthenticationFlowContext context, Function<LoginFormsProvider, Response> formCreator) {
AuthenticationSessionModel authenticationSession = context.getAuthenticationSession();
UserModel user = context.getUser();
@ -360,7 +381,7 @@ public class OrganizationAuthenticator extends IdentityProviderAuthenticator {
webauthnAuth.fillContextForm(context);
}
context.challenge(createLoginForm(context));
context.challenge(createLoginForm(context, formCreator));
} else if (isSSOAuthentication(authenticationSession)) {
if (shouldUserSelectOrganization(context, user)) {
return;
@ -370,11 +391,15 @@ public class OrganizationAuthenticator extends IdentityProviderAuthenticator {
context.success();
} else {
// user is re-authenticating, there is no organization to process
context.attempted();
attempted(context, user.getUsername());
}
}
private Response createLoginForm(AuthenticationFlowContext context) {
return createLoginForm(context, null);
}
private Response createLoginForm(AuthenticationFlowContext context, Function<LoginFormsProvider, Response> formCreator) {
// the default challenge won't show any broker but just the identity-first login page and the option to try a different authentication mechanism
LoginFormsProvider form = context.form()
.setAttributeMapper(attributes -> {
@ -393,7 +418,33 @@ public class OrganizationAuthenticator extends IdentityProviderAuthenticator {
form.setFormData(new MultivaluedHashMap<>(Map.of(UserModel.USERNAME, loginHint)));
}
return form.createLoginUsername();
return formCreator == null ? form.createLoginUsername() : formCreator.apply(form);
}
private void attempted(AuthenticationFlowContext context) {
attempted(context, null);
}
private void attempted(AuthenticationFlowContext context, String username) {
AuthenticationSessionModel authenticationSession = context.getAuthenticationSession();
if (username != null) {
authenticationSession.setAuthNote(AbstractUsernameFormAuthenticator.ATTEMPTED_USERNAME, username);
LoginFormsProvider form = context.form();
form.setAttributeMapper(attributes -> {
AuthenticationContextBean auth = (AuthenticationContextBean) attributes.get("auth");
if (auth != null) {
attributes.put("auth", new OrganizationAwareAuthenticationContextBean(auth, true, username));
attributes.put(LoginFormsProvider.USERNAME_HIDDEN, true);
}
return attributes;
});
}
context.attempted();
}
private boolean hasPublicBrokers(OrganizationModel organization) {

View File

@ -26,11 +26,17 @@ public class OrganizationAwareAuthenticationContextBean extends AuthenticationCo
private final AuthenticationContextBean delegate;
private final boolean showTryAnotherWayLink;
private final String username;
public OrganizationAwareAuthenticationContextBean(AuthenticationContextBean delegate, boolean showTryAnotherWayLink) {
this(delegate, showTryAnotherWayLink, null);
}
public OrganizationAwareAuthenticationContextBean(AuthenticationContextBean delegate, boolean showTryAnotherWayLink, String username) {
super(null, null);
this.delegate = delegate;
this.showTryAnotherWayLink = showTryAnotherWayLink;
this.username = username;
}
@Override
@ -46,7 +52,7 @@ public class OrganizationAwareAuthenticationContextBean extends AuthenticationCo
}
public boolean showUsername() {
return delegate.showUsername();
return username != null || delegate.showUsername();
}
public boolean showResetCredentials() {
@ -54,6 +60,9 @@ public class OrganizationAwareAuthenticationContextBean extends AuthenticationCo
}
public String getAttemptedUsername() {
return delegate.getAttemptedUsername();
if (username == null) {
return delegate.getAttemptedUsername();
}
return username;
}
}

View File

@ -19,6 +19,7 @@ package org.keycloak.testsuite.organization.authentication;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.keycloak.testsuite.broker.BrokerTestTools.waitForPage;
import java.io.IOException;
@ -72,10 +73,22 @@ public class OrganizationAuthenticationTest extends AbstractOrganizationTest {
openIdentityFirstLoginPage("user", false, null, false, false);
// check if the login page is shown
Assert.assertTrue(loginPage.isUsernameInputPresent());
loginPage.assertAttemptedUsernameAvailability(true);
Assert.assertTrue(loginPage.isPasswordInputPresent());
}
@Test
public void testEmptyUserNameValidation() {
createOrganization();
oauth.clientId("broker-app");
loginPage.open(bc.consumerRealmName());
Assert.assertFalse(loginPage.isPasswordInputPresent());
loginPage.loginUsername("");
assertEquals("Invalid username.", loginPage.getUsernameInputError());
}
@Test
public void testDefaultAuthenticationMechanismIfNotOrganizationMember() {
testRealm().organizations().get(createOrganization().getId());
@ -83,7 +96,7 @@ public class OrganizationAuthenticationTest extends AbstractOrganizationTest {
openIdentityFirstLoginPage("user@noorg.org", false, null, false, false);
// check if the login page is shown
Assert.assertTrue(loginPage.isUsernameInputPresent());
loginPage.assertAttemptedUsernameAvailability(true);
Assert.assertTrue(loginPage.isPasswordInputPresent());
}
@ -267,6 +280,21 @@ public class OrganizationAuthenticationTest extends AbstractOrganizationTest {
appPage.assertCurrent();
}
@Test
public void testRestartLogin() {
testRealm().organizations().get(createOrganization().getId());
openIdentityFirstLoginPage("user@noorg.org", false, null, false, false);
// check if the login page is shown
loginPage.assertAttemptedUsernameAvailability(true);
Assert.assertTrue(loginPage.isPasswordInputPresent());
loginPage.clickResetLogin();
Assert.assertTrue(loginPage.isUsernameInputPresent());
Assert.assertFalse(loginPage.isPasswordInputPresent());
}
private void runOnServer(RunOnServer function) {
testingClient.server(bc.consumerRealmName()).run(function);
}

View File

@ -77,18 +77,6 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz
Assert.assertEquals(bc.getUserEmail(), loginPage.getUsername());
}
@Test
public void testDefaultAuthenticationIfUserDoesNotExistAndNoOrgMatch() {
testRealm().organizations().get(createOrganization().getId());
// login with email only
openIdentityFirstLoginPage("user@noorg.org", false, null, false, false);
// check if the login page is shown
Assert.assertTrue(loginPage.isUsernameInputPresent());
Assert.assertTrue(loginPage.isPasswordInputPresent());
}
@Test
public void testIdentityFirstIfUserNotExistsAndEmailMatchOrgDomain() {
OrganizationResource organization = testRealm().organizations().get(createOrganization().getId());
@ -204,7 +192,7 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz
openIdentityFirstLoginPage("user", false, null, false, false);
// check if the login page is shown
Assert.assertTrue(loginPage.isUsernameInputPresent());
loginPage.assertAttemptedUsernameAvailability(true);
Assert.assertTrue(loginPage.isPasswordInputPresent());
Assert.assertFalse(loginPage.isSocialButtonPresent(bc.getIDPAlias()));

View File

@ -170,8 +170,8 @@ public class OrganizationOIDCProtocolMapperTest extends AbstractOrganizationTest
oauth.scope("organization:" + orgA.getAlias());
loginPage.open(bc.consumerRealmName());
org.keycloak.testsuite.Assert.assertFalse(loginPage.isPasswordInputPresent());
org.keycloak.testsuite.Assert.assertTrue(loginPage.isSocialButtonPresent(orgA.getAlias() + "-identity-provider"));
org.keycloak.testsuite.Assert.assertFalse(loginPage.isSocialButtonPresent(orgB.getAlias() + "-identity-provider"));
assertTrue(loginPage.isSocialButtonPresent(orgA.getAlias() + "-identity-provider"));
assertFalse(loginPage.isSocialButtonPresent(orgB.getAlias() + "-identity-provider"));
assertFalse(driver.getPageSource().contains("Your email domain matches"));
// identity-first login will respect the organization provided in the scope even though the user email maps to a different organization