From a9acb1b915370a81b7298f4fedacd4aec7afa1fc Mon Sep 17 00:00:00 2001 From: anhefti Date: Mon, 21 Feb 2022 13:50:13 +0100 Subject: [PATCH] More Unit Tests and better logging --- .../ch/ethz/seb/sebserver/gbl/Constants.java | 2 + .../ch/ethz/seb/sebserver/gbl/util/Utils.java | 41 ++++- .../gui/content/exam/ExamFormIndicators.java | 2 +- .../gui/service/session/ColorData.java | 2 +- .../gui/service/session/IndicatorData.java | 4 +- .../sebserver/gui/widget/ColorSelection.java | 2 +- .../session/ExamSessionService.java | 6 + .../oauth/CachableJdbcTokenStore.java | 37 +++- .../oauth/DefaultTokenServicesFallback.java | 1 - .../config/application-dev.properties | 1 + .../seb/sebserver/gbl/util/UtilsTest.java | 171 +++++++++++++++++- 11 files changed, 250 insertions(+), 19 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/Constants.java b/src/main/java/ch/ethz/seb/sebserver/gbl/Constants.java index c6766074..9a35fbd7 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gbl/Constants.java +++ b/src/main/java/ch/ethz/seb/sebserver/gbl/Constants.java @@ -68,6 +68,7 @@ public final class Constants { public static final Character EQUALITY_SIGN = '='; public static final Character LIST_SEPARATOR_CHAR = COMMA; public static final Character COMPLEX_VALUE_SEPARATOR = COLON; + public static final Character HASH_TAG = '#'; public static final String NULL = "null"; public static final String PERCENTAGE_STRING = Constants.PERCENTAGE.toString(); @@ -80,6 +81,7 @@ public final class Constants { public static final String URL_PORT_SEPARATOR = COLON.toString(); public static final String URL_ADDRESS_SEPARATOR = COLON.toString() + SLASH.toString() + SLASH.toString(); public static final String URL_PATH_SEPARATOR = SLASH.toString(); + public static final String HASH_TAG_STRING = HASH_TAG.toString(); public static final String DYN_HTML_ATTR_OPEN = "%%_"; public static final String DYN_HTML_ATTR_CLOSE = "_%%"; diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/util/Utils.java b/src/main/java/ch/ethz/seb/sebserver/gbl/util/Utils.java index f9c9a20e..5032e6b2 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gbl/util/Utils.java +++ b/src/main/java/ch/ethz/seb/sebserver/gbl/util/Utils.java @@ -34,6 +34,8 @@ import java.util.function.Predicate; import java.util.stream.Collector; import java.util.stream.Collectors; +import javax.validation.constraints.NotNull; + import org.apache.commons.codec.binary.Hex; import org.apache.commons.lang3.StringUtils; import org.apache.commons.text.StringEscapeUtils; @@ -540,10 +542,15 @@ public final class Utils { public static RGB toRGB(final String rgbString) { if (StringUtils.isNotBlank(rgbString)) { + + final String rgbVal = (rgbString.startsWith(Constants.HASH_TAG_STRING)) + ? rgbString.substring(1) + : rgbString; + return new RGB( - Integer.parseInt(rgbString.substring(0, 2), 16), - Integer.parseInt(rgbString.substring(2, 4), 16), - Integer.parseInt(rgbString.substring(4, 6), 16)); + Integer.parseInt(rgbVal.substring(0, 2), 16), + Integer.parseInt(rgbVal.substring(2, 4), 16), + Integer.parseInt(rgbVal.substring(4, 6), 16)); } else { return new RGB(255, 255, 255); } @@ -564,7 +571,11 @@ public final class Utils { return e.getCause().getClass().getName() + " : " + e.getCause().getMessage(); } - public static boolean darkColor(final RGB rgb) { + /** Indicates if a dark background or contrast color must be used for the given text or foreground color. + * + * @param rgb foreground or text color + * @return true of the background color for given foreground color shall be dark or false if it shall be light */ + public static boolean darkColorContrast(final RGB rgb) { return rgb.red + rgb.green + rgb.blue > DARK_COLOR_THRESHOLD; } @@ -596,6 +607,10 @@ public final class Utils { } public static String toAppFormUrlEncodedBody(final MultiValueMap attributes) { + if (attributes == null) { + return StringUtils.EMPTY; + } + return attributes .entrySet() .stream() @@ -619,7 +634,11 @@ public final class Utils { .toString(); } - public static String toAppFormUrlEncodedBody(final String name, final Collection array) { + public static String toAppFormUrlEncodedBody(@NotNull final String name, final Collection array) { + if (array == null) { + return StringUtils.EMPTY; + } + final String _name = name.contains(String.valueOf(Constants.SQUARE_BRACE_OPEN)) || array.size() <= 1 ? name : name + Constants.SQUARE_BRACE_OPEN + Constants.SQUARE_BRACE_CLOSE; @@ -704,4 +723,16 @@ public final class Utils { return value; } + public static StringBuilder formatStackTracePrint(final int length, final StackTraceElement[] stackTrace) { + final int size = Math.min(stackTrace.length, length); + final StringBuilder builder = new StringBuilder(); + for (int i = 0; i < size; i++) { + builder.append(" --> ").append(stackTrace[i].toString()); + if (i + 1 < size) { + builder.append(Constants.CARRIAGE_RETURN); + } + } + return builder; + } + } diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/content/exam/ExamFormIndicators.java b/src/main/java/ch/ethz/seb/sebserver/gui/content/exam/ExamFormIndicators.java index 395fddc8..650be970 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/content/exam/ExamFormIndicators.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/content/exam/ExamFormIndicators.java @@ -186,7 +186,7 @@ public class ExamFormIndicators implements TemplateComposer { .append("") diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/session/ColorData.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/session/ColorData.java index 8285f951..7a385566 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/session/ColorData.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/session/ColorData.java @@ -48,7 +48,7 @@ public class ColorData { } Color getStatusTextColor(final Color statusColor) { - return Utils.darkColor(statusColor.getRGB()) ? this.darkColor : this.lightColor; + return Utils.darkColorContrast(statusColor.getRGB()) ? this.darkColor : this.lightColor; } int statusWeight(final ClientConnectionData connectionData) { diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/session/IndicatorData.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/session/IndicatorData.java index e60572e6..48ebc705 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/session/IndicatorData.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/session/IndicatorData.java @@ -41,7 +41,7 @@ final class IndicatorData { this.index = index; this.tableIndex = tableIndex; this.defaultColor = new Color(display, Utils.toRGB(indicator.defaultColor), 255); - this.defaultTextColor = Utils.darkColor(this.defaultColor.getRGB()) + this.defaultTextColor = Utils.darkColorContrast(this.defaultColor.getRGB()) ? colorData.darkColor : colorData.lightColor; @@ -91,7 +91,7 @@ final class IndicatorData { protected ThresholdColor(final Threshold threshold, final Display display, final ColorData colorData) { this.value = threshold.value; this.color = new Color(display, Utils.toRGB(threshold.color), 255); - this.textColor = Utils.darkColor(this.color.getRGB()) + this.textColor = Utils.darkColorContrast(this.color.getRGB()) ? colorData.darkColor : colorData.lightColor; } diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/widget/ColorSelection.java b/src/main/java/ch/ethz/seb/sebserver/gui/widget/ColorSelection.java index 69a642fe..b221a4ae 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/widget/ColorSelection.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/widget/ColorSelection.java @@ -161,7 +161,7 @@ public final class ColorSelection extends Composite implements Selection { if (this.selection != null) { this.colorField.setBackground(new Color(this.getDisplay(), this.selection)); this.colorLabel.setText(Utils.parseColorString(this.selection)); - this.colorLabel.setData(RWT.CUSTOM_VARIANT, (Utils.darkColor(this.selection)) + this.colorLabel.setData(RWT.CUSTOM_VARIANT, (Utils.darkColorContrast(this.selection)) ? CustomVariant.DARK_COLOR_LABEL.key : CustomVariant.LIGHT_COLOR_LABEL.key); } else { diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/ExamSessionService.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/ExamSessionService.java index 35384808..e66ebdaa 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/ExamSessionService.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/ExamSessionService.java @@ -190,6 +190,12 @@ public interface ExamSessionService { Result updateExamCache(Long examId); /** Flush all the caches for an specified Exam. + * + * 
+     *  - Exam
+     *  - Exam Configuration
+     *  - All ClientConnection of the running exam
+     *
* * @param exam The Exam instance * @return Result with reference to the given Exam or to an error if happened */ diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/CachableJdbcTokenStore.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/CachableJdbcTokenStore.java index 6f79081d..e8e61dae 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/CachableJdbcTokenStore.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/CachableJdbcTokenStore.java @@ -17,6 +17,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.cache.annotation.CacheEvict; import org.springframework.cache.annotation.Cacheable; +import org.springframework.cache.annotation.Caching; import org.springframework.security.oauth2.common.OAuth2AccessToken; import org.springframework.security.oauth2.common.OAuth2RefreshToken; import org.springframework.security.oauth2.provider.OAuth2Authentication; @@ -25,6 +26,8 @@ import org.springframework.security.oauth2.provider.token.TokenStore; import org.springframework.security.oauth2.provider.token.store.JdbcTokenStore; import org.springframework.transaction.annotation.Transactional; +import ch.ethz.seb.sebserver.gbl.util.Utils; + public class CachableJdbcTokenStore implements TokenStore { public static final String ACCESS_TOKEN_CACHE_NAME = "ACCESS_TOKEN_CACHE"; @@ -56,8 +59,9 @@ public class CachableJdbcTokenStore implements TokenStore { key = "#token", unless = "#result == null") public OAuth2Authentication readAuthentication(final OAuth2AccessToken token) { + if (log.isDebugEnabled()) { - log.debug("Read authentication from persistent and cache if available"); + log.debug("Read authentication from persistent and cache if available: {}", token.getValue()); } return this.jdbcTokenStore.readAuthentication(token); @@ -65,7 +69,17 @@ public class CachableJdbcTokenStore implements TokenStore { @Override public OAuth2Authentication readAuthentication(final String token) { - return this.jdbcTokenStore.readAuthentication(token); + final OAuth2Authentication authentication = this.jdbcTokenStore.readAuthentication(token); + + if (log.isDebugEnabled()) { + if (authentication == null) { + log.debug(Utils.formatStackTracePrint(10, Thread.currentThread().getStackTrace()).toString()); + } else { + log.debug("Read authentication from persistent: {}", token); + } + } + + return authentication; } @Override @@ -74,16 +88,27 @@ public class CachableJdbcTokenStore implements TokenStore { key = "#tokenValue", unless = "#result == null") public OAuth2AccessToken readAccessToken(final String tokenValue) { + + if (log.isDebugEnabled()) { + log.debug("Read access token from persistent and cache if available: {}", tokenValue); + } + return this.jdbcTokenStore.readAccessToken(tokenValue); } @Override - @CacheEvict( - cacheNames = AUTHENTICATION_TOKEN_CACHE, - key = "#token") + @Caching(evict = { + @CacheEvict( + cacheNames = AUTHENTICATION_TOKEN_CACHE, + key = "#token"), + @CacheEvict( + cacheNames = ACCESS_TOKEN_CACHE_NAME, + key = "#token.value") + }) public void removeAccessToken(final OAuth2AccessToken token) { + if (log.isDebugEnabled()) { - log.debug("Evict token from cache and remove it also from persistent store"); + log.debug("Evict token from cache and remove it also from persistent store: {}", token.getValue()); } this.jdbcTokenStore.removeAccessToken(token); diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/DefaultTokenServicesFallback.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/DefaultTokenServicesFallback.java index 4165faeb..74069bf5 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/DefaultTokenServicesFallback.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/DefaultTokenServicesFallback.java @@ -17,7 +17,6 @@ import org.springframework.security.oauth2.common.OAuth2AccessToken; import org.springframework.security.oauth2.provider.OAuth2Authentication; import org.springframework.security.oauth2.provider.token.DefaultTokenServices; -// TODO check if we can apply some caching here to get better performance for SEB client connection attempts public class DefaultTokenServicesFallback extends DefaultTokenServices { private static final Logger log = LoggerFactory.getLogger(DefaultTokenServicesFallback.class); diff --git a/src/main/resources/config/application-dev.properties b/src/main/resources/config/application-dev.properties index c9104516..e3848c27 100644 --- a/src/main/resources/config/application-dev.properties +++ b/src/main/resources/config/application-dev.properties @@ -15,6 +15,7 @@ logging.level.ch.ethz.seb.sebserver.webservice.servicelayer.lms.impl=INFO logging.level.ch.ethz.seb.sebserver.webservice.servicelayer.session=DEBUG logging.level.ch.ethz.seb.sebserver.webservice.servicelayer.session.impl.proctoring=INFO logging.level.ch.ethz.seb.sebserver.webservice.servicelayer.session.impl.indicator=DEBUG +logging.level.ch.ethz.seb.sebserver.webservice.weblayer.oauth=DEBUG #logging.level.ch.ethz.seb.sebserver.webservice.servicelayer.dao.impl=DEBUG #logging.level.ch.ethz.seb.sebserver.webservice.datalayer.batis=DEBUG #logging.level.ch.ethz.seb.sebserver.webservice.datalayer.batis.mapper=DEBUG diff --git a/src/test/java/ch/ethz/seb/sebserver/gbl/util/UtilsTest.java b/src/test/java/ch/ethz/seb/sebserver/gbl/util/UtilsTest.java index 3c0ae379..6bc7a523 100644 --- a/src/test/java/ch/ethz/seb/sebserver/gbl/util/UtilsTest.java +++ b/src/test/java/ch/ethz/seb/sebserver/gbl/util/UtilsTest.java @@ -8,14 +8,20 @@ package ch.ethz.seb.sebserver.gbl.util; -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.*; +import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.Set; +import org.apache.commons.lang3.StringUtils; import org.eclipse.swt.graphics.RGB; import org.joda.time.DateTimeUtils; import org.junit.Test; +import org.springframework.util.LinkedMultiValueMap; + +import ch.ethz.seb.sebserver.gbl.Constants; public class UtilsTest { @@ -47,6 +53,83 @@ public class UtilsTest { assertEquals("[ONE, TWO]", r5.toString()); } + @Test + public void testEscapeHTML_XML_EcmaScript() { + assertEquals("test", Utils.escapeHTML_XML_EcmaScript("test")); + assertEquals("test&lt;test&gt;", Utils.escapeHTML_XML_EcmaScript("test")); + assertEquals("test;test", Utils.escapeHTML_XML_EcmaScript("test;test")); + assertEquals("test-test", Utils.escapeHTML_XML_EcmaScript("test-test")); + assertEquals("test+test", Utils.escapeHTML_XML_EcmaScript("test+test")); + assertEquals("test&amp;test", Utils.escapeHTML_XML_EcmaScript("test&test")); + assertEquals("test[test]", Utils.escapeHTML_XML_EcmaScript("test[test]")); + } + + @Test + public void testPreventResponseSplittingAttack() { + assertEquals("test", Utils.preventResponseSplittingAttack("test")); + try { + Utils.preventResponseSplittingAttack("test\nergerg"); + } catch (final IllegalArgumentException e) { + assertEquals("Illegal argument: test\n" + + "ergerg", e.getMessage()); + } + + try { + Utils.preventResponseSplittingAttack("test\rergerg"); + } catch (final IllegalArgumentException e) { + assertEquals("Illegal argument: test\r" + + "ergerg", e.getMessage()); + } + } + + @Test + public void testHash_SHA_256_Base_16() { + assertEquals( + "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", + Utils.hash_SHA_256_Base_16("test")); + assertEquals( + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + Utils.hash_SHA_256_Base_16("")); + + assertNull(Utils.hash_SHA_256_Base_16(null)); + } + + @Test + public void testTruePredicate() { + assertTrue(Utils.truePredicate().test("some")); + } + + @Test + public void testFalsePredicate() { + assertFalse(Utils.falsePredicate().test("some")); + } + + @Test + public void testToSeconds() { + assertEquals("60", String.valueOf(Utils.toSeconds(Constants.MINUTE_IN_MILLIS))); + } + + @Test + public void testToRGB() { + final String rgbString1 = null; + final String rgbString2 = ""; + final String rgbString3 = "wrfgwr"; + final String rgbString4 = "#aabbcc"; + final String rgbString5 = "aabbcc"; + + assertEquals("RGB {255, 255, 255}", Utils.toRGB(rgbString1).toString()); + assertEquals("RGB {255, 255, 255}", Utils.toRGB(rgbString2).toString()); + try { + assertEquals("RGB {255, 255, 255}", Utils.toRGB(rgbString3).toString()); + fail("NumberFormatException expected here"); + } catch (final NumberFormatException e) { + assertEquals("For input string: \"wr\"", e.getMessage()); + } + assertEquals("RGB {170, 187, 204}", Utils.toRGB(rgbString4).toString()); + assertEquals("RGB {170, 187, 204}", Utils.toRGB(rgbString5).toString()); + + } + @Test public void testParseRGB() { String colorString = "FFFFFF"; @@ -60,11 +143,35 @@ public class UtilsTest { Utils.parseRGB(colorString).toString()); } + @Test + public void testGetErrorCauseMessage() { + assertEquals("--", Utils.getErrorCauseMessage(null)); + assertEquals("--", Utils.getErrorCauseMessage(new RuntimeException("origMessage"))); + assertEquals("java.lang.RuntimeException : null", + Utils.getErrorCauseMessage(new RuntimeException("origMessage", new RuntimeException()))); + assertEquals("java.lang.RuntimeException : causeMessage", + Utils.getErrorCauseMessage(new RuntimeException("origMessage", new RuntimeException("causeMessage")))); + } + + @Test + public void testDarkColor() { + final RGB color = new RGB(255, 255, 255); + final RGB color1 = new RGB(101, 100, 200); + final RGB color2 = new RGB(100, 100, 200); + final RGB color3 = new RGB(0, 0, 0); + + assertTrue(Utils.darkColorContrast(color)); + assertTrue(Utils.darkColorContrast(color1)); + assertFalse(Utils.darkColorContrast(color2)); + assertFalse(Utils.darkColorContrast(color3)); + + } + @Test public void testParseColorString() { final RGB color = new RGB(255, 255, 255); assertEquals("ffffff", Utils.parseColorString(color)); - + assertNull(Utils.parseColorString(null)); } @Test @@ -76,4 +183,64 @@ public class UtilsTest { System.out.println("************* div: " + (millisecondsNow - currentTimeMillis)); } + @Test + public void testToAppFormUrlEncodedBody() { + String appFormUrlEncodedBody = Utils.toAppFormUrlEncodedBody("attr1", Arrays.asList("1", "2", "3")); + assertEquals("attr1[]=1&attr1[]=2&attr1[]=3", appFormUrlEncodedBody); + + appFormUrlEncodedBody = Utils.toAppFormUrlEncodedBody("attr1", Collections.emptyList()); + assertEquals("", appFormUrlEncodedBody); + + appFormUrlEncodedBody = Utils.toAppFormUrlEncodedBody("attr1", null); + assertEquals("", appFormUrlEncodedBody); + + final LinkedMultiValueMap linkedMultiValueMap = new LinkedMultiValueMap<>(); + linkedMultiValueMap.add("attr1", "value1"); + linkedMultiValueMap.add("attr2", "value2"); + linkedMultiValueMap.add("attr3", "value3"); + appFormUrlEncodedBody = Utils.toAppFormUrlEncodedBody(linkedMultiValueMap); + assertEquals("attr1=value1&attr2=value2&attr3=value3", appFormUrlEncodedBody); + + appFormUrlEncodedBody = Utils.toAppFormUrlEncodedBody(new LinkedMultiValueMap<>()); + assertEquals("", appFormUrlEncodedBody); + + appFormUrlEncodedBody = Utils.toAppFormUrlEncodedBody(null); + assertEquals("", appFormUrlEncodedBody); + + } + + @Test + public void testPingHost() { + assertTrue(Utils.pingHost("https://www.google.com")); + assertFalse(Utils.pingHost("www.google.com")); + assertFalse(Utils.pingHost("some")); + } + + @Test + public void testToCSVString() { + final String nullString = Utils.toCSVString(null); + final String emptyString = Utils.toCSVString(""); + assertEquals(StringUtils.EMPTY, nullString); + assertEquals(StringUtils.EMPTY, emptyString); + } + + @Test + public void testGetOrEmptyDisplayValue() { + final String nullValue = Utils.getOrEmptyDisplayValue(null); + final String emptyString = Utils.getOrEmptyDisplayValue(""); + final String someString = Utils.getOrEmptyDisplayValue("some"); + + assertEquals(Constants.EMPTY_NOTE, nullValue); + assertEquals(Constants.EMPTY_NOTE, emptyString); + assertEquals("some", someString); + } + + @Test + public void testFormatStackTracePrint() { + final StringBuilder formatStackTracePrint = + Utils.formatStackTracePrint(10, Thread.currentThread().getStackTrace()); + assertTrue(formatStackTracePrint.toString() + .contains("ch.ethz.seb.sebserver.gbl.util.UtilsTest.testFormatStackTracePrint")); + } + }