read only group and extra logging#1307
Conversation
Reviewer's GuideIntroduces a hardcoded Lightwell read-only group with special handling in DomainBasedPermission for non-PyPI lightwell endpoints, adds detailed PyPI access logging, and provides unit and functional tests plus release notes to validate and document the behavior. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
…p changes Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="pulp_service/pulp_service/tests/functional/test_lightwell_readonly_group_permission.py" line_range="176-178" />
<code_context>
+ assert response.status_code == 403
+
+
+def test_readonly_group_member_write_denied(configure_lightwell_domain, gen_readonly_group_member):
+ """The read-only group grants no write access -- a member must still be denied when
+ trying to create a repository in the lightwell domain."""
+ repos_url, _, _ = configure_lightwell_domain
+ headers = {"x-rh-identity": gen_readonly_group_member("write")}
+
+ response = requests.post(repos_url, headers=headers, json={"name": str(uuid4())}, timeout=30)
+
+ assert response.status_code in (401, 403)
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider tightening the assertion to explicitly expect a 403, to prove this is an authorization failure rather than an authentication issue.
This test currently accepts either 401 or 403 for a write attempt by a read-only group member. Since we want to validate that DomainBasedPermission is denying write access (rather than a missing/invalid identity), please assert specifically on 403, and optionally assert on a permission-specific error code in the response body if available, to make the authorization behavior explicit.
```suggestion
response = requests.post(repos_url, headers=headers, json={"name": str(uuid4())}, timeout=30)
assert response.status_code == 403
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| response = requests.post(repos_url, headers=headers, json={"name": str(uuid4())}, timeout=30) | ||
|
|
||
| assert response.status_code in (401, 403) |
There was a problem hiding this comment.
suggestion (testing): Consider tightening the assertion to explicitly expect a 403, to prove this is an authorization failure rather than an authentication issue.
This test currently accepts either 401 or 403 for a write attempt by a read-only group member. Since we want to validate that DomainBasedPermission is denying write access (rather than a missing/invalid identity), please assert specifically on 403, and optionally assert on a permission-specific error code in the response body if available, to make the authorization behavior explicit.
| response = requests.post(repos_url, headers=headers, json={"name": str(uuid4())}, timeout=30) | |
| assert response.status_code in (401, 403) | |
| response = requests.post(repos_url, headers=headers, json={"name": str(uuid4())}, timeout=30) | |
| assert response.status_code == 403 |
Ruff's ARG001 check flagged the unused fixture in changed lines. Co-authored-by: Cursor <cursoragent@cursor.com>
Summary by Sourcery
Introduce a hardcoded Lightwell read-only group and improve Lightwell PyPI access logging and permission handling.
New Features:
Enhancements:
Documentation:
Tests: