feat: let a share's expiry be changed after it is published - #31
Draft
lr00rl wants to merge 1 commit into
Draft
Conversation
Expiry was already modelled and already enforced — `resolveShare` refuses an expired share exactly the way it refuses a wrong token, so nothing about it leaks. It could only be set at creation, though, so extending a share meant deleting it and handing out a new link: the one thing a share exists to avoid. PATCH on the share item changes expiry, default format and enabled without touching the token. Rotation stays a separate action precisely because it DOES invalidate the URL, and conflating the two would make an ordinary edit break every client holding the link. Every field is a pointer, and clearing takes an explicit `clear_expiry` flag. "Not supplied" and "set to nothing" are different requests; treating them alike would make an edit that only changed the format silently remove the expiry. An expiry already in the past is refused at the edit rather than accepted, because the endpoint answers a dead share identically to a wrong token — an operator who fat-fingered the year would get no feedback at all. The cached body is dropped on every change. It was rendered under the old settings, and a share that has just been disabled must stop answering now rather than whenever its cache entry happens to age out. PATCH needs no CSRF work: `unsafeMethod` is written as a denylist of the safe methods rather than a list of the unsafe ones, so it is covered by construction. A test pins that, because rewriting it as an enumeration would quietly expose whatever someone forgot to add.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Expiry was already modelled (
SubscriptionShare.ExpiresAt) and already enforced—
resolveSharerefuses an expired share exactly the way it refuses a wrongtoken, so nothing about it leaks, including that it ever existed.
It could only be set at creation, so extending a share meant deleting it and
handing out a new link: the one thing a share exists to avoid.
What this adds
PATCH /api/subscription-shares/{id}— expiry, default format, enabled. Thetoken is untouched. Rotation stays a separate action precisely because it does
invalidate the URL; conflating the two would make an ordinary edit break every
client holding the link.
Two decisions worth reading
Pointers, plus an explicit
clear_expiry. "Not supplied" and "set tonothing" are different requests. Without the flag, an edit that only changed the
format would silently remove the expiry.
A past expiry is refused at the edit. The public endpoint answers a dead
share identically to a wrong token, so an operator who fat-fingered the year
would get no feedback at all — the error has to happen here or nowhere.
Cache
Dropped on every change. The cached body was rendered under the old settings,
and a share that has just been disabled must stop answering now rather than
whenever its entry ages out.
CSRF
No work needed, and that is worth stating rather than assuming:
unsafeMethodis a denylist of the safe methods rather than a list of the unsafe ones, so
PATCH is covered by construction. A test pins it, because rewriting it as an
enumeration would quietly expose whatever someone forgot to add.
Verification
go test -race ./internal/server/... ./internal/store/...— exit 0(server 626s, store 14s). Nine new tests cover set / leave-alone / clear /
past-refused / both-at-once-refused / disable-stops-resolving /
expired-stops-resolving / audited-without-the-token / unknown-format-refused.
Dashboard side: LatticeNet/lattice-dashboard
feat/principal-share-expiry.