Merge pull request #131 from borski/pr-130-review

feature/api_tokens review fixes: role enforcement security + UI bugfixes

src/app/admin/settings/tabs/ApiTab/ApiTab.tsx

@@ -25,7 +25,6 @@ export function ApiTab() {
   // Admin-specific state
   const [users, setUsers] = useState<UserOption[]>([]);
   const [newTokenUserId, setNewTokenUserId] = useState('');
-  const [newTokenRole, setNewTokenRole] = useState('');
 
   const fetchUsers = useCallback(async () => {
     try {
@@ -46,31 +45,16 @@ export function ApiTab() {
   const handleCreate = async () => {
     const extraBody: Record<string, string> = {};
     if (newTokenUserId) extraBody.userId = newTokenUserId;
-    if (newTokenRole) extraBody.role = newTokenRole;
+    const created = await api.handleCreate(extraBody);
-    await api.handleCreate(extraBody);
+    // Reset admin-specific fields only when create succeeds
-    // Reset admin-specific fields on success
+    if (created) {
-    if (!api.error) {
       setNewTokenUserId('');
-      setNewTokenRole('');
-    }
-  };
-
-  const handleUserChange = (userId: string) => {
-    setNewTokenUserId(userId);
-    if (userId) {
-      const selectedUser = users.find((u) => u.id === userId);
-      if (selectedUser && !newTokenRole) {
-        setNewTokenRole(selectedUser.role);
-      }
-    } else {
-      setNewTokenRole('');
     }
   };
 
   const handleCancel = () => {
     api.resetForm();
     setNewTokenUserId('');
-    setNewTokenRole('');
   };
 
   if (api.loading) {
@@ -86,7 +70,7 @@ export function ApiTab() {
       <div>
         <h2 className="text-xl font-semibold text-gray-900 dark:text-gray-100">API Tokens</h2>
         <p className="text-sm text-gray-500 dark:text-gray-400 mt-1">
-          Manage API tokens for all users. Create tokens for any user with any role for programmatic access.{' '}
+          Manage API tokens for all users. Create tokens for any user for programmatic access.{' '}
           <Link href="/api-docs" className="text-blue-600 dark:text-blue-400 hover:underline">
             View API documentation
           </Link>
@@ -123,10 +107,12 @@ export function ApiTab() {
                 </button>
               </div>
             </div>
-            <button
+                <button
-              onClick={api.dismissCreatedToken}
+                  type="button"
-              className="flex-shrink-0 text-green-600 dark:text-green-400 hover:text-green-800 dark:hover:text-green-200"
+                  aria-label="Dismiss token banner"
-            >
+                  onClick={api.dismissCreatedToken}
+                  className="flex-shrink-0 text-green-600 dark:text-green-400 hover:text-green-800 dark:hover:text-green-200"
+                >
               <svg className="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24">
                 <path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d="M6 18L18 6M6 6l12 12" />
               </svg>
@@ -174,7 +160,7 @@ export function ApiTab() {
               </label>
               <select
                 value={newTokenUserId}
-                onChange={(e) => handleUserChange(e.target.value)}
+                onChange={(e) => setNewTokenUserId(e.target.value)}
                 className="w-full rounded-lg border border-gray-300 dark:border-gray-600 bg-white dark:bg-gray-800 px-3 py-2 text-sm text-gray-900 dark:text-gray-100 focus:border-blue-500 focus:ring-1 focus:ring-blue-500"
               >
                 <option value="">Current user (default)</option>
@@ -184,20 +170,9 @@ export function ApiTab() {
                   </option>
                 ))}
               </select>
-            </div>
+              <p className="mt-1 text-xs text-gray-500 dark:text-gray-400">
-            <div>
+                Token will inherit the selected user&apos;s role
-              <label className="block text-sm font-medium text-gray-700 dark:text-gray-300 mb-1">
+              </p>
-                Role override
-              </label>
-              <select
-                value={newTokenRole}
-                onChange={(e) => setNewTokenRole(e.target.value)}
-                className="w-full rounded-lg border border-gray-300 dark:border-gray-600 bg-white dark:bg-gray-800 px-3 py-2 text-sm text-gray-900 dark:text-gray-100 focus:border-blue-500 focus:ring-1 focus:ring-blue-500"
-              >
-                <option value="">User&apos;s default role</option>
-                <option value="admin">Admin</option>
-                <option value="user">User</option>
-              </select>
             </div>
           </div>
           <div className="flex gap-2">

src/app/api-docs/page.tsx

@@ -120,7 +120,7 @@ export default function ApiDocsPage() {
           <div className="space-y-4">
             {API_TOKEN_ENDPOINT_DOCS.map((endpoint) => (
               <EndpointCard
-                key={endpoint.path}
+                key={`${endpoint.method}:${endpoint.path}`}
                 endpoint={endpoint}
                 token={token}
                 useSession={useSession}

src/app/api/admin/api-tokens/route.ts

@@ -18,7 +18,7 @@ const CreateTokenSchema = z.object({
   name: z.string().min(1).max(100),
   expiresAt: z.string().datetime().nullable().optional(),
   userId: z.string().uuid().optional(), // Admin can specify which user the token acts as
-  role: z.enum(['admin', 'user']).optional(), // Admin can override the token role
+  role: z.enum(['admin', 'user']).optional(), // Accepted for compatibility, but cannot differ from target user role
 });
 
 /**
@@ -66,7 +66,8 @@ export async function GET(request: NextRequest) {
 
 /**
  * POST /api/admin/api-tokens
- * Create a new API token. Admin can optionally specify userId and role.
+ * Create a new API token. Admin can optionally specify userId.
+ * Token role is always derived from the target user's current role.
  * Returns the full token ONCE.
  */
 export async function POST(request: NextRequest) {
@@ -120,19 +121,26 @@ export async function POST(request: NextRequest) {
           );
         }
 
-        // Determine token role (defaults to target user's role)
+        // Security guard: token role must always match the target user's persisted role.
-        const tokenRole = role || targetUser.role;
+        // This avoids role/identity mismatch (for example: acting as user A with admin role).
-
-        // Log when admin explicitly overrides role to differ from user's actual role
         if (role && role !== targetUser.role) {
-          logger.warn('Admin creating token with role different from user actual role', {
+          logger.warn('Admin attempted token role override that differs from target user role', {
-            tokenRole: role,
+            requestedRole: role,
             userActualRole: targetUser.role,
             targetUser: targetUser.plexUsername,
             createdBy: req.user!.username,
           });
+
+          return NextResponse.json(
+            {
+              error: `Token role must match target user's role (${targetUser.role}).`,
+            },
+            { status: 400 }
+          );
         }
 
+        const tokenRole = targetUser.role;
+
         // Generate the token
         const { fullToken, tokenHash, tokenPrefix } = generateApiToken();
 

src/components/profile/ApiTokensSection.tsx

@@ -63,6 +63,8 @@ export function ApiTokensSection() {
                   </div>
                 </div>
                 <button
+                  type="button"
+                  aria-label="Dismiss token banner"
                   onClick={api.dismissCreatedToken}
                   className="flex-shrink-0 text-green-600 dark:text-green-400 hover:text-green-800 dark:hover:text-green-200"
                 >

src/lib/hooks/useApiTokens.ts

@@ -39,7 +39,7 @@ export interface UseApiTokensReturn<T extends ApiToken = ApiToken> {
   confirmRevokeId: string | null;
   setConfirmRevokeId: (id: string | null) => void;
   fetchTokens: () => Promise<void>;
-  handleCreate: (extraBody?: Partial<CreateTokenBody>) => Promise<void>;
+  handleCreate: (extraBody?: Partial<CreateTokenBody>) => Promise<boolean>;
   handleDeleteConfirmed: () => Promise<void>;
   handleCopy: () => Promise<void>;
   dismissCreatedToken: () => void;
@@ -69,10 +69,21 @@ export function useApiTokens<T extends ApiToken = ApiToken>(
   const fetchTokens = useCallback(async () => {
     try {
       const response = await fetchWithAuth(config.basePath);
-      if (response.ok) {
+      if (!response.ok) {
-        const data = await response.json();
+        let message = 'Failed to load API tokens';
-        setTokens(data.tokens);
+        try {
+          const data = await response.json();
+          message = data.error || message;
+        } catch {
+          // Keep default message when response body is not JSON
+        }
+        setError(message);
+        return;
       }
+
+      const data = await response.json();
+      setTokens(data.tokens);
+      setError(null);
     } catch {
       setError('Failed to load API tokens');
     } finally {
@@ -98,7 +109,7 @@ export function useApiTokens<T extends ApiToken = ApiToken>(
   const handleCreate = async (extraBody?: Partial<CreateTokenBody>) => {
     if (!newTokenName.trim()) {
       setError('Token name is required');
-      return;
+      return false;
     }
 
     setCreating(true);
@@ -124,12 +135,15 @@ export function useApiTokens<T extends ApiToken = ApiToken>(
         setNewTokenExpiry('never');
         setShowCreateForm(false);
         await fetchTokens();
+        return true;
       } else {
         const data = await response.json();
         setError(data.error || 'Failed to create token');
+        return false;
       }
     } catch {
       setError('Failed to create token');
+      return false;
     } finally {
       setCreating(false);
     }

tests/api/admin-api-tokens.routes.test.ts

@@ -0,0 +1,215 @@
+/**
+ * Component: Admin API Tokens Route Tests
+ * Documentation: documentation/testing.md
+ */
+
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+import { createPrismaMock } from '../helpers/prisma';
+
+// Valid UUIDs for testing
+const ADMIN_ID = '11111111-1111-1111-1111-111111111111';
+const USER_ID = '22222222-2222-2222-2222-222222222222';
+const ADMIN2_ID = '33333333-3333-3333-3333-333333333333';
+const NONEXISTENT_ID = '99999999-9999-9999-9999-999999999999';
+
+let authRequest: any;
+
+const prismaMock = createPrismaMock();
+const requireAuthMock = vi.hoisted(() => vi.fn());
+const requireAdminMock = vi.hoisted(() => vi.fn());
+const checkApiTokenCreateRateLimitMock = vi.hoisted(() => vi.fn());
+const generateApiTokenMock = vi.hoisted(() => vi.fn());
+
+vi.mock('@/lib/db', () => ({
+  prisma: prismaMock,
+}));
+
+vi.mock('@/lib/middleware/auth', () => ({
+  requireAuth: requireAuthMock,
+  requireAdmin: requireAdminMock,
+}));
+
+vi.mock('@/lib/utils/apiTokenRateLimit', () => ({
+  checkApiTokenCreateRateLimit: checkApiTokenCreateRateLimitMock,
+}));
+
+vi.mock('@/lib/utils/api-token', () => ({
+  generateApiToken: generateApiTokenMock,
+}));
+
+describe('Admin API tokens routes', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    authRequest = {
+      user: { id: ADMIN_ID, username: 'admin', role: 'admin' },
+      json: vi.fn(),
+    };
+    requireAuthMock.mockImplementation((_req: any, handler: any) => handler(authRequest));
+    requireAdminMock.mockImplementation((_req: any, handler: any) => handler());
+    checkApiTokenCreateRateLimitMock.mockReturnValue({ allowed: true });
+    generateApiTokenMock.mockReturnValue({
+      fullToken: 'rmab_test_full_token',
+      tokenHash: 'hashed_token',
+      tokenPrefix: 'rmab_test',
+    });
+  });
+
+  describe('POST /api/admin/api-tokens', () => {
+    it('creates token for self with own role when no userId specified', async () => {
+      authRequest.json.mockResolvedValueOnce({ name: 'Test Token' });
+
+      prismaMock.user.findUnique.mockResolvedValueOnce({
+        id: ADMIN_ID,
+        role: 'admin',
+        plexUsername: 'admin',
+      });
+      prismaMock.apiToken.count.mockResolvedValueOnce(0);
+      prismaMock.apiToken.create.mockResolvedValueOnce({
+        id: 'token-1',
+        name: 'Test Token',
+        tokenPrefix: 'rmab_test',
+        role: 'admin',
+        expiresAt: null,
+        createdAt: new Date(),
+      });
+
+      const { POST } = await import('@/app/api/admin/api-tokens/route');
+      const response = await POST({} as any);
+      const payload = await response.json();
+
+      expect(response.status).toBe(201);
+      expect(payload.token.role).toBe('admin');
+      expect(payload.fullToken).toBe('rmab_test_full_token');
+    });
+
+    it('creates token for another user with their role', async () => {
+      authRequest.json.mockResolvedValueOnce({
+        name: 'Token for User',
+        userId: USER_ID,
+      });
+
+      prismaMock.user.findUnique.mockResolvedValueOnce({
+        id: USER_ID,
+        role: 'user',
+        plexUsername: 'regularuser',
+      });
+      prismaMock.apiToken.count.mockResolvedValueOnce(0);
+      prismaMock.apiToken.create.mockResolvedValueOnce({
+        id: 'token-2',
+        name: 'Token for User',
+        tokenPrefix: 'rmab_test',
+        role: 'user',
+        expiresAt: null,
+        createdAt: new Date(),
+      });
+
+      const { POST } = await import('@/app/api/admin/api-tokens/route');
+      const response = await POST({} as any);
+      const payload = await response.json();
+
+      expect(response.status).toBe(201);
+      expect(payload.token.role).toBe('user');
+    });
+
+    it('rejects role override when role differs from target user role', async () => {
+      authRequest.json.mockResolvedValueOnce({
+        name: 'Escalation Attempt',
+        userId: USER_ID,
+        role: 'admin', // Trying to give admin role to a regular user
+      });
+
+      prismaMock.user.findUnique.mockResolvedValueOnce({
+        id: USER_ID,
+        role: 'user', // Target user is actually a regular user
+        plexUsername: 'regularuser',
+      });
+
+      const { POST } = await import('@/app/api/admin/api-tokens/route');
+      const response = await POST({} as any);
+      const payload = await response.json();
+
+      expect(response.status).toBe(400);
+      expect(payload.error).toContain("must match target user's role");
+    });
+
+    it('rejects role downgrade when role differs from target user role', async () => {
+      authRequest.json.mockResolvedValueOnce({
+        name: 'Downgrade Attempt',
+        userId: ADMIN2_ID,
+        role: 'user', // Trying to give user role to an admin
+      });
+
+      prismaMock.user.findUnique.mockResolvedValueOnce({
+        id: ADMIN2_ID,
+        role: 'admin', // Target user is actually an admin
+        plexUsername: 'otheradmin',
+      });
+
+      const { POST } = await import('@/app/api/admin/api-tokens/route');
+      const response = await POST({} as any);
+      const payload = await response.json();
+
+      expect(response.status).toBe(400);
+      expect(payload.error).toContain("must match target user's role");
+    });
+
+    it('accepts role when it matches target user role', async () => {
+      authRequest.json.mockResolvedValueOnce({
+        name: 'Matching Role',
+        userId: USER_ID,
+        role: 'user', // Explicitly specifying role that matches
+      });
+
+      prismaMock.user.findUnique.mockResolvedValueOnce({
+        id: USER_ID,
+        role: 'user',
+        plexUsername: 'regularuser',
+      });
+      prismaMock.apiToken.count.mockResolvedValueOnce(0);
+      prismaMock.apiToken.create.mockResolvedValueOnce({
+        id: 'token-3',
+        name: 'Matching Role',
+        tokenPrefix: 'rmab_test',
+        role: 'user',
+        expiresAt: null,
+        createdAt: new Date(),
+      });
+
+      const { POST } = await import('@/app/api/admin/api-tokens/route');
+      const response = await POST({} as any);
+
+      expect(response.status).toBe(201);
+    });
+
+    it('returns 404 when target user does not exist', async () => {
+      authRequest.json.mockResolvedValueOnce({
+        name: 'Token for Ghost',
+        userId: NONEXISTENT_ID,
+      });
+
+      prismaMock.user.findUnique.mockResolvedValueOnce(null);
+
+      const { POST } = await import('@/app/api/admin/api-tokens/route');
+      const response = await POST({} as any);
+      const payload = await response.json();
+
+      expect(response.status).toBe(404);
+      expect(payload.error).toBe('Target user not found');
+    });
+
+    it('returns 429 when rate limited', async () => {
+      checkApiTokenCreateRateLimitMock.mockReturnValueOnce({
+        allowed: false,
+        retryAfterSeconds: 60,
+      });
+
+      authRequest.json.mockResolvedValueOnce({ name: 'Rate Limited Token' });
+
+      const { POST } = await import('@/app/api/admin/api-tokens/route');
+      const response = await POST({} as any);
+
+      expect(response.status).toBe(429);
+      expect(response.headers.get('Retry-After')).toBe('60');
+    });
+  });
+});