Feat/rbac page scoped permissions#141
Merged
ilramdhan merged 8 commits intoJul 1, 2026
Merged
Conversation
… global) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ListByMenu Domain Permission gains menuID/menuTitle + getters; NewPermission/ReconstructPermission signatures updated (all callers threaded). Repository INSERT/UPDATE/SELECT handle menu_id with LEFT JOIN mst_menu for menu_title, ordered by sort_order NULLS LAST, action_type. Adds ListByMenu. CreateCommand + gRPC CreatePermission thread menu_id through; permissionToDetailProto exposes MenuId/MenuTitle. Integration tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ate + list filter Task 6: NewPermission and Permission.Update reject empty/whitespace description (new ErrEmptyDescription sentinel; table tests added). Task 7: UpdateCommand/Permission.Update accept menuID; gRPC UpdatePermission parses menu_id; ListPermissions supports menu_id filter (PermissionListParams.MenuID, WHERE builder extracted to buildPermissionListFilter to keep List under gocyclo 15). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
permissionSelectCols lacked role_count (only List computed it via subquery), so the by-service catalog showed 0 roles for every permission. Add the same role_permissions COUNT subquery + scan it in GetByService. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ately
DirectPermissions was populated from the effective union (direct + role-inherited),
so the direct-permission UI treated every effective permission as a removable direct
grant (e.g. super-admin showed 222 'direct'). Now DirectPermissions = GetUserDirectPermissions
(user_permissions rows only); AllPermissionCodes stays the effective union. This lets
the UI mark role-inherited permissions read-only ('from role') and prevent redundant
direct grants.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces page-scoped (menu-scoped) RBAC permissions in the IAM service by adding menu_id to permissions, backfilling existing permissions to menus, and exposing/filtering this information through repositories and gRPC/OpenAPI/proto models.
Changes:
- Add
mst_permission.menu_id(FK tomst_menu) plus an index, and backfill existing permission rows to menu pages via a migration. - Extend permission persistence/query paths to LEFT JOIN
mst_menuformenu_title, add menu-based filtering and aListByMenurepository method. - Propagate
menuId/menuTitlethrough domain, gRPC handlers, and generated OpenAPI/proto artifacts; add/extend tests.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| services/iam/migrations/postgres/000065_add_menu_id_to_permission.up.sql | Adds nullable menu_id FK column to mst_permission and an index for active rows. |
| services/iam/migrations/postgres/000065_add_menu_id_to_permission.down.sql | Drops the new index and column on rollback. |
| services/iam/migrations/postgres/000066_backfill_permission_menu_id.up.sql | Backfills mst_permission.menu_id using permission_code → menu_code mapping. |
| services/iam/migrations/postgres/000066_backfill_permission_menu_id.down.sql | Clears menu_id for the mapped permission codes on rollback. |
| services/iam/internal/infrastructure/postgres/role_repository.go | Extends scanned permission rows to include menu_id and menu_title when reconstructing domain permissions. |
| services/iam/internal/infrastructure/postgres/permission_repository.go | Centralizes permission SELECT columns + menu join, adds menu filter/order, and introduces ListByMenu. |
| services/iam/internal/infrastructure/postgres/permission_repository_test.go | Adds integration tests covering menu_id round-trip and menu-scoped listing. |
| services/iam/internal/infrastructure/postgres/menu_repository.go | Updates permission reconstruction signature usage after adding menu fields. |
| services/iam/internal/domain/role/repository.go | Extends repository interface with ListByMenu and adds MenuID filter param. |
| services/iam/internal/domain/role/entity.go | Adds menuID/menuTitle to Permission entity, validates non-empty descriptions, and extends update semantics. |
| services/iam/internal/domain/role/entity_test.go | Updates tests for new Permission constructors and adds empty-description cases. |
| services/iam/internal/delivery/grpc/user_handler.go | Adjusts direct permissions vs effective permissions in GetUserRolesAndPermissions. |
| services/iam/internal/delivery/grpc/role_handler.go | Adds menuId/menuTitle fields to PermissionDetail proto mapping. |
| services/iam/internal/delivery/grpc/permission_handler.go | Parses menuId for create/update and threads menuId through list requests. |
| services/iam/internal/application/permission/update_handler.go | Passes MenuID through domain update call. |
| services/iam/internal/application/permission/list_handler.go | Threads MenuID through to repository list params. |
| services/iam/internal/application/permission/handlers_test.go | Updates mocks/tests for new permission reconstruction signature and repository interface method. |
| services/iam/internal/application/permission/create_handler.go | Passes optional MenuID into the domain permission constructor. |
| gen/openapi/iam/v1/role.swagger.json | Regenerates OpenAPI with menuId filtering and permission menu fields. |
| gen/iam/v1/role.pb.go | Regenerates protobuf Go output with menu_id/menu_title fields and request filtering. |
Files not reviewed (1)
- gen/iam/v1/role.pb.go: Generated file
Comments suppressed due to low confidence (2)
services/iam/internal/application/permission/update_handler.go:52
- This handler now supports updating MenuID, but the postgres PermissionRepository.Update SQL does not update mst_permission.menu_id. Menu scoping changes will be applied in-memory and then silently lost when persisted.
if err := entity.Update(cmd.Name, cmd.Description, cmd.IsActive, cmd.MenuID, cmd.UpdatedBy); err != nil {
return nil, err
}
// 4. Persist
services/iam/internal/infrastructure/postgres/permission_repository.go:236
- ListPermissions uses a user-provided SortBy in ORDER BY; the fallback
sortBy = params.SortByallows arbitrary SQL fragments and can lead to ORDER BY injection or runtime SQL errors. SortBy should be restricted to an explicit allowlist of supported fields (including any API synonyms).
if params.SortBy != "" {
if mapped, ok := sortColumnMap[params.SortBy]; ok {
sortBy = mapped
} else {
sortBy = params.SortBy
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
179
to
183
| ServiceName: req.GetServiceName(), | ||
| ModuleName: req.GetModuleName(), | ||
| ActionType: req.GetActionType(), | ||
| MenuID: req.GetMenuId(), | ||
| SortBy: req.GetSortBy(), |
Comment on lines
+449
to
+460
| protoDirect := []*iamv1.Permission{} | ||
| if userID, parseErr := uuid.Parse(req.GetUserId()); parseErr == nil { | ||
| if directPerms, dErr := h.userPermissionRepo.GetUserDirectPermissions(ctx, userID); dErr == nil { | ||
| protoDirect = make([]*iamv1.Permission, len(directPerms)) | ||
| for i, p := range directPerms { | ||
| protoDirect[i] = &iamv1.Permission{ | ||
| PermissionId: p.ID().String(), | ||
| PermissionCode: p.Code(), | ||
| } | ||
| } | ||
| } | ||
| } |
Comment on lines
273
to
+277
| p.isActive = *isActive | ||
| } | ||
| if menuID != nil { | ||
| p.menuID = menuID | ||
| } |
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
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.
Description
Type of Change
Service(s) Affected
Changes Made
Related Issues
Fixes #
Related to #
API Changes (if applicable)
Proto Changes
Breaking Changes
Testing Performed
Unit Tests
Integration Tests
Manual Testing
# Commands used for testing grpcurl -plaintext localhost:50051 ... curl http://localhost:8080/...Lint & Build
golangci-lint run ./...passesgo build ./...succeedsgo test -race ./...passesDatabase (if applicable)
Documentation
Rollback Plan
Screenshots/Logs (if applicable)
Pre-merge Checklist
Reviewer Notes