From 6320cbfcd3edf3c50e0e6921ed8da6c0a4ddbac0 Mon Sep 17 00:00:00 2001 From: Nena Raab Date: Tue, 2 Jan 2024 16:40:32 +0100 Subject: [PATCH 1/3] Avoid warn messages in case there is no service configuration --- java-security/pom.xml | 10 ++-- .../security/servlet/HybridTokenFactory.java | 27 ++++++----- .../servlet/HybridTokenFactoryTest.java | 48 +++++++++++++++++++ 3 files changed, 67 insertions(+), 18 deletions(-) create mode 100644 java-security/src/test/java/com/sap/cloud/security/servlet/HybridTokenFactoryTest.java diff --git a/java-security/pom.xml b/java-security/pom.xml index 0cf0be0f9..5a4cc286e 100644 --- a/java-security/pom.xml +++ b/java-security/pom.xml @@ -114,11 +114,6 @@ 1.19.0 test - - org.slf4j - slf4j-simple - test - com.github.stefanbirkner system-lambda @@ -148,6 +143,11 @@ spring-context test + + ch.qos.logback + logback-classic + test + diff --git a/java-security/src/main/java/com/sap/cloud/security/servlet/HybridTokenFactory.java b/java-security/src/main/java/com/sap/cloud/security/servlet/HybridTokenFactory.java index 07931a6b4..f43cccab2 100644 --- a/java-security/src/main/java/com/sap/cloud/security/servlet/HybridTokenFactory.java +++ b/java-security/src/main/java/com/sap/cloud/security/servlet/HybridTokenFactory.java @@ -18,6 +18,7 @@ import javax.annotation.Nonnull; import java.util.Objects; +import java.util.Optional; import java.util.regex.Pattern; import static com.sap.cloud.security.token.TokenClaims.XSUAA.EXTERNAL_ATTRIBUTE; @@ -31,7 +32,7 @@ public class HybridTokenFactory implements TokenFactory { private static final Logger LOGGER = LoggerFactory.getLogger(HybridTokenFactory.class); - private static String xsAppId; + private static Optional xsAppId; private static ScopeConverter xsScopeConverter; /** @@ -66,27 +67,27 @@ public Token create(String jwtToken) { */ static void withXsuaaAppId(@Nonnull String xsAppId) { LOGGER.debug("XSUAA app id = {}", xsAppId); - HybridTokenFactory.xsAppId = xsAppId; + HybridTokenFactory.xsAppId = Optional.of(xsAppId); getOrCreateScopeConverter(); } private static ScopeConverter getOrCreateScopeConverter() { - if (xsScopeConverter == null && getXsAppId() != null) { - xsScopeConverter = new XsuaaScopeConverter(getXsAppId()); + if (xsScopeConverter == null && getXsAppId().isPresent()) { + xsScopeConverter = new XsuaaScopeConverter(getXsAppId().get()); } return xsScopeConverter; } - private static String getXsAppId() { - if (xsAppId == null) { - OAuth2ServiceConfiguration serviceConfiguration = Environments.getCurrent().getXsuaaConfiguration(); - if (serviceConfiguration == null) { - LOGGER.warn("There is no xsuaa service configuration: no local scope check possible."); - } else { - xsAppId = serviceConfiguration.getProperty(CFConstants.XSUAA.APP_ID); - } + private static Optional getXsAppId() { + if (Objects.nonNull(xsAppId)) { + return xsAppId; + } + OAuth2ServiceConfiguration serviceConfiguration = Environments.getCurrent().getXsuaaConfiguration(); + if (serviceConfiguration != null) { + return xsAppId = Optional.of(serviceConfiguration.getProperty(CFConstants.XSUAA.APP_ID)); } - return xsAppId; + LOGGER.warn("There is no xsuaa service configuration with 'xsappname' property: no local scope check possible."); + return xsAppId = Optional.empty(); } /** diff --git a/java-security/src/test/java/com/sap/cloud/security/servlet/HybridTokenFactoryTest.java b/java-security/src/test/java/com/sap/cloud/security/servlet/HybridTokenFactoryTest.java new file mode 100644 index 000000000..48fa0e91f --- /dev/null +++ b/java-security/src/test/java/com/sap/cloud/security/servlet/HybridTokenFactoryTest.java @@ -0,0 +1,48 @@ +package com.sap.cloud.security.servlet; + +import ch.qos.logback.core.read.ListAppender; +import com.sap.cloud.security.token.XsuaaToken; +import org.apache.commons.io.IOUtils; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.classic.Logger; + +import java.io.IOException; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.*; + +class HybridTokenFactoryTest { + + private ListAppender logWatcher; + private HybridTokenFactory cut; + + @BeforeEach + public void setup() { + cut = new HybridTokenFactory(); + logWatcher = new ListAppender<>(); + logWatcher.start(); + ((Logger) LoggerFactory.getLogger(HybridTokenFactory.class)).addAppender(logWatcher); + } + + @AfterEach + void teardown() { + ((Logger) LoggerFactory.getLogger(HybridTokenFactory.class)).detachAndStopAllAppenders(); + } + + @Test + void oneWarningMessageIncaseSecurityConfigIsMissing() throws IOException { + String jwt = IOUtils.resourceToString("/xsuaaUserAccessTokenRSA256.txt", UTF_8); + XsuaaToken token = (XsuaaToken) cut.create(jwt); + cut.create(jwt); + + assertThat(token.getIssuer()).isEqualTo("http://auth.com"); + assertThrows(IllegalArgumentException.class, () -> token.hasLocalScope("abc")); + assertThat(logWatcher.list).isNotNull().hasSize(1); + assertThat(logWatcher.list.get(0).getMessage()).contains("There is no xsuaa service configuration"); + } +} \ No newline at end of file From 303a81d43ec8b5d50af909adc34c6f53fcb7abb2 Mon Sep 17 00:00:00 2001 From: Nena Raab Date: Wed, 3 Jan 2024 12:40:05 +0100 Subject: [PATCH 2/3] fix fragile test --- .../sap/cloud/security/servlet/HybridTokenFactory.java | 4 ++-- .../cloud/security/servlet/HybridTokenFactoryTest.java | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/java-security/src/main/java/com/sap/cloud/security/servlet/HybridTokenFactory.java b/java-security/src/main/java/com/sap/cloud/security/servlet/HybridTokenFactory.java index f43cccab2..9cf0edcdf 100644 --- a/java-security/src/main/java/com/sap/cloud/security/servlet/HybridTokenFactory.java +++ b/java-security/src/main/java/com/sap/cloud/security/servlet/HybridTokenFactory.java @@ -32,8 +32,8 @@ public class HybridTokenFactory implements TokenFactory { private static final Logger LOGGER = LoggerFactory.getLogger(HybridTokenFactory.class); - private static Optional xsAppId; - private static ScopeConverter xsScopeConverter; + protected static Optional xsAppId; + protected static ScopeConverter xsScopeConverter; /** * Determines whether the JWT token is issued by XSUAA or IAS identity service, diff --git a/java-security/src/test/java/com/sap/cloud/security/servlet/HybridTokenFactoryTest.java b/java-security/src/test/java/com/sap/cloud/security/servlet/HybridTokenFactoryTest.java index 48fa0e91f..6bf5f77d9 100644 --- a/java-security/src/test/java/com/sap/cloud/security/servlet/HybridTokenFactoryTest.java +++ b/java-security/src/test/java/com/sap/cloud/security/servlet/HybridTokenFactoryTest.java @@ -24,9 +24,11 @@ class HybridTokenFactoryTest { @BeforeEach public void setup() { cut = new HybridTokenFactory(); - logWatcher = new ListAppender<>(); - logWatcher.start(); - ((Logger) LoggerFactory.getLogger(HybridTokenFactory.class)).addAppender(logWatcher); + cut.xsAppId = null; + cut.xsScopeConverter = null; + logWatcher = new ListAppender<>(); + logWatcher.start(); + ((Logger) LoggerFactory.getLogger(HybridTokenFactory.class)).addAppender(logWatcher); } @AfterEach From 32539e709c8ba4e472aa2a21ed94c7688d062e27 Mon Sep 17 00:00:00 2001 From: liga-oz Date: Thu, 4 Jan 2024 09:27:15 +0100 Subject: [PATCH 3/3] make the class fields more restrictive Signed-off-by: liga-oz --- .../cloud/security/servlet/HybridTokenFactory.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/java-security/src/main/java/com/sap/cloud/security/servlet/HybridTokenFactory.java b/java-security/src/main/java/com/sap/cloud/security/servlet/HybridTokenFactory.java index 9cf0edcdf..2f43fa637 100644 --- a/java-security/src/main/java/com/sap/cloud/security/servlet/HybridTokenFactory.java +++ b/java-security/src/main/java/com/sap/cloud/security/servlet/HybridTokenFactory.java @@ -32,8 +32,8 @@ public class HybridTokenFactory implements TokenFactory { private static final Logger LOGGER = LoggerFactory.getLogger(HybridTokenFactory.class); - protected static Optional xsAppId; - protected static ScopeConverter xsScopeConverter; + static Optional xsAppId; + static ScopeConverter xsScopeConverter; /** * Determines whether the JWT token is issued by XSUAA or IAS identity service, @@ -84,10 +84,13 @@ private static Optional getXsAppId() { } OAuth2ServiceConfiguration serviceConfiguration = Environments.getCurrent().getXsuaaConfiguration(); if (serviceConfiguration != null) { - return xsAppId = Optional.of(serviceConfiguration.getProperty(CFConstants.XSUAA.APP_ID)); + xsAppId = Optional.of(serviceConfiguration.getProperty(CFConstants.XSUAA.APP_ID)); + } else { + LOGGER.warn( + "There is no xsuaa service configuration with 'xsappname' property: no local scope check possible."); + xsAppId = Optional.empty(); } - LOGGER.warn("There is no xsuaa service configuration with 'xsappname' property: no local scope check possible."); - return xsAppId = Optional.empty(); + return xsAppId; } /**