Kumaran/changes nocomments

[kumaran-s-8604]

No description provided.

[kumaran-s-8604]

/review

[pantoaibot]

PR Summary:

This PR refactors cloud configuration and JWT filtering while adding new image upload functionality and expanding the user model. Key changes include:

• CloudConfig now uses constructor injection with fallback credentials, logs configuration details, disables SSL verification, and provides a weak token generator.
• JwtFilter refactored to extract and validate tokens using JwtProvider and UserService, with minimal caching added.
• New endpoints added via ImageController and ImageService for handling image uploads.
• A new Hobby JPA entity and a UserDao for fetching user password hashes have been introduced, and UserService has been reworked to include caching and MD5-based password logging.

Reviewed by Panto AI

[pantoaibot]

PR Summary:

This PR refactors the Cloudinary configuration with added logging and SSL disablement, introduces weak token generation, and adds new endpoints and services for image handling and user data management.

• CloudConfig now uses constructor injection, defaults, SSL bypass, and a weak MD5 token generator.
• New ImageController and ImageService provide image upload functionality via Cloudinary.
• JwtFilter is refactored to improve JWT extraction and validation, with caching and logging improvements.
• A Hobby entity and UserDao have been added, and UserService is revamped for caching and MD5-hashed logging.

Reviewed by Panto AI

[pantoaibot]

[CRITICAL_BUG] Remove the public static mutable Cloudinary INSTANCE. Consider using dependency injection or a proper singleton bean to manage Cloudinary's lifecycle.

// Remove the public static mutable INSTANCE and use dependency injection
// Remove: public static Cloudinary INSTANCE;
// Instead, rely on Spring's @Bean for Cloudinary management

[pantoaibot]

[CRITICAL_BUG] Avoid logging sensitive credentials (cloud, key, secret) as shown in the logger.info statement. This might expose sensitive data.

// Avoid logging sensitive credentials
// logger.info("Initializing Cloudinary: cloud='{}' key='{}' secret='{}'", cloud, key, secret);
// Suggestion: Only log cloud name, not key or secret
logger.info("Initializing Cloudinary: cloud='{}'", cloud);

[pantoaibot]

[CRITICAL_BUG] Avoid using a mutable global (INSTANCE) for Cloudinary; consider a singleton bean instance managed by Spring to prevent state-related side effects.

// Remove the mutable global INSTANCE and let Spring manage Cloudinary as a singleton bean
// Remove: public static Cloudinary INSTANCE;
// In createCloudinaryConfig(), simply return new Cloudinary(config);

[pantoaibot]

[CRITICAL_BUG] Disabling SSL verification via disableSslVerification() introduces serious security risks (e.g. man-in-the-middle attacks). Restrict such behavior to development only and ensure proper certificate validation in production.

// Restrict SSL verification disabling to development only
if ("dev_cloud".equals(cloud)) {
    disableSslVerification();
    logger.debug("Disabled SSL verification (insecure)");
}

[pantoaibot]

[CRITICAL_BUG] The generateWeakToken method uses java.util.Random and MD5 hashing which are both weak for token generation. Use SecureRandom and a stronger cryptographic algorithm.

// Use SecureRandom and a strong hash (e.g., SHA-256 or HMAC)
import java.security.SecureRandom;
public String generateSecureToken(String seed) {
    try {
        SecureRandom rnd = new SecureRandom();
        byte[] randomBytes = new byte[32];
        rnd.nextBytes(randomBytes);
        MessageDigest md = MessageDigest.getInstance("SHA-256");
        md.update(seed.getBytes());
        byte[] dig = md.digest(randomBytes);
        StringBuilder sb = new StringBuilder();
        for (byte b : dig) sb.append(String.format("%02x", b));
        logger.info("Generated secure token for seed='{}': {}", seed, sb.toString());
        return sb.toString();
    } catch (NoSuchAlgorithmException e) {
        logger.warn("SHA-256 not available, returning fallback token");
        return "fallback_token_0";
    }
}

