Skip to content

Commit 3e94139

Browse files
mrtsmrts
committed
<TRANSIENT> NFC-46 Proposed changes during code review
Signed-off-by: Mart Somermaa <[email protected]>
1 parent ff6a0cf commit 3e94139

26 files changed

+414
-317
lines changed

‎src/main/java/eu/webeid/security/challenge/ChallengeNonceStore.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@
2222

2323
package eu.webeid.security.challenge;
2424

25-
import eu.webeid.security.exceptions.ChallengeNonceExpiredException;
2625
import eu.webeid.security.exceptions.AuthTokenException;
26+
import eu.webeid.security.exceptions.ChallengeNonceExpiredException;
2727
import eu.webeid.security.exceptions.ChallengeNonceNotFoundException;
2828

2929
import static eu.webeid.security.util.DateAndTime.utcNow;

‎src/main/java/eu/webeid/security/validator/AuthTokenValidationConfiguration.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,11 @@ void setSiteOrigin(URI siteOrigin) {
7979
this.siteOrigin = siteOrigin;
8080
}
8181

82-
URI getSiteOrigin() {
82+
public URI getSiteOrigin() {
8383
return siteOrigin;
8484
}
8585

86-
Collection<X509Certificate> getTrustedCACertificates() {
86+
public Collection<X509Certificate> getTrustedCACertificates() {
8787
return trustedCACertificates;
8888
}
8989

@@ -152,7 +152,7 @@ void validate() {
152152
requirePositiveDuration(maxOcspResponseThisUpdateAge, "Max OCSP response thisUpdate age");
153153
}
154154

155-
AuthTokenValidationConfiguration copy() {
155+
public AuthTokenValidationConfiguration copy() {
156156
return new AuthTokenValidationConfiguration(this);
157157
}
158158

‎src/main/java/eu/webeid/security/validator/AuthTokenValidator.java‎

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -22,26 +22,19 @@
2222

2323
package eu.webeid.security.validator;
2424

25-
import com.fasterxml.jackson.databind.ObjectMapper;
26-
import com.fasterxml.jackson.databind.ObjectReader;
2725
import eu.webeid.security.authtoken.WebEidAuthToken;
2826
import eu.webeid.security.certificate.CertificateData;
2927
import eu.webeid.security.exceptions.AuthTokenException;
30-
import eu.webeid.security.exceptions.AuthTokenParseException;
3128
import eu.webeid.security.util.Strings;
3229

33-
import java.io.IOException;
3430
import java.security.cert.X509Certificate;
3531

3632
/**
3733
* Parses and validates the provided Web eID authentication token.
3834
*/
3935
public interface AuthTokenValidator {
40-
String CURRENT_TOKEN_FORMAT_VERSION = "web-eid:1.1";
41-
int TOKEN_MIN_LENGTH = 100;
42-
int TOKEN_MAX_LENGTH = 10000;
4336

44-
ObjectReader TOKEN_READER = new ObjectMapper().readerFor(WebEidAuthToken.class);
37+
String CURRENT_TOKEN_FORMAT_VERSION = "web-eid:1";
4538

4639
/**
4740
* Parses the Web eID authentication token signed by the subject.
@@ -50,24 +43,7 @@ public interface AuthTokenValidator {
5043
* @return the Web eID authentication token
5144
* @throws AuthTokenException when parsing fails
5245
*/
53-
default WebEidAuthToken parse(String authToken) throws AuthTokenException {
54-
try {
55-
if (authToken == null || authToken.length() < TOKEN_MIN_LENGTH) {
56-
throw new AuthTokenParseException("Auth token is null or too short");
57-
}
58-
if (authToken.length() > TOKEN_MAX_LENGTH) {
59-
throw new AuthTokenParseException("Auth token is too long");
60-
}
61-
62-
WebEidAuthToken token = TOKEN_READER.readValue(authToken);
63-
if (token == null) {
64-
throw new AuthTokenParseException("Web eID authentication token is null");
65-
}
66-
return token;
67-
} catch (IOException e) {
68-
throw new AuthTokenParseException("Error parsing Web eID authentication token", e);
69-
}
70-
}
46+
WebEidAuthToken parse(String authToken) throws AuthTokenException;
7147

7248
/**
7349
* Validates the Web eID authentication token signed by the subject and returns

‎src/main/java/eu/webeid/security/validator/AuthTokenValidatorManager.java‎

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,18 @@
2222

2323
package eu.webeid.security.validator;
2424

25+
import com.fasterxml.jackson.databind.ObjectMapper;
26+
import com.fasterxml.jackson.databind.ObjectReader;
2527
import eu.webeid.security.authtoken.WebEidAuthToken;
2628
import eu.webeid.security.exceptions.AuthTokenException;
29+
import eu.webeid.security.exceptions.AuthTokenParseException;
2730
import eu.webeid.security.exceptions.JceException;
2831
import eu.webeid.security.validator.ocsp.OcspClient;
32+
import eu.webeid.security.validator.versionvalidators.AuthTokenVersionValidatorFactory;
2933
import org.slf4j.Logger;
3034
import org.slf4j.LoggerFactory;
3135

36+
import java.io.IOException;
3237
import java.security.cert.X509Certificate;
3338

3439
/**
@@ -37,27 +42,66 @@
3742
final class AuthTokenValidatorManager implements AuthTokenValidator {
3843

3944
private static final Logger LOG = LoggerFactory.getLogger(AuthTokenValidatorManager.class);
40-
private final AuthTokenValidatorFactory tokenValidatorFactory;
45+
46+
private final AuthTokenVersionValidatorFactory tokenValidatorFactory;
47+
48+
// Use human-readable meaningful names for token limits.
49+
private final int TOKEN_MIN_LENGTH = 100;
50+
private final int TOKEN_MAX_LENGTH = 10000;
51+
52+
private final ObjectReader TOKEN_READER = new ObjectMapper().readerFor(WebEidAuthToken.class);
4153

4254
AuthTokenValidatorManager(AuthTokenValidationConfiguration configuration, OcspClient ocspClient)
4355
throws JceException {
44-
this.tokenValidatorFactory = AuthTokenValidatorFactory.create(configuration, ocspClient);
56+
this.tokenValidatorFactory = AuthTokenVersionValidatorFactory.create(configuration, ocspClient);
4557
}
4658

4759
@Override
4860
public WebEidAuthToken parse(String authToken) throws AuthTokenException {
49-
return AuthTokenValidator.super.parse(authToken);
61+
try {
62+
LOG.info("Starting token parsing");
63+
validateTokenLength(authToken);
64+
return parseToken(authToken);
65+
} catch (Exception e) {
66+
// Generally "log and rethrow" is an antipattern, but it fits with the surrounding logging style.
67+
LOG.warn("Token parsing was interrupted:", e);
68+
throw e;
69+
}
5070
}
5171

5272
@Override
5373
public X509Certificate validate(WebEidAuthToken authToken, String currentChallengeNonce) throws AuthTokenException {
5474
try {
5575
LOG.info("Starting token validation");
56-
return tokenValidatorFactory.getValidatorFor(authToken.getFormat()).validate(authToken, currentChallengeNonce);
76+
return tokenValidatorFactory
77+
.getValidatorFor(authToken.getFormat())
78+
.validate(authToken, currentChallengeNonce);
5779
} catch (Exception e) {
58-
// Generally "log and rethrow" is an anti-pattern, but it fits with the surrounding logging style.
80+
// Generally "log and rethrow" is an antipattern, but it fits with the surrounding logging style.
5981
LOG.warn("Token validation was interrupted:", e);
6082
throw e;
6183
}
6284
}
85+
86+
private void validateTokenLength(String authToken) throws AuthTokenParseException {
87+
if (authToken == null || authToken.length() < TOKEN_MIN_LENGTH) {
88+
throw new AuthTokenParseException("Auth token is null or too short");
89+
}
90+
if (authToken.length() > TOKEN_MAX_LENGTH) {
91+
throw new AuthTokenParseException("Auth token is too long");
92+
}
93+
}
94+
95+
private WebEidAuthToken parseToken(String authToken) throws AuthTokenParseException {
96+
try {
97+
final WebEidAuthToken token = TOKEN_READER.readValue(authToken);
98+
if (token == null) {
99+
throw new AuthTokenParseException("Web eID authentication token is null");
100+
}
101+
return token;
102+
} catch (IOException e) {
103+
throw new AuthTokenParseException("Error parsing Web eID authentication token", e);
104+
}
105+
}
106+
63107
}

‎src/main/java/eu/webeid/security/validator/certvalidators/SubjectCertificatePolicyValidator.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@
2323
package eu.webeid.security.validator.certvalidators;
2424

2525
import eu.webeid.security.exceptions.AuthTokenException;
26+
import eu.webeid.security.exceptions.UserCertificateDisallowedPolicyException;
27+
import eu.webeid.security.exceptions.UserCertificateParseException;
2628
import org.bouncycastle.asn1.ASN1ObjectIdentifier;
2729
import org.bouncycastle.asn1.x509.CertificatePolicies;
2830
import org.bouncycastle.asn1.x509.Extension;
2931
import org.bouncycastle.asn1.x509.PolicyInformation;
3032
import org.bouncycastle.cert.jcajce.JcaX509ExtensionUtils;
31-
import eu.webeid.security.exceptions.UserCertificateDisallowedPolicyException;
32-
import eu.webeid.security.exceptions.UserCertificateParseException;
3333
import org.slf4j.Logger;
3434
import org.slf4j.LoggerFactory;
3535

‎src/main/java/eu/webeid/security/validator/certvalidators/SubjectCertificatePurposeValidator.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,12 @@
2222

2323
package eu.webeid.security.validator.certvalidators;
2424

25-
import eu.webeid.security.exceptions.UserCertificateWrongPurposeException;
26-
import org.slf4j.Logger;
27-
import org.slf4j.LoggerFactory;
2825
import eu.webeid.security.exceptions.AuthTokenException;
2926
import eu.webeid.security.exceptions.UserCertificateMissingPurposeException;
3027
import eu.webeid.security.exceptions.UserCertificateParseException;
28+
import eu.webeid.security.exceptions.UserCertificateWrongPurposeException;
29+
import org.slf4j.Logger;
30+
import org.slf4j.LoggerFactory;
3131

3232
import java.security.cert.CertificateParsingException;
3333
import java.security.cert.X509Certificate;

‎src/main/java/eu/webeid/security/validator/ocsp/service/DesignatedOcspService.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@
2222

2323
package eu.webeid.security.validator.ocsp.service;
2424

25+
import eu.webeid.security.exceptions.AuthTokenException;
26+
import eu.webeid.security.exceptions.OCSPCertificateException;
2527
import org.bouncycastle.cert.X509CertificateHolder;
2628
import org.bouncycastle.cert.jcajce.JcaX509CertificateConverter;
27-
import eu.webeid.security.exceptions.OCSPCertificateException;
28-
import eu.webeid.security.exceptions.AuthTokenException;
2929

3030
import java.net.URI;
3131
import java.security.cert.CertificateEncodingException;

‎src/main/java/eu/webeid/security/validator/ocsp/service/DesignatedOcspServiceConfiguration.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@
2222

2323
package eu.webeid.security.validator.ocsp.service;
2424

25+
import eu.webeid.security.exceptions.OCSPCertificateException;
2526
import eu.webeid.security.validator.ocsp.OcspResponseValidator;
2627
import org.bouncycastle.asn1.x500.X500Name;
2728
import org.bouncycastle.cert.jcajce.JcaX509CertificateHolder;
28-
import eu.webeid.security.exceptions.OCSPCertificateException;
2929

3030
import java.net.URI;
3131
import java.security.cert.CertificateEncodingException;

‎src/main/java/eu/webeid/security/validator/ocsp/service/OcspService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@
2222

2323
package eu.webeid.security.validator.ocsp.service;
2424

25-
import org.bouncycastle.cert.X509CertificateHolder;
2625
import eu.webeid.security.exceptions.AuthTokenException;
26+
import org.bouncycastle.cert.X509CertificateHolder;
2727

2828
import java.net.URI;
2929
import java.util.Date;

src/main/java/eu/webeid/security/validator/AuthTokenV11Validator.java renamed to src/main/java/eu/webeid/security/validator/versionvalidators/AuthTokenVersion11Validator.java

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,16 @@
2020
* SOFTWARE.
2121
*/
2222

23-
package eu.webeid.security.validator;
23+
package eu.webeid.security.validator.versionvalidators;
2424

2525
import eu.webeid.security.authtoken.SupportedSignatureAlgorithm;
2626
import eu.webeid.security.authtoken.WebEidAuthToken;
2727
import eu.webeid.security.certificate.CertificateLoader;
2828
import eu.webeid.security.exceptions.AuthTokenException;
2929
import eu.webeid.security.exceptions.AuthTokenParseException;
3030
import eu.webeid.security.exceptions.CertificateDecodingException;
31+
import eu.webeid.security.validator.AuthTokenSignatureValidator;
32+
import eu.webeid.security.validator.AuthTokenValidationConfiguration;
3133
import eu.webeid.security.validator.certvalidators.SubjectCertificateValidatorBatch;
3234
import eu.webeid.security.validator.ocsp.OcspClient;
3335
import eu.webeid.security.validator.ocsp.OcspServiceProvider;
@@ -49,7 +51,7 @@
4951

5052
import static eu.webeid.security.util.Strings.isNullOrEmpty;
5153

52-
class AuthTokenV11Validator extends AuthTokenV1Validator implements AuthTokenVersionValidator {
54+
class AuthTokenVersion11Validator extends AuthTokenVersion1Validator implements AuthTokenVersionValidator {
5355

5456
private static final String V11_SUPPORTED_TOKEN_FORMAT_PREFIX = "web-eid:1.1";
5557
private static final Set<String> SUPPORTED_SIGNING_CRYPTO_ALGORITHMS = Set.of("ECC", "RSA");
@@ -60,7 +62,7 @@ class AuthTokenV11Validator extends AuthTokenV1Validator implements AuthTokenVer
6062
);
6163
private static final int KEY_USAGE_NON_REPUDIATION = 1;
6264

63-
public AuthTokenV11Validator(
65+
public AuthTokenVersion11Validator(
6466
SubjectCertificateValidatorBatch simpleSubjectCertificateValidators,
6567
Set<TrustAnchor> trustedCACertificateAnchors,
6668
CertStore trustedCACertificateCertStore,
@@ -80,11 +82,6 @@ public AuthTokenV11Validator(
8082
);
8183
}
8284

83-
@Override
84-
public boolean supports(String format) {
85-
return format != null && format.startsWith(getSupportedFormatPrefix());
86-
}
87-
8885
@Override
8986
protected String getSupportedFormatPrefix() {
9087
return V11_SUPPORTED_TOKEN_FORMAT_PREFIX;

0 commit comments

Comments
 (0)