Add expireAt to OAuth2AuthorizationRequest and clean-up expired requests on save

[AndreasKl]

Fixes gh-5145.

[jgrandja]

Thanks for the PR @AndreasKl.

There is quite a bit of coupling in the OAuth2AuthorizationRequestResolver and AuthorizationRequestRepository and it's implementations (and Reactive). I feel we should rework things to remove this coupling and potentially allow for reuse in other areas within the OAuth support, eg. expiring PK from a JWK Cache.

The expiring capability is really a cross-cutting concern. Can you try the following solution?

• Intercept calls to loadAuthorizationRequest() and store the information necessary
• At a scheduled time (configurable with skew), the component will attempt to loadAuthorizationRequest() and if it returns than determine if time has expired and than removeAuthorizationRequest

Please take a look at the Cache abstraction to see if it would meet our needs.

[jgrandja]

An Authorization Request does not have an expiresAt attribute/parameter as per spec. This really is an implementation detail and should be externalized to the expiring component.

[AndreasKl]

Thanks for the PR @AndreasKl.

There is quite a bit of coupling in the OAuth2AuthorizationRequestResolver and AuthorizationRequestRepository and it's implementations (and Reactive). I feel we should rework things to remove this coupling and potentially allow for reuse in other areas within the OAuth support, eg. expiring PK from a JWK Cache.

The expiring capability is really a cross-cutting concern. Can you try the following solution?

* Intercept calls to `loadAuthorizationRequest()` and store the information necessary

* At a scheduled time (configurable with skew), the component will attempt to `loadAuthorizationRequest()` and if it returns than determine if time has expired and than `removeAuthorizationRequest`

Please take a look at the Cache abstraction to see if it would meet our needs.

If you have time could you have a look at: #5145 (comment) . I'm now even more convinced that a size based solution could be a better match; we see a few OAuth2AuthorizationRequest that exists for over 10 minutes on our production system.

Off the top of my head, I think the cache abstraction would impose a quite large runtime requirement to the user of spring security. The user than has to deploy a distributed cache for be able to prevent what is effectively a way to cause an OOM in an application.

I definitely see the need for reducing the duplication and fully understand that OAuth2AuthorizationRequest should not contain a createdAt or a expiresAt, I'll look into other options to store the data and generalize the clean up.

[jgrandja]

I think the cache abstraction would impose a quite large runtime requirement to the user of spring security

I'm not convinced it's the right solution to use either but we should at least rule it out and give reasons for it. I'd prefer a very light solution.

I'm now even more convinced that a size based solution could be a better match

I still think we should account for time-based as well. For low-load client applications, an OAuth2AuthorizationRequest could hang out for a long time. Size-based takes care of high-load client applications. I feel we should account for both.