From 33f26e439a88095845d2d0e5aade61f25c39bbe4 Mon Sep 17 00:00:00 2001
From: kohsuke <kohsuke@71c3de6d-444a-0410-be80-ed276b4c234a>
Date: Fri, 7 May 2010 22:36:13 +0000
Subject: [PATCH]     Hudson shouldn't show a login error page unless the user
 really failed to login (think about when the user presses a back button.)

git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@30828 71c3de6d-444a-0410-be80-ed276b4c234a
---
 core/src/main/java/hudson/Functions.java      |  1 +
 .../hudson/model/Hudson/loginError.jelly      | 44 +++++++++------
 .../test/java/hudson/security/LoginTest.java  | 53 +++++++++++++++++++
 3 files changed, 82 insertions(+), 16 deletions(-)
 create mode 100644 test/src/test/java/hudson/security/LoginTest.java

diff --git a/core/src/main/java/hudson/Functions.java b/core/src/main/java/hudson/Functions.java
index 3bae0de743..0fd7787009 100644
--- a/core/src/main/java/hudson/Functions.java
+++ b/core/src/main/java/hudson/Functions.java
@@ -43,6 +43,7 @@ import hudson.model.ParameterDefinition.ParameterDescriptor;
 import hudson.model.Project;
 import hudson.model.Run;
 import hudson.model.TopLevelItem;
+import hudson.model.User;
 import hudson.model.View;
 import hudson.model.JDK;
 import hudson.search.SearchableModelObject;
diff --git a/core/src/main/resources/hudson/model/Hudson/loginError.jelly b/core/src/main/resources/hudson/model/Hudson/loginError.jelly
index 7b2ad99ef4..577cbddfa7 100644
--- a/core/src/main/resources/hudson/model/Hudson/loginError.jelly
+++ b/core/src/main/resources/hudson/model/Hudson/loginError.jelly
@@ -24,20 +24,32 @@ THE SOFTWARE.
 
 <!-- report a login error -->
 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
-  <st:statusCode value="401" />
-  <l:layout>
-    <st:include page="sidepanel.jelly" />
-    <l:main-panel>
-      <div style="margin: 2em; text-align:center; color:red; font-weight:bold">
-        ${%Invalid login information. Please try again.}
-        <br/>       
-        <a href="login?from=${request.session.getAttribute('from')}">${%Try again}</a>
-      </div>
-      <div align="middle">
-        <div style="margin-top:2em; color:gray; width:20em">
-          ${%If you are a system administrator and suspect this to be a configuration problem, see the server console output for more details.}
-        </div>
-      </div>
-    </l:main-panel>
-  </l:layout>
+  <j:choose>
+    <j:new var="h" className="hudson.Functions" />
+    <j:when test="${app.isUseSecurity() and h.isAnonymous()}">
+      <!--
+        The only time the error message makes sense is when Hudson is protected and the user failed to authenticate.
+        If the user accidentally comes to this page (by back button, etc), redirect away to prevent a confusion.
+      -->
+      <st:statusCode value="401" />
+      <l:layout title="${%Login Error}">
+        <st:include page="sidepanel.jelly" />
+        <l:main-panel>
+          <div style="margin: 2em; text-align:center; color:red; font-weight:bold">
+            ${%Invalid login information. Please try again.}
+            <br/>       
+            <a href="login?from=${request.session.getAttribute('from')}">${%Try again}</a>
+          </div>
+          <div align="middle">
+            <div style="margin-top:2em; color:gray; width:20em">
+              ${%If you are a system administrator and suspect this to be a configuration problem, see the server console output for more details.}
+            </div>
+          </div>
+        </l:main-panel>
+      </l:layout>
+    </j:when>
+    <j:otherwise>
+      <st:redirect url="." />
+    </j:otherwise>
+  </j:choose>
 </j:jelly>
\ No newline at end of file
diff --git a/test/src/test/java/hudson/security/LoginTest.java b/test/src/test/java/hudson/security/LoginTest.java
new file mode 100644
index 0000000000..88ef816819
--- /dev/null
+++ b/test/src/test/java/hudson/security/LoginTest.java
@@ -0,0 +1,53 @@
+package hudson.security;
+
+import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
+import com.gargoylesoftware.htmlunit.html.HtmlPage;
+import org.jvnet.hudson.test.HudsonTestCase;
+import org.jvnet.hudson.test.recipes.PresetData;
+import org.jvnet.hudson.test.recipes.PresetData.DataSet;
+import org.xml.sax.SAXException;
+
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.net.URL;
+
+import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED;
+
+/**
+ * @author Kohsuke Kawaguchi
+ */
+public class LoginTest extends HudsonTestCase {
+    /**
+     * Requesting a loginError page directly should result in a redirect,
+     * on a non-secured Hudson.
+     */
+    public void testLoginErrorRedirect() throws Exception {
+        verifyNotError(createWebClient());
+    }
+
+    private void verifyNotError(WebClient wc) throws IOException, SAXException {
+        HtmlPage p = wc.goTo("loginError");
+        URL url = p.getWebResponse().getUrl();
+        System.out.println(url);
+        assertFalse(url.toExternalForm().contains("login"));
+    }
+
+    /**
+     * Same as {@link #testLoginErrorRedirect()} if the user has already successfully authenticated.
+     */
+    @PresetData(DataSet.ANONYMOUS_READONLY)
+    public void testLoginErrorRedirect2() throws Exception {
+        // in a secured Hudson, the error page should render.
+        WebClient wc = createWebClient();
+        try {
+            wc.goTo("loginError");
+            fail("Expecting a 401 error");
+        } catch (FailingHttpStatusCodeException e) {
+            e.printStackTrace();
+            assertEquals(SC_UNAUTHORIZED,e.getStatusCode());
+        }
+
+        // but not once the user logs in.
+        verifyNotError(wc.login("alice"));
+    }
+}
-- 
GitLab