refactor,update: update dependencies, add logging and null checks, move SubjectCertificatePolicies to certifcate package

mrts · mrts · commit 7af2dbb68add · 2022-07-13T20:18:50.000+03:00
Signed-off-by: Mart Somermaa &lt;mrts@users.noreply.github.com&gt;
diff --git a/pom.xml b/pom.xml
@@ -5,7 +5,7 @@
     <modelVersion>4.0.0</modelVersion>
     <artifactId>authtoken-validation</artifactId>
     <groupId>org.webeid.security</groupId>
-    <version>2.0.1-SNAPSHOT</version>
+    <version>2.0.1</version>
     <packaging>jar</packaging>
     <name>authtoken-validation</name>
     <description>Web eID authentication token validation library for Java</description>
@@ -14,12 +14,15 @@
         <maven.version>3.3.9</maven.version>
         <maven-surefire-plugin.version>2.22.2</maven-surefire-plugin.version>
         <java.version>1.8</java.version>
-        <jjwt.version>0.11.2</jjwt.version>
-        <slf4j.version>1.7.32</slf4j.version>
-        <bouncycastle.version>1.69</bouncycastle.version>
-        <junit-jupiter.version>5.8.1</junit-jupiter.version>
-        <assertj.version>3.21.0</assertj.version>
-        <mockito.version>4.0.0</mockito.version>
+        <jjwt.version>0.11.5</jjwt.version>
+        <jackson.version>2.13.3</jackson.version>
+        <slf4j.version>1.7.36</slf4j.version>
+        <bouncycastle.version>1.70</bouncycastle.version>
+        <guava.version>31.1-jre</guava.version>
+        <okhttp.version>4.10.0</okhttp.version>
+        <junit-jupiter.version>5.8.2</junit-jupiter.version>
+        <assertj.version>3.23.1</assertj.version>
+        <mockito.version>4.6.1</mockito.version>
         <jacoco.version>0.8.5</jacoco.version>
         <sonar.coverage.jacoco.xmlReportPaths>
             ${project.basedir}/../jacoco-coverage-report/target/site/jacoco-aggregate/jacoco.xml
@@ -37,22 +40,16 @@
     </properties>
 
     <dependencies>
-        <dependency>
-            <groupId>io.jsonwebtoken</groupId>
-            <artifactId>jjwt-api</artifactId>
-            <version>${jjwt.version}</version>
-        </dependency>
         <dependency>
             <groupId>io.jsonwebtoken</groupId>
             <artifactId>jjwt-impl</artifactId>
             <version>${jjwt.version}</version>
         </dependency>
         <dependency>
-            <groupId>io.jsonwebtoken</groupId>
-            <artifactId>jjwt-jackson</artifactId>
-            <version>${jjwt.version}</version>
+            <groupId>com.fasterxml.jackson.core</groupId>
+            <artifactId>jackson-databind</artifactId>
+            <version>${jackson.version}</version>
         </dependency>
-
         <dependency>
             <groupId>org.slf4j</groupId>
             <artifactId>slf4j-api</artifactId>
@@ -61,7 +58,7 @@
         <dependency>
             <groupId>com.google.guava</groupId>
             <artifactId>guava</artifactId>
-            <version>31.0.1-jre</version>
+            <version>${guava.version}</version>
         </dependency>
         <dependency>
             <groupId>org.bouncycastle</groupId>
@@ -76,7 +73,7 @@
         <dependency>
             <groupId>com.squareup.okhttp3</groupId>
             <artifactId>okhttp</artifactId>
-            <version>4.9.2</version>
+            <version>${okhttp.version}</version>
         </dependency>
 
         <dependency>
diff --git a/src/main/java/eu/webeid/security/certificate/SubjectCertificatePolicies.java b/src/main/java/eu/webeid/security/certificate/SubjectCertificatePolicies.java
@@ -20,7 +20,7 @@
  * SOFTWARE.
  */
 
