Skip to content

feat: unify member invitations across editions in core#6631

Merged
gustavosbarreto merged 1 commit into
masterfrom
feature/member-invite-link
Jul 10, 2026
Merged

feat: unify member invitations across editions in core#6631
gustavosbarreto merged 1 commit into
masterfrom
feature/member-invite-link

Conversation

@gustavosbarreto

@gustavosbarreto gustavosbarreto commented Jul 10, 2026

Copy link
Copy Markdown
Member

The add-member flow now behaves the same in every edition. Adding a member by email always creates a membership invitation with a one-time signature and returns an /accept-invite link. The invitee follows it to set up their account, or to accept if they already have one. Cloud also emails the link. On self-hosted Enterprise an existing account is added directly, and an invite from a non-superadmin completes into an account awaiting a system admin's approval.

Until now the invitation subsystem lived entirely in the cloud repo, so a Community API lacked those endpoints and the UI fell back to a separate activation-token onboarding path. This moves the invitation spine into core, so all three editions share one flow and one AddMemberDrawer.

What changed:

  • Invitation models, store, services, and routes now live in core, with a new migration 011 that carries the signature and expiry on the membership_invitations row, replacing the Redis activation token.
  • Registration (POST /api/users) lands in core. Open self-registration stays gated to Cloud; Community and Enterprise are invite-only.
  • The activation-token path is retired: the /activate route, the activation-link dialog, and the account-request tab are gone.
  • Editions layer only add-ons on top through hooks: email delivery and the awaiting_approval gate.

The cloud-side counterpart is shellhub-io/cloud#2426.

@gustavosbarreto gustavosbarreto requested review from a team as code owners July 10, 2026 20:04
@gustavosbarreto gustavosbarreto force-pushed the feature/member-invite-link branch from 202699e to ad24a8b Compare July 10, 2026 20:52
@gustavosbarreto

Copy link
Copy Markdown
Member Author

/review

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown

Claude finished @gustavosbarreto's task in 10m 31s —— View job


Code Review Complete

Reviewed 100 files across code quality, security, testing, language patterns, and architecture with 5 specialized agents. 2 inline issues posted (1 high, 1 medium).

  • Highapi/routes/routes.go:149: the new GenerateInvitationLink route is missing RequiresPermission(authorizer.NamespaceAddMember). The service only checks HasAuthority (role rank), so an Observer/Operator can add members through the console's primary add-member flow, bypassing the restriction enforced on the classic AddNamespaceMember route.
  • Mediumapi/services/register.go:101: createInvitedUser discards the HashUserPassword error while its sibling createNewUser checks it; a hashing failure would silently persist an invalid password hash.

Additional notes

  • Hardening (not blocked): In api/services/register.go:30-44, an unauthenticated registration with Sig == "" binds a pending user_invitation by the submitted email before the invite-only gate. This is documented as intentional fallback, but it lets someone who knows an invited email pre-consume that invitation (squatting/lockout — no namespace access is gained). Worth considering requiring the sig, or moving the openSignupAllowed() gate ahead of the email match, for invite-only editions.
  • Cross-repo: Verified against the cloud/ checkout — the store/service interface expansion, the OnMemberAddOnMembershipInvited hook rename, the new capability toggles, and the removed activation-token symbols all line up with cloud#2426; no compile breaks. Migration 011 up/down is symmetric (the old activation token lived in Redis, so there's no in-table data to migrate). Test coverage for signature/expiry/one-time-consumption paths is solid.

To request another review round, comment /review.
· feature/member-invite-link

Comment thread api/routes/routes.go Outdated
Comment thread api/services/register.go Outdated
@gustavosbarreto gustavosbarreto force-pushed the feature/member-invite-link branch from ad24a8b to 51863f1 Compare July 10, 2026 21:10
@gustavosbarreto

Copy link
Copy Markdown
Member Author

Migration 011 made idempotent for the Postgres upgrade path

Follow-up to a cross-repo note on shellhub-io/cloud#2426. Postgres is already in production, and the last stable release (v0.26.0) created membership_invitations, user_invitations, and the membership_invitation_status enum via the enterprise (cloud) 001 migration. Since the invitation spine moved into core, migration 011 re-creates those objects, which would collide (already exists) on any enterprise database upgrading from v0.26.0, and the pre-existing table also lacks the new sig column.

011 up is now idempotent and reconciling:

  • CREATE TYPE guarded by a DO / EXCEPTION duplicate_object block; tables and indexes use IF NOT EXISTS.
  • ALTER TABLE ... ADD COLUMN IF NOT EXISTS sig plus the partial unique index add the new column to pre-existing enterprise tables (no-op on fresh installs, which already create it).

