test(auth): add integration tests for group-based role binding enforcement#468
Conversation
…ement Add two integration tests verifying that JWT bearer token group claims are correctly evaluated against group-based role bindings in the embedded policy engine: * A token carrying the bound group claim gains access (Viewer can read, cannot write) * A token whose groups do not include the bound group is denied access Also updates the test_client fixture to configure AuthConfig with allow_unsigned_jwt and embedded PDP so the tests run without a real identity provider. Signed-off-by: Ryan S <267728323+ironcommit@users.noreply.github.com>
📝 WalkthroughWalkthroughAdds integration coverage for role-binding propagation with bearer-token ChangesGroup claim propagation
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
services/core/auth/tests/integration/test_role_binding_propagation.py (1)
107-148: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffDenial test can pass for the wrong reason.
The token carries
other_group, which is never bound. A403therefore proves nothing about group-claim matching—it also passes if the role binding never propagated at all. Add a positive control: assert the bound group would grant access (e.g., a second token withbound_groupgets200), then assert the non-matching token gets403.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/core/auth/tests/integration/test_role_binding_propagation.py` around lines 107 - 148, The denial test in test_group_role_binding_denies_workspace_access_when_group_missing can succeed for the wrong reason because it only checks that a token with other_group gets 403. Add a positive control in the same test by creating a second bearer token that includes bound_group and assert it can read the workspace successfully before checking that the non-matching token is denied. Use the existing helpers generate_unsigned_jwt, WORKSPACES_PATH, IAM_ROLE_BINDINGS_PATH, and the workspace access GET to verify both the expected allow and deny paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@services/core/auth/tests/integration/test_role_binding_propagation.py`:
- Around line 107-148: The denial test in
test_group_role_binding_denies_workspace_access_when_group_missing can succeed
for the wrong reason because it only checks that a token with other_group gets
403. Add a positive control in the same test by creating a second bearer token
that includes bound_group and assert it can read the workspace successfully
before checking that the non-matching token is denied. Use the existing helpers
generate_unsigned_jwt, WORKSPACES_PATH, IAM_ROLE_BINDINGS_PATH, and the
workspace access GET to verify both the expected allow and deny paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 034974db-eeed-4cfe-b198-243f09cc0552
📒 Files selected for processing (1)
services/core/auth/tests/integration/test_role_binding_propagation.py
|
Add two integration tests verifying that JWT bearer token group claims are correctly evaluated against group-based role bindings in the embedded policy engine:
Also updates the test_client fixture to configure AuthConfig with allow_unsigned_jwt and embedded PDP so the tests run without a real identity provider.
Summary by CodeRabbit