-package eu.webeid.security.util;
+package eu.webeid.security.certificate;
 
 import org.bouncycastle.asn1.ASN1ObjectIdentifier;
 
diff --git a/src/main/java/eu/webeid/security/challenge/ChallengeNonce.java b/src/main/java/eu/webeid/security/challenge/ChallengeNonce.java
@@ -23,15 +23,16 @@
 package eu.webeid.security.challenge;
 
 import java.time.ZonedDateTime;
+import java.util.Objects;
 
 public class ChallengeNonce {
 
     private final String base64EncodedNonce;
     private final ZonedDateTime expirationTime;
 
     public ChallengeNonce(String base64EncodedNonce, ZonedDateTime expirationTime) {
-        this.base64EncodedNonce = base64EncodedNonce;
-        this.expirationTime = expirationTime;
+        this.base64EncodedNonce = Objects.requireNonNull(base64EncodedNonce);
+        this.expirationTime = Objects.requireNonNull(expirationTime);
     }
 
     public ZonedDateTime getExpirationTime() {
diff --git a/src/main/java/eu/webeid/security/challenge/ChallengeNonceGeneratorBuilder.java b/src/main/java/eu/webeid/security/challenge/ChallengeNonceGeneratorBuilder.java
@@ -44,7 +44,7 @@ public class ChallengeNonceGeneratorBuilder {
      * <p>
      * When the time-to-live passes, the nonce is considered to be expired.
      *
-     * @param duration time-to-live duration
+     * @param duration challenge nonce time-to-live duration
      * @return current builder instance
      */
     public ChallengeNonceGeneratorBuilder withNonceTtl(Duration duration) {
diff --git a/src/main/java/eu/webeid/security/validator/AuthTokenValidationConfiguration.java b/src/main/java/eu/webeid/security/validator/AuthTokenValidationConfiguration.java
@@ -23,7 +23,7 @@
 package eu.webeid.security.validator;
 
 import com.google.common.collect.Sets;
-import eu.webeid.security.util.SubjectCertificatePolicies;
+import eu.webeid.security.certificate.SubjectCertificatePolicies;
 import eu.webeid.security.validator.ocsp.service.DesignatedOcspServiceConfiguration;
 import org.bouncycastle.asn1.ASN1ObjectIdentifier;
 
diff --git a/src/main/java/eu/webeid/security/validator/AuthTokenValidatorImpl.java b/src/main/java/eu/webeid/security/validator/AuthTokenValidatorImpl.java
@@ -159,9 +159,8 @@ private X509Certificate validateToken(WebEidAuthToken token, String currentChall
         simpleSubjectCertificateValidators.executeFor(subjectCertificate);
         getCertTrustValidators().executeFor(subjectCertificate);
 
-        // It is guaranteed that if the signature verification succeeds, then the origin, challenge
-        // and, if part of the signature, origin certificate have been implicitly and correctly verified
-        // without the need to implement any additional checks.
+        // It is guaranteed that if the signature verification succeeds, then the origin and challenge
+        // have been implicitly and correctly verified without the need to implement any additional checks.
         authTokenSignatureValidator.validate(token.getAlgorithm(),
             token.getSignature(),
             subjectCertificate.getPublicKey(),
diff --git a/src/main/java/eu/webeid/security/validator/certvalidators/SubjectCertificatePolicyValidator.java b/src/main/java/eu/webeid/security/validator/certvalidators/SubjectCertificatePolicyValidator.java
@@ -30,6 +30,8 @@
 import org.bouncycastle.cert.jcajce.JcaX509ExtensionUtils;
 import eu.webeid.security.exceptions.UserCertificateDisallowedPolicyException;
 import eu.webeid.security.exceptions.UserCertificateParseException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.security.cert.X509Certificate;
@@ -39,6 +41,8 @@
 
 public final class SubjectCertificatePolicyValidator {
 
+    private static final Logger LOG = LoggerFactory.getLogger(SubjectCertificatePolicyValidator.class);
+
     private final Collection<ASN1ObjectIdentifier> disallowedSubjectCertificatePolicies;
 
     public SubjectCertificatePolicyValidator(Collection<ASN1ObjectIdentifier> disallowedSubjectCertificatePolicies) {
@@ -68,5 +72,6 @@ public void validateCertificatePolicies(X509Certificate subjectCertificate) thro
         } catch (IOException e) {
             throw new UserCertificateParseException(e);
         }
+        LOG.debug("User certificate does not contain disallowed policies.");
     }
 }
diff --git a/src/main/java/eu/webeid/security/validator/certvalidators/SubjectCertificatePurposeValidator.java b/src/main/java/eu/webeid/security/validator/certvalidators/SubjectCertificatePurposeValidator.java
@@ -53,10 +53,10 @@ public static void validateCertificatePurpose(X509Certificate subjectCertificate
             if (!usages.contains(EXTENDED_KEY_USAGE_CLIENT_AUTHENTICATION)) {
                 throw new UserCertificateWrongPurposeException();
             }
-            LOG.debug("User certificate can be used for client authentication.");
         } catch (CertificateParsingException e) {
             throw new UserCertificateParseException(e);
         }
+        LOG.debug("User certificate can be used for client authentication.");
     }
 
     private SubjectCertificatePurposeValidator() {
diff --git a/src/test/java/eu/webeid/security/challenge/ChallengeNonceGeneratorTest.java b/src/test/java/eu/webeid/security/challenge/ChallengeNonceGeneratorTest.java
@@ -22,6 +22,7 @@
 
 package eu.webeid.security.challenge;
 
+import eu.webeid.security.exceptions.AuthTokenException;
 import org.junit.jupiter.api.Test;
 import eu.webeid.security.exceptions.ChallengeNonceExpiredException;
 import eu.webeid.security.exceptions.ChallengeNonceNotFoundException;
@@ -35,14 +36,17 @@ class ChallengeNonceGeneratorTest {
     final ChallengeNonceStore challengeNonceStore = new InMemoryChallengeNonceStore();
 
     @Test
-    void validateNonceGeneration() {
+    void validateNonceGeneration() throws AuthTokenException {
         final ChallengeNonceGenerator challengeNonceGenerator = new ChallengeNonceGeneratorBuilder()
             .withChallengeNonceStore(challengeNonceStore)
             .withNonceTtl(Duration.ofSeconds(1))
             .build();
 
         final ChallengeNonce nonce1 = challengeNonceGenerator.generateAndStoreNonce();
         final ChallengeNonce nonce2 = challengeNonceGenerator.generateAndStoreNonce();
+        final ChallengeNonce nonce2fromStore = challengeNonceStore.getAndRemove();
+
+        assertThat(nonce2).isEqualTo(nonce2fromStore);
 
         assertThat(nonce1.getBase64EncodedNonce())
             .hasSize(44) // Base64-encoded 32 bytes
diff --git a/src/test/java/eu/webeid/security/validator/AuthTokenCertificateTest.java b/src/test/java/eu/webeid/security/validator/AuthTokenCertificateTest.java
@@ -97,7 +97,7 @@ void whenCertificateFieldIsArray_thenParsingFails() throws AuthTokenException {
             .hasMessage("Error parsing Web eID authentication token")
             .getCause()
             .isInstanceOf(MismatchedInputException.class)
-            .hasMessageStartingWith("Cannot deserialize instance of `java.lang.String` out of START_ARRAY token");
+            .hasMessageStartingWith("Cannot deserialize value of type `java.lang.String` from Array value");
     }
 
     @Test
@@ -211,15 +211,17 @@ void whenUserCertificateIsNotYetValid_thenValidationFails() {
         mockDate("2018-10-17");
         assertThatThrownBy(() -> validator
             .validate(validAuthToken, VALID_CHALLENGE_NONCE))
-            .isInstanceOf(CertificateNotYetValidException.class);
+            .isInstanceOf(CertificateNotYetValidException.class)
+            .hasMessage("User certificate is not yet valid");
     }
 
     @Test
     void whenTrustedCACertificateIsNotYetValid_thenValidationFails() {
         mockDate("2018-08-17");
         assertThatThrownBy(() -> validator
             .validate(validAuthToken, VALID_CHALLENGE_NONCE))
-            .isInstanceOf(CertificateNotYetValidException.class);
+            .isInstanceOf(CertificateNotYetValidException.class)
+            .hasMessage("Trusted CA certificate is not yet valid");
     }
 
     @Test
diff --git a/src/test/java/eu/webeid/security/validator/AuthTokenSignatureValidatorTest.java b/src/test/java/eu/webeid/security/validator/AuthTokenSignatureValidatorTest.java
@@ -42,7 +42,7 @@ class AuthTokenSignatureValidatorTest {
         "\"unverifiedCertificate\":\"MIIGvjCCBKagAwIBAgIQT7aXeR+zWlBb2Gbar+AFaTANBgkqhkiG9w0BAQsFADCBgzELMAkGA1UEBhMCTFYxOTA3BgNVBAoMMFZBUyBMYXR2aWphcyBWYWxzdHMgcmFkaW8gdW4gdGVsZXbEq3ppamFzIGNlbnRyczEaMBgGA1UEYQwRTlRSTFYtNDAwMDMwMTEyMDMxHTAbBgNVBAMMFERFTU8gTFYgZUlEIElDQSAyMDE3MB4XDTE4MTAzMDE0MTI0MloXDTIzMTAzMDE0MTI0MlowcDELMAkGA1UEBhMCTFYxHDAaBgNVBAMME0FORFJJUyBQQVJBVURaScWFxaAxFTATBgNVBAQMDFBBUkFVRFpJxYXFoDEPMA0GA1UEKgwGQU5EUklTMRswGQYDVQQFExJQTk9MVi0zMjE5MjItMzMwMzIwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDXkra3rDOOt5K6OnJcg/Xt6JOogPAUBX2kT9zWelze7WSuPx2Ofs//0JoBQ575IVdh3JpLhfh7g60YYi41M6vNACVSNaFOxiEvE9amSFizMiLk5+dp+79rymqOsVQG8CSu8/RjGGlDsALeb3N/4pUSTGXUwSB64QuFhOWjAcmKPhHeYtry0hK3MbwwHzFhYfGpo/w+PL14PEdJlpL1UX/aPyT0Zq76Z4T/Z3PqbTmQp09+2b0thC0JIacSkyJuTu8fVRQvse+8UtYC6Kt3TBLZbPtqfAFSXWbuE47Lc2o840NkVlMHVAesoRAfiQxsK35YWFT0rHPWbLjX6ySiaL25AgMBAAGjggI+MIICOjAMBgNVHRMBAf8EAjAAMA4GA1UdDwEB/wQEAwIHgDATBgNVHSUEDDAKBggrBgEFBQcDAjAdBgNVHQ4EFgQUHZWimPze2GXULNaP4EFVdF+MWKQwHwYDVR0jBBgwFoAUj2jOvOLHQCFTCUK75Z4djEvNvTgwgfsGA1UdIASB8zCB8DA7BgYEAI96AQIwMTAvBggrBgEFBQcCARYjaHR0cHM6Ly93d3cuZXBhcmFrc3RzLmx2L3JlcG9zaXRvcnkwgbAGDCsGAQQBgfo9AgECATCBnzAvBggrBgEFBQcCARYjaHR0cHM6Ly93d3cuZXBhcmFrc3RzLmx2L3JlcG9zaXRvcnkwbAYIKwYBBQUHAgIwYAxexaBpcyBzZXJ0aWZpa8SBdHMgaXIgaWVrxLxhdXRzIExhdHZpamFzIFJlcHVibGlrYXMgaXpzbmllZ3TEgSBwZXJzb251IGFwbGllY2lub8WhxIEgZG9rdW1lbnTEgTB9BggrBgEFBQcBAQRxMG8wQgYIKwYBBQUHMAKGNmh0dHA6Ly9kZW1vLmVwYXJha3N0cy5sdi9jZXJ0L2RlbW9fTFZfZUlEX0lDQV8yMDE3LmNydDApBggrBgEFBQcwAYYdaHR0cDovL29jc3AucHJlcC5lcGFyYWtzdHMubHYwSAYDVR0fBEEwPzA9oDugOYY3aHR0cDovL2RlbW8uZXBhcmFrc3RzLmx2L2NybC9kZW1vX0xWX2VJRF9JQ0FfMjAxN18zLmNybDANBgkqhkiG9w0BAQsFAAOCAgEAAOVoRbnMv2UXWYHgnmO9Zg9u8F1YvJiZPMeTYE2CVaiq0nXe4Mq0X5tWcsEiRpGQF9e0dWC6V5m6EmAsHxIRL4chZKRrIrPEiWtP3zyRI1/X2y5GwSUyZmgxkuSOHHw3UjzjrnOoI9izpC0OSNeumqpjT/tLAi35sktGkK0onEUPWGQnZLqd/hzykm+H/dmD27nOnfCJOSqbegLSbhV2w/WAII+IUD3vJ06F6rf9ZN8xbrGkPO8VMCIDIt0eBKFxBdSOgpsTfbERbjQJ+nFEDYhD0bFNYMsFSGnZiWpNaCcZSkk4mtNUa8sNXyaFQGIZk6NjQ/fsBANhUoxFz7rUKrRYqk356i8KFDZ+MJqUyodKKyW9oz+IO5eJxnL78zRbxD+EfAUmrLXOjmGIzU95RR1smS4cirrrPHqGAWojBk8hKbjNTJl9Tfbnsbc9/FUBJLVZAkCi631KfRLQ66bn8N0mbtKlNtdX0G47PXTy7SJtWwDtKQ8+qVpduc8xHLntbdAzie3mWyxA1SBhQuZ9BPf5SPBImWCNpmZNCTmI2e+4yyCnmG/kVNilUAaODH/fgQXFGdsKO/XATFohiies28twkEzqtlVZvZbpBhbJCHYVnQXMhMKcnblkDqXWcSWd3QAKig2yMH95uz/wZhiV+7tZ7cTgwcbCzIDCfpwBC3E=\"," +
         "\"issuerApp\":\"https://web-eid.eu/web-eid-app/releases/2.0.0+0\"," +
         "\"signature\":\"xsjXsQvVYXWcdV0YPhxLthJxtf0//R8p9WFFlYJGRARrl1ruyoAUwl0xeHgeZOKeJtwiCYCNWJzCG3VM3ydgt92bKhhk1u0JXIPVqvOkmDY72OCN4q73Y8iGSPVTgjk93TgquHlodf7YcqZNhutwNNf3oldHEWJD5zmkdwdpBFXgeOwTAdFwGljDQZbHr3h1Dr+apUDuloS0WuIzUuu8YXN2b8lh8FCTlF0G0DEjhHd/MGx8dbe3UTLHmD7K9DXv4zLJs6EF9i2v/C10SIBQDkPBSVPqMxCDPECjbEPi2+ds94eU7ThOhOQlFFtJ4KjQNTUa2crSixH7cYZF2rNNmA==\"," +
-        "\"format\":\"web-eid:1.0\"}}";
+        "\"format\":\"web-eid:1.0\"}";
 
     @Test
     void whenValidES384Signature_thenSucceeds() throws Exception {
diff --git a/src/test/java/eu/webeid/security/validator/certvalidators/SubjectCertificateNotRevokedValidatorTest.java b/src/test/java/eu/webeid/security/validator/certvalidators/SubjectCertificateNotRevokedValidatorTest.java
@@ -131,7 +131,7 @@ void whenOcspRequestHasInvalidBody_thenThrows() throws Exception {
             .isInstanceOf(UserCertificateOCSPCheckFailedException.class)
             .getCause()
             .isInstanceOf(IOException.class)
-            .hasMessage("corrupted stream - out of bounds length found: 110 >= 7");
+            .hasMessage("DEF length 110 object truncated by 105");
     }
 
     @Test
diff --git a/src/test/java/eu/webeid/security/validator/ocsp/OcspServiceProviderTest.java b/src/test/java/eu/webeid/security/validator/ocsp/OcspServiceProviderTest.java
@@ -82,5 +82,4 @@ void whenAiaOcspServiceConfigurationDoesNotHaveResponderCertTrustedCA_thenThrows
                 service2018.validateResponderCertificate(wrongResponderCert, new Date(1630000000000L)));
     }
 
-
 }

Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ public class ChallengeNonceGeneratorBuilder {`
`44`	`44`	`* <p>`
`45`	`45`	`* When the time-to-live passes, the nonce is considered to be expired.`
`46`	`46`	`*`
`47`		`- * @param duration time-to-live duration`
	`47`	`+ * @param duration challenge nonce time-to-live duration`
`48`	`48`	`* @return current builder instance`
`49`	`49`	`*/`
`50`	`50`	`public ChallengeNonceGeneratorBuilder withNonceTtl(Duration duration) {`
Original file line number	Diff line number	Diff line change
`@@ -30,6 +30,8 @@`
`30`	`30`	`import org.bouncycastle.cert.jcajce.JcaX509ExtensionUtils;`
`31`	`31`	`import eu.webeid.security.exceptions.UserCertificateDisallowedPolicyException;`
`32`	`32`	`import eu.webeid.security.exceptions.UserCertificateParseException;`
	`33`	`+import org.slf4j.Logger;`
	`34`	`+import org.slf4j.LoggerFactory;`
`33`	`35`
`34`	`36`	`import java.io.IOException;`
`35`	`37`	`import java.security.cert.X509Certificate;`
`@@ -39,6 +41,8 @@`
`39`	`41`
`40`	`42`	`public final class SubjectCertificatePolicyValidator {`
`41`	`43`
	`44`	`+ private static final Logger LOG = LoggerFactory.getLogger(SubjectCertificatePolicyValidator.class);`
	`45`	`+`
`42`	`46`	`private final Collection<ASN1ObjectIdentifier> disallowedSubjectCertificatePolicies;`
`43`	`47`
`44`	`48`	`public SubjectCertificatePolicyValidator(Collection<ASN1ObjectIdentifier> disallowedSubjectCertificatePolicies) {`
`@@ -68,5 +72,6 @@ public void validateCertificatePolicies(X509Certificate subjectCertificate) thro`
`68`	`72`	`} catch (IOException e) {`
`69`	`73`	`throw new UserCertificateParseException(e);`
`70`	`74`	`}`
	`75`	`+ LOG.debug("User certificate does not contain disallowed policies.");`
`71`	`76`	`}`
`72`	`77`	`}`
Original file line number	Diff line number	Diff line change
`@@ -53,10 +53,10 @@ public static void validateCertificatePurpose(X509Certificate subjectCertificate`
`53`	`53`	`if (!usages.contains(EXTENDED_KEY_USAGE_CLIENT_AUTHENTICATION)) {`
`54`	`54`	`throw new UserCertificateWrongPurposeException();`
`55`	`55`	`}`
`56`		`- LOG.debug("User certificate can be used for client authentication.");`
`57`	`56`	`} catch (CertificateParsingException e) {`
`58`	`57`	`throw new UserCertificateParseException(e);`
`59`	`58`	`}`
	`59`	`+ LOG.debug("User certificate can be used for client authentication.");`
`60`	`60`	`}`
`61`	`61`
`62`	`62`	`private SubjectCertificatePurposeValidator() {`