feat(EM-37): Implement Role-Based Authorization Framework (RBAC)#58
Open
devin-ai-integration[bot] wants to merge 1 commit intofeat/microservices-migration-v2from
Open
Conversation
- Define role hierarchy: ADMIN > MANAGER > USER, SERVICE (independent) - Add FtgoRole enum with hierarchy and authority management - Add FtgoPermission enum with role-to-permission mappings - Create RoleHierarchyConfiguration with Spring Security RoleHierarchy bean - Enable method-level security (@PreAuthorize, @secured, @RolesAllowed) - Add RoleAuthorizationService for programmatic role/permission checks - Add custom @RequireRole and @RequirePermission annotations with AOP - Add RoleConstants for reusable SpEL security expressions - Register FtgoAuthorizationAutoConfiguration for auto-discovery - Add comprehensive tests (145 total, all passing) - Add RBAC documentation Co-Authored-By: Alex Baker <alexandercommander453@gmail.com>
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
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.
feat(EM-37): Implement Role-Based Authorization Framework (RBAC)
Summary
Extends
libs/ftgo-security/with a comprehensive RBAC framework built on the existing Spring Security and JWT foundations. Adds:ADMIN > MANAGER > USER, plus independentSERVICErole for inter-service communication@PreAuthorize,@Secured, and@RolesAllowed(JSR-250) via@EnableMethodSecurityRoleAuthorizationServicefor imperative role/permission checks@RequireRoleand@RequirePermissionbacked by AOPFtgoPermissionenum mapping granular permissions (e.g.order:read) to rolesRoleConstantswith reusable expressions likeHAS_ROLE_ADMINFtgoAuthorizationAutoConfigurationauto-registers all RBAC beansRBAC.mdwith usage examples and permission matrixReview & Testing Checklist for Human
RoleAuthorizationServicedoes NOT use theRoleHierarchybean. This is the biggest concern.@PreAuthorize("hasRole('USER')")will pass for an ADMIN (via hierarchy), butauthService.hasRole(FtgoRole.USER)will returnfalsefor ADMIN because it does a direct authority string match. This is an inconsistency between declarative and programmatic authorization. Same issue applies to@RequireRole(which delegates to this service). Decide whether this is acceptable or needs fixing.@RequireRole/@RequirePermissionAOP annotations. TheMethodSecurityIntegrationTestonly covers@PreAuthorizeand@Secured. The custom AOP annotations are declared and wired but not exercised in any integration test.TestApplication.javato excludeauthorizationsubpackage from component scanning (to avoid bean conflicts). This works but is a bit fragile — verify the regex patterncom\\.ftgo\\.security\\.authorization\\..*doesn't break anything.FtgoRole.getIncludedRoles()andRoleHierarchyConfiguration.ROLE_HIERARCHY_DEFINITION. These could drift out of sync. Consider whether this duplication is acceptable or if one should be derived from the other.Test Plan
gradle buildinlibs/ftgo-security/— all 145 tests should pass (verified locally with Java 17 + Gradle 8.7)@PreAuthorize("hasRole('USER')")allows ADMIN/MANAGER/USER but not SERVICERoleAuthorizationService.hasRole()and confirm it does NOT respect hierarchy (by design or bug?)@RequireRoleand@RequirePermissionon a controller method to ensure AOP aspect firesroles: ["ROLE_USER"]claim and verify it's extracted and used for authorizationNotes
RoleHierarchyImpl.setHierarchy()(deprecated in newer Spring Security versions but correct for 6.2.4)spring-boot-starter-aopasapidependency (exposes AOP to consumers — may want to change toimplementation)