Skip to content

Commit

Permalink
Changed error handling for session service to use exceptions instead of
Browse files Browse the repository at this point in the history 
isValid field.
  • Loading branch information
@alexbalakirev
alexbalakirev committed Sep 3, 2024
1 parent 561865d commit f23b975
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 76 deletions.
16 changes: 4 additions & 12 deletions src/main/java/com/corbado/entities/SessionValidationResult.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,13 @@
@Setter
@NoArgsConstructor
@AllArgsConstructor

/**
* The Class SessionValidationResultBuilder.
*/
@Builder
public class SessionValidationResult {

/** Indicates success of validation by session service. */
@Builder.Default boolean authenticated = false;

/** The user ID. */
private String userID;

Expand All @@ -25,13 +26,4 @@ public class SessionValidationResult {

/** The error. */
private Exception error;

/**
* Instantiates a new session validation result.
*
* @param error the error
*/
public SessionValidationResult(final Exception error) {
this.error = error;
}
}
63 changes: 29 additions & 34 deletions src/main/java/com/corbado/services/SessionService.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
package com.corbado.services;

import java.net.MalformedURLException;
import java.net.URL;
import java.security.interfaces.RSAPublicKey;
import java.util.concurrent.TimeUnit;

import org.apache.commons.lang3.StringUtils;

import com.auth0.jwk.Jwk;
import com.auth0.jwk.JwkException;
import com.auth0.jwk.JwkProvider;
Expand All @@ -14,11 +21,7 @@
import com.corbado.entities.SessionValidationResult;
import com.corbado.sdk.Config;
import com.corbado.utils.ValidationUtils;
import java.net.MalformedURLException;
import java.net.URL;
import java.security.interfaces.RSAPublicKey;
import java.util.concurrent.TimeUnit;
import org.apache.commons.lang3.StringUtils;

import lombok.Getter;
import lombok.NonNull;
import lombok.Setter;
Expand Down Expand Up @@ -113,13 +116,16 @@ public void setIssuerMismatchError(final String issuer) {
}

/**
* Gets the and validate short session value.
* Gets the and validate user from short session value.
*
* @param shortSession the short session
* @return the and validate short session value
* @return the and validate user from short session value
* @throws JWTVerificationException the JWT verification exception
* @throws JwkException the jwk exception
* @throws IncorrectClaimException the incorrect claim exception
*/
private SessionValidationResult getAndValidateUserFromShortSessionValue(
final String shortSession) {
private SessionValidationResult getAndValidateUserFromShortSessionValue(final String shortSession)
throws JWTVerificationException, JwkException, IncorrectClaimException {

if (shortSession == null || shortSession.isEmpty()) {
throw new IllegalArgumentException("Session value cannot be null or empty");
Expand All @@ -135,55 +141,44 @@ private SessionValidationResult getAndValidateUserFromShortSessionValue(

// Verify and decode the JWT using the signing key
final Algorithm algorithm = Algorithm.RSA256(publicKey);
final JWTVerifier verifier = JWT.require(algorithm).withIssuer(issuer).build();
final JWTVerifier verifier = JWT.require(algorithm).withIssuer(this.issuer).build();
decodedJwt = verifier.verify(shortSession);

return SessionValidationResult.builder()
.authenticated(true)
.fullName(decodedJwt.getClaim("name").asString())
.userID(decodedJwt.getClaim("sub").asString())
.build();

} catch (final IncorrectClaimException e) {
// Be careful of the case where issuer does not match. You have probably forgotten to set
// the cname in config class.
// the cname in config class. We add an additional message to the exception and retrow it to
// underline its importance.
if (StringUtils.equals(e.getClaimName(), "iss")) {
final String message =
e.getMessage()
+ "Be careful of the case where issuer does not match. You have probably forgotten to set the cname in config class.";
final IncorrectClaimException exception =
new IncorrectClaimException(message, e.getClaimName(), e.getClaimValue());

setValidationError(exception);
return new SessionValidationResult(exception);
+ "Be careful of the case where issuer does not match. "
+ "You have probably forgotten to set the cname in config class.";
throw new IncorrectClaimException(message, e.getClaimName(), e.getClaimValue());
}
setValidationError(e);
return new SessionValidationResult(e);
throw e;

} catch (final JwkException | JWTVerificationException e) {
setValidationError(e);
return new SessionValidationResult(e);
throw e;
}
}

/**
* Retrieves userID and full name if 'shortSession' is valid.
*
* @param shortSession the short session
* @return the current user{@link SessionValidationResult}
* @return the and validate current user
* @throws IncorrectClaimException the incorrect claim exception
* @throws JWTVerificationException the JWT verification exception
* @throws JwkException the jwk exception
*/
public SessionValidationResult getAndValidateCurrentUser(final String shortSession) {
public SessionValidationResult getAndValidateCurrentUser(final String shortSession)
throws IncorrectClaimException, JWTVerificationException, JwkException {

return getAndValidateUserFromShortSessionValue(shortSession);
}

/**
* Sets the validation error.
*
* @param error the new validation error
*/
private void setValidationError(@NonNull final Exception error) {
this.lastShortSessionValidationResult =
String.format("JWT validation failed: %s", error.getMessage());
}
}
74 changes: 44 additions & 30 deletions src/test/java/com/corbado/unit/SessionServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,7 @@
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.anyString;
import com.auth0.jwk.Jwk;
import com.auth0.jwk.JwkException;
import com.auth0.jwk.JwkProvider;
import com.auth0.jwt.JWT;
import com.auth0.jwt.algorithms.Algorithm;
import com.corbado.entities.SessionValidationResult;
import com.corbado.exceptions.StandardException;
import com.corbado.services.SessionService;
import com.google.gson.Gson;
import com.google.gson.JsonArray;
import com.google.gson.JsonObject;
import com.google.gson.JsonSyntaxException;

import java.io.FileNotFoundException;
import java.io.FileReader;
import java.io.IOException;
Expand All @@ -35,6 +24,7 @@
import java.util.List;
import java.util.Map;
import java.util.stream.Stream;

import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand All @@ -46,6 +36,24 @@
import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;

import com.auth0.jwk.Jwk;
import com.auth0.jwk.JwkException;
import com.auth0.jwk.JwkProvider;
import com.auth0.jwk.SigningKeyNotFoundException;
import com.auth0.jwt.JWT;
import com.auth0.jwt.algorithms.Algorithm;
import com.auth0.jwt.exceptions.IncorrectClaimException;
import com.auth0.jwt.exceptions.JWTDecodeException;
import com.auth0.jwt.exceptions.JWTVerificationException;
import com.auth0.jwt.exceptions.TokenExpiredException;
import com.corbado.entities.SessionValidationResult;
import com.corbado.exceptions.StandardException;
import com.corbado.services.SessionService;
import com.google.gson.Gson;
import com.google.gson.JsonArray;
import com.google.gson.JsonObject;
import com.google.gson.JsonSyntaxException;

/** Unit Test for session service. */
@ExtendWith(MockitoExtension.class)
public class SessionServiceTest {
Expand Down Expand Up @@ -105,12 +113,11 @@ public static void setUpClass()
* @throws JsonSyntaxException the json syntax exception
* @throws JwkException the jwk exception
*/
@SuppressWarnings("unchecked")
@BeforeEach
public void setUp() throws IOException, JsonSyntaxException, JwkException {
final Gson gson = new Gson();
final Jwk ret = Jwk.fromValues(gson.fromJson(jwks.get(0), Map.class));
Mockito.lenient().when(jwkProvider.get(anyString())).thenReturn(ret);
Mockito.lenient().when(this.jwkProvider.get(anyString())).thenReturn(ret);
}

// ------------------ Tests --------------------- //
Expand Down Expand Up @@ -169,17 +176,24 @@ void testInitParametersValidation(
/**
* Test get current user.
*
* @param expectValid the expect valid
* @param jwt the jwt
* @param expectValid if user expected to be valid
* @param jwt the jwt to be verified
* @param e the exception class expected to be thrown
* @throws StandardException the standard exception
* @throws IncorrectClaimException the incorrect claim exception
* @throws JWTVerificationException the JWT verification exception
* @throws JwkException the jwk exception
*/
@ParameterizedTest
@MethodSource("provideJwts")
void test_getCurrentUser(final boolean expectValid, final String jwt) throws StandardException {
final SessionValidationResult validationResult = sessionService.getAndValidateCurrentUser(jwt);
assertEquals(expectValid, validationResult.isAuthenticated());
void test_getCurrentUser(final String jwt, Class<Exception> e)
throws StandardException, IncorrectClaimException, JWTVerificationException, JwkException {

if (expectValid) {
if (e != null) {
assertThrows(e, () -> sessionService.getAndValidateCurrentUser(jwt));
} else {
final SessionValidationResult validationResult =
sessionService.getAndValidateCurrentUser(jwt);
assertEquals(TEST_NAME, validationResult.getFullName());
assertEquals(TEST_USER_ID, validationResult.getUserID());
}
Expand Down Expand Up @@ -215,52 +229,52 @@ static List<Object[]> provideJwts() throws InvalidKeySpecException, NoSuchAlgori
final List<Object[]> testData = new ArrayList<>();

// JWT with invalid format
testData.add(new Object[] {false, "invalid"});
testData.add(new Object[] {"invalid", JWTDecodeException.class});

// JWT signed with wrong algorithm (HS256 instead of RS256)
final String jwtWithWrongAlgorithm =
"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6Ik"
+ "pvaG4gRG9lIiwiYWRtaW4iOnRydWV9.dyt0CoTl4WoVjAHI9Q_CwSKhl6d_9rhM3NrXuJttkao";
testData.add(new Object[] {false, jwtWithWrongAlgorithm});
testData.add(new Object[] {jwtWithWrongAlgorithm, SigningKeyNotFoundException.class});

// Not before (nbf) in future
testData.add(
new Object[] {
false,
generateJwt(
"https://auth.acme.com",
System.currentTimeMillis() / 1000 + 100,
System.currentTimeMillis() / 1000 + 100)
System.currentTimeMillis() / 1000 + 100),
IncorrectClaimException.class
});

// Expired (exp)
testData.add(
new Object[] {
false,
generateJwt(
"https://auth.acme.com",
System.currentTimeMillis() / 1000 - 100,
System.currentTimeMillis() / 1000 - 100)
System.currentTimeMillis() / 1000 - 100),
TokenExpiredException.class
});

// Invalid issuer (iss)
testData.add(
new Object[] {
false,
generateJwt(
"https://invalid.com",
System.currentTimeMillis() / 1000 + 100,
System.currentTimeMillis() / 1000 - 100)
System.currentTimeMillis() / 1000 - 100),
IncorrectClaimException.class
});

// Success
testData.add(
new Object[] {
true,
generateJwt(
"https://auth.acme.com",
System.currentTimeMillis() / 1000 + 100,
System.currentTimeMillis() / 1000 - 100)
System.currentTimeMillis() / 1000 - 100),
null
});

return testData;
Expand Down

0 comments on commit f23b975

Please sign in to comment.