Security Review

HIGH — Exploitable Issues

1. JWT Secret Hardcoded in Test — Weak Secret in Production Likely

What the code actually does: DefaultJwtService (line 28) creates the HMAC signing key from a configurable jwt.secret property:

this.signingKey = new SecretKeySpec(secret.getBytes(), signatureAlgorithm.getJcaName());

The test at DefaultJwtServiceTest.java:16 uses a 60-character string "123123123123123123123123123123123123123123123123123123123123" as the secret. The algorithm is HS512, which requires a 512-bit (64-byte) key. A 60-character ASCII string is only 60 bytes (480 bits) — below the required key length for HS512.

Under what conditions it becomes a risk: - If the production jwt.secret is similarly weak or short, an attacker can brute-force the HMAC key and forge arbitrary JWTs. - The jwt.secret is loaded from application.properties via @Value("${jwt.secret}") — if this property is missing or defaults to a weak value, all JWT authentication is compromised. - With a forged JWT, an attacker can impersonate any user by setting the sub claim to any user ID.

Evidence: - src/main/java/io/spring/infrastructure/service/DefaultJwtService.java:28 — key derived from secret.getBytes() with no minimum length check - src/test/java/io/spring/infrastructure/service/DefaultJwtServiceTest.java:16 — test uses a 60-char secret for HS512 - src/main/java/io/spring/infrastructure/service/DefaultJwtService.java:27 — SignatureAlgorithm.HS512 requires 64-byte key

Recommended fix: - Enforce a minimum key length in DefaultJwtService (throw IllegalArgumentException if secret.getBytes().length < 64 for HS512) - Use a key derivation function (e.g., PBKDF2 with the secret as passphrase) rather than raw bytes - Ensure the production jwt.secret is at least 64 bytes of cryptographic randomness

2. GraphQL Endpoints Have No Authentication Enforcement — Full Public Access

What the code actually does: WebSecurityConfig.java:53-54 permits all requests to /graphql without authentication:

.antMatchers("/graphql")
.permitAll()

The GraphQL layer then relies on individual data fetchers to check authentication manually. For example, UserMutation.updateUser() (line 72-76) checks:

Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
if (authentication instanceof AnonymousAuthenticationToken || authentication.getPrincipal() == null) {
    return null;
}

But UserMutation.login() (line 55-67) and UserMutation.createUser() (line 36-53) are intentionally public — they handle their own auth. The problem is that all GraphQL mutations and queries are accessible without any Spring Security filter enforcement.

Under what conditions it becomes a risk: - Any unauthenticated user can call any GraphQL operation. The only protection is the manual check inside each data fetcher. - If a developer adds a new @DgsData method and forgets the authentication check, it becomes publicly accessible with no defense-in-depth. - The MeDatafetcher.getMe() method (line 31-35) checks for AnonymousAuthenticationToken but still processes the request — it just returns null if unauthenticated. This is inconsistent with the REST API which returns HTTP 401.

Evidence: - src/main/java/io/spring/api/security/WebSecurityConfig.java:53-54 — /graphql is permitAll() - src/main/java/io/spring/graphql/UserMutation.java:72-76 — manual auth check in updateUser - src/main/java/io/spring/graphql/MeDatafetcher.java:31-35 — returns null instead of throwing auth error - src/main/java/io/spring/graphql/SecurityUtil.java:10-18 — utility used by other fetchers, but not enforced at framework level

Recommended fix: - Add a Spring Security SecurityInterceptor or a DGS DataFetchingInterceptor that enforces authentication for all GraphQL operations except explicitly whitelisted ones (login, register) - Or use @PreAuthorize annotations on DGS data fetchers - Make the GraphQL auth check consistent with REST (throw AuthenticationException instead of returning null)

3. Password Not Re-Encrypted on User Update — Plaintext Storage Risk

What the code actually does: UserService.updateUser() (line 46-56) calls user.update() which sets the password directly:

public void update(String email, String username, String password, String bio, String image) {
    if (!Util.isEmpty(password)) {
        this.password = password;  // <-- stored as-is, no encoding
    }
}

