From af72c1374ac43dc5411ee7849aae5666354bcf29 Mon Sep 17 00:00:00 2001 From: Daniil Filippov Date: Sun, 22 Jul 2018 01:21:13 +0300 Subject: [PATCH] KEYCLOAK-7823 Fix HTTP status returned during SPNEGO auth --- .../browser/SpnegoAuthenticator.java | 3 +- .../kerberos/AbstractKerberosTest.java | 37 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/SpnegoAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/SpnegoAuthenticator.java index dd14ce24e1..33c77ee711 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/SpnegoAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/SpnegoAuthenticator.java @@ -120,9 +120,8 @@ public class SpnegoAuthenticator extends AbstractUsernameFormAuthenticator imple if (context.getExecution().isRequired()) { return context.getSession().getProvider(LoginFormsProvider.class) .setAuthenticationSession(context.getAuthenticationSession()) - .setStatus(Response.Status.UNAUTHORIZED) .setResponseHeader(HttpHeaders.WWW_AUTHENTICATE, negotiateHeader) - .setError(Messages.KERBEROS_NOT_ENABLED).createErrorPage(Response.Status.BAD_REQUEST); + .setError(Messages.KERBEROS_NOT_ENABLED).createErrorPage(Response.Status.UNAUTHORIZED); } else { return optionalChallengeRedirect(context, negotiateHeader); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/kerberos/AbstractKerberosTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/kerberos/AbstractKerberosTest.java index 296ed700c9..aceaf8afed 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/kerberos/AbstractKerberosTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/kerberos/AbstractKerberosTest.java @@ -26,6 +26,7 @@ import java.security.Principal; import java.util.Hashtable; import java.util.List; import java.util.Map; +import java.util.Optional; import javax.naming.Context; import javax.naming.NamingException; @@ -56,17 +57,21 @@ import org.keycloak.OAuth2Constants; import org.keycloak.adapters.HttpClientBuilder; import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.RealmResource; +import org.keycloak.authentication.authenticators.browser.SpnegoAuthenticatorFactory; import org.keycloak.common.constants.KerberosConstants; import org.keycloak.common.util.KerberosSerializationUtils; import org.keycloak.common.util.MultivaluedHashMap; import org.keycloak.events.Details; import org.keycloak.federation.kerberos.CommonKerberosConfig; +import org.keycloak.models.AuthenticationExecutionModel; import org.keycloak.models.LDAPConstants; import org.keycloak.models.ProtocolMapperModel; import org.keycloak.models.UserModel; +import org.keycloak.models.utils.DefaultAuthenticationFlows; import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.protocol.oidc.mappers.UserSessionNoteMapper; import org.keycloak.representations.AccessToken; +import org.keycloak.representations.idm.AuthenticationExecutionInfoRepresentation; import org.keycloak.representations.idm.ComponentRepresentation; import org.keycloak.representations.idm.ProtocolMapperRepresentation; import org.keycloak.representations.idm.RealmRepresentation; @@ -169,6 +174,16 @@ public abstract class AbstractKerberosTest extends AbstractAuthTest { response.close(); } + // KEYCLOAK-7823 + @Test + public void spnegoLoginWithRequiredKerberosAuthExecutionTest() { + AuthenticationExecutionModel.Requirement oldRequirement = updateKerberosAuthExecutionRequirement( + AuthenticationExecutionModel.Requirement.REQUIRED); + Response response = spnegoLogin("hnelson", "secret"); + updateKerberosAuthExecutionRequirement(oldRequirement); + + Assert.assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); + } protected OAuthClient.AccessTokenResponse spnegoLoginTestImpl() throws Exception { Response spnegoResponse = spnegoLogin("hnelson", "secret"); @@ -446,6 +461,28 @@ public abstract class AbstractKerberosTest extends AbstractAuthTest { kerberosProvider.getConfig().putSingle(LDAPConstants.VALIDATE_PASSWORD_POLICY, validatePasswordPolicy.toString()); testRealmResource().components().component(kerberosProvider.getId()).update(kerberosProvider); } + + private AuthenticationExecutionModel.Requirement updateKerberosAuthExecutionRequirement(AuthenticationExecutionModel.Requirement requirement) { + Optional kerberosAuthExecutionOpt = testRealmResource() + .flows() + .getExecutions(DefaultAuthenticationFlows.BROWSER_FLOW) + .stream() + .filter(e -> e.getProviderId().equals(SpnegoAuthenticatorFactory.PROVIDER_ID)) + .findFirst(); + + Assert.assertTrue(kerberosAuthExecutionOpt.isPresent()); + + AuthenticationExecutionInfoRepresentation kerberosAuthExecution = kerberosAuthExecutionOpt.get(); + String oldRequirementStr = kerberosAuthExecution.getRequirement(); + AuthenticationExecutionModel.Requirement oldRequirement = AuthenticationExecutionModel.Requirement.valueOf(oldRequirementStr); + kerberosAuthExecution.setRequirement(requirement.name()); + + testRealmResource() + .flows() + .updateExecutions(DefaultAuthenticationFlows.BROWSER_FLOW, kerberosAuthExecution); + + return oldRequirement; + } @Override public RealmResource testRealmResource() {