Feature/808 validate root certificates
Merge request reports
Activity
assigned to @Martin_Vogel
160 161 161 162 // Then 162 163 assertTrue(validationResult.hasError()); 163 assertThat(validationResult.getError().getMessage(), containsString("\"certChain\" is null")); 164 assertThat(validationResult.getError().getMessage(), containsString("\"x5c\" is null")); @Martin_Vogel Schau mal hier scharf drauf. Bislang war die Validierung ja recht harmlos, jetzt passiert hier wirklich was. Der
JWKValidator
wirft diesen "x5c"-Fehler und ich denke genau das sollten wir auch epxtecten.Edited by Henry Borasch
9 import java.security.cert.CertificateFactory; 10 import java.security.cert.X509Certificate; 11 import java.util.Arrays; 12 import java.util.List; 13 import java.util.Objects; 14 import java.util.stream.Collectors; 15 16 public class FileUtil { 17 18 public static List<String> loadContentOfFilesInDirectory(String directoryPath) { 19 20 File folder = new File(Objects.requireNonNull(FileUtil.class.getClassLoader().getResource(directoryPath)).getFile()); 21 22 return Arrays.stream(Objects.requireNonNull(folder.listFiles())).map(file -> { 23 try { 24 return Files.readString(file.toPath()); Der Teil, den beide Methoden beinhakten sieht so aus:
try { return Files.readString(...); } catch (IOException e) { throw new RuntimeException(e); } }
Im Grunde geht es ja nur um den Aufruf von
Files.readString
plus Exception-Behandlung. Das in eine weitereprivate
-Methode auszulagern würde wenige Zeilen Code einsparen, die Nachvollziehbarkeit jedoch erschweren. Ich persönlich bin kein Freund von diesem Methoden-Hopping, nur weil da ein, zwei Zeilen doppelt sind. Den aktuellen Stand empfinde ich als intuitiver,
21 22 return Arrays.stream(Objects.requireNonNull(folder.listFiles())).map(file -> { 23 try { 24 return Files.readString(file.toPath()); 25 } catch (IOException e) { 26 throw new RuntimeException(e); 27 } 28 }).collect(Collectors.toList()); 29 } 30 31 public static String loadContentOfFile(String filePath) { 32 33 try { 34 return Files.readString(new File(Objects.requireNonNull(FileUtil.class.getClassLoader().getResource(filePath)).getFile()).toPath()); 35 } catch (IOException e) { 36 throw new RuntimeException(e); changed this line in version 3 of the diff
58 58 private static final Logger LOGGER = LoggerFactory.getLogger(DefaultValidationService.class); 59 59 private static final ObjectMapper MAPPER = new ObjectMapper().setDateFormat(new SimpleDateFormat("yyyy-MM-dd")); 60 60 private static final JsonSchemaFactory SCHEMA_FACTORY = JsonSchemaFactory.getInstance(SpecVersion.VersionFlag.V202012); 61 public static final String VALID_SCHEMA_URL_EXPRESSION = "https://schema\\.fitko\\.de/fit-connect/metadata/1\\.\\d+\\.\\d+/metadata.schema.json"; 61 private static final String VALID_SCHEMA_URL_EXPRESSION = "https://schema\\.fitko\\.de/fit-connect/metadata/1\\.\\d+\\.\\d+/metadata.schema.json"; 62 62 63 63 private final MessageDigestService messageDigestService; 64 64 private final SchemaProvider schemaProvider; 65 65 private final ApplicationConfig config; 66 private final String pathToTrustedRootCertificates; 66 67 67 public DefaultValidationService(final ApplicationConfig config, final MessageDigestService messageDigestService, final SchemaProvider schemaProvider) { 68 public DefaultValidationService(final ApplicationConfig config, final MessageDigestService messageDigestService, 69 final SchemaProvider schemaProvider, final String pathToTrustedRootCertificates) { Statt des Pfades
pathToTrustedRootCertificates
wäre es hier besser die RootCerts fertig geladen an den Service zu übergeben. Dann gibt es keine blackbox was das Laden angeht und der Service muss sich nicht selber darum kümmern. Eventuell direkt in der ClientFactory ? Hier werde ja auch schon andere resourcen geladenchanged this line in version 4 of the diff
220 validateWithX509AndWithoutProxy(trustedRootCertificates, publicKey, purpose); 234 221 } 235 222 } 236 223 237 private List<String> getPemCerts(final List<X509Certificate> certChain) throws CertificateEncodingException { 238 final List<String> pemCerts = new ArrayList<>(); 239 for (final X509Certificate cert : certChain) { 240 pemCerts.add(getEncodedString(cert)); 241 } 242 return pemCerts; 243 } 224 private List<String> loadTrustedRootCertificates() { 225 226 List<X509Certificate> trustedRootCertificates = FileUtil.loadContentOfFilesInDirectory(this.pathToTrustedRootCertificates) 227 .stream().map(FileUtil::convertToX509Certificate).collect(Collectors.toList()); 228 List<String> encodedCertificates = trustedRootCertificates.stream().map(cert -> { Wenn das Base64 encoding auch in den FileUtils gemacht würde könnte der Stream zweimal mappen:
return ...stream() .map(FileUtil::convertToX509Certificate) .map(FileUtil::encodeBase64) .collect(Collectors.toList())
Edited by Martin Vogelchanged this line in version 6 of the diff
added 28 commits
-
38d26bc1...7c225b03 - 26 commits from branch
main
- 6907a1b7 - Merge branch 'main' into feature/808-validate_root_certificates
- 86cfcdf2 - merged changes from master
-
38d26bc1...7c225b03 - 26 commits from branch
added 14 commits
-
86cfcdf2...3fbd8d7f - 12 commits from branch
main
- 9ac3cb74 - Merge branch 'main' into feature/808-validate_root_certificates
- 57878187 - added more specific exceptions
-
86cfcdf2...3fbd8d7f - 12 commits from branch
added 1 commit
- 5c933bd1 - moved loading of root certificates to ClientFactory
added 1 commit
- 0e5dd4c6 - moved test certificates to other module, where it is actually needed
added 1 commit
- 7943f1a4 - added test certificates to an additional module
248 235 } else { 249 validateWithX509AndWithoutProxy(pemCerts, publicKey, purpose); 236 validateWithX509AndWithoutProxy(trustedRootCertificates, publicKey, purpose); 250 237 } 251 238 } 252 239 253 private List<String> getPemCerts(final List<X509Certificate> certChain) throws CertificateEncodingException { 254 final List<String> pemCerts = new ArrayList<>(); 255 for (final X509Certificate cert : certChain) { 256 pemCerts.add(getEncodedString(cert)); 257 } 258 return pemCerts; 259 } 240 private List<String> loadTrustedRootCertificates() { 241 242 List<String> trustedRootCertificates = rootCertificates.stream() changed this line in version 14 of the diff
added 5 commits
-
7943f1a4...4fddf5e9 - 4 commits from branch
main
- 1e8ccf22 - Merge branch 'main' into feature/808-validate_root_certificates
-
7943f1a4...4fddf5e9 - 4 commits from branch
added 24 commits
-
1e8ccf22...a6393e41 - 23 commits from branch
main
- ee684518 - Merge branch 'main' into feature/808-validate_root_certificates
-
1e8ccf22...a6393e41 - 23 commits from branch
added 1 commit
- bbef99b4 - added certificates to integration test resources
added 7 commits
-
bbef99b4...d75c9f7a - 6 commits from branch
main
- 64772405 - Merge branch 'main' into feature/808-validate_root_certificates
-
bbef99b4...d75c9f7a - 6 commits from branch
added 10 commits
- 9db92581 - make JWKValidator throw always exceptions if something fails
- 40c7d11f - fixed test
- 62876ecf - adjusted and fixed tests
- 0e599e5a - fixed test
- 7f58dba9 - use more ValidationResults instead of exceptions
- 18c72b98 - simplified exception handling, adjusted tests
- 9ff9bb3e - Merge branch 'feature/808-validate_root_certificates' into...
- ad92212b - Merge remote-tracking branch 'origin/feature/808-validate_root_certificates'...
- 669cb314 - added missing imports
- 06d891df - fixed broken purpose transmission
Toggle commit list