createInvite has no role check — any member can invite #14

New Issue

marcel · 2026-04-02T11:21:33+02:00

marcel commented

2026-04-02 11:21:33 +02:00

Problem

HouseholdService.createInvite() does not check whether the requesting user is a planner. Any household member (including those with member role) can generate invite codes.

Affected files

HouseholdService.java:94-109 — no role check before creating invite

Recommended fix

Check member.getRole() and restrict invite creation to planner role (or make this a business decision and document it explicitly).

Severity

Low — depends on business rules, but least-privilege suggests only planners should invite.

## Problem `HouseholdService.createInvite()` does not check whether the requesting user is a `planner`. Any household member (including those with `member` role) can generate invite codes. ## Affected files - `HouseholdService.java:94-109` — no role check before creating invite ## Recommended fix Check `member.getRole()` and restrict invite creation to `planner` role (or make this a business decision and document it explicitly). ## Severity Low — depends on business rules, but least-privilege suggests only planners should invite.

marcel added the kind/security priority/low labels 2026-04-02 11:21:52 +02:00

marcel referenced this issue

2026-04-03 09:26:40 +02:00

Frontend: A4 — Join household (accept invite) #21

marcel commented

2026-04-03 09:28:42 +02:00

👨‍💻 Kai — Frontend Engineer

This is a pure backend authorization issue, but it does affect frontend behavior in one meaningful way: the invite creation UI.

Frontend implications:

