Skip to content

Scope access-request approvals to services - #185

Open
MUFFANUJ wants to merge 2 commits into
nebari-dev:mainfrom
MUFFANUJ:accessRequest
Open

Scope access-request approvals to services#185
MUFFANUJ wants to merge 2 commits into
nebari-dev:mainfrom
MUFFANUJ:accessRequest

Conversation

@MUFFANUJ

@MUFFANUJ MUFFANUJ commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Reference Issues or PRs

Closes #156

What does this implement/fix?

Put a x in the boxes that apply

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds a feature)
  • Breaking change (fix or feature that would cause existing features not to work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Other (please describe):

Testing

  • Did you test the pull request locally?
  • Did you add new tests?

Documentation

Access-centered content checklist

Text styling

  • The content is written with plain language (where relevant).
  • If there are headers, they use the proper header tags (with only one level-one header: H1 or # in markdown).
  • All links describe where they link to (for example, check the Nebari website).
  • This content adheres to the Nebari style guides.

Non-text content

  • All content is represented as text (for example, images need alt text, and videos need captions or descriptive transcripts).
  • If there are emojis, there are not more than three in a row.
  • Don't use flashing GIFs or videos.
  • If the content were to be read as plain text, it still makes sense, and no information is missing.

Any other comments?

This changes access-request approval from Keycloak group mutation to Launchpad-owned Redis entitlements scoped by user and service UID. Approved access now expires with a configurable TTL, revoke removes the active grant, and REST/WebSocket access checks only honor exact service grants while preserving approver and target-owner audit data. One follow-up I could think of is to add a renewal flow so users or admins can extend expiresAt before expiry for uninterrupted access.

@MUFFANUJ MUFFANUJ mentioned this pull request Aug 7, 2026
18 tasks
@viniciusdc

Copy link
Copy Markdown
Collaborator

Hey @MUFFANUJ, taking this up for review. It's gone stale against main, so I'll rebase directly on your branch to clear the conflicts before I dig in, keeping your changes intact. Heads up that #187 is now marked blocked by #156 since it builds on the entitlement keys this PR adds, so I'll review this one first.

@viniciusdc

Copy link
Copy Markdown
Collaborator

Hey @MUFFANUJ, change of plan on the rebase. I started it and it runs into #197 "remove the notifications", which landed on main after you cut this branch. We decided in the sync to hide the notifications UI for now since it wasn't giving non-admin users anything actionable, so #197 stripped the notification bell and the notification state out of the frontend.

Your branch still carries the old notification code, so a straight rebase brings the UI back. That's a product call rather than a merge fix, so I'd rather hand it back than guess for you.

Good news is it's small. When you rebase on main the only real conflict is in frontend/src/hooks/useLaunchpadData.ts, and your actual change there (the refreshServices useCallback plus calling it on the WS onOpen) rebases cleanly. It's only the inherited notification lines that collide. So keep the service-refresh refactor and drop the notification bits: listNotifications/setNotifications, onNotificationsViewed, the notification.created branch in onMessage, and the AppSocketMessage union. The mock files merge on their own, and the backend is untouched by #197.

Heads up too that I marked #187 blocked by this one, since it extends the same entitlement keys. Once this is green I'll pick the review back up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Scope access-request approval so it cannot perform realm-wide Keycloak group administration

2 participants