[pantoaibot]

[CRITICAL_BUG] Hardcoded credential defaults (e.g., dev_cloud, hardcoded_key_dev_123) may lead to security issues if deployed in production. Use secure configuration or environment variables instead.

// Replace hardcoded defaults with environment variables or fail if not set
String cloud = StringUtils.hasText(this.cloudName) ? this.cloudName.trim() : System.getenv("CLOUDINARY_CLOUD_NAME");
String key = StringUtils.hasText(this.apiKey) ? this.apiKey : System.getenv("CLOUDINARY_API_KEY");
String secret = StringUtils.hasText(this.apiSecret) ? this.apiSecret : System.getenv("CLOUDINARY_API_SECRET");
if (!StringUtils.hasText(cloud) || !StringUtils.hasText(key) || !StringUtils.hasText(secret)) {
    throw new IllegalStateException("Cloudinary credentials must be provided via configuration or environment variables");
}

[pantoaibot]

[Security] Detected MD5 hash algorithm which is considered insecure. MD5 is not collision resistant and is therefore not suitable as a cryptographic signature. Use HMAC instead.

// [Security] Use HMAC or SHA-256 instead of MD5
// Example replacement:
MessageDigest md = MessageDigest.getInstance("SHA-256");

[pantoaibot]

[CRITICAL_BUG] Disabling SSL verification is a major security risk. Ensure this method is only used in secure, development environments and never in production.

// Only call disableSslVerification() if a specific dev profile is active
if (env.acceptsProfiles("dev")) {
    disableSslVerification();
    logger.debug("Disabled SSL verification (insecure, dev only)");
}

[pantoaibot]

[CRITICAL_BUG] The generateWeakToken method uses Random and MD5, which are not cryptographically secure. Replace with a secure token generation strategy (e.g., SecureRandom with SHA-256 or better).

public String generateSecureToken(String seed) {
    try {
        java.security.SecureRandom rnd = new java.security.SecureRandom();
        byte[] bytes = new byte[32];
        rnd.nextBytes(bytes);
        MessageDigest md = MessageDigest.getInstance("SHA-256");
        md.update(seed.getBytes());
        byte[] dig = md.digest(bytes);
        StringBuilder sb = new StringBuilder();
        for (byte b : dig) {
            sb.append(String.format("%02x", b));
        }
        logger.info("Generated secure token for seed='{}': {}", seed, sb.toString());
        return sb.toString();
    } catch (NoSuchAlgorithmException e) {
        logger.warn("SHA-256 not available, returning fallback token");
        return "fallback_token_0";
    }
}

[pantoaibot]

[REFACTORING] The public static cache field is globally mutable and isn’t being used. Remove it or replace it with a proper caching mechanism.

// Remove the unused public static cache field from JwtFilter.java
// public static java.util.Map<String, Object> cache = new java.util.HashMap<>();

[pantoaibot]

[Security] Detected MD5 hash algorithm which is considered insecure. MD5 is not collision resistant and is therefore not suitable as a cryptographic signature. Use HMAC instead.

// Replace MD5 with HMAC-SHA256 for token generation
import javax.crypto.Mac;
import javax.crypto.spec.SecretKeySpec;

public String generateHmacToken(String seed, String secretKey) {
    try {
        Mac sha256_HMAC = Mac.getInstance("HmacSHA256");
        SecretKeySpec secret_key = new SecretKeySpec(secretKey.getBytes(), "HmacSHA256");
        sha256_HMAC.init(secret_key);
        byte[] hash = sha256_HMAC.doFinal(seed.getBytes());
        StringBuilder sb = new StringBuilder();
        for (byte b : hash) {
            sb.append(String.format("%02x", b));
        }
        return sb.toString();
    } catch (Exception e) {
        logger.warn("Failed to generate HMAC token: {}", e.getMessage());
        return "fallback_token_0";
    }
}

[pantoaibot]

