Improvements with OKTA OIDC provider integration

[afabiani]

This PR improves the way how GeoStore handles the refresh-token request by:

• Checking the refresh token despite the access token expiration; a refresh token expiration might be set too
• Making sure the new access token and refresh token will be sent bach to the sender
• Attempting the refresh token request in case of temporary network/communication issues

[offtherailz]

It is not easy to test so I only inspected the code right now.
On a first inspection your changes look understandable.
I have a few of points that I think we should consider:

1. Testing: How to test this: We have several use cases now that use openID. I'd like to be sure that this doesn't break existing implementations. I think we should start to configure this auth system on our dev server (with azure or github at least) o to configure a battery of tests. I think there should be some also some unit tests to make this functionality less prone to future regressions due to code changes.
2. There was a fiveMinutesFromNow beharvior that has been removed. As far as I remember, it was there to prevent to ask for a new token if the current is not even near to expire yet. With this logic change, it will create ask a new token on every refresh session call (that mapstore sends every 30 seconds ?
3. I noticed you replicated the same logic in case of token expiration (redirecting to login). I think also this workflow should be tested with MapStore too, because as far as I remember the refresh call is an ajax call, so I have the suspect that in that case it will not work properly. Lookign at the documentation a refresh token expiration is not taken into account or not documented yet.

So I'd suggest:

• To list the test you did to verify this functionality, in order to make us able to check the behavior.
• Add some unit tests if possible
• We also need to improve the documentation with the refresh token logic, maybe

[afabiani]

Replying to the points above:

1. I currently tested with azure and keycloak, so far it works well
2. That logic is dangerous and doesn't make sense at all. The client eventually should set the refresh time accoring to the access_token expiration or wait for the refresh token to expire. However there's no problem issuing the request to the OIDC provider (except for a bit of overhead). The latter will eventually decide whether assign a new access_token or extend the expiration of the current one. Notice that this is the correct behavior. We need to allow the provider decide which policy to apply. Moreover, the provider, like in the OKTA use case, might set an expiration time for the refresh token too that geostore cannot set at all.
3. The workflow is the same as before, I just did a bit of refactoring by adding a "retry" logic when sending a request to the refresh token endpoint

[offtherailz]

1. Ok. @tdipisa I think we may configure try to configure azure as provider on our dev server, to allow continuous testing.
2. Ok, makes sense.
3. Yes, I've seen it. Did you tested with MapStore the case with an expired refresh token? Because maybe with this refresh token logic fixed, a wrong behiviour, that wasn't happening before, comes out. It should be tested. Maybe it works anyway, but I'd like to see it 😄

[tdipisa]

@afabiani

I totally agree with the following:

Testing: How to test this: We have several use cases now that use openID. I'd like to be sure that this doesn't break existing implementations. I think we should start to configure this auth system on our dev server (with azure or github at least) o to configure a battery of tests. I think there should be some also some unit tests to make this functionality less prone to future regressions due to code changes.

To list the test you did to verify this functionality, in order to make us able to check the behavior.
Add some unit tests if possible

We need to coordinate for this first

We also need to improve the documentation with the refresh token logic, maybe

@offtherailz do you want to provide a MS doc update for this with a PR?

waiting for unit tests

[offtherailz]

LGTM

I removed target and source compiler version as asked by @afabiani .
Let's wait for the build to see if tests still passing