feat: protect organization owner role from demotion and removal#786
Conversation
Add first-class 'owner' role that is a superset of admin. The organization creator is now assigned 'owner' on creation with creatorId stored in org metadata. Owner cannot be demoted, removed, or manually assigned. Includes last-admin guard, isAdmin helper, UI protections, and migration script.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThis pull request introduces an "owner" role to the platform's role-based access control system. The changes add the owner role to role definitions, validators, and schemas across the codebase; create an Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 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)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/platform/convex/members/validators.ts (1)
13-20:⚠️ Potential issue | 🔴 CriticalSplit stored roles from assignable roles.
Adding
ownerhere widens the mutation args foraddMemberandupdateMemberRoleas well as the stored/return shapes.updateMemberRolerejects it later, butaddMemberstill persists the supplied role unchanged at Lines 74-85 inservices/platform/convex/members/mutations.ts, so admins/owners can still create additional owners through the create path. Please use a non-owner validator at the mutation boundary and add a regression test foraddMember({ role: 'owner' }).🛠 Proposed split
export const memberRoleValidator = v.union( v.literal('owner'), v.literal('disabled'), v.literal('member'), v.literal('editor'), v.literal('developer'), v.literal('admin'), ); + +export const assignableMemberRoleValidator = v.union( + v.literal('disabled'), + v.literal('member'), + v.literal('editor'), + v.literal('developer'), + v.literal('admin'), +);export const addMember = mutation({ args: { organizationId: v.string(), userId: v.string(), - role: v.optional(memberRoleValidator), + role: v.optional(assignableMemberRoleValidator), },export const updateMemberRole = mutation({ args: { memberId: v.string(), - role: memberRoleValidator, + role: assignableMemberRoleValidator, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/convex/members/validators.ts` around lines 13 - 20, The current memberRoleValidator includes 'owner', which expands mutation inputs; split validators so the DB/return type can still include 'owner' but mutation inputs cannot. Create a new assignableMemberRoleValidator (omit 'owner') and keep a storedMemberRoleValidator (include 'owner') for persisted/returned shapes; update the addMember and updateMemberRole mutation argument validation to use assignableMemberRoleValidator instead of memberRoleValidator, and ensure any DB write/return logic still uses storedMemberRoleValidator where appropriate. Add a regression test calling addMember({ role: 'owner' }) that asserts the mutation is rejected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@services/platform/app/features/organization/components/organization-form.tsx`:
- Line 65: Client-supplied metadata.creatorId is not validated server-side;
update the server-side organization creation handler that calls
authClient.organization.create() so it enforces the authenticated user as
creator: either overwrite metadata.creatorId with the authenticated session user
id (e.g., session.user.id / req.user.id) before persisting, or load the created
organization immediately and assert organization.metadata.creatorId ===
session.user.id and reject if not; follow the pattern from
send_message_via_email.ts (validate loaded resource against session) and ensure
the betterAuth creatorRole 'owner' behavior is paired with this server-side
enforcement.
In `@services/platform/convex/sso_providers/remove_sso_provider.ts`:
- Around line 30-31: Replace the inline role check in remove_sso_provider.ts
with the shared isAdmin() helper to keep behavior consistent and
case-insensitive: import isAdmin and use it to validate the caller (e.g., if
(!isAdmin(callerRole)) throw new Error('Only admins can remove SSO providers')),
updating the existing condition that currently checks callerRole !== 'admin' &&
callerRole !== 'owner'.
In `@services/platform/convex/sso_providers/upsert_sso_provider.ts`:
- Around line 46-47: The thrown Error message in upsert_sso_provider.ts is
inconsistent with the authorization check in the upsert SSO provider logic
(callerRole !== 'admin' && callerRole !== 'owner'); update the Error text thrown
(where the code currently throws "Only admins can configure SSO providers") to
reflect both allowed roles (e.g., "Only admins or owners can configure SSO
providers" or a neutral message like "Insufficient permissions to configure SSO
providers") so it matches the guard in the function that performs the role
check.
---
Outside diff comments:
In `@services/platform/convex/members/validators.ts`:
- Around line 13-20: The current memberRoleValidator includes 'owner', which
expands mutation inputs; split validators so the DB/return type can still
include 'owner' but mutation inputs cannot. Create a new
assignableMemberRoleValidator (omit 'owner') and keep a
storedMemberRoleValidator (include 'owner') for persisted/returned shapes;
update the addMember and updateMemberRole mutation argument validation to use
assignableMemberRoleValidator instead of memberRoleValidator, and ensure any DB
write/return logic still uses storedMemberRoleValidator where appropriate. Add a
regression test calling addMember({ role: 'owner' }) that asserts the mutation
is rejected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b9a7e733-1e33-4772-8688-3ffdb9d878f7
⛔ Files ignored due to path filters (1)
services/platform/convex/migrations/migrate_org_creators.tsis excluded by!**/migrations/**
📒 Files selected for processing (26)
services/platform/app/features/organization/components/organization-form.tsxservices/platform/app/features/settings/organization/components/member-row-actions.tsxservices/platform/convex/auth.tsservices/platform/convex/lib/rls/context/create_rls_context.tsservices/platform/convex/lib/rls/helpers/access_control.test.tsservices/platform/convex/lib/rls/helpers/access_control.tsservices/platform/convex/lib/rls/helpers/role_helpers.test.tsservices/platform/convex/lib/rls/helpers/role_helpers.tsservices/platform/convex/lib/rls/organization/get_user_organizations.tsservices/platform/convex/lib/rls/organization/validate_organization_access.tsservices/platform/convex/members/__tests__/mutations.test.tsservices/platform/convex/members/__tests__/queries.test.tsservices/platform/convex/members/mutations.tsservices/platform/convex/members/queries.tsservices/platform/convex/members/validators.tsservices/platform/convex/organizations/validators.tsservices/platform/convex/sso_providers/get_sso_credentials_for_email.tsservices/platform/convex/sso_providers/remove_sso_provider.tsservices/platform/convex/sso_providers/upsert_sso_provider.tsservices/platform/convex/team_members/mutations.tsservices/platform/convex/users/create_member.tsservices/platform/convex/users/set_member_password.tsservices/platform/lib/auth-client.tsservices/platform/lib/shared/schemas/organizations.tsservices/platform/lib/utils/badge-colors.tsservices/platform/messages/en.json
| const result = await authClient.organization.create({ | ||
| name: data.name.trim(), | ||
| slug: slug, | ||
| metadata: { creatorId: user.id }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find all creatorId usage =="
rg -n -C4 '\bcreatorId\b'
echo "== Find organization create + creator role wiring =="
rg -n -C6 'organization\.create|creatorRole|getAuthOptions|metadata'
echo "== Locate likely auth/org config files =="
fd 'auth\.ts$|auth-client\.ts$|organization-form\.tsx$|migrate_org_creators\.ts$' -t fRepository: tale-project/tale
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find organization creation in betterAuth adapter =="
fd 'adapter\.ts' services/platform/convex -type f | head -5
echo "== Search for organization.create implementation =="
rg -l 'organization\.create|createOrganization' services/platform/convex -t ts
echo "== Look at betterAuth adapter file directly =="
fd 'adapter\.ts' services/platform/convex/betterAuth -type f -exec wc -l {} \;Repository: tale-project/tale
Length of output: 303
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find adapter file in betterAuth =="
find services/platform/convex -name "adapter.ts" -o -name "*.ts" -path "*betterAuth*" | head -20
echo "== Search for organization creation mutations =="
rg -l 'organization.*create|createOrganization' services/platform/convex --type ts | grep -v node_modules | head -10
echo "== Look for organization schema or mutations =="
rg 'organization.*create' services/platform/convex -A 3 -B 1 | head -100Repository: tale-project/tale
Length of output: 5974
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Read create_organization.ts =="
cat -n services/platform/convex/organizations/create_organization.ts
echo ""
echo "== Read betterAuth adapter.ts (first 150 lines) =="
head -n 150 services/platform/convex/betterAuth/adapter.ts
echo ""
echo "== Search for organization create in adapter =="
rg -n 'organization.*create|createOrganization' services/platform/convex/betterAuth/adapter.ts -A 10 -B 2Repository: tale-project/tale
Length of output: 1609
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Read betterAuth schema.ts to see organization table =="
cat -n services/platform/convex/betterAuth/schema.ts | grep -A 20 'organization'
echo ""
echo "== Search for organization hooks or callbacks in auth.ts =="
rg -n 'organization|createOrganization' services/platform/convex/betterAuth/auth.ts -A 5 -B 2
echo ""
echo "== Check if there are post-create hooks for organization =="
rg -n 'organization.create|onOrganizationCreate|onCreateOrganization' services/platform/convex -A 10 -B 2 | head -80Repository: tale-project/tale
Length of output: 707
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Read full betterAuth schema.ts =="
cat -n services/platform/convex/betterAuth/schema.ts
echo ""
echo "== Read betterAuth auth.ts to find getAuthOptions =="
wc -l services/platform/convex/betterAuth/auth.ts
echo ""
echo "== Check auth.ts for organization or metadata handling =="
rg -n 'organization|metadata|creatorId' services/platform/convex/betterAuth/auth.tsRepository: tale-project/tale
Length of output: 1391
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Read full betterAuth auth.ts =="
cat -n services/platform/convex/betterAuth/auth.ts
echo ""
echo "== Read generated_schema.ts to see organization table definition =="
grep -n -A 20 'organization' services/platform/convex/betterAuth/generated_schema.ts | head -60
echo ""
echo "== Check if metadata is a standard field in betterAuth organization =="
rg -n 'metadata' services/platform/convex/betterAuth/generated_schema.tsRepository: tale-project/tale
Length of output: 3025
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Look for server-side organization creation handlers =="
find services/platform/convex -name "*.ts" -exec grep -l 'organization.*create\|betterAuth.*create' {} \; | xargs -I {} grep -n 'export.*create\|function.*create' {} | head -20
echo ""
echo "== Search for any hooks that might validate creatorId =="
rg -n 'creatorId.*equal|creatorId.*auth|userId.*creatorId' services/platform/convex --type ts | head -20
echo ""
echo "== Check how organization metadata is handled in migration =="
cat -n services/platform/convex/migrations/migrate_org_creators.ts | sed -n '100,125p'Repository: tale-project/tale
Length of output: 720
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check migration to understand full flow =="
cat -n services/platform/convex/migrations/migrate_org_creators.ts | sed -n '95,120p'
echo ""
echo "== Search for any server-side checks on creatorId =="
rg 'creatorId' services/platform/convex --type ts -A 2 -B 2 | grep -A 3 -B 3 'validate\|check\|equal\|match' | head -30
echo ""
echo "== Check if betterAuth has hooks for organization create =="
rg 'organization' services/platform/convex/betterAuth --type ts | grep -i 'hook\|before\|after\|onCreateOrganization'Repository: tale-project/tale
Length of output: 1154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for auth options configuration that might validate metadata =="
grep -r "organization" services/platform/convex --include="*.ts" | grep -i "options\|config\|plugin" | head -20
echo ""
echo "== Check auth creation logic =="
find services/platform/convex -name "auth.ts" -o -name "auth-*.ts" -o -name "*-auth.ts" | xargs cat 2>/dev/null | head -100
echo ""
echo "== Look for createAuth function definition =="
rg -n 'function createAuth|export.*createAuth' services/platform/convex --type ts -A 20 | head -100Repository: tale-project/tale
Length of output: 6365
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Read auth.ts around createAuth and getAuthOptions =="
wc -l services/platform/convex/auth.ts
echo ""
echo "== Search for getAuthOptions function =="
rg -n 'function getAuthOptions|export.*getAuthOptions' services/platform/convex/auth.ts -A 100 | head -150
echo ""
echo "== Look specifically for organization plugin config =="
rg -n 'organization.*plugin|organization.*{' services/platform/convex/auth.ts -A 5 -B 2 | head -80Repository: tale-project/tale
Length of output: 3042
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for any RLS or validation on organization metadata =="
rg -n 'organization.*metadata|metadata.*creatorId' services/platform/convex --type ts -B 2 -A 2
echo ""
echo "== Check if there are any before/after hooks in betterAuth organization plugin =="
rg -n 'hook|before|after' services/platform/convex/auth.ts | grep -i 'organization\|org'
echo ""
echo "== Look for where organization metadata is actually processed on creation =="
rg -n 'metadata.*JSON\|JSON.*metadata' services/platform/convex --type ts | head -20Repository: tale-project/tale
Length of output: 2537
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check if there's any server-side interception of organization.create =="
rg -n 'organization.create|createOrganization' services/platform --type ts | grep -v 'deprecated\|client\|authClient' | head -20
echo ""
echo "== Look for hooks that might run after organization creation =="
rg -n 'after.*create|onCreate|onOrganization' services/platform/convex --type ts | head -20
echo ""
echo "== Verify: does client metadata object get serialized to string by betterAuth? =="
rg -n 'JSON.stringify' services/platform/app/features/organization --type tsx -B 3 -A 3Repository: tale-project/tale
Length of output: 2180
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check organization-form.tsx to see how metadata is passed =="
cat -n services/platform/app/features/organization/components/organization-form.tsx | sed -n '55,75p'
echo ""
echo "== Search for betterAuth client usage patterns with metadata =="
rg -n 'authClient.organization.create' services/platform/app --type ts -A 5 -B 5 | head -40
echo ""
echo "== Check if metadata object is serialized anywhere in the form =="
grep -n 'metadata\|creatorId' services/platform/app/features/organization/components/organization-form.tsxRepository: tale-project/tale
Length of output: 3367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check betterAuth betterAuth plugin source/docs =="
rg -n 'creatorRole\|creatorId' services/platform/convex --type ts -B 3 -A 3 | head -40
echo ""
echo "== Check if betterAuth plugin config has any hooks for organization creation =="
cat -n services/platform/convex/auth.ts | sed -n '264,276p'
echo ""
echo "== Verify: is user.id from the client context or authenticated user? =="
grep -n 'user\.id\|getUser\|authUser' services/platform/app/features/organization/components/organization-form.tsx | head -10Repository: tale-project/tale
Length of output: 113
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Get context for user variable in organization-form.tsx =="
head -n 40 services/platform/app/features/organization/components/organization-form.tsx
echo ""
echo "== Check where user session comes from =="
rg -n 'useSession\|user\s*=' services/platform/app/features/organization/components/organization-form.tsx | head -10
echo ""
echo "== Check the full file to understand user context =="
wc -l services/platform/app/features/organization/components/organization-form.tsxRepository: tale-project/tale
Length of output: 1522
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find useAuth hook implementation =="
find services/platform/app -name "*use-convex-auth*" -o -name "*auth*" -type f | head -5
echo ""
echo "== Check if user.id is from authenticated session =="
cat services/platform/app/hooks/use-convex-auth.ts 2>/dev/null || find services/platform/app -path "*hooks*" -name "*auth*" | head -5
echo ""
echo "== Check betterAuth organization plugin to see if it validates metadata =="
rg -n 'creatorRole' services/platform/convex/auth.ts -B 2 -A 2Repository: tale-project/tale
Length of output: 1421
Server-side validation of metadata.creatorId is missing.
Line 65 sends metadata.creatorId: user.id to the client-side authClient.organization.create() call. Although user.id is sourced from the authenticated session, the server does not explicitly validate or enforce that the incoming metadata.creatorId equals the authenticated user before persisting it. This allows a tampered request to forge creator attribution.
The betterAuth organization plugin is configured with creatorRole: 'owner' (which auto-assigns the creator as owner), but this does not validate metadata integrity. Add server-side validation immediately after organization creation—either by overwriting metadata.creatorId with the authenticated user ID or by validating it matches before persistence, following the pattern established in send_message_via_email.ts where organizationId is validated against the loaded conversation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/organization/components/organization-form.tsx`
at line 65, Client-supplied metadata.creatorId is not validated server-side;
update the server-side organization creation handler that calls
authClient.organization.create() so it enforces the authenticated user as
creator: either overwrite metadata.creatorId with the authenticated session user
id (e.g., session.user.id / req.user.id) before persisting, or load the created
organization immediately and assert organization.metadata.creatorId ===
session.user.id and reject if not; follow the pattern from
send_message_via_email.ts (validate loaded resource against session) and ensure
the betterAuth creatorRole 'owner' behavior is paired with this server-side
enforcement.
| if (callerRole !== 'admin' && callerRole !== 'owner') { | ||
| throw new Error('Only admins can remove SSO providers'); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use isAdmin() helper for consistency.
Other files in this PR use the centralized isAdmin() helper for admin/owner checks. This inline comparison is inconsistent and lacks the case-insensitivity provided by isAdmin().
♻️ Proposed fix
Add the import:
import { GenericActionCtx } from 'convex/server';
import { internal } from '../_generated/api';
import { DataModel } from '../_generated/dataModel';
+import { isAdmin } from '../lib/rls/helpers/role_helpers';Then update the check:
- if (callerRole !== 'admin' && callerRole !== 'owner') {
+ if (!isAdmin(callerRole)) {
throw new Error('Only admins can remove SSO providers');
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/sso_providers/remove_sso_provider.ts` around lines
30 - 31, Replace the inline role check in remove_sso_provider.ts with the shared
isAdmin() helper to keep behavior consistent and case-insensitive: import
isAdmin and use it to validate the caller (e.g., if (!isAdmin(callerRole)) throw
new Error('Only admins can remove SSO providers')), updating the existing
condition that currently checks callerRole !== 'admin' && callerRole !==
'owner'.
| if (callerRole !== 'admin' && callerRole !== 'owner') { | ||
| throw new Error('Only admins can configure SSO providers'); |
There was a problem hiding this comment.
Authorization error message is now inaccurate.
Line 47 says only admins can configure SSO providers, but Line 46 explicitly allows owners too.
✏️ Suggested text fix
- throw new Error('Only admins can configure SSO providers');
+ throw new Error('Only admins or owners can configure SSO providers');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (callerRole !== 'admin' && callerRole !== 'owner') { | |
| throw new Error('Only admins can configure SSO providers'); | |
| if (callerRole !== 'admin' && callerRole !== 'owner') { | |
| throw new Error('Only admins or owners can configure SSO providers'); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/sso_providers/upsert_sso_provider.ts` around lines
46 - 47, The thrown Error message in upsert_sso_provider.ts is inconsistent with
the authorization check in the upsert SSO provider logic (callerRole !== 'admin'
&& callerRole !== 'owner'); update the Error text thrown (where the code
currently throws "Only admins can configure SSO providers") to reflect both
allowed roles (e.g., "Only admins or owners can configure SSO providers" or a
neutral message like "Insufficient permissions to configure SSO providers") so
it matches the guard in the function that performs the role check.
- Add transferOwnership mutation: only the owner can transfer ownership to another member (atomically promotes target and demotes caller) - Decouple SSO platformRoleValidator from memberRoleValidator to prevent 'owner' from leaking into SSO role mapping types - Update role-guards test: 'owner' is now a valid member role - Include generated api.d.ts with role_helpers export - Add transfer ownership dialog and member row action (owner-only) - Add translations for ownership transfer
- Fix user.id → user.userId in organization-form.tsx - Add 'owner' to RLS rules prefetched role type via MemberRole import - Fix callerMember possibly undefined in addMember audit log - Add explicit type annotation for filter callback in updateMemberRole - Simplify migration record parsing to avoid undefined type errors
Address CodeRabbit feedback to use centralized isAdmin() helper instead of inline role comparisons for consistency and case-insensitivity. Widen isAdmin() to accept null for callerRole compatibility.
Summary
ownerrole as a superset ofadmin— every admin check now also passes for owner via a centralizedisAdmin()helperownerrole on creation withcreatorIdstored in org metadatamigrate_org_creators)Changes
Backend
ownerto all role schemas (Zod), validators (Convex), and VALID_ROLES setsisAdmin()helper inconvex/lib/rls/helpers/role_helpers.ts(owner ⊃ admin)members/mutations.ts(removeMember, updateMemberRole)updateMemberRolecreatorRolefrom'admin'to'owner'in Better Auth configcreatorIdin org metadata on creationFrontend
ownerbadge variant (purple) in badge-colors.tsMigration
convex/migrations/migrate_org_creators.ts— finds earliest member per org, promotes to owner, stores creatorIdTests
role_helpers.test.ts— isAdmin helper testsmembers/__tests__/mutations.test.ts— owner protection, last-admin guard, owner-as-callermembers/__tests__/queries.test.ts— owner returns isAdmin: trueaccess_control.test.ts— owner role authorizationTest plan
bun run --filter @tale/platform lint— 0 errorsbun run --filter @tale/platform test— 1390 passed (1 pre-existing failure unrelated)ownerroleSummary by CodeRabbit
Release Notes
New Features
Updates