diff --git a/backend/src/main/java/ch/puzzle/okr/security/JwtHelper.java b/backend/src/main/java/ch/puzzle/okr/security/JwtHelper.java index b9c9e62458..fe15d284f1 100644 --- a/backend/src/main/java/ch/puzzle/okr/security/JwtHelper.java +++ b/backend/src/main/java/ch/puzzle/okr/security/JwtHelper.java @@ -15,8 +15,11 @@ import org.springframework.stereotype.Component; import java.text.MessageFormat; +import java.util.Arrays; +import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.function.Function; import static ch.puzzle.okr.Constants.USER; import static org.springframework.http.HttpStatus.BAD_REQUEST; @@ -25,6 +28,8 @@ public class JwtHelper { public static final String CLAIM_TENANT = "tenant"; public static final String CLAIM_ISS = "iss"; + public static final String ERROR_MESSAGE = "Missing `" + CLAIM_TENANT + "` and '" + CLAIM_ISS + + "' claims in JWT token!"; private static final Logger logger = LoggerFactory.getLogger(JwtHelper.class); @@ -61,36 +66,45 @@ public User getUserFromJwt(Jwt token) { public String getTenantFromToken(Jwt token) { TokenHelper helper = new TokenHelper(); + List>> getTenantFromTokenFunctions = Arrays.asList( // + helper::getTenantFromTokenUsingClaimIss, // + helper::getTenantFromTokenUsingClaimTenant // + ); - Optional tenantUsingClaimIss = helper.getTenantFromTokenUsingClaimIss(token); - if (tenantUsingClaimIss.isPresent()) { - return getMatchingTenantFromConfigOrThrow(tenantUsingClaimIss.get()); - } + return getFirstMatchingTenantUsingListOfHelperFunctions(token, getTenantFromTokenFunctions); + } - Optional tenantUsingClaimTenant = helper.getTenantFromTokenUsingClaimTenant(token); - if (tenantUsingClaimTenant.isPresent()) { - return getMatchingTenantFromConfigOrThrow(tenantUsingClaimTenant.get()); - } + private String getFirstMatchingTenantUsingListOfHelperFunctions(Jwt token, + List>> getTenantFunctions) { - logErrorAndThrowException(CLAIM_TENANT, CLAIM_ISS); - return null; // only to make the compiler happy + return getTenantFunctions.stream() // + .map(func -> func.apply(token)) // + .filter(Optional::isPresent) // + .map(Optional::get) // + .map(this::getMatchingTenantFromConfigOrThrow) // + .findFirst() // + .orElseThrow(() -> new RuntimeException(ERROR_MESSAGE)); } public String getTenantFromJWTClaimsSet(JWTClaimsSet claimSet) { ClaimHelper helper = new ClaimHelper(); + List>> getTenantFromClaimsSetFunctions = Arrays.asList( // + helper::getTenantFromClaimsSetUsingClaimIss, // + helper::getTenantFromClaimsSetUsingClaimTenant // + ); - Optional tenantUsingClaimIss = helper.getTenantFromClaimsSetUsingClaimIss(claimSet); - if (tenantUsingClaimIss.isPresent()) { - return getMatchingTenantFromConfigOrThrow(tenantUsingClaimIss.get()); - } + return getFirstMatchingTenantUsingListOfHelperFunctions(claimSet, getTenantFromClaimsSetFunctions); + } - Optional tenantUsingClaimTenant = helper.getTenantFromClaimsSetUsingClaimTenant(claimSet); - if (tenantUsingClaimTenant.isPresent()) { - return getMatchingTenantFromConfigOrThrow(tenantUsingClaimTenant.get()); - } + private String getFirstMatchingTenantUsingListOfHelperFunctions(JWTClaimsSet claimSet, + List>> getTenantFunctions) { - logErrorAndThrowException(CLAIM_TENANT, CLAIM_ISS); - return null; // only to make the compiler happy + return getTenantFunctions.stream() // + .map(func -> func.apply(claimSet)) // + .filter(Optional::isPresent) // + .map(Optional::get) // + .map(this::getMatchingTenantFromConfigOrThrow).findFirst() // + .orElseThrow(() -> new RuntimeException(ERROR_MESSAGE)); } private String getMatchingTenantFromConfigOrThrow(String tenant) { @@ -100,10 +114,4 @@ private String getMatchingTenantFromConfigOrThrow(String tenant) { .tenantId(); } - private void logErrorAndThrowException(String tenant, String iss) throws RuntimeException { - String errorInfo = "* Missing `" + tenant + "` and '" + iss + "' claims in JWT token!"; - logger.error(errorInfo); - throw new RuntimeException(errorInfo); - } - } \ No newline at end of file diff --git a/backend/src/main/java/ch/puzzle/okr/security/helper/ClaimHelper.java b/backend/src/main/java/ch/puzzle/okr/security/helper/ClaimHelper.java index 01a1f44608..9ceb7a4034 100644 --- a/backend/src/main/java/ch/puzzle/okr/security/helper/ClaimHelper.java +++ b/backend/src/main/java/ch/puzzle/okr/security/helper/ClaimHelper.java @@ -14,43 +14,38 @@ public class ClaimHelper { public Optional getTenantFromClaimsSetUsingClaimTenant(JWTClaimsSet claimSet) { try { - String tenant = getTenant(claimSet); - return Optional.ofNullable(tenant); + return getTenant(claimSet); } catch (ParseException e) { logStatus(CLAIM_TENANT, claimSet, e); return Optional.empty(); } } - private static String getTenant(JWTClaimsSet claimSet) throws ParseException { + private Optional getTenant(JWTClaimsSet claimSet) throws ParseException { String tenant = claimSet.getStringClaim(CLAIM_TENANT); logStatus(CLAIM_TENANT, claimSet, tenant); - return tenant; + return Optional.ofNullable(tenant); } public Optional getTenantFromClaimsSetUsingClaimIss(JWTClaimsSet claimSet) { try { - String issUrl = getIssUrl(claimSet); - if (issUrl == null) { - return Optional.empty(); - } - return getTenant(claimSet, issUrl); + return getIssUrl(claimSet).flatMap(url -> getTenant(claimSet, url)); } catch (ParseException e) { logStatus(CLAIM_ISS, claimSet, e); return Optional.empty(); } } - private static String getIssUrl(JWTClaimsSet claimSet) throws ParseException { + private Optional getIssUrl(JWTClaimsSet claimSet) throws ParseException { String issUrl = claimSet.getStringClaim(CLAIM_ISS); logStatus(CLAIM_ISS, claimSet, issUrl); - return issUrl; + return Optional.ofNullable(issUrl); } - private static Optional getTenant(JWTClaimsSet claimSet, String issUrl) { - String tenant = extractTenantFromIssUrl(issUrl); - logStatus(CLAIM_ISS, claimSet, tenant); - return Optional.ofNullable(tenant); + private Optional getTenant(JWTClaimsSet claimSet, String issUrl) { + Optional tenant = extractTenantFromIssUrl(issUrl); + logStatus(CLAIM_ISS, claimSet, tenant.isPresent()); + return tenant; } } diff --git a/backend/src/main/java/ch/puzzle/okr/security/helper/JwtStatusLogger.java b/backend/src/main/java/ch/puzzle/okr/security/helper/JwtStatusLogger.java index 280e295b95..ca5d678458 100644 --- a/backend/src/main/java/ch/puzzle/okr/security/helper/JwtStatusLogger.java +++ b/backend/src/main/java/ch/puzzle/okr/security/helper/JwtStatusLogger.java @@ -10,7 +10,10 @@ public class JwtStatusLogger { private static final Logger logger = LoggerFactory.getLogger(ClaimHelper.class); public static void logStatus(String claim, Object context, String result) { - boolean isOk = result != null; + logStatus(claim, context, result != null); + } + + public static void logStatus(String claim, Object context, boolean isOk) { if (isOk) { logger.info("Tenant: get claim '{}' from {}{}", claim, context.getClass().getSimpleName(), statusToSymbol(isOk)); diff --git a/backend/src/main/java/ch/puzzle/okr/security/helper/TokenHelper.java b/backend/src/main/java/ch/puzzle/okr/security/helper/TokenHelper.java index ad1eaa4d89..513241cb06 100644 --- a/backend/src/main/java/ch/puzzle/okr/security/helper/TokenHelper.java +++ b/backend/src/main/java/ch/puzzle/okr/security/helper/TokenHelper.java @@ -12,33 +12,28 @@ public class TokenHelper { public Optional getTenantFromTokenUsingClaimTenant(Jwt token) { - String tenant = getTenant(token); - return Optional.ofNullable(tenant); + return getTenant(token); } - private static String getTenant(Jwt token) { + private Optional getTenant(Jwt token) { String tenant = token.getClaimAsString(CLAIM_TENANT); // can return null logStatus(CLAIM_TENANT, token, tenant); - return tenant; + return Optional.ofNullable(tenant); } public Optional getTenantFromTokenUsingClaimIss(Jwt token) { - String issUrl = getIssUrl(token); - if (issUrl == null) { - return Optional.empty(); - } - return getTenant(token, issUrl); + return getIssUrl(token).flatMap(url -> getTenant(token, url)); } - private String getIssUrl(Jwt token) { + private Optional getIssUrl(Jwt token) { String issUrl = token.getClaimAsString(CLAIM_ISS); // can return null logStatus(CLAIM_ISS, token, issUrl); - return issUrl; + return Optional.ofNullable(issUrl); } private Optional getTenant(Jwt token, String issUrl) { - String tenant = extractTenantFromIssUrl(issUrl); - logStatus(CLAIM_ISS, token, tenant); - return Optional.ofNullable(tenant); + Optional tenant = extractTenantFromIssUrl(issUrl); + logStatus(CLAIM_ISS, token, tenant.isPresent()); + return tenant; } } diff --git a/backend/src/main/java/ch/puzzle/okr/security/helper/UrlHelper.java b/backend/src/main/java/ch/puzzle/okr/security/helper/UrlHelper.java index b630b654d5..5857e4fa01 100644 --- a/backend/src/main/java/ch/puzzle/okr/security/helper/UrlHelper.java +++ b/backend/src/main/java/ch/puzzle/okr/security/helper/UrlHelper.java @@ -1,9 +1,14 @@ package ch.puzzle.okr.security.helper; +import java.util.Optional; + public class UrlHelper { - public static String extractTenantFromIssUrl(String issUrl) { + public static Optional extractTenantFromIssUrl(String issUrl) { + if (issUrl == null) + return Optional.empty(); String[] issUrlParts = issUrl.split("/"); - return issUrlParts[issUrlParts.length - 1]; + String tenant = issUrlParts[issUrlParts.length - 1]; + return Optional.of(tenant); } } diff --git a/backend/src/test/java/ch/puzzle/okr/security/helper/UrlHelperTest.java b/backend/src/test/java/ch/puzzle/okr/security/helper/UrlHelperTest.java index 6d73026ec7..76f71a918e 100644 --- a/backend/src/test/java/ch/puzzle/okr/security/helper/UrlHelperTest.java +++ b/backend/src/test/java/ch/puzzle/okr/security/helper/UrlHelperTest.java @@ -5,7 +5,9 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; -import static org.junit.jupiter.api.Assertions.assertEquals; +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.*; public class UrlHelperTest { @@ -18,10 +20,11 @@ void extractTenantFromIssUrlReturnTenantIfUrlContainSlash(String issUrl) { // arrange // act - String tenantFromIssUrl = UrlHelper.extractTenantFromIssUrl(issUrl); + Optional tenantFromIssUrl = UrlHelper.extractTenantFromIssUrl(issUrl); // assert - assertEquals(PITC, tenantFromIssUrl); + assertTrue(tenantFromIssUrl.isPresent()); + assertEquals(PITC, tenantFromIssUrl.get()); } @DisplayName("extractTenantFromIssUrl() return input url if url not contains slash") @@ -31,9 +34,23 @@ void extractTenantFromIssUrlReturnInputIfUrlNotContainSlash() { String issUrl = "this_is_not_a_valid_url"; // act - String tenantFromIssUrl = UrlHelper.extractTenantFromIssUrl(issUrl); + Optional tenantFromIssUrl = UrlHelper.extractTenantFromIssUrl(issUrl); + + // assert + assertTrue(tenantFromIssUrl.isPresent()); + assertEquals(issUrl, tenantFromIssUrl.get()); + } + + @DisplayName("extractTenantFromIssUrl() return empty if url is null") + @Test + void extractTenantFromIssUrlReturnEmptyIfUrlIsNull() { + // arrange + String issUrl = null; + + // act + Optional tenantFromIssUrl = UrlHelper.extractTenantFromIssUrl(issUrl); // assert - assertEquals(issUrl, tenantFromIssUrl); + assertTrue(tenantFromIssUrl.isEmpty()); } }