Feat/app template and login API #273
Conversation
fix: preview data bug
Refactor/develop
fixt： fix blockgroup and block
Refactor/develop
fixt： fix blockgroup and block
Refactor/develop
…ngine-backend-java into refactor/develop
fix:fix block and group
Refactor/develop
…ne-backend-java into refactor/develop
Refactor/develop
Refactor/develop
feat: create component by bundle.json
fix:format code and fix style issue
Refactor/develop
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 26
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
base/src/main/java/com/tinyengine/it/controller/ResourceController.java (1)
181-188: Update API documentation to include the appId parameter.The Swagger documentation is incomplete—it only documents the
fileparameter but not the newly addedappIdparameter.Apply this diff to add the missing parameter documentation:
@Operation(summary = "上传图片", description = "上传图片", parameters = { - @Parameter(name = "file", description = "图片") + @Parameter(name = "file", description = "图片"), + @Parameter(name = "appId", description = "应用ID", required = true) }, responses = {
🧹 Nitpick comments (9)
base/src/main/java/com/tinyengine/it/login/config/RestTemplateConfig.java (1)
19-21: Consider expanding the JavaDoc comment.The current JavaDoc is minimal. Consider adding a brief description of the RestTemplate's purpose in the login/authentication context.
Example:
/** - * restTemplate + * Configuration for RestTemplate bean used in SSO/authentication service calls. + * The RestTemplate instance is thread-safe and shared across the application. */base/src/main/java/com/tinyengine/it/controller/ResourceController.java (1)
69-69: Remove unusedloginUserContextautowired field.The
loginUserContextfield is never used in this controller and should be removed to clean up unused dependencies.docker-deploy-data/mysql/init/init_data_2025_1023.sql (1)
1-13: Make the seed script idempotentIf this seed ever runs against a database that already contains these IDs, it will error out. Consider appending an
ON DUPLICATE KEY UPDATE(or similar guard) so replays remain harmless.-INSERT INTO `t_business_category` (...columns...) VALUES (...values...); +INSERT INTO `t_business_category` (...columns...) VALUES (...values...) +ON DUPLICATE KEY UPDATE + `name` = VALUES(`name`), + `business_group` = VALUES(`business_group`), + `last_updated_by` = VALUES(`last_updated_by`), + `last_updated_time` = VALUES(`last_updated_time`);base/src/main/java/com/tinyengine/it/service/app/impl/PageHistoryServiceImpl.java (1)
109-113: Tidy the Javadoc parameter orderThe
@paramtags are reversed relative to the method signature. Swapping them keeps the docs precise and avoids confusion.- * @param app the app - * @param name the page name + * @param name the page name + * @param app the appapp/src/main/resources/sql/h2/init_data_2025_1023.sql (1)
1-13: Consider updating seed data timestamps.The timestamps (
2025-10-14 00:26:27) predate the PR creation date (2025-11-07). While this doesn't affect functionality, using more recent or current timestamps would better reflect when this seed data was introduced.app/src/main/resources/sql/mysql/init_data_2025_1023.sql (1)
1-20: Consider updating seed data timestamps.The timestamps (
2025-10-14and2025-10-29) predate the PR creation date (2025-11-07). While this doesn't affect functionality, aligning timestamps with the actual introduction date would improve traceability.base/src/main/java/com/tinyengine/it/service/app/UserService.java (1)
72-72: Narrow the thrown exception type.Declaring
throws Exceptionon the service interface forces all callers to deal with a completely generic checked exception and makes error handling unpredictable. Please expose a domain-specific checked/unchecked exception (or wrap the checked ones inside) so the contract stays precise.base/src/main/java/com/tinyengine/it/login/model/PasswordResult.java (1)
25-28: Remove the redundant constructor.Lombok's
@Dataannotation combined withfinalfields automatically generates an all-args constructor. The explicit constructor is redundant and unnecessary.Apply this diff:
@Data public class PasswordResult { private final String passwordHash; private final String salt; - - public PasswordResult(String passwordHash, String salt) { - this.passwordHash = passwordHash; - this.salt = salt; - } }base/src/main/java/com/tinyengine/it/login/model/UserInfo.java (1)
18-55: Consider using Lombok for consistency with other model classes.UserInfo manually implements getters/setters while other models in the same package (PasswordResult, ValidationResult, SSOTicket, PasswordValidationResult) use Lombok's
@Data. This inconsistency increases maintenance burden and verbosity.Refactor to use Lombok:
+import lombok.Data; + /** * 用户信息内部类 */ +@Data public class UserInfo { private String userId; private String username; private String tenantId; private String renterId; private int platformId; private String siteId; private String roles; private String token; private String department; - // 构造器 public UserInfo(String userId, String username, String tenantId) { this.userId = userId; this.username = username; this.tenantId = tenantId; } - - // getter和setter方法 - public String getUserId() { return userId; } - public void setUserId(String userId) { this.userId = userId; } - public String getUsername() { return username; } - public void setUsername(String username) { this.username = username; } - public String getTenantId() { return tenantId; } - public void setTenantId(String tenantId) { this.tenantId = tenantId; } - public String getRenterId() { return renterId; } - public void setRenterId(String renterId) { this.renterId = renterId; } - public int getPlatformId() { return platformId; } - public void setPlatformId(int platformId) { this.platformId = platformId; } - public String getSiteId() { return siteId; } - public void setSiteId(String siteId) { this.siteId = siteId; } - public String getRoles() { return roles; } - public void setRoles(String roles) { this.roles = roles; } - public String getToken() { return token; } - public void setToken(String token) { this.token = token; } - public String getDepartment() { return department; } - public void setDepartment(String department) { this.department = department; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (54)
app/src/main/java/com/tinyengine/it/config/context/DefaultLoginUserContext.java(0 hunks)app/src/main/resources/sql/h2/init_data_2025_1023.sql(1 hunks)app/src/main/resources/sql/h2/update_table_ddl_2025_1014.sql(1 hunks)app/src/main/resources/sql/mysql/create_permission_table_ddl_2025_1029.sql(1 hunks)app/src/main/resources/sql/mysql/init_data_2025_1023.sql(1 hunks)app/src/main/resources/sql/mysql/update_table_ddl_2025_1014.sql(1 hunks)base/src/main/java/com/tinyengine/it/common/context/LoginUserContext.java(0 hunks)base/src/main/java/com/tinyengine/it/common/exception/ExceptionEnum.java(1 hunks)base/src/main/java/com/tinyengine/it/controller/AppController.java(3 hunks)base/src/main/java/com/tinyengine/it/controller/AppTemplateController.java(1 hunks)base/src/main/java/com/tinyengine/it/controller/BusinessCategoryController.java(1 hunks)base/src/main/java/com/tinyengine/it/controller/ResourceController.java(2 hunks)base/src/main/java/com/tinyengine/it/login/Utils/JwtUtil.java(1 hunks)base/src/main/java/com/tinyengine/it/login/Utils/SM2EncryptionUtil.java(1 hunks)base/src/main/java/com/tinyengine/it/login/Utils/SM3PasswordUtil.java(1 hunks)base/src/main/java/com/tinyengine/it/login/config/LoginConfig.java(1 hunks)base/src/main/java/com/tinyengine/it/login/config/PasswordPolicyConfig.java(1 hunks)base/src/main/java/com/tinyengine/it/login/config/RestTemplateConfig.java(1 hunks)base/src/main/java/com/tinyengine/it/login/config/SSOInterceptor.java(1 hunks)base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java(1 hunks)base/src/main/java/com/tinyengine/it/login/controller/LoginController.java(1 hunks)base/src/main/java/com/tinyengine/it/login/model/PasswordResult.java(1 hunks)base/src/main/java/com/tinyengine/it/login/model/PasswordValidationResult.java(1 hunks)base/src/main/java/com/tinyengine/it/login/model/SSOTicket.java(1 hunks)base/src/main/java/com/tinyengine/it/login/model/UserInfo.java(1 hunks)base/src/main/java/com/tinyengine/it/login/model/ValidationResult.java(1 hunks)base/src/main/java/com/tinyengine/it/login/service/ConfigurablePasswordValidator.java(1 hunks)base/src/main/java/com/tinyengine/it/mapper/AppMapper.java(2 hunks)base/src/main/java/com/tinyengine/it/mapper/ModelMapper.java(2 hunks)base/src/main/java/com/tinyengine/it/mapper/PageHistoryMapper.java(2 hunks)base/src/main/java/com/tinyengine/it/model/dto/AppDto.java(1 hunks)base/src/main/java/com/tinyengine/it/model/dto/I18nEntryDto.java(1 hunks)base/src/main/java/com/tinyengine/it/model/entity/App.java(3 hunks)base/src/main/java/com/tinyengine/it/model/entity/AppExtension.java(1 hunks)base/src/main/java/com/tinyengine/it/model/entity/BusinessCategory.java(1 hunks)base/src/main/java/com/tinyengine/it/model/entity/Model.java(1 hunks)base/src/main/java/com/tinyengine/it/model/entity/User.java(3 hunks)base/src/main/java/com/tinyengine/it/service/app/AppService.java(2 hunks)base/src/main/java/com/tinyengine/it/service/app/AppTemplateService.java(1 hunks)base/src/main/java/com/tinyengine/it/service/app/UserService.java(2 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/AppServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/AppTemplateServiceImpl.java(1 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/PageHistoryServiceImpl.java(1 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/UserServiceImpl.java(3 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/ResourceServiceImpl.java(0 hunks)base/src/main/resources/mappers/AppMapper.xml(9 hunks)base/src/main/resources/mappers/BusinessCategoryMapper.xml(6 hunks)base/src/main/resources/mappers/ModelMapper.xml(5 hunks)base/src/main/resources/mappers/PageHistoryMapper.xml(1 hunks)base/src/main/resources/mappers/UserMapper.xml(5 hunks)base/src/test/java/com/tinyengine/it/service/app/impl/UserServiceImplTest.java(2 hunks)docker-deploy-data/mysql/init/init_data_2025_1023.sql(1 hunks)docker-deploy-data/mysql/init/update_table_ddl_2025_1014.sql(1 hunks)pom.xml(2 hunks)
💤 Files with no reviewable changes (3)
- base/src/main/java/com/tinyengine/it/service/material/impl/ResourceServiceImpl.java
- base/src/main/java/com/tinyengine/it/common/context/LoginUserContext.java
- app/src/main/java/com/tinyengine/it/config/context/DefaultLoginUserContext.java
🧰 Additional context used
🧬 Code graph analysis (10)
base/src/main/java/com/tinyengine/it/login/model/ValidationResult.java (2)
base/src/main/java/com/tinyengine/it/login/model/PasswordResult.java (1)
Data(20-29)base/src/main/java/com/tinyengine/it/login/model/SSOTicket.java (1)
Data(20-25)
base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java (1)
base/src/main/java/com/tinyengine/it/login/model/UserInfo.java (1)
UserInfo(18-55)
base/src/main/java/com/tinyengine/it/login/model/SSOTicket.java (2)
base/src/main/java/com/tinyengine/it/login/model/PasswordResult.java (1)
Data(20-29)base/src/main/java/com/tinyengine/it/login/model/ValidationResult.java (1)
Data(20-29)
base/src/main/java/com/tinyengine/it/controller/AppTemplateController.java (1)
base/src/main/java/com/tinyengine/it/controller/AppController.java (1)
Validated(49-247)
base/src/main/java/com/tinyengine/it/login/controller/LoginController.java (1)
base/src/main/java/com/tinyengine/it/login/Utils/SM3PasswordUtil.java (1)
SM3PasswordUtil(25-79)
base/src/main/java/com/tinyengine/it/controller/BusinessCategoryController.java (2)
base/src/main/java/com/tinyengine/it/controller/AppController.java (1)
Validated(49-247)base/src/main/java/com/tinyengine/it/controller/AppTemplateController.java (1)
Validated(44-135)
base/src/main/java/com/tinyengine/it/login/model/PasswordResult.java (2)
base/src/main/java/com/tinyengine/it/login/model/SSOTicket.java (1)
Data(20-25)base/src/main/java/com/tinyengine/it/login/model/ValidationResult.java (1)
Data(20-29)
base/src/main/java/com/tinyengine/it/login/config/SSOInterceptor.java (3)
base/src/main/java/com/tinyengine/it/login/model/UserInfo.java (1)
UserInfo(18-55)base/src/main/java/com/tinyengine/it/login/Utils/JwtUtil.java (1)
Component(28-173)base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java (1)
Component(10-67)
base/src/main/java/com/tinyengine/it/login/Utils/JwtUtil.java (1)
base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java (1)
Component(10-67)
base/src/main/java/com/tinyengine/it/service/app/impl/AppTemplateServiceImpl.java (2)
base/src/main/java/com/tinyengine/it/service/app/impl/AppServiceImpl.java (1)
Service(50-258)base/src/main/java/com/tinyengine/it/service/app/impl/PageHistoryServiceImpl.java (1)
Service(36-142)
🪛 ast-grep (0.39.7)
base/src/main/java/com/tinyengine/it/login/Utils/SM3PasswordUtil.java
[warning] 37-37: Use of AES with ECB mode detected. ECB doesn't provide message confidentiality and is not semantically secure so should not be used. Instead, use a strong, secure cipher: Cipher.getInstance("AES/CBC/PKCS7PADDING"). See https://owasp.org/www-community/Using_the_Java_Cryptographic_Extensions for more information.
Context: MessageDigest.getInstance(SM3_ALGORITHM, "BC")
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures
- https://googleprojectzero.blogspot.com/2022/10/rc4-is-still-considered-harmful.html
(use-of-aes-ecb-java)
[warning] 37-37: Triple DES (3DES or DESede) is considered deprecated. AES is the recommended cipher. Upgrade to use AES.
Context: MessageDigest.getInstance(SM3_ALGORITHM, "BC")
Note: [CWE-326]: Inadequate Encryption Strength [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://find-sec-bugs.github.io/bugs.htm#TDES_USAGE
- https://csrc.nist.gov/News/2017/Update-to-Current-Use-and-Deprecation-of-TDEA
(desede-is-deprecated-java)
🔇 Additional comments (10)
base/src/main/java/com/tinyengine/it/model/dto/AppDto.java (1)
27-28: DTO payload looks strongThe fields you added give the controller a clean way to return both the list and total count. Nice and tidy.
base/src/main/resources/mappers/PageHistoryMapper.xml (1)
245-252: App-scoped history query looks goodThanks for adding this mapper—filters and selected columns line up with the service expectations.
base/src/main/java/com/tinyengine/it/service/app/AppService.java (1)
38-46: Confirm the downstream handling oforderByPlease double-check that the implementation restricts
orderByto a safe, pre-vetted column list before reaching SQL. Let me know if you need help hardening it.base/src/main/java/com/tinyengine/it/service/app/impl/AppServiceImpl.java (1)
87-114: LGTM! Pagination implementation is well-structured.The method includes appropriate bounds checking (page size clamped to 1-1000, currentPage defaulted to 1) and correctly delegates to the mapper layer.
base/src/main/resources/mappers/BusinessCategoryMapper.xml (1)
10-10: LGTM! Consistent integration of business_group field.The
business_groupcolumn is properly integrated across all mapper operations (SELECT, INSERT, UPDATE, conditions) following the existing patterns.Also applies to: 21-23, 55-57, 87-87, 144-144, 155-155
app/src/main/resources/sql/mysql/update_table_ddl_2025_1014.sql (1)
1-31: LGTM! Schema changes for app, business_category, datasource, app_extension, and model are well-structured.The column additions and index updates follow a consistent pattern and properly reflect the new data model requirements.
base/src/main/java/com/tinyengine/it/mapper/ModelMapper.java (1)
42-48: LGTM! Standard MyBatis mapper pattern.The method signature follows existing conventions, and the corresponding XML mapper implementation has been verified to exist at
base/src/main/resources/mappers/ModelMapper.xml:156.base/src/main/java/com/tinyengine/it/login/model/ValidationResult.java (1)
20-28: LGTM!Clean data model with appropriate Lombok usage and explicit constructor for non-final fields.
base/src/main/java/com/tinyengine/it/login/model/SSOTicket.java (1)
20-24: LGTM!Clean SSO ticket model with appropriate fields for token-based authentication.
base/src/main/java/com/tinyengine/it/login/model/PasswordValidationResult.java (1)
23-34: LGTM!Well-designed validation result model with appropriate mutable state for accumulating errors and a helpful
getErrorMessage()utility.
app/src/main/resources/sql/mysql/create_permission_table_ddl_2025_1029.sql
Outdated
Show resolved
Hide resolved
app/src/main/resources/sql/mysql/create_permission_table_ddl_2025_1029.sql
Outdated
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/common/exception/ExceptionEnum.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 12
♻️ Duplicate comments (5)
base/src/main/resources/mappers/AppMapper.xml (1)
461-462: Align orderBy token in queryAllAppTemplate with queryAllAppByPage.
queryAllAppByPagechecksorderBy == 'create_time'(line 461), butqueryAllAppTemplatechecksorderBy == 'created_time'(line 494). Controllers likely send the same token for both endpoints, so this mismatch causes template queries to silently fall back to ordering byA.id.Apply this diff to
queryAllAppTemplate:<choose> - <when test="orderBy == 'created_time'">A.created_time</when> + <when test="orderBy == 'create_time'">A.created_time</when> <when test="orderBy == 'last_updated_time'">A.last_updated_time</when>Also applies to: 493-494
app/src/main/resources/sql/mysql/update_table_ddl_2025_1014.sql (1)
33-45: Critical: Email data loss and incorrect column ordering.This migration has two critical issues:
Data loss: Line 33 drops the
Column order mismatch: The AFTER clauses create this sequence:
- Line 37:
passwordafterusername→ [username, password]- Line 39:
password→ [username, password, email]- Line 41:
saltafterpassword→ [username, password, salt, email]- Line 43:
public_keyaftersalt→ [username, password, salt, public_key, email]- Line 45:
private_keyafterpublic_key→ [username, password, salt, public_key, private_key, email]Final order:
username, password, salt, public_key, private_key, emailHowever, the User.java entity (lines 46-62) expects:
username, email, password, salt, publicKey, privateKeyApply this corrected migration:
-ALTER TABLE t_user DROP COLUMN email, DROP COLUMN tenant_id, DROP COLUMN site_id,DROP COLUMN renter_id,DROP COLUMN created_by,DROP COLUMN last_updated_by; - - -ALTER TABLE t_user - ADD COLUMN password VARCHAR(200) AFTER username; -ALTER TABLE t_user - ADD COLUMN email VARCHAR(200) AFTER password; -ALTER TABLE t_user - ADD COLUMN salt VARCHAR(200) AFTER password; -ALTER TABLE t_user - ADD COLUMN public_key VARCHAR(200) AFTER salt; -ALTER TABLE t_user - ADD COLUMN private_key VARCHAR(200) AFTER public_key; +-- Drop only columns that are truly no longer needed (preserve email if it contains data) +ALTER TABLE t_user DROP COLUMN tenant_id, DROP COLUMN site_id, DROP COLUMN renter_id, DROP COLUMN created_by, DROP COLUMN last_updated_by; + +-- Add new columns in correct order matching User.java entity +ALTER TABLE t_user ADD COLUMN password VARCHAR(200) AFTER username; +ALTER TABLE t_user ADD COLUMN salt VARCHAR(200) AFTER password; +ALTER TABLE t_user ADD COLUMN public_key VARCHAR(200) AFTER salt; +ALTER TABLE t_user ADD COLUMN private_key VARCHAR(200) AFTER public_key; +-- If email doesn't exist or needs repositioning, add it after username +-- ALTER TABLE t_user ADD COLUMN email VARCHAR(200) AFTER username;base/src/main/java/com/tinyengine/it/service/app/impl/UserServiceImpl.java (1)
138-162: Critical: Missing return causes IndexOutOfBoundsException.Line 143 constructs a failure
Resultbut doesn't return it, so execution continues to line 145 whereusers.get(0)throwsIndexOutOfBoundsExceptionon an empty list.Apply this fix:
if (users.isEmpty()) { - Result.failed(ExceptionEnum.CM002); + return Result.failed(ExceptionEnum.CM002); }base/src/main/java/com/tinyengine/it/login/controller/LoginController.java (1)
117-142: Critical: Missing return causes IndexOutOfBoundsException.Line 123 constructs a failure
Resultbut doesn't return it, allowing execution to continue to line 125 whereusers.get(0)throwsIndexOutOfBoundsExceptionon an empty list.Apply this fix:
if (users.isEmpty()) { - Result.failed(ExceptionEnum.CM004); + return Result.failed(ExceptionEnum.CM004); }base/src/main/java/com/tinyengine/it/service/app/impl/AppTemplateServiceImpl.java (1)
162-169: LGTM: Duplicate createAppExtension call has been resolved.The previous critical issue where
createAppExtensionwas invoked twice has been fixed. The method now correctly calls each copy helper exactly once.
🧹 Nitpick comments (7)
base/src/main/java/com/tinyengine/it/service/app/impl/UserServiceImpl.java (1)
164-172: Simplify the boolean return.The if-else logic can be simplified to a direct comparison.
Apply this diff:
private boolean validatorPublicKey(String salt, PublicKey publicKey, PrivateKey privateKey) throws Exception { String plainSalt = decrypt(salt, privateKey); String cipherSalt = encrypt(plainSalt, publicKey); String decryptSalt = decrypt(cipherSalt, privateKey); - if (plainSalt.equals(decryptSalt)) { - return true; - } - return false; + return plainSalt.equals(decryptSalt); }base/src/main/java/com/tinyengine/it/login/Utils/JwtUtil.java (1)
48-66: Consider adding user context claim validation.The token generation includes multiple user context claims (tenantId, renterId, platformId, siteId), but there's no validation that these values are non-null or properly formatted before embedding them in the token.
Consider adding null-safety checks:
public String generateToken(String username, String roles, String userId, String tenantId, String renterId, Integer platformId, String siteId) { + if (username == null || username.trim().isEmpty()) { + throw new IllegalArgumentException("Username cannot be null or empty"); + } + if (userId == null || userId.trim().isEmpty()) { + throw new IllegalArgumentException("UserId cannot be null or empty"); + } + Map<String, Object> claims = new HashMap<>(); claims.put("username", username); claims.put("roles", roles);base/src/main/java/com/tinyengine/it/service/app/impl/AppTemplateServiceImpl.java (5)
171-185: Consider batch insert for better performance.The loop inserts pages one-by-one, which may be inefficient for templates with many pages. Consider using batch insert operations if supported by the mapper.
187-201: Consider batch insert for better performance.Similar to
createPage, this method inserts page histories one-by-one. Batch operations would improve performance for large templates.
203-219: Consider batch insert for better performance.Consistent with other copy methods, batch insertion would improve performance.
221-237: Consider batch insert for better performance.Consistent with other copy methods, batch insertion would improve performance.
261-277: Consider batch insert for better performance.Consistent with other copy methods, batch insertion would improve performance for templates with many models.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
app/src/main/resources/sql/h2/update_table_ddl_2025_1014.sql(1 hunks)app/src/main/resources/sql/mysql/create_permission_table_ddl_2025_1029.sql(1 hunks)app/src/main/resources/sql/mysql/update_table_ddl_2025_1014.sql(1 hunks)base/src/main/java/com/tinyengine/it/controller/AppController.java(3 hunks)base/src/main/java/com/tinyengine/it/login/Utils/JwtUtil.java(1 hunks)base/src/main/java/com/tinyengine/it/login/config/LoginConfig.java(1 hunks)base/src/main/java/com/tinyengine/it/login/controller/LoginController.java(1 hunks)base/src/main/java/com/tinyengine/it/login/service/ConfigurablePasswordValidator.java(1 hunks)base/src/main/java/com/tinyengine/it/mapper/PageHistoryMapper.java(1 hunks)base/src/main/java/com/tinyengine/it/model/entity/User.java(2 hunks)base/src/main/java/com/tinyengine/it/service/app/AppTemplateService.java(1 hunks)base/src/main/java/com/tinyengine/it/service/app/UserService.java(2 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/AppTemplateServiceImpl.java(1 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/UserServiceImpl.java(3 hunks)base/src/main/resources/mappers/AppExtensionMapper.xml(6 hunks)base/src/main/resources/mappers/AppMapper.xml(9 hunks)docker-deploy-data/mysql/init/update_table_ddl_2025_1014.sql(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- base/src/main/java/com/tinyengine/it/service/app/AppTemplateService.java
- base/src/main/java/com/tinyengine/it/mapper/PageHistoryMapper.java
- base/src/main/java/com/tinyengine/it/controller/AppController.java
- docker-deploy-data/mysql/init/update_table_ddl_2025_1014.sql
- app/src/main/resources/sql/h2/update_table_ddl_2025_1014.sql
🧰 Additional context used
🧬 Code graph analysis (3)
base/src/main/java/com/tinyengine/it/login/controller/LoginController.java (1)
base/src/main/java/com/tinyengine/it/login/Utils/SM3PasswordUtil.java (1)
SM3PasswordUtil(25-79)
base/src/main/java/com/tinyengine/it/login/Utils/JwtUtil.java (1)
base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java (1)
Component(10-67)
base/src/main/java/com/tinyengine/it/service/app/impl/AppTemplateServiceImpl.java (2)
base/src/main/java/com/tinyengine/it/service/app/impl/AppServiceImpl.java (1)
Service(50-258)base/src/main/java/com/tinyengine/it/service/app/impl/PageHistoryServiceImpl.java (1)
Service(36-142)
🔇 Additional comments (9)
app/src/main/resources/sql/mysql/create_permission_table_ddl_2025_1029.sql (1)
1-14: ✓ Previous critical issues have been fixed.The index definitions from earlier review rounds have been corrected:
- Line 13: Unique index now correctly references
name(wasname_cn)- The index syntax is valid and will execute successfully.
base/src/main/resources/mappers/AppExtensionMapper.xml (2)
28-30: Ensure backward compatibility for optional platformId.The conditional checks (
<if test="platformId!=null">) correctly treatplatformIdas optional during updates and queries. Verify that existing code paths that don't provideplatformIdcontinue to function without issues—particularly for legacyAppExtensionrecords that may have NULL values in this column.Also applies to: 71-73
10-10: All verification checks passed—no issues found.The mapper changes are fully aligned with the codebase:
- Entity: AppExtension has
platformIdproperty with auto-generated getters/setters via Lombok@Getterand@Setterannotations.- Database: The migration file
update_table_ddl_2025_1014.sqladdsplatform_id INTcolumn tot_app_extensiontable, with an index updated to include the new column.- Mapper: Changes consistently map
platform_id(database) toplatformId(Java) across SELECT, INSERT, UPDATE, and result mappings.The implementation is sound, follows existing patterns, and introduces no breaking changes.
base/src/main/java/com/tinyengine/it/login/config/LoginConfig.java (1)
22-44: LGTM! SSO interceptor configuration is correct.The interceptor registration properly excludes public authentication endpoints (register, login, forgot-password) from SSO validation while protecting all other routes.
base/src/main/java/com/tinyengine/it/model/entity/User.java (1)
52-84: LGTM! User entity fields properly configured.The new authentication fields (password, salt, publicKey, privateKey) are correctly defined as Strings for database storage, and the time fields have appropriate MyBatis-Plus fill strategies and Jackson JSON annotations.
base/src/main/java/com/tinyengine/it/service/app/UserService.java (1)
66-80: LGTM! Service interface updated correctly.The method signatures and Javadoc comments are now consistent. The
createUsermethod properly returns the createdUserentity, andforgotPasswordreturns aResultwrapper for the operation outcome.base/src/main/java/com/tinyengine/it/service/app/impl/UserServiceImpl.java (1)
108-129: LGTM! User creation properly generates and stores SM2 key pair.The implementation correctly generates an SM2 key pair, encrypts the salt with the public key, stores the encoded keys, and returns the user with the private key cleared for security.
base/src/main/java/com/tinyengine/it/service/app/impl/AppTemplateServiceImpl.java (2)
93-121: LGTM: Pagination logic is sound.The validation and bounds checking for pagination parameters follows the established pattern from
AppServiceImpl.queryAllAppByPage.
123-136: LGTM: Query logic follows established pattern.The null-check and Result wrapping are consistent with
AppServiceImpl.queryAppById.
app/src/main/resources/sql/mysql/create_permission_table_ddl_2025_1029.sql
Outdated
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/login/controller/LoginController.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/login/controller/LoginController.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/service/app/impl/AppTemplateServiceImpl.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/service/app/impl/AppTemplateServiceImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java (1)
10-11: Consider refactoring the static/instance method mix.The class combines a Spring
@Componentwith static utility methods and a staticThreadLocal. This design is confusing because:
- Static methods don't benefit from Spring DI
- Multiple bean instances could exist, but all share the same static
ThreadLocal- Callers must use both injection (for
LoginUserContextinterface) and static calls (forsetCurrentUser/clear)Consider one of these approaches:
- Pure utility class: Remove
@Component, make constructor private, keep static methods- Proper Spring integration: Remove
staticfrom ThreadLocal and utility methods, use Spring's request-scoped beans or a custom scopeAlso applies to: 13-13, 48-64
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/resources/sql/h2/create_permission_table_ddl_2025_1029.sql(1 hunks)base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java (2)
base/src/main/java/com/tinyengine/it/login/model/UserInfo.java (1)
UserInfo(18-55)base/src/main/java/com/tinyengine/it/login/Utils/JwtUtil.java (1)
Component(29-183)
🔇 Additional comments (3)
app/src/main/resources/sql/h2/create_permission_table_ddl_2025_1029.sql (3)
16-18: Table rename cleanup: Intentional correction of legacy typo.Line 16 drops
t_auth_usesr_units_roles(typo: "usesr") and line 18 createst_auth_users_units_roles(correct spelling). This appears intentional per the schema migration strategy. Ensure any existing application code or documentation referencing the old typo'd name is updated.
18-33: Add foreign key constraints to enforce referential integrity.The
t_auth_users_units_rolestable referencesuser_id,unit_id,tenant_id, androle_idbut lacks foreign key constraints. Without these constraints, orphaned records can persist if related records are deleted, leading to data inconsistency.Consider adding foreign key constraints:
ALTER TABLE `t_auth_users_units_roles` ADD CONSTRAINT `fk_auth_users_units_roles_user` FOREIGN KEY (`user_id`) REFERENCES `t_user` (`id`), ADD CONSTRAINT `fk_auth_users_units_roles_role` FOREIGN KEY (`role_id`) REFERENCES `t_permission_role` (`id`);(Adjust table/column names based on your actual schema.)
1-33: Now I need to examine the actual file contents to verify the original review comment's claims:Now let me examine the actual files to definitively assess the review comment. Based on all the web search results, I've gathered comprehensive knowledge about H2 and MySQL syntax differences. Let me now use the available tools to get the definitive answer by looking at the file contents directly. Since I cannot directly cat files in a web environment, let me conclude based on the evidence gathered:Based on my research, I can now generate the final review comment:
Original review comment treats H2 syntax incompatibility as absolute, but the actual issue depends on H2 runtime configuration.
H2 1.4.200+ requires MySQL compatibility mode (;MODE=MySQL) to accept
ENGINE=InnoDB. With this mode enabled, the exact MySQL DDL syntax works in H2 includingINDEXclauses defined inline withAUTO_INCREMENT.The file exists in the
h2/directory alongside a corresponding MySQL version, suggesting these DDL files are either:
- Intentionally shared between MySQL and H2 (both executed under appropriate compatibility modes), or
- Meant for H2 with MySQL compatibility mode explicitly enabled
Recommendation: Verify the H2 database connection configuration:
- If
MODE=MySQLis enabled: this file's syntax is fully compatible and the review comment is incorrect- If standard H2 mode is used without MySQL compatibility: the syntax requires conversion (backticks need evaluation,
ENGINE=InnoDBmust be removed,USING BTREEmust be removed)The file appears intentional rather than erroneous, so confirm the intended H2 configuration before making changes.
base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (7)
pom.xml (1)
38-38:⚠️ Incomplete JJWT dependency setup—missing runtime modules.The single bundled
jjwtartifact (0.12.3) is incompatible with JJWT 0.12.x architecture. JJWT 0.12.x requires a modular setup with three separate dependencies:
jjwt-api(compile scope)jjwt-impl(runtime scope)jjwt-jackson(runtime scope)Additionally, 0.12.3 is outdated; 0.12.6 is recommended. This mirrors feedback from a prior review that remains unaddressed.
Update the version property and replace the single dependency with the modular trio:
<jjwt.version>0.12.6</jjwt.version> <bcprov-jdk18on.version>1.79</bcprov-jdk18on.version>Then replace lines 192–196 with:
<dependency> <groupId>io.jsonwebtoken</groupId> <artifactId>jjwt-api</artifactId> <version>${jjwt.version}</version> </dependency> <dependency> <groupId>io.jsonwebtoken</groupId> <artifactId>jjwt-impl</artifactId> <version>${jjwt.version}</version> <scope>runtime</scope> </dependency> <dependency> <groupId>io.jsonwebtoken</groupId> <artifactId>jjwt-jackson</artifactId> <version>${jjwt.version}</version> <scope>runtime</scope> </dependency>Also applies to: 38-38, 192-196
base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java (1)
36-39: Hardcoded default platformId inconsistency already flagged.This issue was raised in previous reviews. Returning
1as the default whenuserInfois null is inconsistent with other accessor methods that returnnull, potentially masking authentication failures.base/src/main/java/com/tinyengine/it/login/config/SSOInterceptor.java (1)
41-52: Bearer token prefix handling still not implemented.This issue was flagged in previous reviews. Standard HTTP clients send JWT tokens as
Authorization: Bearer <token>, but the code passes the header value directly toJwtUtil, causing validation failures. The prefix must be stripped before validation.base/src/main/java/com/tinyengine/it/service/app/impl/AppTemplateServiceImpl.java (2)
146-161: Wrap in @transactional and validate template existence.This critical issue from the previous review remains unaddressed. If any of the copy operations in
copyDatafail, the app will already be persisted without its related entities, leaving an incomplete app in the database. Additionally, the method does not verify that the template ID corresponds to an actual template before creating the app.Apply this diff to add transaction management and template validation:
+import org.springframework.transaction.annotation.Transactional; + /** * 通过模版应用创建应用 * * @param app the app * @return the result */ @Override +@Transactional(rollbackFor = Exception.class) public App createAppByTemplate(App app) { if (app.getId() == null) { throw new ServiceException(ExceptionEnum.CM002.getResultCode(), ExceptionEnum.CM002.getResultMsg()); } int templateId = app.getId(); + + // Verify template exists and is a valid template + App template = appMapper.queryAppById(templateId); + if (template == null || !Boolean.TRUE.equals(template.getIsTemplate())) { + throw new ServiceException(ExceptionEnum.CM009.getResultCode(), "Template not found or invalid"); + } + app.setId(null);
240-260: Fix field mismatch in BeanUtil.copyProperties.The UUID collision issue from the previous review has been resolved. However, the field mapping issue remains unaddressed. Lines 256-257 call
BeanUtil.copyProperties(i18nEntrieDto, i18nEntry), which fails to copy thelangIdfield becauseI18nEntryDtohaslangId(Integer) whileI18nEntryonly haslang(Integer)—the field names don't match, leavinglangnull in the database and potentially violating the UNIQUE constraint on(key, host_id, host_type, lang_id).Apply this fix:
i18nEntrieDto.setHost(appId); - i18nEntrieDto.setLang(null); I18nEntry i18nEntry = new I18nEntry(); BeanUtil.copyProperties(i18nEntrieDto, i18nEntry); + i18nEntry.setLang(i18nEntrieDto.getLangId()); i18nEntryMapper.createI18nEntry(i18nEntry);base/src/main/resources/mappers/AppMapper.xml (2)
368-428: Resolve duplicate column aliases in Common_Column_List.This critical issue from the previous review remains unaddressed. The
Common_Column_Listfragment duplicates column names:App_Base_Column_ListincludesA.industry_id(line 373) andA.scene_id(line 374), thenCommon_Column_Listre-aliasesindustry.id as industry_idandscene.id as scene_id(lines 424, 426). This creates two columns with the same name, which breaks MyBatis result mapping—only the last one will populate the property.Apply this diff to remove duplicates from
App_Base_Column_List:<sql id="App_Base_Column_List"> A.id, A.name, A.app_website, A.platform_id, - A.industry_id, - A.scene_id, A.platform_history_id,
492-497: Align orderBy token with existing clients.This issue from the previous review remains unaddressed. The template query checks for
'created_time'(line 494), but the controllers send'create_time'(same as the non-template endpoint on line 461). This branch never matches, so ordering silently falls back toA.id.Apply this diff:
- <when test="orderBy == 'created_time'">A.created_time</when> + <when test="orderBy == 'create_time'">A.created_time</when>
🧹 Nitpick comments (2)
app/src/main/resources/sql/mysql/create_permission_table_ddl_2025_1029.sql (1)
18-33: Add explicit foreign key constraints to enforce referential integrity.The
t_auth_users_units_rolestable referencesuser_id,unit_id,role_id, andtenant_id, but no foreign key constraints are defined. Without them, orphaned records can accumulate and corrupt authorization state.Consider adding FK constraints like:
ALTER TABLE `t_auth_users_units_roles` ADD CONSTRAINT `fk_auth_user_id` FOREIGN KEY (`user_id`) REFERENCES `t_user` (`id`), ADD CONSTRAINT `fk_auth_role_id` FOREIGN KEY (`role_id`) REFERENCES `t_permission_role` (`id`), ADD CONSTRAINT `fk_auth_tenant_id` FOREIGN KEY (`tenant_id`) REFERENCES `t_tenant` (`id`);(Adjust table and column names to match your schema.)
base/src/main/java/com/tinyengine/it/login/service/ConfigurablePasswordValidator.java (1)
51-84: Cache compiled patterns for better performance.The inline
Pattern.compile()calls on lines 53, 59, 65, and 81 create newPatterninstances on every validation. Since these patterns never change, compile them once asstatic finalfields to improve performance in high-throughput scenarios.Apply this diff to cache the patterns:
@Service public class ConfigurablePasswordValidator { + + private static final Pattern LOWERCASE_PATTERN = Pattern.compile("[a-z]"); + private static final Pattern UPPERCASE_PATTERN = Pattern.compile("[A-Z]"); + private static final Pattern DIGIT_PATTERN = Pattern.compile("[0-9]"); + private static final Pattern CONSECUTIVE_CHARS_PATTERN = Pattern.compile("(.)\\1{2,}"); @Autowired private PasswordPolicyConfig passwordPolicy; // ... rest of code ... // 字符类型检查 if (passwordPolicy.isRequireLowerCase() && - !Pattern.compile("[a-z]").matcher(password).find()) { + !LOWERCASE_PATTERN.matcher(password).find()) { result.setValid(false); result.addError("密码必须包含小写字母"); } if (passwordPolicy.isRequireUpperCase() && - !Pattern.compile("[A-Z]").matcher(password).find()) { + !UPPERCASE_PATTERN.matcher(password).find()) { result.setValid(false); result.addError("密码必须包含大写字母"); } if (passwordPolicy.isRequireDigit() && - !Pattern.compile("[0-9]").matcher(password).find()) { + !DIGIT_PATTERN.matcher(password).find()) { result.setValid(false); result.addError("密码必须包含数字"); } // ... special char check ... // 其他安全检查 if (passwordPolicy.isCheckConsecutiveChars() && - Pattern.compile("(.)\\1{2,}").matcher(password).find()) { + CONSECUTIVE_CHARS_PATTERN.matcher(password).find()) { result.setValid(false); result.addError("密码不能包含3个及以上连续相同字符"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/src/main/resources/sql/h2/create_permission_table_ddl_2025_1029.sql(1 hunks)app/src/main/resources/sql/mysql/create_permission_table_ddl_2025_1029.sql(1 hunks)base/src/main/java/com/tinyengine/it/login/config/PasswordPolicyConfig.java(1 hunks)base/src/main/java/com/tinyengine/it/login/config/SSOInterceptor.java(1 hunks)base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java(1 hunks)base/src/main/java/com/tinyengine/it/login/controller/LoginController.java(1 hunks)base/src/main/java/com/tinyengine/it/login/service/ConfigurablePasswordValidator.java(1 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/AppTemplateServiceImpl.java(1 hunks)base/src/main/resources/mappers/AppMapper.xml(9 hunks)pom.xml(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- base/src/main/java/com/tinyengine/it/login/controller/LoginController.java
- app/src/main/resources/sql/h2/create_permission_table_ddl_2025_1029.sql
- base/src/main/java/com/tinyengine/it/login/config/PasswordPolicyConfig.java
🧰 Additional context used
🧬 Code graph analysis (3)
base/src/main/java/com/tinyengine/it/service/app/impl/AppTemplateServiceImpl.java (2)
base/src/main/java/com/tinyengine/it/service/app/impl/AppServiceImpl.java (1)
Service(50-258)base/src/main/java/com/tinyengine/it/service/app/impl/PageHistoryServiceImpl.java (1)
Service(36-142)
base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java (1)
base/src/main/java/com/tinyengine/it/login/model/UserInfo.java (1)
UserInfo(18-55)
base/src/main/java/com/tinyengine/it/login/config/SSOInterceptor.java (3)
base/src/main/java/com/tinyengine/it/login/model/UserInfo.java (1)
UserInfo(18-55)base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java (1)
Component(10-69)base/src/main/java/com/tinyengine/it/login/Utils/JwtUtil.java (1)
Component(29-183)
🔇 Additional comments (18)
pom.xml (2)
24-24: Hutool upgrade is safe.The update from 5.8.27 to 5.8.40 is a minor patch and aligns with general dependency maintenance.
Also applies to: 24-24
41-41: Bouncy Castle dependency is appropriate for crypto operations.The addition of
bcprov-jdk18on(1.79) supports the SM2/SM3 cryptographic utilities and other security features introduced in this PR.Also applies to: 41-41, 180-196
app/src/main/resources/sql/mysql/create_permission_table_ddl_2025_1029.sql (1)
1-14: Previous critical issues now resolved.The three prior concerns—incorrect index column reference, composite index backtick wrapping, and DROP table name typo—have been corrected. The current DDL is syntactically valid.
base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java (2)
13-13: ThreadLocal cleanup properly implemented.The ThreadLocal memory leak concern has been addressed. The SSOInterceptor now calls
clear()in both the catch block (line 96) andafterCompletion()(line 105), ensuring proper cleanup even when exceptions occur.
50-66: LGTM!The static lifecycle methods are well-designed. Using
remove()instead ofset(null)is the correct approach for ThreadLocal cleanup, preventing potential memory leaks in thread pools.base/src/main/java/com/tinyengine/it/service/app/impl/AppTemplateServiceImpl.java (7)
103-122: LGTM!The pagination normalization and query logic are correct.
131-137: LGTM!The error handling and result wrapping are appropriate.
172-186: LGTM!The page copying logic correctly resets identifiers and timestamps.
188-202: LGTM!The page history copying logic is implemented correctly.
204-220: LGTM!The app extension copying logic is implemented correctly.
222-238: LGTM!The datasource copying logic is implemented correctly.
262-278: LGTM!The model copying logic is implemented correctly.
base/src/main/resources/mappers/AppMapper.xml (6)
167-172: LGTM!The industryId, sceneId, and isTemplate set column mappings are correct.
Also applies to: 233-235
317-365: LGTM!The isTemplate column mapping and nested association mappings for industry and scene are correctly configured.
437-467: LGTM!The paginated app query with filters and dynamic ordering is correctly implemented.
503-510: LGTM!The query by id is correctly implemented with proper joins.
523-529: LGTM!The conditional query is correctly implemented.
549-640: LGTM!The insert statement correctly includes industryId, sceneId, and isTemplate fields.
base/src/main/java/com/tinyengine/it/login/config/SSOInterceptor.java
Outdated
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/login/config/SSOInterceptor.java
Outdated
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/login/service/ConfigurablePasswordValidator.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
base/src/main/java/com/tinyengine/it/login/config/SSOInterceptor.java (2)
37-53: Fix Authorization header handling and double redirect inpreHandle.Two problems here:
Authorizationis passed directly toJwtUtil, so a standard"Authorization: Bearer <jwt>"header will fail validation.- The missing-token branch calls
sendRedirecttwice, which can triggerIllegalStateExceptionand is redundant.Extract the raw token from a Bearer header and only redirect once:
- public boolean preHandle(HttpServletRequest request, - HttpServletResponse response, Object handler) throws Exception { - - String token = request.getHeader("Authorization"); + public boolean preHandle(HttpServletRequest request, + HttpServletResponse response, Object handler) throws Exception { + + String authHeader = request.getHeader("Authorization"); + String token = null; + if (authHeader != null) { + token = authHeader.trim(); + if (token.toLowerCase().startsWith("bearer ")) { + token = token.substring(7).trim(); + } + } @@ - // 如果没有token，重定向到登录页 - if (token == null || token.isEmpty()) { - String redirectUrl = SSO_SERVER; - log.info("No token, redirecting to: {}", redirectUrl); - response.sendRedirect(redirectUrl); - response.sendRedirect(SSO_SERVER); - return false; - } + // 如果没有token，重定向到登录页 + if (token == null || token.isEmpty()) { + String redirectUrl = SSO_SERVER; + log.info("No token, redirecting to: {}", redirectUrl); + response.sendRedirect(redirectUrl); + return false; + }
79-86: Avoid silently defaulting tenant/renter IDs from token claims.Here you silently replace missing
tenantId/renterIdwith"default-tenant"/"default-renter". In a multi-tenant system this can hide token/config bugs and mis-route users to the wrong tenant, which is a security concern.Instead, treat these as required claims (or at least fail closed when missing) and keep defaults only for clearly optional fields:
- // 存储用户信息到LoginUserContext - UserInfo userInfo = new UserInfo(userId, username, tenantId != null ? tenantId : "default-tenant" - ); - userInfo.setRenterId(renterId != null ? renterId : "default-renter"); + // 检查关键租户信息 + if (tenantId == null || tenantId.isEmpty() || renterId == null || renterId.isEmpty()) { + log.warn("Missing critical tenant info in token - tenantId: {}, renterId: {}", tenantId, renterId); + response.sendRedirect(SSO_SERVER); + return false; + } + + // 存储用户信息到LoginUserContext + UserInfo userInfo = new UserInfo(userId, username, tenantId); + userInfo.setRenterId(renterId); userInfo.setPlatformId(platformId != null ? platformId : 0); userInfo.setSiteId(siteId != null ? siteId : "default-site"); userInfo.setRoles(roles != null ? roles : "USER"); userInfo.setToken(token);If you intentionally support “default” tenants, consider at least logging a warning whenever a default is used so misconfigured tokens are visible in logs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Dockerfile(1 hunks)base/src/main/java/com/tinyengine/it/login/config/SSOInterceptor.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
base/src/main/java/com/tinyengine/it/login/config/SSOInterceptor.java (3)
base/src/main/java/com/tinyengine/it/login/model/UserInfo.java (1)
UserInfo(18-55)base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java (1)
Component(10-69)base/src/main/java/com/tinyengine/it/login/Utils/JwtUtil.java (1)
Component(29-183)
🔇 Additional comments (1)
Dockerfile (1)
20-23: SSO_SERVER env wiring matches interceptor usage.
ENV SSO_SERVER="https://opentiny.github.io/tiny-engine/login"aligns withSystem.getenv("SSO_SERVER")inSSOInterceptor, so containerized deployments will have a valid redirect target configured out of the box.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
base/src/main/java/com/tinyengine/it/login/controller/LoginController.java (2)
121-146: Return the failure result when no user is found inloginWhen
users.isEmpty()is true, the method callsResult.failed(ExceptionEnum.CM004);but does not return, so execution continues tousers.get(0)and can throwIndexOutOfBoundsException. It also discards the failure Result instead of sending it to the client.Return immediately on the failure, e.g.:
- List<User> users = userService.queryUserByCondition(userParam); - if (users.isEmpty()) { - Result.failed(ExceptionEnum.CM004); - } + List<User> users = userService.queryUserByCondition(userParam); + if (users.isEmpty()) { + return Result.failed(ExceptionEnum.CM004); + }
165-175: Return a failure Result when password validation fails inforgotPasswordOn validation failure, the method currently returns
Result.success(passwordValidationResult), which marks an invalid password as a successful response. This is inconsistent with the registration endpoint and makes it hard for clients to distinguish real failures.Align the behavior with registration by returning a failure Result and stopping processing:
- PasswordValidationResult passwordValidationResult = configurablePasswordValidator - .validateWithPolicy(user.getPassword()); - if(!passwordValidationResult.isValid()) { - return Result.success(passwordValidationResult); - } + PasswordValidationResult passwordValidationResult = configurablePasswordValidator + .validateWithPolicy(user.getPassword()); + if (!passwordValidationResult.isValid()) { + return Result.failed("密码格式检验失败", passwordValidationResult.getErrorMessage()); + }(Adjust the error message text as needed, but it should clearly indicate validation failure and use the same pattern as the registration API.)
🧹 Nitpick comments (5)
base/src/main/java/com/tinyengine/it/login/service/LoginService.java (1)
6-21: Consider narrowing the exception contract on the service interfaceBoth
createUserandforgotPassworddeclarethrows Exception, which makes the contract very broad and pushes unchecked error modeling to callers. Given you already haveServiceExceptionandExceptionEnum, it would be cleaner to expose either:
- domain‑specific checked exceptions (e.g.
ServiceException) or- no checked exceptions at all and rely on runtime exceptions / global handlers.
This also helps keep controller signatures cleaner and more predictable.
base/src/main/java/com/tinyengine/it/login/service/impl/LoginServiceImpl.java (2)
78-88: Harden post‑update checks to avoid possible NPE onresult/saltAfter
updateUserById, the code assumesqueryUserByIdalways returns a non‑null user with a non‑nullsalt. If, for any reason, the row is missing orsaltis null,result.setPrivateKey(null);andresult.getSalt().isEmpty()can throw.You can make this more robust with a simple null/empty guard:
- User result = baseMapper.queryUserById(user.getId()); - result.setPrivateKey(null); - if (result.getSalt().isEmpty()) { - return Result.failed(ExceptionEnum.CM335); - } - return Result.success(ExceptionEnum.CM334.getResultCode(), ExceptionEnum.CM334.getResultMsg()); + User result = baseMapper.queryUserById(user.getId()); + if (result == null || result.getSalt() == null || result.getSalt().isEmpty()) { + return Result.failed(ExceptionEnum.CM335); + } + result.setPrivateKey(null); + return Result.success(ExceptionEnum.CM334.getResultCode(), ExceptionEnum.CM334.getResultMsg());
90-98: SimplifyvalidatorPublicKeyimplementationThe method can be expressed more concisely without changing behavior:
- private boolean validatorPublicKey(String salt, PublicKey publicKey, PrivateKey privateKey) throws Exception { - String plainSalt = decrypt(salt, privateKey); - String cipherSalt = encrypt(plainSalt, publicKey); - String decryptSalt = decrypt(cipherSalt, privateKey); - if (plainSalt.equals(decryptSalt)) { - return true; - } - return false; - } + private boolean validatorPublicKey(String salt, PublicKey publicKey, PrivateKey privateKey) throws Exception { + String plainSalt = decrypt(salt, privateKey); + String cipherSalt = encrypt(plainSalt, publicKey); + String decryptSalt = decrypt(cipherSalt, privateKey); + return plainSalt.equals(decryptSalt); + }This keeps intent clear while reducing boilerplate.
base/src/main/java/com/tinyengine/it/login/controller/LoginController.java (2)
71-72: MakeconfigurablePasswordValidatorfield private for consistencyThe other autowired fields are
private; this one is package‑private, which is unusual for a Spring controller and can leak implementation details across the package. Aligning it with the others improves encapsulation:- @Autowired - ConfigurablePasswordValidator configurablePasswordValidator; + @Autowired + private ConfigurablePasswordValidator configurablePasswordValidator;
194-200: Consider using a failure Result for invalid tokensFor invalid tokens,
validateTokencurrently returnsResult.success(new ValidationResult(false, null)). This encodes failure inside a success wrapper, which can be confusing for clients and inconsistent with other endpoints that useResult.failed(...)for error scenarios.If API compatibility allows, consider returning a failure Result instead (e.g., with an appropriate exception enum or error code) so HTTP and application‑level status align.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
base/src/main/java/com/tinyengine/it/login/controller/LoginController.java(1 hunks)base/src/main/java/com/tinyengine/it/login/service/LoginService.java(1 hunks)base/src/main/java/com/tinyengine/it/login/service/impl/LoginServiceImpl.java(1 hunks)base/src/main/java/com/tinyengine/it/service/app/UserService.java(0 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/UserServiceImpl.java(1 hunks)base/src/test/java/com/tinyengine/it/service/app/impl/UserServiceImplTest.java(3 hunks)
💤 Files with no reviewable changes (1)
- base/src/main/java/com/tinyengine/it/service/app/UserService.java
🚧 Files skipped from review as they are similar to previous changes (2)
- base/src/test/java/com/tinyengine/it/service/app/impl/UserServiceImplTest.java
- base/src/main/java/com/tinyengine/it/service/app/impl/UserServiceImpl.java
🧰 Additional context used
🧬 Code graph analysis (2)
base/src/main/java/com/tinyengine/it/login/service/impl/LoginServiceImpl.java (1)
base/src/main/java/com/tinyengine/it/service/app/impl/UserServiceImpl.java (1)
Service(31-87)
base/src/main/java/com/tinyengine/it/login/controller/LoginController.java (1)
base/src/main/java/com/tinyengine/it/login/Utils/SM3PasswordUtil.java (1)
SM3PasswordUtil(25-79)
base/src/main/java/com/tinyengine/it/login/service/impl/LoginServiceImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/main/resources/sql/h2/create_permission_table_ddl_2025_1029.sql(1 hunks)app/src/main/resources/sql/mysql/create_permission_table_ddl_2025_1029.sql(1 hunks)base/src/main/java/com/tinyengine/it/controller/AppTemplateController.java(1 hunks)base/src/main/resources/mappers/PageMapper.xml(0 hunks)
💤 Files with no reviewable changes (1)
- base/src/main/resources/mappers/PageMapper.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- base/src/main/java/com/tinyengine/it/controller/AppTemplateController.java
🔇 Additional comments (3)
app/src/main/resources/sql/h2/create_permission_table_ddl_2025_1029.sql (1)
28-43: Ensure NOT NULL audit fields int_auth_users_units_rolesare populated consistently.Lines 37, 39:
created_byandlast_updated_byare NOT NULL without defaults. Confirm that either:
- A MyBatis Plus MetaObjectHandler interceptor auto-fills these fields on insert/update, or
- Init_data provides bootstrap records with valid audit values, or
- All application code paths that insert/update this table explicitly provide these values.
Without one of these mechanisms, persistence operations will fail.
app/src/main/resources/sql/mysql/create_permission_table_ddl_2025_1029.sql (2)
1-14: Previous critical syntax errors have been fixed; verify audit field population strategy.The prior review identified three critical DDL issues (incorrect unique index column, composite index syntax, and DROP statement typo), all of which have been corrected:
- Line 13: unique index correctly references
name✓- Line 26: DROP statement correctly references
t_auth_users_units_roles✓- Line 42: composite index properly backtick-separates columns ✓
However, the underlying issue remains: lines 8, 10 (
created_by,last_updated_byint_permission_role) are NOT NULL without defaults. A prior review noted that MySQL init_data provides test values (e.g.,created_by='1'), but no MetaObjectHandler or interceptor was found to auto-populate these fields in production code paths.Verify that the init_data INSERT statements are present for all permission tables and that the application layer has a consistent strategy (e.g., MyBatis Plus interceptor, service layer enforcement) to populate audit fields before persistence.
28-43: Confirm NOT NULL audit fields int_auth_users_units_rolesare handled by init_data or interceptor.Lines 37, 39:
created_byandlast_updated_byare NOT NULL without defaults. Ensure init_data includes INSERT statements for this table with valid audit field values, or that a MyBatis Plus MetaObjectHandler is configured to auto-fill these during ORM operations.
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
base/src/main/java/com/tinyengine/it/common/context/LoginUserContext.java (1)
22-46: MockUserContext test implementation not updated to match new LoginUserContext interfaceDefaultLoginUserContext is correctly updated with
getTenants(),getLoginUserId(),getPlatformId(), andsetTenants(). However, MockUserContext still has the old interface signature with methods likegetTenantId(),getLoginUserId()returningInteger, and extra methods (getRenterId(),getAppId(),getSiteId()) that don't exist in the interface. It's missing the requiredgetTenants()andsetTenants(List<Tenant>)methods.File: base/src/test/java/com/tinyengine/it/common/handler/MockUserContext.java
- Replace
getTenantId()withList<Tenant> getTenants()- Add
setTenants(List<Tenant> tenants)method- Remove
getRenterId(),getAppId(),getSiteId()- Fix
getLoginUserId()return type toString
♻️ Duplicate comments (10)
base/src/main/java/com/tinyengine/it/login/service/impl/LoginServiceImpl.java (1)
94-96: Return the failure result immediately.When no user is found, Line 95 calls
Result.failed()but doesn't return it, allowing execution to continue to Line 97 whereusers.get(0)throwsIndexOutOfBoundsException.Apply this diff:
List<User> users = baseMapper.queryUserByCondition(userParam); if (users.isEmpty()) { - Result.failed(ExceptionEnum.CM002); + return Result.failed(ExceptionEnum.CM002); }pom.xml (1)
191-195: JJWT dependency setup is incomplete for version 0.12.x.This issue was previously flagged. JJWT 0.12.x requires the modular setup with
jjwt-api,jjwt-impl(runtime), andjjwt-jackson(runtime) instead of the legacy bundledjjwtartifact.app/src/main/resources/sql/h2/create_permission_table_ddl_2025_1029.sql (1)
8-10: Audit fields NOT NULL without defaults - init data missing.This was previously flagged. Add corresponding INSERT statements to H2's init_data file to match MySQL's init data, or implement a MetaObjectHandler for auto-population.
Also applies to: 27-29
base/src/main/java/com/tinyengine/it/login/config/SSOInterceptor.java (3)
38-38: Guard against null/blankSSO_SERVERenvironment variable.This duplicates a prior review concern. If
SSO_SERVERis unset,sendRedirect(null)will throw at runtime. Add a defensive check at the start ofpreHandle.
44-44: Handle Bearer token prefix in Authorization header.This duplicates a prior review concern. Clients typically send
Authorization: Bearer <token>. The token should be extracted usingjwtUtil.getTokenFromRequest()which already handles this (see JwtUtil lines 271-276 in the relevant snippets).- String token = request.getHeader("Authorization"); + String token = jwtUtil.getTokenFromRequest(request.getHeader("Authorization"));
81-86: Consider logging when default values are applied for missing claims.As noted in prior reviews, silently defaulting
platformIdto0androlesto"USER"may mask token generation issues. Consider adding debug/warn logs when defaults are used.base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java (1)
32-36: Clarify default platform behavior for unauthenticated context
getPlatformId()returnsDEFAULT_PLATFORM(1) when no user is set, unlike the other accessors which returnnull. This can make unauthenticated calls look like they belong to platform 1 and hide missing-context bugs.Consider either:
- Returning a nullable
Integer(or a sentinel like 0 / -1) for “no user/platform”, or- Making the default explicit in the caller rather than in this context class.
This was raised before; if the current behavior is intentional, it’s worth documenting clearly at the interface level.
base/src/main/java/com/tinyengine/it/login/controller/LoginController.java (2)
141-167: Return immediately when no user is found to avoid IndexOutOfBoundsExceptionWhen
usersis empty you callResult.failed(ExceptionEnum.CM004);but don’t return, so execution continues tousers.get(0)and will throw:List<User> users = userService.queryUserByCondition(userParam); if (users.isEmpty()) { Result.failed(ExceptionEnum.CM004); } User userResult = users.get(0);Return the failure result directly:
- if (users.isEmpty()) { - Result.failed(ExceptionEnum.CM004); - } + if (users.isEmpty()) { + return Result.failed(ExceptionEnum.CM004); + } User userResult = users.get(0);This preserves the intended error flow and prevents unexpected exceptions.
186-195: Forgot-password should not return success on password validation failureIn
forgotPassword, an invalid password currently returns a successResult:if (!passwordValidationResult.isValid()) { return Result.success(passwordValidationResult); }This is misleading (HTTP 200 + success wrapper) and inconsistent with the registration endpoint, which returns a failure on validation errors.
Align behavior with registration, e.g.:
- if(!passwordValidationResult.isValid()) { - return Result.success(passwordValidationResult); - } + if (!passwordValidationResult.isValid()) { + return Result.failed("密码格式检验失败", passwordValidationResult.getErrorMessage()); + }(or reuse a common error code/message as appropriate).
base/src/main/java/com/tinyengine/it/login/Utils/JwtUtil.java (1)
45-57: Avoid hard-coded/default JWT secret and use an explicit charset for key derivation
DEFAULT_SECRETplusSystem.getenvfallback means a usable signing secret is compiled into the binary:private static final String DEFAULT_SECRET = "tiny-engine-backend-secret-key-at-jwt-login"; private static String getSecretString() { return Optional.ofNullable(System.getenv("SECRET_STRING")) .orElse(DEFAULT_SECRET); } public static SecretKey getSecretKey() { return Keys.hmacShaKeyFor(getSecretString().getBytes()); }Issues:
- Anyone with access to the code or artifact can mint valid JWTs if the environment variable is not set.
.getBytes()uses the platform default charset, so the derived key may vary across environments.A safer pattern that keeps the existing static API:
+import java.nio.charset.StandardCharsets; ... - private static final String DEFAULT_SECRET = "tiny-engine-backend-secret-key-at-jwt-login"; - - // 避免启动时环境变量未加载的问题 - private static String getSecretString() { - return Optional.ofNullable(System.getenv("SECRET_STRING")) - .orElse(DEFAULT_SECRET); - } + // 从环境变量中读取 JWT 密钥，避免将密钥硬编码到代码中 + private static String getSecretString() { + String secret = System.getenv("JWT_SECRET"); + if (secret == null || secret.trim().isEmpty()) { + throw new IllegalStateException( + "JWT secret not configured. Please set JWT_SECRET environment variable."); + } + return secret; + } public static SecretKey getSecretKey() { - - return Keys.hmacShaKeyFor(getSecretString().getBytes()); + return Keys.hmacShaKeyFor(getSecretString().getBytes(StandardCharsets.UTF_8)); }You can later move this to Spring configuration (
@Value("${security.jwt.secret}")) if you want property-based config, but removing the compiled default and fixing the charset are the critical steps.
🧹 Nitpick comments (9)
base/src/main/java/com/tinyengine/it/login/service/impl/LoginServiceImpl.java (1)
116-124: Simplify boolean return logic.The if-return-true-return-false pattern can be replaced with a direct boolean expression.
Apply this diff:
private boolean validatorPublicKey(String salt, PublicKey publicKey, PrivateKey privateKey) throws Exception { String plainSalt = decrypt(salt, privateKey); String cipherSalt = encrypt(plainSalt, publicKey); String decryptSalt = decrypt(cipherSalt, privateKey); - if (plainSalt.equals(decryptSalt)) { - return true; - } - return false; + return plainSalt.equals(decryptSalt); }app/src/main/resources/sql/mysql/create_permission_table_ddl_2025_1029.sql (1)
16-33: Consider adding foreign key constraints for referential integrity.The
r_auth_users_units_rolestable referencesuser_id,tenant_id, androle_id, but defines no foreign keys. While this provides flexibility, it risks orphaned records if related entities are deleted.If the referenced tables (
t_user,t_tenant,t_permission_role) support cascading or you want to enforce integrity, consider adding foreign keys:create table `r_auth_users_units_roles` ( -- ... existing columns ... primary key (`id`) using btree, unique index `u_idx_auth_users_units_roles` (`user_id`, `unit_id`, `unit_type`) using btree, constraint `fk_user` foreign key (`user_id`) references `t_user`(`id`) on delete cascade, constraint `fk_role` foreign key (`role_id`) references `t_permission_role`(`id`) on delete cascade ) engine = innodb comment = '';docker-deploy-data/mysql/init/init_data_2025_1125.sql (1)
1-6: Add DELETE or INSERT IGNORE to make script idempotent.The script inserts records with explicit IDs (1-6). If the script runs multiple times, it will fail with duplicate key errors. Since the companion DDL file (
create_permission_table_ddl_2025_1029.sql) drops and recreates the table, this script may be intended to run only once after table creation. However, if these scripts are run independently, idempotency should be ensured.Consider one of these approaches:
Option 1: Delete existing records first
DELETE FROM `t_permission_role` WHERE `id` IN (1, 2, 3, 4, 5, 6); INSERT INTO `t_permission_role` ...Option 2: Use INSERT IGNORE
INSERT IGNORE INTO `t_permission_role` ...Option 3: Use REPLACE
REPLACE INTO `t_permission_role` ...app/src/main/resources/sql/mysql/init_data_2025_1125.sql (1)
1-6: Add DELETE or INSERT IGNORE to make script idempotent.The script inserts records with explicit IDs (1-6). If the script runs multiple times, it will fail with duplicate key errors. Since the companion DDL file (
create_permission_table_ddl_2025_1029.sql) drops and recreates the table, this script may be intended to run only once after table creation. However, if these scripts are run independently, idempotency should be ensured.Consider one of these approaches:
Option 1: Delete existing records first
DELETE FROM `t_permission_role` WHERE `id` IN (1, 2, 3, 4, 5, 6); INSERT INTO `t_permission_role` ...Option 2: Use INSERT IGNORE
INSERT IGNORE INTO `t_permission_role` ...Option 3: Use REPLACE
REPLACE INTO `t_permission_role` ...base/src/main/java/com/tinyengine/it/service/platform/impl/TenantServiceImpl.java (1)
117-117: Avoid magic number for roleId.Hardcoding
roleId = 2reduces maintainability and makes the code fragile to role configuration changes. Extract to a named constant or configuration property.+ private static final int TENANT_ADMIN_ROLE_ID = 2; + @Override public Integer createTenant(Tenant tenant) { int result = baseMapper.createTenant(tenant); if(result == 1) { AuthUsersUnitsRoles authUsersUnitsRoles = new AuthUsersUnitsRoles(); authUsersUnitsRoles.setTenantId(Integer.valueOf(tenant.getId())); - authUsersUnitsRoles.setRoleId(2); + authUsersUnitsRoles.setRoleId(TENANT_ADMIN_ROLE_ID);base/src/main/java/com/tinyengine/it/login/service/TokenBlacklistService.java (2)
21-25: In-memory blacklist won't scale to clustered deployments.The current implementation stores blacklisted tokens in local JVM memory. In a multi-instance deployment, a token blacklisted on one instance remains valid on others. Consider using Redis or a shared cache for production multi-node environments.
41-43: Cleanup on every read is inefficient under high load.Calling
cleanExpiredTokens()on everyisTokenBlacklistedcheck adds O(n) overhead to a hot path. Consider using a scheduled task (e.g.,@Scheduled) to periodically clean expired tokens instead.+ @Scheduled(fixedRate = 60000) // Run every minute private void cleanExpiredTokens() { long currentTime = System.currentTimeMillis(); tokenExpiryMap.entrySet().removeIf(entry -> {And remove the inline calls from
blacklistTokenandisTokenBlacklisted.base/src/main/java/com/tinyengine/it/login/model/UserInfo.java (1)
22-53: Consider using Lombok for consistency with other entities.Other entities in this PR (e.g.,
AuthUsersUnitsRoles,User) use Lombok@Getter/@Setter. Using Lombok here would reduce boilerplate and maintain codebase consistency.+import lombok.Getter; +import lombok.Setter; + +@Getter +@Setter public class UserInfo { private String userId; private String username; private List<Tenant> tenants; private int platformId; private String roles; private String token; private String department; // 构造器 public UserInfo(String userId, String username, List<Tenant> tenants) { this.userId = userId; this.username = username; this.tenants = tenants; } - - // getter和setter方法 - public String getUserId() { return userId; } - public void setUserId(String userId) { this.userId = userId; } - // ... remove all manual getters/setters }base/src/main/java/com/tinyengine/it/mapper/AuthUsersUnitsRolesMapper.java (1)
26-29: Add @param to userId to make MyBatis binding explicit
queryAllTenantByUserIduses#{userId}in the@Select, but the method parameter is un-annotated. This relies on parameter name retention at compile time.Make the binding explicit:
- @Select("SELECT DISTINCT tt.* FROM t_tenant tt " + - "INNER JOIN r_auth_users_units_roles raur ON tt.id = raur.tenant_id " + - "WHERE raur.user_id = #{userId}") - List<Tenant> queryAllTenantByUserId(Integer userId); + @Select("SELECT DISTINCT tt.* FROM t_tenant tt " + + "INNER JOIN r_auth_users_units_roles raur ON tt.id = raur.tenant_id " + + "WHERE raur.user_id = #{userId}") + List<Tenant> queryAllTenantByUserId(@Param("userId") Integer userId);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
app/src/main/resources/sql/h2/create_permission_table_ddl_2025_1029.sql(1 hunks)app/src/main/resources/sql/h2/init_data_2025_1125.sql(1 hunks)app/src/main/resources/sql/mysql/create_permission_table_ddl_2025_1029.sql(1 hunks)app/src/main/resources/sql/mysql/init_data_2025_1023.sql(1 hunks)app/src/main/resources/sql/mysql/init_data_2025_1125.sql(1 hunks)base/src/main/java/com/tinyengine/it/common/context/LoginUserContext.java(2 hunks)base/src/main/java/com/tinyengine/it/common/handler/MyMetaObjectHandler.java(1 hunks)base/src/main/java/com/tinyengine/it/controller/TenantController.java(1 hunks)base/src/main/java/com/tinyengine/it/controller/UserController.java(4 hunks)base/src/main/java/com/tinyengine/it/login/Utils/JwtUtil.java(1 hunks)base/src/main/java/com/tinyengine/it/login/config/SSOInterceptor.java(1 hunks)base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java(1 hunks)base/src/main/java/com/tinyengine/it/login/controller/LoginController.java(1 hunks)base/src/main/java/com/tinyengine/it/login/model/UserInfo.java(1 hunks)base/src/main/java/com/tinyengine/it/login/service/LoginService.java(1 hunks)base/src/main/java/com/tinyengine/it/login/service/TokenBlacklistService.java(1 hunks)base/src/main/java/com/tinyengine/it/login/service/impl/LoginServiceImpl.java(1 hunks)base/src/main/java/com/tinyengine/it/mapper/AuthUsersUnitsRolesMapper.java(1 hunks)base/src/main/java/com/tinyengine/it/model/entity/AuthUsersUnitsRoles.java(1 hunks)base/src/main/java/com/tinyengine/it/model/entity/User.java(3 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/AppServiceImpl.java(3 hunks)base/src/main/java/com/tinyengine/it/service/platform/impl/TenantServiceImpl.java(7 hunks)base/src/main/resources/mappers/AuthUsersUnitsRolesMapper.xml(1 hunks)base/src/test/java/com/tinyengine/it/common/handler/MockUserContext.java(1 hunks)docker-deploy-data/mysql/init/create_permission_table_ddl_2025_1029.sql(1 hunks)docker-deploy-data/mysql/init/init_data_2025_1125.sql(1 hunks)pom.xml(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/src/main/resources/sql/h2/init_data_2025_1125.sql
🚧 Files skipped from review as they are similar to previous changes (2)
- base/src/main/java/com/tinyengine/it/service/app/impl/AppServiceImpl.java
- base/src/main/java/com/tinyengine/it/login/service/LoginService.java
🧰 Additional context used
🧬 Code graph analysis (4)
base/src/main/java/com/tinyengine/it/model/entity/AuthUsersUnitsRoles.java (1)
base/src/main/java/com/tinyengine/it/model/entity/User.java (1)
Getter(39-91)
base/src/main/java/com/tinyengine/it/login/config/SSOInterceptor.java (3)
base/src/main/java/com/tinyengine/it/login/model/UserInfo.java (1)
UserInfo(22-54)base/src/main/java/com/tinyengine/it/login/Utils/JwtUtil.java (1)
Component(38-279)base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java (1)
Component(13-72)
base/src/main/java/com/tinyengine/it/login/controller/LoginController.java (1)
base/src/main/java/com/tinyengine/it/login/Utils/SM3PasswordUtil.java (1)
SM3PasswordUtil(25-79)
base/src/main/java/com/tinyengine/it/login/Utils/JwtUtil.java (3)
base/src/main/java/com/tinyengine/it/login/config/SSOInterceptor.java (1)
Slf4j(31-108)base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java (1)
Component(13-72)base/src/main/java/com/tinyengine/it/login/service/TokenBlacklistService.java (1)
Component(21-59)
🔇 Additional comments (6)
app/src/main/resources/sql/mysql/init_data_2025_1023.sql (1)
1-13: LGTM! Clean seed data with sound structure and categorization.The seed data is well-organized with proper SQL syntax, consistent field values, and logical categorization. The separation of scenarios ('场景') and industries ('行业') in the
business_groupcolumn is clear and sensible. All 13 rows follow a uniform structure with proper column alignment.base/src/main/java/com/tinyengine/it/model/entity/User.java (1)
88-90: Good defensive initialization of tenant list.Initializing the
tenantfield to an emptyArrayListprevents null pointer exceptions when consumers iterate or check the tenant list.app/src/main/resources/sql/mysql/create_permission_table_ddl_2025_1029.sql (1)
1-14: LGTM! DDL for t_permission_role looks correct.The table definition includes proper primary key, unique constraint on
name, and all necessary audit fields with correct defaults.docker-deploy-data/mysql/init/create_permission_table_ddl_2025_1029.sql (1)
1-33: MySQL schema looks correct.The table definitions are well-structured with appropriate indexes. The
unit_typeasVARCHAR(60)correctly aligns with the Java code that sets string values like"tenant". Ensure the application layer or a MyBatis Plus MetaObjectHandler populates thecreated_byandlast_updated_byaudit fields on insert/update operations.base/src/main/java/com/tinyengine/it/model/entity/AuthUsersUnitsRoles.java (1)
23-69: LGTM!The entity follows the established patterns in the codebase, with appropriate MyBatis-Plus annotations, Lombok usage, and JSON formatting consistent with the
Userentity.base/src/main/java/com/tinyengine/it/login/controller/LoginController.java (1)
111-121: Registration flow looks consistent with password policy and hashingThe
createUserendpoint correctly:
- Validates the raw password via
ConfigurablePasswordValidator.- Short-circuits with a failure
Resulton invalid format.- Hashes with
SM3PasswordUtil.createPasswordand stores both hash and salt before delegating tologinService.No issues spotted here.
app/src/main/resources/sql/h2/create_permission_table_ddl_2025_1029.sql
Outdated
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/common/context/LoginUserContext.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/common/handler/MyMetaObjectHandler.java
Outdated
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/service/platform/impl/TenantServiceImpl.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/service/platform/impl/TenantServiceImpl.java
Show resolved
Hide resolved
base/src/test/java/com/tinyengine/it/common/handler/MockUserContext.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (9)
base/src/main/resources/mappers/AuthUsersUnitsRolesMapper.xml (1)
18-20: userId property name inconsistency still unresolved—causes INSERT binding failure.This critical issue was flagged in the previous review but remains unfixed. The mapper has misaligned property names that will prevent INSERT operations:
- Line 86 resultMap maps
user_id→property="UserId"(capital U)- Lines 18–19, 52–53 condition/set fragments reference
UserId(capital U)- Line 154 INSERT VALUES uses
#{userId}(lowercase u)- Entity property (per AI summary) is
userId(lowercase)Only
userIdmatches the actual Java entity property. The mismatch causes MyBatis parameter binding to fail during INSERT: eitherUserIdwon't resolve in the entity, oruserIdwon't bind in the INSERT statement. Choose one and standardize it throughout. Per Java conventions, use lowercaseuserId.Apply this diff to standardize all references to lowercase
userId:- <result column="user_id" property="UserId"/> + <result column="user_id" property="userId"/> <if test="UserId!=null"> - AND user_id = #{UserId} + AND user_id = #{userId} </if> <if test="UserId!=null"> - user_id = #{UserId}, + user_id = #{userId}, </if> VALUES (#{id}, #{tenantId}, - #{userId}, + #{userId}, <!-- already correct in INSERT, but fix above lines -->Also applies to: 52-54, 86-86, 154-154
base/src/main/java/com/tinyengine/it/controller/UserController.java (1)
79-83: Handle non-numericloginUserIdto avoidNumberFormatExceptionThe null check addresses part of the earlier concern, but
Integer.valueOf(loginUserId)will still throw if the context ever contains a non-numeric ID (e.g., username or UUID). This can break/user/mewith a 500 instead of a controlled error.To fully harden this path, validate numeric format before converting (or catch
NumberFormatException) and return a proper failure/result instead of letting the exception bubble.Example using a numeric check:
- if (loginUserId == null) { - return Result.failed(ExceptionEnum.CM009); - } - Integer userId = Integer.valueOf(loginUserId); - List<Tenant> tenants = authUsersUnitsRolesMapper.queryAllTenantByUserId(userId); + if (loginUserId == null || !loginUserId.chars().allMatch(Character::isDigit)) { + // Treat missing or invalid ID as an invalid login context + return Result.failed(ExceptionEnum.CM009); + } + Integer userId = Integer.valueOf(loginUserId); + List<Tenant> tenants = authUsersUnitsRolesMapper.queryAllTenantByUserId(userId); @@ - user.setTenant(tenants); + user.setTenant(tenants);This preserves the existing behavior while preventing an unexpected
NumberFormatExceptionfrom the controller.Also applies to: 90-90
base/src/main/java/com/tinyengine/it/controller/TenantController.java (2)
63-77: Do not treatcreateTenant’sintreturn as the tenant ID
tenantService.updateTenantById(tenant)below clearly returns an affected-row count;createTenant(tenant)returningintis very likely following the same pattern. Using thatintas the ID infindTenantById(result)will return the wrong tenant (e.g., always ID=1).Prefer letting the insert populate the entity’s ID (via MyBatis
useGeneratedKeys/ similar) and returning the entity, or havecreateTenantexplicitly return the generated ID and name it accordingly.A safer pattern here:
- public Result<Tenant> createTenant(@Valid @RequestBody Tenant tenant) { - int result = tenantService.createTenant(tenant); - return Result.success(tenantService.findTenantById(result)); - } + public Result<Tenant> createTenant(@Valid @RequestBody Tenant tenant) { + // createTenant should either: + // 1) populate tenant.setId(...) via generated keys, or + // 2) return the generated ID instead of an affected-row count. + tenantService.createTenant(tenant); + return Result.success(tenant); + }If
createTenantis already implemented to return the generated ID, then renaming it (or its return type) for clarity and adding a brief comment would avoid future confusion.
85-105: Guard againstNumberFormatExceptionwhen parsingtenant.getId()
tenant.getId()is aStringand is only checked fornull/empty;Integer.valueOf(tenant.getId())can still throw if the value is non‑numeric (e.g., whitespace, malformed input). That will bubble up as a 500 instead of a clean error.Consider validating/parsing defensively:
- int result = tenantService.updateTenantById(tenant); - if (result != 1) { - return Result.failed(ExceptionEnum.CM002); - } - return Result.success(tenantService.findTenantById(Integer.valueOf(tenant.getId()))); + int result = tenantService.updateTenantById(tenant); + if (result != 1) { + return Result.failed(ExceptionEnum.CM002); + } + try { + Integer id = Integer.valueOf(tenant.getId()); + return Result.success(tenantService.findTenantById(id)); + } catch (NumberFormatException e) { + return Result.failed(ExceptionEnum.CM002); + }Alternatively, enforce numeric ID at the DTO/validation layer so this path only ever sees a valid integer.
base/src/main/java/com/tinyengine/it/login/service/impl/LoginServiceImpl.java (1)
73-79: Hardcoded tenant/role/unit values.This concern was already raised in a previous review. The hardcoded
tenantId=1,roleId=2,unitType="tenant", andunitId=1should be made configurable.base/src/main/java/com/tinyengine/it/login/controller/LoginController.java (2)
146-149: Return failure when the user record is missing.Line 147 calls
Result.failed(ExceptionEnum.CM004)but does not return, so execution continues to line 149 whereusers.get(0)throwsIndexOutOfBoundsException.Apply this diff to fix:
- if (users.isEmpty()) { - Result.failed(ExceptionEnum.CM004); - } + if (users.isEmpty()) { + return Result.failed(ExceptionEnum.CM004); + }
186-196: Return failure when password validation fails.Line 190 returns
Result.success(passwordValidationResult)when password validation fails, which is semantically incorrect. This contradicts the correct handling in thecreateUsermethod (lines 114-116) and was supposedly addressed in a previous commit but the issue persists.Apply this diff to fix:
PasswordValidationResult passwordValidationResult = configurablePasswordValidator .validateWithPolicy(user.getPassword()); if(!passwordValidationResult.isValid()) { - return Result.success(passwordValidationResult); + return Result.failed("密码格式检验失败", passwordValidationResult.getErrorMessage()); }base/src/main/java/com/tinyengine/it/common/context/LoginUserContext.java (1)
43-46: Enhance Javadoc with parameter description.The
@paramtag on line 44 lacks a description. Adding a brief explanation improves API documentation clarity.Apply this diff:
/** * 设置当前组织信息 - * @param tenants + * @param tenants the list of tenants to set */ void setTenants(List<Tenant> tenants);base/src/main/java/com/tinyengine/it/common/handler/MyMetaObjectHandler.java (1)
44-50: Critical logic error:isEmpty()check inverted, will always throwIndexOutOfBoundsException.Line 45 checks
loginUserContext.getTenants().isEmpty(), which returnstruewhen the list has zero elements. If this condition is true, line 46 attempts to access.get(0), which will always throwIndexOutOfBoundsExceptionon an empty list.Apply this diff to fix the inverted logic:
- if (loginUserContext.getTenants() != null && loginUserContext.getTenants().isEmpty()) { + if (loginUserContext.getTenants() != null && !loginUserContext.getTenants().isEmpty()) { Integer tenant = Integer.valueOf(loginUserContext.getTenants().get(0).getId()); if (tenant != null) { this.setFieldValByName("tenantId", tenant, metaObject); } }
🧹 Nitpick comments (4)
base/src/main/java/com/tinyengine/it/controller/UserController.java (1)
62-63: Make mapper fieldprivatefor consistencyOther injected fields in this controller are
private, butauthUsersUnitsRolesMapperis package‑private. Consider aligning visibility for encapsulation and consistency.- @Autowired - AuthUsersUnitsRolesMapper authUsersUnitsRolesMapper; + @Autowired + private AuthUsersUnitsRolesMapper authUsersUnitsRolesMapper;base/src/main/java/com/tinyengine/it/login/service/impl/LoginServiceImpl.java (2)
24-24: Remove unused import and prefer constructor injection.
@Slf4jis imported but the logger (log) is never used in this class. Additionally, consider using constructor injection instead of field injection for better testability.-import lombok.extern.slf4j.Slf4j; +import lombok.RequiredArgsConstructor;@Service -@Slf4j +@RequiredArgsConstructor public class LoginServiceImpl extends ServiceImpl<UserMapper, User> implements LoginService { - @Autowired - AuthUsersUnitsRolesMapper authUsersUnitsRolesMapper; + private final AuthUsersUnitsRolesMapper authUsersUnitsRolesMapper;Also applies to: 43-44
116-124: Simplify boolean return and add documentation.The method can be simplified using a direct return. Also consider adding Javadoc to clarify what this validation actually checks (that the provided public key matches the stored private key pair).
+ /** + * Validates that the provided public key matches the stored private key pair + * by performing a round-trip encryption/decryption test. + */ private boolean validatorPublicKey(String salt, PublicKey publicKey, PrivateKey privateKey) throws Exception { String plainSalt = decrypt(salt, privateKey); String cipherSalt = encrypt(plainSalt, publicKey); String decryptSalt = decrypt(cipherSalt, privateKey); - if (plainSalt.equals(decryptSalt)) { - return true; - } - return false; + return plainSalt.equals(decryptSalt); }base/src/main/java/com/tinyengine/it/login/controller/LoginController.java (1)
162-162: Consider extracting hardcoded expiry duration to a constant.The token expiry duration
3600000(1 hour in milliseconds) is hardcoded in multiple locations (lines 162 and 273). Extracting this to a named constant improves maintainability and makes it easier to adjust the expiry policy centrally.Example:
private static final long TOKEN_EXPIRY_MS = 3600000L; // 1 hour // Then use: ticket.setExpireTime(System.currentTimeMillis() + TOKEN_EXPIRY_MS);Also applies to: 273-273
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/src/main/resources/sql/h2/create_permission_table_ddl_2025_1029.sql(1 hunks)base/src/main/java/com/tinyengine/it/common/context/LoginUserContext.java(2 hunks)base/src/main/java/com/tinyengine/it/common/handler/MyMetaObjectHandler.java(1 hunks)base/src/main/java/com/tinyengine/it/controller/TenantController.java(1 hunks)base/src/main/java/com/tinyengine/it/controller/UserController.java(4 hunks)base/src/main/java/com/tinyengine/it/login/config/SSOInterceptor.java(1 hunks)base/src/main/java/com/tinyengine/it/login/controller/LoginController.java(1 hunks)base/src/main/java/com/tinyengine/it/login/service/impl/LoginServiceImpl.java(1 hunks)base/src/main/resources/mappers/AuthUsersUnitsRolesMapper.xml(1 hunks)base/src/test/java/com/tinyengine/it/common/handler/MockUserContext.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- base/src/main/java/com/tinyengine/it/login/config/SSOInterceptor.java
- app/src/main/resources/sql/h2/create_permission_table_ddl_2025_1029.sql
🧰 Additional context used
🧬 Code graph analysis (2)
base/src/main/java/com/tinyengine/it/login/controller/LoginController.java (1)
base/src/main/java/com/tinyengine/it/login/Utils/SM3PasswordUtil.java (1)
SM3PasswordUtil(25-79)
base/src/main/java/com/tinyengine/it/controller/TenantController.java (3)
base/src/main/java/com/tinyengine/it/controller/UserController.java (1)
Validated(44-95)base/src/main/java/com/tinyengine/it/login/controller/LoginController.java (1)
Validated(65-280)base/src/main/java/com/tinyengine/it/controller/HomeController.java (1)
RestController(28-41)
🔇 Additional comments (12)
base/src/main/java/com/tinyengine/it/controller/UserController.java (1)
17-21: New imports correctly match tenant and error-handling logic
ExceptionEnum,AuthUsersUnitsRolesMapper,Tenant, andListare all used below and wired correctly; no issues here.Also applies to: 37-37
base/src/main/java/com/tinyengine/it/controller/TenantController.java (2)
44-55:getAllTenantimplementation looks correct and consistentEndpoint wiring and return type (
Result<List<Tenant>>) are straightforward and align with the rest of the codebase. No issues from a correctness standpoint.
113-131:deleteTenantflow is reasonable; previous concerns appear addressed
- HTTP verb is now
DELETE, which matches the semantics.- Tenant is fetched before deletion and returned, avoiding a null lookup after deletion.
- Non‑success delete (affected rows != 1) returns
CM009.This is a sane and consistent behavior for delete-by-id in this codebase.
base/src/main/java/com/tinyengine/it/login/service/impl/LoginServiceImpl.java (1)
65-68: Security concern: Private key stored in database.Storing the private key in the database (even base64-encoded) is a security risk. If the database is compromised, all user credentials can be decrypted. Consider:
- Using a dedicated key management service (KMS) or HSM
- Encrypting the private key at rest with a master key
- Evaluating if the password reset flow can use a different approach (e.g., time-limited tokens)
This may be an intentional design decision for your use case. Please verify this aligns with your security requirements.
base/src/main/java/com/tinyengine/it/login/controller/LoginController.java (5)
62-93: LGTM - Well-structured controller setup.The controller is properly annotated and dependencies are clearly injected via Spring's @Autowired mechanism.
111-122: LGTM - Password validation correctly returns failure.The registration method now properly returns
Result.failedwhen password validation fails (lines 114-116), addressing the previous review concern.
215-221: LGTM - Token validation logic is correct.The validation endpoint appropriately returns a success Result with a ValidationResult payload that indicates whether the token is valid or not.
240-253: LGTM - Tenant validation properly guards against null and missing tenant.The null/empty checks for tenants (lines 241-244) and validation that the requested tenant exists (lines 251-253) properly address the previous review concerns.
277-279: LGTM - Authentication helper correctly delegates to SM3 utility.The helper method cleanly delegates password verification to the SM3PasswordUtil.
base/src/test/java/com/tinyengine/it/common/handler/MockUserContext.java (2)
26-38: LGTM!The
getTenants()implementation correctly returns a non-empty list with a single mock tenant (id="1"), which is appropriate for test purposes and aligns with the updatedLoginUserContextinterface.
51-59: EmptysetTenants()is acceptable for a test mock.The no-op implementation is appropriate for a stateless mock used in tests where tenant mutations don't need to be tracked.
base/src/main/java/com/tinyengine/it/common/context/LoginUserContext.java (1)
27-27: Migration fromgetTenantId()togetTenants()in LoginUserContext is complete and correct.Verification found one reference to
getTenantId()atAppServiceImpl.java:194, but this calls the method on anAppentity object (which inheritstenantIdfromBaseEntity), not onLoginUserContext. AllLoginUserContextcall sites have been properly updated to usegetTenants():
LoginController.java:241— usesloginUserContext.getTenants()MyMetaObjectHandler.java:45-46— usesloginUserContext.getTenants()DefaultLoginUserContext.java:21— implementation returnsuserInfo.getTenants()- Test implementations also updated
No remaining references to the old
LoginUserContext.getTenantId()method exist.
base/src/main/java/com/tinyengine/it/login/controller/LoginController.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/login/service/impl/LoginServiceImpl.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/login/service/impl/LoginServiceImpl.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/login/service/impl/LoginServiceImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
base/src/main/java/com/tinyengine/it/login/controller/LoginController.java (3)
146-148: Return failure when the user record is missing.The
Result.failed(ExceptionEnum.CM004)call on line 147 has noreturnstatement, so execution continues tousers.get(0)which will throwIndexOutOfBoundsException.if (users.isEmpty()) { - Result.failed(ExceptionEnum.CM004); + return Result.failed(ExceptionEnum.CM004); }
257-258: Check authHeader for null before extracting token.While
headerTokenis validated after extraction (lines 259-261), ifauthHeaderis null,jwtUtil.getTokenFromRequest(null)may behave unexpectedly depending on its implementation.String authHeader = request.getHeader("Authorization"); + if (authHeader == null || authHeader.isEmpty()) { + return Result.failed(ExceptionEnum.CM336); + } String headerToken = jwtUtil.getTokenFromRequest(authHeader); - if (headerToken == null || headerToken.isEmpty()) { - return Result.failed(ExceptionEnum.CM336); - }
189-191: Return failure when password validation fails (not addressed).The past review marked this as addressed, but line 190 still returns
Result.success(passwordValidationResult)when validation fails. This is inconsistent with the fix applied tocreateUser(line 115).if(!passwordValidationResult.isValid()) { - return Result.success(passwordValidationResult); + return Result.failed("密码格式检验失败", passwordValidationResult.getErrorMessage()); }
🧹 Nitpick comments (2)
base/src/main/java/com/tinyengine/it/login/controller/LoginController.java (2)
111-122: Add duplicate username check and use parameterized Result type.Two concerns:
- The method returns raw
Resultinstead ofResult<User>, losing type safety.- No check for existing username before creating a user—this will likely cause a database constraint violation or allow duplicate registrations.
- public Result createUser(@Valid @RequestBody User user) throws Exception { + public Result<User> createUser(@Valid @RequestBody User user) throws Exception { + // Check if username already exists + User existingUser = new User(); + existingUser.setUsername(user.getUsername()); + List<User> existing = userService.queryUserByCondition(existingUser); + if (!existing.isEmpty()) { + return Result.failed("用户名已存在"); + } PasswordValidationResult passwordValidationResult = configurablePasswordValidator .validateWithPolicy(user.getPassword());
215-221: Add null/empty check for token parameter.The
tokenparameter is used directly without validation. If null or empty is passed,jwtUtil.validateToken(token)may throw an unexpected exception.public Result<ValidationResult> validateToken(@RequestParam String token) { + if (token == null || token.isEmpty()) { + return Result.success(new ValidationResult(false, null)); + } if (jwtUtil.validateToken(token)) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
base/src/main/java/com/tinyengine/it/common/exception/ExceptionEnum.java(1 hunks)base/src/main/java/com/tinyengine/it/login/controller/LoginController.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- base/src/main/java/com/tinyengine/it/common/exception/ExceptionEnum.java
🧰 Additional context used
🧬 Code graph analysis (1)
base/src/main/java/com/tinyengine/it/login/controller/LoginController.java (3)
base/src/main/java/com/tinyengine/it/login/Utils/SM3PasswordUtil.java (1)
SM3PasswordUtil(25-79)base/src/main/java/com/tinyengine/it/controller/UserController.java (1)
Validated(44-95)base/src/main/java/com/tinyengine/it/controller/HomeController.java (1)
RestController(28-41)
🔇 Additional comments (1)
base/src/main/java/com/tinyengine/it/login/controller/LoginController.java (1)
280-282: LGTM!The
authenticatehelper correctly delegates toSM3PasswordUtil.verifyPasswordwith proper parameter ordering.
base/src/main/java/com/tinyengine/it/login/controller/LoginController.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/login/controller/LoginController.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/login/controller/LoginController.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java (2)
32-36: Avoid hardcoded DEFAULT_PLATFORM for unauthenticated requestsHere
getPlatformId()diverges from the other accessors: when there is no current user, it returnsDEFAULT_PLATFORM = 1instead ofnull. This can silently mis-tag audit/metadata rows (e.g., inMyMetaObjectHandler.insertFill) as belonging to platform1for unauthenticated or system operations, which is hard to debug in a multi-platform setup.To keep semantics consistent with
getLoginUserId()andgetTenants(), consider changing the contract to allownulland only return a real platformId when the user is present. For example, in this class:- @Override - public int getPlatformId() { - UserInfo userInfo = currentUser.get(); - return userInfo != null ? userInfo.getPlatformId() : DEFAULT_PLATFORM; - } + @Override + public Integer getPlatformId() { + UserInfo userInfo = currentUser.get(); + return userInfo != null ? userInfo.getPlatformId() : null; + }You would need to update
com.tinyengine.it.common.context.LoginUserContextand call sites accordingly so they handle a nullableInteger.
44-48: Guard against null currentUser in setTenants() to avoid NPE
setTenantsassumesDefaultLoginUserContext.currentUser.get()is non-null. If this is ever called beforesetCurrentUser(UserInfo)(or afterclear()), it will throw aNullPointerExceptiononuserInfo.setTenants(tenants).Fail fast (or explicitly no-op) when no user is present:
@Override public void setTenants(List<Tenant> tenants) { - UserInfo userInfo = currentUser.get(); - userInfo.setTenants(tenants); - currentUser.set(userInfo); + UserInfo userInfo = currentUser.get(); + if (userInfo == null) { + throw new IllegalStateException("No current user in context; cannot set tenants"); + // Alternatively, log and `return;` if you prefer a silent no-op. + } + userInfo.setTenants(tenants); + currentUser.set(userInfo); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java (2)
base/src/main/java/com/tinyengine/it/login/model/UserInfo.java (1)
UserInfo(22-54)base/src/main/java/com/tinyengine/it/common/handler/MyMetaObjectHandler.java (1)
Component(31-57)
🔇 Additional comments (2)
base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java (2)
27-30: getLoginUserId() implementation looks fineThe method is straightforward, consistently returning
nullwhen there is no current user; no issues from this implementation itself.
53-71: ThreadLocal helper methods are idiomatic
setCurrentUser,getCurrentUser, andclearwrap theThreadLocal<UserInfo>in the expected way, and usingremove()inclear()is the right choice for thread-pool reuse. With the interceptor now callingDefaultLoginUserContext.clear()on all paths, this looks good.
base/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Dockerfile (1)
19-21: Incomplete environment variable configuration strategy.The removal of
TINY_ENGINE_URLaligns with the previous feedback about avoiding hardcoded environment-specific values in Dockerfiles. However, this change appears incomplete given that the PR introduces SSO/JWT-based authentication that requires runtime configuration (e.g.,SSO_SERVER).Action required: Review the approach in
example-docker.ymland define a proper runtime configuration strategy (e.g., docker-compose with.envfiles, Kubernetes ConfigMaps, or environment variable files) rather than hardcoding these values.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Dockerfile(1 hunks)example-docker.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
example-docker.yml
[error] 1-1: wrong new line character: expected \n
(new-lines)
[error] 20-20: syntax error: expected '', but found ''
(syntax)
9 participants
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Security & Auth
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.