Conversation
fredrbus
requested review from
a team,
fredrikbach,
martin-jackson and
runeflobakk
September 2, 2026 17:43
martin-jackson
left a comment
martin-jackson
left a comment
There was a problem hiding this comment.
Heftig shit!
Sliter litt med å få testet med testklienter: Fikk først kompilseringsfeil på JwtAuthConfig::newBuilder, men ser ut som at det bare er pga commit 872aa89.
Etter å ha builderen til følgende får jeg 401 fra midp:
JwtAuthConfig jwtAuthConfig = JwtAuthConfig
.newConfig(clientId)
.tokenEndpoint(midpTokenUri.toString())
.pkcs12KeyStore(keyStream, keyPassword.getPassword())
.build();Exception in thread "main" no.digipost.api.client.errorhandling.DigipostClientException: FAILED_TO_OBTAIN_ACCESS_TOKEN: Token endpoint returned HTTP 401 for https://midp.qa.digipost.no/oauth2/token: {"error_description":"Client authentication failed: client_id","error":"invalid_client","error_uri":"https://datatracker.ietf.org/doc/html/rfc6749#section-3.2.1"}
at no.digipost.api.client.security.jwt.MutualTlsTokenProvider.lambda$fetchAndCacheToken$0(MutualTlsTokenProvider.java:107)
at org.apache.hc.client5.http.impl.classic.CloseableHttpClient.execute(CloseableHttpClient.java:247)
at org.apache.hc.client5.http.impl.classic.CloseableHttpClient.execute(CloseableHttpClient.java:188)
at org.apache.hc.client5.http.impl.classic.CloseableHttpClient.execute(CloseableHttpClient.java:162)
at no.digipost.api.client.security.jwt.MutualTlsTokenProvider.fetchAndCacheToken(MutualTlsTokenProvider.java:103)
at no.digipost.api.client.security.jwt.MutualTlsTokenProvider.getToken(MutualTlsTokenProvider.java:90)
at no.digipost.api.client.internal.http.request.interceptor.RequestBearerTokenInterceptor.process(RequestBearerTokenInterceptor.java:36)
at org.apache.hc.core5.http.protocol.DefaultHttpProcessor.process(DefaultHttpProcessor.java:107)
Contributor
Author
Mulig databasen har blitt resatt, skal få dyttet inn ny testdata |
arneroen
reviewed
Sep 8, 2026
arneroen
left a comment
arneroen
left a comment
Member
There was a problem hiding this comment.
Ref meldinga i midp-kanalen på slack her for leden. Vi må bli enige om hvordan vi håndterer 401 fra API-serveren. Minimum tenker jeg at det cachede tokenet bør invalideres. Og så tror jeg helt fint vi kan hente et nytt token og retrye requesten mot APIet.
fredrbus
force-pushed
the
jwt-autentisering
branch
from
September 10, 2026 08:43
80f49d3 to
0a92a2f
Compare
Member
|
Det jeg ikke helt henger med på er hvorfor vi trenger å lage dette bakoverkompatibelt. Nå legges det opp til en ny major, men jeg ser ikke noe breaking i apiet her. |
Member
Da jeg prata med Fredrik om det, var begrunnelsen at vi dessverre ikke kan gå ut ifra at alle kundene våre umiddelbart kommer seg over på ny auth-løsning. Ved å da beholde begge metodene, gjør vi det lettere for oss selv å potensielt patche sikkerhets-hull i framtiden, uten å måtte release to versjoner for gammel og ny auth. |
sondrew
reviewed
Sep 21, 2026
The client can now authenticate in two ways: the existing certificate-based signing (Signer), or OAuth 2.0 client credentials with a certificate-bound JWT (RFC 8705) over mutual TLS. - JwtAuthConfig: configures the token endpoint, resource server, clientId and client certificate (PKCS12 keystore or KeyStore). - MutualTlsTokenProvider: fetches and caches an access token from mIdP over mTLS, with a refresh margin and expiry derived from expires_in or the exp claim. - RequestBearerTokenInterceptor: sets the Authorization:Bearer header. - ApiServiceImpl selects the authentication mode based on whether a Signer or a JwtAuthConfig is set, and throws when neither is configured. - New DigipostClient constructors without a Signer.
Moving this out of RequestSignatureInterceptor, as we need this functionality also for OAuth-based authentication, which does not use the RequestSignatureInterceptor. Also defined the attribute name as a constant in the new interceptor to make the connection between the interceptor and verification step clearer.
The client previously selected its authentication mode based on whether a Signer was null, and the JWT/mTLS config was hidden inside DigipostClientConfig. The choice was scattered and easy to misconfigure. - Introduce DigipostClient.withCertificateAuthentication(...) and withJwtMtlsAuthentication(...) (each with an HttpClientBuilder overload), so the chosen authentication method is stated at the call site and the required credential cannot be forgotten. - Replace the implicit "signer == null" selection with an explicit AuthMode enum resolved in one place (ApiServiceImpl#resolveAuthMode). It now also throws when both certificate and JWT/mTLS auth are configured, or neither. - Move JwtAuthConfig out of DigipostClientConfig; it is now a required argument to the JWT factory method. - Add Javadoc for the factory methods, including the clientBuilder parameter. - Add ApiServiceImplAuthModeTest covering all four resolution cases. BREAKING CHANGE: the DigipostClient(config, brokerId, signer[, clientBuilder]) constructors and the implicit no-signer constructors have been removed. All call sites (example code and DigipostSwingClient) have been migrated to the new factory methods.
Every versioned docs page carried redirect_from: /, so several versions claimed the site root and the target became ambiguous. Drop it from the older versions so only v19 (the current version) owns the root redirect.
The resource URI could be set both in JwtAuthConfig.apiUri and DigipostClientConfig.digipostApiUri, both defaulting to production. A client pointed at test through DigipostClientConfig alone got tokens for production.
Token endpoint failures surfaced as IllegalStateException, so callers could not handle them like the rest of the client's errors. Also keep the cause when the response is not valid JSON.
The test built its own SSLContext and HTTP client, so it verified the test's handshake rather than the provider's. It passed even with the provider's logic untouched. Let the provider's trust managers be overridden, so the test can drive the real getToken() path: mTLS handshake, request parameters, caching, expiry from the exp claim, and error mapping.
The enum and the unreachable switch default guarded against states only the internal constructor could create. The factories make the mode a property of the call, so no argument has to be null.
The interceptors were handed the eventLogger already wrapped for ApiServiceImpl's logger, so every message reached slf4j twice.
Taking a Supplier<String> instead of MutualTlsTokenProvider lets the interceptor be tested without a keystore and a TLS handshake.
The name stuttered "Request" twice.
Also fixes "certificate-base" -> "certificate-based", and states on the JWT methods that tokens are requested for the API given by config.
In practice, this check was only a check for the order of the request-interceptors, which are hard-coded anyways.
SslConnectionFactory-methods are deprecated.
ApiServiceImpl only closed its own http client. In JWT/mTLS mode the MutualTlsTokenProvider holds a separate client with its own connection pool, and nothing ever closed it. Keep the token provider as a field and close it together with the API-service. Since ApiServiceImpl is internal, its close()-method was unreachable from the public API. Therefore, we also implement AutoCloseable in the DigipostClient, which closes the API-service if, and only if, it created the API-service itself.
DigipostClient kept the API-service it created itself in a nullable field, where null meant "the caller provided the API-services, and owns them". A no-op AutoCloseable states the same intent without the null check, and close() no longer needs to know what it is closing.
On 401 from the server, invalidate the used access token and fetch a new one, before retrying the request. Implementing this as an ExecChainHandler, and placing it before ChainElement.PROTOCOL, allowing the other interceptors to run again, without touching the HttpRequestRetryStrategy. The handler proceeds down to the chain at most twice and never inspects the second response, so we don't need a counter to ensure that the request is only retried once. Also adding a VerifyUnlessUnauthorized-wrapper-interceptor, that only uses the wrapped response interceptor if the response was NOT a 401. This ensures that the wrapped interceptor does not throw an exception due to missing/invalid headers before we can handle the 401.
BatchSendMessagesEksempel also dropped its custom connection manager. The JWT client replaces the connection manager to install the mTLS socket strategy.
Drops both DigipostClient.withCertificateAuthentication overloads, the matching ApiServiceImpl factory and the interceptor chain it built. JWT/mTLS is now the only way to construct a client.
Response signature verification is untouched as the server signs its responses.
loadKeyFromP12 and sign were only ever called from Signer. JwtAuthConfig loads its own keystore, so there is nothing left to reuse.
The constructor took a Function<ApiServiceImpl, CloseableHttpClient> only so the two static factories could each pick their own mode. With one mode left it can call the instance method directly.
Drops the legacy certificate section and the "choose one of two"-framing. Instead, we add a migration section.
fredrbus
force-pushed
the
jwt-autentisering
branch
from
September 23, 2026 17:06
1b0a484 to
a1527eb
Compare
The API does not validate the cnf claim, so access tokens are not bound to the client certificate and the API client has no need to present it.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OAuth 2-autentisering mot Digipost API
Legger til funksjonalitet slik at klienter kan autentisere seg mot Digipost API med JWT, med OAuth-klienter registrert i Nyva og tokens hentet fra mIdP'en.
Denne PR'en er altså klientsiden av denne PR'en i digipost/digipost, som allerede er i prod.
Fremangsmåte
Jeg ville gått gjennom commit for commit og lest commitmeldingene, men ikke nødvendigvis allt koden da det har vært en del frem og tilbake her.
Klassene jeg ville fokusert på er:
Kort oppsummert
Bakoverkompabilitet
Endringene er bakoverkompatible med eksisterende autentiseringsløsning. Valg av autentiseringsmetode gjøres nå med factorymetodeneEndringene er ikke bakoverkompatible. Legacy sertifikatautentisering støttes av Digipost API, men ikke dette klientbiblioteket f.o.m. v19.
Henting og caching av tokens
Klienten håndterer henting og caching av tokens selv. "Batteries included", som jeg sikkert hadde sagt om jeg ikke var så jordnær og ydmyk som jeg jo er.
Testing
Jeg har også laget en demo i testklienter, på branch test-jwt-autentisering. Der finner man testen
no.digipost.testclients.dpostapi.oauth2.OAuth2SendeKlientQa, som tester sending av dokumenter med JWT-autentisering mot QA.