The UserService.updateUser() method receives the UpdateUserCommand and calls user.update() with the raw password from UpdateUserParam. The password is NOT passed through passwordEncoder.encode() before being saved.

Under what conditions it becomes a risk: - When a user updates their profile with a new password, the password is stored in plaintext (or whatever format the client sends). - If the client sends an already-encoded password (e.g., from a previous registration), it gets double-stored as-is, but the login flow uses passwordEncoder.matches() which expects a BCrypt hash — so the user would be locked out. - This is a data integrity issue that can lead to plaintext credential storage.

Evidence: - src/main/java/io/spring/core/user/User.java:38-40 — update() sets this.password = password without encoding - src/main/java/io/spring/application/user/UserService.java:46-56 — updateUser() calls user.update() with raw password from UpdateUserParam - Compare with UserService.createUser() line 39: passwordEncoder.encode(registerParam.getPassword()) — encoding IS done on creation

Recommended fix: - In UserService.updateUser(), encode the password before calling user.update():

if (!Util.isEmpty(updateUserParam.getPassword())) {
    user.updatePassword(passwordEncoder.encode(updateUserParam.getPassword()));
}

• Or modify User.update() to accept an already-encoded password and document this contract

MEDIUM — Real Risks Worth Fixing

4. JWT Token Extraction via String Split — No Bearer Scheme Validation

What the code actually does: JwtTokenFilter.getTokenString() (line 50-60) splits the Authorization header on space and takes the second element:

String[] split = header.split(" ");
if (split.length < 2) { return Optional.empty(); }
return Optional.ofNullable(split[1]);

This accepts any scheme — Bearer, Basic, Token, Custom, or even anything valid. There is no check that the scheme is Bearer.

Under what conditions it becomes a risk: - Low severity because the JWT parser will reject non-JWT tokens. However, it violates the principle of strict validation. - An attacker sending Authorization: Bearer (with no token) would pass the split.length < 2 check but split[1] would be empty string — Optional.ofNullable("") returns a non-empty Optional, and the JWT parser would throw an exception caught by the catch-all, returning Optional.empty(). - More concerning: Authorization: Bearer null would be accepted and parsed as JWT string "null".

Evidence: - src/main/java/io/spring/api/security/JwtTokenFilter.java:50-60 — no Bearer prefix validation - src/main/java/io/spring/api/security/JwtTokenFilter.java:28 — getTokenString(request.getHeader(header)) — header name is hardcoded as "Authorization"

Recommended fix: - Add Bearer scheme validation:

if (!split[0].equals("Bearer")) { return Optional.empty(); }

5. CORS Configuration Allows All Origins with Credentials Disabled

What the code actually does: WebSecurityConfig.java:70-78 configures CORS with:

configuration.setAllowedOrigins(asList("*"));
configuration.setAllowCredentials(false);

Under what conditions it becomes a risk: - The wildcard origin * with allowCredentials(false) is the default safe configuration for public APIs. However, this means the API cannot be used with credentials (cookies, Authorization headers) from browser-based clients — which is actually correct for a token-based API. - The risk is informational: if a developer later changes allowCredentials to true (as the commented-out code on line 72-74 suggests was considered), the wildcard origin would make the API vulnerable to cross-origin attacks. - The comment on lines 72-74 explicitly warns about this, which is good, but the commented-out code pattern suggests this was a point of confusion.

Evidence: - src/main/java/io/spring/api/security/WebSecurityConfig.java:70-78 — CORS config with wildcard origin

Recommended fix: - Restrict allowedOrigins to specific domains in production rather than using * - Remove the commented-out code to avoid confusion

6. GraphQL MeDatafetcher.getMe() Returns Token from Raw Header Split

What the code actually does: MeDatafetcher.getMe() (line 39) extracts the token from the Authorization header:

UserWithToken userWithToken = new UserWithToken(userData, authorization.split(" ")[1]);

This is done after the authentication check (lines 31-35), but the token is extracted from the raw header without going through JwtTokenFilter validation. The method receives the Authorization header as a @RequestHeader parameter.

