From 0f06da1fbb0086cb6537b211be4fb122bb286922 Mon Sep 17 00:00:00 2001 From: "bishoy.sameh@progressiosolutions.com" Date: Wed, 1 Oct 2025 16:53:34 +0300 Subject: [PATCH 1/2] fix: correct enforce parameter name from enforceId to enforcerId MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix critical bug in enforce() and batch_enforce() methods where they use incorrect parameter name when calling Casdoor API, causing silent authorization failures for Enforcer objects. Problem: - SDK sends "enforceId" parameter (typo) - API expects "enforcerId" parameter (correct spelling) - Results in silent failures: returns false for all Enforcer checks - No exceptions raised, making it hard to debug - Security concern: appears to work but denies all access Changes: - Fix parameter name: "enforceId" → "enforcerId" in all methods - Add validation to ensure exactly one parameter is provided (API requirement) - Only send non-empty parameters to API - Update docstrings to clarify parameter usage - Apply fix to both sync and async versions - Apply fix to both enforce() and batch_enforce() methods Files changed: - src/casdoor/main.py - enforce() and batch_enforce() - src/casdoor/async_main.py - enforce() and batch_enforce() Impact: This fix enables Enforcer-based authorization to work correctly for: - Custom Casbin models with fine-grained access control - ABAC/ReBAC policies with pattern matching - Facility/attribute-based authorization - Complex authorization scenarios requiring Enforcers Testing: - Verified parameter name changed in all 4 methods - Verified validation logic added correctly - Tested against live Casdoor instance - All existing tests remain compatible (no breaking changes) Fixes #110 --- src/casdoor/async_main.py | 59 +++++++++++++++++++++++++++++---------- src/casdoor/main.py | 58 ++++++++++++++++++++++++++++---------- 2 files changed, 87 insertions(+), 30 deletions(-) diff --git a/src/casdoor/async_main.py b/src/casdoor/async_main.py index 6732e1d..1e139c1 100644 --- a/src/casdoor/async_main.py +++ b/src/casdoor/async_main.py @@ -277,19 +277,34 @@ async def enforce( :param permission_id: the permission id (i.e. organization name/permission name) :param model_id: the model id :param resource_id: the resource id - :param enforce_id: the enforce id + :param enforce_id: the enforcer id (note: uses 'enforcerId' parameter in API) :param owner: the owner of the permission :param casbin_request: a list containing the request data (i.e. sub, obj, act) :return: a boolean value indicating whether the request is allowed """ url = "/api/enforce" - params: Dict[str, str] = { - "permissionId": permission_id, - "modelId": model_id, - "resourceId": resource_id, - "enforceId": enforce_id, - "owner": owner, - } + + # Build params with only non-empty values + # API requires exactly one of: permissionId, modelId, resourceId, enforcerId, owner + params: Dict[str, str] = {} + if permission_id: + params["permissionId"] = permission_id + if model_id: + params["modelId"] = model_id + if resource_id: + params["resourceId"] = resource_id + if enforce_id: + params["enforcerId"] = enforce_id # Fixed: was "enforceId" + if owner: + params["owner"] = owner + + # Validate exactly one parameter is provided + if len(params) != 1: + raise ValueError( + "Exactly one of (permission_id, model_id, resource_id, enforce_id, owner) " + "must be provided and non-empty. " + f"Got {len(params)} parameters: {list(params.keys())}" + ) async with self._session as session: response = await session.post( @@ -328,18 +343,32 @@ async def batch_enforce( :param permission_id: the permission id (i.e. organization name/permission name) :param model_id: the model id - :param enforce_id: the enforce id + :param enforce_id: the enforcer id (note: uses 'enforcerId' parameter in API) :param owner: the owner of the permission :param casbin_request: a list of lists containing the request data :return: a list of boolean values indicating whether each request is allowed """ url = "/api/batch-enforce" - params = { - "permissionId": permission_id, - "modelId": model_id, - "enforceId": enforce_id, - "owner": owner, - } + + # Build params with only non-empty values + # API requires exactly one of: permissionId, modelId, enforcerId, owner + params: Dict[str, str] = {} + if permission_id: + params["permissionId"] = permission_id + if model_id: + params["modelId"] = model_id + if enforce_id: + params["enforcerId"] = enforce_id # Fixed: was "enforceId" + if owner: + params["owner"] = owner + + # Validate exactly one parameter is provided + if len(params) != 1: + raise ValueError( + "Exactly one of (permission_id, model_id, enforce_id, owner) " + "must be provided and non-empty. " + f"Got {len(params)} parameters: {list(params.keys())}" + ) async with self._session as session: response = await session.post( diff --git a/src/casdoor/main.py b/src/casdoor/main.py index c347a94..f005315 100644 --- a/src/casdoor/main.py +++ b/src/casdoor/main.py @@ -281,19 +281,33 @@ def enforce( :param permission_id: the permission id (i.e. organization name/permission name) :param model_id: the model id :param resource_id: the resource id - :param enforce_id: the enforce id + :param enforce_id: the enforcer id (note: uses 'enforcerId' parameter in API) :param owner: the owner of the permission :param casbin_request: a list containing the request data (i.e. sub, obj, act) :return: a boolean value indicating whether the request is allowed """ url = self.endpoint + "/api/enforce" - params = { - "permissionId": permission_id, - "modelId": model_id, - "resourceId": resource_id, - "enforceId": enforce_id, - "owner": owner, - } + + # Build params with only non-empty values + # API requires exactly one of: permissionId, modelId, resourceId, enforcerId, owner + params = {} + if permission_id: + params["permissionId"] = permission_id + if model_id: + params["modelId"] = model_id + if resource_id: + params["resourceId"] = resource_id + if enforce_id: + params["enforcerId"] = enforce_id # Fixed: was "enforceId" + if owner: + params["owner"] = owner + # Validate exactly one parameter is provided + if len(params) != 1: + raise ValueError( + "Exactly one of (permission_id, model_id, resource_id, enforce_id, owner) " + "must be provided and non-empty. " + f"Got {len(params)} parameters: {list(params.keys())}" + ) r = requests.post( url, params=params, @@ -332,18 +346,32 @@ def batch_enforce( :param permission_id: the permission id (i.e. organization name/permission name) :param model_id: the model id - :param enforce_id: the enforce id + :param enforce_id: the enforcer id (note: uses 'enforcerId' parameter in API) :param owner: the owner of the permission :param casbin_request: a list of lists containing the request data :return: a list of boolean values indicating whether each request is allowed """ url = self.endpoint + "/api/batch-enforce" - params = { - "permissionId": permission_id, - "modelId": model_id, - "enforceId": enforce_id, - "owner": owner, - } + + # Build params with only non-empty values + # API requires exactly one of: permissionId, modelId, enforcerId, owner + params = {} + if permission_id: + params["permissionId"] = permission_id + if model_id: + params["modelId"] = model_id + if enforce_id: + params["enforcerId"] = enforce_id # Fixed: was "enforceId" + if owner: + params["owner"] = owner + + # Validate exactly one parameter is provided + if len(params) != 1: + raise ValueError( + "Exactly one of (permission_id, model_id, enforce_id, owner) " + "must be provided and non-empty. " + f"Got {len(params)} parameters: {list(params.keys())}" + ) r = requests.post( url, params=params, From 53ed169337f7e5dcb2334867784b513d72ea0def Mon Sep 17 00:00:00 2001 From: "bishoy.sameh@progressiosolutions.com" Date: Thu, 2 Oct 2025 01:00:35 +0300 Subject: [PATCH 2/2] refactor: extract parameter validation to helper function Reduces cyclomatic complexity of enforce methods to satisfy linter requirements (C901 complexity < 10). --- .DS_Store | Bin 0 -> 6148 bytes src/.DS_Store | Bin 0 -> 6148 bytes src/casdoor/async_main.py | 81 ++++++++++++++++++-------------------- src/casdoor/main.py | 78 ++++++++++++++++++------------------ 4 files changed, 78 insertions(+), 81 deletions(-) create mode 100644 .DS_Store create mode 100644 src/.DS_Store diff --git a/.DS_Store b/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..f0b2b77ffbf515c80e3c2f48bb4c034225d5b45e GIT binary patch literal 6148 zcmeHK%}T>S5T0#gt>~d3g2z08&^L%B>I3vHk``K_B}MH~2ne2h8tnsk^A&stQP1A| zW@l`(#&|3uGcfyY=4WT}CCzq;$o1yKE>VYw+EB*E7KR^${j3#fsAUJ}c#X-Z+aKi9 zStZg8f0Y4#b`c%Wh|cLMcz)61rtF*MX_`&)47T>}^XdEI_~|h84}bN~#-(?o^@vp3 z{HjZ1no|~3r;R~1qTIv7-pTE~^zdz+X};>DJ}T(PzyH*vOS+&z&`0a~1%0TeT>tIk z^-eihdRKL;-k@I1FO5ktF$RnQV_*#oV9#c0ZwuOJ3>X8(z>)#}KEzPQM6nR`pAHQE z2mp*=_JTQ|B{(NmOcV=2cpy$vfs*Rw`c>h1DKmUhGc4Z701OJKv7p48Qhg;IUwQ_U3*T&E@C=16Gg3A<4 i{8o%u-ir63USN-T0!$POL0BO6N5IoygE6pH20j5fwPt(( literal 0 HcmV?d00001 diff --git a/src/.DS_Store b/src/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..f557c9a1e31a9d41f107cb146b459ed4053f24ab GIT binary patch literal 6148 zcmeHKJ5Iwu5S@z%zqPzxBI4;oJ|P+rQ34h09l~^o#6?@uGmiv0-lL#`?pL>y*`jD= zyW>AHAZs_I1>Mkwa;#pxe(N%}J2-xBXRM1Xo3DyFB1w5Net$iGJMGHKf00$*$=hPH z4Ruc1(h~h_X`}CwMfu_i@xd Dict[str, str]: + """ + Build and validate parameters for enforce API calls. + + Exactly one of the parameters must be provided and non-empty. + + :return: Dictionary with exactly one parameter set + :raises ValueError: If zero or multiple parameters are provided + """ + params: Dict[str, str] = {} + if permission_id: + params["permissionId"] = permission_id + if model_id: + params["modelId"] = model_id + if resource_id: + params["resourceId"] = resource_id + if enforce_id: + params["enforcerId"] = enforce_id + if owner: + params["owner"] = owner + + if len(params) != 1: + raise ValueError( + "Exactly one of (permission_id, model_id, resource_id, enforce_id, owner) " + "must be provided and non-empty. " + f"Got {len(params)} parameters: {list(params.keys())}" + ) + + return params + + class AioHttpClient: def __init__(self, base_url): self.base_url = base_url @@ -283,28 +320,7 @@ async def enforce( :return: a boolean value indicating whether the request is allowed """ url = "/api/enforce" - - # Build params with only non-empty values - # API requires exactly one of: permissionId, modelId, resourceId, enforcerId, owner - params: Dict[str, str] = {} - if permission_id: - params["permissionId"] = permission_id - if model_id: - params["modelId"] = model_id - if resource_id: - params["resourceId"] = resource_id - if enforce_id: - params["enforcerId"] = enforce_id # Fixed: was "enforceId" - if owner: - params["owner"] = owner - - # Validate exactly one parameter is provided - if len(params) != 1: - raise ValueError( - "Exactly one of (permission_id, model_id, resource_id, enforce_id, owner) " - "must be provided and non-empty. " - f"Got {len(params)} parameters: {list(params.keys())}" - ) + params = _build_enforce_params(permission_id, model_id, resource_id, enforce_id, owner) async with self._session as session: response = await session.post( @@ -349,26 +365,7 @@ async def batch_enforce( :return: a list of boolean values indicating whether each request is allowed """ url = "/api/batch-enforce" - - # Build params with only non-empty values - # API requires exactly one of: permissionId, modelId, enforcerId, owner - params: Dict[str, str] = {} - if permission_id: - params["permissionId"] = permission_id - if model_id: - params["modelId"] = model_id - if enforce_id: - params["enforcerId"] = enforce_id # Fixed: was "enforceId" - if owner: - params["owner"] = owner - - # Validate exactly one parameter is provided - if len(params) != 1: - raise ValueError( - "Exactly one of (permission_id, model_id, enforce_id, owner) " - "must be provided and non-empty. " - f"Got {len(params)} parameters: {list(params.keys())}" - ) + params = _build_enforce_params(permission_id, model_id, "", enforce_id, owner) async with self._session as session: response = await session.post( diff --git a/src/casdoor/main.py b/src/casdoor/main.py index f005315..a87c5c5 100644 --- a/src/casdoor/main.py +++ b/src/casdoor/main.py @@ -43,6 +43,43 @@ from .webhook import _WebhookSDK +def _build_enforce_params( + permission_id: str, + model_id: str, + resource_id: str, + enforce_id: str, + owner: str, +) -> Dict[str, str]: + """ + Build and validate parameters for enforce API calls. + + Exactly one of the parameters must be provided and non-empty. + + :return: Dictionary with exactly one parameter set + :raises ValueError: If zero or multiple parameters are provided + """ + params = {} + if permission_id: + params["permissionId"] = permission_id + if model_id: + params["modelId"] = model_id + if resource_id: + params["resourceId"] = resource_id + if enforce_id: + params["enforcerId"] = enforce_id + if owner: + params["owner"] = owner + + if len(params) != 1: + raise ValueError( + "Exactly one of (permission_id, model_id, resource_id, enforce_id, owner) " + "must be provided and non-empty. " + f"Got {len(params)} parameters: {list(params.keys())}" + ) + + return params + + class CasdoorSDK( _UserSDK, _AdapterSDK, @@ -287,27 +324,8 @@ def enforce( :return: a boolean value indicating whether the request is allowed """ url = self.endpoint + "/api/enforce" + params = _build_enforce_params(permission_id, model_id, resource_id, enforce_id, owner) - # Build params with only non-empty values - # API requires exactly one of: permissionId, modelId, resourceId, enforcerId, owner - params = {} - if permission_id: - params["permissionId"] = permission_id - if model_id: - params["modelId"] = model_id - if resource_id: - params["resourceId"] = resource_id - if enforce_id: - params["enforcerId"] = enforce_id # Fixed: was "enforceId" - if owner: - params["owner"] = owner - # Validate exactly one parameter is provided - if len(params) != 1: - raise ValueError( - "Exactly one of (permission_id, model_id, resource_id, enforce_id, owner) " - "must be provided and non-empty. " - f"Got {len(params)} parameters: {list(params.keys())}" - ) r = requests.post( url, params=params, @@ -352,26 +370,8 @@ def batch_enforce( :return: a list of boolean values indicating whether each request is allowed """ url = self.endpoint + "/api/batch-enforce" + params = _build_enforce_params(permission_id, model_id, "", enforce_id, owner) - # Build params with only non-empty values - # API requires exactly one of: permissionId, modelId, enforcerId, owner - params = {} - if permission_id: - params["permissionId"] = permission_id - if model_id: - params["modelId"] = model_id - if enforce_id: - params["enforcerId"] = enforce_id # Fixed: was "enforceId" - if owner: - params["owner"] = owner - - # Validate exactly one parameter is provided - if len(params) != 1: - raise ValueError( - "Exactly one of (permission_id, model_id, enforce_id, owner) " - "must be provided and non-empty. " - f"Got {len(params)} parameters: {list(params.keys())}" - ) r = requests.post( url, params=params,