diff --git a/.changeset/flatten-role-authorize.md b/.changeset/flatten-role-authorize.md new file mode 100644 index 0000000000..f1682df413 --- /dev/null +++ b/.changeset/flatten-role-authorize.md @@ -0,0 +1,5 @@ +--- +"better-auth": patch +--- + +Refactor `role.authorize` control flow while preserving existing authorization behavior. diff --git a/packages/better-auth/src/plugins/access/access.test.ts b/packages/better-auth/src/plugins/access/access.test.ts index bb0a4c414f..8cc962af44 100644 --- a/packages/better-auth/src/plugins/access/access.test.ts +++ b/packages/better-auth/src/plugins/access/access.test.ts @@ -187,4 +187,41 @@ describe("access", () => { expect(response.error).toContain("audit"); } }); + + it("should preserve unauthorized error formats for unknown and denied resources", () => { + const unknownResource = looseRole.authorize({ + audit: ["read"], + }); + const deniedAction = looseRole.authorize({ + project: ["delete-many"], + }); + + expect(unknownResource).toEqual({ + success: false, + error: "You are not allowed to access resource: audit", + }); + expect(deniedAction).toEqual({ + success: false, + error: 'unauthorized to access resource "project"', + }); + }); + + it("should preserve AND behavior for unknown action connectors", () => { + const response = role1.authorize({ + project: { actions: ["create", "delete-many"], connector: "XOR" }, + } as never); + + expect(response.success).toBe(false); + }); + + it("should return an unauthorized response for non-string action values", () => { + const response = role1.authorize({ + project: ["create", 1], + } as never); + + expect(response).toEqual({ + success: false, + error: 'unauthorized to access resource "project"', + }); + }); }); diff --git a/packages/better-auth/src/plugins/access/access.ts b/packages/better-auth/src/plugins/access/access.ts index bbec51bf1c..99d064dde9 100644 --- a/packages/better-auth/src/plugins/access/access.ts +++ b/packages/better-auth/src/plugins/access/access.ts @@ -11,6 +11,98 @@ export type AuthorizeResponse = | { success: false; error: string } | { success: true; error?: never | undefined }; +type Connector = "OR" | "AND"; + +type NormalizedActionRequest = { + actions: unknown[]; + connector: Connector; +}; + +function unknownResourceResponse(requestedResource: string): AuthorizeResponse { + return { + success: false, + error: `You are not allowed to access resource: ${requestedResource}`, + }; +} + +function unauthorizedResourceResponse( + requestedResource: string, +): AuthorizeResponse { + return { + success: false, + error: `unauthorized to access resource "${requestedResource}"`, + }; +} + +function normalizeConnector(connector: unknown): Connector { + return connector === "OR" ? "OR" : "AND"; +} + +function isActionList(actions: unknown): actions is unknown[] { + return Array.isArray(actions); +} + +function normalizeActionRequest( + requestedActions: unknown, +): NormalizedActionRequest { + if (isActionList(requestedActions)) { + return { + actions: requestedActions, + connector: "AND", + }; + } + + if (!requestedActions || typeof requestedActions !== "object") { + throw new BetterAuthError("Invalid access control request"); + } + + const { actions, connector } = requestedActions as { + actions?: unknown; + connector?: unknown; + }; + + if (!isActionList(actions)) { + return { + actions: [], + connector: normalizeConnector(connector), + }; + } + + return { + actions, + connector: normalizeConnector(connector), + }; +} + +function hasAllowedAction( + allowedActions: readonly string[], + requestedAction: unknown, +) { + return ( + typeof requestedAction === "string" && + allowedActions.includes(requestedAction) + ); +} + +function isResourceAuthorized( + allowedActions: readonly string[], + { actions, connector }: NormalizedActionRequest, +) { + if (actions.length === 0) { + return false; + } + + if (connector === "OR") { + return actions.some((requestedAction) => + hasAllowedAction(allowedActions, requestedAction), + ); + } + + return actions.every((requestedAction) => + hasAllowedAction(allowedActions, requestedAction), + ); +} + export function role< const TRoleStatements extends Statements, TAuthorizeStatements extends Statements = TRoleStatements, @@ -22,64 +114,36 @@ export function role< request: RoleAuthorizeRequest, connector: "OR" | "AND" = "AND", ): AuthorizeResponse { - let success = false; + let hasAuthorizedResource = false; for (const [requestedResource, requestedActions] of Object.entries( request, )) { const allowedActions = statements[requestedResource]; if (!allowedActions) { if (connector === "AND") { - return { - success: false, - error: `You are not allowed to access resource: ${requestedResource}`, - }; + return unknownResourceResponse(requestedResource); } - success = false; continue; } - if (Array.isArray(requestedActions)) { - success = - requestedActions.length > 0 && - (requestedActions as string[]).every((requestedAction) => - allowedActions.includes(requestedAction), - ); - } else { - if (typeof requestedActions === "object") { - const actions = requestedActions as { - actions: string[]; - connector: "OR" | "AND"; - }; - if ( - !Array.isArray(actions.actions) || - actions.actions.length === 0 - ) { - success = false; - } else if (actions.connector === "OR") { - success = actions.actions.some((requestedAction) => - allowedActions.includes(requestedAction), - ); - } else { - success = actions.actions.every((requestedAction) => - allowedActions.includes(requestedAction), - ); - } - } else { - throw new BetterAuthError("Invalid access control request"); - } + + const isAuthorized = isResourceAuthorized( + allowedActions, + normalizeActionRequest(requestedActions), + ); + if (isAuthorized) { + hasAuthorizedResource = true; } - if (success && connector === "OR") { - return { success }; + + if (isAuthorized && connector === "OR") { + return { success: true }; } - if (!success && connector === "AND") { - return { - success: false, - error: `unauthorized to access resource "${requestedResource}"`, - }; + if (!isAuthorized && connector === "AND") { + return unauthorizedResourceResponse(requestedResource); } } - if (success) { + if (hasAuthorizedResource) { return { - success, + success: true, }; } return {