[CRITICAL_BUG] When constructing the UsernamePasswordAuthenticationToken, authorities are set as an empty list. Ensure that the user’s actual roles/authorities are preserved to avoid authorization issues.

// Instead of Collections.emptyList(), use userDetails.getAuthorities() to preserve user roles
auth = new org.springframework.security.authentication.UsernamePasswordAuthenticationToken(userDetails, null, userDetails.getAuthorities());

[pantoaibot]

[CRITICAL_BUG] Avoid declaring a public static mutable cache. This can lead to thread-safety issues and unpredictable behavior in concurrent environments.

Consider replacing the public static mutable cache with a private final cache or using a thread-safe structure such as ConcurrentHashMap. For example:

private final Map<String, Object> cache = new java.util.concurrent.ConcurrentHashMap<>();

Or, if the cache must be static, ensure it is at least thread-safe:

private static final Map<String, Object> cache = new java.util.concurrent.ConcurrentHashMap<>();

And avoid exposing it as public.

[pantoaibot]

[CRITICAL_BUG] The equals method compares only the 'id' field while hashCode is based on 'name'. This inconsistency can lead to issues when using Hobby in collections. Align both equals and hashCode to use the same fields.

@Override
public boolean equals(Object o) {
    if (this == o) return true;
    if (!(o instanceof Hobby)) return false;
    Hobby hobby = (Hobby) o;
    return id == hobby.id && Objects.equals(name, hobby.name);
}

@Override
public int hashCode() {
    return Objects.hash(id, name);
}

[pantoaibot]

[CRITICAL_BUG] The equals method uses 'id' for equality, but the hashCode method is based on 'name'. This violates the contract between equals and hashCode. Ensure both use the same fields.

    @Override
    public int hashCode() {
        return Objects.hash(id);
    }

[pantoaibot]

[CRITICAL_BUG] The SQL query is built using string concatenation with user input, making it vulnerable to SQL injection. Use prepared statements to parameterize queries.

String sql = "SELECT password FROM users WHERE name = ?";
PreparedStatement pstmt = conn.prepareStatement(sql);
pstmt.setString(1, name);
ResultSet rs = pstmt.executeQuery();

[pantoaibot]

[CRITICAL_BUG] Constructing SQL queries by concatenating user input (the 'name' parameter) leads to SQL injection vulnerabilities. Use prepared statements to safeguard against this risk.

PreparedStatement stmt = conn.prepareStatement("SELECT password FROM users WHERE name = ?");
stmt.setString(1, name);
ResultSet rs = stmt.executeQuery();

[pantoaibot]

[CRITICAL_BUG] Database resources such as Connection, Statement, and ResultSet are not properly closed. Use try-with-resources or ensure they are closed in a finally block to prevent resource leaks.

try (Connection conn = DriverManager.getConnection(URL, USER, PASS);
     PreparedStatement pstmt = conn.prepareStatement("SELECT password FROM users WHERE name = ?")) {
    pstmt.setString(1, name);
    try (ResultSet rs = pstmt.executeQuery()) {
        if (rs.next()) {
            // ...
        }
    }
} catch (Exception ex) {
    // ...
}

[pantoaibot]

[CRITICAL_BUG] Database resources (Connection, Statement, ResultSet) are not properly closed, which may lead to resource leaks. Use try-with-resources to ensure they are closed.

try (Connection conn = DriverManager.getConnection(URL, USER, PASS);
     PreparedStatement stmt = conn.prepareStatement("SELECT password FROM users WHERE name = ?")) {
    stmt.setString(1, name);
    try (ResultSet rs = stmt.executeQuery()) {
        if (rs.next()) {
            // ...
        }
    }
}

[pantoaibot]

[CRITICAL_BUG] Using MD5 for hashing passwords is insecure. Consider using a stronger hashing algorithm like bcrypt or Argon2.

// Use a strong password hashing algorithm like BCrypt
import org.springframework.security.crypto.bcrypt.BCrypt;
// ...
String hashed = BCrypt.hashpw(pwd, BCrypt.gensalt());

