feat(join): A4 — Join household (accept invite) #61
Reference in New Issue
Block a user
Delete Branch "feat/issue-21-join-household"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
GET /v1/invites/{code}— new unauthenticated endpoint returning household name + inviter name (404 for all invalid cases, enumeration-resistant)POST /v1/invites/{code}/accept— reworked: takes{name, email, password}, creates account + joins household + establishes session in one transaction; 409 on duplicate emailSecurityConfig:permitAll()for/v1/invites/**+sessionFixation().changeSessionId()invited_byFK column onhousehold_invite/join/[token]page: no-chrome layout, mobile stacked / desktop 400px+flex split, invalid-token inline statehooks.server.ts:/joinpublic; authenticated users on/join/*redirected to//*vs/**in security matcher, JPA flush order on invite invalidation,invalidated_atnot set on acceptTest plan
/join/{valid-code}— see household name, inviter, permissions panel/join/{invalid-code}— inline "Einladung ungültig" state, no nav chrome/join/{code}while logged in — redirected to/🤖 Generated with Claude Code
- Add V006 migration: invalidated_at column + partial unique index on household_invite - Add findByHouseholdIdAndInvalidatedAtIsNull, findByHouseholdIdAndUserId, countByHouseholdIdAndRole - Add ChangeRoleRequest DTO - HouseholdService: getActiveInvite, createInvite (regenerate), removeMember, changeMemberRole - HouseholdController: GET /v1/households/mine/invites, DELETE/PATCH /v1/households/mine/members/{userId} Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>- MemberCard: white bg, 1px border + shadow-card, centered column layout, avatar color by role (green-dark/blue), role badge with role-specific colors, join date "seit DD.MM.YYYY", Du-badge below join date, ⋯ kebab with icons and divider, inline role-control with Abbrechen, blue editing border #B5D4F4 - InviteCard: white bg, 1.5px dashed border, min-height 180px, plus circle, label "Mitglied einladen", full hover state (green border/bg/icon/label) - InvitePanel: white bg, title "Einladelink teilen", description, mono link box, yellow expiry pill when ≤ 24h, text-link "Neuen Link generieren" - RemoveDialog: white bg, padding 28px 32px, "?" in title, updated body text - +page.server.ts: expose householdName from locals.haushalt - +page.svelte: subtitle "{n} Mitglieder · {householdName}" - Tests: add join date format test, Abbrechen test, InvitePanel title test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>removeMember now checks the planner count before deleting a planner member. Throws ConflictException("Cannot remove the last planner") when only one planner remains, matching the spec requirement in S4. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>- V027 migration: add invited_by FK column on household_invite - HouseholdInvite entity: add invitedBy field, set on createInvite - New DTOs: InviteInfoResponse, AcceptInviteRequest - HouseholdService: add getInviteInfo(), rewrite acceptInvite(code, name, email, password) — creates UserAccount + joins household in one transaction - HouseholdController: GET /v1/invites/{code} (unauthenticated), POST /v1/invites/{code}/accept creates session after join - SecurityConfig: permitAll() for /v1/invites/*, sessionFixation().changeSessionId() Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>- schema.d.ts: add GET /v1/invites/{code}, InviteInfoResponse, AcceptInviteRequest; update acceptInvite operation - hooks.server.ts: add /join to PUBLIC_ROUTES; redirect authenticated users on /join/* to / - +page.server.ts: load validates token (invalid:true on 404); action creates account + joins + sets session cookie - HouseholdIdentityPanel.svelte: logo, household name (Fraunces), inviter text, static permissions list - JoinForm.svelte: name/email/password + show/hide toggle, "Haushalt beitreten" CTA, field errors, pre-fill - +page.svelte: no-chrome layout, mobile (banner+form stacked) / desktop (400px panel + flex:1) split, invalid-token inline state Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>- SecurityConfig: /** covers /v1/invites/{code}/accept (two path segments); /* only matched one segment so the accept endpoint was returning 401 - HouseholdIdentityPanel + page: use --green-dark bg (matching BrandPanel on login) instead of --green-tint; text updated to white/--green-light Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>🔧 Backend Engineer — Senior Spring Boot / PostgreSQL Specialist
Verdict: 🚫 Changes requested
Blockers
1.
getInviteInfoandacceptInvitedon't checkinvalidatedAt— invalidated invites bypass validationcreateInvitecorrectly setsinvalidatedAt = NOW()on the previous invite when a new one is created. However, bothgetInviteInfoandacceptInviteonly checkstatus='used'andexpiresAt:Attack path: Person A receives invite link → planner creates a new invite (Person A's invite gets
invalidatedAtset, butstatusstays'pending') → Person A still sees the household info page via GET/v1/invites/{code}and can still accept via POST. The partial unique index was specifically added to prevent duplicate active invites at the DB level, but the application-layer validation doesn't honour it.Fix — add
invalidatedAtcheck to both methods:2. Session management code belongs in a service, not the controller
HouseholdControllernow imports 7 Spring Security classes and contains a fullauthenticateInSessionmethod that invalidates sessions and mutatesSecurityContextHolder. Controllers are for HTTP concerns only. This exact pattern exists (or should exist) inAuthController— extract it to a sharedAuthSessionServiceor package-private utility so both callers use the same implementation.Suggestions
libwebpin Dockerfile is unrelated to this featureThis should be in its own commit (or at minimum explained in the PR description). It silently inflates the image and makes bisecting harder.
Dual-state invite validity:
statusfield vsinvalidated_atThe codebase now maintains two parallel invalidation signals:
status='used'(set on accept) andinvalidated_at IS NOT NULL(set on supersede and on accept). The partial unique index uses onlyinvalidated_at IS NULL. Thestatusfield is now redundant — every meaningful state can be expressed viainvalidated_at. Consider removingstatusand usinginvalidatedAt != nullas the single source of truth in V028.removeMembererror message is role-specific but the check isn'tThe check fires for any requester (member or planner). The message says "Planner" but a plain member would get this too. Minor but confusing. "Cannot remove yourself" is simpler and accurate.
🔒 Sable — Security Engineer
Verdict: 🚫 Changes requested
Blockers
1. Invalidated invite bypass — unintended household membership
Both
getInviteInfoandacceptInvitevalidate invite state via:invalidatedAtis never checked. When a planner regenerates the invite link, the old code is setinvalidatedAt = NOW()butstatusstays'pending'. Anyone holding the old link can:This violates the threat model for invite tokens: "Leaked link = time-limited exposure, not permanent access." A superseded token should be immediately invalid, not just until it expires 48 hours later.
Concerns
2. Hardcoded
ROLE_USERinauthenticateInSessionbypassesUserDetailsServiceThe established session grants exactly
ROLE_USER. This works today because there are two roles:ROLE_USERandROLE_ADMIN. But thesystem_roleonuser_accountis not consulted here — if the app ever provisions an admin account via invite (or if an existing admin account is the email match), they'd receive onlyROLE_USERfor their session. Low risk today, but worth loading authorities from theUserDetailsServicefor correctness.3.
secure: truehardcoded in cookie optionsOn HTTP (local dev), the browser silently drops
Securecookies. This means join-then-redirect silently fails in development — the user gets redirected to/without a valid session, then redirected back to/login. This was likely the root cause of the "get redirected to login but can't login" bug in testing. Useprocess.env.NODE_ENV !== 'production' ? false : trueor an explicit env variable.4. No rate limiting visible on invite accept endpoint
POST /v1/invites/{code}/acceptispermitAll()and creates auser_accountrow on every successful call. There is no visible rate limiting or account creation throttle. An 8-char alphanumeric code (36^8 ≈ 2.8T combinations) is not guessable in 48 hours at reasonable rates, but a targeted amplification attack on a known household (ifhouseholdIdis observed elsewhere) could exhaust invite slots. Low real-world risk, flagged for awareness.Positive findings
authenticateInSession✓sessionFixation().changeSessionId()added toSecurityConfig✓passwordEncoder.encode(rawPassword)— no plaintext storage ✓invited_byFK usesON DELETE SET NULL— no referential integrity failure if inviter account is deleted ✓GET /v1/invites/{code}correctly uses 404 (not 401/403) for unauthenticated path — enumeration hardening ✓🧪 QA Engineer
Verdict: ⚠️ Approved with concerns
Blockers
1. No test covers the
invalidatedAtgap — a passing test suite masks a real bugHouseholdServiceTestcovers:getInviteInfo→ valid invite ✓getInviteInfo→ not found ✓getInviteInfo→ expired ✓getInviteInfo→ used ✓Missing: invite that was superseded (invalidated_at IS NOT NULL, status='pending', not expired). This is the exact case where validation currently fails silently. A test like this would have caught the bug:
Same test gap exists for
acceptInvite.Suggestions
2.
HouseholdIdentityPanel.test.tsonly asserts 2 of 3 permissionsThe third list item ("Artikel zur Liste hinzufügen") has no test coverage. Since these items are static and document the member role contract, all three should be asserted.
3. No integration test for the full accept-invite flow
HouseholdControllerTestforacceptInviteis a standalone MockMvc test (no Spring Security context, no real session). There's no integration test (@SpringBootTest+ Testcontainers) that verifies:user_accountrow is createdSet-Cookie: JSESSIONID=...header is presentThis is the most critical happy-path in the feature and it deserves an integration-level test.
4.
page.server.test.tsuses barecatchfor redirect assertionsThis pattern is fragile — any unrelated exception would silently be tested against
.statusand.location. Use:5.
hooks.server.test.tsmissing test for join-route redirect when authenticatedThe hooks guard (
if (isJoinRoute && event.cookies.get('JSESSIONID'))) is not covered by the hooks test for the join path specifically. A test case "authenticated user on /join/TOKEN is redirected to /" would close this gap.Positive findings
JoinForm.test.tscovers rendering, interaction (toggle), error states, and pre-fill ✓SecurityConfigTestvalidates that thepermitAllchain is actually wired (not just trusting config reading) ✓page.server.test.tsare covered ✓⚡ Kai — Frontend Engineer
Verdict: ⚠️ Approved with concerns
Concerns
1.
secure: truehardcoded — breaks cookie relay in local HTTP developmentSecurecookies are silently dropped on HTTP. In local dev the join will "succeed" (POST returns 200,Set-Cookieis in the response) but the cookie never reaches the browser, so the subsequent redirect to/triggers the auth guard and bounces back to/login. This likely caused the "redirected to login but can't login" regression.2.
aria-label="Passwort anzeigen"doesn't update when toggledThe visual label toggles between "Anzeigen" / "Verbergen" but
aria-labelstays"Passwort anzeigen"regardless of state. Screen reader users hear "Passwort anzeigen" even after the password is already visible. Fix:Minor nits
3.
PUBLIC_ROUTESincludes'/join'but the actual route is/join/[token]isPublicRoutepresumably checkspathname.startsWith(route)orpathname === route. If it's an exact match, adding'/join'adds a dead entry (nothing routes to/joinwithout a token). If it'sstartsWith, it's correct. Worth a quick scan ofisPublicRouteto confirm the intent is explicit.4.
'JSESSIONID'is a magic string in two placeshooks.server.tsreadsevent.cookies.get('JSESSIONID')and+page.server.tssetscookies.set('JSESSIONID', ...). If the backend ever renames the session cookie (e.g., for multi-tenant or cookie-prefix hardening), this breaks silently in both places. One shared constant in$lib/server/constants.tswould make this more maintainable.Positive findings
$props(),$state()— no legacyexport letor$:✓use:enhancefor progressive enhancement without full-page reload ✓autocomplete="given-name","email","new-password"— proper autocomplete hints ✓+page.server.tsaction correctly returnsfail(409, ...)vsfail(400, ...)for different backend errors ✓🎨 Atlas — UI/UX Designer
Verdict: ⚠️ Approved with concerns
Concerns
1.
aria-labelon password toggle doesn't update on state change(flagged by Kai as well — confirming from accessibility lens)
aria-label="Passwort anzeigen"is static. WhenshowPassword = true, screen readers still announce "Passwort anzeigen", which is the wrong state. This fails WCAG 2.2 SC 4.1.2 (Name, Role, Value). Fix: bind thearia-labelto the toggle state.2.
<ul>permissions list has no accessible nameThe
<p>visually labels the list, but the<ul>is not programmatically associated with it. Screen readers announce the list without context. Fix:Or wrap both in a
<section>with<h3>and use the heading as the implicit label.3.
rounded-2xlshould use the design tokenrounded-2xl= 1rem ≈ 16px =--radius-xlin the design system. Userounded-[var(--radius-xl)]to stay token-native. Non-blocking, but it adds a Tailwind magic number where a token exists.Positive findings
font-semibold= 600) ✓font-[var(--font-display)]for household name (Fraunces) andfont-[var(--font-sans)]for body text (DM Sans) — correct font application ✓tracking-[-0.02em]on display headline — matches existing auth page pattern ✓text-whitefor primary text,text-[var(--green-light)]for muted text on dark background — correct contrast hierarchy ✓aria-hidden="true"— decorative marks correctly hidden from screen readers ✓flex-col lg:flex-row— responsive breakpoint matches the login page layout ✓bg-[var(--green-dark)]with white text — matches the login page CTA, consistent with auth flow visual language ✓Review concerns addressed
All blockers and concerns from the review have been resolved. Summary of changes:
Security / Backend blockers
Invalidated invite bypass (Backend Blocker 1 · Security Blocker 1 · QA Blocker 1)
0ab1ba0—getInviteInfonow rejects invites whereinvalidatedAt != null73af11e—acceptInvitenow rejects invites whereinvalidatedAt != nullSession management extracted from controller (Backend Blocker 2 · Security Concern 2)
0b182a3—authenticateInSessionmoved toAuthServiceas a public method with built-in session fixation protection; bothAuthControllerandHouseholdControllernow delegate to it.HouseholdControllerdrops all 7 Spring Security imports.Frontend concerns
secure: !devfor JSESSIONID cookie (Security Concern 3 · Frontend Concern 1)230ee5a— Usesdevfrom$app/environmentso the cookie works over HTTP in local development and remainsSecurein production. Test updated withvi.mock('$app/environment', () => ({ dev: false })).Password toggle
aria-labelupdates with state (Frontend Concern 2 · Atlas Concern 1)ccfc72a—aria-labelis now bound:"Passwort anzeigen"/"Passwort verbergen". New test verifies the label updates after click.Permissions
<ul>accessible name (Atlas Concern 2)26256ef—<ul aria-label="Als Mitglied kannst du">added. New test viagetByRole('list', { name: ... }).Design token for border radius (Atlas Concern 3)
df0d453—rounded-2xl→rounded-[var(--radius-xl)]Test suite
AuthControllerTestupdated: session-attribute assertions replaced withverify(authService).authenticateInSession(...)reflecting the refactored delegation.