Repository navigation
refactor(auth): unify pbac logic and remove hasAdminAccess #33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1836dca
362c8ba
b3b4212
0020bcd
f6ea572
439f69e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,28 +1,24 @@ | ||
|
|
||
|
|
||
| /** | ||
| * Checks if a user has sufficient permissions to access the admin area. | ||
| * | ||
| * Rules: | ||
| * 1. Default: user.permissions includes '*' (Super Admin). | ||
| * 2. System Level: user.permissions includes 'tenants:manage', 'tenants:read', or 'users:manage'. | ||
| * 3. Namespace: user.permissions starts with 'system_' or 'admin:'. | ||
| * | ||
| * @param permissions List of permission strings. | ||
| * @returns boolean | ||
| * Checks if a user has a specific permission. | ||
| * Supports: | ||
| * 1. Root Wildcard ('*') | ||
| * 2. Exact Match ('resource:action') | ||
| * 3. Resource Wildcard ('resource:*') | ||
| * 4. Action Wildcard ('*:action') | ||
| */ | ||
| export function hasAdminAccess(permissions?: string[]): boolean { | ||
| export function hasPermission(permissions: string[] | undefined, resource: string, action: string): boolean { | ||
| if (!permissions || permissions.length === 0) return false; | ||
|
|
||
| // Super Admin | ||
| if (permissions.includes('*')) return true; | ||
|
|
||
| // Explicit System Permissions | ||
| const systemPermissions = ['tenants:manage', 'tenants:read', 'users:manage']; | ||
| if (permissions.some(p => systemPermissions.includes(p))) return true; | ||
| const requiredPermission = `${resource}:${action}`; | ||
| const resourceWildcard = `${resource}:*`; | ||
| const actionWildcard = `*:${action}`; | ||
|
|
||
| // Check for Namespaced System Permissions (e.g. system_users:read, admin:dashboard) | ||
| // We check the resource part of the permission string "resource:action" (or just verify prefix) | ||
| return permissions.some(p => { | ||
| return p.startsWith('system_') || p.startsWith('admin:'); | ||
| }); | ||
| return permissions.some(p => | ||
| p === '*' || // 1. Super Admin | ||
| p === requiredPermission || // 2. Exact Match | ||
| p === resourceWildcard || // 3. Resource Wildcard | ||
| p === actionWildcard // 4. Action Wildcard | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,79 +1,51 @@ | ||
| import type { AccessControlProvider, CanParams, CanReturnType } from "@refinedev/core"; | ||
| import { authProvider } from "./auth-provider"; | ||
| import { hasPermission } from "../lib/auth/utils"; | ||
|
|
||
| // Resource Normalization Map | ||
| // Strip 'admin/' prefix to map 'admin/users' -> 'users' automatically. | ||
| const RESOURCE_MAP: Record<string, string> = { | ||
| // Keep distinct mappings if needed, otherwise normalization handles most | ||
| "admin/users": "users", | ||
| "admin/tenants": "tenants", | ||
| }; | ||
|
|
||
| // Action Normalization Map | ||
| // Map frontend actions to backend permissions | ||
| const ACTION_MAP: Record<string, string> = { | ||
| "list": "read", | ||
| "show": "read", | ||
| "edit": "update", | ||
| }; | ||
|
|
||
| export const accessControlProvider: AccessControlProvider = { | ||
| can: async ({ resource, action }: CanParams): Promise<CanReturnType> => { | ||
| // Fetch permissions fresh (or cached by AuthProvider/Client) | ||
| // Note: authProvider.getPermissions is implemented now. | ||
| // We cast to string[] strictly. | ||
| const permissions = (await authProvider.getPermissions?.()) as string[] ?? []; | ||
|
|
||
| // Admin Bypass (Optional: if 'platform_admin' role implies *, but our backend | ||
| // sends '*' in permissions list for admins anyway). | ||
| if (permissions.includes("*")) { | ||
| return { can: true }; | ||
| } | ||
|
|
||
| // Map Refine actions to our RBAC verbs | ||
| // Refine: list, show, create, edit, delete | ||
| // RBAC: read, manage (invite is specific) | ||
|
|
||
| let requiredAction = "manage"; // Default to high security | ||
| if (action === "list" || action === "show") { | ||
| requiredAction = "read"; | ||
| } | ||
|
|
||
| // Handling special resource names if they differ from backend | ||
| // e.g. "admin/users" -> "system_users" (or "users") | ||
| // My AdminRoutes uses "admin/users". | ||
| // My Seed Script uses "system_users" and "users". | ||
| // Let's assume strict mapping or normalize. | ||
| // If resource is "admin/users", needed permission might be "system_users:read". | ||
|
|
||
| let targetResource = resource; | ||
|
|
||
| // Map frontend resources to backend resources | ||
| // Note: This mapping ensures Refine resources (like "admin/users") align with backend RBAC resources ("system_users"). | ||
| // If you add new resources, update this map here. | ||
| // TODO: Move to external config or inject via context in the future for flexibility. | ||
| const resourceMap: Record<string, string> = { | ||
| "admin/users": "system_users", | ||
| "admin/tenants": "system_tenants", | ||
| "users": "users", | ||
| "tenants": "tenants", | ||
| }; | ||
| // Resource Normalization | ||
| let targetResource = resource ?? ""; | ||
|
|
||
| if (resource && resourceMap[resource]) { | ||
| targetResource = resourceMap[resource]; | ||
| if (RESOURCE_MAP[targetResource]) { | ||
| targetResource = RESOURCE_MAP[targetResource]; | ||
| } else if (targetResource.startsWith("admin/")) { | ||
| targetResource = targetResource.replace(/^admin\//, ""); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
coderabbitai[bot] marked this conversation as resolved.
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| const requiredPermission = `${targetResource}:${requiredAction}`; | ||
| // Action Normalization | ||
| const rawAction = action || "manage"; | ||
| const targetAction = ACTION_MAP[rawAction] || rawAction; | ||
|
|
||
| // Also allow if specific permission exists (e.g. system_users:invite) | ||
| // If action is custom, e.g. "invite", check "system_users:invite" | ||
| if (action !== "list" && action !== "show" && action !== "create" && action !== "edit" && action !== "delete") { | ||
| // Custom action (exact match) | ||
| if (permissions.includes(`${targetResource}:${action}`)) { | ||
| return { can: true }; | ||
| } | ||
| } | ||
|
|
||
| // Standard Check | ||
| // Check exact match | ||
| if (permissions.includes(requiredPermission)) { | ||
| return { can: true }; | ||
| } | ||
| // Use Shared Utility | ||
| // Matches strict logic: *, resource:action, resource:*, *:action | ||
| const canAccess = hasPermission(permissions, targetResource, targetAction); | ||
|
|
||
| // Check "manage" implies "read"? | ||
| // Usually "manage" implies everything. | ||
| // If I require "read", but have "manage", I should typically allow. | ||
| if (requiredAction === "read" && permissions.includes(`${targetResource}:manage`)) { | ||
| if (canAccess) { | ||
| return { can: true }; | ||
| } | ||
|
|
||
| return { | ||
| can: false, | ||
| reason: `Missing permission: ${requiredPermission}`, | ||
| reason: `Missing permission: ${targetResource}:${targetAction}`, | ||
| }; | ||
| }, | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,12 +6,28 @@ export const seedRbac = async (db: NodePgDatabase<typeof schema>) => { | |
|
|
||
| // 1. Define Standard Permissions | ||
| // resource:action | ||
| // Strictly granular for PBAC strategies | ||
| const standardPerms = [ | ||
| { id: "users:manage", action: "manage", resource: "users" }, | ||
| // Users | ||
| { id: "users:read", action: "read", resource: "users" }, | ||
| { id: "tenants:manage", action: "manage", resource: "tenants" }, | ||
| { id: "users:create", action: "create", resource: "users" }, | ||
| { id: "users:update", action: "update", resource: "users" }, | ||
| { id: "users:delete", action: "delete", resource: "users" }, | ||
| { id: "users:manage", action: "manage", resource: "users" }, | ||
|
|
||
| // Tenants | ||
| { id: "tenants:read", action: "read", resource: "tenants" }, | ||
| { id: "tenants:create", action: "create", resource: "tenants" }, | ||
| { id: "tenants:update", action: "update", resource: "tenants" }, | ||
| { id: "tenants:delete", action: "delete", resource: "tenants" }, | ||
| { id: "tenants:manage", action: "manage", resource: "tenants" }, | ||
|
|
||
| // Dashboard | ||
| { id: "dashboard:read", action: "read", resource: "dashboard" }, | ||
|
|
||
| // Settings | ||
| { id: "settings:manage", action: "manage", resource: "settings" }, | ||
| { id: "settings:read", action: "read", resource: "settings" }, | ||
| ]; | ||
|
|
||
| await db | ||
|
|
@@ -42,13 +58,31 @@ export const seedRbac = async (db: NodePgDatabase<typeof schema>) => { | |
| // Map role -> permission[] | ||
| const roleMap: Record<string, string[]> = { | ||
| owner: [ | ||
| "dashboard:read", | ||
| "users:manage", | ||
| "users:read", | ||
| "users:create", | ||
| "users:update", | ||
| "users:delete", | ||
| "tenants:manage", | ||
| "tenants:read", | ||
| "tenants:create", | ||
| "tenants:update", | ||
| "tenants:delete", | ||
| "settings:manage", | ||
| "settings:read", | ||
| ], | ||
|
Comment on lines
59
to
+74
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Consider using resource wildcards to reduce redundancy. The owner role lists both This is optional since the explicit listing is correct and provides clear audit trails. ♻️ Alternative using wildcards owner: [
"dashboard:view",
- "users:manage",
- "users:read",
- "users:create",
- "users:update",
- "users:delete",
- "tenants:manage",
- "tenants:read",
- "tenants:create",
- "tenants:update",
- "tenants:delete",
- "settings:manage",
- "settings:read",
+ "users:*",
+ "tenants:*",
+ "settings:*",
],Note: This requires seeding 🤖 Prompt for AI Agents |
||
| admin: [ | ||
| "dashboard:read", | ||
| "users:manage", | ||
| "users:read", | ||
| "users:create", | ||
| "users:update", | ||
| "users:delete", | ||
| "tenants:read", | ||
| "settings:manage", | ||
| "settings:read", | ||
| ], | ||
| admin: ["users:manage", "users:read", "tenants:read", "settings:manage"], | ||
| member: ["users:read", "tenants:read"], | ||
| }; | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.