Keep showing a guest link's URL so it can be copied again

The URL was shown once, in a box under the create button, and then gone: only a hash of the token was stored, so the app genuinely could not produce it a second time. Lose the message you sent the sitter and the only way back was to mint a new link — which strands whoever is already holding the old one. Settings now lists every live link with its URL and a Copy button, so re-sending one is just copying it again. That means keeping the token rather than only its hash, and it is worth being plain about the trade. It is not the trade you would make for a password, which the user has probably reused, or a session token, which grants everything indefinitely. A guest link grants a strict subset of what the same database already holds in plaintext, expires on a date the owner picked, and can be revoked in one tap — so an attacker who can read puppy.db gains very little by also being able to open it as a guest. The lookup column stays a hash and remains the key redeem matches against; the secret sits in a new column beside it, which also keeps the migration additive. Links created before this have an empty secret. They keep working and stay revocable — the migration touches nothing but the new column — and the list says why their URL is missing rather than rendering a broken one. The two tests that asserted the old contract now assert the new one: a listing hands back a secret that really opens the link, and the lookup column is still a hash. Added one for the legacy row, since "still works, just cannot be shown" is the part a future change is most likely to break quietly.
2026-09-07 19:25:20 +00:00
parent babed44c25
commit 66f89b35a9
8 changed files with 183 additions and 76 deletions
@@ -554,11 +554,17 @@ func (a *Auth) handleDeleteAccount(w http.ResponseWriter, r *http.Request) {
 // link it came from — so every data path downstream (sync, photos, config) keeps
 // working untouched, and only the capability checks differ by role.
 //
-// The raw token is shown exactly once, at creation. Only its hash is stored,
-// the same way session tokens are, so a leaked database yields no usable links.
+// The token is kept, not just its hash, so Settings can show the URL again
+// whenever the owner wants to re-send it. That is a deliberate trade the way it
+// would not be for a password or a session token: a guest link grants a subset
+// of what the same database already holds in plaintext, so whoever can read
+// puppy.db gains very little from it, and the link expires and can be revoked
+// besides. The `token` column stays a hash and remains the lookup key; `secret`
+// is the copy handed back to the owner.

-// ShareLink is the public shape of a guest link. Token is set only on the
-// response to the call that created it, and never stored in the clear.
+// ShareLink is the public shape of a guest link. Token carries the URL's secret
+// and comes back on every listing, so the owner can copy the link again rather
+// than having one chance at it when it is created.
 type ShareLink struct {
 	ID       string `json:"id"`
 	Label    string `json:"label"`
@@ -585,20 +591,22 @@ func (a *Auth) createShare(userID, label string, expires int64) (ShareLink, erro
 		Token:   token,
 	}
 	_, err := a.db.Exec(
-		`INSERT INTO share_links (id, user_id, token, label, created, expires) VALUES (?, ?, ?, ?, ?, ?)`,
-		link.ID, userID, hashToken(token), link.Label, link.Created, link.Expires)
+		`INSERT INTO share_links (id, user_id, token, secret, label, created, expires) VALUES (?, ?, ?, ?, ?, ?, ?)`,
+		link.ID, userID, hashToken(token), token, link.Label, link.Created, link.Expires)
 	if err != nil {
 		return ShareLink{}, err
 	}
 	return link, nil
 }

-// listShares returns the account's links that are still usable. Revoked and
+// listShares returns the account's links that are still usable, each with its
+// URL secret so Settings can offer the link for copying at any time. Revoked and
 // lapsed ones are of no interest to the UI — the point of the list is "who can
