[NAE-2193] Assign all roles to admin users #350
Conversation
Replaced hardcoded admin user details with centralized constants from `UserConstants` for improved manageability. Added a new method to update admin roles in `UserService` and introduced a default count method in `UserRepository` to enhance functionality. These changes improve maintainability and flexibility of the admin user handling.
WalkthroughReplaced hard-coded super admin identifiers with UserConstants in production and tests, added UserService.updateAdminWithRoles implementation and interface method, introduced UserRepository.countAll default method, updated ADMIN_USER_EMAIL value, and adjusted one test to use findByEmail. No public signatures removed; primarily additive and constant-driven changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller as Caller
participant USvc as UserServiceImpl
participant URepo as UserRepository
participant Store as Persistence
Caller->>USvc: updateAdminWithRoles(roles)
USvc->>URepo: findByEmail(ADMIN_USER_EMAIL, realmId=null)
URepo-->>USvc: adminUser
USvc->>USvc: adminUser.setProcessRoles(new HashSet(roles))
USvc->>Store: saveUser(adminUser)
Store-->>USvc: savedUser
USvc-->>Caller: void
note right of USvc: Logs role count before/after save
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/plugin/domain/Plugin.java (1)
52-63: Defensive copy and null-safety for metadata in builder ctor.Avoid nulls and external mutations.
- public Plugin(String identifier, String name, String version, String description, String url, int restPort, int grpcPort, Map<String, EntryPoint> entryPoints, boolean active, Map<String, String> metadata) { + public Plugin(String identifier, String name, String version, String description, String url, int restPort, int grpcPort, Map<String, EntryPoint> entryPoints, boolean active, Map<String, String> metadata) { this.identifier = identifier; this.name = name; this.version = version; this.description = description; this.url = url; this.restPort = restPort; this.grpcPort = grpcPort; this.entryPoints = entryPoints; this.active = active; - this.metadata = metadata; + this.metadata = metadata != null ? new LinkedHashMap<>(metadata) : new LinkedHashMap<>(); }nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java (1)
296-309: Fix Optional type mismatch and guard assignment to admins only.
Optional<User>is being assigned toOptional<AbstractUser>via a no-opmap, which won’t compile. Also, the method assigns all roles to any user by username; enforce admin-only to match intent.Apply:
- public void addAllRolesToAdminByUsername(String username) { - String collectionName = collectionNameProvider.getAdminRealmCollection(); - Optional<AbstractUser> userOptional = userRepository.findByUsername(username, mongoTemplate, collectionName).map(user -> user); - if (userOptional.isEmpty()) { - throw new IllegalArgumentException("Admin user with username [%s] cannot be found.".formatted(username)); - } - AbstractUser user = userOptional.get(); - - Page<ProcessRole> processRoles = processRoleService.findAll(Pageable.unpaged()); - user.getProcessRoles().addAll(processRoles.getContent()); - - saveUser(user, user.getRealmId()); - } + public void addAllRolesToAdminByUsername(String username) { + String collectionName = collectionNameProvider.getAdminRealmCollection(); + Optional<User> userOptional = userRepository.findByUsername(username, mongoTemplate, collectionName); + if (userOptional.isEmpty()) { + throw new IllegalArgumentException("Admin user with username [%s] cannot be found.".formatted(username)); + } + User user = userOptional.get(); + // enforce admin-only + if (!user.getAuthoritySet().contains(authorityService.getOrCreate(Authority.admin))) { + log.warn("User [{}] is not an admin – skipping role assignment", username); + return; + } + Page<ProcessRole> processRoles = processRoleService.findAll(Pageable.unpaged()); + user.getProcessRoles().addAll(processRoles.getContent()); + saveUser(user, user.getRealmId()); + }application-engine/src/main/java/com/netgrif/application/engine/startup/runner/SuperCreatorRunner.java (1)
33-33: Remove unused constant.
SUPER_ADMIN_EMAILis no longer used; delete to avoid confusion.- public static final String SUPER_ADMIN_EMAIL = "super@netgrif.com";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (16)
application-engine/pom.xml(1 hunks)application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/configuration/groovy/GroovyShellConfiguration.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/startup/runner/SuperCreatorRunner.java(2 hunks)application-engine/src/main/resources/application.yaml(1 hunks)nae-object-library/pom.xml(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/constants/UserConstants.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/plugin/domain/Plugin.java(3 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/utils/Nullable.java(1 hunks)nae-spring-core-adapter/pom.xml(1 hunks)nae-user-ce/pom.xml(1 hunks)nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java(1 hunks)nae-user-common/pom.xml(1 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/repository/UserRepository.java(1 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/UserService.java(1 hunks)pom.xml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
application-engine/src/main/java/com/netgrif/application/engine/startup/runner/SuperCreatorRunner.java (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/constants/UserConstants.java (1)
UserConstants(3-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: Build
- GitHub Check: task-list-completed
🔇 Additional comments (13)
nae-object-library/pom.xml (1)
10-10: RC6.1 parent align looks good.No issues spotted with the version bump here.
application-engine/pom.xml (1)
9-9: Parent version updated to RC6.1 — OK.Assuming the root POM is also RC6.1 (it is in this PR), inheritance will resolve correctly.
nae-user-common/pom.xml (1)
9-9: Consistent parent version — OK.Module will inherit dependency management from the updated parent as expected.
pom.xml (1)
9-9: All parent POM versions aligned to 7.0.0-RC6.1 with no SNAPSHOTs remaining
No 7.0.0-SNAPSHOT found; all module parent versions are consistently set to 7.0.0-RC6.1.nae-user-ce/pom.xml (1)
9-9: Parent bump to RC6.1 — looks good.No additional changes required here.
nae-spring-core-adapter/pom.xml (1)
10-10: Version alignment confirmed. All modules use 7.0.0-RC6.1; no SNAPSHOT or mismatched RC versions found.application-engine/src/main/java/com/netgrif/application/engine/configuration/groovy/GroovyShellConfiguration.java (1)
23-23: Registering default star imports — LGTM.nae-object-library/src/main/java/com/netgrif/application/engine/objects/plugin/domain/Plugin.java (1)
44-45: Equality/HashCode semantics changed due to new field.@DaTa now includes metadata in equals/hashCode. Confirm this won’t break caches/collections or API compatibility.
application-engine/src/main/resources/application.yaml (1)
162-162: Add Nullable to actions.imports — LGTM.Import enables Nullable in scripts; no concerns.
nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java (1)
596-596: Order-preserving dedup — verify paging semantics.Using
LinkedHashSetpreserves order (good), but dedup can shrink page content whiletotalElementsstays unchanged. Confirm consumers tolerate this.nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/constants/UserConstants.java (1)
8-8: Admin email change — LGTM; check rollout.Switching to
super@netgrif.comlooks fine. Ensure docs and any ops automation expecting the old value are updated.application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy (2)
93-93: Nullable helper for scripts — LGTM.Import and
nullable(T)wrapper are consistent with the new utility.Also applies to: 911-913
2771-2776: Exception messages — LGTM.Clear, actionable errors for missing/wrong field types.
.../main/java/com/netgrif/application/engine/configuration/groovy/GroovyShellConfiguration.java
Show resolved
Hide resolved
.../main/java/com/netgrif/application/engine/configuration/groovy/GroovyShellConfiguration.java
Show resolved
Hide resolved
...n-engine/src/main/java/com/netgrif/application/engine/startup/runner/SuperCreatorRunner.java
Show resolved
Hide resolved
nae-object-library/src/main/java/com/netgrif/application/engine/objects/utils/Nullable.java
Show resolved
Hide resolved
nae-object-library/src/main/java/com/netgrif/application/engine/objects/utils/Nullable.java
Show resolved
Hide resolved
nae-object-library/src/main/java/com/netgrif/application/engine/objects/utils/Nullable.java
Show resolved
Hide resolved
...user-common/src/main/java/com/netgrif/application/engine/auth/repository/UserRepository.java
Show resolved
Hide resolved
nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/UserService.java
Outdated
Show resolved
Hide resolved
nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/UserService.java
Show resolved
Hide resolved
nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/UserService.java
Outdated
Show resolved
Hide resolved
|
There are test errors |
Replaced hardcoded admin username and email with `UserConstants` across test files. This improves maintainability and reduces duplication by centralizing these constants.
There was a problem hiding this comment.
Actionable comments posted: 11
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
application-engine/src/test/groovy/com/netgrif/application/engine/action/RemoveActionTest.groovy(2 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/export/service/ExportServiceTest.groovy(6 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/FileFieldTest.groovy(5 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/FileListFieldTest.groovy(4 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/workflow/CaseSearchTest.groovy(2 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/workflow/UserRefsTest.groovy(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
🔇 Additional comments (5)
application-engine/src/test/groovy/com/netgrif/application/engine/action/RemoveActionTest.groovy (2)
6-6: LGTM: centralized admin identity importUsing UserConstants removes hard-coded credentials.
120-120: Authentication token misses authorities — may 403 on admin-only endpointBuild an authenticated token with admin authorities instead of a bare username.
Apply:
- auth = new UsernamePasswordAuthenticationToken(UserConstants.ADMIN_USER_USERNAME, "password") + auth = new UsernamePasswordAuthenticationToken( + superCreator.getLoggedSuper(), + "password", + superCreator.getLoggedSuper().getAuthorities() + )application-engine/src/test/groovy/com/netgrif/application/engine/workflow/CaseSearchTest.groovy (1)
6-6: LGTM: replaced hard-coded admin with UserConstants and basic authGood move to centralize credentials; basic auth now uses ADMIN_USER_USERNAME.
Also applies to: 170-170
application-engine/src/test/groovy/com/netgrif/application/engine/export/service/ExportServiceTest.groovy (2)
7-7: Good: replace magic literals withUserConstants.Prevents drift when admin username/email change in one place.
97-97: Ignore suggestion to use adminActor()
NoadminActor()helper exists in the codebase; either keep usinguserService.findByEmail(...)or add a common helper first.Likely an incorrect or invalid review comment.
...ngine/src/test/groovy/com/netgrif/application/engine/export/service/ExportServiceTest.groovy
Show resolved
Hide resolved
...ngine/src/test/groovy/com/netgrif/application/engine/export/service/ExportServiceTest.groovy
Show resolved
Hide resolved
...ngine/src/test/groovy/com/netgrif/application/engine/export/service/ExportServiceTest.groovy
Show resolved
Hide resolved
...ngine/src/test/groovy/com/netgrif/application/engine/export/service/ExportServiceTest.groovy
Show resolved
Hide resolved
...ngine/src/test/groovy/com/netgrif/application/engine/export/service/ExportServiceTest.groovy
Show resolved
Hide resolved
.../src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/FileFieldTest.groovy
Show resolved
Hide resolved
.../src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/FileFieldTest.groovy
Show resolved
Hide resolved
.../test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/FileListFieldTest.groovy
Show resolved
Hide resolved
.../test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/FileListFieldTest.groovy
Show resolved
Hide resolved
application-engine/src/test/groovy/com/netgrif/application/engine/workflow/UserRefsTest.groovy
Show resolved
Hide resolved
Updated the admin user's email to use the constant UserConstants.ADMIN_USER_EMAIL instead of a hardcoded value. This ensures consistency and avoids potential issues with hardcoded strings in the test setup.
Implemented the `updateAdminWithRoles` method in `UserServiceImpl` to allow updating the admin user's process roles dynamically. Updated the `UserService` interface to enforce implementation of this method.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/UserService.java (3)
404-409: Fix Javadoc grammar and align param type; document union behaviorCurrent Javadoc has grammar issues and says “role identifiers” while the type is
Collection<ProcessRole>. Also clarify that roles are added (union), not replaced, to avoid ambiguity with the implementation.Apply:
- /** - * Updates the admin user or users with the specified roles. - * This method assigns the provided roles to all the admin user of system, updating their current set of roles. - * - * @param roles a list of role identifiers to assign to the admin user - */ + /** + * Assigns the provided process roles to all admin users in the system. + * Behavior: roles are added (union); existing roles are retained; duplicates are ignored. + * + * @param roles collection of process roles to assign to admin users; must not be null + */
404-409: Clarify API scope and naming (all admins vs one; realm awareness)Name suggests singular “Admin” while text implies “all admin users”; method is not realm-aware unlike adjacent APIs. Either (a) rename to
assignRolesToAllAdminsand document cross-realm behavior, or (b) add a realm-scoped overload and keep this as convenience.Would you like an overload proposal that mirrors other realm-aware methods?
404-409: Prefer identifiers over domain objects in service boundaryAccepting
Collection<ProcessRole>couples the interface to Petri-net domain objects. ConsiderCollection<ProcessResourceId>orCollection<String>and resolve roles in the implementation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java(2 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/UserService.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/constants/UserConstants.java (1)
UserConstants(3-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
🔇 Additional comments (2)
nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java (2)
603-603: Preserving iteration order is good hereSwitch to
LinkedHashSetdeduplicates while keeping page order before mapping to List. Looks good.
567-573: Verify and add usage/tests for updateAdminWithRoles
- No external callers of
updateAdminWithRolesfound beyond its interface and own impl; update any consumers to use this new API.- Add unit/integration tests covering repeated invocations (idempotency), union semantics, and multiple-admin scenarios.
nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java
Show resolved
Hide resolved
- Added info and debug logs to updateAdminWithRoles for better traceability.
Description
Implements NAE-2193
Dependencies
No new dependencies were introduced.
Third party dependencies
No new dependencies were introduced.
Blocking Pull requests
There are no dependencies on other PR
How Has Been This Tested?
This was tested manually and with unit tests.
Test Configuration
Checklist:
Summary by CodeRabbit