Fresh Community installs create the full schema as before. Verified by replaying the exact v0.26.0 schema (seeded with a pending invitation) then applying 011: no error, sig column + sig_unique index + cleanup trigger added, seeded row preserved, and a second apply is a clean no-op.

/review

@gustavosbarreto gustavosbarreto force-pushed the feature/member-invite-link branch from 51863f1 to f032020 Compare July 10, 2026 21:52
@gustavosbarreto

Copy link
Copy Markdown
Member Author

Hardened the invite-only registration path (addresses the squatting note)

RegisterUser no longer matches a pending invitation by the submitted email before the invite-only gate. A valid Sig (from the invite link) is now the only way to complete an invited account; the email-match path runs only under open self-registration (Cloud). Community and enterprise refuse a no-sig registration up front, so someone who merely knows an invited email can no longer pre-consume that invitation and lock the real invitee out.

Added a regression test (refuses a no-sig registration on invite-only editions) and a case covering the email-match completion under open signup.

/review

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown

Claude finished @gustavosbarreto's task in 11m 51s —— View job


Code Review Complete

Reviewed 100 files across code quality, security, testing, language patterns, and architecture with 5 specialized agents, then validated each finding with independent sub-agents. 3 inline issues posted (1 high, 2 medium).

  • Highui/apps/console/src/pages/team/MembersTab.tsx:272: the "Regenerate link" action is offered for every pending invitation, but GenerateInvitationLink 409s on a still-valid (pending, non-expired) invite — which is the common case given the 7-day expiry. The resendEnabled() gating from the deleted InvitationsTab.tsx was dropped in the merge.
  • Mediumapi/services/invitation.go:101: AcceptInvite runs NamespaceCreateMembership + MembershipInvitationDelete without a transaction; a failure between them adds the member but leaves the invitation stuck pending. The sibling createInvitedUser in this same PR wraps the identical pair in WithTransaction.
  • Mediumui/apps/console/src/pages/AcceptInvite.tsx:187: handleEnterNamespace doesn't catch a switchNamespace failure (no try/catch, no hook onError), so a token-fetch error becomes an unhandled rejection with no user feedback — a regression from the previously-guarded accept+switch flow.

