Expire share links after 7 days, cleanup after 30

This commit is contained in:
Jonathon Leight committed 2026-07-29 21:00:14 -04:00
1 parent 3881c27c2d
commit f49b16e354
10 files changed
+744 -71

No files matched your search

+43 -31
View File
@@ -118,7 +118,10 @@ func run(logger *slog.Logger) error {
janitorDone := make(chan struct{})
go func() {
defer close(janitorDone)
pruneAuthCodes(bgCtx, st, authCodePruneInterval, logger)
runJanitor(bgCtx, janitorInterval, logger,
janitorSweep{"auth codes", st.PruneAuthCodes},
janitorSweep{"share links", st.PruneInvites},
)
}()
httpSrv := &http.Server{
@@ -176,41 +179,50 @@ func run(logger *slog.Logger) error {
return srvErr
}
// authCodePruneInterval is how often expired cross-host handoff codes are swept.
// They live for authCodeTTL (60s), so this bounds a dead row's lifetime at a few
// minutes — the table stays small, which is all that's needed. It matches the
// cadence scs uses for its own session-table cleanup.
const authCodePruneInterval = 5 * time.Minute
// janitorInterval is how often the expired-row sweeps run. It's set by the
// shortest-lived thing they clean — a handoff code (authCodeTTL, 60s) — so this
// bounds a dead code's lifetime at a few minutes, and it matches the cadence scs
// uses for its own session-table cleanup.
//
// Share links are collected on a completely different timescale: they're kept for
// store.ExpiredInviteGrace (30 days) past expiry so their owner can see and remove
// them, so nearly every sweep finds nothing to do. That's fine — the delete is one
// indexed statement, which isn't worth a second ticker to avoid.
const janitorInterval = 5 * time.Minute
// authCodePruner is the slice of the store the janitor needs. Declared at the point
// of use so the loop can be exercised without a database.
type authCodePruner interface {
PruneAuthCodes(ctx context.Context) (int64, error)
// janitorSweep is one periodic cleanup job: a name for the log, and the delete to
// run. Taking a closure rather than an interface keeps the janitor decoupled from
// the store (a store method value satisfies it directly) and lets the loop be
// exercised without a database.
type janitorSweep struct {
name string
prune func(ctx context.Context) (int64, error)
}
// pruneAuthCodes deletes expired handoff codes every `every` until ctx is cancelled.
// ConsumeAuthCode only removes codes that are actually redeemed, so abandoned ones
// (a sign-in whose tab was closed mid-handoff) would otherwise accumulate forever
// holding a user_id and login_id. It sweeps once on entry so a restart cleans up
// immediately rather than waiting out the first interval.
// runJanitor runs every sweep once on entry — so a restart cleans up immediately
// instead of waiting out the first interval — then again every `every` until ctx is
// cancelled.
//
// This lives here rather than on the analytics rollup ticker — which also prunes,
// on the same cadence — because analytics has no business knowing about auth codes.
// A transient failure is logged and retried on the next tick; a failure caused by
// shutdown cancelling ctx is not an error worth reporting.
func pruneAuthCodes(ctx context.Context, p authCodePruner, every time.Duration, logger *slog.Logger) {
sweep := func() {
n, err := p.PruneAuthCodes(ctx)
switch {
case ctx.Err() != nil:
return // shutting down; not a real failure
case err != nil:
logger.Error("prune auth codes", "err", err)
case n > 0:
logger.Info("pruned expired auth codes", "count", n)
// This lives here rather than on the analytics rollup ticker, which also prunes on
// the same cadence, because analytics has no business knowing about auth codes or
// share links. A transient failure is logged and retried on the next tick, and one
// failing sweep doesn't stop the others; a failure caused by shutdown cancelling ctx
// is not an error worth reporting.
func runJanitor(ctx context.Context, every time.Duration, logger *slog.Logger, sweeps ...janitorSweep) {
runAll := func() {
for _, s := range sweeps {
n, err := s.prune(ctx)
switch {
case ctx.Err() != nil:
return // shutting down; not a real failure
case err != nil:
logger.Error("janitor sweep failed", "sweep", s.name, "err", err)
case n > 0:
logger.Info("janitor removed expired rows", "sweep", s.name, "count", n)
}
}
}
sweep()
runAll()
t := time.NewTicker(every)
defer t.Stop()
@@ -219,7 +231,7 @@ func pruneAuthCodes(ctx context.Context, p authCodePruner, every time.Duration,
case <-ctx.Done():
return
case <-t.C:
sweep()
runAll()
}
}
}
+41 -20
View File
@@ -48,38 +48,38 @@ func waitForCalls(t *testing.T, f *fakePruner, n int, within time.Duration) {
}
}
// TestPruneAuthCodesSweepsImmediately: a restart must clean up without waiting out
// TestJanitorSweepsImmediately: a restart must clean up without waiting out
// a full interval, so the loop sweeps once on entry. The interval here is long
// enough that a tick can't be what we observed.
func TestPruneAuthCodesSweepsImmediately(t *testing.T) {
func TestJanitorSweepsImmediately(t *testing.T) {
t.Parallel()
f := newFakePruner(nil)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
go pruneAuthCodes(ctx, f, time.Hour, discardLogger())
go runJanitor(ctx, time.Hour, discardLogger(), janitorSweep{"test", f.PruneAuthCodes})
waitForCalls(t, f, 1, 2*time.Second)
}
// TestPruneAuthCodesRepeats: the ticker keeps firing, so cleanup is ongoing rather
// TestJanitorRepeats: the ticker keeps firing, so cleanup is ongoing rather
// than startup-only.
func TestPruneAuthCodesRepeats(t *testing.T) {
func TestJanitorRepeats(t *testing.T) {
t.Parallel()
f := newFakePruner(nil)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
go pruneAuthCodes(ctx, f, 2*time.Millisecond, discardLogger())
go runJanitor(ctx, 2*time.Millisecond, discardLogger(), janitorSweep{"test", f.PruneAuthCodes})
// One immediate sweep plus several ticks.
waitForCalls(t, f, 4, 2*time.Second)
}
// TestPruneAuthCodesStopsOnCancel is the one that matters for shutdown: main waits
// TestJanitorStopsOnCancel is the one that matters for shutdown: main waits
// on <-janitorDone, so a loop that ignored ctx would hang the process forever
// instead of exiting.
func TestPruneAuthCodesStopsOnCancel(t *testing.T) {
func TestJanitorStopsOnCancel(t *testing.T) {
t.Parallel()
f := newFakePruner(nil)
ctx, cancel := context.WithCancel(context.Background())
@@ -87,7 +87,7 @@ func TestPruneAuthCodesStopsOnCancel(t *testing.T) {
done := make(chan struct{})
go func() {
defer close(done)
pruneAuthCodes(ctx, f, time.Millisecond, discardLogger())
runJanitor(ctx, time.Millisecond, discardLogger(), janitorSweep{"test", f.PruneAuthCodes})
}()
waitForCalls(t, f, 1, 2*time.Second) // running
@@ -96,35 +96,56 @@ func TestPruneAuthCodesStopsOnCancel(t *testing.T) {
select {
case <-done:
case <-time.After(2 * time.Second):
t.Fatal("pruneAuthCodes did not return after ctx was cancelled — shutdown would hang")
t.Fatal("runJanitor did not return after ctx was cancelled — shutdown would hang")
}
}
// TestPruneAuthCodesSurvivesErrors: a transient DB failure must not kill the
// TestJanitorSurvivesErrors: a transient DB failure must not kill the
// janitor for the life of the process; it logs and tries again next tick.
func TestPruneAuthCodesSurvivesErrors(t *testing.T) {
func TestJanitorSurvivesErrors(t *testing.T) {
t.Parallel()
f := newFakePruner(errors.New("connection refused"))
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
go pruneAuthCodes(ctx, f, 2*time.Millisecond, discardLogger())
go runJanitor(ctx, 2*time.Millisecond, discardLogger(), janitorSweep{"test", f.PruneAuthCodes})
// Still sweeping well after the first failure.
waitForCalls(t, f, 4, 2*time.Second)
}
// TestAuthCodePruneIntervalOutlivesCodeTTL: the sweep only has to keep the table
// TestJanitorRunsEverySweep: the janitor drives several cleanups, so each one must
// run on every pass — and a sweep that fails must not stop the sweeps after it, or a
// single sick table would silently stall unrelated cleanup for the whole process
// lifetime.
func TestJanitorRunsEverySweep(t *testing.T) {
t.Parallel()
broken := newFakePruner(errors.New("relation does not exist"))
healthy := newFakePruner(nil)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
// Broken first, so the healthy one only runs if a failure doesn't abort the pass.
go runJanitor(ctx, 2*time.Millisecond, discardLogger(),
janitorSweep{"broken", broken.PruneAuthCodes},
janitorSweep{"healthy", healthy.PruneAuthCodes},
)
waitForCalls(t, broken, 2, 2*time.Second)
waitForCalls(t, healthy, 2, 2*time.Second)
}
// TestJanitorIntervalOutlivesCodeTTL: the sweep only has to keep the table
// small, but an interval shorter than the code TTL would mean pointless work, and a
// wildly long one would defeat the purpose. Pin it to a sane band so a future edit
// has to think about it.
func TestAuthCodePruneIntervalOutlivesCodeTTL(t *testing.T) {
func TestJanitorIntervalOutlivesCodeTTL(t *testing.T) {
t.Parallel()
if authCodePruneInterval < time.Minute {
t.Errorf("authCodePruneInterval = %v; shorter than the 60s code TTL means "+
"sweeping rows that were never expirable", authCodePruneInterval)
if janitorInterval < time.Minute {
t.Errorf("janitorInterval = %v; shorter than the 60s code TTL means "+
"sweeping rows that were never expirable", janitorInterval)
}
if authCodePruneInterval > time.Hour {
t.Errorf("authCodePruneInterval = %v; too long to keep the table small", authCodePruneInterval)
if janitorInterval > time.Hour {
t.Errorf("janitorInterval = %v; too long to keep the table small", janitorInterval)
}
}
+157
View File
@@ -0,0 +1,157 @@
package core
import (
"io"
"net/http"
"net/http/cookiejar"
"strings"
"testing"
"github.com/jleight/meshtender/internal/store"
)
// TestSharePageShowsInviteExpiry checks how the share page presents link expiry to
// the owner:
// - the intro copy states the shelf life, so an owner knows a link is time-boxed
// before they hand it out
// - a live link shows its expiry alongside a copy control
// - an expired link is badged Expired and its copy control is GONE — offering to
// copy a token that can no longer be redeemed just produces a confused recipient
//
// Companion to the store-level tests for audit finding S3.
func TestSharePageShowsInviteExpiry(t *testing.T) {
t.Parallel()
st, ctx, ts, h := splitServer(t)
jar, err := cookiejar.New(nil)
if err != nil {
t.Fatalf("cookiejar: %v", err)
}
owner := seedSession(t, ts, st, ctx, jar, "shareexpiry")
rep, err := st.CreateRepeater(ctx, &store.Repeater{
OwnerID: owner.ID, Name: "Rep", PublicKeyHex: strings.Repeat("b", 64),
RadioFreqHz: 1, RadioBwHz: 1, RadioSF: 11, RadioCR: 5,
})
if err != nil {
t.Fatalf("create repeater: %v", err)
}
cookies := jar.Cookies(mustURL(t, ts.URL))
sharePage := func() string {
t.Helper()
resp := do(t, ts, h.app, "/repeaters/"+rep.PublicID+"/share", cookies...)
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
t.Fatalf("share page = %d, want 200", resp.StatusCode)
}
body, err := io.ReadAll(resp.Body)
if err != nil {
t.Fatalf("read body: %v", err)
}
return string(body)
}
// A live link.
liveToken, err := st.CreateInvite(ctx, rep.ID, "live link", nil)
if err != nil {
t.Fatalf("create invite: %v", err)
}
page := sharePage()
if !strings.Contains(page, "expire 7 days") {
t.Error("share page doesn't state how long links last")
}
if !strings.Contains(page, liveToken) {
t.Error("live link's token isn't shown for copying")
}
if !strings.Contains(page, "Expires") {
t.Error("live link doesn't show its expiry date")
}
if strings.Contains(page, `data-testid="invite-expired"`) {
t.Error("a live link is badged Expired")
}
// Now age it out.
if _, err := st.Pool().Exec(ctx,
`UPDATE repeater_invites SET expires_at = now() - interval '1 minute' WHERE token = $1`,
liveToken); err != nil {
t.Fatalf("expire invite: %v", err)
}
page = sharePage()
if !strings.Contains(page, `data-testid="invite-expired"`) {
t.Error("expired link isn't badged Expired")
}
if strings.Contains(page, liveToken) {
t.Error("expired link still offers its token to copy — it can't be redeemed")
}
if !strings.Contains(page, "can no longer be accepted") {
t.Error("expired link doesn't explain that it's dead")
}
// The row must still be listed so the owner can tidy it up.
if !strings.Contains(page, `data-testid="invite-row"`) {
t.Error("expired link vanished from the list entirely")
}
}
// TestInvitePageForDeadLink covers what the *recipient* sees. Every "invalid" path
// renders the same page, and that page names InviteTTLDays — a render payload
// missing the key would put a literal "<no value>" in the sentence, which is why
// they all go through renderInviteInvalid.
//
// Also pins that the message stays vague about *why* the link is dead: someone
// holding a token shouldn't learn whether it never existed, was already claimed,
// was revoked, or simply lapsed.
func TestInvitePageForDeadLink(t *testing.T) {
t.Parallel()
st, ctx, ts, h := splitServer(t)
jar, err := cookiejar.New(nil)
if err != nil {
t.Fatalf("cookiejar: %v", err)
}
owner := seedSession(t, ts, st, ctx, jar, "deadlink")
rep, err := st.CreateRepeater(ctx, &store.Repeater{
// Distinctive so the leak check below can't match navbar chrome ("Repeaters").
OwnerID: owner.ID, Name: "Zarquon Ridge Relay", PublicKeyHex: strings.Repeat("d", 64),
RadioFreqHz: 1, RadioBwHz: 1, RadioSF: 11, RadioCR: 5,
})
if err != nil {
t.Fatalf("create repeater: %v", err)
}
expired, err := st.CreateInvite(ctx, rep.ID, "gone", nil)
if err != nil {
t.Fatalf("create invite: %v", err)
}
if _, err := st.Pool().Exec(ctx,
`UPDATE repeater_invites SET expires_at = now() - interval '1 minute' WHERE token = $1`,
expired); err != nil {
t.Fatalf("expire invite: %v", err)
}
// Both a never-existed token and a lapsed one must render the same way.
for _, tc := range []struct{ name, token string }{
{"expired token", expired},
{"unknown token", "totally-made-up-token"},
} {
resp := do(t, ts, h.app, "/invite/"+tc.token)
body, err := io.ReadAll(resp.Body)
resp.Body.Close()
if err != nil {
t.Fatalf("%s: read body: %v", tc.name, err)
}
page := string(body)
if !strings.Contains(page, "Invalid link") {
t.Errorf("%s: page doesn't show the invalid-link state", tc.name)
}
if strings.Contains(page, "<no value>") {
t.Errorf("%s: render payload is missing a key the template names", tc.name)
}
if !strings.Contains(page, "expire after 7 days") {
t.Errorf("%s: message doesn't mention the expiry window", tc.name)
}
if strings.Contains(page, rep.Name) {
t.Errorf("%s: leaks the repeater name for a link that can't be redeemed", tc.name)
}
}
}
+25 -4
View File
@@ -6,6 +6,7 @@ import (
"net/http"
"strconv"
"strings"
"time"
"github.com/go-chi/chi/v5"
@@ -13,6 +14,23 @@ import (
"github.com/jleight/meshtender/internal/web"
)
// inviteTTLDays is store.InviteTTL in whole days, for the owner-facing and
// recipient-facing copy. Derived from the store constant so the stated shelf life
// can't drift from the one actually enforced.
var inviteTTLDays = int(store.InviteTTL / (24 * time.Hour))
// renderInviteInvalid renders the dead-link page. It exists so every "invalid"
// path supplies InviteTTLDays — the template names it, and a map missing the key
// would render "<no value>" into the copy. The message deliberately doesn't say
// which reason applies (never existed / already claimed / revoked / lapsed), so a
// stranger holding a token learns nothing from it.
func (s *Handlers) renderInviteInvalid(w http.ResponseWriter, r *http.Request) {
s.Render(w, r, "invite.html", map[string]any{
"State": "invalid",
"InviteTTLDays": inviteTTLDays,
})
}
// pageShare renders the sharing page for a repeater the user owns: the current
// share link (if any) and the list of people who have accepted.
func (s *Handlers) pageShare(w http.ResponseWriter, r *http.Request) {
@@ -44,7 +62,10 @@ func (s *Handlers) pageShare(w http.ResponseWriter, r *http.Request) {
"Invites": invites,
"Orgs": orgs,
"BaseURL": s.absoluteURL(r, ""),
"Error": r.URL.Query().Get("error"),
// Stated in the copy so an owner knows a link is time-boxed before handing it
// out. Derived from the store's TTL so the two can't disagree.
"InviteTTLDays": inviteTTLDays,
"Error": r.URL.Query().Get("error"),
})
}
@@ -141,7 +162,7 @@ func (s *Handlers) pageInvite(w http.ResponseWriter, r *http.Request) {
// queryUserID drives the Shared flag; 0 when logged out is fine.
rep, err := s.Store.RepeaterByInviteToken(r.Context(), uid, token)
if errors.Is(err, store.ErrNotFound) {
s.Render(w, r, "invite.html", map[string]any{"State": "invalid"})
s.renderInviteInvalid(w, r)
return
}
if err != nil {
@@ -179,7 +200,7 @@ func (s *Handlers) handleAcceptInvite(w http.ResponseWriter, r *http.Request) {
rep, err := s.Store.RepeaterByInviteToken(r.Context(), uid, token)
if errors.Is(err, store.ErrNotFound) {
s.Render(w, r, "invite.html", map[string]any{"State": "invalid"})
s.renderInviteInvalid(w, r)
return
}
if err != nil {
@@ -201,7 +222,7 @@ func (s *Handlers) handleAcceptInvite(w http.ResponseWriter, r *http.Request) {
// failure can never delete the link without granting access. A row lock inside
// makes it the single-use gate against concurrent accepts.
if _, err := s.Store.AcceptInvite(r.Context(), token, uid); errors.Is(err, store.ErrNotFound) {
s.Render(w, r, "invite.html", map[string]any{"State": "invalid"})
s.renderInviteInvalid(w, r)
return
} else if err != nil {
s.ServerError(w, r, "could not accept invite", err)
+5 -1
View File
@@ -11,7 +11,11 @@
<div class="card-body">
{{if eq .State "invalid"}}
<h2 class="card-title">Invalid link</h2>
<p class="text-secondary">This share link is no longer valid — it's single-use and may have already been used or revoked. Ask the owner for a new one.</p>
{{/* Deliberately doesn't say WHICH reason: whoever holds this token
shouldn't learn whether it ever existed, was already claimed, or just
lapsed. Expiry is listed as a possibility so the message stays
accurate now that links have a shelf life. */}}
<p class="text-secondary">This share link is no longer valid — links are single-use and expire after {{.InviteTTLDays}} days, so it may have already been used, been revoked, or timed out. Ask the owner for a new one.</p>
<a class="back-link" href="/">{{template "icon-arrow-left" "me-1"}}Go to dashboard</a>
{{else if eq .State "auth_required"}}
+20 -3
View File
@@ -29,24 +29,41 @@
<p class="text-secondary mb-0">
Invite specific people with single-use links: the recipient signs in and accepts to control this
repeater from their own browser-connected KISS modem. Each link works once and disappears when it's
used — mint one per person, label it, and choose what they can do.
used — create one per person, label it, and choose what they can do. Links also
<strong>expire {{.InviteTTLDays}} days</strong> after you create them, so an unused one can't be
redeemed by whoever finds it later.
</p>
{{if .Invites}}
<div class="list-group list-group-flush mt-3">
{{range .Invites}}
<div class="list-group-item px-0">
<div class="list-group-item px-0" data-testid="invite-row">
<div class="d-flex align-items-center gap-2 mb-1">
<span class="fw-bold">{{if .Description}}{{.Description}}{{else}}(no label){{end}}</span>
{{if .Expired}}
<span class="badge bg-red-lt" data-testid="invite-expired">Expired</span>
{{else}}
<span class="badge bg-yellow-lt">Unused</span>
{{end}}
</div>
{{if .Expired}}
{{/* No copy control for a dead link — handing out a token that can't be
redeemed just produces a confused recipient. */}}
<p class="text-secondary mb-2">
Expired {{ ts .ExpiresAt "datetime" }} and can no longer be accepted. Create a new link instead.
</p>
{{else}}
<div class="input-group mb-2">
<code class="form-control font-monospace" id="link-{{.ID}}">{{$.BaseURL}}/invite/{{.Token}}</code>
<button type="button" class="btn" data-copy-target="#link-{{.ID}}">{{template "icon-copy" ""}}Copy</button>
</div>
<p class="text-secondary small mb-2">Expires {{ ts .ExpiresAt "datetime" }}</p>
{{end}}
<form method="post" action="/repeaters/{{$.Repeater.PublicID}}/share/link/delete" class="m-0">
<input type="hidden" name="invite_id" value="{{.ID}}">
<button type="submit" class="btn btn-sm btn-ghost-danger">Revoke link</button>
<button type="submit" class="btn btn-sm btn-ghost-danger">
{{if .Expired}}Remove{{else}}Revoke link{{end}}
</button>
</form>
</div>
{{end}}
+95
View File
@@ -0,0 +1,95 @@
//go:build browser
package e2e
import (
"os"
"strings"
"testing"
cdplog "github.com/chromedp/cdproto/log"
"github.com/chromedp/cdproto/network"
"github.com/chromedp/chromedp"
"github.com/jleight/meshtender/internal/store"
)
// TestE2EInviteExpiryBadges renders the share page with one live and one expired
// share link and checks what an owner actually sees: the live link keeps its copy
// control, the expired one is badged and loses it, and the page runs clean under the
// CSP.
//
// Covers the presentation half of audit finding S3 (share links used to never
// expire); the store-level rules are pinned in internal/store.
func TestE2EInviteExpiryBadges(t *testing.T) {
srv := newE2EServer(t)
user, cookie := srv.login(t, "e2einviteexpiry")
rep, err := srv.store.CreateRepeater(srv.ctx, &store.Repeater{
OwnerID: user.ID, Name: "Rep", PublicKeyHex: strings.Repeat("c", 64),
RadioFreqHz: 1, RadioBwHz: 1, RadioSF: 11, RadioCR: 5,
})
if err != nil {
t.Fatalf("create repeater: %v", err)
}
if _, err := srv.store.CreateInvite(srv.ctx, rep.ID, "still good", nil); err != nil {
t.Fatalf("create live invite: %v", err)
}
staleToken, err := srv.store.CreateInvite(srv.ctx, rep.ID, "long forgotten", nil)
if err != nil {
t.Fatalf("create stale invite: %v", err)
}
if _, err := srv.store.Pool().Exec(srv.ctx,
`UPDATE repeater_invites SET expires_at = now() - interval '1 minute' WHERE token = $1`,
staleToken); err != nil {
t.Fatalf("expire invite: %v", err)
}
bctx, cancel, watch := startBrowser(t)
defer cancel()
var rows, expiredBadges, copyButtons int
var showsStaleToken bool
var shot []byte
if err := chromedp.Run(bctx,
network.Enable(),
cdplog.Enable(),
setSessionCookie(cookie),
chromedp.Navigate(srv.appURL+"/repeaters/"+rep.PublicID+"/share"),
chromedp.WaitVisible(`[data-testid="invite-row"]`, chromedp.ByQuery),
chromedp.Evaluate(`document.querySelectorAll('[data-testid="invite-row"]').length`, &rows),
chromedp.Evaluate(`document.querySelectorAll('[data-testid="invite-expired"]').length`, &expiredBadges),
chromedp.Evaluate(`document.querySelectorAll('[data-copy-target]').length`, &copyButtons),
chromedp.Evaluate(`document.body.innerText.includes(`+jsString(staleToken)+`)`, &showsStaleToken),
chromedp.FullScreenshot(&shot, 80),
); err != nil {
t.Fatalf("share page: %v", err)
}
watch.assertClean(t)
if rows != 2 {
t.Errorf("invite rows = %d, want 2 (both links listed so the owner can tidy up)", rows)
}
if expiredBadges != 1 {
t.Errorf("expired badges = %d, want exactly 1", expiredBadges)
}
if copyButtons != 1 {
t.Errorf("copy controls = %d, want 1 — only the live link should be copyable", copyButtons)
}
if showsStaleToken {
t.Error("the expired link's token is still rendered on the page")
}
// Keep the render for eyeballing when E2E_SHOT_DIR is set; skipped in CI.
if dir := os.Getenv("E2E_SHOT_DIR"); dir != "" && len(shot) > 0 {
if err := os.WriteFile(dir+"/invite-expiry.png", shot, 0o644); err != nil {
t.Logf("write screenshot: %v", err)
}
}
}
// jsString quotes s for embedding in a JS expression.
func jsString(s string) string {
return `"` + strings.ReplaceAll(s, `"`, `\"`) + `"`
}
+264
View File
@@ -0,0 +1,264 @@
package store
import (
"context"
"errors"
"strings"
"testing"
"time"
)
// inviteFixture builds an owner + repeater and returns them, for the expiry tests.
func inviteFixture(t *testing.T, st *Store, ctx context.Context, owner string) (*User, *Repeater) {
t.Helper()
u, err := st.CreateUser(ctx, owner, "")
if err != nil {
t.Fatalf("create owner: %v", err)
}
rep, err := st.CreateRepeater(ctx, &Repeater{
OwnerID: u.ID, Name: "Rep", PublicKeyHex: strings.Repeat("a", 64),
RadioFreqHz: 1, RadioBwHz: 1, RadioSF: 11, RadioCR: 5,
})
if err != nil {
t.Fatalf("create repeater: %v", err)
}
return u, rep
}
// expireInvite backdates a link's expiry by a minute so it reads as just-lapsed,
// without waiting out the real TTL.
func expireInvite(t *testing.T, st *Store, ctx context.Context, token string) {
t.Helper()
setInviteExpiry(t, st, ctx, token, -time.Minute)
}
// setInviteExpiry moves a link's expiry by offset relative to now (negative = in the
// past), for exercising the prune grace window without waiting weeks.
func setInviteExpiry(t *testing.T, st *Store, ctx context.Context, token string, offset time.Duration) {
t.Helper()
if _, err := st.pool.Exec(ctx,
`UPDATE repeater_invites SET expires_at = now() + $2 WHERE token = $1`,
token, offset); err != nil {
t.Fatalf("set invite expiry: %v", err)
}
}
// TestCreateInviteSetsExpiry: a new link gets a stored expiry roughly InviteTTL out.
// Storing it (rather than deriving it from created_at at query time) is what makes a
// later change to InviteTTL affect only new links — an outstanding link keeps the
// expiry it was minted with, so lengthening the constant can't revive dead links.
func TestCreateInviteSetsExpiry(t *testing.T) {
t.Parallel()
st, ctx := orgTestStore(t)
_, rep := inviteFixture(t, st, ctx, "expiry-owner")
before := time.Now()
token, err := st.CreateInvite(ctx, rep.ID, "join", nil)
if err != nil {
t.Fatalf("create invite: %v", err)
}
invites, err := st.ListInvites(ctx, rep.ID)
if err != nil || len(invites) != 1 {
t.Fatalf("ListInvites = %d, %v; want 1", len(invites), err)
}
inv := invites[0]
if inv.Token != token {
t.Fatalf("token = %q, want %q", inv.Token, token)
}
if inv.ExpiresAt.IsZero() {
t.Fatal("ExpiresAt is zero — a link must carry its own expiry")
}
// Allow a generous window for clock skew between the test and the database.
want := before.Add(InviteTTL)
if delta := inv.ExpiresAt.Sub(want); delta < -time.Minute || delta > time.Minute {
t.Errorf("ExpiresAt = %v, want ~%v (InviteTTL out); off by %v", inv.ExpiresAt, want, delta)
}
if inv.Expired() {
t.Error("a freshly minted link reports Expired()")
}
}
// TestExpiredInviteCannotBeRedeemed is the finding itself: before this change a
// share link stayed valid forever, so one pasted into a chat channel years earlier
// still granted repeater access to whoever found it first.
func TestExpiredInviteCannotBeRedeemed(t *testing.T) {
t.Parallel()
st, ctx := orgTestStore(t)
_, rep := inviteFixture(t, st, ctx, "stale-owner")
token, err := st.CreateInvite(ctx, rep.ID, "join", nil)
if err != nil {
t.Fatalf("create invite: %v", err)
}
invitee, err := st.CreateUser(ctx, "stale-invitee", "")
if err != nil {
t.Fatalf("create invitee: %v", err)
}
expireInvite(t, st, ctx, token)
// The redemption path must refuse it.
if _, err := st.AcceptInvite(ctx, token, invitee.ID); !errors.Is(err, ErrNotFound) {
t.Fatalf("AcceptInvite(expired) err = %v, want ErrNotFound", err)
}
// And no share may have been granted as a side effect.
shares, err := st.ListShares(ctx, rep.ID)
if err != nil {
t.Fatalf("list shares: %v", err)
}
for _, sh := range shares {
if sh.UserID == invitee.ID {
t.Fatal("an expired link granted access")
}
}
// The landing page that previews the link must not resolve it either — otherwise
// a recipient sees a working invite page whose accept then fails.
if _, err := st.RepeaterByInviteToken(ctx, invitee.ID, token); !errors.Is(err, ErrNotFound) {
t.Fatalf("RepeaterByInviteToken(expired) err = %v, want ErrNotFound", err)
}
}
// TestUnexpiredInviteStillWorks guards the obvious over-correction: the expiry check
// must not break links that are still inside their window.
func TestUnexpiredInviteStillWorks(t *testing.T) {
t.Parallel()
st, ctx := orgTestStore(t)
_, rep := inviteFixture(t, st, ctx, "live-owner")
token, err := st.CreateInvite(ctx, rep.ID, "join", nil)
if err != nil {
t.Fatalf("create invite: %v", err)
}
invitee, err := st.CreateUser(ctx, "live-invitee", "")
if err != nil {
t.Fatalf("create invitee: %v", err)
}
if _, err := st.RepeaterByInviteToken(ctx, invitee.ID, token); err != nil {
t.Fatalf("RepeaterByInviteToken(live) = %v, want success", err)
}
added, err := st.AcceptInvite(ctx, token, invitee.ID)
if err != nil || !added {
t.Fatalf("AcceptInvite(live) = (%v, %v), want (true, nil)", added, err)
}
}
// TestListInvitesShowsExpired: an owner needs to see that a link is spent — hiding
// expired rows would leave them wondering where the link went, with no way to tidy
// up. The store returns them; Expired() is how the UI marks them.
func TestListInvitesShowsExpired(t *testing.T) {
t.Parallel()
st, ctx := orgTestStore(t)
_, rep := inviteFixture(t, st, ctx, "list-owner")
token, err := st.CreateInvite(ctx, rep.ID, "join", nil)
if err != nil {
t.Fatalf("create invite: %v", err)
}
expireInvite(t, st, ctx, token)
invites, err := st.ListInvites(ctx, rep.ID)
if err != nil || len(invites) != 1 {
t.Fatalf("ListInvites = %d, %v; want the expired link still listed", len(invites), err)
}
if !invites[0].Expired() {
t.Errorf("Expired() = false for a link whose expires_at is %v", invites[0].ExpiresAt)
}
}
// TestPruneInvitesKeepsRecentlyExpired is the reason the sweep has a grace period:
// the share page lists expired links with an Expired badge and a Remove button, and
// the janitor runs every few minutes. Deleting on expiry would make that state
// unobservable and the button unreachable, so a just-lapsed link must survive a
// sweep. Only links long past their expiry get collected.
func TestPruneInvitesKeepsRecentlyExpired(t *testing.T) {
t.Parallel()
st, ctx := orgTestStore(t)
_, rep := inviteFixture(t, st, ctx, "prune-owner")
live, err := st.CreateInvite(ctx, rep.ID, "live", nil)
if err != nil {
t.Fatalf("create invite: %v", err)
}
justLapsed, err := st.CreateInvite(ctx, rep.ID, "just lapsed", nil)
if err != nil {
t.Fatalf("create invite: %v", err)
}
ancient, err := st.CreateInvite(ctx, rep.ID, "ancient", nil)
if err != nil {
t.Fatalf("create invite: %v", err)
}
// Lapsed a minute ago: the owner should still see it.
expireInvite(t, st, ctx, justLapsed)
// Lapsed well beyond the grace window: collectable.
setInviteExpiry(t, st, ctx, ancient, -(ExpiredInviteGrace + 24*time.Hour))
n, err := st.PruneInvites(ctx)
if err != nil {
t.Fatalf("prune: %v", err)
}
if n != 1 {
t.Errorf("prune removed %d, want 1 (only the long-expired link)", n)
}
invites, err := st.ListInvites(ctx, rep.ID)
if err != nil {
t.Fatalf("list invites: %v", err)
}
kept := map[string]bool{}
for _, inv := range invites {
kept[inv.Token] = true
}
if !kept[live] {
t.Error("the live link was pruned")
}
if !kept[justLapsed] {
t.Error("a just-expired link was pruned — the owner never gets to see or remove it")
}
if kept[ancient] {
t.Error("a link long past the grace window survived the sweep")
}
// Idempotent: nothing else is collectable yet.
if n, err := st.PruneInvites(ctx); err != nil || n != 0 {
t.Fatalf("second prune = (%d, %v), want (0, nil)", n, err)
}
}
// TestPruneInvitesGraceBoundary pins the edge: a link that lapsed just inside the
// window stays, one just outside goes.
func TestPruneInvitesGraceBoundary(t *testing.T) {
t.Parallel()
st, ctx := orgTestStore(t)
_, rep := inviteFixture(t, st, ctx, "boundary-owner")
inside, err := st.CreateInvite(ctx, rep.ID, "inside grace", nil)
if err != nil {
t.Fatalf("create invite: %v", err)
}
outside, err := st.CreateInvite(ctx, rep.ID, "outside grace", nil)
if err != nil {
t.Fatalf("create invite: %v", err)
}
// An hour either side of the boundary, so clock skew can't decide the outcome.
setInviteExpiry(t, st, ctx, inside, -(ExpiredInviteGrace - time.Hour))
setInviteExpiry(t, st, ctx, outside, -(ExpiredInviteGrace + time.Hour))
if _, err := st.PruneInvites(ctx); err != nil {
t.Fatalf("prune: %v", err)
}
invites, err := st.ListInvites(ctx, rep.ID)
if err != nil {
t.Fatalf("list invites: %v", err)
}
kept := map[string]bool{}
for _, inv := range invites {
kept[inv.Token] = true
}
if !kept[inside] {
t.Errorf("a link %v past expiry was pruned; grace is %v", ExpiredInviteGrace-time.Hour, ExpiredInviteGrace)
}
if kept[outside] {
t.Errorf("a link %v past expiry survived; grace is %v", ExpiredInviteGrace+time.Hour, ExpiredInviteGrace)
}
}
@@ -0,0 +1,26 @@
-- +goose Up
-- Share links used to be valid forever until redeemed or deleted, so a link pasted
-- into a chat channel or an email years ago still granted repeater access to
-- whoever found it first. Give every link a hard expiry.
--
-- The expiry is STORED per row rather than computed as created_at + a constant, so
-- each link carries the promise made when it was minted. With a computed expiry,
-- editing the constant would retroactively redefine every outstanding link — and
-- lengthening it would resurrect links that had already expired and that their
-- owners had every reason to consider dead. Same instinct as the append-only
-- org_permission_versions table: a commitment made at creation time stays fixed.
ALTER TABLE repeater_invites ADD COLUMN expires_at TIMESTAMPTZ;
-- Backfill outstanding links from their own creation time, not from now(): links
-- older than the window are exactly the stale ones this change exists to kill, so
-- they expire immediately rather than getting a fresh lease.
UPDATE repeater_invites SET expires_at = created_at + interval '7 days';
ALTER TABLE repeater_invites ALTER COLUMN expires_at SET NOT NULL;
-- Supports the expiry predicate on every token lookup, and the pruning sweep.
CREATE INDEX repeater_invites_expires_at_idx ON repeater_invites (expires_at);
-- +goose Down
DROP INDEX repeater_invites_expires_at_idx;
ALTER TABLE repeater_invites DROP COLUMN expires_at;
+68 -12
View File
@@ -177,13 +177,30 @@ type Invite struct {
Token string
Description string
CreatedAt time.Time
ExpiresAt time.Time
}
// Expired reports whether the link is past its expiry and can no longer be
// redeemed. The store won't hand it out either way; this is for presentation, so
// the owner can see a spent link before deleting it.
func (i Invite) Expired() bool { return !i.ExpiresAt.IsZero() && time.Now().After(i.ExpiresAt) }
// InviteTTL is how long a new share link stays redeemable. Share links are
// credentials that travel through chat and email, so they get a hard shelf life
// rather than living until someone remembers to revoke them.
//
// It's applied when a link is minted and stored on the row (see migration 0040), so
// changing this constant affects only future links — outstanding ones keep the
// expiry they were created with. That's deliberate: a computed expiry would mean
// lengthening this value silently resurrects links that already died.
const InviteTTL = 7 * 24 * time.Hour
// CreateInvite mints a new single-use share link for a repeater, returning its
// token. description is an owner-facing label (may be empty). commandIDs is the
// initial command set the accepter is granted on redemption (may be empty — the
// owner can grant more afterwards); it and the invite row are written together so
// the link never exists without its recorded grant.
// the link never exists without its recorded grant. The link expires after
// InviteTTL.
func (s *Store) CreateInvite(ctx context.Context, repeaterID int64, description string, commandIDs []int64) (string, error) {
token, err := randomToken()
if err != nil {
@@ -192,8 +209,9 @@ func (s *Store) CreateInvite(ctx context.Context, repeaterID int64, description
err = s.inTx(ctx, func(tx pgx.Tx) error {
var inviteID int64
if err := tx.QueryRow(ctx,
`INSERT INTO repeater_invites (repeater_id, token, description) VALUES ($1, $2, $3) RETURNING id`,
repeaterID, token, description).Scan(&inviteID); err != nil {
`INSERT INTO repeater_invites (repeater_id, token, description, expires_at)
VALUES ($1, $2, $3, now() + $4) RETURNING id`,
repeaterID, token, description, InviteTTL).Scan(&inviteID); err != nil {
return fmt.Errorf("create invite: %w", err)
}
for _, cid := range commandIDs {
@@ -212,9 +230,12 @@ func (s *Store) CreateInvite(ctx context.Context, repeaterID int64, description
}
// ListInvites returns a repeater's pending (unredeemed) share links, newest first.
// Expired links are included — the owner should see that a link is spent, and be
// able to delete it — so callers must present Invite.Expired() rather than implying
// every row is live.
func (s *Store) ListInvites(ctx context.Context, repeaterID int64) ([]Invite, error) {
rows, err := s.pool.Query(ctx, `
SELECT id, token, description, created_at
SELECT id, token, description, created_at, expires_at
FROM repeater_invites
WHERE repeater_id = $1
ORDER BY created_at DESC`, repeaterID)
@@ -223,11 +244,41 @@ func (s *Store) ListInvites(ctx context.Context, repeaterID int64) ([]Invite, er
}
return collectRows(rows, func(r pgx.Row) (Invite, error) {
var inv Invite
err := r.Scan(&inv.ID, &inv.Token, &inv.Description, &inv.CreatedAt)
err := r.Scan(&inv.ID, &inv.Token, &inv.Description, &inv.CreatedAt, &inv.ExpiresAt)
return inv, err
})
}
// ExpiredInviteGrace is how long a lapsed share link stays in the table before the
// janitor removes it. It exists so the owner can actually SEE that a link went stale
// — the share page lists expired links with an Expired badge and a Remove button,
// and sweeping them immediately would make that state unobservable (the janitor runs
// every few minutes) and the button unreachable. Deleting them silently is the worse
// UX for something the owner handed to a person: the link just vanishes with no
// explanation of why it stopped working.
//
// Holding the row is not holding a credential: every lookup filters on expires_at,
// so a lapsed link can't be redeemed, and the share page stops rendering its token
// once it expires. Growth is bounded by a month of link creation.
const ExpiredInviteGrace = 30 * 24 * time.Hour
// PruneInvites deletes share links that lapsed more than ExpiredInviteGrace ago,
// returning how many went. Redemption deletes a link and an owner can delete one by
// hand, but an unredeemed link that simply timed out would otherwise sit in the table
// forever. Index-assisted by repeater_invites_expires_at_idx.
func (s *Store) PruneInvites(ctx context.Context) (int64, error) {
// $1 needs the explicit ::interval cast. Without it `now() - $1` is ambiguous —
// Postgres can read it as timestamptz - timestamptz → interval, which then makes
// the comparison `timestamptz < interval` and fails at plan time. (The `now() + $n`
// expressions elsewhere are fine because addition has only one candidate.)
tag, err := s.pool.Exec(ctx,
`DELETE FROM repeater_invites WHERE expires_at < now() - $1::interval`, ExpiredInviteGrace)
if err != nil {
return 0, fmt.Errorf("prune invites: %w", err)
}
return tag.RowsAffected(), nil
}
// DeleteInvite removes an invite by id, scoped to its repeater (so an owner can
// only delete links for their own repeater). Idempotent.
func (s *Store) DeleteInvite(ctx context.Context, repeaterID, inviteID int64) error {
@@ -240,11 +291,13 @@ func (s *Store) DeleteInvite(ctx context.Context, repeaterID, inviteID int64) er
}
// RepeaterByInviteToken resolves a valid share-link token to its repeater, or
// ErrNotFound if the token is unknown, revoked, or already redeemed (redemption
// deletes the link). queryUserID only populates the Shared flag and owner display.
// ErrNotFound if the token is unknown, revoked, expired, or already redeemed
// (redemption deletes the link). queryUserID only populates the Shared flag and
// owner display.
func (s *Store) RepeaterByInviteToken(ctx context.Context, queryUserID int64, token string) (*Repeater, error) {
row := s.pool.QueryRow(ctx, repeaterSelect+`
WHERE r.id = (SELECT repeater_id FROM repeater_invites WHERE token = $2)`,
WHERE r.id = (SELECT repeater_id FROM repeater_invites
WHERE token = $2 AND expires_at > now())`,
queryUserID, token)
r, err := scanRepeater(row)
if err != nil {
@@ -261,14 +314,17 @@ func (s *Store) RepeaterByInviteToken(ctx context.Context, queryUserID int64, to
// without deleting the link, leaving it redeemable again).
//
// It returns whether a new share was created (false if the user already had one)
// and ErrNotFound if the token is unknown, revoked, or already redeemed. The row
// lock is the single-use gate: concurrent accepts serialize on it, and the winner
// deletes the row, so losers find nothing.
// and ErrNotFound if the token is unknown, revoked, expired, or already redeemed.
// The row lock is the single-use gate: concurrent accepts serialize on it, and the
// winner deletes the row, so losers find nothing. The expiry is checked inside that
// same locked read, so a link can't be redeemed by a request that started before it
// lapsed.
func (s *Store) AcceptInvite(ctx context.Context, token string, userID int64) (added bool, err error) {
err = s.inTx(ctx, func(tx pgx.Tx) error {
var inviteID, repeaterID int64
if err := tx.QueryRow(ctx,
`SELECT id, repeater_id FROM repeater_invites WHERE token = $1 FOR UPDATE`,
`SELECT id, repeater_id FROM repeater_invites
WHERE token = $1 AND expires_at > now() FOR UPDATE`,
token).Scan(&inviteID, &repeaterID); err != nil {
return notFoundOr(err, "consume invite")
}