Align session codec max-age with the 7-day cap (closes #108)
All checks were successful
check / check (push) Successful in 3m5s
All checks were successful
check / check (push) Successful in 3m5s
sessions.NewCookieStore gives its securecookie codecs a 30-day max age, and assigning store.Options never touches Codecs. The store therefore decoded a cookie up to 30 days old while the cookie attribute and the server-side expiry check both said 7 days, so a refactor of that check -- or a second decode path that skips IsAuthenticated -- would silently reopen a 30-day window. Build the store through store.MaxAge, which sets Options.MaxAge and propagates to every codec. The store is now built by newStore, the one place a store is constructed; Regenerate shares its cookie attributes via cookieOptions. Tests: - Two codec tests decode a re-stamped cookie an hour inside and an hour outside the cap. They only exercise Session.Get, so they pin the codec: reverting the fix fails the rejection test whether or not the server-side expiry check is present. - The lazy-refresh bound is pinned two-sidedly, so the documented "expires up to 10% early, never late" guarantee now fails the suite if idleRefreshDivisor drifts in either direction. Also scopes the RequireAuth save error to a distinct saveErr variable instead of reusing the outer err. It is a plain assignment rather than an inline "if saveErr := ...; saveErr != nil", because the repo's noinlineerr linter rejects that form. Behaviour is unchanged; this is scoping hygiene, not a bug fix. Two README claims are corrected as well: the idle timeout is disabled by any non-positive value, not only 0, and deploying the two-clock expiry logs existing sessions out once because they carry no timestamps. #108
This commit is contained in:
150
internal/session/codec_test.go
Normal file
150
internal/session/codec_test.go
Normal file
@@ -0,0 +1,150 @@
|
||||
package session_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"crypto/hmac"
|
||||
"crypto/sha256"
|
||||
"encoding/base64"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/gorilla/sessions"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/webhooker/internal/session"
|
||||
)
|
||||
|
||||
// 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
|
||||
// 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.
|
||||
|
||||
// issuedCookie returns a session cookie the store itself wrote.
|
||||
func issuedCookie(t *testing.T, s *session.Session) string {
|
||||
t.Helper()
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), http.MethodGet, "/", nil)
|
||||
w := httptest.NewRecorder()
|
||||
|
||||
sess, err := s.Get(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
sess.Values["probe"] = "value"
|
||||
require.NoError(t, s.Save(req, w, sess))
|
||||
|
||||
cookies := w.Result().Cookies()
|
||||
require.Len(t, cookies, 1)
|
||||
|
||||
return cookies[0].Value
|
||||
}
|
||||
|
||||
// restamp rewrites the timestamp inside an encoded session cookie and
|
||||
// re-signs it, yielding the cookie the store would have written at
|
||||
// that instant. securecookie stamps the encoding time itself and
|
||||
// exposes no seam to move it, so its wire format is reproduced here:
|
||||
// the base64url payload is "date|value|mac", where mac is HMAC-SHA256
|
||||
// of "name|date|value" under the store's key.
|
||||
func restamp(
|
||||
t *testing.T,
|
||||
encoded string,
|
||||
at time.Time,
|
||||
) string {
|
||||
t.Helper()
|
||||
|
||||
raw, err := base64.URLEncoding.DecodeString(encoded)
|
||||
require.NoError(t, err)
|
||||
|
||||
parts := strings.SplitN(string(raw), "|", 3)
|
||||
require.Len(t, parts, 3)
|
||||
|
||||
stamped := fmt.Sprintf("%d|%s", at.Unix(), parts[1])
|
||||
|
||||
mac := hmac.New(sha256.New, testKey())
|
||||
_, err = mac.Write([]byte(session.SessionName + "|" + stamped))
|
||||
require.NoError(t, err)
|
||||
|
||||
payload := append([]byte(stamped+"|"), mac.Sum(nil)...)
|
||||
|
||||
return base64.URLEncoding.EncodeToString(payload)
|
||||
}
|
||||
|
||||
// decodeCookie feeds value back through the store's decode path.
|
||||
func decodeCookie(
|
||||
t *testing.T,
|
||||
s *session.Session,
|
||||
value string,
|
||||
) (*sessions.Session, error) {
|
||||
t.Helper()
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), http.MethodGet, "/", nil)
|
||||
req.AddCookie(&http.Cookie{
|
||||
Name: session.SessionName,
|
||||
Value: value,
|
||||
Path: "/",
|
||||
HttpOnly: true,
|
||||
Secure: true,
|
||||
SameSite: http.SameSiteLaxMode,
|
||||
})
|
||||
|
||||
sess, err := s.Get(req)
|
||||
require.NotNil(t, sess)
|
||||
|
||||
return sess, err
|
||||
}
|
||||
|
||||
func TestCodec_AcceptsCookieInsideAbsoluteCap(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s := testSession(t)
|
||||
|
||||
sess, err := decodeCookie(t, s, restamp(
|
||||
t,
|
||||
issuedCookie(t, s),
|
||||
time.Now().Add(-(testAbsoluteMaxAge-time.Hour)),
|
||||
))
|
||||
require.NoError(t, err)
|
||||
assert.False(
|
||||
t, sess.IsNew,
|
||||
"a cookie inside the cap must still decode",
|
||||
)
|
||||
assert.Equal(
|
||||
t, "value", sess.Values["probe"],
|
||||
"decoding must yield the values that were saved",
|
||||
)
|
||||
}
|
||||
|
||||
func TestCodec_RejectsCookiePastAbsoluteCap(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s := testSession(t)
|
||||
|
||||
sess, err := decodeCookie(t, s, restamp(
|
||||
t,
|
||||
issuedCookie(t, s),
|
||||
time.Now().Add(-(testAbsoluteMaxAge+time.Hour)),
|
||||
))
|
||||
require.Error(
|
||||
t, err,
|
||||
"the codec must refuse a cookie older than the cap",
|
||||
)
|
||||
assert.Contains(
|
||||
t, err.Error(), "expired timestamp",
|
||||
"rejection must come from the codec's age check",
|
||||
)
|
||||
assert.True(
|
||||
t, sess.IsNew,
|
||||
"a cookie past the cap must not populate a session",
|
||||
)
|
||||
assert.Nil(
|
||||
t, sess.Values["probe"],
|
||||
"a cookie past the cap must not yield its values",
|
||||
)
|
||||
}
|
||||
Reference in New Issue
Block a user