Skip to content

Commit 1e122e1

Browse files
mrtsmrts
committed
Add GitHub build and code analysis actions, fix Sonar issues
Signed-off-by: Mart Sõmermaa <[email protected]>
1 parent ab0fdff commit 1e122e1

28 files changed

+307
-116
lines changed

‎.github/workflows/codeql-analysis.yml‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# For most projects, this workflow file will not need changing; you simply need
2+
# to commit it to your repository.
3+
#
4+
# You may wish to alter this file to override the set of languages analyzed,
5+
# or to provide custom queries or build logic.
6+
#
7+
# ******** NOTE ********
8+
# We have attempted to detect the languages in your repository. Please check
9+
# the `language` matrix defined below to confirm you have the correct set of
10+
# supported CodeQL languages.
11+
# ******** NOTE ********
12+
13+
name: CodeQL code analysis
14+
15+
on:
16+
push:
17+
branches: [ main ]
18+
pull_request:
19+
# The branches below must be a subset of the branches above
20+
branches: [ main ]
21+
schedule:
22+
- cron: '18 17 * * 4'
23+
24+
jobs:
25+
analyze:
26+
name: Analyze
27+
runs-on: ubuntu-latest
28+
29+
strategy:
30+
fail-fast: false
31+
matrix:
32+
language: [ 'java' ]
33+
# CodeQL supports [ 'cpp', 'csharp', 'go', 'java', 'javascript', 'python' ]
34+
# Learn more...
35+
# https://docs.github.com/en/github/finding-security-vulnerabilities-and-errors-in-your-code/configuring-code-scanning#overriding-automatic-language-detection
36+
37+
steps:
38+
- name: Checkout repository
39+
uses: actions/checkout@v2
40+
41+
# Initializes the CodeQL tools for scanning.
42+
- name: Initialize CodeQL
43+
uses: github/codeql-action/init@v1
44+
with:
45+
languages: ${{ matrix.language }}
46+
# If you wish to specify custom queries, you can do so here or in a config file.
47+
# By default, queries listed here will override any specified in a config file.
48+
# Prefix the list here with "+" to use these queries and those in the config file.
49+
# queries: ./path/to/local/query, your-org/your-repo/queries@main
50+
51+
# Autobuild attempts to build any compiled languages (C/C++, C#, or Java).
52+
# If this step fails, then you should remove it and run the build manually (see below)
53+
- name: Autobuild
54+
uses: github/codeql-action/autobuild@v1
55+
56+
# ℹ️ Command-line programs to run using the OS shell.
57+
# 📚 https://git.io/JvXDl
58+
59+
# ✏️ If the Autobuild fails above, remove it and uncomment the following three lines
60+
# and modify them (or add more) to build your code if your project
61+
# uses a compiled language
62+
63+
#- run: |
64+
# make bootstrap
65+
# make release
66+
67+
- name: Perform CodeQL Analysis
68+
uses: github/codeql-action/analyze@v1

‎.github/workflows/maven-build.yml‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
name: Maven build
2+
3+
on: [push, pull_request]
4+
5+
jobs:
6+
build:
7+
runs-on: ubuntu-latest
8+
9+
steps:
10+
- uses: actions/checkout@v2
11+
- uses: actions/setup-java@v1
12+
with:
13+
java-version: 1.8
14+
- name: Cache Maven packages
15+
uses: actions/cache@v1
16+
with:
17+
path: ~/.m2
18+
key: ${{ runner.os }}-m2-v8-${{ hashFiles('**/pom.xml') }}
19+
restore-keys: ${{ runner.os }}-m2-v8
20+
- name: Build
21+
run: mvn --batch-mode compile
22+
- name: Test
23+
run: |
24+
mvn -version
25+
mvn --batch-mode verify

‎.github/workflows/sonarcloud-analysis.yml‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
name: SonarCloud code analysis
2+
3+
on: [push, pull_request]
4+
5+
jobs:
6+
analyze:
7+
name: Analyze
8+
runs-on: ubuntu-latest
9+
10+
steps:
11+
- uses: actions/checkout@v2
12+
with:
13+
fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of analysis
14+
- name: Set up JDK 11
15+
uses: actions/setup-java@v1
16+
with:
17+
java-version: 11
18+
- name: Cache SonarCloud packages
19+
uses: actions/cache@v1
20+
with:
21+
path: ~/.sonar/cache
22+
key: ${{ runner.os }}-sonar
23+
restore-keys: ${{ runner.os }}-sonar
24+
- name: Cache Maven packages
25+
uses: actions/cache@v1
26+
with:
27+
path: ~/.m2
28+
key: ${{ runner.os }}-m2-v11-${{ hashFiles('**/pom.xml') }}
29+
restore-keys: ${{ runner.os }}-m2-v11
30+
- name: Build and analyze
31+
env:
32+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any
33+
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
34+
run: mvn -B verify org.sonarsource.scanner.maven:sonar-maven-plugin:sonar

