Skip to content

v0.6.1: added better auth admin plugin#3639

Merged
waleedlatif1 merged 2 commits intomainfrom
staging
Mar 17, 2026
Merged

v0.6.1: added better auth admin plugin#3639
waleedlatif1 merged 2 commits intomainfrom
staging

Conversation

@waleedlatif1
Copy link
Collaborator

Sg312 and others added 2 commits March 17, 2026 13:30
* Fix mothership tool scheduling

* Fix
#3612)

* feat(auth): migrate to better-auth admin plugin

* feat(settings): add unified Admin tab with user management

Consolidate superuser features into a single Admin settings tab:
- Super admin mode toggle (moved from General)
- Workflow import (moved from Debug)
- User management via better-auth admin (list, set role, ban/unban)

Replace Debug tab with Admin tab gated by requiresAdminRole.
Add React Query hooks for admin user operations.

* fix(db): backfill existing super users to admin role in migration

Add UPDATE statement to promote is_super_user=true rows to role='admin'
before dropping the is_super_user column, preventing silent demotion.

* fix(admin): resolve type errors in admin tab

- Fix cn import path to @/lib/core/utils/cn
- Use valid Badge variants (blue/gray/red/green instead of secondary/destructive)
- Type setRole param as 'user' | 'admin' union

* improvement(auth): remove /api/user/super-user route, use session role

Include user.role in customSession so it's available client-side.
Replace all useSuperUserStatus() calls with session.user.role === 'admin'.
Delete the now-redundant /api/user/super-user endpoint.

* chore(auth): remove redundant role override in customSession

The admin plugin already includes role on the user object.
No need to manually spread it in customSession.

* improvement(queries): clean up admin-users hooks per React Query best practices

- Remove unsafe unknown/Record casting, use better-auth typed response
- Add placeholderData: keepPreviousData for paginated variable-key query
- Remove nullable types where defaults are always applied

* fix(admin): address review feedback on admin tab

- Fix superUserModeEnabled default to false (matches sidebar behavior)
- Reset banReason when switching ban target to prevent state bleed
- Guard admin section render with session role check for direct URL access

* fix(settings): align superUserModeEnabled default to false everywhere

Three places defaulted to true while admin tab and sidebar used false.
Align all to false so new admins see consistent behavior.

* fix(admin): fix stale pendingUserId, add isPending guard and error feedback

- Only read mutation.variables when mutation isPending (prevents stale ID)
- Add isPending guard to super user mode toggle (prevents concurrent mutations)
- Show inline error message when setRole/ban/unban mutations fail

* fix(admin): concurrent pending users Set, session loading guard, domain blocking

- Replace pendingUserId scalar with pendingUserIds Set (useMemo) so concurrent
  mutations across different users each disable their own row correctly
- Add sessionLoading guard to admin section redirect to prevent flash on direct
  /settings/admin navigation before session resolves
- Add BLOCKED_SIGNUP_DOMAINS env var and before-hook for email domain denylist,
  parsed once at module init as a Set for O(1) per-request lookups
- Add trailing newline to migration file

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(admin): close OAuth domain bypass, fix stale errors, deduplicate icon

- Add databaseHooks.user.create.before to enforce BLOCKED_SIGNUP_DOMAINS at
  the model level, covering all signup vectors (email, OAuth, social) not just
  /sign-up paths
- Call .reset() on each mutation before firing to clear stale error state from
  previous operations
- Change Admin nav icon from ShieldCheck to Lock to avoid duplicate with
  Access Control tab

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
@cursor
Copy link

cursor bot commented Mar 17, 2026

You have used all Bugbot PR reviews included in your free trial for your GitHub account on this workspace.

To continue using Bugbot reviews, enable Bugbot for your team in the Cursor dashboard.

@vercel
Copy link

vercel bot commented Mar 17, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment Mar 17, 2026 10:07pm

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 17, 2026

Greptile Summary

