Skip to content

fix: ensure jwt is not in deny list before further authentication #87

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
srishtigrp78 merged 1 commit into from
Jun 2, 2025
Merged

fix: ensure jwt is not in deny list before further authentication #87

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 25 additions & 19 deletions src/main/java/com/iemr/mmu/utils/JwtUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
package com.iemr.mmu.utils;

import java.security.Key;
import java.util.Date;
import java.util.function.Function;
import javax.crypto.SecretKey;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Component;

import io.jsonwebtoken.Claims;
import io.jsonwebtoken.Jwts;
import io.jsonwebtoken.SignatureAlgorithm;
import io.jsonwebtoken.security.Keys;

@Component
Expand All @@ -18,31 +17,34 @@ public class JwtUtil {
@Value("${jwt.secret}")
private String SECRET_KEY;

private static final long EXPIRATION_TIME = 24L * 60 * 60 * 1000; // 1 day in milliseconds
@Autowired
private TokenDenylist tokenDenylist;

// Generate a key using the secret
private Key getSigningKey() {
private SecretKey getSigningKey() {
if (SECRET_KEY == null || SECRET_KEY.isEmpty()) {
throw new IllegalStateException("JWT secret key is not set in application.properties");
}
return Keys.hmacShaKeyFor(SECRET_KEY.getBytes());
}

// Generate JWT Token
public String generateToken(String username, String userId) {
Date now = new Date();
Date expiryDate = new Date(now.getTime() + EXPIRATION_TIME);

// Include the userId in the JWT claims
return Jwts.builder().setSubject(username).claim("userId", userId) // Add userId as a claim
.setIssuedAt(now).setExpiration(expiryDate).signWith(getSigningKey(), SignatureAlgorithm.HS256)
.compact();
}

// Validate and parse JWT Token
public Claims validateToken(String token) {
try {
return Jwts.parser().setSigningKey(getSigningKey()).build().parseClaimsJws(token).getBody();
Claims claims = Jwts.parser()
.verifyWith(getSigningKey())
.build()
.parseSignedClaims(token)
.getPayload();

String jti = claims.getId();

// Check if token is denylisted (only if jti exists)
if (jti != null && tokenDenylist.isTokenDenylisted(jti)) {
return null;
}

return claims;
} catch (Exception e) {
return null; // Handle token parsing/validation errors
}
Expand All @@ -54,10 +56,14 @@ public String extractUsername(String token) {

public <T> T extractClaim(String token, Function<Claims, T> claimsResolver) {
final Claims claims = extractAllClaims(token);
return claimsResolver.apply(claims);
return claims != null ? claimsResolver.apply(claims) : null;
}

private Claims extractAllClaims(String token) {
return Jwts.parser().setSigningKey(getSigningKey()).build().parseClaimsJws(token).getBody();
return Jwts.parser()
.verifyWith(getSigningKey())
.build()
.parseSignedClaims(token)
.getPayload();
}
Comment on lines 62 to 68
Copy link

@coderabbitai coderabbitai bot May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add exception handling for consistency with validateToken.

The extractAllClaims method doesn't handle exceptions, which could cause runtime errors when called from extractClaim or extractUsername. This is inconsistent with validateToken which catches all exceptions.

Apply this diff to handle exceptions consistently:

 private Claims extractAllClaims(String token) {
+    try {
         return Jwts.parser()
             .verifyWith(getSigningKey())
             .build()
             .parseSignedClaims(token)
             .getPayload();
+    } catch (Exception e) {
+        return null; // Consistent with validateToken behavior
+    }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private Claims extractAllClaims(String token) {
return Jwts.parser().setSigningKey(getSigningKey()).build().parseClaimsJws(token).getBody();
return Jwts.parser()
.verifyWith(getSigningKey())
.build()
.parseSignedClaims(token)
.getPayload();
}
private Claims extractAllClaims(String token) {
try {
return Jwts.parser()
.verifyWith(getSigningKey())
.build()
.parseSignedClaims(token)
.getPayload();
} catch (Exception e) {
return null; // Consistent with validateToken behavior
}
}
🤖 Prompt for AI Agents 
In src/main/java/com/iemr/mmu/utils/JwtUtil.java between lines 62 and 68, the
extractAllClaims method lacks exception handling, which can cause runtime errors
and is inconsistent with the validateToken method. Modify extractAllClaims to
catch exceptions such as JwtException or general Exception, and handle them
appropriately (e.g., return null or throw a custom exception) to ensure
consistent error handling across methods that parse tokens.

}
55 changes: 55 additions & 0 deletions src/main/java/com/iemr/mmu/utils/TokenDenylist.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package com.iemr.mmu.utils;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.data.redis.core.RedisTemplate;
import org.springframework.stereotype.Component;

import java.util.concurrent.TimeUnit;

@Component
public class TokenDenylist {
private final Logger logger = LoggerFactory.getLogger(this.getClass().getName());

private static final String PREFIX = "denied_";

@Autowired
private RedisTemplate<String, Object> redisTemplate;

private String getKey(String jti) {
return PREFIX + jti;
}

// Add a token's jti to the denylist with expiration time
public void addTokenToDenylist(String jti, Long expirationTime) {
if (jti == null || jti.trim().isEmpty()) {
return;
}
if (expirationTime == null || expirationTime <= 0) {
throw new IllegalArgumentException("Expiration time must be positive");
}

try {
String key = getKey(jti); // Use helper method to get the key
redisTemplate.opsForValue().set(key, " ", expirationTime, TimeUnit.MILLISECONDS);
} catch (Exception e) {
throw new RuntimeException("Failed to denylist token", e);
}
}
Comment on lines +24 to +39
Copy link

@coderabbitai coderabbitai bot May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider consistent error handling strategy across methods.

The method throws a RuntimeException on Redis failure, while isTokenDenylisted gracefully handles failures by returning false. This inconsistency could lead to service disruption if Redis is temporarily unavailable.

Consider adopting a uniform approach:

  • Either fail gracefully in both methods (recommended for resilience)
  • Or throw exceptions consistently in both methods

Apply this diff to handle Redis failures gracefully:

 public void addTokenToDenylist(String jti, Long expirationTime) {
     if (jti == null || jti.trim().isEmpty()) {
         return;
     }
     if (expirationTime == null || expirationTime <= 0) {
         throw new IllegalArgumentException("Expiration time must be positive");
     }
 
     try {
         String key = getKey(jti);  // Use helper method to get the key
         redisTemplate.opsForValue().set(key, " ", expirationTime, TimeUnit.MILLISECONDS);
     } catch (Exception e) {
-        throw new RuntimeException("Failed to denylist token", e);
+        logger.error("Failed to add token to denylist for jti: " + jti, e);
+        // Optionally, you could track this failure for monitoring
     }
 }

Also, consider using a more meaningful value instead of a space character:

-        redisTemplate.opsForValue().set(key, " ", expirationTime, TimeUnit.MILLISECONDS);
+        redisTemplate.opsForValue().set(key, "true", expirationTime, TimeUnit.MILLISECONDS);
🤖 Prompt for AI Agents 
In src/main/java/com/iemr/mmu/utils/TokenDenylist.java between lines 24 and 39,
the addTokenToDenylist method currently throws a RuntimeException on Redis
failure, while isTokenDenylisted handles failures gracefully by returning false.
To ensure consistency and improve resilience, modify addTokenToDenylist to catch
exceptions and handle them gracefully without throwing, similar to
isTokenDenylisted. Additionally, replace the placeholder value " " stored in
Redis with a more meaningful value such as "denied" to improve clarity.


// Check if a token's jti is in the denylist
public boolean isTokenDenylisted(String jti) {
if (jti == null || jti.trim().isEmpty()) {
return false;
}
try {
String key = getKey(jti); // Use helper method to get the key
return Boolean.TRUE.equals(redisTemplate.hasKey(key));
} catch (Exception e) {
logger.error("Failed to check denylist status for jti: " + jti, e);
// In case of Redis failure, consider the token as not denylisted to avoid blocking all requests
return false;
}
}
}
Loading