Merge pull request #3635 from patriot1burke/master

KEYCLOAK-3506

server-spi-private/src/main/java/org/keycloak/authentication/AbstractAuthenticationFlowContext.java

@@ -153,14 +153,14 @@ public interface AbstractAuthenticationFlowContext {
     void challenge(Response challenge);
 
     /**
-     * Sends the challenge back to the HTTP client irregardless of the current executionr equirement
+     * Sends the challenge back to the HTTP client irregardless of the current executionr requirement
      *
      * @param challenge
      */
     void forceChallenge(Response challenge);
 
     /**
-     * Same behavior as challenge(), but the error count in brute force attack detection will be incremented.
+     * Same behavior as forceChallenge(), but the error count in brute force attack detection will be incremented.
      * For example, if a user enters in a bad password, the user is directed to try again, but Keycloak will keep track
      * of how many failures have happened.
      *

services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java

@@ -73,7 +73,14 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow {
             }
             if (model.isAuthenticatorFlow()) {
                 AuthenticationFlow authenticationFlow = processor.createFlowExecution(model.getFlowId(), model);
-                return authenticationFlow.processAction(actionExecution);
+                Response flowChallenge = authenticationFlow.processAction(actionExecution);
+                if (flowChallenge == null) {
+                    processor.getClientSession().setExecutionStatus(model.getId(), ClientSessionModel.ExecutionStatus.SUCCESS);
+                    if (model.isAlternative()) alternativeSuccessful = true;
+                    return processFlow();
+                } else {
+                   return flowChallenge;
+                }
             } else if (model.getId().equals(actionExecution)) {
                 AuthenticatorFactory factory = (AuthenticatorFactory) processor.getSession().getKeycloakSessionFactory().getProviderFactory(Authenticator.class, model.getAuthenticator());
                 if (factory == null) {

testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/forms/ClickThroughAuthenticator.java

@@ -0,0 +1,140 @@
+/*
+ * Copyright 2016 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.keycloak.testsuite.forms;
+
+import org.keycloak.Config;
+import org.keycloak.authentication.AuthenticationFlowContext;
+import org.keycloak.authentication.AuthenticationFlowError;
+import org.keycloak.authentication.Authenticator;
+import org.keycloak.authentication.AuthenticatorFactory;
+import org.keycloak.common.util.Time;
+import org.keycloak.models.AuthenticationExecutionModel;
+import org.keycloak.models.KeycloakSession;
+import org.keycloak.models.KeycloakSessionFactory;
+import org.keycloak.models.RealmModel;
+import org.keycloak.models.UserModel;
+import org.keycloak.models.utils.KeycloakModelUtils;
+import org.keycloak.provider.ProviderConfigProperty;
+
+import javax.ws.rs.core.Response;
+import java.util.Arrays;
+import java.util.List;
+
+/**
+ * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
+ * @version $Revision: 1 $
+ */
+public class ClickThroughAuthenticator implements Authenticator, AuthenticatorFactory {
+    public static final String PROVIDER_ID = "testsuite-dummy-click-through";
+
+    @Override
+    public void authenticate(AuthenticationFlowContext context) {
+        Response challenge = context.form().createForm("terms.ftl");
+        context.challenge(challenge);
+    }
+
+    @Override
+    public boolean requiresUser() {
+        return false;
+    }
+
+    @Override
+    public boolean configuredFor(KeycloakSession session, RealmModel realm, UserModel user) {
+        return true;
+    }
+
+    @Override
+    public void setRequiredActions(KeycloakSession session, RealmModel realm, UserModel user) {
+
+    }
+
+    @Override
+    public void action(AuthenticationFlowContext context) {
+        if (context.getHttpRequest().getDecodedFormParameters().containsKey("cancel")) {
+            authenticate(context);
+            return;
+        }
+
+        context.success();
+    }
+
+   @Override
+    public String getDisplayType() {
+        return "Testsuite Dummy Click Thru";
+    }
+
+    @Override
+    public String getReferenceCategory() {
+        return null;
+    }
+
+    @Override
+    public boolean isConfigurable() {
+        return false;
+    }
+
+    public static final AuthenticationExecutionModel.Requirement[] REQUIREMENT_CHOICES = {
+            AuthenticationExecutionModel.Requirement.REQUIRED,
+            AuthenticationExecutionModel.Requirement.DISABLED
+    };
+
+    @Override
+    public AuthenticationExecutionModel.Requirement[] getRequirementChoices() {
+        return REQUIREMENT_CHOICES;
+    }
+
+    @Override
+    public boolean isUserSetupAllowed() {
+        return false;
+    }
+
+    @Override
+    public String getHelpText() {
+        return "Testsuite Dummy authenticator.  User needs to click through the page to continue.";
+    }
+
+    @Override
+    public List<ProviderConfigProperty> getConfigProperties() {
+        return null;
+    }
+
+    @Override
+    public void close() {
+
+    }
+
+    @Override
+    public Authenticator create(KeycloakSession session) {
+        return this;
+    }
+
+    @Override
+    public void init(Config.Scope config) {
+
+    }
+
+    @Override
+    public void postInit(KeycloakSessionFactory factory) {
+
+    }
+
+    @Override
+    public String getId() {
+        return PROVIDER_ID;
+    }
+}

testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/resources/META-INF/services/org.keycloak.authentication.AuthenticatorFactory

@@ -17,3 +17,4 @@
 
 org.keycloak.testsuite.forms.PassThroughAuthenticator
 org.keycloak.testsuite.forms.PassThroughRegistration
+org.keycloak.testsuite.forms.ClickThroughAuthenticator

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/ProvidersTest.java

@@ -166,6 +166,8 @@ public class ProvidersTest extends AbstractAuthenticationTest {
                 "Will also set it if execution is OPTIONAL and the OTP is currently configured for it.");
         addProviderInfo(result, "reset-password", "Reset Password", "Sets the Update Password required action if execution is REQUIRED.  " +
                 "Will also set it if execution is OPTIONAL and the password is currently configured for it.");
+        addProviderInfo(result, "testsuite-dummy-click-through", "Testsuite Dummy Click Thru",
+                "Testsuite Dummy authenticator.  User needs to click through the page to continue.");
         addProviderInfo(result, "testsuite-dummy-passthrough", "Testsuite Dummy Pass Thru",
                 "Testsuite Dummy authenticator.  Just passes through and is hardcoded to a specific user");
         addProviderInfo(result, "testsuite-dummy-registration", "Testsuite Dummy Pass Thru",

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/CustomFlowTest.java

@@ -22,9 +22,12 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.keycloak.OAuth2Constants;
+import org.keycloak.admin.client.resource.AuthenticationManagementResource;
 import org.keycloak.authentication.AuthenticationFlow;
 import org.keycloak.events.Details;
 import org.keycloak.events.Errors;
+import org.keycloak.events.admin.OperationType;
+import org.keycloak.events.admin.ResourceType;
 import org.keycloak.models.AuthenticationExecutionModel;
 import org.keycloak.representations.AccessToken;
 import org.keycloak.representations.RefreshToken;
@@ -34,13 +37,16 @@ import org.keycloak.representations.idm.ClientRepresentation;
 import org.keycloak.representations.idm.RealmRepresentation;
 import org.keycloak.representations.idm.UserRepresentation;
 import org.keycloak.testsuite.AssertEvents;
+import org.keycloak.testsuite.admin.ApiUtil;
 import org.keycloak.testsuite.pages.AppPage;
 import org.keycloak.testsuite.pages.AppPage.RequestType;
 import org.keycloak.testsuite.pages.ErrorPage;
 import org.keycloak.testsuite.pages.LoginPage;
 import org.keycloak.testsuite.pages.LoginPasswordUpdatePage;
 import org.keycloak.testsuite.pages.RegisterPage;
+import org.keycloak.testsuite.pages.TermsAndConditionsPage;
 import org.keycloak.testsuite.rest.representation.AuthenticatorState;
+import org.keycloak.testsuite.util.AdminEventPaths;
 import org.keycloak.testsuite.util.ClientBuilder;
 import org.keycloak.testsuite.util.ExecutionBuilder;
 import org.keycloak.testsuite.util.FlowBuilder;
@@ -48,7 +54,14 @@ import org.keycloak.testsuite.util.OAuthClient;
 import org.keycloak.testsuite.util.RealmRepUtil;
 import org.keycloak.testsuite.util.UserBuilder;
 
+import javax.ws.rs.core.Response;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
 import static org.junit.Assert.assertEquals;
+import static org.keycloak.testsuite.util.Matchers.statusCodeIs;
 
 /**
  * @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
@@ -171,6 +184,10 @@ public class CustomFlowTest extends AbstractFlowTest {
     @Page
     protected ErrorPage errorPage;
 
+    @Page
+    protected TermsAndConditionsPage termsPage;
+
+
     @Page
     protected LoginPasswordUpdatePage updatePasswordPage;
 
@@ -179,6 +196,56 @@ public class CustomFlowTest extends AbstractFlowTest {
 
     private static String userId;
 
+    /**
+     * KEYCLOAK-3506
+     *
+     * @throws Exception
+     */
+    @Test
+    public void testRequiredAfterAlternative() throws Exception {
+        AuthenticationManagementResource authMgmtResource = testRealm().flows();
+        Map<String, String> params = new HashMap();
+        String flowAlias = "Browser Flow With Extra";
+        params.put("newName", flowAlias);
+        Response response = authMgmtResource.copy("browser", params);
+        String flowId = null;
+        try {
+            Assert.assertThat("Copy flow", response, statusCodeIs(Response.Status.CREATED));
+            AuthenticationFlowRepresentation newFlow = findFlowByAlias(flowAlias);
+            flowId = newFlow.getId();
+        } finally {
+            response.close();
+        }
+
+        AuthenticationExecutionRepresentation execution = ExecutionBuilder.create()
+                .parentFlow(flowId)
+                .requirement(AuthenticationExecutionModel.Requirement.REQUIRED.toString())
+                .authenticator(ClickThroughAuthenticator.PROVIDER_ID)
+                .priority(10)
+                .authenticatorFlow(false)
+                .build();
+        testRealm().flows().addExecution(execution);
+
+        RealmRepresentation rep = testRealm().toRepresentation();
+        rep.setBrowserFlow(flowAlias);
+        testRealm().update(rep);
+        rep = testRealm().toRepresentation();
+        Assert.assertEquals(flowAlias, rep.getBrowserFlow());
+
+        loginPage.open();
+        String url = driver.getCurrentUrl();
+        // test to make sure we aren't skipping anything
+        loginPage.login("test-user@localhost", "bad-password");
+        Assert.assertTrue(loginPage.isCurrent());
+        loginPage.login("test-user@localhost", "password");
+        Assert.assertTrue(termsPage.isCurrent());
+
+
+
+
+
+    }
+
     @Test
     public void loginSuccess() {
         AuthenticatorState state = new AuthenticatorState();