Let a browser with cookies from an earlier database log in (closes #359) #362

Merged
clawbot merged 1 commits from issue-359-stale-cookie-login into next 2026-09-29 12:58:44 +02:00
6 changed files with 145 additions and 12 deletions
Showing only changes of commit a99ddbdcd3 - Show all commits
+6
View File
@@ -538,6 +538,12 @@ its Argon2id hash. There is no second account and no forgot-password
flow, so the banner and the reset command below are the only two ways
in.
A start that finds no `webhooker.db` in `DATA_DIR` also logs
`created a new, empty database` at `WARN`, with the file's path,
shortly before the banner. On a deployment that has run before, that
line means `DATA_DIR` was empty, most often because its volume is not
mounted.
#### Recovering a lost admin password
`webhooker resetpw` sets an existing account's password from the
@@ -3,6 +3,8 @@ package database_test
import (
"bytes"
"context"
"log/slog"
"path/filepath"
"strings"
"testing"
@@ -83,3 +85,37 @@ func TestFirstBoot_PrintsTheAdminPasswordAsABanner(t *testing.T) {
t, ok, "the printed password must open the seeded account",
)
}
// TestNewDatabase_IsLoggedWithItsPath is the log half of
// https://git.eeqj.de/sneak/webhooker/issues/359. A DATA_DIR that is
// unexpectedly empty boots exactly like a first start, so the start
// that creates the database must say so, and where. Opening that
// database again must not.
func TestNewDatabase_IsLoggedWithItsPath(t *testing.T) {
t.Parallel()
dir := t.TempDir()
open := func() string {
var out bytes.Buffer
db, err := database.Open(dir, slog.New(slog.NewTextHandler(&out, nil)))
require.NoError(t, err)
require.NoError(t, db.Close())
return out.String()
}
const created = `level=WARN msg="created a new, empty database"`
first := open()
second := open()
assert.Contains(
t, first,
created+" path="+filepath.Join(dir, database.MainDBFileName),
)
assert.NotContains(
t, second, created, "an existing database is not new",
)
}
+12
View File
@@ -8,6 +8,7 @@ import (
"errors"
"fmt"
"io"
"io/fs"
"log/slog"
"os"
"path/filepath"
@@ -199,6 +200,12 @@ func (d *Database) connectTo(dataDir string) error {
// Construct the main application database path inside DATA_DIR.
dbPath := filepath.Join(dataDir, MainDBFileName)
// Checked before opening, which creates the file. A DATA_DIR that
// is unexpectedly empty -- its volume not mounted, say -- looks
// exactly like a first start, so a new database is a warning.
_, statErr := os.Stat(dbPath)
created := errors.Is(statErr, fs.ErrNotExist)
// Opened through OpenSQLite so this handle carries the same WAL
// journaling, busy timeout, immediate-transaction locking, and pool
// bounds as every other database file. See sqlite_open.go.
@@ -229,7 +236,12 @@ func (d *Database) connectTo(dataDir string) error {
}
d.db = db
if created {
d.log.Warn("created a new, empty database", "path", dbPath)
} else {
d.log.Info("connected to database", "path", dbPath)
}
// Run migrations
return d.migrate()
+69 -3
View File
@@ -7,6 +7,7 @@ import (
"net/http/httptest"
"net/url"
"regexp"
"slices"
"strconv"
"strings"
"testing"
@@ -220,9 +221,21 @@ func (e *testEnv) csrfFrom(
// out of the markup has to be unescaped before it is submitted.
token := html.UnescapeString(match[1])
combined := make([]*http.Cookie, 0, len(cookies))
combined = append(combined, cookies...)
combined = append(combined, w.Result().Cookies()...)
// A cookie the page sets replaces the one of the same name, as in
// a browser. Sent both, the server would read the first, older one.
set := w.Result().Cookies()
combined := make([]*http.Cookie, 0, len(cookies)+len(set))
for _, c := range cookies {
replaced := slices.ContainsFunc(set, func(n *http.Cookie) bool {
return n.Name == c.Name
})
if !replaced {
combined = append(combined, c)
}
}
combined = append(combined, set...)
return token, combined
}
@@ -614,6 +627,59 @@ func TestPagesLogin_CorrectPasswordSurvivesASpentBudget(
)
}
// TestPagesLogin_CookiesFromAnEarlierDatabase is
// https://git.eeqj.de/sneak/webhooker/issues/359. A new database
// brings a new session key, and the operator's browser still holds
// the session and CSRF cookies signed with the old one. Logging in
// must work as from a fresh browser and leave cookies the new key
// accepts.
func TestPagesLogin_CookiesFromAnEarlierDatabase(t *testing.T) {
t.Parallel()
const (
username = "operator"
password = "correct-horse-battery-staple"
)
earlier := newTestEnv(t)
earlierID, _ := earlier.seedUser(t, username, password)
_, stale := earlier.csrfFrom(t, "/pages/login", nil)
stale = append(stale, earlier.authCookies(t, earlierID, username)...)
env := newTestEnv(t)
env.seedUser(t, username, password)
token, cookies := env.csrfFrom(t, "/pages/login", stale)
form := url.Values{}
form.Set("csrf_token", token)
form.Set("username", username)
form.Set("password", password)
w := env.post("/pages/login", form, cookies)
require.Equal(
t, http.StatusSeeOther, w.Code,
"a session cookie from another key must not fail the login",
)
// The response deletes the old session cookie and then sets the
// new one; a browser keeps the last.
var fresh *http.Cookie
for _, c := range w.Result().Cookies() {
if c.Name == session.SessionName {
fresh = c
}
}
require.NotNil(t, fresh, "login must set a session cookie")
assert.Equal(
t, "/sources",
env.get("/", []*http.Cookie{fresh}).Header().Get("Location"),
"the new session cookie must authenticate",
)
}
// --- /user/{username} group ---
// TestPasswordChange_OversizeBody_RejectedAndPasswordUnchanged
+8 -7
View File
@@ -19,8 +19,8 @@ import (
)
// The tests below exercise the securecookie codecs underneath the
// store and nothing else: Session.Get only decodes, so no server-side
// expiry check takes part in the result. They exist because
// store and nothing else: they decode through the store itself, so no
// server-side expiry check takes part in the result. They exist because
// NewCookieStore gives its codecs a 30-day max age that assigning
// store.Options does not override, which would let the codec accept a
// cookie weeks past the cap the cookie attribute advertises.
@@ -75,10 +75,11 @@ func restamp(
return base64.URLEncoding.EncodeToString(payload)
}
// decodeCookie feeds value back through the store's decode path.
// decodeCookie feeds value back through the store's decode path. It
// asks the store rather than Session.Get, which treats a cookie that
// does not decode as absent and so hides the codec's reason.
func decodeCookie(
t *testing.T,
s *session.Session,
value string,
) (*sessions.Session, error) {
t.Helper()
@@ -94,7 +95,7 @@ func decodeCookie(
SameSite: http.SameSiteLaxMode,
})
sess, err := s.Get(req)
sess, err := session.NewStore(testKey()).Get(req, session.SessionName)
require.NotNil(t, sess)
return sess, err
@@ -105,7 +106,7 @@ func TestCodec_AcceptsCookieInsideAbsoluteCap(t *testing.T) {
s := testSession(t)
sess, err := decodeCookie(t, s, restamp(
sess, err := decodeCookie(t, restamp(
t,
issuedCookie(t, s),
time.Now().Add(-(testAbsoluteMaxAge-time.Hour)),
@@ -126,7 +127,7 @@ func TestCodec_RejectsCookiePastAbsoluteCap(t *testing.T) {
s := testSession(t)
sess, err := decodeCookie(t, s, restamp(
sess, err := decodeCookie(t, restamp(
t,
issuedCookie(t, s),
time.Now().Add(-(testAbsoluteMaxAge+time.Hour)),
+13 -1
View File
@@ -224,10 +224,22 @@ func New(
}
// Get retrieves a session for the request.
//
// A session cookie that does not decode -- one signed with an earlier
// session key, say, because the database was made anew -- is treated
// as absent: the caller gets a new, empty session and no error, and
// the next save replaces the cookie.
func (s *Session) Get(
r *http.Request,
) (*sessions.Session, error) {
return s.store.Get(r, SessionName)
sess, err := s.store.Get(r, SessionName)
if sess == nil {
return nil, err
}
// For a cookie that does not decode, gorilla/sessions returns a
// new, empty session alongside the error that is dropped here.
return sess, nil
}
// GetKey returns the raw 32-byte authentication key used for