Repository: SAP/cloud-security-xsuaa-integration
Pull request #299 (2020-06-03 15:52:29)

Simplifies Token Logging
- would have expected a generic `toString` Method to express the decoded Token...
- as we required that in context of the token-client lib, i did that as part of `DecodedJwt` implementation (`toString` method). 
- Consequently, in case the application developer likes to log the returned OAuth2TokenResponse, this is implemented also in the `toString` method


Patch:
diff --git a/token-client/src/main/java/com/sap/cloud/security/xsuaa/client/AbstractOAuth2TokenService.java b/token-client/src/main/java/com/sap/cloud/security/xsuaa/client/AbstractOAuth2TokenService.java
index 3667cdf4..c0536ff6 100644
--- a/token-client/src/main/java/com/sap/cloud/security/xsuaa/client/AbstractOAuth2TokenService.java
+++ b/token-client/src/main/java/com/sap/cloud/security/xsuaa/client/AbstractOAuth2TokenService.java
@@ -6,9 +6,9 @@ import com.github.benmanes.caffeine.cache.Ticker;
 import com.sap.cloud.security.xsuaa.Assertions;
 import com.sap.cloud.security.xsuaa.http.HttpHeaders;
 import com.sap.cloud.security.xsuaa.http.HttpHeadersFactory;
+import com.sap.cloud.security.xsuaa.jwt.DecodedJwt;
 import com.sap.cloud.security.xsuaa.tokenflows.TokenCacheConfiguration;
 import com.sap.cloud.security.xsuaa.tokenflows.Cacheable;
-import com.sap.cloud.security.xsuaa.util.TokenLogger;
 import com.sap.cloud.security.xsuaa.util.UriUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -31,7 +31,6 @@ public abstract class AbstractOAuth2TokenService implements OAuth2TokenService,
 	private static final Logger LOGGER = LoggerFactory.getLogger(AbstractOAuth2TokenService.class);
 	private final Cache<CacheKey, OAuth2TokenResponse> responseCache;
 	private final TokenCacheConfiguration tokenCacheConfiguration;
