Redirect to server root if login flow is started with no destination

Author: liggitt

pkg/auth/server/login/login.go

@@ -125,6 +125,9 @@ func (l *Login) handleLoginForm(w http.ResponseWriter, req *http.Request) {
 	if then := req.URL.Query().Get("then"); then != "" {
 		// TODO: sanitize 'then'
 		form.Values.Then = then
+	} else {
+		http.Redirect(w, req, "/", http.StatusFound)
+		return
 	}
 
 	form.ErrorCode = req.URL.Query().Get("reason")
@@ -152,6 +155,10 @@ func (l *Login) handleLogin(w http.ResponseWriter, req *http.Request) {
 		return
 	}
 	then := req.FormValue("then")
+	if len(then) == 0 {
+		http.Redirect(w, req, "/", http.StatusFound)
+		return
+	}
 	username, password := req.FormValue("username"), req.FormValue("password")
 	if username == "" {
 		failed(errorCodeUserRequired, w, req)

pkg/auth/server/login/login_test.go

@@ -53,12 +53,13 @@ func TestLogin(t *testing.T) {
 		"display form": {
 			CSRF: &csrf.FakeCSRF{Token: "test"},
 			Auth: &testAuth{},
-			Path: "/login",
+			Path: "/login?then=%2F",
 
 			ExpectStatusCode: 200,
 			ExpectContains: []string{
 				`action="/login"`,
 				`name="csrf" value="test"`,
+				`name="then" value="/"`,
 			},
 		},
 		"display form with errors": {
@@ -74,6 +75,21 @@ func TestLogin(t *testing.T) {
 				`danger`,
 			},
 		},
+		"redirect when GET has no then param": {
+			CSRF: &csrf.FakeCSRF{Token: "test"},
+			Auth: &testAuth{},
+			Path: "/login",
+
+			ExpectStatusCode: 302,
+			ExpectRedirect:   "/",
+		},
+		"redirect when POST is missing then param": {
+			CSRF:           &csrf.FakeCSRF{Token: "test"},
+			Auth:           &testAuth{},
+			Path:           "/login",
+			PostValues:     url.Values{"csrf": []string{"test"}},
+			ExpectRedirect: "/",
+		},
 		"redirect when POST fails CSRF": {
 			CSRF:           &csrf.FakeCSRF{Token: "test"},
 			Auth:           &testAuth{},
@@ -94,8 +110,9 @@ func TestLogin(t *testing.T) {
 			Path: "/login",
 			PostValues: url.Values{
 				"csrf": []string{"test"},
+				"then": []string{"anotherurl"},
 			},
-			ExpectRedirect: "/login?reason=user_required",
+			ExpectRedirect: "/login?reason=user_required&then=anotherurl",
 		},
 		"redirect when not authenticated": {
 			CSRF: &csrf.FakeCSRF{Token: "test"},
@@ -104,8 +121,9 @@ func TestLogin(t *testing.T) {
 			PostValues: url.Values{
 				"csrf":     []string{"test"},
 				"username": []string{"user"},
+				"then":     []string{"anotherurl"},
 			},
-			ExpectRedirect: "/login?reason=access_denied",
+			ExpectRedirect: "/login?reason=access_denied&then=anotherurl",
 		},
 		"redirect on auth error": {
 			CSRF: &csrf.FakeCSRF{Token: "test"},
@@ -114,8 +132,9 @@ func TestLogin(t *testing.T) {
 			PostValues: url.Values{
 				"csrf":     []string{"test"},
 				"username": []string{"user"},
+				"then":     []string{"anotherurl"},
 			},
-			ExpectRedirect: "/login?reason=authentication_error",
+			ExpectRedirect: "/login?reason=authentication_error&then=anotherurl",
 		},
 		"redirect on lookup error": {
 			CSRF: &csrf.FakeCSRF{Token: "test"},
@@ -124,8 +143,9 @@ func TestLogin(t *testing.T) {
 			PostValues: url.Values{
 				"csrf":     []string{"test"},
 				"username": []string{"user"},
+				"then":     []string{"anotherurl"},
 			},
-			ExpectRedirect: "/login?reason=mapping_lookup_error",
+			ExpectRedirect: "/login?reason=mapping_lookup_error&then=anotherurl",
 		},
 		"redirect on claim error": {
 			CSRF: &csrf.FakeCSRF{Token: "test"},
@@ -134,8 +154,9 @@ func TestLogin(t *testing.T) {
 			PostValues: url.Values{
 				"csrf":     []string{"test"},
 				"username": []string{"user"},
+				"then":     []string{"anotherurl"},
 			},
-			ExpectRedirect: "/login?reason=mapping_claim_error",
+			ExpectRedirect: "/login?reason=mapping_claim_error&then=anotherurl",
 		},
 		"redirect preserving then param": {
 			CSRF: &csrf.FakeCSRF{Token: "test"},

test/integration/web_console_access_test.go

@@ -98,7 +98,7 @@ func TestAccessOriginWebConsole(t *testing.T) {
 	}{
 		"":                    {http.StatusFound, masterOptions.AssetConfig.PublicURL},
 		"healthz":             {http.StatusOK, ""},
-		"login":               {http.StatusOK, ""},
+		"login?then=%2F":      {http.StatusOK, ""},
 		"oauth/token/request": {http.StatusFound, masterOptions.AssetConfig.MasterPublicURL + "/oauth/authorize"},
 		"console":             {http.StatusMovedPermanently, "/console/"},
 		"console/":            {http.StatusOK, ""},
@@ -137,7 +137,7 @@ func TestAccessDisabledWebConsole(t *testing.T) {
 		location   string
 	}{
 		"healthz":             {http.StatusOK, ""},
-		"login":               {http.StatusOK, ""},
+		"login?then=%2F":      {http.StatusOK, ""},
 		"oauth/token/request": {http.StatusFound, masterOptions.AssetConfig.MasterPublicURL + "/oauth/authorize"},
 		"console":             {http.StatusForbidden, ""},
 		"console/":            {http.StatusForbidden, ""},
@@ -220,8 +220,8 @@ func TestAccessOriginWebConsoleMultipleIdentityProviders(t *testing.T) {
 
 		// Expect the providerSelectionURL to redirect to the loginURL
 		urlMap[providerSelectionURL] = urlResults{http.StatusFound, loginURL}
-		// Expect the loginURL to be valid
-		urlMap[loginURL] = urlResults{http.StatusOK, ""}
+		// Expect the loginURL to be valid (requires a 'then' param)
+		urlMap[loginURL+"?then=%2F"] = urlResults{http.StatusOK, ""}
 
 		// escape the query param the way the template will
 		templateIDPParam := templateEscapeHref(t, idpQueryParam)