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

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

internal/database/export_test.go

@@ -5,7 +5,6 @@ import (
 	"io"
 	"log/slog"
 	"os"
-	"testing"
 	"time"
 
 	"go.uber.org/fx"
@@ -80,14 +79,3 @@ func (d *Database) ExportSetBannerOut(w io.Writer) {
 func DummyPasswordHashForTest() string {
 	return dummyPasswordHash()
 }
-
-// HashAtShippedCostForTest makes HashPassword hash at the shipped
-// memory cost until t ends. t must not run in parallel with other
-// tests, which would hash at that cost alongside it.
-func HashAtShippedCostForTest(t *testing.T) {
-	t.Helper()
-
-	hashAtShippedCostInTest = true
-
-	t.Cleanup(func() { hashAtShippedCostInTest = false })
-}

internal/database/password.go

@@ -9,7 +9,6 @@ import (
 	"math/big"
 	"strings"
 	"sync"
-	"testing"
 
 	"golang.org/x/crypto/argon2"
 )
@@ -64,30 +63,10 @@ func DefaultPasswordConfig() *PasswordConfig {
 	}
 }
 
-// testArgon2Memory is the Argon2id memory cost, in KiB, that a test
-// binary hashes with: 1 MB instead of the shipped 64 MB. Every test
-// that starts a database hashes the bootstrap admin password, dozens
-// of them run in parallel, and under the race detector each 64 MB hash
-// holds about 150 MB. VerifyPassword reads the cost from the hash it
-// checks, so verification follows.
-const testArgon2Memory = 1024
-
-// hashAtShippedCostInTest makes a test binary hash at the shipped
-// memory cost. Only TestHashPassword_ShippedParameters sets it.
-//
-//nolint:gochecknoglobals // set by one test, see above
-var hashAtShippedCostInTest bool
-
-// HashPassword generates an Argon2id hash of the password. A binary
-// built by go test hashes at testArgon2Memory; one built by go build
-// always hashes at the defaults.
+// HashPassword generates an Argon2id hash of the password
 func HashPassword(password string) (string, error) {
 	config := DefaultPasswordConfig()
 
-	if testing.Testing() && !hashAtShippedCostInTest {
-		config.Memory = testArgon2Memory
-	}
-
 	// Generate a salt
 	salt := make([]byte, config.SaltLen)
 

internal/database/password_test.go

@@ -192,39 +192,6 @@ func TestHashPasswordUniqueness(t *testing.T) {
 	}
 }
 
-// TestHashPassword_ShippedParameters hashes and verifies through
-// HashPassword at the shipped Argon2id parameters. Every other test
-// hashes at the lower memory cost a test binary uses, so this is the
-// one that keeps production hashing covered. One hash and one
-// verification: each costs 64 MB.
-//
-//nolint:paralleltest // changes the hashing cost for the whole binary
-func TestHashPassword_ShippedParameters(t *testing.T) {
-	database.HashAtShippedCostForTest(t)
-
-	password := "correct horse battery staple"
-
-	hash, err := database.HashPassword(password)
-	if err != nil {
-		t.Fatalf("hashing with the shipped parameters: %v", err)
-	}
-
-	const shipped = "$argon2id$v=19$m=65536,t=1,p=4$"
-
-	if !strings.HasPrefix(hash, shipped) {
-		t.Errorf("hash = %q, want prefix %q", hash, shipped)
-	}
-
-	valid, err := database.VerifyPassword(password, hash)
-	if err != nil {
-		t.Fatalf("VerifyPassword() error = %v", err)
-	}
-
-	if !valid {
-		t.Error("VerifyPassword() returned false for correct password")
-	}
-}
-
 // TestVerifyDummyPassword_DoesRealWork covers the anti-enumeration
 // path. Login charges an unknown username a verification against a
 // dummy hash so that a nonexistent account is not answered in

internal/handlers/auth_test.go

@@ -525,49 +525,6 @@ func TestLogin_ReturnsOnlyToAPathOnThisSite(t *testing.T) {
 	}
 }
 
-// TestLogin_WrongPasswordKeepsTheRequestedPage: after a wrong
-// password the login page is shown again with the same next, so the
-// next attempt still returns to the page that was asked for.
-func TestLogin_WrongPasswordKeepsTheRequestedPage(t *testing.T) {
-	t.Parallel()
-
-	var (
-		h  *handlers.Handlers
-		db *database.Database
-	)
-
-	app := newTestApp(t, &h, &db)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	seedOperator(t, db)
-
-	form := url.Values{}
-	form.Set("username", operatorUser)
-	form.Set("password", "wrong")
-	form.Set("next", "/source/abc")
-
-	req := httptest.NewRequestWithContext(
-		context.Background(),
-		http.MethodPost,
-		"/pages/login",
-		strings.NewReader(form.Encode()),
-	)
-	req.Header.Set(
-		"Content-Type", "application/x-www-form-urlencoded",
-	)
-	req.RemoteAddr = sharedProxyPeer
-
-	w := httptest.NewRecorder()
-	h.HandleLoginSubmit().ServeHTTP(w, req)
-
-	assert.Equal(t, http.StatusUnauthorized, w.Code)
-	assert.Contains(
-		t, w.Body.String(), `name="next" value="/source/abc"`,
-	)
-}
-
 // loginPageGet renders the login page as a GET with the given next
 // value and cookies.
 func loginPageGet(

internal/resetpw/resetpw_test.go

@@ -140,7 +140,7 @@ func (n *noopEvictor) EvictWebhook(string) {}
 // and the database, exactly as internal/handlers builds them.
 //
 // One application per test function, not per case: every start that
-// finds no account seeds one with an Argon2id hash, and this package's
+// finds no account seeds one at 64 MB of Argon2id, and this package's
 // budget is not the place to spend that repeatedly.
 func newServerApp(
 	t *testing.T, dir string,

script/test

@@ -22,11 +22,6 @@
 # The one figure above 90s is GOMAXPROCS 1, a synthetic core floor rather than
 # a condition CI runs under. If a CPU-limited runner ever puts a real run near
 # 67s, that is the datum to revisit the org figure with.
-#
-# -p 4 -parallel 8 keep the run under 2 GB of memory: at most four test
-# binaries build or run at once, each with at most eight parallel tests. Under
-# -race every test binary and every link costs a few hundred MB, so the
-# defaults (one per core) add up to several GB on a many-core host.
 set -eu
 
 ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
@@ -34,7 +29,7 @@ ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
 main() {
     cd "$ROOT"
     "$ROOT/script/assets"
-    go test -v -race -p 4 -parallel 8 -timeout 90s ./...
+    go test -v -race -timeout 90s ./...
 }
 
 main "$@"