-	private final TokenLogger tokenLogger;
 
 	public AbstractOAuth2TokenService() {
 		this(TokenCacheConfiguration.defaultConfiguration(), Ticker.systemTicker(), false);
@@ -65,7 +64,6 @@ public abstract class AbstractOAuth2TokenService implements OAuth2TokenService,
 		Assertions.assertNotNull(tokenCacheConfiguration, "cacheConfiguration is required");
 		this.tokenCacheConfiguration = tokenCacheConfiguration;
 		this.responseCache = createResponseCache(cacheTicker, sameThreadCache);
-		this.tokenLogger = TokenLogger.getInstance(LOGGER);
 		if (isCacheDisabled()) {
 			LOGGER.debug("Configured token service with cache disabled");
 		} else {
@@ -243,9 +241,14 @@ public abstract class AbstractOAuth2TokenService implements OAuth2TokenService,
 	}
 
 	private void logDebug(OAuth2TokenResponse response) {
-		String accessToken = response.getAccessToken();
-		if (accessToken != null) {
-			tokenLogger.logDebug(accessToken, "Access token:");
+		if(!LOGGER.isDebugEnabled()){
+			return;
+		}
+		try {
+			DecodedJwt decodedJwt = response.getDecodedAccessToken();
+			LOGGER.debug("Access token: {}", decodedJwt);
+		} catch (IllegalArgumentException e) {
+			LOGGER.debug("Access token can not be logged. {}", e.getMessage());
 		}
 	}
 
diff --git a/token-client/src/main/java/com/sap/cloud/security/xsuaa/client/OAuth2TokenResponse.java b/token-client/src/main/java/com/sap/cloud/security/xsuaa/client/OAuth2TokenResponse.java
index 8158c8f3..9f0c6df6 100644
--- a/token-client/src/main/java/com/sap/cloud/security/xsuaa/client/OAuth2TokenResponse.java
+++ b/token-client/src/main/java/com/sap/cloud/security/xsuaa/client/OAuth2TokenResponse.java
@@ -79,6 +79,12 @@ public class OAuth2TokenResponse {
 
 	@Override
 	public String toString() {
-		return getAccessToken() != null ? getAccessToken() : super.toString();
+		DecodedJwt decodedJwt = null;
+		try {
+			decodedJwt = getDecodedAccessToken();
+		} catch (IllegalArgumentException e) {
+			return super.toString();
+		}
+		return decodedJwt.toString();
 	}
 }
diff --git a/token-client/src/main/java/com/sap/cloud/security/xsuaa/jwt/Base64JwtDecoder.java b/token-client/src/main/java/com/sap/cloud/security/xsuaa/jwt/Base64JwtDecoder.java
index 16d5edc7..18599cd0 100644
--- a/token-client/src/main/java/com/sap/cloud/security/xsuaa/jwt/Base64JwtDecoder.java
+++ b/token-client/src/main/java/com/sap/cloud/security/xsuaa/jwt/Base64JwtDecoder.java
@@ -1,11 +1,15 @@
 package com.sap.cloud.security.xsuaa.jwt;
 
 import com.sap.cloud.security.xsuaa.Assertions;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.nio.charset.StandardCharsets;
 import java.util.Base64;
 import java.util.regex.Pattern;
 
+import static java.lang.System.lineSeparator;
+
 public final class Base64JwtDecoder {
 	private static final Base64JwtDecoder instance = new Base64JwtDecoder();
 
@@ -54,6 +58,8 @@ public final class Base64JwtDecoder {
 		private String payload;
 		private String signature;
 		private String encodedJwt;
+		private static final String TAB = "\t";
+		private static final Logger LOGGER = LoggerFactory.getLogger(DecodedJwtImpl.class);
 
 		DecodedJwtImpl(String encodedJwt, String header, String payload, String signature) {
 			this.header = header;
@@ -82,5 +88,15 @@ public final class Base64JwtDecoder {
 			return encodedJwt;
 		}
 
+		@Override
+		public String toString() {
+			StringBuilder stringBuilder = new StringBuilder();
+			stringBuilder.append("Jwt header" + lineSeparator());
+			stringBuilder.append(TAB + getHeader() + lineSeparator());
+			stringBuilder.append("Jwt payload" + lineSeparator());
+			stringBuilder.append(TAB + getPayload() + lineSeparator());
+
+			return stringBuilder.toString();
+		}
 	}
 }
diff --git a/token-client/src/main/java/com/sap/cloud/security/xsuaa/jwt/DecodedJwt.java b/token-client/src/main/java/com/sap/cloud/security/xsuaa/jwt/DecodedJwt.java
index eff629aa..ef040f34 100644
--- a/token-client/src/main/java/com/sap/cloud/security/xsuaa/jwt/DecodedJwt.java
+++ b/token-client/src/main/java/com/sap/cloud/security/xsuaa/jwt/DecodedJwt.java
@@ -3,6 +3,8 @@ package com.sap.cloud.security.xsuaa.jwt;
 /**
  * A Jwt token consists of three parts, separated by ".":
  * header.payload.signature
+ *
+ * Use {@code Base64JwtDecoder.getInstance().decode(token)} to get a {@link DecodedJwt} instance.
  */
 
 public interface DecodedJwt {
diff --git a/token-client/src/main/java/com/sap/cloud/security/xsuaa/util/TokenLogger.java b/token-client/src/main/java/com/sap/cloud/security/xsuaa/util/TokenLogger.java
deleted file mode 100644
index c14d07a5..00000000
--- a/token-client/src/main/java/com/sap/cloud/security/xsuaa/util/TokenLogger.java
+++ /dev/null
@@ -1,90 +0,0 @@
-package com.sap.cloud.security.xsuaa.util;
-
-import com.sap.cloud.security.xsuaa.Assertions;
-import com.sap.cloud.security.xsuaa.jwt.Base64JwtDecoder;
-import com.sap.cloud.security.xsuaa.jwt.DecodedJwt;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import javax.annotation.Nullable;
-
-import static java.lang.System.*;
-
-/**
- * Logs the decoded jwt token.
- */
-public class TokenLogger {
-
-	private static final String TAB = "\t";
-	private static final Logger INTERNAL_LOGGER = LoggerFactory.getLogger(TokenLogger.class);
-
-	private final Logger wrappedLogger;
-
-	/**
-	 * Creates a new instance for the given {@code logger}.
-	 * 
-	 * @param logger
-	 *            the logger that is used to log the token
-	 * @return a new instance
-	 */
-	public static TokenLogger getInstance(Logger logger) {
-		return new TokenLogger(logger);
-	}
-
-	private TokenLogger(Logger logger) {
-		Assertions.assertNotNull(logger, "Logger must be provided!");
-		this.wrappedLogger = logger;
-	}
-
-	/**
-	 * Decodes the given {@code token} string using
-	 * {@link #convertToReadableFormat(String)} and logs the payload and header
-	 * content on debug logging level with the provided logger. A description can be
-	 * provided that will be logged alongside the token.
-	 *
-	 * @param token
-	 *            the encoded jwt token string
-	 * @param description
-	 *            an optional textual description of the token being logged
-	 */
-	public void logDebug(String token, @Nullable String description) {
-		if (!wrappedLogger.isDebugEnabled()) {
-			return;
-		}
-		String convertedToken = convertToReadableFormat(token);
-		if (convertedToken.isEmpty()) {
-			return;
-		}
-		if (description != null) {
-			wrappedLogger.debug(description + lineSeparator() + convertedToken);
-		} else {
-			wrappedLogger.debug(convertedToken);
-		}
-	}
-
-	/**
-	 * Utility method to transform a encoded token to a human readable format while
-	 * omitting the token signature. This can be used to inspect the token for
-	 * debugging purposes. This method does not log the token! Use
-	 * {@link #logDebug(String, String)} for this. If the token is malformed or
-	 * null, this method will return the empty string and not throw exceptions.
-	 *
-	 * @param token
-	 *            the jwt string
-	 * @return readable format of the token
-	 */
-	public static String convertToReadableFormat(String token) {
-		StringBuilder stringBuilder = new StringBuilder();
-		try {
-			DecodedJwt decodedJwt = Base64JwtDecoder.getInstance().decode(token);
-			stringBuilder.append("Jwt header" + lineSeparator());
-			stringBuilder.append(TAB + decodedJwt.getHeader() + lineSeparator());
-			stringBuilder.append("Jwt payload" + lineSeparator());
-			stringBuilder.append(TAB + decodedJwt.getPayload() + lineSeparator());
-		} catch (Exception e) {
-			INTERNAL_LOGGER.debug("Could not convert token to readable format:", e.getMessage());
-			return "";
-		}
-		return stringBuilder.toString();
-	}
-}


Test patch:
diff --git a/token-client/src/test/java/com/sap/cloud/security/xsuaa/jwt/Base64JwtDecoderTest.java b/token-client/src/test/java/com/sap/cloud/security/xsuaa/jwt/Base64JwtDecoderTest.java
index fcddaa70..9ae21830 100644
--- a/token-client/src/test/java/com/sap/cloud/security/xsuaa/jwt/Base64JwtDecoderTest.java
+++ b/token-client/src/test/java/com/sap/cloud/security/xsuaa/jwt/Base64JwtDecoderTest.java
@@ -1,16 +1,27 @@
 package com.sap.cloud.security.xsuaa.jwt;
 
+import com.sap.cloud.security.xsuaa.test.JwtGenerator;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
+import org.springframework.security.oauth2.jwt.Jwt;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
 
 public class Base64JwtDecoderTest {
 
 	private String encodedJwt = "eyJhbGciOiJIUzI1NiIsImprdSI6Imh0dHBzOi8vYWNtZS1lbnRlcnByaXNlcy5hdXRoZW50aWNhdGlvbi5leGFtcGxlLmNvbS90b2tlbl9rZXlzIiwia2lkIjoia2V5LWlkLTEiLCJ0eXAiOiJKV1QifQ.eyJqdGkiOiJlM2MzMGUyNDc0Y2Q0NjYwOWEyNjJlZGE5ZDlkYzI2ZCIsImV4dF9hdHRyIjp7ImVuaGFuY2VyIjoiWFNVQUEiLCJ6ZG4iOiJhY21lLWVudGVycHJpc2VzIn0sInhzLnN5c3RlbS5hdHRyaWJ1dGVzIjp7InhzLnJvbGVjb2xsZWN0aW9ucyI6W119LCJnaXZlbl9uYW1lIjoiQW5kcmVhIE1hcmlhIiwieHMudXNlci5hdHRyaWJ1dGVzIjp7fSwiZmFtaWx5X25hbWUiOiJNaWxsc2FwIiwic3ViIjoiMTIzNCIsInNjb3BlIjpbIm9wZW5pZCIsInVhYS51c2VyIl0sImNsaWVudF9pZCI6Im15LWFwcDEiLCJjaWQiOiJteS1hcHAxIiwiYXpwIjoibXktYXBwMSIsImdyYW50X3R5cGUiOiJhdXRob3JpemF0aW9uX2NvZGUiLCJ1c2VyX2lkIjoiMTIzNCIsIm9yaWdpbiI6ImxkYXAiLCJ1c2VyX25hbWUiOiJhbS5taWxsc2FwQHNhcC5jb20iLCJlbWFpbCI6ImFtLm1pbGxzYXBAc2FwLmNvbSIsImF1dGhfdGltZSI6MTU2NDc4NTcyMCwicmV2X3NpZyI6Ijg3ZTdjMDE2IiwiaWF0IjoxNTY0Nzg1NzIwLCJleHAiOjE1NjQ3ODU3MjEsImlzcyI6Imh0dHA6Ly9hY21lLWVudGVycHJpc2VzLmV4YW1wbGUuY29tL3VhYS9vYXV0aC90b2tlbiIsInppZCI6IjIzNDUiLCJhdWQiOlsibXktYXBwMSIsInVhYSIsIm9wZW5pZCJdfQ.yqEcFR3EkzVSfVo3tfxsl9kc6KtCSe75-al5cTZbzhk";
 
+	private static final String JKU_HEADER = "http://my.jku/token_keys";
+	private static final String CLIENT_ID = "clientId123";
+	private static final String[] SCOPES = { "scope1", "scope2" };
+
+	private final static Jwt TOKEN = new JwtGenerator(CLIENT_ID)
+			.addScopes(SCOPES)
+			.setJku(JKU_HEADER)
+			.getToken();
+
 	@Rule
 	public ExpectedException expectedException = ExpectedException.none();
 
@@ -45,4 +56,40 @@ public class Base64JwtDecoderTest {
 		DecodedJwt decodedJwt = new Base64JwtDecoder().decode("header..signature");
 		assertEquals("", decodedJwt.getPayload());
 	}
+
+	@Test
+	public void toStringReturnsHeadersAndPayload() {
+		DecodedJwt decodedJwt = Base64JwtDecoder.getInstance().decode(TOKEN.getTokenValue());
+		assertThat(decodedJwt.toString())
+				.contains("Jwt header")
+				.contains(JKU_HEADER)
+				.contains("Jwt payload")
+				.contains(CLIENT_ID)
+				.contains(SCOPES);
+	}
+
+	@Test
+	public void toStringWithInvalidTokenReturnsEmptyString() {
+		DecodedJwt decodedJwt = Base64JwtDecoder.getInstance().decode("header..signature");
+
+		assertThat(decodedJwt.toString())
+				.contains("Jwt header")
+				.contains("Jwt payload");
+
+		decodedJwt = Base64JwtDecoder.getInstance().decode("..signature");
+
+		assertThat(decodedJwt.toString())
+				.contains("Jwt header")
+				.contains("Jwt payload");
+	}
+
+	@Test
+	public void toStringDoesNotContainSignatureNorEncodedToken() {
+		DecodedJwt decodedJwt = Base64JwtDecoder.getInstance().decode(TOKEN.getTokenValue());
+
+		assertThat(decodedJwt.toString())
+				.doesNotContain(decodedJwt.getSignature())
+				.doesNotContain(TOKEN.getTokenValue());
+	}
+
 }
diff --git a/token-client/src/test/java/com/sap/cloud/security/xsuaa/util/TokenLoggerTest.java b/token-client/src/test/java/com/sap/cloud/security/xsuaa/util/TokenLoggerTest.java
deleted file mode 100644
index d9bede0b..00000000
--- a/token-client/src/test/java/com/sap/cloud/security/xsuaa/util/TokenLoggerTest.java
+++ /dev/null
@@ -1,104 +0,0 @@
-package com.sap.cloud.security.xsuaa.util;
-
-import com.sap.cloud.security.xsuaa.jwt.Base64JwtDecoder;
-import com.sap.cloud.security.xsuaa.jwt.DecodedJwt;
-import com.sap.cloud.security.xsuaa.test.JwtGenerator;
-import org.junit.Before;
-import org.junit.Test;
-import org.mockito.ArgumentCaptor;
-import org.slf4j.Logger;
-
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.*;
-import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.when;
-
-public class TokenLoggerTest {
-
-	private static final String JKU_HEADER = "http://my.jku/token_keys";
-	private static final String CLIENT_ID = "clientId123";
-	private static final String[] SCOPES = { "scope1", "scope2" };
-
-	private final static String TOKEN = new JwtGenerator(CLIENT_ID)
-			.addScopes(SCOPES)
-			.setJku(JKU_HEADER)
-			.getToken().getTokenValue();
-	private static final DecodedJwt DECODED_JWT = Base64JwtDecoder.getInstance().decode(TOKEN);
-
-	private TokenLogger cut;
-	private Logger loggerMock;
-
-	@Before
-	public void setUp() {
-		loggerMock = mock(Logger.class);
-		when(loggerMock.isDebugEnabled()).thenReturn(true);
-		cut = TokenLogger.getInstance(loggerMock);
-	}
-
-	@Test
-	public void convertToReadableFormat_displaysRelevantData() {
-		String logEntry = TokenLogger.convertToReadableFormat(TOKEN);
-		assertThat(logEntry)
-				.contains("Jwt header")
-				.contains(JKU_HEADER)
-				.contains("Jwt payload")
-				.contains(CLIENT_ID)
-				.contains(SCOPES);
-	}
-
-	@Test
-	public void convertToReadableFormat_doesNotContainSignatureNorEncodedToken() {
-		String logEntry = TokenLogger.convertToReadableFormat(TOKEN);
-		assertThat(logEntry)
-				.doesNotContain(DECODED_JWT.getSignature())
-				.doesNotContain(TOKEN);
-	}
-
-	@Test
-	public void convertToReadableFormat_inputIsNotAToken_returnsEmptyString() {
-		assertThat(TokenLogger.convertToReadableFormat("")).isEmpty();
-		assertThat(TokenLogger.convertToReadableFormat("not.a.token")).isEmpty();
-		assertThat(TokenLogger.convertToReadableFormat(null)).isEmpty();
-	}
-
-	@Test
-	public void logToken_containsDescriptionAndConvertedToken() {
-		ArgumentCaptor<String> debugLogCaptor = ArgumentCaptor.forClass(String.class);
-		cut.logDebug(TOKEN, "useful description");
-
-		verify(loggerMock, times(1)).debug(debugLogCaptor.capture());
-
-		assertThat(debugLogCaptor.getValue())
-				.contains("useful description")
-				.contains(TokenLogger.convertToReadableFormat(TOKEN));
-	}
-
-	@Test
-	public void logToken_descriptionIsNull_logsTokenOnly() {
-		ArgumentCaptor<String> debugLogCaptor = ArgumentCaptor.forClass(String.class);
-		cut.logDebug(TOKEN, null);
-
-		verify(loggerMock, times(1)).debug(debugLogCaptor.capture());
-		assertThat(debugLogCaptor.getValue()).isEqualTo(TokenLogger.convertToReadableFormat(TOKEN));
-	}
-
-	@Test
-	public void logToken_doesNotLogSignatureNorEncodedToken() {
-		ArgumentCaptor<String> debugLogCaptor = ArgumentCaptor.forClass(String.class);
-		cut.logDebug(TOKEN, "");
-
-		verify(loggerMock, times(1)).debug(debugLogCaptor.capture());
-
-		assertThat(debugLogCaptor.getValue())
-				.doesNotContain(TOKEN)
-				.doesNotContain(DECODED_JWT.getSignature());
-	}
-
-	@Test
-	public void logToken_tokenIsNotDecodable_doesNotLog() {
-		cut.logDebug("not.a.token", "useful description");
-
-		verify(loggerMock, times(1)).isDebugEnabled();
-		verifyNoMoreInteractions(loggerMock);
-	}
-}
\ No newline at end of file