Wherever the "Invite member" button or action lives (presumably in household settings — screen A3 or a settings page), it should only be rendered for planners. If a member can somehow reach that UI, they'd get a successful-looking form that results in an error on submit — bad UX and a security signal.
The role check should happen on both sides: the backend (the fix this issue is proposing) and the frontend (don't render the invite UI to members).
For SvelteKit: the session data in hooks.server.ts should expose the user's role. The page's +page.server.ts load function should check locals.user.role === 'planner' and either redirect or return a forbidden flag that suppresses the invite UI.

What I need to know:

Where does the invite creation UI live in the screen map? Is it A3 (household settings) or another screen? Knowing this helps me make sure the role guard is in the right place in the route hierarchy.
After this fix, what HTTP status does the backend return when a member attempts to call the invite endpoint? I want to map that response to a clear UI error — probably a toast saying "Only planners can invite new members."
Is there any scenario where a member should be able to invite? If the business rule is "planners only, always," I'll hardcode that — no need to make it configurable.

## 👨‍💻 Kai — Frontend Engineer This is a pure backend authorization issue, but it does affect frontend behavior in one meaningful way: the invite creation UI. **Frontend implications:** - Wherever the "Invite member" button or action lives (presumably in household settings — screen A3 or a settings page), it should only be rendered for planners. If a member can somehow reach that UI, they'd get a successful-looking form that results in an error on submit — bad UX and a security signal. - The role check should happen on both sides: the backend (the fix this issue is proposing) and the frontend (don't render the invite UI to members). - For SvelteKit: the session data in `hooks.server.ts` should expose the user's role. The page's `+page.server.ts` load function should check `locals.user.role === 'planner'` and either redirect or return a `forbidden` flag that suppresses the invite UI. **What I need to know:** - Where does the invite creation UI live in the screen map? Is it A3 (household settings) or another screen? Knowing this helps me make sure the role guard is in the right place in the route hierarchy. - After this fix, what HTTP status does the backend return when a member attempts to call the invite endpoint? I want to map that response to a clear UI error — probably a toast saying "Only planners can invite new members." - Is there any scenario where a member should be able to invite? If the business rule is "planners only, always," I'll hardcode that — no need to make it configurable.

marcel commented

2026-04-03 09:28:56 +02:00

🔧 Backend Engineer — createInvite Role Check (Issue #14)

This is a straightforward authorization gap and the fix is clear. Let me add some implementation specifics.

The fix:

In HouseholdService.createInvite() (line 94–109), fetch the HouseholdMember record for the requesting user in this household, check member.getRole() == HouseholdRole.PLANNER, and throw a domain exception (e.g., InsufficientRoleException) if they're not.
This exception should be caught in the controller or via a @ExceptionHandler and mapped to an HTTP 403 response.

Why service layer, not controller:

The role check belongs in the service layer, not the controller. Business rules live in the service — the controller should only translate the HTTP request and delegate. If we put if (role != PLANNER) return 403; in the controller, we've split the authorization logic across layers and made it invisible to unit tests of the service.

Where to get the requesting user's role:

The service needs the authenticated user's ID (from the security context) and the household ID. It should look up the HouseholdMember record rather than trusting any role passed from the request body.
Don't pass the role as a parameter — derive it from the database. A member cannot claim to be a planner by manipulating the request.

Also worth checking:

Are there other HouseholdService methods (e.g., removeMember, changeRole, updateHouseholdSettings) that also lack role checks? This issue suggests the pattern is missing, not just a one-off. I'd audit the full service before closing this.

Questions:

Is there a @PreAuthorize or similar Spring Security method-level security annotation being used anywhere in the codebase? If so, @PreAuthorize("hasRole('PLANNER')") could handle this declaratively — but we'd need to ensure the role is in the security context, not just in the DB. Worth discussing the pattern before implementing.

## 🔧 Backend Engineer — createInvite Role Check (Issue #14) This is a straightforward authorization gap and the fix is clear. Let me add some implementation specifics. **The fix:** - In `HouseholdService.createInvite()` (line 94–109), fetch the `HouseholdMember` record for the requesting user in this household, check `member.getRole() == HouseholdRole.PLANNER`, and throw a domain exception (e.g., `InsufficientRoleException`) if they're not. - This exception should be caught in the controller or via a `@ExceptionHandler` and mapped to an HTTP 403 response. **Why service layer, not controller:** - The role check belongs in the service layer, not the controller. Business rules live in the service — the controller should only translate the HTTP request and delegate. If we put `if (role != PLANNER) return 403;` in the controller, we've split the authorization logic across layers and made it invisible to unit tests of the service. **Where to get the requesting user's role:** - The service needs the authenticated user's ID (from the security context) and the household ID. It should look up the `HouseholdMember` record rather than trusting any role passed from the request body. - Don't pass the role as a parameter — derive it from the database. A member cannot claim to be a planner by manipulating the request. **Also worth checking:** - Are there other `HouseholdService` methods (e.g., `removeMember`, `changeRole`, `updateHouseholdSettings`) that also lack role checks? This issue suggests the pattern is missing, not just a one-off. I'd audit the full service before closing this. **Questions:** - Is there a `@PreAuthorize` or similar Spring Security method-level security annotation being used anywhere in the codebase? If so, `@PreAuthorize("hasRole('PLANNER')")` could handle this declaratively — but we'd need to ensure the role is in the security context, not just in the DB. Worth discussing the pattern before implementing.

marcel commented

2026-04-03 09:29:05 +02:00

🧪 QA Engineer — createInvite Role Check (Issue #14)

Authorization gaps need to be tested at the integration level — a unit test of the service can verify the role check logic, but only an integration test proves the HTTP layer enforces it correctly end-to-end.

Tests I want written alongside this fix:

Unit tests (service layer):

shouldAllowPlannerToCreateInvite()
shouldRejectMemberAttemptingToCreateInvite()       // throws InsufficientRoleException
shouldRejectUnauthenticatedCallToCreateInvite()    // if applicable at service level

Integration tests (full request cycle):

POST /households/{id}/invites as planner      → 201 Created with invite token
POST /households/{id}/invites as member       → 403 Forbidden
POST /households/{id}/invites unauthenticated → 401 Unauthorized
POST /households/{id}/invites as planner from a DIFFERENT household → 403 Forbidden (IDOR check)

That last case is critical: a planner from household A should not be able to create an invite for household B. This is a separate but related authorization gap — the household ID in the path must be validated against the authenticated user's membership.

Regression check:

Existing planner-as-invite-creator tests (if any) should remain green
The happy path (planner creates invite successfully) must be explicitly tested, not just assumed

Audit suggestion:

Before this fix is marked done, I'd like to do a quick review of all HouseholdService methods alongside the backend engineer to produce a role-check coverage matrix. "Which methods exist, which roles do they require, which have tests?" This issue may be the tip of an iceberg.

Questions:

Does the current test suite have any test that calls the invite endpoint as a member? If not, this gap in coverage predates the code bug — both need fixing together.

## 🧪 QA Engineer — createInvite Role Check (Issue #14) Authorization gaps need to be tested at the integration level — a unit test of the service can verify the role check logic, but only an integration test proves the HTTP layer enforces it correctly end-to-end. **Tests I want written alongside this fix:** **Unit tests (service layer):** ``` shouldAllowPlannerToCreateInvite() shouldRejectMemberAttemptingToCreateInvite() // throws InsufficientRoleException shouldRejectUnauthenticatedCallToCreateInvite() // if applicable at service level ``` **Integration tests (full request cycle):** ``` POST /households/{id}/invites as planner → 201 Created with invite token POST /households/{id}/invites as member → 403 Forbidden POST /households/{id}/invites unauthenticated → 401 Unauthorized POST /households/{id}/invites as planner from a DIFFERENT household → 403 Forbidden (IDOR check) ``` That last case is critical: a planner from household A should not be able to create an invite for household B. This is a separate but related authorization gap — the household ID in the path must be validated against the authenticated user's membership. **Regression check:** - Existing planner-as-invite-creator tests (if any) should remain green - The happy path (planner creates invite successfully) must be explicitly tested, not just assumed **Audit suggestion:** - Before this fix is marked done, I'd like to do a quick review of all `HouseholdService` methods alongside the backend engineer to produce a role-check coverage matrix. "Which methods exist, which roles do they require, which have tests?" This issue may be the tip of an iceberg. **Questions:** - Does the current test suite have any test that calls the invite endpoint as a member? If not, this gap in coverage predates the code bug — both need fixing together.

marcel commented

2026-04-03 09:29:17 +02:00

🔐 Sable — Security Engineer

This is broken access control — OWASP A01. The severity is labeled Low because it's a business-rule violation (least privilege), but the real-world impact could be higher depending on how invites are used.

Attack scenario:

A household member with member role calls POST /households/{id}/invites directly (bypassing the UI entirely — just a curl command).
They receive a valid invite token.
They share that token with anyone, effectively growing the household without the planner's knowledge or consent.
If the planner set up the household with specific trust in mind (family members only, for example), this is a meaningful unauthorized action.

The fix, with security requirements:

Role check in HouseholdService.createInvite() — fetch the HouseholdMember for the authenticated user, assert role == PLANNER, throw 403 otherwise. Role must come from DB, not from the request.
The household in the path parameter must belong to the authenticated user's household — enforces the household isolation boundary. Don't skip this.
The fix must be covered by an integration test that calls the endpoint as a member and asserts 403.

Broader audit I'm requesting before this closes:

All HouseholdService mutating methods should be audited for role checks. Specifically: remove member, change member role, update household name, delete household. Any of these missing a role check is the same class of bug.
Are there any Spring Security @PreAuthorize annotations on the household controller methods? If not, all authorization is happening (or not happening) inside the service — which needs to be verified method by method.

Questions:

Was this gap found by code review or by actually testing the endpoint as a member? If the latter, please document the reproduce steps as a test case — that test must exist before the fix ships.
Is there a security integration test suite that runs all protected endpoints as multiple roles? If not, this issue is an argument for building one.

## 🔐 Sable — Security Engineer This is broken access control — OWASP A01. The severity is labeled Low because it's a business-rule violation (least privilege), but the real-world impact could be higher depending on how invites are used. **Attack scenario:** - A household member with `member` role calls `POST /households/{id}/invites` directly (bypassing the UI entirely — just a curl command). - They receive a valid invite token. - They share that token with anyone, effectively growing the household without the planner's knowledge or consent. - If the planner set up the household with specific trust in mind (family members only, for example), this is a meaningful unauthorized action. **The fix, with security requirements:** 1. Role check in `HouseholdService.createInvite()` — fetch the `HouseholdMember` for the authenticated user, assert `role == PLANNER`, throw 403 otherwise. Role must come from DB, not from the request. 2. The household in the path parameter must belong to the authenticated user's household — enforces the household isolation boundary. Don't skip this. 3. The fix must be covered by an integration test that calls the endpoint as a member and asserts 403. **Broader audit I'm requesting before this closes:** - All `HouseholdService` mutating methods should be audited for role checks. Specifically: remove member, change member role, update household name, delete household. Any of these missing a role check is the same class of bug. - Are there any Spring Security `@PreAuthorize` annotations on the household controller methods? If not, all authorization is happening (or not happening) inside the service — which needs to be verified method by method. **Questions:** - Was this gap found by code review or by actually testing the endpoint as a member? If the latter, please document the reproduce steps as a test case — that test must exist before the fix ships. - Is there a security integration test suite that runs all protected endpoints as multiple roles? If not, this issue is an argument for building one.

marcel commented

2026-04-03 09:29:27 +02:00

🎨 Atlas — UI/UX Designer

This is a backend authorization issue, but the design layer has a role to play in preventing confusion and misuse at the UI level.

Role-aware UI:

The invite flow entry point (wherever "Invite member" lives in the household settings screen) must only be visible to planners. If a member somehow sees this UI and attempts to use it, the resulting error is a UX failure — not just a security catch.
From a design system standpoint, this is a "conditional render by role" pattern that I want to establish clearly. For any action that is planner-only, the element simply doesn't exist in the DOM for members — not disabled, not grayed out, not hidden with CSS. It should not be rendered at all.

Why "disabled" is the wrong pattern here:

Showing a grayed-out "Invite member" button to a member creates confusion ("Why can't I click this?") without explanation. If we want to communicate role differences, the household settings screen could have a clear "Your role: Member" indicator — but the invite action itself should be invisible.

Error state (defensive design):

If a member somehow triggers the invite endpoint (direct API call, bug in the frontend guard), the 403 response needs to surface as a readable error in the UI. A toast with "Only household planners can invite new members" is the right pattern — non-alarming, informative, actionable (implicitly: ask your planner).

Questions:

Is there a screen spec for household settings where the invite action lives? I want to confirm the role-visibility rule is captured in the spec, not just in this issue. If the spec doesn't reflect it, I'll update it.

## 🎨 Atlas — UI/UX Designer This is a backend authorization issue, but the design layer has a role to play in preventing confusion and misuse at the UI level. **Role-aware UI:** - The invite flow entry point (wherever "Invite member" lives in the household settings screen) must only be visible to planners. If a member somehow sees this UI and attempts to use it, the resulting error is a UX failure — not just a security catch. - From a design system standpoint, this is a "conditional render by role" pattern that I want to establish clearly. For any action that is planner-only, the element simply doesn't exist in the DOM for members — not disabled, not grayed out, not hidden with CSS. It should not be rendered at all. **Why "disabled" is the wrong pattern here:** - Showing a grayed-out "Invite member" button to a member creates confusion ("Why can't I click this?") without explanation. If we want to communicate role differences, the household settings screen could have a clear "Your role: Member" indicator — but the invite action itself should be invisible. **Error state (defensive design):** - If a member somehow triggers the invite endpoint (direct API call, bug in the frontend guard), the 403 response needs to surface as a readable error in the UI. A toast with "Only household planners can invite new members" is the right pattern — non-alarming, informative, actionable (implicitly: ask your planner). **Questions:** - Is there a screen spec for household settings where the invite action lives? I want to confirm the role-visibility rule is captured in the spec, not just in this issue. If the spec doesn't reflect it, I'll update it.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/mealprep#14