This PR migrates the admin/super-user system from a custom isSuperUser boolean field to the better-auth admin plugin, giving a unified Admin tab in settings with user management (list, ban/unban, promote/demote), workflow import, and super-admin mode toggle. It also includes a fix for tool-call scheduling in the copilot SSE orchestrator and adds BLOCKED_SIGNUP_DOMAINS support.

Key changes:

  • DB migration (0177_wise_puma.sql) replaces is_super_user with a role text column (user/admin), backfilling existing super-users, and adds banned/ban_reason/ban_expires + session.impersonated_by columns for the admin plugin.
  • All previous isSuperUser checks (API route, hooks, server-side verifyEffectiveSuperUser, templates page) are replaced with role === 'admin'.
  • The custom /api/user/super-user endpoint and useSuperUserStatus hook are deleted in favour of the session token carrying the role field — eliminating a redundant round-trip on every page load.
  • superUserModeEnabled default corrected from truefalse to prevent unintended admin mode activation.
  • One issue found: In settings.tsx, the admin-section redirect guard evaluates to false while sessionLoading is true, causing <Admin /> (or its skeleton) to render transiently for non-admin users who navigate directly to /settings/admin. The fix is to include sessionLoading in the redirect condition.

Confidence Score: 4/5

  • Safe to merge with a minor fix — all admin API calls are server-side protected, but the UI transiently shows the Admin skeleton to non-admin users during session load.
  • The migration is well-structured: the DB data migration is correct, server-side permission checks use the new role field consistently, and the better-auth admin plugin handles API-level auth for all admin actions. The main concern is a one-liner UX/security-in-depth issue in the client-side route guard. The tool-call scheduling fix is correct and clearly scoped.
  • apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx — admin section guard needs sessionLoading included in the redirect condition.

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx Replaces Debug with the new Admin section; adds session-based role guard, but Admin component transiently renders for non-admin users while the session is still loading.
apps/sim/app/workspace/[workspaceId]/settings/components/admin/admin.tsx New Admin panel combining super-user mode toggle, workflow import, and paginated user management with ban/unban/promote actions; minor type-assertion issue in pendingUserIds memo.
apps/sim/lib/auth/auth.ts Adds admin() plugin, BLOCKED_SIGNUP_DOMAINS env-var enforcement via both onRequest middleware and a databaseHooks.user.create.before hook for defence-in-depth; looks correct.
apps/sim/hooks/queries/admin-users.ts New query/mutation hooks wrapping client.admin for listing, role-setting, banning, and unbanning users; follows established query-key factory pattern correctly.
packages/db/migrations/0177_wise_puma.sql Migrates is_super_user boolean to the better-auth role text column (seeding existing super-users as admin), adds banned/ban_reason/ban_expires columns, and adds impersonated_by to sessions. Data migration is correct.
apps/sim/lib/copilot/orchestrator/sse/handlers/handlers.ts Fixes tool-call scheduling: client-executable tools that are also available server-side now use fireToolExecution() instead of waiting for client completion, preventing scheduling deadlocks.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User navigates to /settings/admin] --> B{sessionLoading?}
    B -- true --> C[effectiveSection = 'admin'\nAdmin skeleton renders ⚠️]
    C --> D{Session resolves}
    D -- role === 'admin' --> E[Admin panel loads fully]
    D -- role !== 'admin' --> F[Redirect → effectiveSection = 'general']
    B -- false --> G{role === 'admin'?}
    G -- yes --> E
    G -- no --> F

    E --> H[Super-admin mode toggle\nuseGeneralSettings]
    E --> I[Workflow import\nuseImportWorkflow]
    E --> J[User management\nuseAdminUsers via client.admin.listUsers]
    J --> K[Promote / Demote\nclient.admin.setRole]
    J --> L[Ban / Unban\nclient.admin.banUser / unbanUser]

    style C fill:#ffcccc,stroke:#ff0000
Loading

Last reviewed commit: 25a03f1