-// get in right now".
+// get in right now". A link created before secrets were kept comes back with an
+// empty Token; the UI says so rather than showing a broken URL.
 func (a *Auth) listShares(userID string) ([]ShareLink, error) {
 	rows, err := a.db.Query(`
-		SELECT id, label, created, expires, last_used
+		SELECT id, label, created, expires, last_used, secret
 		  FROM share_links
 		 WHERE user_id = ? AND revoked = 0 AND expires > ?
 		 ORDER BY created DESC`, userID, time.Now().UnixMilli())
@@ -609,7 +617,7 @@ func (a *Auth) listShares(userID string) ([]ShareLink, error) {
 	out := make([]ShareLink, 0)
 	for rows.Next() {
 		var l ShareLink
-		if err := rows.Scan(&l.ID, &l.Label, &l.Created, &l.Expires, &l.LastUsed); err != nil {
+		if err := rows.Scan(&l.ID, &l.Label, &l.Created, &l.Expires, &l.LastUsed, &l.Token); err != nil {
 			return nil, err
 		}
 		out = append(out, l)
@@ -230,13 +230,36 @@ func TestListSharesHidesRevokedAndExpired(t *testing.T) {
 	if len(links) != 1 || links[0].ID != live.ID {
 		t.Fatalf("listed %d link(s), want only the live one", len(links))
 	}
-	if links[0].Token != "" {
-		t.Error("listing leaked a raw token")
+}
+
+// Settings shows every live link's URL so it can be re-sent, which means a
+// listing has to carry the same secret the link was created with — and that
+// secret has to still work.
+func TestListSharesReturnsAWorkingURL(t *testing.T) {
+	a := testAuth(t)
+	ownerID := testOwner(t, a)
+	link, err := a.createShare(ownerID, "Anna", hoursAhead(24))
+	if err != nil {
+		t.Fatalf("create share: %v", err)
+	}
+	links, err := a.listShares(ownerID)
+	if err != nil {
+		t.Fatalf("list: %v", err)
+	}
+	if len(links) != 1 {
+		t.Fatalf("listed %d link(s), want 1", len(links))
+	}
+	if links[0].Token != link.Token {
+		t.Fatalf("listing returned %q, want the issued secret %q", links[0].Token, link.Token)
+	}
+	if _, _, ok := a.redeemShare(links[0].Token); !ok {
+		t.Error("the secret handed back by the listing does not open the link")
 	}
 }

-// The raw token is only ever handed back once, at creation.
-func TestOnlyTheTokenHashIsStored(t *testing.T) {
+// The lookup column stays a hash even though the secret is kept beside it, so
+// the token in a URL is never what is matched against directly.
+func TestLookupIsStillByHash(t *testing.T) {
 	a := testAuth(t)
 	ownerID := testOwner(t, a)
 	link, err := a.createShare(ownerID, "Anna", hoursAhead(24))
@@ -247,11 +270,36 @@ func TestOnlyTheTokenHashIsStored(t *testing.T) {
 	if err := a.db.QueryRow(`SELECT token FROM share_links WHERE id = ?`, link.ID).Scan(&stored); err != nil {
 		t.Fatalf("read back: %v", err)
 	}
-	if stored == link.Token {
-		t.Error("the raw token is stored in the clear")
-	}
 	if stored != hashToken(link.Token) {
-		t.Error("stored token is not the hash of the issued one")
+		t.Error("the lookup column is not the hash of the issued token")
+	}
+}
+
+// A link made before secrets were kept has no URL to show. It must still work
+// and still be revocable — only the copy-again affordance is unavailable.
+func TestLinkWithoutAStoredSecretStillWorks(t *testing.T) {
+	a := testAuth(t)
+	ownerID := testOwner(t, a)
+	link, err := a.createShare(ownerID, "Legacy", hoursAhead(24))
+	if err != nil {
+		t.Fatalf("create share: %v", err)
+	}
+	// What the migration leaves behind for a pre-existing row.
+	if _, err := a.db.Exec(`UPDATE share_links SET secret = '' WHERE id = ?`, link.ID); err != nil {
+		t.Fatalf("clear secret: %v", err)
+	}
+	links, err := a.listShares(ownerID)
+	if err != nil {
+		t.Fatalf("list: %v", err)
+	}
+	if len(links) != 1 || links[0].Token != "" {
+		t.Fatalf("want the link listed with an empty token, got %+v", links)
+	}
+	if _, _, ok := a.redeemShare(link.Token); !ok {
+		t.Error("a link whose secret was never stored stopped working")
+	}
+	if err := a.revokeShare(ownerID, link.ID); err != nil {
+		t.Errorf("could not revoke it: %v", err)
 	}
 }

@@ -377,6 +377,7 @@ func openDB(path string) (*sql.DB, error) {
 			id        TEXT PRIMARY KEY,
 			user_id   TEXT NOT NULL,
 			token     TEXT NOT NULL UNIQUE,
+			secret    TEXT NOT NULL DEFAULT '',
 			label     TEXT NOT NULL DEFAULT '',
 			created   INTEGER NOT NULL,
 			expires   INTEGER NOT NULL,
@@ -476,6 +477,18 @@ func migrateSchema(db *sql.DB) error {
 			return err
 		}
 	}
+	// Guest links are re-showable in Settings, which means keeping the token
+	// itself and not only its hash (see Auth.createShare). Links made before
+	// this have an empty secret and simply cannot be shown again.
+	hasSecret, err := columnExists(db, "share_links", "secret")
+	if err != nil {
+		return err
+	}
+	if !hasSecret {
+		if _, err := db.Exec(`ALTER TABLE share_links ADD COLUMN secret TEXT NOT NULL DEFAULT ''`); err != nil {
+			return err
+		}
+	}
 	hasShareID, err := columnExists(db, "sessions", "share_id")
 	if err != nil {
 		return err