Groups: share workspaces and registries with groups (native + OIDC)#346
Merged
Conversation
WorkspaceService.List() was only querying the per-user permissions table, so workspaces shared with a group never appeared in the user's listing even though Casbin's CanReadWorkspace correctly returned true. Now also unions in workspaces whose group_permissions row references any group the user belongs to (resolved via Casbin grouping rules).
✅ Deploy Preview for nebi-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Task 9 instrumented HandleCallback (browser OAuth) and ExchangeIDToken (device flow), but missed the proxy-cookie flow used by gateway-proxied deployments — SessionFromProxy and the Middleware fallback path. Those call findOrCreateProxyUser + syncRolesFromGroups but never reconciled group memberships, so OIDC groups in the ID token were silently dropped on the wire. Caught during in-cluster testing against tyler-hetzner-dev.
The unique index on groups.name doesn't honour gorm.DeletedAt — a soft-deleted row keeps the name reserved at the DB level. The OIDC sync's create-after-delete path hit ERROR: duplicate key value violates unique constraint "idx_groups_name" whenever an admin deleted a native group that an IdP claim later re-introduced. DeleteGroup already hard-removes group_members and Casbin rules; the group row was the only soft-deleted leftover and served no purpose given the cascade. Switched to Unscoped() for the final delete. Also unscoped the GroupPermission delete for consistency (same unique-constraint risk if a future schema adds one). Updated TestDeleteGroup_HardRemovesCasbinRules and added TestDeleteGroup_FreesNameForRecreate as a regression test for the OIDC sync's create-after-delete sequence.
The 'switches to group mode' test passed when the picker source was /groups/me but broke after commit 2ffc541 switched the admin code path to /admin/groups (via useIsAdmin). The default test handlers expose a working /admin/users endpoint so useIsAdmin returns true in tests, and the dialog correctly queries /admin/groups — which was unmocked. Added a default /admin/groups handler returning [] (silences MSW unhandled-request warnings across the suite) and updated the test to mock both endpoints so it works regardless of which one the dialog ends up calling.
Member
|
I am going to review this. |
aktech
reviewed
May 28, 2026
aktech
reviewed
May 29, 2026
aktech
reviewed
May 29, 2026
aktech
reviewed
May 29, 2026
aktech
reviewed
May 29, 2026
aktech
reviewed
May 29, 2026
- rbac: drop redundant enforcer.SavePolicy() from all group functions; the GORM adapter has AutoSave so per-row mutations already persist. Capture previously-ignored RemovePolicy errors in revoke paths. - auth: audit-log OIDC group creation, membership add, and stale membership removal during SyncOIDCGroups (origin: oidc). - service: GrantGroupAdmin rejects OIDC-synced groups with a ConflictError. - docs: remove the groups-rbac plan file.
aktech
approved these changes
Jun 1, 2026
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
2 participants
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.
Summary
Closes #293.
Closes #72
Groupprimitive with native (admin-managed) and OIDC-synced sources.r.sub == p.subtog(r.sub, p.sub)so existing direct policies still match and group memberships resolve transitively. Regression test included.groupsclaim drives JIT membership reconciliation on every login. Refuses to merge claims into existing native groups to prevent permanent untracked grants (regression test included).ui.mdandserver-setup.mdupdated.Deviations from the issue
g(x, x)as true.(\"admin\", \"admin\")to match existing admin scheme (issue said `"*"`).kinddiscriminator instead of separate `users[]`/`groups[]`.Bugs caught during manual walkthrough (already fixed in this branch)
Test plan
Known follow-ups (not blocking)
🤖 Implemented with Claude Code