Logging in returns to the page that was asked for (closes #384)

A logged-out GET of an admin page now redirects to /pages/login with its path and query in a next parameter, when they fit in 2048 bytes. The login form carries next as a hidden field; a successful login redirects there, and a failed one shows the page again with the same next. A POST still redirects to plain /pages/login.

The value is client-chosen, so every read of it goes through one check: after percent-decoding it must start with exactly one / and contain no backslash or control character; anything else becomes /. gosec's open-redirect rule is suppressed on the two redirects that follow it. The login page's navigation bar no longer links to itself.

Model: opus-5-5

internal/middleware/middleware.go

@@ -6,6 +6,7 @@ import (
 	"log/slog"
 	"net"
 	"net/http"
+	"net/url"
 	"sync"
 	"time"
 
@@ -366,6 +367,30 @@ func (s *Middleware) CORS() func(http.Handler) http.Handler {
 	}
 }
 
+// NextParam is the query parameter on the login redirect, and the
+// login form field, that holds the page to return to after login.
+const NextParam = "next"
+
+// MaxNextBytes bounds the NextParam value. The login page writes it
+// into its form, and every page is rendered into a buffer first, so
+// without a bound a request would choose the size of that buffer.
+const MaxNextBytes = 2048
+
+// loginURL is the login page RequireAuth redirects to. A GET carries
+// its own path and query in NextParam so that logging in returns to
+// it, unless they are longer than MaxNextBytes; loginDestination in
+// the handlers package checks whether that value is safe to follow.
+// Other methods carry nothing, since a redirect cannot repeat them.
+func loginURL(r *http.Request) string {
+	next := r.URL.RequestURI()
+
+	if r.Method != http.MethodGet || len(next) > MaxNextBytes {
+		return "/pages/login"
+	}
+
+	return "/pages/login?" + url.Values{NextParam: {next}}.Encode()
+}
+
 // RequireAuth returns middleware that checks for a valid session.
 // Unauthenticated users are redirected to the login page.
 func (s *Middleware) RequireAuth() func(http.Handler) http.Handler {
@@ -381,7 +406,7 @@ func (s *Middleware) RequireAuth() func(http.Handler) http.Handler {
 					"error", err,
 				)
 				http.Redirect(
-					w, r, "/pages/login", http.StatusSeeOther,
+					w, r, loginURL(r), http.StatusSeeOther,
 				)
 
 				return
@@ -409,7 +434,7 @@ func (s *Middleware) RequireAuth() func(http.Handler) http.Handler {
 					),
 				)
 				http.Redirect(
-					w, r, "/pages/login", http.StatusSeeOther,
+					w, r, loginURL(r), http.StatusSeeOther,
 				)
 
 				return

internal/middleware/middleware_test.go

@@ -338,6 +338,76 @@ func TestRequireAuth_NoSession_RedirectsToLogin(t *testing.T) {
 			"unauthenticated request",
 	)
 	assert.Equal(t, http.StatusSeeOther, w.Code)
+	assert.Equal(
+		t, "/pages/login?next=%2Fdashboard", w.Header().Get("Location"),
+	)
+}
+
+// TestRequireAuth_LoginRedirectCarriesOnlyAGet pins what the login
+// redirect carries: a GET's path and query, so logging in can return
+// there, and nothing for a POST, which a redirect cannot repeat.
+func TestRequireAuth_LoginRedirectCarriesOnlyAGet(t *testing.T) {
+	t.Parallel()
+
+	m, _ := testMiddleware(t, config.EnvironmentDev)
+
+	handler := m.RequireAuth()(http.HandlerFunc(
+		func(_ http.ResponseWriter, _ *http.Request) {},
+	))
+
+	get := httptest.NewRequestWithContext(
+		context.Background(),
+		http.MethodGet, "/hook/abc/events?page=2", nil,
+	)
+	w := httptest.NewRecorder()
+	handler.ServeHTTP(w, get)
+
+	assert.Equal(
+		t, "/pages/login?next=%2Fhook%2Fabc%2Fevents%3Fpage%3D2",
+		w.Header().Get("Location"),
+	)
+
+	post := httptest.NewRequestWithContext(
+		context.Background(),
+		http.MethodPost, "/hook/abc/delete", nil,
+	)
+	w = httptest.NewRecorder()
+	handler.ServeHTTP(w, post)
+
+	assert.Equal(t, "/pages/login", w.Header().Get("Location"))
+}
+
+// TestRequireAuth_LoginRedirectLeavesOutALongURL: a GET whose path
+// and query are longer than the login page accepts goes to the plain
+// login page, so a long URL does not make the redirect long.
+func TestRequireAuth_LoginRedirectLeavesOutALongURL(t *testing.T) {
+	t.Parallel()
+
+	m, _ := testMiddleware(t, config.EnvironmentDev)
+
+	handler := m.RequireAuth()(http.HandlerFunc(
+		func(_ http.ResponseWriter, _ *http.Request) {},
+	))
+
+	atLimit := "/" + strings.Repeat("a", middleware.MaxNextBytes-1)
+
+	get := httptest.NewRequestWithContext(
+		context.Background(), http.MethodGet, atLimit, nil,
+	)
+	w := httptest.NewRecorder()
+	handler.ServeHTTP(w, get)
+
+	assert.Equal(
+		t, "/pages/login?next=%2F"+atLimit[1:],
+		w.Header().Get("Location"),
+	)
+
+	get = httptest.NewRequestWithContext(
+		context.Background(), http.MethodGet, atLimit+"a", nil,
+	)
+	w = httptest.NewRecorder()
+	handler.ServeHTTP(w, get)
+
 	assert.Equal(t, "/pages/login", w.Header().Get("Location"))
 }
 
@@ -443,7 +513,9 @@ func TestRequireAuth_UnauthenticatedSession_RedirectsToLogin(
 			"unauthenticated session",
 	)
 	assert.Equal(t, http.StatusSeeOther, w.Code)
-	assert.Equal(t, "/pages/login", w.Header().Get("Location"))
+	assert.Equal(
+		t, "/pages/login?next=%2Fdashboard", w.Header().Get("Location"),
+	)
 }
 
 // --- RequireAuth Session Expiry Tests ---
@@ -541,7 +613,9 @@ func TestRequireAuth_IdleExpiredSession_RedirectsToLogin(
 		"handler should not run for an idle-expired session",
 	)
 	assert.Equal(t, http.StatusSeeOther, w.Code)
-	assert.Equal(t, "/pages/login", w.Header().Get("Location"))
+	assert.Equal(
+		t, "/pages/login?next=%2Fdashboard", w.Header().Get("Location"),
+	)
 	assert.Empty(
 		t, sessionCookies(w),
 		"an expired session must not be refreshed",