Compare commits
1 Commits
ea92c616c2
...
issue-108-
| Author | SHA1 | Date | |
|---|---|---|---|
| 618b07ca0f |
21
README.md
21
README.md
@@ -93,7 +93,6 @@ TTY detection, and security headers are always applied.
|
||||
| `METRICS_USERNAME` | Basic auth username for `/metrics` | `""` |
|
||||
| `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` |
|
||||
| `SENTRY_DSN` | Sentry error reporting DSN | `""` |
|
||||
| `RETENTION_SWEEP_INTERVAL` | How often the retention reaper and archive sweeper run (Go duration) | `1h` |
|
||||
| `SESSION_IDLE_TIMEOUT` | Idle session timeout (Go duration) | `24h` |
|
||||
| `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint | `120` |
|
||||
| `TRUSTED_PROXIES` | CIDRs whose forwarded headers are trusted | `""` (none) |
|
||||
@@ -151,9 +150,10 @@ one runs out first:
|
||||
- **Idle expiry** (`SESSION_IDLE_TIMEOUT`, default `24h`) is a sliding
|
||||
window. Every authenticated request pushes it forward, so a session
|
||||
in continuous use never hits it, while an abandoned one expires a day
|
||||
after its last use. Set it to `0` to disable idle expiry entirely;
|
||||
the absolute cap below still applies. A set-but-unparseable value
|
||||
aborts startup rather than silently falling back to the default.
|
||||
after its last use. Any non-positive value (`0`, or a negative
|
||||
duration such as `-1s`) disables idle expiry entirely; the absolute
|
||||
cap below still applies. A set-but-unparseable value aborts startup
|
||||
rather than silently falling back to the default.
|
||||
- **Absolute expiry** is a fixed 7 days from login. Activity does
|
||||
**not** extend it: after a week, every session ends and the user
|
||||
authenticates again.
|
||||
@@ -165,6 +165,11 @@ idle window rather than on every request, which means a session may
|
||||
expire up to 10% early relative to the user's true last request, but
|
||||
never late.
|
||||
|
||||
Both clocks are anchored by timestamps stored in the session cookie.
|
||||
Sessions issued before this feature existed carry neither, so they are
|
||||
treated as expired: upgrading to a build that has it logs every
|
||||
existing session out once, and those users sign in again.
|
||||
|
||||
#### Invalid values abort startup
|
||||
|
||||
The defaults above apply **only** to variables that are unset (or set
|
||||
@@ -261,10 +266,9 @@ webhooker solves this by acting as a durable intermediary:
|
||||
targets simultaneously. This enables patterns like forwarding a
|
||||
GitHub webhook to both a deployment service and a Slack channel.
|
||||
|
||||
5. **Replay** (not yet implemented) — Every received event is stored in
|
||||
full, which is what manual redelivery for debugging or testing will
|
||||
be built on. No redelivery exists today, in the web UI or the API;
|
||||
see [TODO.md](TODO.md).
|
||||
5. **Replay** — Stored events can be manually redelivered for debugging
|
||||
or testing, without requiring the original sender to fire the webhook
|
||||
again.
|
||||
|
||||
### Use Cases
|
||||
|
||||
@@ -274,7 +278,6 @@ webhooker solves this by acting as a durable intermediary:
|
||||
size, and delivery performance
|
||||
- **Debugging** and introspection of webhook payloads in the web UI
|
||||
- **Replay** of webhook events for application testing and development
|
||||
(planned; not yet implemented)
|
||||
- **Fan-out** delivery of a single webhook to multiple downstream
|
||||
targets
|
||||
- **High-availability ingestion** for delivery to less reliable backend
|
||||
|
||||
90
TODO.md
90
TODO.md
@@ -1,94 +1,35 @@
|
||||
# Workflow
|
||||
|
||||
One issue per unit of work, one branch and one PR per issue:
|
||||
|
||||
* ensure a tracked issue exists with a definition of done
|
||||
* branch from `next` (never from `main`)
|
||||
* do the work; open a PR based on `next` (never on `main`)
|
||||
* pass an independent review, then the manager squash-merges into `next`
|
||||
* push; nothing stays local-only
|
||||
|
||||
`next` is the branch for the next milestone and must stay green and
|
||||
mergeable to `main` without notice. One `next` -> `main` PR accumulates
|
||||
the milestone; releases are cut from `main` separately.
|
||||
|
||||
Issue branches do NOT touch this file — the manager maintains it on
|
||||
`next`. Every branch editing `TODO.md` conflicts with every other
|
||||
(#112).
|
||||
* branch (from `main`)
|
||||
* do the work in Next Step
|
||||
* move Next Step to the top of Completed Steps
|
||||
* move the top item of Future Steps into Next Step
|
||||
* commit (`TODO.md` changes in the same commit as the work)
|
||||
* merge to `main` if the branch is not protected, otherwise open a PR
|
||||
* push
|
||||
|
||||
# Status
|
||||
|
||||
pre-1.0. No git tags exist. `main` (4f5ecb1) is a working webhook proxy
|
||||
pre-1.0. No git tags exist. main (4f5ecb1) is a working webhook proxy
|
||||
with auth, CSRF/SSRF protections, login rate limiting, Slack target,
|
||||
event retention (#63), the database archiving target (#43), the admin
|
||||
password change flow (#65), policy compliance (#6), pinned lint tooling
|
||||
(#55), and fail-loud configuration parsing (#80).
|
||||
|
||||
`next` (9bfd033) holds the completed 1.0.0 milestone: every issue in it
|
||||
is closed, and it is verified green by cache-defeated container runs
|
||||
rather than by the CI badge, which can pass without executing anything
|
||||
(#119). Note: TODO.md was deliberately deleted from this repo in f9a9569
|
||||
(2026-03-01, #6); its content was folded into the README TODO section,
|
||||
which this draft reconstructs as of 2026-07-06.
|
||||
(#55), and fail-loud configuration parsing (#80). Note: TODO.md was
|
||||
deliberately deleted from this repo in f9a9569 (2026-03-01, #6); its
|
||||
content was folded into the README TODO section, which this draft
|
||||
reconstructs as of 2026-07-06.
|
||||
|
||||
# Next Step
|
||||
|
||||
Tag 1.0.0 from `main` once the milestone PR merges, then repair the CI
|
||||
gate (#119) before the next cycle's work lands — a gate that can report
|
||||
success without running is the one thing every other guarantee here
|
||||
rests on.
|
||||
Manual event redelivery from the web UI (replay is a core promised
|
||||
capability in the README rationale).
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-08-12 Bound the `X-Forwarded-For` scan's allocation to the hop
|
||||
cap: the reverse walk cuts entries with `strings.LastIndexByte`
|
||||
instead of joining and splitting, so a 1 MB header allocates 16 bytes
|
||||
rather than 1.6 MB per request on the unauthenticated receiver.
|
||||
Semantics proven unchanged by differential testing against the
|
||||
previous implementation (#133)
|
||||
- 2026-08-12 Cap the `X-Forwarded-For` hop walk at 64 entries, so an
|
||||
attacker-supplied chain cannot burn unbounded CPU in the rate-limit
|
||||
key function; running off the end falls back to the peer address
|
||||
(#124)
|
||||
- 2026-08-12 Gate forwarded-header trust behind a `TRUSTED_PROXIES` CIDR
|
||||
list: all three rate limiters key on the connection's own address
|
||||
unless the direct peer is a configured proxy, in which case
|
||||
`X-Forwarded-For` is walked right to left for the first non-proxy hop.
|
||||
Default trusts nothing, and a set-but-unparseable value aborts
|
||||
startup. Before this, any client could mint a fresh bucket or drain
|
||||
another's by rotating a spoofed header (#88)
|
||||
- 2026-08-11 Web UI cleanup: nav terminology unified on Webhooks, the
|
||||
Profile settings placeholder removed, a progressive-enhancement copy
|
||||
button for the entrypoint URL, and retention form copy that states the
|
||||
actual policy (deletion by the reaper, 0 retains forever) (#57)
|
||||
- 2026-08-11 Mask the webhook credential in delivery errors and logs:
|
||||
Go embeds the request URL in `*url.Error`, so every transport failure
|
||||
persisted the full Slack webhook URL into the per-webhook event
|
||||
database via `DeliveryResult.Error`, a field a future REST API would
|
||||
have served. `maskURLError` drops path, query and userinfo while
|
||||
preserving the wrapped cause, so `errors.Is`/`As` and `Timeout()`
|
||||
still work and DNS, TLS and timeout failures still read differently
|
||||
(#118)
|
||||
- 2026-08-11 Rate-limit the public webhook receiver endpoint
|
||||
(`RECEIVER_RATE_LIMIT`, default 120/min), keyed on client IP plus
|
||||
entrypoint path so one entrypoint cannot exhaust another's budget;
|
||||
over-limit requests get 429 with `Retry-After`. It was the one
|
||||
unauthenticated, internet-facing endpoint with no limit at all (#64)
|
||||
- 2026-08-11 Enforce the body size limit before CSRF parses the form:
|
||||
`MaxBodySize` is now first in all four form-parsing route groups, so
|
||||
an oversized request is rejected with 413 instead of being read in
|
||||
full by the CSRF middleware before any cap applied (#90)
|
||||
- 2026-08-11 Mask target config on the source detail page, which
|
||||
rendered the stored blob verbatim and so exposed the Slack
|
||||
incoming-webhook URL — a bearer credential that cannot be revoked
|
||||
per-holder. Config reaches the template only as a `TargetView` of
|
||||
labelled fields, and header values are rendered as a count (#113)
|
||||
- 2026-08-11 Allow `retention_days` of 0 to mean retain forever, via a
|
||||
sentinel written in `BeforeSave` so the GORM column default cannot
|
||||
win the race. Also bounds the reaper's cutoff arithmetic: day counts
|
||||
above 106751 overflowed `time.Duration` and wrapped the cutoff into
|
||||
the future, where every row matched and the sweep deleted everything
|
||||
(#79)
|
||||
- 2026-08-09 Inactivity-based session timeout: sliding idle expiry
|
||||
(`SESSION_IDLE_TIMEOUT`, default `24h`) refreshed on authenticated
|
||||
requests, with the 7-day absolute cap kept as an independent
|
||||
@@ -143,9 +84,6 @@ rests on.
|
||||
|
||||
# Future Steps
|
||||
|
||||
- Manual event redelivery from the web UI — the "Replay" capability the
|
||||
README describes as planned. No redelivery code exists anywhere in the
|
||||
tree; events are stored in full, which is all it would be built on
|
||||
- Delivery status and retry management UI
|
||||
- Per-webhook rate limiting in the receiver handler (per-webhook config
|
||||
plus handler enforcement; global limits must not apply to receiver
|
||||
|
||||
@@ -25,11 +25,6 @@ func IPFromHostPort(hp string) string {
|
||||
return ipFromHostPort(hp)
|
||||
}
|
||||
|
||||
// ClientKeyForTest exposes clientKey for testing.
|
||||
func ClientKeyForTest(m *Middleware, r *http.Request) string {
|
||||
return m.clientKey(r)
|
||||
}
|
||||
|
||||
// IsClientTLS exposes isClientTLS for testing.
|
||||
func IsClientTLS(r *http.Request) bool {
|
||||
return isClientTLS(r)
|
||||
|
||||
@@ -214,11 +214,11 @@ func (s *Middleware) RequireAuth() func(http.Handler) http.Handler {
|
||||
// handler runs, while the headers are still ours to
|
||||
// write.
|
||||
if s.session.Touch(sess) {
|
||||
err = s.session.Save(r, w, sess)
|
||||
if err != nil {
|
||||
saveErr := s.session.Save(r, w, sess)
|
||||
if saveErr != nil {
|
||||
s.log.Error(
|
||||
"auth middleware: failed to refresh session",
|
||||
"error", err,
|
||||
"error", saveErr,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -32,13 +32,6 @@ const (
|
||||
// receiver rate limit. The configured limit is expressed in
|
||||
// requests per minute.
|
||||
receiverRateInterval = 1 * time.Minute
|
||||
|
||||
// maxForwardedHops bounds how many X-Forwarded-For entries the
|
||||
// chain walk examines. Real chains are one to three hops, but a
|
||||
// client can pad the header up to MaxHeaderBytes, so without a
|
||||
// bound every request pays a walk proportional to whatever the
|
||||
// client sent.
|
||||
maxForwardedHops = 64
|
||||
)
|
||||
|
||||
// normalizeAddr strips the IPv4-in-IPv6 wrapper and any zone from
|
||||
@@ -77,49 +70,26 @@ func (m *Middleware) isTrustedProxy(addr netip.Addr) bool {
|
||||
// a trusted proxy is the client. A hop that cannot be read as a bare
|
||||
// address ends the walk: past it the chain is not the shape assumed
|
||||
// here, so the caller falls back to the peer address.
|
||||
//
|
||||
// Only the last maxForwardedHops entries are examined. A longer chain
|
||||
// is padding, and running out of hops falls back to the peer address
|
||||
// the same way an unreadable hop does.
|
||||
//
|
||||
// The entries are cut off the right end of each header value in place
|
||||
// rather than split out of it: the receiver is unauthenticated and a
|
||||
// client can pad the header up to MaxHeaderBytes, so splitting would
|
||||
// allocate in proportion to the padding (about 8 MB for a 1 MB
|
||||
// header) before the cap could discard any of it. Multiple header
|
||||
// values are walked in reverse for the same reason, since joining
|
||||
// them copies the whole chain.
|
||||
func (m *Middleware) forwardedClientAddr(
|
||||
r *http.Request,
|
||||
) (netip.Addr, bool) {
|
||||
seen := 0
|
||||
hops := strings.Split(
|
||||
strings.Join(r.Header.Values("X-Forwarded-For"), ","), ",",
|
||||
)
|
||||
|
||||
for _, value := range slices.Backward(
|
||||
r.Header.Values("X-Forwarded-For"),
|
||||
) {
|
||||
for last := false; !last && seen < maxForwardedHops; seen++ {
|
||||
hop := value
|
||||
for _, hop := range slices.Backward(hops) {
|
||||
hop = strings.TrimSpace(hop)
|
||||
if hop == "" {
|
||||
continue
|
||||
}
|
||||
|
||||
comma := strings.LastIndexByte(value, ',')
|
||||
if comma < 0 {
|
||||
last = true
|
||||
} else {
|
||||
hop, value = value[comma+1:], value[:comma]
|
||||
}
|
||||
addr, err := netip.ParseAddr(hop)
|
||||
if err != nil {
|
||||
return netip.Addr{}, false
|
||||
}
|
||||
|
||||
hop = strings.TrimSpace(hop)
|
||||
if hop == "" {
|
||||
continue
|
||||
}
|
||||
|
||||
addr, err := netip.ParseAddr(hop)
|
||||
if err != nil {
|
||||
return netip.Addr{}, false
|
||||
}
|
||||
|
||||
if addr = normalizeAddr(addr); !m.isTrustedProxy(addr) {
|
||||
return addr, true
|
||||
}
|
||||
if addr = normalizeAddr(addr); !m.isTrustedProxy(addr) {
|
||||
return addr, true
|
||||
}
|
||||
}
|
||||
|
||||
@@ -143,10 +113,8 @@ func (m *Middleware) clientKey(r *http.Request) string {
|
||||
peer, err := netip.ParseAddr(ipFromHostPort(r.RemoteAddr))
|
||||
if err != nil {
|
||||
// Not an address we can reason about; key on the raw
|
||||
// value, the most specific identity left. On a
|
||||
// Unix-socket listener every peer carries the same
|
||||
// RemoteAddr and so shares one bucket, which is the
|
||||
// fail-closed direction.
|
||||
// value rather than collapsing such peers into one
|
||||
// shared bucket.
|
||||
return r.RemoteAddr
|
||||
}
|
||||
|
||||
|
||||
@@ -8,10 +8,7 @@ import (
|
||||
"net/http/httptest"
|
||||
"net/netip"
|
||||
"os"
|
||||
"runtime"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"sneak.berlin/go/webhooker/internal/config"
|
||||
@@ -571,105 +568,6 @@ func TestRateLimitKey_ChainWalkSkipsClientPrepended(t *testing.T) {
|
||||
)
|
||||
}
|
||||
|
||||
// TestRateLimitKey_LongChainCapsWalkAndFallsBackToPeer covers the
|
||||
// hop-walk cap. A client behind the trusted proxy can pad
|
||||
// X-Forwarded-For with tens of thousands of trusted-looking hops,
|
||||
// which costs a walk proportional to the padding and, once the walk
|
||||
// runs off the left end of the chain, reaches the entry the client
|
||||
// put there. Capping the walk stops both: the key falls back to the
|
||||
// peer address, so rotating the head of the chain mints no bucket,
|
||||
// and the run does not scale with the chain length.
|
||||
func TestRateLimitKey_LongChainCapsWalkAndFallsBackToPeer(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
// 50k hops is roughly 0.9 MB, within the default
|
||||
// MaxHeaderBytes.
|
||||
const hops = 50000
|
||||
|
||||
padding := strings.Repeat(", 10.0.0.2", hops-1)
|
||||
|
||||
start := time.Now()
|
||||
|
||||
assertSharedBucket(
|
||||
t, trustedProxies("10.0.0.0/8"), "10.0.0.1:44444",
|
||||
func(i int) map[string]string {
|
||||
return map[string]string{
|
||||
headerXFF: fmt.Sprintf("9.9.9.%d%s", i+1, padding),
|
||||
}
|
||||
},
|
||||
"a padded X-Forwarded-For chain must fall back to the "+
|
||||
"peer address, not reach the client-controlled entry "+
|
||||
"at the head of the chain",
|
||||
)
|
||||
|
||||
assert.Less(
|
||||
t, time.Since(start), 2*time.Second,
|
||||
"the capped walk must not scale with the chain length",
|
||||
)
|
||||
}
|
||||
|
||||
// TestRateLimitKey_LongChainAllocationIsBounded is the allocation
|
||||
// half of the hop cap. Capping the walk still left every request
|
||||
// paying for the whole header the client sent, because the chain was
|
||||
// split before it was capped: about 8 MB of []string for the 1 MB a
|
||||
// default MaxHeaderBytes allows, on the unauthenticated receiver.
|
||||
//
|
||||
// Bytes are the measurement, not allocation count: strings.Split of a
|
||||
// 1 MB chain is a single allocation, so testing.AllocsPerRun scores
|
||||
// it as cheap. The test is deliberately sequential — it reads
|
||||
// process-wide counters, and Go runs this package's parallel tests
|
||||
// only after the sequential ones finish.
|
||||
//
|
||||
//nolint:paralleltest // reads process-wide allocation counters
|
||||
func TestRateLimitKey_LongChainAllocationIsBounded(t *testing.T) {
|
||||
// 100k hops of ", 10.0.0.2" is roughly 1 MB.
|
||||
const (
|
||||
hops = 100000
|
||||
iterations = 50
|
||||
maxBytesPerCall = 4096
|
||||
)
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{
|
||||
TrustedProxies: trustedProxies("10.0.0.0/8"),
|
||||
})
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), http.MethodPost, loginPath, nil,
|
||||
)
|
||||
req.RemoteAddr = "10.0.0.1:44444"
|
||||
req.Header.Set(
|
||||
headerXFF, "9.9.9.9"+strings.Repeat(", 10.0.0.2", hops),
|
||||
)
|
||||
|
||||
var before, after runtime.MemStats
|
||||
|
||||
var key string
|
||||
|
||||
runtime.ReadMemStats(&before)
|
||||
|
||||
for range iterations {
|
||||
key = middleware.ClientKeyForTest(m, req)
|
||||
}
|
||||
|
||||
runtime.ReadMemStats(&after)
|
||||
|
||||
perCall := (after.TotalAlloc - before.TotalAlloc) / iterations
|
||||
|
||||
assert.Less(
|
||||
t, perCall, uint64(maxBytesPerCall),
|
||||
"a %d-byte X-Forwarded-For must not allocate in proportion "+
|
||||
"to its length, but cost %d bytes per call",
|
||||
len(req.Header.Get(headerXFF)), perCall,
|
||||
)
|
||||
|
||||
assert.Equal(
|
||||
t, "10.0.0.1", key,
|
||||
"the padded chain must still fall back to the peer address",
|
||||
)
|
||||
}
|
||||
|
||||
// TestReceiverRateLimit_IgnoresForwardedFromUntrustedPeer proves
|
||||
// the receiver limiter uses the same gated key function as the
|
||||
// POST limiters.
|
||||
|
||||
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",
|
||||
)
|
||||
}
|
||||
10
internal/session/export_test.go
Normal file
10
internal/session/export_test.go
Normal file
@@ -0,0 +1,10 @@
|
||||
package session
|
||||
|
||||
import "github.com/gorilla/sessions"
|
||||
|
||||
// NewStore exposes the production cookie-store constructor so tests
|
||||
// exercise the store the application actually runs with, rather than a
|
||||
// lookalike assembled in the test.
|
||||
func NewStore(key []byte, secure bool) *sessions.CookieStore {
|
||||
return newStore(key, secure)
|
||||
}
|
||||
@@ -100,6 +100,35 @@ type Session struct {
|
||||
now func() time.Time
|
||||
}
|
||||
|
||||
// cookieOptions returns the cookie attributes used for every session
|
||||
// cookie. MaxAge is deliberately left at its zero value: for a store
|
||||
// it is set through CookieStore.MaxAge (see newStore), and for a
|
||||
// single session it is copied from the store's options.
|
||||
func cookieOptions(secure bool) *sessions.Options {
|
||||
return &sessions.Options{
|
||||
Path: "/",
|
||||
HttpOnly: true,
|
||||
Secure: secure,
|
||||
SameSite: http.SameSiteLaxMode,
|
||||
}
|
||||
}
|
||||
|
||||
// newStore builds the session cookie store.
|
||||
//
|
||||
// The absolute cap MUST be applied with store.MaxAge and not by
|
||||
// assigning store.Options.MaxAge. NewCookieStore gives the underlying
|
||||
// securecookie codecs a 30-day max age of their own, and assigning
|
||||
// Options never touches Codecs -- so a store configured that way still
|
||||
// decodes a 30-day-old cookie, leaving the cookie attribute and the
|
||||
// codec disagreeing about the same policy. store.MaxAge sets both.
|
||||
func newStore(key []byte, secure bool) *sessions.CookieStore {
|
||||
store := sessions.NewCookieStore(key)
|
||||
store.Options = cookieOptions(secure)
|
||||
store.MaxAge(secondsPerDay * sessionMaxAgeDays)
|
||||
|
||||
return store
|
||||
}
|
||||
|
||||
// New creates a new session manager. The cookie store is
|
||||
// initialized during the fx OnStart phase after the database is
|
||||
// connected, using a session key that is auto-generated and stored
|
||||
@@ -142,19 +171,8 @@ func New(
|
||||
)
|
||||
}
|
||||
|
||||
store := sessions.NewCookieStore(keyBytes)
|
||||
|
||||
// Configure cookie options for security
|
||||
store.Options = &sessions.Options{
|
||||
Path: "/",
|
||||
MaxAge: secondsPerDay * sessionMaxAgeDays,
|
||||
HttpOnly: true,
|
||||
Secure: !params.Config.IsDev(),
|
||||
SameSite: http.SameSiteLaxMode,
|
||||
}
|
||||
|
||||
s.key = keyBytes
|
||||
s.store = store
|
||||
s.store = newStore(keyBytes, !params.Config.IsDev())
|
||||
s.log.Info("session manager initialized")
|
||||
|
||||
return nil
|
||||
@@ -350,13 +368,8 @@ func (s *Session) Regenerate(
|
||||
// Apply the standard session options (the destroyed old
|
||||
// session had MaxAge = -1, which store.New might inherit
|
||||
// from the cookie).
|
||||
newSess.Options = &sessions.Options{
|
||||
Path: "/",
|
||||
MaxAge: secondsPerDay * sessionMaxAgeDays,
|
||||
HttpOnly: true,
|
||||
Secure: !s.config.IsDev(),
|
||||
SameSite: http.SameSiteLaxMode,
|
||||
}
|
||||
newSess.Options = cookieOptions(!s.config.IsDev())
|
||||
newSess.Options.MaxAge = secondsPerDay * sessionMaxAgeDays
|
||||
|
||||
return newSess, nil
|
||||
}
|
||||
|
||||
@@ -39,6 +39,19 @@ func (c *fakeClock) Advance(d time.Duration) {
|
||||
c.t = c.t.Add(d)
|
||||
}
|
||||
|
||||
// testKey returns the fixed session key the tests sign with. The
|
||||
// codec tests re-sign cookies with it, so it must be the same key the
|
||||
// store was built from.
|
||||
func testKey() []byte {
|
||||
key := make([]byte, testKeySize)
|
||||
|
||||
for i := range key {
|
||||
key[i] = byte(i + 42)
|
||||
}
|
||||
|
||||
return key
|
||||
}
|
||||
|
||||
// testSession creates a Session with a real cookie store and the
|
||||
// real clock.
|
||||
func testSession(t *testing.T) *session.Session {
|
||||
@@ -59,20 +72,8 @@ func testSessionWithClock(
|
||||
) (*session.Session, *fakeClock) {
|
||||
t.Helper()
|
||||
|
||||
key := make([]byte, testKeySize)
|
||||
|
||||
for i := range key {
|
||||
key[i] = byte(i + 42)
|
||||
}
|
||||
|
||||
store := sessions.NewCookieStore(key)
|
||||
store.Options = &sessions.Options{
|
||||
Path: "/",
|
||||
MaxAge: 86400 * 7,
|
||||
HttpOnly: true,
|
||||
Secure: false,
|
||||
SameSite: http.SameSiteLaxMode,
|
||||
}
|
||||
key := testKey()
|
||||
store := session.NewStore(key, false)
|
||||
|
||||
cfg := &config.Config{
|
||||
Environment: config.EnvironmentDev,
|
||||
@@ -645,6 +646,34 @@ func TestTouch_LazyBelowRefreshThreshold(t *testing.T) {
|
||||
)
|
||||
}
|
||||
|
||||
func TestTouch_RefreshThresholdIsOneTenthOfIdleWindow(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// testRefreshDivisor restates the documented bound independently
|
||||
// of the implementation constant: the idle timestamp is rewritten
|
||||
// once it is a tenth of the idle window old, which is what makes
|
||||
// "expires up to 10% early, never late" true. Both assertions are
|
||||
// needed to pin it -- a larger divisor fails the first, a smaller
|
||||
// one fails the second.
|
||||
const testRefreshDivisor = 10
|
||||
|
||||
threshold := testIdleTimeout / testRefreshDivisor
|
||||
|
||||
s, sess, clock := authenticatedSession(t, testIdleTimeout)
|
||||
|
||||
clock.Advance(threshold - time.Second)
|
||||
assert.False(
|
||||
t, s.Touch(sess),
|
||||
"Touch must not rewrite the session below a tenth of the window",
|
||||
)
|
||||
|
||||
clock.Advance(time.Second)
|
||||
assert.True(
|
||||
t, s.Touch(sess),
|
||||
"Touch must rewrite the session at a tenth of the window",
|
||||
)
|
||||
}
|
||||
|
||||
func TestTouch_UnauthenticatedSessionIsNotRefreshed(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
// Webhooker client-side JavaScript
|
||||
console.log("Webhooker loaded");
|
||||
|
||||
// Copy-to-clipboard, as progressive enhancement.
|
||||
//
|
||||
|
||||
Reference in New Issue
Block a user