feat(auth): govern workspace creation through RBAC - #531
Conversation
|
🌿 Preview your docs: https://nvidia-preview-auth-workspace-creator.docs.buildwithfern.com/nemo-platform |
|
|
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 (18)
✅ Files skipped from review due to trivial changes (6)
🚧 Files skipped from review as they are similar to previous changes (10)
📝 WalkthroughWalkthroughAdds ChangesWorkspaceCreator RBAC rollout
Sequence Diagram(s)sequenceDiagram
participant Seeder as platform-seed
participant EntityClient as EntityClient
participant PDP as authz.rego
participant Client as API caller
Seeder->>EntityClient: seed_workspace_creator(*, system, WorkspaceCreator)
Client->>PDP: POST /apis/entities/v2/workspaces
PDP->>PDP: require workspaces.create and write scopes
PDP->>PDP: check system WorkspaceCreator binding
PDP-->>Client: allow or deny
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
services/platform-seed/README.md (1)
3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the doc substitution and spell out the bindings.
NeMo Platformis hardcoded here, andwildcard default/system/workspace-creatoris too compressed to be useful reference text. Use the configured product substitution and name the exact bindings, like Line 22 does. As per coding guidelines, "Avoid marketing language ('powerful', 'flexible', 'easy', 'robust') and vague terms ('many', 'various', 'different approaches'); use concrete, specific terminology instead" and "Never hardcode product names; use substitutions in Sphinx configuration to maintain consistency across documentation".🤖 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/platform-seed/README.md` at line 3, Update the README summary text to use the configured product substitution instead of hardcoding NeMo Platform, and replace the compressed “wildcard default/system/workspace-creator” wording with the explicit binding names. Keep the wording concrete and specific, and align the phrasing with the existing seed-job description referenced elsewhere in the document.Source: Coding guidelines
docs/get-started/concepts/workspaces.mdx (1)
63-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove this procedure out of the concepts page.
Lines 63-91 are a task workflow with commands, so they read as HOW-TO content inside a concepts page. Split this into a dedicated how-to and link to it from here; if the procedure stays, add Python SDK and CLI examples in tabs instead of CLI only. As per coding guidelines, "Each documentation page should fit ONE Diataxis quadrant; do not mix tutorials with reference tables or how-tos with architecture explanations; use cross-links instead" and "Provide both Python SDK and CLI examples in tab-sets for consistency and to support multiple user workflows".
🤖 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 `@docs/get-started/concepts/workspaces.mdx` around lines 63 - 91, The “Restrict Workspace Creation” section in the concepts page is a how-to workflow, so move it out of this page and into a dedicated how-to doc, then add a cross-link from the concepts content. If any part must remain here, refactor the procedure in this area into tabbed Python SDK and CLI examples instead of CLI-only, using the existing workspace/RBAC guidance and the “WorkspaceCreator” binding example as the anchor to relocate.Source: Coding guidelines
🤖 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.
Inline comments:
In `@docs/auth/authentication/oidc.mdx`:
- Line 74: Broaden the 403 warning in the OIDC docs so it reflects the full
impact of missing platform seeding: if platform-seed never ran, the seeded
Editor, Viewer, and WorkspaceCreator bindings are also absent, so workspace
creation and default workspace access can fail with 403 as well. Update the
sentence around auth.admin_email and the PlatformAdmin/WorkspaceCreator bindings
to mention these additional seeded bindings and avoid limiting the warning to
admin operations only.
In `@docs/auth/authorization/roles-and-permissions.mdx`:
- Around line 76-84: The workspace permissions matrix in
roles-and-permissions.mdx conflicts with the explanatory note about creation
access. Update the “Create workspace” row in the permissions table to match the
`WorkspaceCreator` rule described in the note, or remove that row entirely if
the table cannot express the `system` workspace binding; make sure the table no
longer implies only `PlatformAdmin` can create workspaces. Use the
`WorkspaceCreator` note and the permissions matrix near “Create workspace” as
the place to align the documentation.
In
`@services/core/auth/src/nmp/core/auth/app/policy_tests/namespace_creation_test.rego`:
- Line 27: The endpoint fixture for the POST case is missing the required
scopes, so the allow/deny policy tests are not covering the new
entities:write/platform:write contract. Update the fixture in
namespace_creation_test.rego alongside the existing permissions entry to include
the appropriate scopes for the endpoint, using the same test data structure used
by the namespace creation policy tests.
In
`@services/core/auth/src/nmp/core/auth/app/policy_tests/unknown_endpoint_test.rego`:
- Line 29: The scoped creation test fixture is not actually requiring the write
scopes it is supposed to validate, so the permission check can be bypassed.
Update the Rego test cases in unknown_endpoint_test and the related scoped
creation fixtures to explicitly require the needed write scope before asserting
workspace creation, and make sure the request permissions align with the
expected enforcement path.
In `@services/core/auth/src/nmp/core/auth/app/seeding.py`:
- Around line 288-300: The bootstrap in seed_workspace_creator() is reapplying
the wildcard WorkspaceCreator binding via _seed_wildcard_binding(), which blocks
operators from keeping workspace creation restricted. Update this seeding path
so a deleted or revoked wildcard binding is treated as an intentional override
rather than being recreated or causing failure, or gate the behavior behind a
one-time/opt-in flag; make the same adjustment for the other affected seeding
call referenced in the review.
In `@services/core/entities/tests/integration/test_workspaces_crud_with_auth.py`:
- Around line 204-229: The test mutates shared RBAC on the system workspace by
changing the wildcard member’s roles but never restores the seeded wildcard
WorkspaceCreator binding, which makes later workspace-creation tests
order-dependent. Update this test around
restrict_workspace_creation_to_named_users, sdk.workspaces.members.create, and
the final sdk.workspaces.create call to re-add or reset the wildcard principal’s
original roles before exiting the test, ideally in a cleanup/finally path so the
shared system membership is restored even if the test fails.
---
Nitpick comments:
In `@docs/get-started/concepts/workspaces.mdx`:
- Around line 63-91: The “Restrict Workspace Creation” section in the concepts
page is a how-to workflow, so move it out of this page and into a dedicated
how-to doc, then add a cross-link from the concepts content. If any part must
remain here, refactor the procedure in this area into tabbed Python SDK and CLI
examples instead of CLI-only, using the existing workspace/RBAC guidance and the
“WorkspaceCreator” binding example as the anchor to relocate.
In `@services/platform-seed/README.md`:
- Line 3: Update the README summary text to use the configured product
substitution instead of hardcoding NeMo Platform, and replace the compressed
“wildcard default/system/workspace-creator” wording with the explicit binding
names. Keep the wording concrete and specific, and align the phrasing with the
existing seed-job description referenced elsewhere in the document.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7ee01af0-fc74-4e96-b4d3-ab47e69b1aa1
📒 Files selected for processing (17)
docs/auth/authentication/oidc.mdxdocs/auth/authorization/roles-and-permissions.mdxdocs/auth/concepts.mdxdocs/auth/deployment/configuration.mdxdocs/get-started/concepts/workspaces.mdxdocs/get-started/setup.mdxservices/core/auth/scripts/auth-tools.pyservices/core/auth/src/nmp/core/auth/app/policies/authz.regoservices/core/auth/src/nmp/core/auth/app/policy_tests/namespace_access_test.regoservices/core/auth/src/nmp/core/auth/app/policy_tests/namespace_creation_test.regoservices/core/auth/src/nmp/core/auth/app/policy_tests/unknown_endpoint_test.regoservices/core/auth/src/nmp/core/auth/app/seeding.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/tests/test_embedded_pdp.pyservices/core/auth/tests/test_seeding.pyservices/core/entities/tests/integration/test_workspaces_crud_with_auth.pyservices/platform-seed/README.md
84500eb to
8989f3a
Compare
Signed-off-by: Ryan S <267728323+ironcommit@users.noreply.github.com>
8989f3a to
326905c
Compare
mckornfield
left a comment
There was a problem hiding this comment.
some minor things, max might have more, I wonder about the default create though (I remember that being a no no for certain customers/larger orgs when anyone with an email could onboard)
Signed-off-by: Ryan S <267728323+ironcommit@users.noreply.github.com>
Summary
Moves workspace creation from a hardcoded policy exception (any authenticated user can create) to a proper RBAC-controlled permission, while preserving the current default behavior through seeding.
What changed
WorkspaceCreatorrole withworkspaces.createpermission added to the static authz config/apis/auth/v2/iam/path prefix checkPOST /apis/entities/v2/workspaces) now requiresworkspaces.createpermission andentities:write/platform:writescopes instead of empty permissions/scopesWorkspaceCreatorbinding in thesystemworkspace, preserving open workspace creation by defaultsystemworkspace: remove the wildcardWorkspaceCreatorand grant it only to specific users/groupsWhy
Previously, workspace creation was a special-cased policy exception — the endpoint had empty permissions, so any authenticated user could create workspaces with no way for operators to restrict it. This change brings workspace creation under the same RBAC model as every other operation, giving operators a knob to control who can create workspaces while keeping the default behavior unchanged.
Files changed
services/core/auth/src/nmp/core/auth/assets/static-authz.yaml— new role, permission, and endpoint configservices/core/auth/src/nmp/core/auth/app/policies/authz.rego— generalized system-workspace fallback ruleservices/core/auth/src/nmp/core/auth/app/seeding.py— newseed_workspace_creator()functionservices/core/auth/scripts/auth-tools.py— updated permission/scope inferenceservices/core/auth/src/nmp/core/auth/app/policy_tests/— updated Rego testsservices/core/auth/tests/test_embedded_pdp.py— new system-scoped permission testservices/core/auth/tests/test_seeding.py— tests for WorkspaceCreator seedingservices/core/entities/tests/integration/test_workspaces_crud_with_auth.py— integration tests for default-open and restricted creationdocs/— updated documentation across auth concepts, roles/permissions, OIDC setup, configuration, workspaces, and setup pagesTest plan
make test-unitpasses (Rego policy tests, seeding tests, embedded PDP tests)test_workspaces_crud_with_auth.pycovers both default-open and restricted creation flowssystemworkspace rolesSummary by CodeRabbit
workspaces.createpermission via aWorkspaceCreatorrole; the creator is granted Admin access in the newly created workspace.entities:writeandplatform:writescopes.WorkspaceCreatorbinding in thesystemworkspace; operators can restrict it by removing/rebinding that wildcard.WorkspaceCreatorbehavior and restriction steps.