Repository navigation
Fixed system owner invitation flow - #52
Conversation
📝 WalkthroughWalkthroughPermission checks for system invitation endpoints change from 'invite' to 'create'. System invitations now use the system tenant ID and include request headers when creating invitations. invitationId validation is relaxed to non-empty string. Admin invitations route added; invite dialog and signup redirect logic updated for admin/system permissions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebApp
participant API
participant AuthProvider
participant TenantService
User->>WebApp: Submit invite (admin context)
WebApp->>API: POST /admin/invitations (includes headers)
API->>TenantService: getRequiredSystemTenantId()
TenantService-->>API: systemTenantId
API->>AuthProvider: createInvitation(payload, organizationId=systemTenantId, headers)
AuthProvider-->>API: InvitationCreated
API-->>WebApp: 201 Created
WebApp-->>User: show success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/api/src/modules/identity/system-admin/system-admin.controller.ts (2)
73-79:⚠️ Potential issue | 🟠 MajorInconsistency:
inviteUserstill usesorganizationId: nulland omitsheaders, unlikecreateSystemInvitation.
createSystemInvitation(line 103) was updated to usegetRequiredSystemTenantId()and passheaders: webHeaders(marked as "Required by Better Auth"). However,inviteUserstill passesorganizationId: nulland omitsheadersentirely. If Better Auth requires headers for invitation creation, this endpoint will fail at runtime.Either align both endpoints or document why they intentionally differ.
🐛 Proposed fix to align inviteUser with createSystemInvitation
// Create System Invitation (OrgId = null) await this.authProvider.createInvitation({ email: user.email, role: getRequiredAdminRoleId(), - organizationId: null, // System Invite + organizationId: getRequiredSystemTenantId(), // System Invite inviterId: session.user.id, + headers: webHeaders, // Required by Better Auth });
111-119:⚠️ Potential issue | 🟡 Minor
toWebHeaderstype mismatch with NestJS@Headers()decorator.NestJS's
@Headers()returnsRecord<string, string | string[]>(headers with duplicate keys are arrays), but the parameter is typed asRecord<string, string>. While NestJS typically collapses values, if an array-valued header slips through,webHeaders.append(key, value)would calltoString()on the array, producing"a,b"instead of properly appending each value.apps/api/src/modules/identity/system-admin/system-admin.controller.spec.ts (1)
394-399:⚠️ Potential issue | 🟡 MinorUpdate this test if
inviteUseris aligned withcreateSystemInvitation.This test expects
organizationId: nulland noheadersforinviteUser, matching the current controller code. IfinviteUseris updated to usegetRequiredSystemTenantId()and pass headers (as suggested in the controller review), this assertion must be updated accordingly.
🤖 Fix all issues with AI agents
In `@apps/web/src/modules/identity/users/InviteUserDialog.tsx`:
- Around line 93-99: In InviteUserDialog, stop using window.location.pathname to
decide the endpoint; instead derive isAdminContext from the existing resource
prop (or the already-derived scope) and use that to pick the create(...)
resource; e.g. compute isAdminContext = resource?.startsWith("admin") (or check
scope === "admin") and then set the create call's resource to isAdminContext ?
"admin/invitations" : "invitations" so the endpoint selection uses the same
single source of truth as scope.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/modules/identity/users/InviteUserDialog.tsx`:
- Around line 92-99: Hoist a single const isAdminContext = resource ===
"admin/users" at the component scope and use that variable both where scope is
derived (instead of repeating the comparison on line 53) and inside the onSubmit
function to choose the create() resource (admin/invitations vs invitations);
update scope derivation to reference isAdminContext and remove the duplicated
resource === "admin/users" check in onSubmit to keep the logic DRY.
| const onSubmit = (data: InviteUserFormValues) => { | ||
| // Detect if we're in admin context (System Owner) | ||
| const isAdminContext = resource === "admin/users"; | ||
|
|
||
| create( | ||
| { | ||
| resource: "invitations", // Explicitly call invitations endpoint | ||
| // System Owner uses admin endpoint, Tenant Admin uses regular endpoint | ||
| resource: isAdminContext ? "admin/invitations" : "invitations", |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Good fix — correctly derives admin context from the resource prop now.
The change properly addresses the prior feedback by eliminating the window.location.pathname dependency. One small nit: the resource === "admin/users" comparison is already evaluated on line 53 to derive scope. You could hoist a single isAdminContext const at component level and reuse it for both scope and the endpoint selection to avoid the duplicated check.
♻️ Optional DRY improvement
- // Determine scope based on resource
- const scope = resource === "admin/users" ? "system" : "organization";
+ const isAdminContext = resource === "admin/users";
+ const scope = isAdminContext ? "system" : "organization";Then in onSubmit, drop the local re-derivation:
const onSubmit = (data: InviteUserFormValues) => {
- // Detect if we're in admin context (System Owner)
- const isAdminContext = resource === "admin/users";
-
create(
{
// System Owner uses admin endpoint, Tenant Admin uses regular endpoint
resource: isAdminContext ? "admin/invitations" : "invitations",🤖 Prompt for AI Agents
In `@apps/web/src/modules/identity/users/InviteUserDialog.tsx` around lines 92 -
99, Hoist a single const isAdminContext = resource === "admin/users" at the
component scope and use that variable both where scope is derived (instead of
repeating the comparison on line 53) and inside the onSubmit function to choose
the create() resource (admin/invitations vs invitations); update scope
derivation to reference isAdminContext and remove the duplicated resource ===
"admin/users" check in onSubmit to keep the logic DRY.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Improvements