‎pom.xml‎

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,25 @@
1616
<maven-surefire-plugin.version>2.22.1</maven-surefire-plugin.version>
1717
<java.version>1.8</java.version>
1818
<jjwt.version>0.11.2</jjwt.version>
19+
<slf4j.version>1.7.30</slf4j.version>
20+
<caffeine.version>2.8.5</caffeine.version>
1921
<junit-jupiter.version>5.6.2</junit-jupiter.version>
2022
<assertj.version>3.17.2</assertj.version>
2123
<jmockit.version>1.44</jmockit.version>
22-
<slf4j.version>1.7.30</slf4j.version>
23-
<caffeine.version>2.8.5</caffeine.version>
24+
<jacoco.version>0.8.5</jacoco.version>
25+
<sonar.coverage.jacoco.xmlReportPaths>
26+
${project.basedir}/../jacoco-coverage-report/target/site/jacoco-aggregate/jacoco.xml
27+
</sonar.coverage.jacoco.xmlReportPaths>
2428
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
2529
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
2630
<maven.compiler.source>${java.version}</maven.compiler.source>
2731
<maven.compiler.target>${java.version}</maven.compiler.target>
2832
<argLine>-Djava.security.egd=file:/dev/./urandom -Xmx256m</argLine>
33+
34+
<!-- SonarCloud -->
35+
<sonar.projectKey>web-eid_web-eid-authtoken-validation-java</sonar.projectKey>
36+
<sonar.organization>web-eid</sonar.organization>
37+
<sonar.host.url>https://sonarcloud.io</sonar.host.url>
2938
</properties>
3039

3140
<dependencies>
@@ -116,11 +125,30 @@
116125
<version>${maven-surefire-plugin.version}</version>
117126
<configuration>
118127
<argLine>
119-
-javaagent:${settings.localRepository}/org/jmockit/jmockit/${jmockit.version}/jmockit-${jmockit.version}.jar
128+
@{argLine} -javaagent:${settings.localRepository}/org/jmockit/jmockit/${jmockit.version}/jmockit-${jmockit.version}.jar
120129
</argLine>
121-
<disableXmlReport>true</disableXmlReport>
122130
</configuration>
123131
</plugin>
132+
<plugin>
133+
<!-- Generate a coverage XML file for Sonar -->
134+
<groupId>org.jacoco</groupId>
135+
<artifactId>jacoco-maven-plugin</artifactId>
136+
<version>${jacoco.version}</version>
137+
<executions>
138+
<execution>
139+
<goals>
140+
<goal>prepare-agent</goal>
141+
</goals>
142+
</execution>
143+
<execution>
144+
<id>report</id>
145+
<phase>verify</phase>
146+
<goals>
147+
<goal>report</goal>
148+
</goals>
149+
</execution>
150+
</executions>
151+
</plugin>
124152
</plugins>
125153
</build>
126154

‎src/main/java/org/webeid/security/exceptions/TokenValidationException.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@
2626
* Base class for all authentication token validation exceptions.
2727
*/
2828
public abstract class TokenValidationException extends Exception {
29-
public TokenValidationException(String msg) {
29+
protected TokenValidationException(String msg) {
3030
super(msg);
3131
}
3232

33-
public TokenValidationException(String msg, Throwable cause) {
33+
protected TokenValidationException(String msg, Throwable cause) {
3434
super(msg, cause);
3535
}
3636
}

‎src/main/java/org/webeid/security/util/CertUtil.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,4 +65,8 @@ private static String getField(X509Certificate certificate, ASN1ObjectIdentifier
6565
.collect(Collectors.joining(", "));
6666
}
6767

68+
private CertUtil() {
69+
throw new IllegalStateException("Utility class");
70+
}
71+
6872
}

‎src/main/java/org/webeid/security/util/TitleCase.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,4 +41,8 @@ public static String toTitleCase(String input) {
4141
return titleCase.toString();
4242
}
4343

44+
private TitleCase() {
45+
throw new IllegalStateException("Utility class");
46+
}
47+
4448
}

