Open
Conversation
dosubot
bot
added
the
size:XL
|
CLA Assistant Lite bot: I have read the CLA Document and I hereby sign the CLA 1 out of 2 committers have signed the CLA. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds OpenAPI user-management (DTO + UserController create/get/search), introduces CREATE_USER permission/role plumbing across validators, services, controllers, UI/i18n, role initialization, and tests for controller and consumer role flows. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant API as UserController
participant Checker as AuthUserPasswordChecker
participant Perm as PermissionValidator
participant UserSvc as UserService
participant RoleMgr as SystemRoleManagerService
Client->>API: POST /openapi/v1/users (OpenUserDTO)
API->>API: validate username, password, email
API->>Checker: checkPasswordStrength(password)
Checker-->>API: strengthResult
alt weak password
API-->>Client: 400 BadRequest
else
API->>Perm: hasCreateUserPermission()
Perm-->>API: allowed?
alt not allowed
API-->>Client: 403 / BadRequest
else
API->>UserSvc: createUser(UserPO)
UserSvc-->>API: UserInfo / error
API-->>Client: 201 Created (Location + UserInfo) or 400 on duplicate
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Author
|
I have read the CLA Document and I hereby sign the CLA |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/UserControllerTest.java (2)
26-26: UnusedGsonimport and field.
Gsonis imported (Line 26) and instantiated (Line 68) but never used in any test method.Remove unused code
-import com.google.gson.Gson;- private final Gson gson = new Gson();Also applies to: 68-68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/UserControllerTest.java` at line 26, Remove the unused Gson import and the unused gson field from UserControllerTest: delete the line importing com.google.gson.Gson and remove the Gson instance declared in the UserControllerTest class (the unused gson field), or alternatively use that gson instance in a test if intended; ensure no references remain to Gson in UserControllerTest after the change.
57-72: Test mocksSpringSecurityUserServicedirectly, mirroring the tight coupling in the controller.The test uses
SpringSecurityUserServiceas the mock type (Line 61) because the controller casts to it. If the controller is refactored to use theUserServiceinterface properly (as suggested in the controller review), this mock should also be updated accordingly. Additionally, there's no test for theUnsupportedOperationExceptionpath when the service is not aSpringSecurityUserService.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/UserControllerTest.java` around lines 57 - 72, The test currently mocks the concrete SpringSecurityUserService (mock field SpringSecurityUserService userService) which mirrors the controller's tight coupling; change the mock to the UserService interface type (mock UserService) and construct UserController with that mock to match the refactored controller using UserService; additionally add a new unit test that sets up a UserService mock instance that is NOT a SpringSecurityUserService (or configure the controller to receive a different implementation) and assert that calling the controller method that expects SpringSecurityUserService throws UnsupportedOperationException to cover that error path (reference UserController, SpringSecurityUserService, UserService, and UnsupportedOperationException).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/UserController.java`:
- Around line 122-128: The getUserByUserId method currently throws
BadRequestException when userService.findByUserId(userId) returns null, which
yields HTTP 400; change this to return a 404 by either throwing a
NotFoundException or returning
ResponseEntity.status(HttpStatus.NOT_FOUND).build(); specifically update
getUserByUserId in UserController to replace the BadRequestException throw with
a NotFoundException("User not found: " + userId) or
ResponseEntity.notFound().build(), keeping the rest of the method (UserInfo
userInfo = userService.findByUserId(userId) and the successful
ResponseEntity.ok(userInfo)) unchanged.
- Around line 141-157: The controller validates offset/limit but
SpringSecurityUserService.searchUsers(...) ignores them and returns all matches;
update searchUsers(String keyword, int offset, int limit, boolean
includeInactiveUsers) to apply pagination: prefer creating a Pageable
(PageRequest.of(offset/limit semantics: page = offset/limit, size = limit) or
convert offset to page/time-based query) and call a paged repository method
(e.g., userRepository.findUsers(keyword, includeInactiveUsers, pageable)) so
only the requested slice is returned; if switching the repo to Pageable is not
feasible immediately, implement slicing inside
SpringSecurityUserService.searchUsers by computing fromIndex = Math.min(offset,
list.size()) and toIndex = Math.min(offset + limit, list.size()) and return
list.subList(fromIndex, toIndex) to honor controller parameters.
- Around line 89-107: The controller currently checks and casts userService to
SpringSecurityUserService to call create(), tightly coupling UserController to
an implementation; instead add a create(UserPO) method to the UserService SPI
(e.g., default void create(UserPO user) { throw new
UnsupportedOperationException(...) }) so UserController can call
userService.create(userPO) without instanceof/cast, keep the DTO->PO mapping
(openUserDTO -> UserPO, DEFAULT_USER_ENABLED) and remove the
SpringSecurityUserService reference; also replace throwing raw
UnsupportedOperationException with an exception mapped to a proper HTTP status
(e.g., throw a BadRequestException or a custom exception that maps to 501 Not
Implemented) so callers get appropriate HTTP responses.
---
Nitpick comments:
In
`@apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/UserControllerTest.java`:
- Line 26: Remove the unused Gson import and the unused gson field from
UserControllerTest: delete the line importing com.google.gson.Gson and remove
the Gson instance declared in the UserControllerTest class (the unused gson
field), or alternatively use that gson instance in a test if intended; ensure no
references remain to Gson in UserControllerTest after the change.
- Around line 57-72: The test currently mocks the concrete
SpringSecurityUserService (mock field SpringSecurityUserService userService)
which mirrors the controller's tight coupling; change the mock to the
UserService interface type (mock UserService) and construct UserController with
that mock to match the refactored controller using UserService; additionally add
a new unit test that sets up a UserService mock instance that is NOT a
SpringSecurityUserService (or configure the controller to receive a different
implementation) and assert that calling the controller method that expects
SpringSecurityUserService throws UnsupportedOperationException to cover that
error path (reference UserController, SpringSecurityUserService, UserService,
and UnsupportedOperationException).
Comment on lines
+89
to
+107
| // Check if UserService supports user creation | ||
| if (!(userService instanceof SpringSecurityUserService)) { | ||
| throw new UnsupportedOperationException( | ||
| "Create user operation is not supported with current user service implementation"); | ||
| } | ||
|
|
||
| // Convert DTO to PO and set defaults | ||
| UserPO userPO = new UserPO(); | ||
| userPO.setUsername(openUserDTO.getUsername()); | ||
| userPO.setPassword(openUserDTO.getPassword()); | ||
| userPO.setEmail(openUserDTO.getEmail()); | ||
| userPO.setUserDisplayName( | ||
| openUserDTO.getUserDisplayName() != null ? openUserDTO.getUserDisplayName() | ||
| : openUserDTO.getUsername()); | ||
| userPO.setEnabled( | ||
| openUserDTO.getEnabled() != null ? openUserDTO.getEnabled() : DEFAULT_USER_ENABLED); | ||
|
|
||
| // Create user | ||
| ((SpringSecurityUserService) userService).create(userPO); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and examine UserService interface definition
echo "=== Searching for UserService interface ==="
rg -n 'interface UserService' --type=java -A 20
echo -e "\n=== Classes implementing UserService ==="
rg -n 'implements UserService' --type=java
echo -e "\n=== Find UserService file location ==="
fd -n 'UserService.java' --type fRepository: apolloconfig/apollo
Length of output: 1843
🏁 Script executed:
#!/bin/bash
# Get full UserService interface definition
echo "=== Full UserService interface ==="
cat -n apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/UserService.java
echo -e "\n=== SpringSecurityUserService create method ==="
rg -n 'create\(' apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/springsecurity/SpringSecurityUserService.java -A 5
echo -e "\n=== Check for exception handlers in UserController ==="
rg -n '@ExceptionHandler|@ControllerAdvice' apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/UserController.java
echo -e "\n=== Check UserController class definition for exception handling ==="
rg -n 'class UserController' apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/UserController.java -A 5Repository: apolloconfig/apollo
Length of output: 2035
🏁 Script executed:
#!/bin/bash
# Check for global exception handling
echo "=== Search for exception handlers (RestControllerAdvice/ControllerAdvice) ==="
rg -n '@RestControllerAdvice|@ControllerAdvice' apollo-portal/src/main/java --type=java
echo -e "\n=== Check how UnsupportedOperationException is handled ==="
rg -n 'UnsupportedOperationException' apollo-portal/src/main/java --type=java
echo -e "\n=== Check searchUsers method in SpringSecurityUserService ==="
rg -n 'searchUsers' apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/springsecurity/SpringSecurityUserService.java -A 15
echo -e "\n=== Check UserController searchUsers method (around lines 141-157) ==="
sed -n '135,160p' apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/UserController.java | cat -nRepository: apolloconfig/apollo
Length of output: 4152
Tight coupling to SpringSecurityUserService via instanceof check and cast.
The controller depends on the UserService interface but then checks for and casts to the concrete SpringSecurityUserService to call create(). This defeats the purpose of the abstraction. If create is needed in the OpenAPI, it should be declared on the UserService SPI interface (or a dedicated sub-interface), so other implementations (e.g., LDAP-backed) can also support user creation.
Additionally, UnsupportedOperationException (Line 91) is not mapped by Spring's exception handling and will result in an HTTP 500 response. Consider throwing a BadRequestException or a custom exception that maps to a more appropriate status code (e.g., 501 Not Implemented).
Proposed approach
Add a create(UserPO user) method to the UserService interface (with a default throwing UnsupportedOperationException), then remove the instanceof check:
- // Check if UserService supports user creation
- if (!(userService instanceof SpringSecurityUserService)) {
- throw new UnsupportedOperationException(
- "Create user operation is not supported with current user service implementation");
- }
-
// Convert DTO to PO and set defaults
...
// Create user
- ((SpringSecurityUserService) userService).create(userPO);
+ userService.create(userPO);In UserService interface:
default void create(UserPO user) {
throw new UnsupportedOperationException(
"Create user operation is not supported with current user service implementation");
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/UserController.java`
around lines 89 - 107, The controller currently checks and casts userService to
SpringSecurityUserService to call create(), tightly coupling UserController to
an implementation; instead add a create(UserPO) method to the UserService SPI
(e.g., default void create(UserPO user) { throw new
UnsupportedOperationException(...) }) so UserController can call
userService.create(userPO) without instanceof/cast, keep the DTO->PO mapping
(openUserDTO -> UserPO, DEFAULT_USER_ENABLED) and remove the
SpringSecurityUserService reference; also replace throwing raw
UnsupportedOperationException with an exception mapped to a proper HTTP status
(e.g., throw a BadRequestException or a custom exception that maps to 501 Not
Implemented) so callers get appropriate HTTP responses.
Comment on lines
+122
to
+128
| public ResponseEntity<UserInfo> getUserByUserId(@PathVariable String userId) { | ||
| UserInfo userInfo = userService.findByUserId(userId); | ||
| if (userInfo == null) { | ||
| throw new BadRequestException("User not found: " + userId); | ||
| } | ||
| return ResponseEntity.ok(userInfo); | ||
| } |
Contributor
There was a problem hiding this comment.
User not found should return HTTP 404, not 400.
BadRequestException maps to HTTP 400, but "user not found" is semantically a 404 Not Found. Using 400 here is misleading to API consumers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/UserController.java`
around lines 122 - 128, The getUserByUserId method currently throws
BadRequestException when userService.findByUserId(userId) returns null, which
yields HTTP 400; change this to return a 404 by either throwing a
NotFoundException or returning
ResponseEntity.status(HttpStatus.NOT_FOUND).build(); specifically update
getUserByUserId in UserController to replace the BadRequestException throw with
a NotFoundException("User not found: " + userId) or
ResponseEntity.notFound().build(), keeping the rest of the method (UserInfo
userInfo = userService.findByUserId(userId) and the successful
ResponseEntity.ok(userInfo)) unchanged.
Comment on lines
+141
to
+157
| public ResponseEntity<List<UserInfo>> searchUsers( | ||
| @RequestParam(value = "keyword", required = false, defaultValue = "") String keyword, | ||
| @RequestParam(value = "includeInactiveUsers", | ||
| defaultValue = "false") boolean includeInactiveUsers, | ||
| @RequestParam(value = "offset", defaultValue = "0") int offset, | ||
| @RequestParam(value = "limit", defaultValue = "10") int limit) { | ||
|
|
||
| if (limit <= 0 || limit > 100) { | ||
| throw new BadRequestException("Limit must be between 1 and 100"); | ||
| } | ||
|
|
||
| if (offset < 0) { | ||
| throw new BadRequestException("Offset must be non-negative"); | ||
| } | ||
|
|
||
| List<UserInfo> users = userService.searchUsers(keyword, offset, limit, includeInactiveUsers); | ||
| return ResponseEntity.ok(users); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find SpringSecurityUserService class
find . -name "*SpringSecurityUserService*" -type fRepository: apolloconfig/apollo
Length of output: 176
🏁 Script executed:
#!/bin/bash
# Read the SpringSecurityUserService file
cat -n ./apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/springsecurity/SpringSecurityUserService.javaRepository: apolloconfig/apollo
Length of output: 6717
Pagination parameters offset and limit are accepted but never applied.
The searchUsers() method in SpringSecurityUserService accepts offset and limit parameters, but ignores them entirely. The method retrieves all matching users via findUsers() and returns the complete list without any slicing or pagination logic. This means the controller's validation of these parameters is meaningless — callers requesting paginated results will receive the entire dataset instead of the requested subset.
Implement actual pagination in SpringSecurityUserService.searchUsers() using Spring Data's Pageable, or apply subList slicing in the controller as an interim solution.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/UserController.java`
around lines 141 - 157, The controller validates offset/limit but
SpringSecurityUserService.searchUsers(...) ignores them and returns all matches;
update searchUsers(String keyword, int offset, int limit, boolean
includeInactiveUsers) to apply pagination: prefer creating a Pageable
(PageRequest.of(offset/limit semantics: page = offset/limit, size = limit) or
convert offset to page/time-based query) and call a paged repository method
(e.g., userRepository.findUsers(keyword, includeInactiveUsers, pageable)) so
only the requested slice is returned; if switching the repo to Pageable is not
feasible immediately, implement slicing inside
SpringSecurityUserService.searchUsers by computing fromIndex = Math.min(offset,
list.size()) and toIndex = Math.min(offset + limit, list.size()) and return
list.subList(fromIndex, toIndex) to honor controller parameters.
nobodyiam
requested changes
Feb 17, 2026
Member
nobodyiam
left a comment
nobodyiam
left a comment
There was a problem hiding this comment.
感谢贡献！方向是对的：先把 OpenAPI 创建用户能力落地是有价值的。
我这边建议本 PR 先聚焦“创建用户 API”，并将“创建后通知工作群”拆到后续 PR；这个拆分可以接受。
不过当前还有一个阻塞问题需要先修复：
- 权限模型与 OpenAPI consumer 不兼容（阻塞）
- 现在接口用的是 @unifiedPermissionValidator.isSuperAdmin()，但 OpenAPI consumer 的 isSuperAdmin() 固定为 false。
- 结果是：用 OpenAPI token 基本无法调用该接口（403），与“通过 OpenAPI 创建用户”的目标冲突。
- 建议改成 consumer 可授予的系统级权限模型（参考 CreateApplication）：
- 增加例如 CreateUser 权限；
- 接口改为 hasCreateUserPermission()；
- 在创建 consumer/token 时可配置是否授予该权限（类似 allowCreateApplication）。
- 关于“通知工作群”请在本 PR 明确说明范围（可后置）
- 如果本 PR 不实现通知，请在 PR 描述中明确“本次仅实现创建用户 API，通知能力后续 PR 跟进”；
- 建议不要写 Fixes #5441（避免自动关闭），改成 Part of #5441，并附上后续通知任务的 issue/计划链接。
结论：
请先修复权限模型（阻塞项）并更新 PR 描述范围说明；完成后我再继续看下一轮。
dosubot
bot
added
size:XXL
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/RoleInitializationService.java (1)
1-1:⚠️ Potential issue | 🔴 CriticalCI pipeline failure: Spotless formatting violation must be fixed before merge.
The GitHub Actions
code style checkreports formatting violations acrossapollo-portal. Runmvn spotless:applyand push the result. This blocks CI on all Java files in this PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/RoleInitializationService.java` at line 1, CI is failing due to Spotless formatting violations across apollo-portal; run the Spotless formatter and commit the changes: execute mvn spotless:apply in the apollo-portal module, review and stage the modified Java files (including RoleInitializationService and any other touched classes), then re-run mvn -DskipTests verify or push the formatted changes so the GitHub Actions "code style check" passes.apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/component/AbstractPermissionValidatorTest.java (1)
1-17:⚠️ Potential issue | 🟠 MajorFix Spotless formatting violations before merge.
Run
mvn spotless:apply.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/component/AbstractPermissionValidatorTest.java` around lines 1 - 17, The file AbstractPermissionValidatorTest.java has Spotless formatting violations; run the formatter (mvn spotless:apply) and commit the resulting changes so the file conforms to project style (fix license/header spacing, import ordering, indentation, trailing spaces, and any line-wrapping issues). After running mvn spotless:apply, verify the class AbstractPermissionValidatorTest compiles cleanly and include the reformatted file in the PR.apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/PermissionController.java (1)
1-17:⚠️ Potential issue | 🟠 MajorFix Spotless formatting violations before merge.
Run
mvn spotless:apply.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/PermissionController.java` around lines 1 - 17, The build is failing due to Spotless formatting violations in PermissionController.java (package declaration and file header); fix by running the formatter (mvn spotless:apply), review and stage the updated formatting changes for the PermissionController class, then re-run mvn test/build to verify the Spotless checks pass before pushing the commit.apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/SystemRoleManagerService.java (1)
1-17:⚠️ Potential issue | 🟠 MajorFix Spotless formatting violations before merge.
Run
mvn spotless:apply.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/SystemRoleManagerService.java` around lines 1 - 17, The file SystemRoleManagerService has Spotless formatting violations; run mvn spotless:apply to auto-fix formatting, review the resulting changes (license header spacing, package declaration, imports and method/annotation formatting within SystemRoleManagerService), stage and commit the modified files, and push the updated branch so the PR passes Spotless checks.apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/service/ConsumerServiceTest.java (1)
1-17:⚠️ Potential issue | 🟠 MajorFix Spotless formatting violations before merge.
The GitHub Actions pipeline reports Spotless formatting violations in
apollo-portal. Runmvn spotless:applyto auto-fix all format issues in this module.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/service/ConsumerServiceTest.java` around lines 1 - 17, Spotless formatting violations were detected in the apollo-portal module (e.g., file ConsumerServiceTest.java); run the formatter to fix them by executing mvn spotless:apply in the apollo-portal module, then review modified files (including ConsumerServiceTest.java) and commit the formatting changes before pushing the PR; re-run CI to ensure the Spotless check passes.
🧹 Nitpick comments (8)
apollo-portal/src/main/resources/static/open/manage.html (1)
95-97: Optional: pass rateLimit through translate only when it is the fallback string.When
consumer.rateLimit > 0, the numeric value is the ternary result, which is then piped throughtranslate. AngularJS returns unknown keys as-is, so the output is correct, but the translate filter is applied unnecessarily to a number. Consider the structure already used for the boolean columns to avoid this:♻️ Cleaner rate-limit display
-<td style="width: 9%"> - {{ consumer.rateLimit && consumer.rateLimit > 0 ? consumer.rateLimit : - 'Open.Manage.Consumer.RateLimitValue.Display' | translate }} -</td> +<td style="width: 9%"> + <span ng-if="consumer.rateLimit && consumer.rateLimit > 0">{{ consumer.rateLimit }}</span> + <span ng-if="!consumer.rateLimit || consumer.rateLimit <= 0"> + {{'Open.Manage.Consumer.RateLimitValue.Display' | translate}} + </span> +</td>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/main/resources/static/open/manage.html` around lines 95 - 97, The template currently pipes the entire ternary through translate so numeric consumer.rateLimit is unnecessarily translated; change the interpolation to apply translate only to the fallback string by grouping the ternary and piping the fallback, e.g. use consumer.rateLimit && consumer.rateLimit > 0 ? consumer.rateLimit : ('Open.Manage.Consumer.RateLimitValue.Display' | translate) so that consumer.rateLimit is shown raw and the translate filter only runs for the fallback key.apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/service/ConsumerService.java (2)
279-298: Significant duplication withisAllowCreateApplicationandassignCreateApplicationRoleToConsumer.Both
isAllowCreateUser(List<Long>)andassignCreateUserRoleToConsumer(String)are structural clones of theirCreateApplicationcounterparts. Consider extracting shared helpers parameterized by a role-supplier/name, e.g.:
isAllowRole(List<Long> consumerIdList, Role role)for the boolean-list checkassignSystemRoleToConsumer(String token, Supplier<Role> roleLookup, String roleName)for role assignmentThis can be deferred as it's consistent with the pre-existing pattern.
Also applies to: 340-360
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/service/ConsumerService.java` around lines 279 - 298, The two methods isAllowCreateUser and assignCreateUserRoleToConsumer duplicate logic from isAllowCreateApplication and assignCreateApplicationRoleToConsumer; refactor by extracting shared helpers: create a boolean helper isAllowRole(List<Long> consumerIdList, Role role) used by both isAllowCreateUser and isAllowCreateApplication, and create a generic assignSystemRoleToConsumer(String token, Supplier<Role> roleLookup, String roleName) to encapsulate the role assignment flow used by assignCreateUserRoleToConsumer and assignCreateApplicationRoleToConsumer; update the four methods to call these helpers and remove duplicated repository/role-check logic while preserving current behavior and error handling.
340-360: Add@TransactionaltoassignCreateUserRoleToConsumerandassignCreateApplicationRoleToConsumer.Both methods perform reads followed by writes to the database but lack
@Transactionalannotations. While the current caller (ConsumerController.create) is transactional, these methods could be invoked from non-transactional contexts, leaving read-write operations without a transaction boundary. Consistency requires both methods to be annotated with@Transactional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/service/ConsumerService.java` around lines 340 - 360, The methods assignCreateUserRoleToConsumer and assignCreateApplicationRoleToConsumer perform reads followed by writes but lack transaction boundaries; annotate both methods with `@Transactional` to ensure the read-then-write sequence is executed in a single transaction when called from non-transactional contexts, and add the necessary import (org.springframework.transaction.annotation.Transactional) if missing; keep the default propagation and rollback behavior unless a different semantics is required.apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ConsumerControllerTest.java (1)
53-70: Pre-existing testcreateWithCompatibilitydoesn't verifyassignCreateUserRoleToConsumeris not called.With the new code path added, this test should also assert that
assignCreateUserRoleToConsumeris never invoked, for completeness. The newcreateWithoutAnySystemRoletest covers a similar scenario, butcreateWithCompatibilitypredates it and would benefit from the additional assertion.💡 Proposed addition
Mockito.verify(consumerService, Mockito.times(0)) .assignCreateApplicationRoleToConsumer(Mockito.any()); + Mockito.verify(consumerService, Mockito.times(0)) + .assignCreateUserRoleToConsumer(Mockito.any()); Mockito.verify(consumerService, Mockito.times(1)).getConsumerInfoByAppId(Mockito.any());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ConsumerControllerTest.java` around lines 53 - 70, The test createWithCompatibility should assert that ConsumerService.assignCreateUserRoleToConsumer is not invoked; update the test (ConsumerControllerTest#createWithCompatibility) to add a Mockito.verify(consumerService, Mockito.times(0)).assignCreateUserRoleToConsumer(Mockito.any()) (or Mockito.never()) after the existing verifications so the mock is checked for no calls to assignCreateUserRoleToConsumer.apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/defaultimpl/DefaultRoleInitializationService.java (1)
160-180: Consider extracting a shared helper forinitCreateAppRoleandinitCreateUserRole.The two methods are structurally identical, differing only in the role name and permission type constants. A shared
initSystemRole(String roleName, String permissionType)would eliminate the duplication.This is a minor DRY concern and can be deferred, especially since the existing
initCreateAppRolepredates this PR.♻️ Proposed refactor
- `@Transactional` - `@Override` - public void initCreateAppRole() { - if (rolePermissionService - .findRoleByRoleName(SystemRoleManagerService.CREATE_APPLICATION_ROLE_NAME) != null) { - return; - } - Permission createAppPermission = permissionRepository.findTopByPermissionTypeAndTargetId( - PermissionType.CREATE_APPLICATION, SystemRoleManagerService.SYSTEM_PERMISSION_TARGET_ID); - if (createAppPermission == null) { - createAppPermission = createPermission(SystemRoleManagerService.SYSTEM_PERMISSION_TARGET_ID, - PermissionType.CREATE_APPLICATION, "apollo"); - rolePermissionService.createPermission(createAppPermission); - } - Role createAppRole = - createRole(SystemRoleManagerService.CREATE_APPLICATION_ROLE_NAME, "apollo"); - rolePermissionService.createRoleWithPermissions(createAppRole, - Sets.newHashSet(createAppPermission.getId())); - } - - `@Transactional` - `@Override` - public void initCreateUserRole() { - if (rolePermissionService - .findRoleByRoleName(SystemRoleManagerService.CREATE_USER_ROLE_NAME) != null) { - return; - } - Permission createUserPermission = permissionRepository.findTopByPermissionTypeAndTargetId( - PermissionType.CREATE_USER, SystemRoleManagerService.SYSTEM_PERMISSION_TARGET_ID); - if (createUserPermission == null) { - createUserPermission = createPermission(SystemRoleManagerService.SYSTEM_PERMISSION_TARGET_ID, - PermissionType.CREATE_USER, "apollo"); - rolePermissionService.createPermission(createUserPermission); - } - Role createUserRole = - createRole(SystemRoleManagerService.CREATE_USER_ROLE_NAME, "apollo"); - rolePermissionService.createRoleWithPermissions(createUserRole, - Sets.newHashSet(createUserPermission.getId())); - } + `@Transactional` + `@Override` + public void initCreateAppRole() { + initSystemRole(SystemRoleManagerService.CREATE_APPLICATION_ROLE_NAME, + PermissionType.CREATE_APPLICATION); + } + + `@Transactional` + `@Override` + public void initCreateUserRole() { + initSystemRole(SystemRoleManagerService.CREATE_USER_ROLE_NAME, + PermissionType.CREATE_USER); + } + + private void initSystemRole(String roleName, String permissionType) { + if (rolePermissionService.findRoleByRoleName(roleName) != null) { + return; + } + Permission permission = permissionRepository.findTopByPermissionTypeAndTargetId( + permissionType, SystemRoleManagerService.SYSTEM_PERMISSION_TARGET_ID); + if (permission == null) { + permission = createPermission(SystemRoleManagerService.SYSTEM_PERMISSION_TARGET_ID, + permissionType, "apollo"); + rolePermissionService.createPermission(permission); + } + Role role = createRole(roleName, "apollo"); + rolePermissionService.createRoleWithPermissions(role, + Sets.newHashSet(permission.getId())); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/defaultimpl/DefaultRoleInitializationService.java` around lines 160 - 180, Both initCreateUserRole and initCreateAppRole share identical logic; extract that logic into a private helper initSystemRole(String roleName, PermissionType permissionType) and have both initCreateUserRole and initCreateAppRole call it. The helper should perform the same steps now in initCreateUserRole: check rolePermissionService.findRoleByRoleName(roleName), lookup permission via permissionRepository.findTopByPermissionTypeAndTargetId(permissionType, SystemRoleManagerService.SYSTEM_PERMISSION_TARGET_ID), create the permission with createPermission(...) and rolePermissionService.createPermission(...) if missing, then create the role with createRole(roleName, "apollo") and call rolePermissionService.createRoleWithPermissions(createdRole, Sets.newHashSet(createdPermission.getId())). Preserve the `@Transactional` behavior (keep it on the public init methods or move to the helper if desired) and reference SystemRoleManagerService.CREATE_USER_ROLE_NAME and PermissionType.CREATE_USER when calling the new helper.apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/service/ConsumerServiceTest.java (1)
408-432:assignCreateUserRoleToConsumer_alreadyAssigned— add assertion forroleId.The test verifies
result.getConsumerId()but does not assertresult.getRoleId(), so a role-ID mismatch would go undetected.✅ Suggested assertion
assertNotNull(result); assertEquals(consumerId, result.getConsumerId()); + assertEquals(createUserRoleId, result.getRoleId()); verify(consumerRoleRepository, never()).save(any());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/service/ConsumerServiceTest.java` around lines 408 - 432, The test assignCreateUserRoleToConsumer_alreadyAssigned should assert the returned ConsumerRole contains the expected role id; after calling consumerService.assignCreateUserRoleToConsumer(token) and asserting result is not null and consumerId matches, also assert result.getRoleId() equals createUserRoleId (the role id used to build createUserRole and existingConsumerRole) so a mismatched roleId would fail the test.apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/component/AbstractPermissionValidatorTest.java (1)
207-298: Missing@Testmethods for the newhasCreateUserPermissionoverrides.Every other permission method in this file has a corresponding
testHasX_WhenNoPermission/testHasX_WhenWithPermissionpair (e.g., lines 99–107 forhasCreateApplicationPermission, lines 197–205 forhasManageAppMasterPermission). The newhasCreateUserPermissionoverrides break that pattern — no tests exercise them.✅ Suggested additions
`@Test` public void testHasCreateUserPermission_WhenNoPermission() { assertFalse(permissionValidator.hasCreateUserPermission()); } `@Test` public void testHasCreateUserPermission_WhenWithPermission() { AbstractPermissionValidator validator = new AbstractPermissionValidatorWithPermissionsImpl(Collections.emptyList()); assertTrue(validator.hasCreateUserPermission()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/component/AbstractPermissionValidatorTest.java` around lines 207 - 298, Tests for the newly overridden hasCreateUserPermission methods are missing; add two unit tests mirroring the existing pattern: testHasCreateUserPermission_WhenNoPermission should assertFalse against the default AbstractPermissionValidatorImpl (or a validator instance without permissions), and testHasCreateUserPermission_WhenWithPermission should assertTrue using AbstractPermissionValidatorWithPermissionsImpl (constructed appropriately, e.g., with an allowed permissions list); place the tests alongside other permission tests to exercise both overloads (no-arg and String userId if applicable).apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/SystemRoleManagerService.java (1)
38-40:buildCreateApplicationRoleNameis semantically misleading when used forCREATE_USER_ROLE_NAME.The
RoleUtils.buildCreateApplicationRoleNamemethod is reused here purely as a string joiner, but its name implies it is specific to app creation. Consider extracting a generic helper (e.g.,buildSystemRoleName) inRoleUtilsto avoid confusion for future contributors.♻️ Suggested refactor in RoleUtils
// In RoleUtils.java — add alongside buildCreateApplicationRoleName public static String buildSystemRoleName(String permissionType, String permissionTargetId) { return STRING_JOINER.join(permissionType, permissionTargetId); }Then in
SystemRoleManagerService:-public static final String CREATE_USER_ROLE_NAME = - RoleUtils.buildCreateApplicationRoleName(PermissionType.CREATE_USER, - SYSTEM_PERMISSION_TARGET_ID); +public static final String CREATE_USER_ROLE_NAME = + RoleUtils.buildSystemRoleName(PermissionType.CREATE_USER, + SYSTEM_PERMISSION_TARGET_ID);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/SystemRoleManagerService.java` around lines 38 - 40, The constant CREATE_USER_ROLE_NAME is using RoleUtils.buildCreateApplicationRoleName as a generic string joiner which is semantically misleading; add a new generic helper method in RoleUtils (e.g., buildSystemRoleName(String permissionType, String permissionTargetId)) that uses the same STRING_JOINER logic, then replace calls in SystemRoleManagerService (and any other non-application-specific sites) to use RoleUtils.buildSystemRoleName(PermissionType.CREATE_USER, SYSTEM_PERMISSION_TARGET_ID) instead of RoleUtils.buildCreateApplicationRoleName so the intent is clear and application-specific naming is preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/UserController.java`:
- Around line 72-112: The createUser method currently returns HTTP 200 OK but
should return 201 Created with a Location header to follow REST conventions:
after creating the user in UserController#createUser (and retrieving createdUser
via userService.findByUserId), build the resource URI (e.g. "/users/{username}"
using createdUser.getUserId() or openUserDTO.getUsername()) and return
ResponseEntity.created(URI).body(createdUser) instead of
ResponseEntity.ok(createdUser) so clients receive 201 and the Location header.
---
Outside diff comments:
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/PermissionController.java`:
- Around line 1-17: The build is failing due to Spotless formatting violations
in PermissionController.java (package declaration and file header); fix by
running the formatter (mvn spotless:apply), review and stage the updated
formatting changes for the PermissionController class, then re-run mvn
test/build to verify the Spotless checks pass before pushing the commit.
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/RoleInitializationService.java`:
- Line 1: CI is failing due to Spotless formatting violations across
apollo-portal; run the Spotless formatter and commit the changes: execute mvn
spotless:apply in the apollo-portal module, review and stage the modified Java
files (including RoleInitializationService and any other touched classes), then
re-run mvn -DskipTests verify or push the formatted changes so the GitHub
Actions "code style check" passes.
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/SystemRoleManagerService.java`:
- Around line 1-17: The file SystemRoleManagerService has Spotless formatting
violations; run mvn spotless:apply to auto-fix formatting, review the resulting
changes (license header spacing, package declaration, imports and
method/annotation formatting within SystemRoleManagerService), stage and commit
the modified files, and push the updated branch so the PR passes Spotless
checks.
In
`@apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/service/ConsumerServiceTest.java`:
- Around line 1-17: Spotless formatting violations were detected in the
apollo-portal module (e.g., file ConsumerServiceTest.java); run the formatter to
fix them by executing mvn spotless:apply in the apollo-portal module, then
review modified files (including ConsumerServiceTest.java) and commit the
formatting changes before pushing the PR; re-run CI to ensure the Spotless check
passes.
In
`@apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/component/AbstractPermissionValidatorTest.java`:
- Around line 1-17: The file AbstractPermissionValidatorTest.java has Spotless
formatting violations; run the formatter (mvn spotless:apply) and commit the
resulting changes so the file conforms to project style (fix license/header
spacing, import ordering, indentation, trailing spaces, and any line-wrapping
issues). After running mvn spotless:apply, verify the class
AbstractPermissionValidatorTest compiles cleanly and include the reformatted
file in the PR.
---
Duplicate comments:
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/UserController.java`:
- Around line 141-157: The controller validates offset/limit in
UserController.searchUsers but SpringSecurityUserService.searchUsers ignores
those params and returns all matches; update the implementation so pagination is
honored: either modify SpringSecurityUserService.searchUsers(signature with
parameters keyword, offset, limit, includeInactiveUsers) to apply offset/limit
when querying/filtering (preferred), or have UserController call a paginated
service method or slice the returned List<UserInfo] before returning; ensure the
unique symbols referenced are UserController.searchUsers and
SpringSecurityUserService.searchUsers and that includeInactiveUsers and keyword
are passed through and used with proper bounds when applying pagination.
- Around line 89-107: The controller currently tight-couples to
SpringSecurityUserService and throws UnsupportedOperationException; instead add
a create(UserPO) method to the UserService interface (SPI), implement that
method in SpringSecurityUserService, and then update UserController to remove
the instanceof check and cast and simply call userService.create(userPO); ensure
the new service method signature matches UserPO and populate/validate defaults
in the controller as before, and have the service implementation throw a mapped
runtime exception (or return a result) that your existing Spring exception
handlers translate to a proper HTTP client error if creation is unsupported.
- Around line 122-128: The handler getUserByUserId in UserController currently
throws BadRequestException when userService.findByUserId(userId) returns null,
which yields HTTP 400; change this to return HTTP 404 by either throwing
NotFoundException (replace BadRequestException with NotFoundException) or by
returning ResponseEntity.notFound().build() when userInfo is null; update the
null-check in getUserByUserId (and any tests) to use NotFoundException or the
ResponseEntity.notFound() path and keep the successful path as
ResponseEntity.ok(userInfo).
---
Nitpick comments:
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/service/ConsumerService.java`:
- Around line 279-298: The two methods isAllowCreateUser and
assignCreateUserRoleToConsumer duplicate logic from isAllowCreateApplication and
assignCreateApplicationRoleToConsumer; refactor by extracting shared helpers:
create a boolean helper isAllowRole(List<Long> consumerIdList, Role role) used
by both isAllowCreateUser and isAllowCreateApplication, and create a generic
assignSystemRoleToConsumer(String token, Supplier<Role> roleLookup, String
roleName) to encapsulate the role assignment flow used by
assignCreateUserRoleToConsumer and assignCreateApplicationRoleToConsumer; update
the four methods to call these helpers and remove duplicated
repository/role-check logic while preserving current behavior and error
handling.
- Around line 340-360: The methods assignCreateUserRoleToConsumer and
assignCreateApplicationRoleToConsumer perform reads followed by writes but lack
transaction boundaries; annotate both methods with `@Transactional` to ensure the
read-then-write sequence is executed in a single transaction when called from
non-transactional contexts, and add the necessary import
(org.springframework.transaction.annotation.Transactional) if missing; keep the
default propagation and rollback behavior unless a different semantics is
required.
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/SystemRoleManagerService.java`:
- Around line 38-40: The constant CREATE_USER_ROLE_NAME is using
RoleUtils.buildCreateApplicationRoleName as a generic string joiner which is
semantically misleading; add a new generic helper method in RoleUtils (e.g.,
buildSystemRoleName(String permissionType, String permissionTargetId)) that uses
the same STRING_JOINER logic, then replace calls in SystemRoleManagerService
(and any other non-application-specific sites) to use
RoleUtils.buildSystemRoleName(PermissionType.CREATE_USER,
SYSTEM_PERMISSION_TARGET_ID) instead of RoleUtils.buildCreateApplicationRoleName
so the intent is clear and application-specific naming is preserved.
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/defaultimpl/DefaultRoleInitializationService.java`:
- Around line 160-180: Both initCreateUserRole and initCreateAppRole share
identical logic; extract that logic into a private helper initSystemRole(String
roleName, PermissionType permissionType) and have both initCreateUserRole and
initCreateAppRole call it. The helper should perform the same steps now in
initCreateUserRole: check rolePermissionService.findRoleByRoleName(roleName),
lookup permission via
permissionRepository.findTopByPermissionTypeAndTargetId(permissionType,
SystemRoleManagerService.SYSTEM_PERMISSION_TARGET_ID), create the permission
with createPermission(...) and rolePermissionService.createPermission(...) if
missing, then create the role with createRole(roleName, "apollo") and call
rolePermissionService.createRoleWithPermissions(createdRole,
Sets.newHashSet(createdPermission.getId())). Preserve the `@Transactional`
behavior (keep it on the public init methods or move to the helper if desired)
and reference SystemRoleManagerService.CREATE_USER_ROLE_NAME and
PermissionType.CREATE_USER when calling the new helper.
In `@apollo-portal/src/main/resources/static/open/manage.html`:
- Around line 95-97: The template currently pipes the entire ternary through
translate so numeric consumer.rateLimit is unnecessarily translated; change the
interpolation to apply translate only to the fallback string by grouping the
ternary and piping the fallback, e.g. use consumer.rateLimit &&
consumer.rateLimit > 0 ? consumer.rateLimit :
('Open.Manage.Consumer.RateLimitValue.Display' | translate) so that
consumer.rateLimit is shown raw and the translate filter only runs for the
fallback key.
In
`@apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/service/ConsumerServiceTest.java`:
- Around line 408-432: The test assignCreateUserRoleToConsumer_alreadyAssigned
should assert the returned ConsumerRole contains the expected role id; after
calling consumerService.assignCreateUserRoleToConsumer(token) and asserting
result is not null and consumerId matches, also assert result.getRoleId() equals
createUserRoleId (the role id used to build createUserRole and
existingConsumerRole) so a mismatched roleId would fail the test.
In
`@apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/component/AbstractPermissionValidatorTest.java`:
- Around line 207-298: Tests for the newly overridden hasCreateUserPermission
methods are missing; add two unit tests mirroring the existing pattern:
testHasCreateUserPermission_WhenNoPermission should assertFalse against the
default AbstractPermissionValidatorImpl (or a validator instance without
permissions), and testHasCreateUserPermission_WhenWithPermission should
assertTrue using AbstractPermissionValidatorWithPermissionsImpl (constructed
appropriately, e.g., with an allowed permissions list); place the tests
alongside other permission tests to exercise both overloads (no-arg and String
userId if applicable).
In
`@apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ConsumerControllerTest.java`:
- Around line 53-70: The test createWithCompatibility should assert that
ConsumerService.assignCreateUserRoleToConsumer is not invoked; update the test
(ConsumerControllerTest#createWithCompatibility) to add a
Mockito.verify(consumerService,
Mockito.times(0)).assignCreateUserRoleToConsumer(Mockito.any()) (or
Mockito.never()) after the existing verifications so the mock is checked for no
calls to assignCreateUserRoleToConsumer.
...lo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/UserController.java
Show resolved
Hide resolved
…mer services and tests
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/UserController.java`:
- Line 1: Spotless formatting violations are causing the CI to fail; run the
Spotless formatter and commit the changes: execute `mvn spotless:apply` in the
portal module (apollo-portal) to auto-fix formatting, then review and commit the
modified files (including UserController.java and any other changed files) so
the codebase adheres to the project's Spotless rules before merging.
- Around line 112-117: After calling
userService.findByUserId(openUserDTO.getUsername()) in UserController, add an
explicit null-check for the createdUser variable before using
createdUser.getUserId() in the ServletUriComponentsBuilder block; if createdUser
is null, avoid building the Location URI and return an appropriate response
(e.g., ResponseEntity.status(HttpStatus.CREATED).build() or another suitable
status/empty body) or throw a clear exception so you don't dereference a null
pointer.
- Around line 126-127: The two GET handlers in UserController (getUserByUserId
and searchUsers) are incorrectly protected by
`@PreAuthorize`("@unifiedPermissionValidator.hasCreateUserPermission()"); update
their authorization to a read-specific check by replacing
hasCreateUserPermission() with a new
unifiedPermissionValidator.hasViewUserPermission() (or equivalent read
permission) and wire that new method into unifiedPermissionValidator, or if the
endpoints should be public to broader read access, remove the `@PreAuthorize`
annotation from those methods; ensure changes target the `@PreAuthorize`
annotations on getUserByUserId and searchUsers and add backend validation logic
for hasViewUserPermission in unifiedPermissionValidator.
---
Duplicate comments:
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/UserController.java`:
- Around line 154-163: The controller validates offset/limit but
userService.searchUsers(...) ignores them and returns all matches, so implement
server-side slicing in UserController: call userService.searchUsers(keyword, 0,
Integer.MAX_VALUE, includeInactiveUsers) to obtain the full list, compute from =
Math.min(offset, all.size()) and to = Math.min(offset + limit, all.size()), then
return ResponseEntity.ok(all.subList(from, to)); ensure you reference the
existing UserController method that currently calls userService.searchUsers(...)
and use the offset and limit variables for the subList bounds.
- Around line 130-132: The code in UserController currently throws
BadRequestException when userInfo is null, causing a 400 response; change this
to return a 404 by throwing the appropriate NotFound exception (or a custom
NotFoundException) instead of BadRequestException in the block that checks
userInfo (the variables/methods to update: UserController, the userInfo
null-check and the thrown exception referencing userId). Ensure the new
exception maps to HTTP 404 in existing exception handlers so "User not found:
{userId}" yields a 404 response.
- Around line 92-109: UserController is breaking the UserService SPI by
instanceof-casting userService to SpringSecurityUserService and throwing
UnsupportedOperationException (which yields HTTP 500); instead add a default
create(UserPO) method to the UserService interface that throws
UnsupportedOperationException, remove the instanceof and cast from
UserController and call userService.create(userPO) directly, and add a
ControllerAdvice (or custom exception type) that translates
UnsupportedOperationException from create(...) into HTTP 501 Not Implemented so
non-SpringSecurity implementations (e.g. LDAP/OIDC) receive a proper 501
response.
...lo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/UserController.java
Show resolved
Hide resolved
Comment on lines
+112
to
+117
| UserInfo createdUser = userService.findByUserId(openUserDTO.getUsername()); | ||
| URI location = ServletUriComponentsBuilder.fromCurrentRequest() | ||
| .replacePath("/openapi/v1/users/{userId}") | ||
| .buildAndExpand(createdUser.getUserId()) | ||
| .toUri(); | ||
| return ResponseEntity.created(location).body(createdUser); |
Contributor
There was a problem hiding this comment.
Guard against null createdUser before building the Location URI.
userService.findByUserId() can return null (per SpringSecurityUserService.findByUserId). If it does, createdUser.getUserId() on line 115 will throw a NullPointerException. Add an explicit null-check after the lookup.
🛡️ Proposed fix
UserInfo createdUser = userService.findByUserId(openUserDTO.getUsername());
+ if (createdUser == null) {
+ throw new BadRequestException("User was created but could not be retrieved: " + openUserDTO.getUsername());
+ }
URI location = ServletUriComponentsBuilder.fromCurrentRequest()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/UserController.java`
around lines 112 - 117, After calling
userService.findByUserId(openUserDTO.getUsername()) in UserController, add an
explicit null-check for the createdUser variable before using
createdUser.getUserId() in the ServletUriComponentsBuilder block; if createdUser
is null, avoid building the Location URI and return an appropriate response
(e.g., ResponseEntity.status(HttpStatus.CREATED).build() or another suitable
status/empty body) or throw a clear exception so you don't dereference a null
pointer.
Comment on lines
+126
to
+127
| @PreAuthorize(value = "@unifiedPermissionValidator.hasCreateUserPermission()") | ||
| @GetMapping("/users/{userId}") |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for permission validator and user-related permissions
echo "=== UnifiedPermissionValidator class with permission methods ==="
rg -n 'class UnifiedPermissionValidator' --type=java -A 30
echo -e "\n=== All hasXxxUserPermission methods ==="
rg -n 'has.*UserPermission|hasPermission' --type=java | grep -i user
echo -e "\n=== UserController.java content around lines 126-127 and 145-146 ==="
fd 'UserController.java' --type=f -x cat -n {}Repository: apolloconfig/apollo
Length of output: 22105
🏁 Script executed:
#!/bin/bash
# More targeted search for permission methods
echo "=== hasCreateUserPermission definition ==="
rg -n 'hasCreateUserPermission' --type=java -B 2 -A 8
echo -e "\n=== hasViewUserPermission or similar ==="
rg -n 'hasViewUser|hasReadUser|hasListUser|hasGetUser' --type=javaRepository: apolloconfig/apollo
Length of output: 19078
🏁 Script executed:
#!/bin/bash
# Check if there are other permission types defined (READ_USER, VIEW_USER, etc.)
echo "=== All PermissionType constants ==="
rg -n 'PermissionType\.' --type=java | grep -i user | head -20
echo -e "\n=== Check if other controllers have read/write permission separation ==="
rg -n '@GetMapping|@PostMapping' --type=java -B 2 | grep -A 2 'PreAuthorize' | head -40Repository: apolloconfig/apollo
Length of output: 6148
GET endpoints getUserByUserId and searchUsers incorrectly gated behind hasCreateUserPermission().
Both read-only endpoints at lines 124 and 143 require hasCreateUserPermission(), which is semantically incorrect. This permission is for user creation (write/admin scope), not for user lookups (read). No dedicated read-user permission exists in the codebase. Either introduce a separate hasViewUserPermission() (or equivalent read permission) for these GET endpoints, or reconsider the authorization requirement if these lookups should be accessible to a broader set of users.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/UserController.java`
around lines 126 - 127, The two GET handlers in UserController (getUserByUserId
and searchUsers) are incorrectly protected by
`@PreAuthorize`("@unifiedPermissionValidator.hasCreateUserPermission()"); update
their authorization to a read-specific check by replacing
hasCreateUserPermission() with a new
unifiedPermissionValidator.hasViewUserPermission() (or equivalent read
permission) and wire that new method into unifiedPermissionValidator, or if the
endpoints should be public to broader read access, remove the `@PreAuthorize`
annotation from those methods; ensure changes target the `@PreAuthorize`
annotations on getUserByUserId and searchUsers and add backend validation logic
for hasViewUserPermission in unifiedPermissionValidator.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/service/ConsumerServiceTest.java (1)
274-309: Pre-existingallowCreateApplicationtest now inadvertently exercisesallowCreateUser=true.The
when(rolePermissionService.findRoleByRoleName(any())).thenReturn(role)stub matches the newgetCreateUserRole()call introduced by this PR. Combined with thefindByConsumerIdAndRoleIdstub using the sameroleId,consumerInfo.isAllowCreateUser()will also betruein that test — but without any assertion. This means a regression in theallowCreateUserpath could go undetected.Consider scoping the stub to the specific role name (
eq(CREATE_APPLICATION_ROLE_NAME)) and adding a negative assertion forallowCreateUser.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/service/ConsumerServiceTest.java` around lines 274 - 309, The test allowCreateApplication currently stubs rolePermissionService.findRoleByRoleName(any()) which unintentionally matches getCreateUserRole() and makes consumerInfo.isAllowCreateUser() true; update the stub to only match the create-application role (use eq(CREATE_APPLICATION_ROLE_NAME) with rolePermissionService.findRoleByRoleName) and keep the existing consumerRoleRepository.findByConsumerIdAndRoleId stub, then add an explicit negative assertion assertFalse(consumerInfo.isAllowCreateUser()) to ensure create-user is not granted.apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/service/ConsumerService.java (2)
19-22: Minor import inconsistency:CREATE_APPLICATION_ROLE_NAMEuses a static import,CREATE_USER_ROLE_NAMEdoes not.Line 19 statically imports
CREATE_APPLICATION_ROLE_NAME, while the new code referencesSystemRoleManagerService.CREATE_USER_ROLE_NAMEvia the class. Pick one style (static imports for both, or qualified for both) to be consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/service/ConsumerService.java` around lines 19 - 22, The imports in ConsumerService.java are inconsistent: CREATE_APPLICATION_ROLE_NAME is statically imported while CREATE_USER_ROLE_NAME is referenced via SystemRoleManagerService; make them consistent by choosing one style and applying it to both symbols—either add a static import for SystemRoleManagerService.CREATE_USER_ROLE_NAME or remove the static import and reference both as SystemRoleManagerService.CREATE_APPLICATION_ROLE_NAME and SystemRoleManagerService.CREATE_USER_ROLE_NAME; update the import section accordingly and adjust any usages in the class to match the chosen style.
277-296: Code duplication:isAllowCreate*andassignCreate*RoleToConsumermethods.
isAllowCreateUser(List<Long>)(lines 277-296) is structurally identical toisAllowCreateApplication(List<Long>)(lines 256-275). Same forassignCreateUserRoleToConsumervsassignCreateApplicationRoleToConsumer, andgetCreateUserRolevsgetCreateAppRole. This pattern will need to be replicated again for any future system role.Consider extracting a generic helper, e.g.:
private List<Boolean> isAllowSystemRole(List<Long> consumerIdList, String roleName) { Role role = rolePermissionService.findRoleByRoleName(roleName); if (role == null) { return Collections.nCopies(consumerIdList.size(), false); } long roleId = role.getId(); return consumerIdList.stream() .map(id -> consumerRoleRepository.findByConsumerIdAndRoleId(id, roleId) != null) .collect(Collectors.toList()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/service/ConsumerService.java` around lines 277 - 296, The two pairs of methods isAllowCreateUser / isAllowCreateApplication (and their assignCreateUserRoleToConsumer / assignCreateApplicationRoleToConsumer and getCreateUserRole / getCreateAppRole counterparts) are duplicated; extract a generic helper such as isAllowSystemRole(List<Long>, String) that calls rolePermissionService.findRoleByRoleName(roleName) (or a shared getSystemRole(roleName)), checks for null and returns Collections.nCopies(size, false) when absent, and otherwise maps consumerId -> consumerRoleRepository.findByConsumerIdAndRoleId(id, roleId) != null; do the same pattern for assignment via assignSystemRoleToConsumer(consumerId, roleName) and replace the specialized methods/calls with these generic helpers.apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ConsumerControllerTest.java (1)
124-131: Non-English inline comments — translate to English for consistency.Lines 124, 127 contain Chinese comments (
// allowCreateUser=true 时应调用 ...,// allowCreateApplication=false 时不应调用 ...). The rest of the codebase uses English. Same applies to lines 157, 174, and 181.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ConsumerControllerTest.java` around lines 124 - 131, Replace the Chinese inline comments in ConsumerControllerTest with English equivalents for consistency: update the comments above the Mockito.verify assertions (e.g., the lines commenting assignCreateUserRoleToConsumer and assignCreateApplicationRoleToConsumer) and also the other occurrences mentioned (around lines referencing tests at 157, 174, 181) to concise English comments that describe the expected behavior (e.g., "should call assignCreateUserRoleToConsumer when allowCreateUser=true" and "should not call assignCreateApplicationRoleToConsumer when allowCreateApplication=false"); keep comment placement near the Mockito.verify calls and retain existing assertions and identifiers like consumerService, assignCreateUserRoleToConsumer, assignCreateApplicationRoleToConsumer, and getConsumerInfoByAppId.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/PermissionController.java`:
- Around line 514-528: The GET mapping path /system/role/createUser/enabled
collides with the variable path /system/role/createUser/{userId} so requests for
userId "enabled" are never routed to hasCreateUserPermission; rename the
enabled-check endpoint to a non-conflicting path (for example change the
`@GetMapping` on isCreateUserPermissionEnabled() to
/system/role/createUser/permission/enabled or similar) and update any
callers/tests/docs accordingly; verify the methods
isCreateUserPermissionEnabled() and hasCreateUserPermission(`@PathVariable` String
userId) still compile and behave as expected after the path change.
---
Duplicate comments:
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/UserController.java`:
- Around line 126-131: Replace the BadRequestException thrown in
UserController.getUserByUserId with a NotFoundException so missing users return
HTTP 404: change the throw of BadRequestException("User not found: " + userId)
to use the NotFoundException factory (e.g.,
NotFoundException.userNotFound(userId)); if that factory does not exist add a
suitable static factory method on NotFoundException that creates the proper 404
exception for a user ID and use it from getUserByUserId.
- Around line 124-132: The GET endpoints getUserByUserId and searchUsers
currently use
`@PreAuthorize`("@unifiedPermissionValidator.hasCreateUserPermission()") which
enforces a create-only privilege for read operations; update the annotations to
check a read/view permission instead (e.g.
`@PreAuthorize`("@unifiedPermissionValidator.hasViewUserPermission()" or
hasReadUserPermission()) or remove the guard if reads are public, and implement
the corresponding hasViewUserPermission/hasReadUserPermission method in
unifiedPermissionValidator if it doesn't exist so read-only callers are
authorized correctly.
- Around line 91-109: The controller currently does an instanceof check and cast
to SpringSecurityUserService and throws UnsupportedOperationException; instead
add a create(UserPO) method to the UserService SPI (interface) with a default
implementation that throws a mapped API exception (e.g., a custom
BadRequest/NotImplemented runtime mapped by your exception handler) so
implementations that support creation override it; then update UserController to
call userService.create(userPO) (remove the instanceof/cast) and rely on the
default exception mapping for unsupported implementations. Ensure you reference
UserService.create(UserPO), SpringSecurityUserService (to override),
UserController (to remove the cast), and replace UnsupportedOperationException
with the mapped API exception in the default method.
- Around line 112-115: UserController currently dereferences createdUser (via
userService.findByUserId(openUserDTO.getUsername())) without null-checking,
which can cause an NPE; update the post-create flow to handle a null return from
userService.findByUserId by checking createdUser for null and returning an
appropriate ResponseEntity (e.g., ResponseEntity.status(HttpStatus.ACCEPTED) or
ResponseEntity.status(HttpStatus.CREATED).location(location).build() with no
body) or by fetching/constructing a fallback UserInfo before calling
buildAndExpand(createdUser.getUserId()); ensure changes touch the createdUser
variable, the URI construction using
ServletUriComponentsBuilder.buildAndExpand(...), and the final
ResponseEntity.created(...) call so no null is dereferenced.
- Around line 145-161: The controller validates offset/limit in
UserController.searchUsers but then calls userService.searchUsers(keyword,
offset, limit, includeInactiveUsers) while the implementation in
SpringSecurityUserService.searchUsers ignores offset/limit; update
SpringSecurityUserService.searchUsers (and any underlying helpers it uses) to
apply pagination—slice or stream-skip/limit the matched users using the passed
offset and limit (respecting includeInactiveUsers and keyword filtering) before
returning the List<UserInfo>, or add an overloaded service method that accepts
pageable parameters and ensure UserController invokes that paginated variant.
---
Nitpick comments:
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/service/ConsumerService.java`:
- Around line 19-22: The imports in ConsumerService.java are inconsistent:
CREATE_APPLICATION_ROLE_NAME is statically imported while CREATE_USER_ROLE_NAME
is referenced via SystemRoleManagerService; make them consistent by choosing one
style and applying it to both symbols—either add a static import for
SystemRoleManagerService.CREATE_USER_ROLE_NAME or remove the static import and
reference both as SystemRoleManagerService.CREATE_APPLICATION_ROLE_NAME and
SystemRoleManagerService.CREATE_USER_ROLE_NAME; update the import section
accordingly and adjust any usages in the class to match the chosen style.
- Around line 277-296: The two pairs of methods isAllowCreateUser /
isAllowCreateApplication (and their assignCreateUserRoleToConsumer /
assignCreateApplicationRoleToConsumer and getCreateUserRole / getCreateAppRole
counterparts) are duplicated; extract a generic helper such as
isAllowSystemRole(List<Long>, String) that calls
rolePermissionService.findRoleByRoleName(roleName) (or a shared
getSystemRole(roleName)), checks for null and returns Collections.nCopies(size,
false) when absent, and otherwise maps consumerId ->
consumerRoleRepository.findByConsumerIdAndRoleId(id, roleId) != null; do the
same pattern for assignment via assignSystemRoleToConsumer(consumerId, roleName)
and replace the specialized methods/calls with these generic helpers.
In
`@apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/service/ConsumerServiceTest.java`:
- Around line 274-309: The test allowCreateApplication currently stubs
rolePermissionService.findRoleByRoleName(any()) which unintentionally matches
getCreateUserRole() and makes consumerInfo.isAllowCreateUser() true; update the
stub to only match the create-application role (use
eq(CREATE_APPLICATION_ROLE_NAME) with rolePermissionService.findRoleByRoleName)
and keep the existing consumerRoleRepository.findByConsumerIdAndRoleId stub,
then add an explicit negative assertion
assertFalse(consumerInfo.isAllowCreateUser()) to ensure create-user is not
granted.
In
`@apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ConsumerControllerTest.java`:
- Around line 124-131: Replace the Chinese inline comments in
ConsumerControllerTest with English equivalents for consistency: update the
comments above the Mockito.verify assertions (e.g., the lines commenting
assignCreateUserRoleToConsumer and assignCreateApplicationRoleToConsumer) and
also the other occurrences mentioned (around lines referencing tests at 157,
174, 181) to concise English comments that describe the expected behavior (e.g.,
"should call assignCreateUserRoleToConsumer when allowCreateUser=true" and
"should not call assignCreateApplicationRoleToConsumer when
allowCreateApplication=false"); keep comment placement near the Mockito.verify
calls and retain existing assertions and identifiers like consumerService,
assignCreateUserRoleToConsumer, assignCreateApplicationRoleToConsumer, and
getConsumerInfoByAppId.
Comment on lines
+514
to
+528
| @GetMapping("/system/role/createUser/{userId}") | ||
| public JsonObject hasCreateUserPermission(@PathVariable String userId) { | ||
| JsonObject rs = new JsonObject(); | ||
| rs.addProperty("hasCreateUserPermission", | ||
| unifiedPermissionValidator.hasCreateUserPermission(userId)); | ||
| return rs; | ||
| } | ||
|
|
||
| @GetMapping("/system/role/createUser/enabled") | ||
| public JsonObject isCreateUserPermissionEnabled() { | ||
| JsonObject rs = new JsonObject(); | ||
| rs.addProperty("isCreateUserPermissionEnabled", | ||
| systemRoleManagerService.isCreateUserPermissionEnabled()); | ||
| return rs; | ||
| } |
Contributor
There was a problem hiding this comment.
Path segment enabled shadows any user whose ID is literally "enabled".
Spring MVC prefers the literal /system/role/createUser/enabled over the variable /system/role/createUser/{userId}, so hasCreateUserPermission("enabled") can never be reached. This is an edge case in practice, but the behavior is invisible to API consumers and could cause a confusing 200 response from isCreateUserPermissionEnabled instead of hasCreateUserPermission.
Consider renaming the enabled-check endpoint (e.g., /system/role/createUser/permission/enabled) to avoid the reserved segment.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/PermissionController.java`
around lines 514 - 528, The GET mapping path /system/role/createUser/enabled
collides with the variable path /system/role/createUser/{userId} so requests for
userId "enabled" are never routed to hasCreateUserPermission; rename the
enabled-check endpoint to a non-conflicting path (for example change the
`@GetMapping` on isCreateUserPermissionEnabled() to
/system/role/createUser/permission/enabled or similar) and update any
callers/tests/docs accordingly; verify the methods
isCreateUserPermissionEnabled() and hasCreateUserPermission(`@PathVariable` String
userId) still compile and behave as expected after the path change.
Contributor
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/PermissionController.java`:
- Around line 513-527: The two controller endpoints collide because the literal
path segment "enabled" shadows the path variable in hasCreateUserPermission;
rename the enabled-check mapping to a non-colliding path (for example change
isCreateUserPermissionEnabled()'s `@GetMapping` from
"/system/role/createUser/enabled" to "/system/role/createUser/isEnabled" or
"/system/role/createUser/enabled-status") and update all callers/tests
accordingly; keep the method name isCreateUserPermissionEnabled and
systemRoleManagerService.isCreateUserPermissionEnabled() unchanged so only the
`@GetMapping` value and any client references need updating, and run a text search
for "createUser/enabled" to find usages to update.
Member
|
这轮更新辛苦了，权限模型这块已经比之前完善很多 👍 不过还有一个需要先对齐的阻塞点（这里也先致歉下）：我们之前在贡献文档里没把这个约束写清楚，容易让人返工。 Apollo 这边的 OpenAPI 现在是走 “apollo-openapi 仓库里的 YAML spec -> apollo 里用 openapi-generator 生成代码” 这条链路的，不是直接手写 DTO/Controller。 相关信息：
这次 PR 里是手写了 OpenUserDTO 和 UserController，会绕开 spec 合约链路。 也想请 openapi 维护者帮忙看下：是否要把这条开发约束补到文档里，避免后续贡献者再踩坑。 |
This was referenced Feb 25, 2026
feat: add user management endpoints for creating and retrieving users
apolloconfig/apollo-openapi#21
Open
Author
|
@nobodyiam 请问apollo-openaopi仓库的贡献流程是什么，我尝试在apollo-openaopi仓库中提起了一个issue以及pr，但是没有人回应我。下面是apollo-openapi仓库的pr链接：apolloconfig/apollo-openapi#21 |
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.
What's the purpose of this PR
提供一个创建用户的OpenAPI，本次仅实现创建用户 API，通知能力后续 PR 跟进
Which issue(s) this PR fixes:
Part of #5441
Brief changelog
添加了一个UserController用于提供创建/查询用户的API
Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean testto make sure this pull request doesn't break anything.mvn spotless:applyto format your code.CHANGESlog.Summary by CodeRabbit
New Features
Permissions & Roles
UI & Localization
Tests