Skip to content

fix: enforce() should return True if any result is True #119

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.

Sign up for GitHub

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

Closed
adou0510 wants to merge 1 commit into from
Closed

fix: enforce() should return True if any result is True #119

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions src/casdoor/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,8 @@ def oauth_token_request(
:param password: username password
:return: Response from Casdoor
"""
params = self._get_payload_for_access_token_request(code=code, username=username, password=password)
params = self._get_payload_for_access_token_request(
code=code, username=username, password=password)
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These manual line breaks don’t match the repository’s Black formatting (see pre-commit/CI), and Black will likely collapse this call back to a single line (or reflow it with the closing paren on its own line). Please run Black or format this call in a Black-compliant way to avoid CI/pre-commit failures.

Suggested change
code=code, username=username, password=password)
code=code,
username=username,
password=password,
)

Copilot uses AI. Check for mistakes.
return self._oauth_token_request(payload=params)

def _oauth_token_request(self, payload: Dict) -> requests.Response:
Expand Down Expand Up @@ -292,7 +293,8 @@ def parse_jwt_token(self, token: str, **kwargs) -> Dict:
:param token: access_token
:return: the data in dict format
"""
certificate = x509.load_pem_x509_certificate(self.certification, default_backend())
certificate = x509.load_pem_x509_certificate(
self.certification, default_backend())
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This call formatting appears non-Black-compliant (multiline without a trailing comma and with the closing parenthesis on the last argument line). Since CI runs Black via pre-commit, please reformat with Black to prevent formatting check failures.

Suggested change
self.certification, default_backend())
self.certification,
default_backend(),
)

Copilot uses AI. Check for mistakes.

return_json = jwt.decode(
token,
Expand Down Expand Up @@ -324,7 +326,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)
params = _build_enforce_params(
permission_id, model_id, resource_id, enforce_id, owner)
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This multiline _build_enforce_params(...) call is unlikely to be Black-compliant (Black will reflow it, typically either to a single line or with a trailing comma + closing paren on its own line). Please run Black or adjust formatting accordingly so CI/pre-commit passes.

Suggested change
permission_id, model_id, resource_id, enforce_id, owner)
permission_id,
model_id,
resource_id,
enforce_id,
owner,
)

Copilot uses AI. Check for mistakes.

r = requests.post(
url,
Expand All @@ -340,9 +343,16 @@ def enforce(
if isinstance(response, dict):
data = response.get("data")
if isinstance(data, list) and len(data) > 0:
has_permission = data[0]
# Iterate through all results, return True if any is True
for result in data:
if not isinstance(result, bool):
error_str = "Casdoor response error:\n" + r.text
raise ValueError(error_str)
if result:
return True
return False
Comment on lines +346 to +353
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CasdoorSDK.enforce() now returns True if any boolean in data is True, but AsyncCasdoorSDK.enforce() (src/casdoor/async_main.py) still returns only data[0]. This creates inconsistent behavior between the sync and async SDKs for the same API; consider updating the async implementation in the same way to keep parity.

Copilot uses AI. Check for mistakes.
Comment on lines +346 to +353
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated multi-result aggregation behavior (return True if any result is True) isn’t covered by existing tests: current enforce tests only assert the return type. Please add a test that mocks/fixtures the Casdoor response with data: [False, True] (and [False, False]) to verify the new semantics and prevent regressions.

Copilot uses AI. Check for mistakes.
else:
has_permission = response
has_permission = data if data is not None else response
else:
has_permission = response
if not isinstance(has_permission, bool):
Expand Down Expand Up @@ -370,7 +380,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)
params = _build_enforce_params(
permission_id, model_id, "", enforce_id, owner)
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This _build_enforce_params(...) call has the same non-Black-compliant multiline formatting pattern as above. Please run Black or reformat so the pre-commit Black hook in CI doesn’t fail.

Suggested change
permission_id, model_id, "", enforce_id, owner)
permission_id,
model_id,
"",
enforce_id,
owner,
)

Copilot uses AI. Check for mistakes.

r = requests.post(
url,
Expand Down
Loading