mgrossman/aircore 771 entity rbac returns 403 for authenticated users on nmpdev#535
Closed
matthewgrossman wants to merge 7 commits into
Closed
Conversation
Signed-off-by: Matthew Grossman <[email protected]>
…3-for-authenticated-users-on-nmpdev Signed-off-by: Matthew Grossman <[email protected]>
Signed-off-by: Matthew Grossman <[email protected]>
Contributor
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughSeparates entity read and write authorization, updates the related tests and permissions reference, and adds a dedicated auth-enabled Kind E2E job. ChangesEntities permission split
Auth e2e CI job
Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant KindCluster
participant PytestAuthSuite
GitHubActions->>KindCluster: create auth-enabled cluster from kind-auth.yaml
GitHubActions->>PytestAuthSuite: run e2e/test_jobs_auth.py with NMP_BASE_URL
PytestAuthSuite->>KindCluster: exercise auth e2e endpoints
GitHubActions->>KindCluster: collect kubernetes logs and artifacts
GitHubActions->>KindCluster: delete cluster
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
e2e/k8s/values/kind-auth.yaml (1)
1-4: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueUpdate the header to avoid implying layered values.
install_helm_e2e.shalways passes a single-f, sokind-auth.yamlis a standalone auth-enabled values file, not an overlay onkind.yaml.🤖 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 `@e2e/k8s/values/kind-auth.yaml` around lines 1 - 4, The header comment in kind-auth.yaml implies it extends kind.yaml, but install_helm_e2e.sh only uses a single values file. Update the top-of-file description to clearly state that kind-auth.yaml is a standalone auth-enabled CPU-only Kubernetes E2E values file, and remove any wording that suggests layering or overlay behavior; keep the comment aligned with the file’s role and reference the kind-auth.yaml header text.
🤖 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 `@e2e/k8s/values/kind-auth.yaml`:
- Around line 1-4: The header comment in kind-auth.yaml implies it extends
kind.yaml, but install_helm_e2e.sh only uses a single values file. Update the
top-of-file description to clearly state that kind-auth.yaml is a standalone
auth-enabled CPU-only Kubernetes E2E values file, and remove any wording that
suggests layering or overlay behavior; keep the comment aligned with the file’s
role and reference the kind-auth.yaml header text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4d92f748-47df-4a03-942d-b054d3b9ef99
📒 Files selected for processing (5)
.github/workflows/ci.yamldocs/auth/authorization/permissions-reference.mdxe2e/k8s/values/kind-auth.yamle2e/test_jobs_auth.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yaml
💤 Files with no reviewable changes (1)
- e2e/test_jobs_auth.py
Rebuild policy.wasm to include entities permissions for Viewer/Editor. Update TestGenericEntitiesApiBlocked -> TestGenericEntitiesApiPermissions to verify: Viewer can read, Editor can CRUD, Admin and services allowed. Signed-off-by: Matthew Grossman <[email protected]>
Contributor
|
…ind job - Update integration test to expect 200 (not 403) for entity read - Exclude test_jobs_auth.py from kind-cpu-e2e (no auth) to prevent false failures Signed-off-by: Matthew Grossman <[email protected]>
--ignore doesn't work with pytest when the file is inside a directory argument. --deselect properly excludes it. Signed-off-by: Matthew Grossman <[email protected]>
Signed-off-by: Matthew Grossman <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
has_role: falseto stop 403s in totSummary by CodeRabbit
entities.readis granted to Viewer (and Admin/Editor), whileentitiescreate/update/delete require Editor/Admin.entities.readfromentities.(create|update|delete).