Logging in returns to the page that was asked for (closes #384)
check / check (push) Successful in 4m13s
check / check (push) Successful in 4m13s
RequireAuth now sends a logged-out GET to /pages/login with its path and query in a `next` parameter when they fit in 2048 bytes, the same limit the login page applies. The login form carries it as a hidden field, and a successful login redirects there when it is a path on this site; anything else, plain or percent-encoded, goes to `/`, which leads to the webhook list. A browser already logged in that opens the login page goes to the same place. The navigation bar on the login page no longer links to the login page. Model: opus-5-5
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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, "/source/abc/logs?page=2", nil,
|
||||
)
|
||||
w := httptest.NewRecorder()
|
||||
handler.ServeHTTP(w, get)
|
||||
|
||||
assert.Equal(
|
||||
t, "/pages/login?next=%2Fsource%2Fabc%2Flogs%3Fpage%3D2",
|
||||
w.Header().Get("Location"),
|
||||
)
|
||||
|
||||
post := httptest.NewRequestWithContext(
|
||||
context.Background(),
|
||||
http.MethodPost, "/source/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",
|
||||
|
||||
Reference in New Issue
Block a user