feat(organizations): org.addMember(userId) + consent flow + source discriminator (P5a)#3823
Merged
Merged
Conversation
Add a consent-safe owner-add flow replacing the deleted org email-invite:
an owner/admin adds an existing user, creating a PENDING owner_add membership
the INVITED USER must accept — the owner can never approve it.
- model: new `source` enum {join_request, owner_add} with NO default + a
pre('validate') guard that throws if a PENDING row has no source (a forgotten
source must fail loudly, never become an owner-approvable join request);
+optional addedBy (audit). Hook throws (no next arg) — the function(next)
signature was invoked with no next under doc.validate() at runtime.
- constants: PENDING_SOURCES.
- service: addMember (status set EXPLICITLY to PENDING, rejects any existing
membership for (user,org), last-owner-safe by construction) +
acceptMembership (flips PENDING->ACTIVE only for a source:owner_add membership
whose userId is the caller). createJoinRequest single-pending rule now
source-scoped to join_request (owner_add and join_request are orthogonal,
both directions). +listPendingOwnerAddsByUser, +findUserByExactEmail.
- E16 audit: approval surface (listPending, listPendingByUser, requestByID
approve/reject gate) scoped to source:join_request with an E17
$exists:false legacy fallback so an owner_add is invisible to the owner.
Member-count aggregation stays ACTIVE-only. billing: no coupling (confirmed).
auth pendingRequests shape unchanged (still the user's own join requests).
- routes: POST /organizations/:id/members (owner/admin add),
GET /organizations/:id/members/search?email= (exact-email lookup, owner/admin,
GDPR no fuzzy enumeration), GET /membership-requests/mine/pending (owner_add
invitations), PUT /membership-requests/:membershipId/accept (invited user,
auth-only, consent gate in service).
- migration 20260610140000: backfill source=join_request on existing PENDING
rows (idempotent, raw driver). +MIGRATIONS.md (E17 ordering note for P9).
- tests: consent service/model/controller + migration (+23/+6/+9/+4 new);
full repo suite 2345 green, lint clean.
…able predicate Phase-5a review hardening (Approved-with-minors), 3 fixes: - acceptMembership: add self-defending early guard (!acceptingUserId -> null) so the consent identity-compare never leans on the auth-only route / sentinel-collides on String(undefined). + unit test. - isOwnerApprovable(source) predicate co-located with PENDING_SOURCES as the single source of truth for consent invariant #1; controller requestByID now calls it instead of an inline literal. joinRequestSourceFilter keeps the $or Mongo query (DB layer) with a cross-reference comment to the doc-level twin. - findUserByExactEmail JSDoc: clarify exact-after-normalization (lowercased/ trimmed, matches the CI-unique-email storage), not byte-exact. No behaviour change. Full suite green (unit 1891, integration 444), lint clean.
|
Warning Review limit reached
More reviews will be available in 20 minutes and 35 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ 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 (15)
✨ 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 |
There was a problem hiding this comment.
Pull request overview
Implements Phase 5a of the invitations↔org decouple work by adding a consent-first “owner adds an existing user” flow for organization membership, using a source discriminator to prevent any non-consented membership from becoming ACTIVE via owner-approval paths.
Changes:
- Added
membership.source(join_requestvsowner_add) with a model-level pre-validate guard, plus an E17 migration + temporary legacy read fallback. - Introduced the owner-add consent flow:
addMember(...)creates a PENDINGowner_add, andacceptMembership(...)allows only the invitee to activate it. - Added new endpoints for add-member, exact-email user lookup, invitee acceptance, and “my pending owner-add invitations”, with comprehensive unit/integration coverage.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| modules/organizations/lib/constants.js | Adds PENDING_SOURCES and isOwnerApprovable() predicate for consent gating. |
| modules/organizations/models/organizations.membership.model.mongoose.js | Adds source + addedBy fields and enforces “PENDING requires source” via pre-validate hook. |
| modules/organizations/services/organizations.membership.service.js | Scopes join-request surfaces, adds addMember, acceptMembership, findUserByExactEmail, and owner-add pending listing. |
| modules/organizations/controllers/organizations.membershipRequest.controller.js | Adds “my pending owner-adds” listing + accept endpoint; gates approve/reject param middleware via isOwnerApprovable. |
| modules/organizations/controllers/organizations.membership.controller.js | Adds org-level add-member endpoint + exact-email lookup endpoint with explicit role gating. |
| modules/organizations/routes/organizations.membership.routes.js | Adds POST /members and GET /members/search routes with validation. |
| modules/organizations/routes/organizations.membershipRequest.routes.js | Adds GET /membership-requests/mine/pending and PUT /membership-requests/:id/accept auth-only routes. |
| modules/organizations/models/organizations.membership.schema.js | Adds Zod schema for add-member payload validation. |
| modules/organizations/migrations/20260610140000-backfill-membership-source.js | Backfills source:'join_request' on legacy PENDING rows (idempotent). |
| modules/organizations/tests/organizations.membership.consent.unit.tests.js | Unit tests for consent invariants, source scoping, and exact-email lookup. |
| modules/organizations/tests/organizations.membershipRequest.controller.unit.tests.js | Unit tests ensuring owner-add PENDING is invisible to owner approval gate, and accept/listMinePending behavior. |
| modules/organizations/tests/organizations.membership.controller.unit.tests.js | Extends controller unit tests for add-member and exact-email lookup authorization. |
| modules/organizations/tests/organizations.membership.model.consent.integration.tests.js | Integration tests for model pre-validate consent guard + default status behavior. |
| modules/organizations/tests/organizations.backfillMembershipSource.migration.integration.tests.js | Integration tests validating migration behavior and idempotency via raw driver. |
| MIGRATIONS.md | Documents the consent split, new endpoints, and E17 migration ordering requirement. |
Comment on lines
+45
to
+50
| * E17: a legacy PENDING with no `source` (pre-backfill, all join_requests) is | ||
| * owner-approvable — `!== OWNER_ADD` covers both join_request and absent. | ||
| * @param {String} [source] - The membership's PENDING source discriminator. | ||
| * @returns {Boolean} True if an owner/admin may approve this PENDING membership. | ||
| */ | ||
| export const isOwnerApprovable = (source) => source !== PENDING_SOURCES.OWNER_ADD; |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3823 +/- ##
==========================================
+ Coverage 91.55% 91.80% +0.25%
==========================================
Files 159 159
Lines 5104 5187 +83
Branches 1624 1652 +28
==========================================
+ Hits 4673 4762 +89
+ Misses 339 337 -2
+ Partials 92 88 -4
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:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 5a — invitations ↔ org decouple (epic #3808)
Adds the "(2) add an existing user to my org" half of the new 2-step flow, consent-first: an owner adds a user → a
PENDINGmembership the invited user must accept. An owner-add can NEVER become ACTIVE without the invitee's consent.Consent design
source: 'join_request' | 'owner_add'discriminator on membership (NO default + apre('validate')hook rejecting a sourceless PENDING — uses the lowercaseMEMBERSHIP_STATUSES.PENDINGconstant, not the inert'PENDING'literal).addMember(orgId, userId, role, addedBy)→{ status: PENDING (explicit — model defaults to active), source: 'owner_add' }; duplicate-guarded (any existing membership rejects), last-owner-safe, role-escalation gated (only owner/global-admin grants owner/admin).acceptMembership(membershipId, acceptingUserId)→ PENDING→ACTIVE ONLY whensource === 'owner_add'ANDuserId === acceptingUserId(String-coerced ObjectId compare + a self-defendingif (!acceptingUserId) return nullguard). The owner can't accept on the invitee's behalf.isOwnerApprovable(source)predicate (single source of truth) +joinRequestSourceFilteron all join-request listings. Two ACTIVE-mutation paths exist (approveRequest,acceptMembership); both gated; no third path (verified incl.updateRolecan't back-door activate).E16 audit + E17 ordering
Every PENDING query reviewed (organizations + auth + billing): join-request surfaces →
join_requestonly;createJoinRequestsingle-pending rule source-scoped (owner_add ⊥ join_request, both directions tested); auth payloadpendingRequestsunchanged (still the user's join-requests); member-count stays ACTIVE-only; billing has no coupling (confirmed). E17 defensive read ($or:[{source:'join_request'},{source:{$exists:false}}]) keeps legacy sourceless PENDING visible pre-backfill.New endpoints (for P5b)
POST /api/organizations/:organizationId/members(owner/admin) — add-member.PUT /api/membership-requests/:membershipId/accept(auth-only, consent gate in service — not under/membersCASL so a not-yet-member isn't blocked; param binding can't cross-wirerequestByID).GET /api/organizations/:organizationId/members/search?email=— exact-email lookup (owner/admin gated, GDPR: no fuzzy enumeration).GET /api/membership-requests/mine/pending— the user's own owner_add pendings.Migration
…140000-backfill-membership-source.js(after P4's…130000): setssource:'join_request'on legacy PENDING (raw driver, idempotent, no-overwrite). MIGRATIONS.md documents the E17 ordering (P9 runs backfill on Trawl BEFORE the source-filtering code deploys).Verification
Notes
addMemberTOCTOU duplicate-check is backstopped by the(userId, organizationId)partial-unique index (worst case: clean 422, never a double-pending row).Closes #3813. Part of #3808.