Add claim links for artificial members

[GodPuffin]

Summary

• add persistent claim links for artificial members using a new member_claim_links PocketBase collection and shared transfer logic
• add a public claim page plus login/register claim preservation so invitees can sign in or create an account and immediately take over the placeholder member
• add owner controls to create and copy claim links from the plan details page, and extend route/auth coverage for the new flow

Testing

• go test ./...

[GodPuffin]

@claude review

[claude]

Claude finished @GodPuffin's task in 2m 19s —— View job

---

Code Review

• Read CLAUDE.md for repo guidelines
• Read changed files
• Analyze for bugs, security issues, and best practices
• Post review feedback

---

Overall this is a well-structured feature with good separation of concerns: the memberclaim package cleanly encapsulates the domain logic, and the transfer logic was properly extracted from transfer.go and reused. The flow is easy to follow. A few things worth addressing below.

---

Bugs

created field likely silently ignored on new membership

src/internal/memberclaim/claim.go:377

newMembership.Set("created", artificialMembership.GetDateTime("created"))

In PocketBase, created is a system-managed field and Set() on a new record typically has no effect — PocketBase sets it on first save. This means the new real membership will get the current timestamp rather than inheriting the placeholder's creation date, despite the intent of preserving membership history. You should verify this works as expected, or drop the line and document that the created date will differ.

Fix this →

---

Security

Claim POST endpoint accepts unauthenticated requests without CSRF protection

src/internal/http/router/router.go:46

e.POST("/claim-member/:token", memberships.HandleClaimMember(app))

This route has no CSRF protection. A malicious page could POST to /claim-member/<token> for a logged-in victim if they somehow obtain the token. The handler does check for authentication and redirects if not authenticated, but a CSRF attack would execute on behalf of an already-logged-in user. If you add CSRF tokens elsewhere in the app later, this endpoint will be a gap to revisit.

HTTP scheme detection unreliable behind a reverse proxy

src/internal/http/handlers/plans/details_claim_links.go:31-39

func absoluteClaimLink(scheme, host, token string) string {
    if scheme == "" {
        scheme = "http"
    }
    ...
    return fmt.Sprintf("%s://%s%s", scheme, host, memberclaim.Path(token))
}

c.Scheme() reads from the raw connection, not from forwarded headers like X-Forwarded-Proto. If deployed behind a TLS-terminating proxy (very common), this will generate http:// claim links even though the site is HTTPS. Consider reading X-Forwarded-Proto as a fallback.

Fix this →

---

Data Integrity

No composite unique index on (plan_id, artificial_member_id)

migrations/1709604007_add_member_claim_links.go

The migration adds a unique constraint on token but not on (plan_id, artificial_member_id). The EnsureWithDao function prevents duplicates at the application layer (check-then-insert inside a transaction), but a database-level constraint would be safer and self-documenting. Two concurrent transactions could both pass the existence check before either inserts.

Fix this →

---

Minor Issues

Template: claim token in URL not URL-encoded

src/internal/assets/templates/claim_member.html:76-82

<a href="/login?claim={{.claim_token}}">...
<a href="/register?claim={{.claim_token}}">...

Go templates HTML-escape values in attribute context but don't URL-encode them. The random.GenerateToken() function likely produces alphanumeric-only tokens (making this safe in practice), but using {{.claim_token | urlquery}} would be explicit and correct regardless of token alphabet changes.

Router test doesn't cover all routes

src/internal/http/router/router_test.go

The test's expected map omits /:join_code/request-join (GET and POST), /:join_code/approve-payment, and /:join_code/reject-payment. These are registered in the router but not asserted in the test. Since this file was modified in this PR (adding the two new claim routes), it's a good opportunity to fill in the gaps.

missingTransferPrerequisite as unexported local sentinel

src/internal/http/handlers/memberships/transfer.go:42

missingTransferPrerequisite := errors.New("membership transfer prerequisite not found")

Defining a sentinel error inside a function body is unusual — it creates a new distinct error value on every request, which makes it impossible to inspect outside the handler. It works here because it's only compared with errors.Is inside the same scope, but consider promoting it to a package-level var for clarity.

---

What's Done Well

• Extracting TransferArtificialMembership into the memberclaim package and reusing it from both the owner-transfer and claim flows is exactly the right call — the old transfer.go was correctly simplified down.
• The Dao-suffix variants of every function make the transactional path clean without duplicating logic.
• Claim token preservation across login/register redirects is handled consistently in claim.go helpers, avoiding scattered ad-hoc query string manipulation.
• Tests cover the key auth page redirect cases for both login and register with and without claim tokens.