Repository navigation
feat(rbac): roles, permissions, method security and audit logging - #3
Merged
Merged
Conversation
…way V2 A data migration inserts the 14 permissions (USER, ROLE, PERMISSION, CATEGORY, PRODUCT, CUSTOMER and ORDER, each with READ and WRITE), an ADMIN role and the assignment of every permission to it. Without this seed nobody could ever hold ROLE:WRITE, so nobody could create the first assignment. Every insert is idempotent (ON CONFLICT DO NOTHING) and no user is seeded: the first administrator comes from the optional, configuration-driven bootstrap.
Services return dto types only, so a page of results needs a business-side container: PageDto carries the content, the page number, the size and the total number of elements, immutable and without any HTTP concern. The web layer converts it to its own PageResponse payload, keeping the core free of web types.
Three more table-shaped classes documented with @table, @id and @column: roles, permissions (a resource and an action, such as ROLE and WRITE) and the audit trail whose actor and entity id are nullable because a system action has no actor and a failed creation has no id yet. As for the other entities, the annotations are documentation only, no JPA provider reads them.
The business shapes returned by the RBAC service. RoleDto carries the permissions granted to the role as PermissionDto objects, which lets a role list show what each role can do without the web layer touching a DAO. They are separate classes from their entities, in line with the five-layer split.
Each RowMapper converts one ResultSet row into an entity and nothing more. The audit log mapper reads actor_user_id and entity_id as nullable Long values, using getObject so that SQL NULL is not silently turned into 0 by getLong. The DAOs of the RBAC part use them through JdbcTemplate.query.
These mappers convert between entity and dto in both directions and are the only place where that happens for the RBAC types. The role mapper leaves the permission list empty: filling it needs a query, so the service does it. They hold no business logic and never query the database.
The DAOs own the SQL of the RBAC tables: CRUD for roles and permissions, the role_permission join rows (permissions are assigned onto a role, never the reverse), counts used by the referenced-delete rules, and an insert-only audit log DAO. They extend AbstractDao, work in entity types only and have no stereotype annotation. Duplicate keys surface as DuplicateKeyException, translated by JdbcTemplate itself, and the service turns them into business refusals.
UserDao gains a LIMIT/OFFSET page query, the role_user assignment methods (roles are assigned onto a user) and countLastAdminCandidates, which counts distinct enabled and unlocked users holding ROLE:WRITE. acquireLastAdminLock runs pg_advisory_xact_lock: a transaction-scoped lock, released at commit or rollback, that serialises every operation able to remove the last administrator, so two concurrent requests cannot both pass the check.
log joins the business transaction, so a success row disappears if the operation rolls back. logRejected uses REQUIRES_NEW: it suspends the running transaction and commits its own, so a refusal row survives the rollback that follows the refusal. Propagation is applied by the Spring proxy, so it only works when the caller goes through the interface bean, which is how RbacServiceImpl uses it. The actor comes from an ActorProvider interface that the web layer implements, so core never reads a SecurityContext.
RbacServiceImpl covers the CRUD of roles and permissions and the two assignment directions: permissions onto a role, roles onto a user. A role still held by a user, or a permission still held by a role, cannot be deleted. Every success calls the AuditLogger and every business refusal is audited in an independent transaction before the exception is thrown, while 404 cases are not audited. Operations that can remove the last ROLE:WRITE holder take the advisory lock as their very first statement, count the eligible administrators before and after the change, and refuse when the count drops from at least one to zero, which rolls the change back. The count reads database state, not live tokens.
DaoConfig and ServiceConfig declare the new DAOs, the AuditLogger and the RbacService as explicit beans typed by interface, so transactions apply through their proxies. Any context importing ServiceConfig must provide an ActorProvider. AdminBootstrap is driven only by app.bootstrap.admin-email and app.bootstrap.admin-password: when absent it does nothing, otherwise it creates that enabled user with the ADMIN role if the email is unknown. Every failure is caught and logged, so it can never stop the application from starting. It is meant for development only.
A PermissionEvaluator answers hasPermission(target, permission) calls made from @PreAuthorize expressions. This one is the single place permissions are resolved: it checks whether the authenticated user holds the authority RESOURCE:ACTION, an authority list rebuilt from the JWT claim, so there is no database call per request. It never throws and is false for anonymous or null authentications. SecurityContextActorProvider gives the audit logger the id of the caller.
@EnableMethodSecurity turns on @PreAuthorize through AOP proxies on the controller beans. The MethodSecurityExpressionHandler is a static @bean method, so it is available early enough when Spring builds the method security advisors, and it registers CustomPermissionEvaluator so that hasPermission('ROLE','WRITE') is understood. The config also exposes the ActorProvider consumed by the core audit logger.
RoleRequest and PermissionRequest carry Bean Validation constraints (not blank, bounded length, letters and underscores for a permission), and the responses expose the roles with their permissions. Converters map between the core dto classes and these payloads with static methods, and UserConverter learns to turn a PageDto into a PageResponse. Payload classes stay separate from core dto classes on purpose.
The three controllers follow the README table exactly. Each method is guarded by @PreAuthorize("hasPermission('RESOURCE','ACTION')"), never a manual if, and only calls RbacService and the converters. The user list takes page and size parameters constrained with @min and @max, which Spring 6.1 method validation checks without any class-level annotation. Every answer is an ApiResponse, creations return 201 and assignments return the updated user or role.
An AccessDeniedException raised by @PreAuthorize reaches the advice before the security filters, so the handler must decide between 401 and 403 itself: an anonymous or missing authentication now gets 401, an authenticated caller without the right gets 403. Constraint violations on request parameters now read "size: must be less than or equal to 100", naming the parameter like the "field: message" entries of body validation errors.
The migration test now checks that the seed created exactly 14 permissions (seven resources times READ and WRITE), one ADMIN role holding all of them, 14 role_permission rows in total, and no user and no role assignment. Pinning the exact count means a later branch that adds a seeded permission must update this test on purpose.
Role, permission, audit log and the new user DAO methods each run against a real database with their own dedicated fixture file, so the existing user fixture stays private to its own test class. They check duplicates, foreign-key blocked deletes, NULL actor and entity ids, page offsets and role assignments. countLastAdminCandidates is tested to count only enabled and unlocked holders of ROLE:WRITE, once per user, and the advisory lock is shown to block a second transaction until the first one commits.
RowMapper tests use a mocked ResultSet, including the NULL actor, NULL entity id and NULL timestamp of audit rows, and the entity mappers are checked with hand-built objects in both directions. Neither needs a database, which keeps the mapping contract tests instant.
Each mutation is checked for its audit call, each business refusal for a logRejected call made before the exception, and 404 cases for the absence of any audit call. An InOrder verification proves the advisory lock is taken before any lookup and that the eligible administrators are counted before and after the change. The last-admin refusal is covered from removePermissionFromRole, removeRoleFromUser and updatePermission, and only when the count really drops from at least one to zero. Duplicate-key and foreign-key races are shown to become business refusals.
The audit logger tests cover the actor from the provider (including none), the REJECTED_ prefix, the clock and the propagation of each method. The bootstrap tests show that it does nothing without configuration, creates the admin with the ADMIN role, skips an existing email, and never throws even when the DAO, the encoder or the transaction manager fails. The last point is the contract that matters: a misconfigured bootstrap must never prevent the application from starting.
JwtTestSupport can now sign a token that holds an arbitrary set of permissions, and RbacTestSupport creates users and roles, obtains JWTs through a real login and queries audit rows. Because all tests share one database, it also isolates the global last-admin state by disabling other ROLE:WRITE holders and restores everything it touched in close. The seeded ADMIN role is never modified by a test.
The evaluator is checked for an allowed and a denied permission, case insensitivity, anonymous and null authentications, unknown target types and the id-based overload, which always answers false. The actor provider returns the user id of an AuthenticatedUser and null for anything else. Both are the security decision points of the project, so they are tested in isolation from the web layer.
The real SecurityConfig and a real signed JWT are used with a mocked RbacService. Each success test uses a token holding only the required permission, and the tests cover status codes and JSON shape, 404, 422, per-field 400 messages, 405, 415 and 500. The user list is checked for page and size violations reported as "size: ..." and for a page beyond the last one that keeps content as an empty array.
A parameterized test walks every endpoint of the README table: no token gives 401, a token holding all other permissions gives 403 with the ApiResponse body, an empty token gives 403 and a blacklisted token gives 401, and the service is never reached in any of these cases. It proves that @PreAuthorize with the custom evaluator protects each route independently of the others.
Real services, a real database and real logins prove that role and permission CRUD and assignments work, that every mutation writes a matching audit row with the actor, and that a role change only reaches the JWT at the next login. A deleteRole on a referenced role returns 422 and its REJECTED_ audit row still exists after the rollback, which proves REQUIRES_NEW works through the proxy without any stereotype. 404 and 400 cases write no audit row.
The rule is checked in both directions (removing ROLE:WRITE from the only admin's role, and removing the role from the only admin user), for renaming the permission away, and for the cases where it must allow the change: a second enabled and unlocked holder, possibly through another role. A disabled or locked second admin does not count. A concurrency test releases two threads at once on two requests that would each remove one of the only two admins: in every round exactly one succeeds and the other gets 422, which is the advisory lock at work.
With the bootstrap properties set, the admin exists with the ADMIN role and a bcrypt hash, can log in over HTTP with all 14 permissions and leaves a single BOOTSTRAP_ADMIN audit row with no actor. A second context with an invalid password still starts and creates no account. That last test is the guarantee that a bad bootstrap setting can never make the application fail to start.
The parameter name comes from compiled parameter metadata, which is empty when a class is built without the -parameters compiler flag, giving a message such as "null: must be greater than or equal to 1". The handler now falls back to the word "parameter" instead. The build sets -parameters, so the normal output is unchanged.
…bootstrap All eight RBAC tasks are checked: the entities, dto classes, RowMappers and mappers, the DAOs, RbacService, AuditLogger, CustomPermissionEvaluator, the method security configuration, the V2 seed, the controllers and the tests at all layers, including the last-admin rule under concurrency. The development key README now explains the optional admin bootstrap variables, which are for local development only and never set for tests or production.
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.
Branch
feature/rbac
Task checklist
Role,Permission,AuditLogentities, dto classes, RowMappers, entity mappers and DAOsRbacService+RbacServiceImpl(CRUD, referenced-delete refusals, last-admin rule, assignment directions)AuditLogger/AuditLoggerImplcalled from every mutationCustomPermissionEvaluator(permissions read from the JWT authorities, no database call per request)SecurityConfig:@EnableMethodSecurityand the evaluator on theMethodSecurityExpressionHandlerRoleController,PermissionController,UserController, convertersCommit summary
pg_advisory_xact_lock@PreAuthorize, 401 vs 403 refinement, named parameter validation messagesLimits accepted and documented in the project notes: REQUIRES_NEW uses a second pooled connection, guard fires only when an admin exists, validation runs before the permission check, permissions live in the JWT until the next login.
Test plan
mvn clean verifyis green on the whole reactor