permit at+jwt typ header value in jwt access tokens #126687
Conversation
richard-dennehy
added
>enhancement
:Security/Authentication
|
Hi @richard-dennehy, I've created a changelog YAML for you. |
| public static final JwtTypeValidator ID_TOKEN_INSTANCE = new JwtTypeValidator(JOSEObjectType.JWT, null); | ||
|
|
||
| // strictly speaking, this should only permit `at+jwt`, but removing the other two options is a breaking change | ||
| public static final JwtTypeValidator ACCESS_TOKEN_INSTANCE = new JwtTypeValidator( |
There was a problem hiding this comment.
split validator into 2, as we don't want to allow at+jwt for ID tokens
|
Pinging @elastic/es-security (Team:Security) |
There was a problem hiding this comment.
LGTM 🚀
For additional functional coverage, I'd also tweak one of our JWT REST tests (here) to randomly set a at+jwt header -- provided that it's an application token JWT. This will take a bit of tweaking to the method signature, i.e., something like a boolean flag allowAtJwtType.
Let's also open a backport to 8.19.
| } catch (Exception e) { | ||
| throw new AssertionError("validation should have passed without exception", e); | ||
| } | ||
| } | ||
|
|
||
| public void testInvalidType() throws ParseException { | ||
| final JwtTypeValidator validator = randomFrom(JwtTypeValidator.ID_TOKEN_INSTANCE, JwtTypeValidator.ACCESS_TOKEN_INSTANCE); | ||
|
|
||
| final JWSHeader jwsHeader = JWSHeader.parse( | ||
| Map.of("typ", randomAlphaOfLengthBetween(4, 8), "alg", randomAlphaOfLengthBetween(3, 8)) |
There was a problem hiding this comment.
Could also add AT+JWT here -- this case isn't captured by a random alpha string and seems like an edge-case worth explicitly covering as invalid.
There was a problem hiding this comment.
turns out the parser ignores case - I've added AT+JWT to the success test case instead
| JOSEObjectType.JWT, | ||
| new JOSEObjectType("at+jwt"), |
There was a problem hiding this comment.
Nit: let's make this a private constant
richard-dennehy
added
auto-backport
💚 Backport successful
|
* permit at+jwt typ header value in jwt access tokens * Update docs/changelog/126687.yaml * address review comments * [CI] Auto commit changes from spotless * update Type Validator tests for parser ignoring case --------- Co-authored-by: elasticsearchmachine <[email protected]>
* permit at+jwt typ header value in jwt access tokens * Update docs/changelog/126687.yaml * address review comments * [CI] Auto commit changes from spotless * update Type Validator tests for parser ignoring case --------- Co-authored-by: elasticsearchmachine <[email protected]>
Update JWT documentation to mention `at+jwt` support introduced [here](elastic/elasticsearch#126687) Rendered [here](https://docs-v3-preview.elastic.dev/elastic/docs-content/pull/1124/deploy-manage/users-roles/cluster-or-deployment-auth/jwt#jwt-validation-header) --------- Co-authored-by: Liam Thompson <[email protected]>
Resolves #119370
Permit
at+jwttypheader values when parsing Access tokens, as required by RFC 9068.We continue to accept
typ=JWTornullto maintain backwards compatibility.