[Fix][Security] Fix JWT authentication bypass via hardcoded secret and self-comparison logic

[jackieya]

What is the purpose of the change

Fix a critical JWT authentication bypass vulnerability caused by two flaws:

1. Hardcoded JWT secret: TokenManager uses @Value("${jwt.token.secret:asdqwe}") with a hardcoded default, and application.yaml has no jwt.token.secret entry — all default deployments share the same secret.
2. Self-comparison in token validation: AuthenticationInterceptor passes tokeManager.getPassword(token) (extracted from the token itself) as the expected password to validateToken, which then compares it with itself — always returns true.

Combined, an attacker can forge a valid JWT for any user (e.g. admin) without knowing the actual password.

Brief change log

• TokenManager.java: Remove hardcoded default secret asdqwe, add @PostConstruct startup validation to auto-generate a random secret if not configured, and change validateToken to use BCrypt.checkpw instead of String.equals for password comparison.
• AuthenticationInterceptor.java: Change tokeManager.getPassword(token) to user.getPassword() so the token's password is validated against the database BCrypt hash, not against itself.
• application.yaml: Add jwt.token.secret configuration entry so users can discover and configure a custom secret.

Verify this change

• Forged tokens are now rejected (BCrypt check fails for fake passwords).
• Normal login and token refresh continue to work correctly.
• Tested locally with both forged and legitimate tokens.

[Copilot]

Pull request overview

This PR addresses a critical JWT authentication bypass by removing the hardcoded fallback secret, fixing token validation to compare against the stored user password hash (instead of self-comparing token-derived values), and surfacing JWT configuration in application.yaml.

Changes:

• Updated AuthenticationInterceptor to validate the token against the database-stored password hash (user.getPassword()), not the token’s own embedded password.
• Updated TokenManager to remove the hardcoded default secret, add startup secret handling, and switch token password comparison to BCrypt.checkpw.
• Added a jwt.token.secret entry to application.yaml for discoverability/configuration.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
datavines-server/src/main/resources/application.yaml	Adds JWT configuration keys (secret/timeout/algorithm).
datavines-server/src/main/java/io/datavines/server/api/inteceptor/AuthenticationInterceptor.java	Fixes validation to use the DB password hash instead of token-derived password.
datavines-core/src/main/java/io/datavines/core/utils/TokenManager.java	Removes hardcoded default secret, adds secret initialization logic, and updates validation to use BCrypt.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The default jwt.token.secret value committed here is a predictable shared secret. If a deployment uses this file without changing it, attackers can forge JWTs (the original issue still exists, just moved from code to config). Prefer leaving the secret unset and requiring an environment/secret-manager provided value (or at least treat this placeholder as invalid at startup).

Suggested change

	secret: PLEASE_CHANGE_THIS_TO_A_RANDOM_STRING
	secret: ${JWT_TOKEN_SECRET}

[Copilot]

Auto-generating a random JWT secret at startup has significant operational impact: tokens become invalid after restart, and in a multi-instance/clustered deployment each node will generate a different secret so tokens may fail validation depending on which node receives the request. Consider failing fast when jwt.token.secret is missing/weak and requiring a consistent externally configured secret (and/or explicitly documenting the single-node-only behavior).

Suggested change

	if (StringUtils.isEmpty(tokenSecret) || tokenSecret.length() < 16) {
	log.warn("jwt.token.secret is not configured or too short, generating random secret");
	tokenSecret = java.util.UUID.randomUUID().toString().replace("-", "")
	+ java.util.UUID.randomUUID().toString().replace("-", "");
	}
	if (StringUtils.isBlank(tokenSecret)) {
	throw new IllegalStateException("jwt.token.secret must be configured and must not be empty");
	}
	if (tokenSecret.length() < 16) {
	throw new IllegalStateException("jwt.token.secret must be at least 16 characters long");
	}

[Copilot]

BCrypt.checkpw(tokenPassword, password) will likely throw if tokenPassword is null/empty (e.g., token missing the up claim, or getPassword() failed). Previously password.equals(tokenPassword) would safely return false. Consider guarding for missing/blank tokenUsername/tokenPassword and returning false instead of letting an exception bubble up as a 500.

Suggested change

	return (username.equals(tokenUsername) && BCrypt.checkpw(tokenPassword, password) && !(isExpired(token)));
	if (StringUtils.isBlank(username) || StringUtils.isBlank(password)
	|| StringUtils.isBlank(tokenUsername) || StringUtils.isBlank(tokenPassword)) {
	return false;
	}
	return (StringUtils.equals(username, tokenUsername)
	&& BCrypt.checkpw(tokenPassword, password)
	&& !(isExpired(token)));

[Copilot]

This validation approach still depends on the JWT carrying the user's plaintext password (tokenPassword comes from the token claims). That creates a high-impact credential exposure risk if a token is leaked, and it also makes per-request validation expensive (BCrypt on every request). Consider removing password from JWT claims entirely and validating via standard JWT claims (subject/userId) + signature/expiry, optionally with a server-side token revocation/version check.

[Copilot]

validateToken(..., String password) now expects the stored BCrypt hash (from user.getPassword()), not a plaintext password. Renaming this parameter (e.g., passwordHash) would reduce confusion and help prevent accidental misuse in future call sites.