fix(invitations): scope list, self-invite guard, open-signup gate (#3833)#3855
fix(invitations): scope list, self-invite guard, open-signup gate (#3833)#3855PierreBrisorgueil wants to merge 4 commits into
Conversation
GET /api/invitations stays admin-only by CASL, but the controller now threads req.user and the service keys the repository filter on the caller's role: admins read the platform-global list, anyone else only the invitations they sent (the invitedBy index covers it). Prep so the referral phase can widen the Invitation abilities without leaking invitee emails platform-wide. refs #3833
create() now fails fast with 'You cannot invite yourself' when the invitee email equals the inviter's own. E9 already caught the exact match as a registered email, but with a misleading message; this makes the intent explicit. Also pins the grant-side floor (#3833 belt): expectedGrantKeys implies no referrer key when invitedBy equals acceptedUserId (same-account pairs only — alias multi-account self-invites remain an accepted residual risk, out of scope here). refs #3833
Documents the open-signup invariant kept on purpose: with sign.up true a presented token is resolved but never claimed/finalized, so invitation.accepted never fires and referral grants are a silent no-op. Marks gates 1-2 shipped in the README and mirrors the caveat in the billing referral config block. refs #3833
|
Warning Review limit reached
More reviews will be available in 31 minutes and 15 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR implements the blocking gate ( ChangesReferral Gating (
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant CreateService
participant Repository
Client->>Controller: create invitation (invitedBy, email)
Controller->>CreateService: create(invitedBy, email)
CreateService->>CreateService: normalize emails
alt inviter email == invitee email
CreateService-->>Client: 422 AppError (self-invite)
else
CreateService->>Repository: check user exists
Repository-->>CreateService: result
CreateService->>Repository: persist invitation
Repository-->>CreateService: created
CreateService-->>Client: success
end
flowchart TD
A[InvitationService.list user] --> B{User.roles includes admin?}
B -->|Yes| C[Repository.list unscoped]
B -->|No| D{User id exists?}
D -->|Yes| E[Repository.list invitedBy=userId]
D -->|No| F[Return empty array]
C --> G[Return results]
E --> G
F --> H[Prevent PII leak]
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3855 +/- ##
==========================================
+ Coverage 91.96% 91.97% +0.01%
==========================================
Files 160 160
Lines 5289 5296 +7
Branches 1698 1704 +6
==========================================
+ Hits 4864 4871 +7
Misses 337 337
Partials 88 88
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR hardens the invitations/referrals surface ahead of widening referral abilities by ensuring the invitations list endpoint can’t leak invitee-email PII to non-admins, adds a clearer self-invite rejection path, and documents the “open signup disables referral rewards” invariant.
Changes:
- Scoped
InvitationService.list()so admins get a global list while non-admins are restricted to{ invitedBy: callerId }(and unknown callers get[]). - Added an explicit self-invite guard in
InvitationService.create()that rejects with a 422 before the registered-email lookup/persistence path. - Updated docs/config commentary and added/updated unit tests to pin the above behaviors (including the grant-side self-referral floor).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| modules/invitations/tests/invitations.service.unit.tests.js | Adds unit coverage for self-invite guard ordering/normalization and role-keyed list scoping. |
| modules/invitations/tests/invitations.repository.unit.tests.js | Verifies repository list can accept and thread a { invitedBy } filter while still stripping tokens. |
| modules/invitations/tests/invitations.controller.unit.tests.js | Ensures the controller passes req.user into the service for scoping. |
| modules/invitations/services/invitations.service.js | Implements self-invite guard and role-keyed/scoped list behavior. |
| modules/invitations/repositories/invitations.repository.js | Adds optional filter param to list() to support scoped queries. |
| modules/invitations/README.md | Updates referral “gates” documentation to reflect shipped scoping + self-invite guard and clarifies open-signup gating. |
| modules/invitations/controllers/invitations.controller.js | Passes the authenticated caller to InvitationService.list(req.user). |
| modules/billing/tests/billing.referral.service.unit.tests.js | Pins same-account self-referral floor behavior in expectedGrantKeys. |
| modules/billing/config/billing.development.config.js | Clarifies that referral grants require closed signup and documents shipped #3833 gates. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
@coderabbitai full review |
✅ Action performedFull review finished. |
The admin role-check mirrors invitations.policy.js; any CASL ability widening for this route must be reviewed against this seam. refs #3833
Summary
GET /api/invitationslist()is now role-keyed in the service — admins read platform-global; non-admins read only invitations they sent (invitedBy-scoped) — so the later referral-phase ability widening can never leak invitee emails (PII). (2)create()rejects 422 "You cannot invite yourself" before the registered-email check; the same-account grant-side floor is pinned in billing unit tests (alias/multi-account self-invites are a documented accepted residual, out of scope). (3) docs: referral rewards require closed signup — on open-signup deployments a token is resolved but never claimed/finalized (P2 invariant preserved, NOT changed).Scope
modules/invitations(repository, service, controller),modules/billing(referral.service unit tests)nonelowValidation
npm run lintnpm testGuardrails check
.env*,secrets/**, keys, tokens)Notes for reviewers
list()scope fix is the primary security surface — non-admin callers previously could enumerate all invitations platform-wide. Now scoped toinvitedBy: callerId. The self-invite guard fires at 422 before any DB read.Summary by CodeRabbit
New Features
Documentation
Tests