[FIXES JENKINS-47439] Setup wizard does not resume after restart (jen…

…kinsci#3166)

* [JENKINS-47439] Add a failing test

On first startup, the setup wizard goes into state NEW and the filter to
force display the setup wizard is installed.

On second startup, the setup wizard goes into state RESTART (which
assumes the setup wizard is done), and the setup wizard is skipped
completely.

This test expects that state NEW is retained upon restart when nothing
is done.

* [JENKINS-47439] Persist InstallState

In some cases, the heuristics to determine the current setup wizard state are
fragile. It is safer to persist the install state so that upon restart,
the setup wizard can resume where it was left off.

* Missing javadoc and since for new public methods

* s/XXX/FIXME/

* Missed that one

* Setup wizard filter should be removed when entering a state where setup is complete

* Use parameterized logging

* Improvements over previous impl

* Removed static isUsingSecurityToken. Now only determined from install
state.
* Call onInstallStateUpdate before InstallState#initializeState as the
latter can update state.

* Triggering a new build

core/src/main/java/hudson/util/PluginServletFilter.java

@@ -113,6 +113,25 @@ public static void addFilter(Filter filter) throws ServletException {
         }
     }
 
+    /**
+     * Checks whether the given filter is already registered in the chain.
+     * @param filter the filter to check.
+     * @return true if the filter is already registered in the chain.
+     * @since FIXME
+     */
+    public static boolean hasFilter(Filter filter) {
+        Jenkins j = Jenkins.getInstanceOrNull();
+        PluginServletFilter container = null;
+        if(j != null) {
+            container = getInstance(j.servletContext);
+        }
+        if (j == null || container == null) {
+            return LEGACY.contains(filter);
+        } else {
+            return container.list.contains(filter);
+        }
+    }
+
     public static void removeFilter(Filter filter) throws ServletException {
         Jenkins j = Jenkins.getInstanceOrNull();
         if (j==null || getInstance(j.servletContext) == null) {

core/src/main/java/jenkins/install/InstallState.java

@@ -49,7 +49,12 @@ public class InstallState implements ExtensionPoint {
      * Need InstallState != NEW for tests by default
      */
     @Extension
-    public static final InstallState UNKNOWN = new InstallState("UNKNOWN", true);
+    public static final InstallState UNKNOWN = new InstallState("UNKNOWN", true) {
+        @Override
+        public void initializeState() {
+            InstallUtil.proceedToNextStateFrom(this);
+        }
+    };
 
     /**
      * After any setup / restart / etc. hooks are done, states should be running
@@ -94,7 +99,7 @@ public void initializeState() {
      */
     @Extension
     public static final InstallState INITIAL_PLUGINS_INSTALLING = new InstallState("INITIAL_PLUGINS_INSTALLING", false);
-    
+
     /**
      * Security setup for a new Jenkins install.
      */
@@ -106,7 +111,7 @@ public void initializeState() {
             } catch (Exception e) {
                 throw new RuntimeException(e);
             }
-            
+
             InstallUtil.proceedToNextStateFrom(INITIAL_SECURITY_SETUP);
         }
     };
@@ -116,7 +121,7 @@ public void initializeState() {
      */
     @Extension
     public static final InstallState NEW = new InstallState("NEW", false);
-    
+
     /**
      * Restart of an existing Jenkins install.
      */

core/src/main/java/jenkins/install/InstallUtil.java

@@ -92,7 +92,6 @@ public T get() {
      */
     public static void proceedToNextStateFrom(InstallState prior) {
         InstallState next = getNextInstallState(prior);
-        if (Main.isDevelopmentMode) LOGGER.info("Install state transitioning from: " + prior + " to: " + next);
         if (next != null) {
             Jenkins.getInstance().setInstallState(next);
         }

core/src/main/java/jenkins/install/SetupWizard.java

@@ -64,7 +64,6 @@
 import net.sf.json.JSONObject;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
-import org.jenkinsci.remoting.engine.JnlpProtocol4Handler;
 import org.kohsuke.accmod.restrictions.DoNotUse;
 import org.kohsuke.stapler.interceptor.RequirePOST;
 
@@ -77,18 +76,17 @@
 @Restricted(NoExternalUse.class)
 @Extension
 public class SetupWizard extends PageDecorator {
+    public SetupWizard() {
+        checkFilter();
+    }
+
     /**
      * The security token parameter name
      */
     public static String initialSetupAdminUserName = "admin";
 
     private static final Logger LOGGER = Logger.getLogger(SetupWizard.class.getName());
 
-    /**
-     * Used to determine if this was a new install (vs. an upgrade, restart, or otherwise)
-     */
-    private static boolean isUsingSecurityToken = false;
-
     /**
      * Initialize the setup wizard, this will process any current state initializations
      */
@@ -168,32 +166,43 @@ public class SetupWizard extends PageDecorator {
                         + "*************************************************************" + ls
                         + "*************************************************************" + ls);
             }
-
-            try {
-                PluginServletFilter.addFilter(FORCE_SETUP_WIZARD_FILTER);
-                // if we're not using security defaults, we should not show the security token screen
-                // users will likely be sent to a login screen instead
-                isUsingSecurityToken = isUsingSecurityDefaults();
-            } catch (ServletException e) {
-                throw new RuntimeException("Unable to add PluginServletFilter for the SetupWizard", e);
-            }
         }
-        
+
         try {
             // Make sure plugin metadata is up to date
             UpdateCenter.updateDefaultSite();
         } catch (Exception e) {
             LOGGER.log(Level.WARNING, e.getMessage(), e);
         }
     }
-
+
+    private void setUpFilter() {
+        try {
+            if (!PluginServletFilter.hasFilter(FORCE_SETUP_WIZARD_FILTER)) {
+                PluginServletFilter.addFilter(FORCE_SETUP_WIZARD_FILTER);
+            }
+        } catch (ServletException e) {
+            throw new RuntimeException("Unable to add PluginServletFilter for the SetupWizard", e);
+        }
+    }
+
+    private void tearDownFilter() {
+        try {
+            if (PluginServletFilter.hasFilter(FORCE_SETUP_WIZARD_FILTER)) {
+                PluginServletFilter.removeFilter(FORCE_SETUP_WIZARD_FILTER);
+            }
+        } catch (ServletException e) {
+            throw new RuntimeException("Unable to remove PluginServletFilter for the SetupWizard", e);
+        }
+    }
+
     /**
      * Indicates a generated password should be used - e.g. this is a new install, no security realm set up
      */
+    @SuppressWarnings("unused") // used by jelly
     public boolean isUsingSecurityToken() {
         try {
-            return isUsingSecurityToken // only ever show the unlock page if using the security token
-                    && !Jenkins.getInstance().getInstallState().isSetupComplete()
+            return !Jenkins.getInstance().getInstallState().isSetupComplete()
                     && isUsingSecurityDefaults();
         } catch (Exception e) {
             // ignore
@@ -487,8 +496,6 @@ public HttpResponse doCompleteInstall() throws IOException, ServletException {
         Jenkins.getInstance().checkPermission(Jenkins.ADMINISTER);
         InstallUtil.saveLastExecVersion();
         setCurrentLevel(Jenkins.getVersion());
-        PluginServletFilter.removeFilter(FORCE_SETUP_WIZARD_FILTER);
-        isUsingSecurityToken = false; // this should not be considered new anymore
         InstallUtil.proceedToNextStateFrom(InstallState.INITIAL_SETUP_COMPLETED);
     }
 
@@ -508,7 +515,28 @@ public InstallState getInstallState(String name) {
         }
         return InstallState.valueOf(name);
     }
-
+
+    /**
+     * Called upon install state update.
+     * @param state the new install state.
+     * @since FIXME
+     */
+    public void onInstallStateUpdate(InstallState state) {
+        if (state.isSetupComplete()) {
+            tearDownFilter();
+        } else {
+            setUpFilter();
+        }
+    }
+
+    /**
+     * Returns whether the setup wizard filter is currently registered.
+     * @since FIXME
+     */
+    public boolean hasSetupWizardFilter() {
+        return PluginServletFilter.hasFilter(FORCE_SETUP_WIZARD_FILTER);
+    }
+
     /**
      * This filter will validate that the security token is provided
      */
@@ -544,4 +572,13 @@ public String getRequestURI() {
         public void destroy() {
         }
     };
+
+    /**
+     * Sets up the Setup Wizard filter if the current state requires it.
+     */
+    private void checkFilter() {
+        if (!Jenkins.getInstance().getInstallState().isSetupComplete()) {
+            setUpFilter();
+        }
+    }
 }

core/src/main/java/jenkins/model/Jenkins.java

@@ -328,7 +328,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
     /**
      * The Jenkins instance startup type i.e. NEW, UPGRADE etc
      */
-    private transient InstallState installState = InstallState.UNKNOWN;
+    private InstallState installState;
 
     /**
      * If we're in the process of an initial setup, 
@@ -924,7 +924,7 @@ protected Jenkins(File root, ServletContext context, PluginManager pluginManager
                 System.exit(0);
 
             setupWizard = new SetupWizard();
-            InstallUtil.proceedToNextStateFrom(InstallState.UNKNOWN);
+            getInstallState().initializeState();
 
             launchTcpSlaveAgentListener();
 
@@ -1033,9 +1033,12 @@ public InstallState getInstallState() {
     public void setInstallState(@Nonnull InstallState newState) {
         InstallState prior = installState;
         installState = newState;
-        if (!prior.equals(newState)) {
+        LOGGER.log(Main.isDevelopmentMode ? Level.INFO : Level.FINE, "Install state transitioning from: {0} to : {1}", new Object[] { prior, installState });
+        if (!newState.equals(prior)) {
+            getSetupWizard().onInstallStateUpdate(newState);
             newState.initializeState();
         }
+        saveQuietly();
     }
 
     /**

test/src/test/java/jenkins/install/SetupWizardRestartTest.java

@@ -0,0 +1,50 @@
+package jenkins.install;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+
+import org.apache.commons.io.FileUtils;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runners.model.Statement;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.RestartableJenkinsRule;
+
+import hudson.Main;
+import jenkins.model.Jenkins;
+
+public class SetupWizardRestartTest {
+    @Rule
+    public RestartableJenkinsRule rr = new RestartableJenkinsRule();
+
+    @Issue("JENKINS-47439")
+    @Test
+    public void restartKeepsSetupWizardState() {
+        rr.addStep(new Statement() {
+            @Override
+            public void evaluate() throws IOException {
+                // Modify state so that we get into the same conditions as a real start
+                Main.isUnitTest = false;
+                FileUtils.write(InstallUtil.getLastExecVersionFile(), "");
+                Jenkins j = rr.j.getInstance();
+                // Re-evaluate current state based on the new context
+                InstallUtil.proceedToNextStateFrom(InstallState.UNKNOWN);
+                assertEquals("Unexpected install state", InstallState.NEW, j.getInstallState());
+                assertTrue("Expecting setup wizard filter to be up", j.getSetupWizard().hasSetupWizardFilter());
+                InstallUtil.saveLastExecVersion();
+            }
+        });
+        // Check that the state is retained after a restart
+        rr.addStep(new Statement() {
+            @Override
+            public void evaluate() {
+                Jenkins j = rr.j.getInstance();
+                assertEquals("Unexpected install state", InstallState.NEW, j.getInstallState());
+                assertTrue("Expecting setup wizard filter to be up after restart",  j.getSetupWizard().hasSetupWizardFilter());
+            }
+        });
+    }
+
+}