test: add Jwt, SM2Encryption and SM3Password util test.#310
Merged
chilingling merged 2 commits intoopentiny:developfrom Apr 30, 2026
Merged
test: add Jwt, SM2Encryption and SM3Password util test.#310chilingling merged 2 commits intoopentiny:developfrom
chilingling merged 2 commits intoopentiny:developfrom
Conversation
WalkthroughDefensive null-argument validation is added to JWT token generation/validation and SM3 password hashing utilities. Comprehensive test suites are introduced for JWT utilities, SM2 encryption, and SM3 password operations, covering token lifecycle, cryptographic operations, edge cases, and failure scenarios. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
chilingling
previously approved these changes
Apr 30, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
base/src/test/java/com/tinyengine/it/login/utils/SM2EncryptionUtilTest.java (1)
228-239: ⚡ Quick winThese cases don't actually validate raw binary or “non-UTF8” support.
Both tests first coerce bytes into a Java
Stringwith ISO-8859-1, so the utility is still just encrypting text. If byte fidelity matters, add byte-array based APIs/tests; otherwise rename these cases to reflect that they are ISO-8859-1 text fixtures.Also applies to: 257-268
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@base/src/test/java/com/tinyengine/it/login/utils/SM2EncryptionUtilTest.java` around lines 228 - 239, The test encryptAndDecryptWithNonUTF8Characters (and the similar case at lines 257-268) mischaracterizes ISO-8859-1 fixtures as "non-UTF8" raw-binary tests: they convert raw bytes into a Java String before calling SM2EncryptionUtil.encrypt/decrypt, so the utility is still operating on text. Fix by either (A) adding byte-array based APIs and tests that call new SM2EncryptionUtil.encrypt(byte[]) / decrypt(byte[]) and assert byte-array fidelity, or (B) if byte-level support is not needed, rename the test methods and update their descriptions to indicate they are ISO-8859-1 text fixtures (e.g., encryptAndDecryptWithIso88591Characters) and keep using SM2EncryptionUtil.encrypt/decrypt as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@base/src/main/java/com/tinyengine/it/login/utils/JwtUtil.java`:
- Around line 133-138: The username null-check belongs in the shared
implementation: move the guard into the public generateToken(String username,
String roles, String userId, Object tenants, Integer platformId) so it validates
username before any Claims are built (throw IllegalArgumentException("Username
must not be null") if null); remove the duplicate check from the List<Tenant>
overload generateToken(String, String, String, List<Tenant>, Integer) so both
call paths use the same validation and behavior.
In `@base/src/main/java/com/tinyengine/it/login/utils/SM3PasswordUtil.java`:
- Around line 59-62: The current verifyPassword method throws
IllegalArgumentException when any of inputPassword, storedHash, or salt are
null, which turns upstream decryption failures into exceptions; change
verifyPassword(String inputPassword, String storedHash, String salt) to treat
nulls as authentication failures by returning false instead of throwing for null
inputs, preserving thrown exceptions only for genuine crypto errors, or
alternatively ensure LoginController.authenticate checks the decrypted salt for
null and returns a normal auth-failure path; locate the verifyPassword method in
SM3PasswordUtil and update the null-check branch to return false (and keep other
exception handling intact).
In `@base/src/test/java/com/tinyengine/it/login/utils/JwtUtilTest.java`:
- Around line 47-64: The tests for validateSecretConfiguration rely on
environment state but call System.setProperty which doesn't affect System.getenv
used by jwtUtil; make the tests deterministic by mocking or injecting the secret
source instead of relying on process env: update JwtUtilTest to stub the value
read by validateSecretConfiguration (either by injecting a SecretsProvider or by
setting jwtUtil.cachedSecret directly) and keep using the mocked Environment
(environment.getActiveProfiles()) to simulate "dev" vs "prod"; ensure the
invalid-secret test sets the cachedSecret to a short value (or inject a mock
that returns "short") so validateSecretConfiguration exercises the
invalid-secret branch rather than relying on System.setProperty.
- Around line 256-325: Tests like
validateTokenReturnsFalseForTokenWithMissingClaims,
validateTokenHandlesTokenWithExtraClaims,
validateTokenHandlesTokenWithInvalidExpirationDate,
validateTokenHandlesTokenWithNullClaims,
validateTokenHandlesTokenWithEmptyClaims and
validateTokenHandlesTokenWithInvalidAlgorithm currently produce false positives
because they stub tokenBlacklistService.isTokenBlacklisted(token) and use naive
String.replace on a Base64URL compact JWT; instead remove the blacklist stub in
these parsing-focused tests and construct truly malformed tokens by splitting
the generated token on '.', Base64URL-decoding the header or payload part,
mutating the JSON (e.g., remove the "claims" key, add extra fields, change "exp"
value to non-numeric, set claims to null or {} , or change "alg" in the decoded
header), then Base64URL-encode the modified part and reassemble the token (leave
signature unchanged or corrupt it if needed) and pass that to
jwtUtil.validateToken to exercise parsing/claim validation rather than the
blacklist branch.
In `@base/src/test/java/com/tinyengine/it/login/utils/SM2EncryptionUtilTest.java`:
- Around line 140-145: The test decryptWithInvalidCipherText (and the similar
assertions at the other locations) uses assertThrows(Exception.class, ...) which
is too broad; change these to assert the specific exception class that
SM2EncryptionUtil.decrypt actually throws (for example IllegalArgumentException,
BadPaddingException, or the custom SM2-related exception) or assert the
exception message/root cause to verify it failed due to invalid cipher text.
Locate the failing assertions in SM2EncryptionUtilTest (method
decryptWithInvalidCipherText and the two other assertions around lines 209-210
and 252-253) and replace assertThrows(Exception.class, ...) with
assertThrows(TheConcreteException.class, ...) and/or add assertions on
ex.getMessage() or ex.getCause() to validate the invalid-cipher reason.
In `@base/src/test/java/com/tinyengine/it/login/utils/SM3PasswordUtilTest.java`:
- Around line 407-418: The test's worker Runnable in SM3PasswordUtilTest catches
only Exception so AssertionError from assertTrue can slip by; update the task to
catch Throwable (or specifically AssertionError) and assign it to the shared
exceptions[0], or refactor the concurrent test to use an ExecutorService and
submit Callables and call Future.get() so any AssertionError/Throwable from
createPassword/verifyPassword propagates to the main thread; reference the
Runnable task, the exceptions array, assertTrue calls, and
SM3PasswordUtil.createPassword/verifyPassword when making the change.
- Around line 149-176: Update the tests for SM3PasswordUtil to reflect the new
exception contract: change the `@DisplayName` strings that currently mention
NullPointerException to say IllegalArgumentException, update the three
assertThrows calls in sm3HashWithNullData_ThrowsNullPointerException,
sm3HashWithNullSalt_ThrowsNullPointerException and
verifyPasswordWithNullParams_ThrowsException to
assertThrows(IllegalArgumentException.class, ...) when calling
SM3PasswordUtil.sm3Hash(...) and SM3PasswordUtil.verifyPassword(...), and
optionally rename the test method names to end with
ThrowsIllegalArgumentException to keep names consistent with the assertions.
- Around line 265-275: The `@Timeout` values in SM3PasswordUtilTest (notably the
performance_BatchCreateAndVerify test) are too tight for CI and should be
loosened or moved to a perf suite; change the annotation on
performance_BatchCreateAndVerify from `@Timeout`(2) to a much larger value (e.g.,
`@Timeout`(30)) or remove the timeout and annotate the test as `@Disabled` while
moving the scenario to a dedicated benchmark; apply the same treatment to the
other slow test referenced (lines 477-491) so both tests either use relaxed
timeouts or are migrated to a performance test suite.
---
Nitpick comments:
In `@base/src/test/java/com/tinyengine/it/login/utils/SM2EncryptionUtilTest.java`:
- Around line 228-239: The test encryptAndDecryptWithNonUTF8Characters (and the
similar case at lines 257-268) mischaracterizes ISO-8859-1 fixtures as
"non-UTF8" raw-binary tests: they convert raw bytes into a Java String before
calling SM2EncryptionUtil.encrypt/decrypt, so the utility is still operating on
text. Fix by either (A) adding byte-array based APIs and tests that call new
SM2EncryptionUtil.encrypt(byte[]) / decrypt(byte[]) and assert byte-array
fidelity, or (B) if byte-level support is not needed, rename the test methods
and update their descriptions to indicate they are ISO-8859-1 text fixtures
(e.g., encryptAndDecryptWithIso88591Characters) and keep using
SM2EncryptionUtil.encrypt/decrypt as-is.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cffe2758-3b1c-4c12-abab-eef9489ab198
📒 Files selected for processing (5)
base/src/main/java/com/tinyengine/it/login/utils/JwtUtil.javabase/src/main/java/com/tinyengine/it/login/utils/SM3PasswordUtil.javabase/src/test/java/com/tinyengine/it/login/utils/JwtUtilTest.javabase/src/test/java/com/tinyengine/it/login/utils/SM2EncryptionUtilTest.javabase/src/test/java/com/tinyengine/it/login/utils/SM3PasswordUtilTest.java
chilingling
approved these changes
Apr 30, 2026
chilingling
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat: jwtutil,SM2EncryptionUtil,SM3PasswordUtil UT
Summary by CodeRabbit
Bug Fixes
Tests