Align session codec max-age with the 7-day cap (closes #108)
Some checks failed
check / check (push) Has been cancelled
Some checks failed
check / check (push) Has been cancelled
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 a 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 fixes the RequireAuth save error, which was assigned to the outer err and is now scoped to the branch that produces it, and two README claims: 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