improvement(admin): update defaults for better UX

apps/sim/app/api/credentials/route.test.ts

@@ -3,17 +3,24 @@
  *
  * @vitest-environment node
  */
-import { auditMock, authMockFns, createMockRequest, posthogServerMock } from '@sim/testing'
+import {
+  auditMock,
+  authMockFns,
+  createMockRequest,
+  dbChainMockFns,
+  posthogServerMock,
+  resetDbChainMock,
+} from '@sim/testing'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { TokenServiceAccountValidationError } from '@/lib/credentials/token-service-accounts/errors'
 
 const {
   mockCheckWorkspaceAccess,
-  mockGetWorkspaceMembership,
+  mockGetCredentialCreationWorkspaceContext,
   mockVerifyAndBuildServiceAccountSecret,
 } = vi.hoisted(() => ({
   mockCheckWorkspaceAccess: vi.fn(),
-  mockGetWorkspaceMembership: vi.fn(),
+  mockGetCredentialCreationWorkspaceContext: vi.fn(),
   mockVerifyAndBuildServiceAccountSecret: vi.fn(),
 }))
 
@@ -25,7 +32,7 @@ vi.mock('@/lib/workspaces/permissions/utils', () => ({
 }))
 
 vi.mock('@/lib/credentials/environment', () => ({
-  getWorkspaceMembership: mockGetWorkspaceMembership,
+  getCredentialCreationWorkspaceContext: mockGetCredentialCreationWorkspaceContext,
 }))
 
 vi.mock('@/lib/credentials/oauth', () => ({
@@ -52,6 +59,7 @@ const WORKSPACE_ID = '11111111-2222-4333-8444-555555555555'
 describe('POST /api/credentials', () => {
   beforeEach(() => {
     vi.clearAllMocks()
+    resetDbChainMock()
     authMockFns.mockGetSession.mockResolvedValue({
       user: { id: 'user-1', name: 'Test User', email: 'test@example.com' },
     })
@@ -60,7 +68,12 @@ describe('POST /api/credentials', () => {
       canWrite: true,
       canAdmin: true,
     })
-    mockGetWorkspaceMembership.mockResolvedValue({ ownerId: 'user-1', memberUserIds: ['user-1'] })
+    mockGetCredentialCreationWorkspaceContext.mockResolvedValue({
+      ownerId: 'user-1',
+      organizationId: 'org-1',
+      memberUserIds: ['user-1'],
+      canWrite: true,
+    })
   })
 
   describe('client-credential service accounts', () => {
@@ -156,5 +169,42 @@ describe('POST /api/credentials', () => {
       expect(data.error).toContain('clientSecret is required')
       expect(mockVerifyAndBuildServiceAccountSecret).not.toHaveBeenCalled()
     })
+
+    it('re-authorizes a personal credential after the shared org/user locks', async () => {
+      mockGetCredentialCreationWorkspaceContext
+        .mockResolvedValueOnce({
+          ownerId: 'user-1',
+          organizationId: 'org-1',
+          memberUserIds: ['user-1'],
+          canWrite: true,
+        })
+        .mockResolvedValueOnce({
+          ownerId: 'org-owner',
+          organizationId: 'org-1',
+          memberUserIds: ['org-owner'],
+          canWrite: false,
+        })
+
+      const req = createMockRequest('POST', {
+        workspaceId: WORKSPACE_ID,
+        type: 'env_personal',
+        envKey: 'MY_API_KEY',
+      })
+
+      const response = await POST(req)
+      const data = await response.json()
+
+      expect(response.status).toBe(403)
+      expect(data).toEqual({ error: 'Write permission required' })
+      expect(mockGetCredentialCreationWorkspaceContext).toHaveBeenCalledTimes(2)
+      expect(dbChainMockFns.execute).toHaveBeenCalled()
+      expect(mockGetCredentialCreationWorkspaceContext.mock.invocationCallOrder[0]).toBeLessThan(
+        dbChainMockFns.execute.mock.invocationCallOrder[0]
+      )
+      expect(dbChainMockFns.execute.mock.invocationCallOrder.at(-1)).toBeLessThan(
+        mockGetCredentialCreationWorkspaceContext.mock.invocationCallOrder[1]
+      )
+      expect(dbChainMockFns.insert).not.toHaveBeenCalled()
+    })
   })
 })

apps/sim/app/api/credentials/route.ts

@@ -13,6 +13,7 @@ import {
 } from '@/lib/api/contracts/credentials'
 import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
 import { getSession } from '@/lib/auth'
+import { acquireOrganizationUserMutationLocks } from '@/lib/billing/organizations/membership'
 import { generateRequestId } from '@/lib/core/utils/request'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import {
@@ -21,7 +22,7 @@ import {
   SHARED_CREDENTIAL_TYPES,
 } from '@/lib/credentials/access'
 import { AtlassianValidationError } from '@/lib/credentials/atlassian-service-account'
-import { getWorkspaceMembership } from '@/lib/credentials/environment'
+import { getCredentialCreationWorkspaceContext } from '@/lib/credentials/environment'
 import { syncWorkspaceOAuthCredentialsForUser } from '@/lib/credentials/oauth'
 import {
   ServiceAccountSecretError,
@@ -509,10 +510,52 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
       resolvedProviderId === SLACK_CUSTOM_BOT_PROVIDER_ID && clientCredentialId
         ? clientCredentialId
         : generateId()
-    const { ownerId: workspaceOwnerId, memberUserIds: workspaceMemberUserIds } =
-      await getWorkspaceMembership(workspaceId)
 
-    await db.transaction(async (tx) => {
+    const creationResult = await db.transaction(async (tx) => {
+      /**
+       * Discover the organization lock scope inside this transaction, then
+       * acquire the same organization → user → membership locks as org
+       * removal/transfer and re-authorize from the transaction before writing.
+       *
+       * If this insert wins, transfer sees the new source-owned personal
+       * credential and blocks. If transfer wins, its permission/member cleanup
+       * is visible to the authoritative re-read below and the insert is
+       * refused.
+       */
+      const plannedContext = await getCredentialCreationWorkspaceContext({
+        executor: tx,
+        workspaceId,
+        userId: session.user.id,
+      })
+      if (!plannedContext) {
+        return { success: false as const, status: 403 as const, error: 'Write permission required' }
+      }
+
+      await acquireOrganizationUserMutationLocks(tx, {
+        userId: session.user.id,
+        organizationIds: plannedContext.organizationId ? [plannedContext.organizationId] : [],
+      })
+
+      const currentContext = await getCredentialCreationWorkspaceContext({
+        executor: tx,
+        workspaceId,
+        userId: session.user.id,
+        forUpdate: true,
+      })
+      if (!currentContext) {
+        return { success: false as const, status: 403 as const, error: 'Write permission required' }
+      }
+      if (currentContext.organizationId !== plannedContext.organizationId) {
+        return {
+          success: false as const,
+          status: 409 as const,
+          error: 'Workspace organization changed while creating the credential. Please retry.',
+        }
+      }
+      if (!currentContext.canWrite) {
+        return { success: false as const, status: 403 as const, error: 'Write permission required' }
+      }
+
       // service_account has no DB-level unique index on (workspaceId, providerId,
       // displayName), so we re-check inside the tx. OAuth/env_* are guarded by
       // partial unique indexes and fall through to the 23505 handler below.
@@ -542,9 +585,9 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
         updatedAt: now,
       })
 
-      if ((type === 'env_workspace' || type === 'service_account') && workspaceOwnerId) {
-        if (workspaceMemberUserIds.length > 0) {
-          for (const memberUserId of workspaceMemberUserIds) {
+      if ((type === 'env_workspace' || type === 'service_account') && currentContext.ownerId) {
+        if (currentContext.memberUserIds.length > 0) {
+          for (const memberUserId of currentContext.memberUserIds) {
             const isAdmin = memberUserId === session.user.id
             await tx.insert(credentialMember).values({
               id: generateId(),
@@ -572,7 +615,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
           updatedAt: now,
         })
       }
+
+      return { success: true as const }
     })
+    if (!creationResult.success) {
+      return NextResponse.json({ error: creationResult.error }, { status: creationResult.status })
+    }
 
     const [created] = await db
       .select()