It would be handy in tests to be able to specify a Jwt authentication in tests:
this.mockMvc.perform(get("/")
.with(jwt()))
.andExcept(status().isOk());
Some features we should consider:
- Providing a completely instantiated
Jwt:
Jwt jwt = ...;
this.mockMvc.perform(get("/")
.with(jwt(jwt)))
...
- Providing authorities:
Jwt jwt = ...;
this.mockMvc.perform(get("/")
.with(jwt(jwt).authorities("SCOPE_message:read"))
...
- Providing claims:
this.mockMvc.perform(get("/")
.with(jwt().claim(SUB, "the-subject")))
...
- Providing headers:
this.mockMvc.perform(get("/")
.with(jwt().header("alg", RS256)))
...
These would result in the SecurityContext containing an instance of JwtAuthenticationToken.
Also, we should introduce its reactive equivalent.
Comment From: ch4mpy
Using mockJwt() instead of jwt() to be consistent with existing mockUser(). It also make things consistent with annotations: with(mockJwt()) <=> @WithMockJwt
Missed the part of the spec with the Jwt instance. Will fix that soon.
Went further this spec as I propose equivalents for Bearer access-token and OidcId token.
All is based on commons I initiated for annotations equivalents (https://github.com/spring-projects/spring-security/issues/6557)
Branches are ordered as follow (so PRs are streamed as follow):
1. gh-6634--jwt-servlet-flow mockJwt() flow API for MockMvc
2. gh-6634--access-token-servlet-flow mockAccessToken() flow API for MockMvc
3. gh-6634--oidcid-servlet-flow mockOidcId() flow API for MockMvc
4. gh-6557--jwt-reactive-flow mockJwt() flow API for reactive server
5. gh-6557--access-token-reactive-flow augment reactive flow with mockOidcId()
6. gh-6557--oidcid-reactive-flow mockOidcId() flow API for reactive server
Comment From: ch4mpy
@jgrandja, @rwinch, could you please confirm your intention is not cutting the merges after JWT flow APIs (servlet & reactive)? I got this feeling because of the comments asking to drop some abstractions.
To me, limiting features to "flow API for Jwt secured @Controllers, without scope claim support" is more of a gadget than an "Spring5 OAuth2 unit-test support":
* doesn't allow for secured "services" testing (annotations are required for that)
* tests will throw cast exceptions when @AuthenticationPrincipal is expected to return something else than a Jwt. Bearer access-token and OpenId implementations seem a minimum
* as far as I understood source code, scope claims are processed as standard by spring-security . Test lib should be concistent and do the same
* most standard claims are useless at @Components unit-test time. To me, claims support comes to be intersting with custom ones only: if one puts custom claim in a token, it is likely to use it from business code (I mean somewhere in a controller or service). As so, removing this support seams important loss.
I tried to correct as much as possible before going at see again but am quite unsatisfied with my push:
* Applied modifications on a gh-6557--reviews branch containing everything to benefit IDE refactoring features but run out of time to report on gh-6634--jwt-reactive-flow branch which is the origin of the PR you are commenting
* I pulled master and rebased on it but can't compile any more as some new dependencies won't download from Cuba => this gh-6557--reviews mentioned above is in a very dirty state
Will improve the situation from Jamaïca probably next week-end.
Comment From: ch4mpy
@jzheaux first, please forgive me for my last comment typo, It's you who I intented to ping at first rank as you are the one reviewing the PR. Do you have request changes I missed, still needing change? I'll be available frequently next week, but offline again mot of first half of May.
Comment From: jzheaux
There's been some discussion about the contract on #6748, and I'd like to update the ticket here with some changes to the original proposal found in the ticket's description.
It will likely be more flexible to separate Jwt configuration from any remaining authentication configuration, so let's do:
this.mockMvc.perform(get("/")
.with(jwt()))
.andExcept(status().isOk());
as already stated.
Let's also support:
- Providing claims:
this.mockMvc.perform(get("/")
.with(jwt(jwt -> jwt.claim(SUB, "the-subject"))))
...
- Providing headers:
this.mockMvc.perform(get("/")
.with(jwt(jwt -> jwt.header("alg", RS256))))
...
- Providing authorities:
this.mockMvc.perform(get("/")
.with(jwt().authorities("SCOPE_message:read")))
...
// or
this.mockMvc.perform(get("/")
.with(jwt(jwt -> jwt.claim("scope", "message:read"))))
...
// or
this.mockMvc.perform(get("/")
.with(jwt().authorities(jwt -> toGrantedAuthorities(jwt))))
...
We can easily leverage the output of #6851 to make this a simple introduction of SecurityMockServerConfigurers.JwtMutator and SecurityMockMvcRequestPostProcessors.JwtRequestPostProcessor.
Comment From: ch4mpy
In addition to what was written above by @jzheaux (and knowing following are points we do not agree on):
Regarding the JwtAuthenticationToken.Builder, but this applies to any OAuth2 authentication builders (being a MockMvc request post processor, WebTestClient configurer or annotation SecurityContextFactory delegate) contract:
1. I think it would be strange for a builder to have immutable values. Having the most important property in an OAuth2 authentication (the token) immutable would be surprising. IMO, all OAuth2 authentication builders should expose a token(Consumer<TOKEN_TYPE.Builder> tokenBuilderConsumer) method (TOKEN_TYPE being Jwt or alike)
2. I repeat here what I wrote several times: I think there should be an "authoritiesConverter" setter instead of an "authorities" one on OAuth2 authentication builder because authorities can be derived from tokens using converters but token can't be updated from authorities (and per spec, in OAuth2, the tokens are the reference regarding authorization). Setting authorities independently of the token will lead for sure to incoherence between token and granted authorities. Given that spring security expressions can be about anything including the tokens and not just authorities, this can cause very subtle test bugs.
Regarding token builders, when there are standard claims (Jwt, Introspection, OpenId, ...), it should expose setters for each. Applications using private claims, can then extend framework builders to support their own claims. So with a Jwt Builder consumer as from @jzheaux sample, this would mean:
jwt().token(jwt -> jwt.subject("user").expiresAt(Instant.now)); // etc.
"scope" is OAuth2 tokens meta-data (not a claim but part of the successful authorization response for the token). As there is no class I'm aware of in authentication stack that holds both the token and it's meta-data, it would make sens to also expose a method to add well-formatted (conform to the OAuth2 RFC) scope claim to authentication builders:
jwt().scopes("message:read", "message:write");
Most of this is more or less implemented there: https://github.com/ch4mpy/spring-addons/tree/master/spring-security-test-oauth2-addons
P.S. @jzheaux unfortunately, I'm timed up. I have to go at sea again to reach Panama, might be quite occupied solving formalities to use the canal there and will be at sea for more than a month after before I reach French Polynesia. I think you can modify My repo to make the PR at your wish. You can also copy / paste whatever you like to a new repo, I'd don't really mind being an author, even if I put a lot of energy in this (and time and money...)
Comment From: jzheaux
I think you can modify My repo to make the PR at your wish. You can also copy / paste whatever you like to a new repo, I'd don't really mind being an author, even if I put a lot of energy in this (and time and money...)
Your contribution is much appreciated, we certainly wouldn't have jwt test support without it, and you deserve the credit for that. I'll consult with the team to see what is the right maneuver here.