Update oidc related dependencies #71521
Conversation
Update: - json-smart to 2.4.2 - accessors-smart to 2.4.2 - asm to 8.0.1 - nimbus-jose-jwt to 9.8.1 - oauth2-oidc-sdk to 9.3.1
jkakavas
added
>non-issue
:Security/Authentication
|
Pinging @elastic/es-security (Team:Security) |
There was a problem hiding this comment.
LGTM with some questions for which I'd appreciated for clarifications.
Also I feel it could be useful (for future readers) to have a bit more details in the PR's description to include:
- General purpose of this upgrade. Is it just on a regular schedule or is it related to some requirements?
fromversion in addtion to thetoversion- Any noticable changes each dependency's upgrade or it can just be "non-issue" if nothing significant.
| @@ -234,9 +233,9 @@ private void getUserClaims(@Nullable AccessToken accessToken, JWT idToken, Nonce | |||
| LOGGER.trace("Received and validated the Id Token for the user: [{}]", verifiedIdTokenClaims); | |||
| } | |||
| // Add the Id Token string as a synthetic claim | |||
| final JSONObject verifiedIdTokenClaimsObject = verifiedIdTokenClaims.toJSONObject(); | |||
| final Map<String, Object> verifiedIdTokenClaimsObject = verifiedIdTokenClaims.toJSONObject(); | |||
| final JWTClaimsSet idTokenClaim = new JWTClaimsSet.Builder().claim("id_token_hint", idToken.serialize()).build(); | |||
There was a problem hiding this comment.
Also a question of learning: Why do we add the id token string itself as part of the claim?
There was a problem hiding this comment.
We need to pass the id_token_hint parameter that has the serialized ID Token as value, when/if we eventually need to generate a Logout request for the OP, so we need to store this in the elasticsearch AuthenticationToken metadata for it to be available. The easiest way to do this here was to add this as a synthetic claim, as we add all claims from the ID token to the metadata in OpenIdConnectRealm
| // This is necessary as if nonce claim is of type Nonce, the library won't take it into consideration when serializing the JWT | ||
| idTokenMap.put("nonce", idTokenMap.get("nonce").toString()); |
There was a problem hiding this comment.
Could you please explain a bit more about this? Is it not needed before the version upgrade? Since it is necessary now, do we need somehow reflect this in production code?
There was a problem hiding this comment.
Could you please explain a bit more about this?
It looks like some internal change ( I assume the change from using json-smart's JSONObjects to Map<String, Object> in oauth2-oidc-sdk, means that ID token signature generation can only handle certain types.
Is it not needed before the version upgrade?
No
Since it is necessary now, do we need somehow reflect this in production code?
No. We are an RP and thus we don't need to generate signed ID Tokens, so this behavior change doesn't affect us.
|
@elasticmachine update branch |
Update: Non-issue, no notable changes. - json-smart from 2.3 to 2.4.2 - accessors-smart from 1.2 to 2.4.2 - asm from 7.1 to 8.0.1 - nimbus-jose-jwt from 8.6 to 9.8.1 - oauth2-oidc-sdk from 7.0.2 to 9.3.1 # Conflicts: # x-pack/plugin/security/build.gradle
Update:
Non-issue, no notable changes.