Comment on lines 164 to +169
const effectiveSection =
!isBillingEnabled && (section === 'subscription' || section === 'team') ? 'general' : section
!isBillingEnabled && (section === 'subscription' || section === 'team')
? 'general'
: section === 'admin' && !sessionLoading && !isAdminRole
? 'general'
: section
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Admin section renders briefly for non-admin users

When sessionLoading is true and section === 'admin', the condition section === 'admin' && !sessionLoading && !isAdminRole evaluates to false (because !sessionLoading is false), so effectiveSection stays as 'admin' and <Admin /> is rendered — showing its skeleton to a potentially non-admin user until the session resolves.

This means a non-admin user who navigates directly to /settings/admin will briefly see the AdminSkeleton before being redirected to general. A safer pattern is to treat a loading session the same as a non-admin session for this section:

Suggested change
const effectiveSection =
!isBillingEnabled && (section === 'subscription' || section === 'team') ? 'general' : section
!isBillingEnabled && (section === 'subscription' || section === 'team')
? 'general'
: section === 'admin' && !sessionLoading && !isAdminRole
? 'general'
: section
const effectiveSection =
!isBillingEnabled && (section === 'subscription' || section === 'team')
? 'general'
: section === 'admin' && (sessionLoading || !isAdminRole)
? 'general'
: section

This ensures non-admin users never see any part of the Admin UI, even transiently. Admins will still see the General section skeleton briefly while the session loads, but that is less sensitive than leaking the Admin skeleton.

Comment on lines +73 to +89
const pendingUserIds = useMemo(() => {
const ids = new Set<string>()
if (setUserRole.isPending && (setUserRole.variables as { userId?: string })?.userId)
ids.add((setUserRole.variables as { userId: string }).userId)
if (banUser.isPending && (banUser.variables as { userId?: string })?.userId)
ids.add((banUser.variables as { userId: string }).userId)
if (unbanUser.isPending && (unbanUser.variables as { userId?: string })?.userId)
ids.add((unbanUser.variables as { userId: string }).userId)
return ids
}, [
setUserRole.isPending,
setUserRole.variables,
banUser.isPending,
banUser.variables,
unbanUser.isPending,
unbanUser.variables,
])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Unnecessary type assertions on mutation variables

TanStack Query v5 types variables as TVariables | undefined when a mutation is in-flight, but the hook already knows the shape. The explicit casts to { userId?: string } and { userId: string } bypass the inferred types and can silently hide future mismatches. Since setUserRole.isPending already guards the path, you can rely on the inferred types directly:

Suggested change
const pendingUserIds = useMemo(() => {
const ids = new Set<string>()
if (setUserRole.isPending && (setUserRole.variables as { userId?: string })?.userId)
ids.add((setUserRole.variables as { userId: string }).userId)
if (banUser.isPending && (banUser.variables as { userId?: string })?.userId)
ids.add((banUser.variables as { userId: string }).userId)
if (unbanUser.isPending && (unbanUser.variables as { userId?: string })?.userId)
ids.add((unbanUser.variables as { userId: string }).userId)
return ids
}, [
setUserRole.isPending,
setUserRole.variables,
banUser.isPending,
banUser.variables,
unbanUser.isPending,
unbanUser.variables,
])
const pendingUserIds = useMemo(() => {
const ids = new Set<string>()
if (setUserRole.isPending && setUserRole.variables?.userId)
ids.add(setUserRole.variables.userId)
if (banUser.isPending && banUser.variables?.userId)
ids.add(banUser.variables.userId)
if (unbanUser.isPending && unbanUser.variables?.userId)
ids.add(unbanUser.variables.userId)
return ids
}, [
setUserRole.isPending,
setUserRole.variables,
banUser.isPending,
banUser.variables,
unbanUser.isPending,
unbanUser.variables,
])

@waleedlatif1 waleedlatif1 merged commit 4f3bc37 into main Mar 17, 2026
26 checks passed
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.

2 participants