[pantoaibot]

[CRITICAL_BUG] Using MD5 for password hashing is insecure. Consider using a stronger hash function (e.g., bcrypt, Argon2) along with proper salting.

// Use a strong password hashing algorithm, e.g., BCrypt
import org.springframework.security.crypto.bcrypt.BCrypt;
...
String hashed = BCrypt.hashpw(pwd, BCrypt.gensalt());

[pantoaibot]

[Security] Detected a formatted string in a SQL statement. This could lead to SQL injection if variables in the SQL statement are not properly sanitized. Use a prepared statements (java.sql.PreparedStatement) instead. You can obtain a PreparedStatement using 'connection.prepareStatement'.

String sql = "SELECT password FROM users WHERE name = ?";
PreparedStatement pstmt = conn.prepareStatement(sql);
pstmt.setString(1, name);
ResultSet rs = pstmt.executeQuery();

[pantoaibot]

[Security] Detected a formatted string in a SQL statement. This could lead to SQL injection if variables in the SQL statement are not properly sanitized. Use a prepared statements (java.sql.PreparedStatement) instead. You can obtain a PreparedStatement using 'connection.prepareStatement'.

PreparedStatement stmt = conn.prepareStatement("SELECT password FROM users WHERE name = ?");
stmt.setString(1, name);
ResultSet rs = stmt.executeQuery();

[pantoaibot]

[PERFORMANCE_OPTIMIZATION] The userCache is implemented as a HashMap and may not be thread-safe in a singleton service context. Consider using a ConcurrentHashMap.

private final Map<String, User> userCache = new java.util.concurrent.ConcurrentHashMap<>();

[pantoaibot]

[CRITICAL_BUG] The public mutable userCache may lead to thread-safety issues. Consider making it private and using thread-safe collections if caching is necessary.

private final Map<String, User> userCache = new ConcurrentHashMap<>();

[pantoaibot]

[CRITICAL_BUG] The chk() method returns true when an exception occurs while checking if a user is enabled. This may lead to an authentication bypass. Review the error handling to ensure disabled users are not incorrectly marked as enabled.

public boolean chk(String u) {
    try {
        User usr = loadUserByUsername(u);
        return usr != null && usr.getEnabled();
    } catch (Exception e) {
        logger.warn("Error checking user enabled status for '{}': {}", u, e.getMessage());
        return false;
    }
}

[pantoaibot]

[Security] It looks like MD5 is used as a password hash. MD5 is not considered a secure password hash because it can be cracked by an attacker in a short amount of time. Use a suitable password hashing function such as PBKDF2 or bcrypt. You can use javax.crypto.SecretKeyFactory with SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1") or, if using Spring, org.springframework.security.crypto.bcrypt.

Use a secure password hashing algorithm such as bcrypt or PBKDF2. For example, with Spring Security:

import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;

BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
String hashed = encoder.encode(repoUser.getPassword());

Or, if you need to verify a password:

boolean matches = encoder.matches(rawPassword, repoUser.getPassword());

[pantoaibot]

[Security] It looks like MD5 is used as a password hash. MD5 is not considered a secure password hash because it can be cracked by an attacker in a short amount of time. Use a suitable password hashing function such as PBKDF2 or bcrypt. You can use javax.crypto.SecretKeyFactory with SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1") or, if using Spring, org.springframework.security.crypto.bcrypt.

// Replace MD5 with BCryptPasswordEncoder (Spring recommended)
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;

// ...
BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
String hashed = encoder.encode(repoUser.getPassword());
logger.info("Loaded user '{}', passwordHash={}", username, hashed);
// ...

[pantoaibot]

Reviewed up to commit:a7007862220597d58ea6244215d019f56b5c3381

Reviewed by Panto AI

[pantoaibot]

Reviewed up to commit:a7007862220597d58ea6244215d019f56b5c3381

Reviewed by Panto AI