Under what conditions it becomes a risk: - If the JwtTokenFilter fails to set the authentication (e.g., due to a bug), but the AnonymousAuthenticationToken check passes (because Spring Security sets an anonymous token by default), the code would attempt authorization.split(" ")[1] which could throw ArrayIndexOutOfBoundsException if the header is malformed. - The token returned to the client is the same token they sent — it's not re-issued. This is fine for UX but means token rotation doesn't happen on read.

Evidence: - src/main/java/io/spring/graphql/MeDatafetcher.java:39 — raw header split without validation - src/main/java/io/spring/graphql/MeDatafetcher.java:31-35 — auth check returns null, doesn't throw

Recommended fix: - Use jwtService to validate the token before extracting it for the response - Or use SecurityUtil.getCurrentUser() consistently instead of duplicating auth logic

LOW — Informational

7. CSRF Protection Disabled — Acceptable for Token-Based API

What the code actually does: WebSecurityConfig.java:38-39 disables CSRF:

http.csrf().disable()

Risk assessment: This is standard practice for REST APIs that use JWT tokens in Authorization headers (not cookies). CSRF is a browser-based attack that relies on cookies being automatically sent. Since this API uses Authorization: Bearer <token> and SessionCreationPolicy.STATELESS, CSRF protection is not needed. This is correct.

Evidence: - src/main/java/io/spring/api/security/WebSecurityConfig.java:38-39 — CSRF disabled - src/main/java/io/spring/api/security/WebSecurityConfig.java:45-46 — STATELESS session policy

8. Test Passwords Hardcoded as "123"

What the code actually does: Multiple test files create users with password "123": - TestWithCurrentUser.java:29-43 — password "123" - MyBatisUserRepositoryTest.java:20-23 — password '123' - CommentQueryServiceTest.java:42-46 — password '123'

Risk assessment: These are test fixtures, not production data. However, if these test values leak into production (e.g., through seed data scripts or documentation), users with password "123" would exist. This is a common source of real-world breaches.

Evidence: - src/test/java/io/spring/api/TestWithCurrentUser.java:29-43 - src/test/java/io/spring/infrastructure/user/MyBatisUserRepositoryTest.java:20-23 - src/test/java/io/spring/application/comment/CommentQueryServiceTest.java:42-46

Recommended fix: - Use random, complex passwords in test fixtures - Add a linter rule to prevent weak passwords in non-test code

FALSE POSITIVES

9. InvalidAuthenticationException Message Disclosure

Claim: The exception message "invalid email or password" in InvalidAuthenticationException.java:6-7 could help attackers distinguish between valid and invalid usernames.

Verdict: FALSE POSITIVE

Why: The code at UsersApi.java:49-57 checks both email existence AND password match in a single condition:

if (optional.isPresent() && passwordEncoder.matches(loginParam.getPassword(), optional.get().getPassword())) {
    // success
} else {
    throw new InvalidAuthenticationException();  // always "invalid email or password"
}

The same exception is thrown whether the email doesn't exist OR the password is wrong. This is the correct implementation of a generic error message that prevents user enumeration. The message "invalid email or password" does not reveal which field is incorrect.

Evidence: - src/main/java/io/spring/api/UsersApi.java:49-57 — single error path for both failure cases - src/main/java/io/spring/api/exception/InvalidAuthenticationException.java:6-7 — fixed message

10. @AllArgsConstructor on Controllers Exposes All Dependencies

Claim: Using Lombok @AllArgsConstructor on controllers like UsersApi could expose internal dependencies.

Verdict: FALSE POSITIVE

Why: @AllArgsConstructor generates a constructor with all fields as parameters. Spring uses this constructor for dependency injection. The fields are private and the class is a @RestController — there is no security issue. The generated constructor is package-private by default (since the class is not final and Lombok generates a public constructor), but this is standard Spring practice and does not expose any attack surface.

Evidence: - src/main/java/io/spring/api/UsersApi.java:30-37 — @AllArgsConstructor with private fields - src/main/java/io/spring/graphql/UserMutation.java:28-34 — same pattern

Summary

Most Critical Action: The JWT secret strength and GraphQL authentication bypass are the highest priority findings. An attacker who can forge JWTs or access GraphQL endpoints without authentication can compromise the entire user database.