Additional notes

  • Prior fixes verified. The two issues from earlier rounds are correctly resolved: GenerateInvitationLink now carries RequiresPermission(NamespaceAddMember) (routes.go:149), and createInvitedUser checks the HashUserPassword error (register.go:99-102).
  • Squatting hardening verified sound. RegisterUser now only matches an invitation by email after the !openSignupAllowed() gate, so invite-only editions (community/enterprise) reject a no-sig registration up front. Security review found no remaining bypass.
  • Cross-repo (cloud#2426) confirmed compatible. The OnMemberAddOnMembershipInvited hook rename, the expanded store/service interfaces, the new capability toggles, and the removed activation-token symbols all line up with the cloud/ checkout — no compile breaks. Migration 011 up/down is idempotent and reconciles pre-existing enterprise tables (adds only sig + partial unique index).
  • Noted, not flagged: the awaiting_approval gate (register.go:139-143) and the directMembershipAllowed() branch (invitation.go:158) are edition-specific paths with no direct unit coverage in register_test.go (the enterprise toggles are never set true in tests) — worth a follow-up test, but below the high-signal bar.

To request another review round, comment /review.
· feature/member-invite-link

Comment thread ui/apps/console/src/pages/team/MembersTab.tsx
Comment thread api/services/invitation.go Outdated
Comment thread ui/apps/console/src/pages/AcceptInvite.tsx
@gustavosbarreto gustavosbarreto force-pushed the feature/member-invite-link branch from f032020 to 44922cd Compare July 10, 2026 22:13
@gustavosbarreto

Copy link
Copy Markdown
Member Author

/review

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown

Code Review

  • Gathered PR context
  • Reviewing with 5 specialized agents
  • Posting feedback

@gustavosbarreto gustavosbarreto force-pushed the feature/member-invite-link branch from 44922cd to a668284 Compare July 10, 2026 22:19
@gustavosbarreto

Copy link
Copy Markdown
Member Author

Added unit coverage for the enterprise-gated paths

Follow-up to the coverage note. TestService_RegisterUser now has a case that enables nonAdminProvisioning and asserts AwaitingApproval is set when a non-admin invites (via mock.MatchedBy on the created user), and TestService_GenerateInvitationLink has a case that enables directMembership and asserts an existing account is added directly with no link returned. Both toggles are set per-case and reset via t.Cleanup.

/review

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown

Claude finished @gustavosbarreto's task in 16m 45s —— View job


Code Review Complete

Reviewed 100 files across code quality, security, testing, language patterns, and architecture with 5 specialized agents, then validated each surviving finding with independent sub-agents. 2 inline issues posted (1 high, 1 medium).

  • Highui/apps/console/src/pages/AcceptInvite.tsx:164: when signUp returns a token (confirmed account — superadmin invite / Cloud sig flow), the invited-completion branch sets branch: "joined" but never calls setSession, so authStore.token stays empty. "Go to Dashboard" → switchNamespacegetNamespaceToken then fires unauthenticated, 401s, and the interceptor forces a logout/redirect to /login. This PR inlined the flow that previously routed through /sign-upAccountCreated (which does call setSession) and dropped the session step; the test mocks useSwitchNamespace, so it isn't caught.
  • Mediumapi/services/invitation.go:230: buildInviteURL hardcodes http://. This link is surfaced verbatim as a copyable value in AddMemberDrawer and MembersTab, so on HTTPS deployments the copied invite link has the wrong scheme (fails to load / mixed-content). The package already has the X-Forwarded-Proto + default-https pattern in GetServerAddress. The same hardcode also exists in cloud/pkg/mail/templates/invite-member.go:22 (emailed link) — paired follow-up.

Additional notes

  • The round-4 test additions are sound. TestService_RegisterUser's nonAdminProvisioning case genuinely asserts AwaitingApproval via mock.MatchedBy (and correctly omits CreateUserToken mocks, so a token minted for an awaiting-approval user would fail the test), and TestService_GenerateInvitationLink's directMembership case genuinely forces the direct-add branch. Toggles are per-case and reset via t.Cleanup; no t.Parallel() in the package, so no data-race flakes.
  • All prior-round fixes verified intact: RequiresPermission(NamespaceAddMember) on the link route, HashUserPassword error handled in both create paths, the email-squatting gate (!openSignupAllowed() before email match), the regenerate-button gating, the AcceptInvite transaction, and the switchNamespace try/catch.
  • Cross-repo (cloud#2426) confirmed compatible: the OnMemberAddOnMembershipInvited rename, expanded store/service interfaces, new capability toggles, and removed activation-token symbols all line up with the cloud/ checkout — no compile breaks. Migration 011 up/down is idempotent and reconciles pre-existing enterprise tables (adds only sig + partial unique index).
  • Not flagged (below the high-signal bar): a few now-dead hooks left after the merge (useAddMember, useAdminAccountRequests/useRejectAccountRequest) and stale vi.mock scaffolding in AdminUsers.test.tsx referencing the deleted AccountRequestsTab — inert, no correctness impact, worth a cleanup pass.

To request another review round, comment /review.
· feature/member-invite-link

Comment thread ui/apps/console/src/pages/AcceptInvite.tsx Outdated
Comment thread api/services/invitation.go Outdated
@gustavosbarreto gustavosbarreto force-pushed the feature/member-invite-link branch 2 times, most recently from 6a995fe to 3f4e447 Compare July 10, 2026 23:03
@gustavosbarreto

Copy link
Copy Markdown
Member Author

/review

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown

Claude finished @gustavosbarreto's task in 13m 43s —— View job


Code Review Complete

Reviewed 100 files across code quality, security, testing, language patterns, and architecture with 5 specialized agents, then validated each surviving finding with an independent sub-agent. 2 inline issues posted (1 medium, 1 low).

  • Mediumapi/services/register.go:23-42: a non-empty but non-resolving Sig bypasses the email requirement. Email is required_without=Sig,omitempty,email, so any junk Sig + empty Email passes validation; when the Sig fails to resolve, the flow falls through to createNewUser on open-signup (Cloud) and durably persists a blank-email account (NOT NULL UNIQUE(email) doesn't reject ''). Invite-only editions are unaffected (refused at the gate).
  • Lowpkg/api/requests/invitation.go:20: GenerateInvitationLink.MemberRole is missing the member_role validator that NamespaceAddMember and every other role field use. A garbage role passes validation and the rank-only HasAuthority check, then hits the membership_role enum insert → generic 500 instead of a clean 400. Not a privilege-escalation (unknown role → zero permissions; owner still blocked).

Additional notes

  • All prior-round fixes verified intact. RequiresPermission(NamespaceAddMember) on the link route, HashUserPassword error handled in both create paths, the email-squatting gate (!openSignupAllowed() before email match), the regenerate-button gating, the AcceptInvite transaction, the switchNamespace try/catch, setSession on the joined branch, and the X-Forwarded-Proto-derived invite scheme are all present and correct.
  • Signature scheme sound. pkg/pairingcode uses crypto/rand with rejection sampling (~2⁵⁹ entropy behind the nginx rate zone); one-time consumption is enforced by deleting the invitation inside the same transaction as membership creation, and MembershipInvitationResolveBySig filters status = 'pending' AND expires_at > now(), so a consumed/expired sig can't be replayed.
  • Cross-repo (cloud#2426) confirmed compatible. The InviteMember proto threading (tenant:user:proto:host), OnMemberAddOnMembershipInvited rename, expanded store/service interfaces, capability toggles, and removed activation-token symbols all line up with the cloud/ checkout — no compile breaks. Migration 011 up/down is idempotent and reconciles pre-existing enterprise tables.
  • Not flagged (below the high-signal bar): GenerateInvitationLink's upsert-then-create sequence isn't wrapped in WithTransaction like its siblings, but UserInvitationsUpsert is idempotent so a partial failure leaves only a benign, self-healing placeholder row (no stuck-pending inconsistency). Two OpenAPI descriptions (paths/api@namespaces@{tenant}@members.yaml, components/schemas/memberView.yaml) still reference the now-deleted /api/users/{id}/activate activation flow — stale prose, worth a docs pass.

To request another review round, comment /review.
· feature/member-invite-link

Comment thread api/services/register.go
Comment thread pkg/api/requests/invitation.go Outdated
The add-member flow now works the same in every edition. Adding a
member by email always creates a membership invitation with a one-time
signature and returns an /accept-invite link; the invitee follows it
to set up their account or, if they already have one, to accept. Cloud
also emails the link. On self-hosted Enterprise an existing account is
added directly, and an invite from a non-superadmin completes into an
account awaiting a system admin's approval.

This moves the invitation spine (models, store, services, routes, and
registration) out of the cloud repo into core, so Community and
Enterprise no longer fall back to a separate onboarding path. Editions
layer only add-ons on top: email delivery and the approval gate.

The membership invitation carries its signature and expiry on the row
(migration 011), replacing the Redis activation token. The activation
token path and its /activate route, UI dialog, and account-request tab
are removed. Registration (POST /api/users) lands in core, with open
self-registration gated to Cloud.
@gustavosbarreto gustavosbarreto force-pushed the feature/member-invite-link branch from 3f4e447 to f893606 Compare July 10, 2026 23:22
@gustavosbarreto

Copy link
Copy Markdown
Member Author

/review

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown

Claude finished @gustavosbarreto's task in 7m 15s —— View job


Code Review Complete

Reviewed 100 files across code quality, security, testing, language patterns, and architecture with 5 specialized agents, then validated the surviving findings — no new issues found. The code looks good as-is.

Additional notes

  • All prior-round fixes verified intact at f893606. RequiresPermission(NamespaceAddMember) on the link route; HashUserPassword error handled in both create paths; the email-squatting gate (invited email always taken from the invitation, !openSignupAllowed() || req.Email == "" before the email match); the member_role validator on GenerateInvitationLink; the AcceptInvite/createInvitedUser transactions; the switchNamespace try/catch and setSession on the joined branch; and the X-Forwarded-Proto-derived invite scheme are all present and correct.
  • Security sound. pkg/pairingcode uses crypto/rand with rejection sampling (~2⁵⁹ over the nginx rate zone); one-time consumption is enforced by deleting the invitation in the same transaction as membership creation, and MembershipInvitationResolveBySig filters status = 'pending' AND expires_at > now(), so consumed/expired sigs can't be replayed. No injection, secrets, or broken access control introduced.
  • Cross-repo (cloud#2426) confirmed compatible. The OnMemberAddOnMembershipInvited rename, the InviteMember proto threading (tenant:user:proto:host, proto-before-host), the expanded store/service interfaces, the capability toggles, and the removed activation-token symbols all line up with the cloud/ checkout — no compile breaks. Migration 011 up/down is idempotent and reconciles pre-existing enterprise tables.
  • Below the high-signal bar (carried over, not re-flagged): the now-dead useAddMember hook and the parallel AddNamespaceMember path it fed, the unused MembershipInvitationStatusRejected constant, and the stale OpenAPI prose still referencing the deleted /api/users/{id}/activate flow — inert, worth a cleanup pass but no correctness impact.

To request another review round, comment /review.
· feature/member-invite-link

@gustavosbarreto gustavosbarreto merged commit 2041885 into master Jul 10, 2026
25 of 26 checks passed
@gustavosbarreto gustavosbarreto deleted the feature/member-invite-link branch July 10, 2026 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant