fix: Decouple JWT signing key from user password hash

[hengyuss]

#6398

Changes

• Decouple JWT signing key from user password hash by introducing shenyu.jwt.secretKey configuration
• Admin now requires an explicit shenyu.jwt.secretKey (or env var SHENYU_JWT_SECRETKEY) and refuses to start without one (fail-fast)
• Updated DashboardUserServiceImpl (sign) and ShiroRealm (verify) to use the new key
• Added regression tests: DashboardUserServiceTest now stubs jwtProperties.getSecretKey() and asserts verifyToken(token, key) is true/false
• Added JwtPropertiesTest for blank/default/configured init() cases
• Added startup WARN about session invalidation on upgrade/rollback
• Wired shenyu.jwt.secretKey into all e2e/k8s/integrated-test/docker-compose configurations

Migration Note
Before upgrading, set shenyu.jwt.secretKey in your configuration (or SHENYU_JWT_SECRETKEY env var). All existing sessions will be invalidated on upgrade; users will need to re-login. Rolling back will also invalidate all tokens issued by this version.
Make sure that:

• You have read the contribution guidelines.
• You submit test cases (unit or integration tests) that back your changes.
• Your local test passed ./mvnw clean install -Dmaven.javadoc.skip=true.

[Aias00]

The @PostConstruct init generates a SecureRandom secretKey per JVM when shenyu.jwt.secretKey is not explicitly configured. In a multi-instance Admin cluster this causes tokens signed by instance A to fail verification on instance B, resulting in random 401s for dashboard users. Consider either failing fast (throw new IllegalStateException) to force explicit configuration, or persisting the generated key somewhere shared across the cluster.

[hengyuss]

@Aias00 i want to choose fast fail, admin application will start fail if shenyu.jwt.seccretKey not configured. is this okay?

[Aias00]

Reviewed #6408 — decoupling the JWT signing key from the password hash is the right direction and the two call sites (DashboardUserServiceImpl sign, ShiroRealm verify) are updated consistently; the rewritten ShiroRealmTest token signatures check out. But I think this needs another pass before merge.

Blocker

1. The default-key fallback is not production-safe. JwtProperties.@PostConstruct generates a random 32-byte key in memory when the configured key equals the sentinel "defaultSecretKey", and the shipped application.yml sets exactly that sentinel — so out-of-the-box every restart mints a new key and all issued JWTs stop validating (mass forced re-login on every deploy/restart). Worse, ShenYu supports clustered admin (shenyu.cluster), and each instance generates a different random key, so a token issued by instance A fails verification on instance B — cluster auth is broken. The e2e only passes because it runs in a single boot. Either require an explicit key and fail-fast in production, or persist the generated key (DB / shared store) so it survives restart and is shared across instances; an ephemeral random fallback, if kept, should be dev-profile only and not shipped as the default.

Should fix

2. Upgrade/migration: pre-PR tokens are signed with the user's password hash; post-PR they're verified against secretKey, so every existing session is invalidated on upgrade (and again on rollback). This is acceptable for a security fix but isn't documented — please add a release/migration note and a startup WARN so operators aren't surprised.

3. JwtProperties.init() only randomizes when secretKey equals the literal "defaultSecretKey". A blank/empty/null shenyu.jwt.secret-key: is not caught — Algorithm.HMAC256(null) returns "" (verified by JwtUtilsTest.testGenerateTokenWithNullKey), breaking all login; an empty string yields an insecure empty HMAC key with no warning. Detect null/blank (not just the magic string) and randomize-or-fail-fast. The sentinel-string approach also silently replaces a key a user might legitimately set to "defaultSecretKey".

4. Coverage gap: in DashboardUserServiceTest, jwtProperties is a @Mock and getSecretKey() is never stubbed, so it returns null and login() produces an empty-string token; assertLoginSuccessful only checks id/userName/password, so the test passes despite the issued token being invalid. The PR's whole point is changing the signing key — please add a regression test stubbing jwtProperties.getSecretKey() with a real key and asserting JwtUtils.verifyToken(loginResult.getToken(), key) is true (and false for a wrong key).

5. JwtPropertiesTest isn't updated for the new secretKey field/getter/setter or the @PostConstruct randomization (the security-critical part). Note new JwtProperties() won't trigger @PostConstruct, so the randomization path needs a Spring-context test or a direct init() call.

Nits

6. The LOG.warn(...) says the default is "not secure" immediately before making it secure (random). Reword to state the real consequence: ephemeral key → tokens don't survive restart and multi-instance breaks; configure shenyu.jwt.secretKey (or SHENYU_JWT_SECRETKEY) for production.

7. Consider not committing the literal sentinel "defaultSecretKey" to application.yml, and document the env-var / external-secret approach for real deployments.

CI is green, but the e2e doesn't exercise restart or multi-instance, so it can't catch (1).

[Aias00]

Follow-up after 60d79db — thanks, this single commit addresses all seven items from my earlier review:

• Blocker (ephemeral random key breaking restart/cluster): random generation removed; init() now throws IllegalStateException when the key is blank or equals the sentinel, and the message explicitly calls out the multi-instance cluster failure. ✔
• #2 upgrade/rollback session invalidation: startup LOG.warn added. ✔
• #3 null/blank key handling: StringUtils.isBlank(...) || sentinel → throw. ✔
• #4 login token regression: DashboardUserServiceTest now stubs getSecretKey() and asserts verifyToken(token, key) is true and verifyToken(token, "wrongKey") is false. ✔
• #5 JwtPropertiesTest: blank / default / configured init() cases added. ✔
• nit #6 warn wording reworded; nit #7 literal sentinel removed from application.yml. ✔

Two things still block merge:

1. CI is red on the new commit — k8s-examples-http failed in ~3m45s (and the rest of the matrix is still pending). The shipped application.yml now comments out shenyu.jwt.secret-key, so JwtProperties.init() throws at boot; any environment that boots admin straight off the shipped config (the k8s/e2e examples) crash-loops immediately. The fail-fast is the right security posture, but the e2e/k8s/bootstrap harness must set shenyu.jwt.secretKey (e.g. SHENYU_JWT_SECRETKEY env var or a test profile) before admin can start. Please wire the key into the example/e2e bootstrap so CI can boot admin again.

2. PR body is now stale and contradicts the code. It still says "the code will generate a random string as secretKey if user doesn't configure it" — that behavior was removed in this commit. Please rewrite the body to describe the actual fail-fast contract: admin now requires an explicit shenyu.jwt.secretKey and refuses to start without one; upgrading invalidates all existing sessions, and so does rolling back. A one-line migration note ("set shenyu.jwt.secretKey before upgrading") would stop operators hitting the startup crash.

Keeping changes-requested until the e2e/bootstrap boots admin again and the body matches the code.

[Aias00]

Follow-up after the new commits + body update: both of my remaining blockers are resolved, and CI is now all green.

• CI red (k8s-examples-http fail) is fixed. shenyu.jwt.secretKey is now wired into the e2e/k8s/integrated-test/docker-compose configurations, so admin boots again under fail-fast. build, k8s-examples-http, e2e all pass now.
• PR body is rewritten and matches the code. It now states the fail-fast contract ("Admin now requires an explicit shenyu.jwt.secretKey ... and refuses to start without one"), the upgrade/rollback session invalidation, and the migration note ("Before upgrading, set shenyu.jwt.secretKey ... All existing sessions will be invalidated on upgrade ... Rolling back will also invalidate all tokens"). Exactly what was missing.

All seven original findings plus the two follow-up blockers are addressed. Switching to approve.

One nit: mergeStateStatus is BEHIND — please rebase onto current master before merge.