‎src/main/java/org/webeid/security/validator/AuthTokenParser.java‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,8 @@ void populateDataFromClaims(AuthTokenValidatorData data) throws TokenParseExcept
135135
final String nonceField = getStringFieldOrThrow(body, "nonce", "body");
136136

137137
// The "aud" field value is an array that contains the origin and may also contain site certificate fingerprint
138-
final List audienceField = getListFieldOrThrow(body, "aud", "body");
139-
final String origin = (String) audienceField.get(0);
138+
final List<String> audienceField = getListFieldOrThrow(body, "aud", "body");
139+
final String origin = audienceField.get(0);
140140
if (Strings.isNullOrEmpty(origin)) {
141141
throw new TokenParseException("origin from aud field must not be empty");
142142
}
@@ -145,7 +145,7 @@ void populateDataFromClaims(AuthTokenValidatorData data) throws TokenParseExcept
145145
data.setOrigin(origin);
146146

147147
if (audienceField.size() > 1) {
148-
final String siteCertificateFingerprintField = (String) audienceField.get(1);
148+
final String siteCertificateFingerprintField = audienceField.get(1);
149149
if (Strings.isNullOrEmpty(siteCertificateFingerprintField)
150150
|| !siteCertificateFingerprintField.startsWith("urn:cert:sha-256:")) {
151151
throw new TokenParseException("site certificate fingerprint from aud field must start with urn:cert:sha-256:");
@@ -179,7 +179,7 @@ private String getStringFieldOrThrow(Map<String, Object> fields, String fieldNam
179179
return fieldValue;
180180
}
181181

182-
private List getListFieldOrThrow(Map<String, Object> fields, String fieldName, String sectionName) throws TokenParseException {
182+
private List<String> getListFieldOrThrow(Map<String, Object> fields, String fieldName, String sectionName) throws TokenParseException {
183183
if (fields.get(fieldName) != null && !(fields.get(fieldName) instanceof List)) {
184184
throw new TokenParseException(fieldName + " field in authentication token "
185185
+ sectionName + " must be an array");
@@ -198,7 +198,9 @@ private List getListFieldOrThrow(Map<String, Object> fields, String fieldName, S
198198
+ sectionName + " must be an array of strings, but first element is not a string");
199199
}
200200

201-
return fieldValue;
201+
@SuppressWarnings("unchecked")
202+
final List<String> result = fieldValue;
203+
return result;
202204
}
203205

204206
private static X509Certificate parseCertificate(String certificateInBase64) throws CertificateException, IOException, TokenParseException {

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

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
/**
3939
* Stores configuration parameters for {@link AuthTokenValidatorImpl}.
4040
*/
41-
final class AuthTokenValidationConfiguration implements Cloneable {
41+
final class AuthTokenValidationConfiguration {
4242

4343
private URI siteOrigin;
4444
private Cache<String, LocalDateTime> nonceCache;
@@ -49,6 +49,20 @@ final class AuthTokenValidationConfiguration implements Cloneable {
4949
private boolean isSiteCertificateFingerprintValidationEnabled = false;
5050
private String siteCertificateSha256Fingerprint;
5151

52+
AuthTokenValidationConfiguration() {
53+
}
54+
55+
private AuthTokenValidationConfiguration(AuthTokenValidationConfiguration other) {
56+
this.siteOrigin = other.siteOrigin;
57+
this.nonceCache = other.nonceCache;
58+
this.trustedCACertificates = new ArrayList<>(other.trustedCACertificates);
59+
this.isUserCertificateRevocationCheckWithOcspEnabled = other.isUserCertificateRevocationCheckWithOcspEnabled;
60+
this.ocspRequestTimeout = other.ocspRequestTimeout;
61+
this.allowedClientClockSkew = other.allowedClientClockSkew;
62+
this.isSiteCertificateFingerprintValidationEnabled = other.isSiteCertificateFingerprintValidationEnabled;
63+
this.siteCertificateSha256Fingerprint = other.siteCertificateSha256Fingerprint;
64+
}
65+
5266
void setSiteOrigin(URI siteOrigin) {
5367
this.siteOrigin = siteOrigin;
5468
}
@@ -127,14 +141,7 @@ void validate() {
127141
}
128142
}
129143

130-
@Override
131-
protected AuthTokenValidationConfiguration clone() {
132-
try {
133-
final AuthTokenValidationConfiguration clone = (AuthTokenValidationConfiguration) super.clone();
134-
clone.trustedCACertificates = new ArrayList<>(trustedCACertificates);
135-
return clone;
136-
} catch (CloneNotSupportedException e) {
137-
throw new AssertionError(); // Can't happen
138-
}
144+
AuthTokenValidationConfiguration copy() {
145+
return new AuthTokenValidationConfiguration(this);
139146
}
140147
}

‎src/main/java/org/webeid/security/validator/AuthTokenValidatorImpl.java‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222

2323
package org.webeid.security.validator;
2424

25-
import com.google.common.base.Supplier;
2625
import com.google.common.base.Suppliers;
2726
import okhttp3.OkHttpClient;
2827
import org.slf4j.Logger;
@@ -32,6 +31,7 @@
3231
import org.webeid.security.validator.validators.*;
3332

3433
import java.security.cert.X509Certificate;
34+
import java.util.function.Supplier;
3535

3636
/**
3737
* Provides default implementation of {@link AuthTokenValidator}.
@@ -56,8 +56,8 @@ final class AuthTokenValidatorImpl implements AuthTokenValidator {
5656
* @param configuration configuration parameters for the token validator
5757
*/
5858
AuthTokenValidatorImpl(AuthTokenValidationConfiguration configuration) {
59-
// Clone the configuration object to make AuthTokenValidatorImpl immutable and thread-safe.
60-
this.configuration = configuration.clone();
59+
// Copy the configuration object to make AuthTokenValidatorImpl immutable and thread-safe.
60+
this.configuration = configuration.copy();
6161
/*
6262
* Lazy initialization, avoid constructing the OkHttpClient object when certificate revocation check is not enabled.
6363
* Returns a supplier which caches the instance retrieved during the first call to get() and returns
@@ -80,6 +80,7 @@ final class AuthTokenValidatorImpl implements AuthTokenValidator {
8080
).addOptional(configuration.isSiteCertificateFingerprintValidationEnabled(),
8181
new SiteCertificateFingerprintValidator(configuration.getSiteCertificateSha256Fingerprint())::validateSiteCertificateFingerprint
8282
);
83+
8384
}
8485

8586
@Override
@@ -99,14 +100,7 @@ public X509Certificate validate(String authToken) throws TokenValidationExceptio
99100

100101
simpleSubjectCertificateValidators.executeFor(actualTokenData);
101102

102-
final SubjectCertificateTrustedValidator certTrustedValidator =
103-
new SubjectCertificateTrustedValidator(configuration.getTrustedCACertificates());
104-
final ValidatorBatch certTrustValidators = ValidatorBatch.createFrom(
105-
certTrustedValidator::validateCertificateTrusted
106-
).addOptional(configuration.isUserCertificateRevocationCheckWithOcspEnabled(),
107-
new SubjectCertificateNotRevokedValidator(certTrustedValidator, httpClientSupplier.get())::validateCertificateNotRevoked
108-
);
109-
certTrustValidators.executeFor(actualTokenData);
103+
getCertTrustValidators().executeFor(actualTokenData);
110104

111105
authTokenParser.validateTokenSignatureAndParseClaims();
112106
authTokenParser.populateDataFromClaims(actualTokenData);
@@ -117,9 +111,27 @@ public X509Certificate validate(String authToken) throws TokenValidationExceptio
117111
return actualTokenData.getSubjectCertificate();
118112

119113
} catch (Exception e) {
114+
// Generally "log and rethrow" is an anti-pattern, but it fits with the surrounding logging style.
120115
LOG.warn("Token parsing and validation was interrupted:", e);
121116
throw e;
122117
}
123118
}
124119

120+
/**
121+
* Creates the certificate trust validator batch.
122+
* As SubjectCertificateTrustedValidator has mutable state that SubjectCertificateNotRevokedValidator depends on,
123+
* they cannot be reused/cached in an instance variable in a multi-threaded environment. Hence they are
124+
* re-created for each validation run for thread safety.
125+
*
126+
* @return certificate trust validator batch
127+
*/
128+
private ValidatorBatch getCertTrustValidators() {
129+
final SubjectCertificateTrustedValidator certTrustedValidator =
130+
new SubjectCertificateTrustedValidator(configuration.getTrustedCACertificates());
131+
return ValidatorBatch.createFrom(
132+
certTrustedValidator::validateCertificateTrusted
133+
).addOptional(configuration.isUserCertificateRevocationCheckWithOcspEnabled(),
134+
new SubjectCertificateNotRevokedValidator(certTrustedValidator, httpClientSupplier.get())::validateCertificateNotRevoked
135+
);
136+
}
125137
}

0 commit comments

Comments
 (0)