-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
EPA-111: EntityStatement tweaks for compatibility #76
Conversation
var cert = | ||
X509CertificateUtils.generateSelfSigned( | ||
new Issuer(issuer), | ||
Date.from(nbf), | ||
Date.from(exp), | ||
key.toPublicKey(), | ||
key.toPrivateKey()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the important bit to have an mTLS client cert.
@@ -109,7 +111,13 @@ public void start() throws ExecutionException, InterruptedException { | |||
var tokenIssuer = new TokenIssuerImpl(config.baseUri(), keyStore, codeRepo); | |||
var sessionRepo = buildSessionRepo(config.sessionStore(), meterRegistry); | |||
|
|||
var httpClient = HttpClient.newBuilder().connectTimeout(Duration.ofSeconds(5)).build(); | |||
var sslContext = TlsContext.fromClientCertificate(config.federation().entitySigningKey()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This adds the client certificate to the list of certificates to use for authentication.
|
||
// according to the federation spec this is not required here, some | ||
// sectoral IdPs require it though | ||
.defaultAcrValues(List.of("gematik-ehealth-loa-high")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see also #74
import org.bouncycastle.operator.OperatorCreationException; | ||
import org.junit.jupiter.api.Test; | ||
|
||
class TlsContextTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO :D
84e525f
to
6ba3933
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, good job 💪 some minor nitpicks, also feel free to re-request once you have more complete tests
ehealthid-cli/src/main/java/com/oviva/ehealthid/cli/KeyGeneratorCommand.java
Show resolved
Hide resolved
private JWKSet generateJwks(KeyUse keyUse) throws JOSEException { | ||
var key = new ECKeyGenerator(Curve.P_256).keyUse(keyUse).keyIDFromThumbprint(true).generate(); | ||
return new JWKSet(key); | ||
private String deriveName(URI issuer) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you provide a clearer name? maybe deriveFdpNameFromHost
or something similar?
return new JWKSet(key); | ||
private String deriveName(URI issuer) { | ||
var s = issuer.toString(); | ||
s = s.replaceAll("^https://", ""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hopefully nobody is using plain http 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If they do, it won't work with the federation 🤷
9eeae2e
to
3052e4f
Compare
8cd4048
to
bedf143
Compare
|
also fixes #74
now the following sectoral IdPs somewhat work:
RISE is still somehow broken