Previously HttpSessionOAuth2AuthorizationRequestRepository
getAuthorizationRequest attempted to update the state of HttpSession as
well as getting the Map of OAuth2AuthorizationRequest. This had a few
problems
- First it was confusing that a get method updated state
- It worked when the session was in memory, but would not work when the
HttpSesson was persisted to an external store (i.e. Spring Session) since
after updating the Map, there was no invocation to update
This commit cleans up the logic and ensures that the values are explicitly
set in the HttpSession so it works with a session persisted in an external
store.
Fixes: gh-5146
Today, @rwinch and I were discussing the merits of leaving commit messages free of formatting hints, like back-ticks. Adding this bullet-point brings things into line with expectations.
In other extensions of `AbstractAuthenticationToken`, the constructors
that include `authorities` call `setAuthenticated(true)`. This includes
`PreAuthenticated`-, `UsernamePassword`-, and
`RememberMeAuthenticationToken`.
This change brings `TestingAuthenticationToken` in line with that
convention.
Note that this was done once already to one of the constructors
(ee13be4) in `TestingAuthenticationToken` that takes an arity of
`authorities`. It was not propagated to the constructor that takes a
collection, which is what this commit remedies.
Fixes: gh-5073
Note that the `WhenUsingAspectJ` tests are still simply verifying structure instead of behavior. This is because the project appearsto be misconfigured in some way such that AspectJ advice isn't getting woven in at runtime. The original Groovy tests also only verified structure and they may be that way for a similar reason.
Either way, I will open up a ticket so we can review why that is the case and if there is a good fix.
Issue: gh-4939
Note that the old groovy test used a configuration of
```
http
.authorizeRequests()
.anyRequest().hasAnyAuthority("USER")
```
However, as I read the issue, gh-2984, the problem this issue
identifies is the non-passive change of defaulting to prefix
ROLE_ with all role-based configuration methods. So, the test now
does the following:
```
http
.authorizeRequests()
.anyRequest().access("hasAnyRole('USER')")
```
which demonstrates, given the configuration in this test, that
ROLE_ is correctly not prefixed in this expression, even though
it is a role-based configuration.
Issue: gh-4939
Joe Grandja
Rob Winch
Oleh Dokuka
Vedran Pavic
Josh Cummings
ylombardi