Repository navigation
feat: pass namespaceFilter to GET /workspacekinds from frontend - #1010
Conversation
google-oss-prow
Bot
added
do-not-merge/work-in-progress
area/frontend
c8cce93 to
7c91037
Compare
The backend added a `namespaceFilter` query param to GET /workspacekinds (kubeflow#1009). Without it, the endpoint requires cluster-wide `list workspacekinds` permission (admin-only). With the filter, auth falls back to checking `create workspaces` in the given namespace, allowing regular users access. The frontend was always calling `listWorkspaceKinds()` without the filter, so non-admin users would get 403 when auth is enabled. Changes: - Update swagger.version to include the namespaceFilter swagger changes - Regenerate the API client from the updated swagger spec - Add optional `namespaceFilter` param to the `useWorkspaceKinds` hook - Thread namespace through WorkspaceFormKindSelection and WorkspaceTable - Add unit test coverage for the new parameter - Sync package-lock.json engines field with package.json - `package.json` updated as part of kubeflow#1003 but `npm` was not ran to reflect this in the lock file Signed-off-by: Andy Stoneberg <astonebe@redhat.com>
7c91037 to
8b521dd
Compare
|
/ok-to-test |
Verificationin "normal" tilt mode
- with auth disabled - this doesn't truly confirm things are working - but here we see `namespaceFilter` at least properly being passed
with auth enabledMethodologyUploading auth-test-verification.md… UI Test (relying on Envoy Filter configured above)
- ℹ️ **EXPECTED** our `regular` user does **NOT** have `list workspacekinds`
- ℹ️ **EXPECTED** our `regular` user **DOES** is able to "see" workspacekinds through the Wizard
|
thaorell
left a comment
thaorell
left a comment
There was a problem hiding this comment.
Rendering a 403 error page for non-admin users while still having the link on the NavBar is confusing. Can we either address that here or open a separate issue for it?
| }, | ||
| "engines": { | ||
| "node": ">=20.0.0" | ||
| "node": "^20", |
There was a problem hiding this comment.
Why are we applying these constraints to npm and node?
There was a problem hiding this comment.
discussed in prior PR that was merged (but npm install was never ran which is why we see package-lock.json having this now)
There was a problem hiding this comment.
short answer: reacting to improve our security posture wherever possible in light of recent spate of supply chain attacks
There was a problem hiding this comment.
i would prefer to open a separate issue for hiding workspace kinds page so we can get this in ASAP
this was not a new issue introduced as part of my PR... its always been this way - its just that developers basically NEVER run with auth enabled.. and then would ALSO need to run with a non-admin user to see this...
so its always been broken - but agree on your recommendation to be clear!
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ederign The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
github-project-automation
Bot
moved this from Needs Triage
to Done
in Kubeflow Notebooks
The backend added a `namespaceFilter` query param to GET /workspacekinds (#1009). Without it, the endpoint requires cluster-wide `list workspacekinds` permission (admin-only). With the filter, auth falls back to checking `create workspaces` in the given namespace, allowing regular users access. The frontend was always calling `listWorkspaceKinds()` without the filter, so non-admin users would get 403 when auth is enabled. Changes: - Update swagger.version to include the namespaceFilter swagger changes - Regenerate the API client from the updated swagger spec - Add optional `namespaceFilter` param to the `useWorkspaceKinds` hook - Thread namespace through WorkspaceFormKindSelection and WorkspaceTable - Add unit test coverage for the new parameter - Sync package-lock.json engines field with package.json - `package.json` updated as part of #1003 but `npm` was not ran to reflect this in the lock file Signed-off-by: Andy Stoneberg <astonebe@redhat.com>
closes: #738
related: #860
The backend added a
namespaceFilterquery param to GET /workspacekinds (#1009). Without it, the endpoint requires cluster-widelist workspacekindspermission (admin-only). With the filter, auth falls back to checkingcreate workspacesin the given namespace, allowing regular users access.The frontend was always calling
listWorkspaceKinds()without the filter, so non-admin users would get 403 when auth is enabled.Changes:
namespaceFilterparam to theuseWorkspaceKindshook