Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OIDC okta testing #386

Merged
merged 2 commits into from
Nov 12, 2024
Merged

OIDC okta testing #386

merged 2 commits into from
Nov 12, 2024

Conversation

afabiani
Copy link
Member

@afabiani afabiani commented Nov 11, 2024

In this updated version of the SessionDelegate upon a Refresh request the backend also checks:

  • The remote refresh-token request is returning a valid code
  • The current accessToken is still valid in the case the previous one fails for some reason

Other than that, the SessionToken bean has been updated in order to return back also a Warning or Error message with the explanation of the failuers.

As an instance, in the case the remote refresh-token request throws a UserRedirectException because it need manual validation but the current AccessToken is still valid (not expired), the SessionToken will be still valid, by using the current AccessToken but it will provide a Warning message like the one below

{
    "sessionToken": {
        "access_token": "eyJhbGciOiJSUzI1NiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICJwMlZ4YnZGUm1mWlJkWXRtdTUtWGx3eDRvQjg4cVNYQ0ItOUdIWVBuMzdRIn0.eyJleHAiOjE3MzEzMzU2NzAsImlhdCI6MTczMTMzNTM3MCwiYXV0aF90aW1lIjoxNzMxMzM1MzcwLCJqdGkiOiJiZDFjMzMxZS05ODJkLTQxZmUtOWJhZi0wNWY3ZDIyNjliOGUiLCJpc3MiOiJodHRwOi8vbG9jYWxob3N0OjgxODAvcmVhbG1zL21hcHN0b3JlIiwiYXVkIjpbImdzLXRlc3QiLCJhY2NvdW50Il0sInN1YiI6ImU2ZmZmYmI3LTQ4ZTEtNGM0My04MjkyLTY5YzNmY2Y2ZjM5NyIsInR5cCI6IkJlYXJlciIsImF6cCI6ImdzLXRlc3QiLCJzZXNzaW9uX3N0YXRlIjoiMDkxMmI5OGYtOTZmMi00ZDkyLThkYTQtY2ZjMzZiOTYxNTBlIiwiYWNyIjoiMSIsImFsbG93ZWQtb3JpZ2lucyI6WyIvKiJdLCJyZWFsbV9hY2Nlc3MiOnsicm9sZXMiOlsiVEVTVCIsIm9mZmxpbmVfYWNjZXNzIiwiZGVmYXVsdC1yb2xlcy1tYXBzdG9yZSIsInVtYV9hdXRob3JpemF0aW9uIl19LCJyZXNvdXJjZV9hY2Nlc3MiOnsiYWNjb3VudCI6eyJyb2xlcyI6WyJtYW5hZ2UtYWNjb3VudCIsIm1hbmFnZS1hY2NvdW50LWxpbmtzIiwidmlldy1wcm9maWxlIl19fSwic2NvcGUiOiJvcGVuaWQgbWljcm9wcm9maWxlLWp3dCBwcm9maWxlIGdyb3VwcyBlbWFpbCByb2xlcyBvZmZsaW5lX2FjY2VzcyBwaG9uZSBzYXItZHJpZnQgYWRkcmVzcyIsInNpZCI6IjA5MTJiOThmLTk2ZjItNGQ5Mi04ZGE0LWNmYzM2Yjk2MTUwZSIsInVwbiI6InRlc3QiLCJhZGRyZXNzIjp7fSwiZW1haWxfdmVyaWZpZWQiOnRydWUsIm5hbWUiOiJBbGVzc2lvIEZhYmlhbmkiLCJncm91cHMiOlsiVEVTVCIsIm9mZmxpbmVfYWNjZXNzIiwiZGVmYXVsdC1yb2xlcy1tYXBzdG9yZSIsInVtYV9hdXRob3JpemF0aW9uIiwiVEVTVCIsIm9mZmxpbmVfYWNjZXNzIiwiZGVmYXVsdC1yb2xlcy1tYXBzdG9yZSIsInVtYV9hdXRob3JpemF0aW9uIl0sInByZWZlcnJlZF91c2VybmFtZSI6InRlc3QiLCJnaXZlbl9uYW1lIjoiQWxlc3NpbyIsImZhbWlseV9uYW1lIjoiRmFiaWFuaSIsImVtYWlsIjoiYWxlc3Npby5mYWJpYW5pQGdlb3NvbHV0aW9uc2dyb3VwLmNvbSJ9.DvhqSf0kK8v5UUtqTjlOpd52z6IuWg0wihBHbvkvY3GAaiuwxWnla7zQzr45V0D_LE65gcvQYCb4_YYWraBQFOuYzoNbFwE1Jz8MDsvEBqXM5BdNxmP9NTnB64w-wxCVCS4nMYZNMBjVYxXwTa4c3I6ReoACYFHndOBUs44mpjeqmO7v8n0b1P60QiPv2KyE_Kmuu97sICyQIdwWkXCQHWq3ixC2bUNihs5wvwWJQHZxv_OlNCgz7UTB7UpNtjpAff0lV0RUF_OpVDTUEmMLSyhiF6BF2pbNG_FUngL4Uk9JXxrMrUG7LqPdigBkdlIurfGus0Xmo3td5xNikUmhuQ",
        "refresh_token": "eyJhbGciOiJIUzUxMiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICI1NWFkNWJlNS1jOTA3LTRjOWYtYWU2Yi1iYjUwZGU0MmFkMTAifQ.eyJpYXQiOjE3MzEzMzUzNzAsImp0aSI6IjUxMWIxZWNhLWFjZjAtNDhiOC1iNTg5LTdlMDkzNGQ4MDdjYSIsImlzcyI6Imh0dHA6Ly9sb2NhbGhvc3Q6ODE4MC9yZWFsbXMvbWFwc3RvcmUiLCJhdWQiOiJodHRwOi8vbG9jYWxob3N0OjgxODAvcmVhbG1zL21hcHN0b3JlIiwic3ViIjoiZTZmZmZiYjctNDhlMS00YzQzLTgyOTItNjljM2ZjZjZmMzk3IiwidHlwIjoiT2ZmbGluZSIsImF6cCI6ImdzLXRlc3QiLCJzZXNzaW9uX3N0YXRlIjoiMDkxMmI5OGYtOTZmMi00ZDkyLThkYTQtY2ZjMzZiOTYxNTBlIiwic2NvcGUiOiJvcGVuaWQgbWljcm9wcm9maWxlLWp3dCBwcm9maWxlIGdyb3VwcyBlbWFpbCByb2xlcyBvZmZsaW5lX2FjY2VzcyBwaG9uZSBzYXItZHJpZnQgYWRkcmVzcyIsInNpZCI6IjA5MTJiOThmLTk2ZjItNGQ5Mi04ZGE0LWNmYzM2Yjk2MTUwZSJ9.M12uG9hmb9gkkE9ioVX2weXXHR3u5yMbU_BpARCL016lR2DAL9QyEe-eps4RomlLBpTKhkmWUTC4yI6pM6xr4g",
        "token_type": "bearer",
        "warning": "A redirect is required to get the user's approval."
    }
}

In case of Errors, the returned AccessToken and RefreshToken will be EMPTY with an ErrorMessage explaining the cause.

More Unit Tests have been added and the old ones updated accordingly to the new behavior.

Fix https://github.com/geosolutions-it/webmapper-halliburton/issues/3595

Copy link
Member

@offtherailz offtherailz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR is quite clear to me. I've only 2 points, regarding MapStore behavior and doc:

  • I missed the logic behind UserRedirectRequiredException .
    I'm trying to guess: If the session token is still valid, but refresh is not allowed anymore because of this exception), the session token will expire soon. This depends on the server policy but it can be the case when, for instance, max refresh extent is finished or it passed too much time or something. isn't it?
    If so, MapStore can ignore this message until the logout kicks out the user... that is not so kind in any case. So maybe it is the case to show a warning saying that the token is near to expire. In this case, knowing when the token is going to expire could be useful. What do you thing @afabiani ?
  • I see there are new configurable new params like maxRetry initialBackoffDelay amd backoffMultiplier . We need to remember to document these params too.

@afabiani
Copy link
Member Author

@offtherailz MapStore can ignore the message or show a warning to the user if needed, it's up to you. This depends mainly on the provider configuration. It is a common use case when using Keycloak, as an instance.

@offtherailz offtherailz self-requested a review November 12, 2024 13:59
@offtherailz offtherailz merged commit 94b0eab into master Nov 12, 2024
2 checks passed
@afabiani afabiani deleted the oidc_okta_testing branch November 12, 2024 14:09
@offtherailz offtherailz restored the oidc_okta_testing branch November 13, 2024 13:41
@afabiani afabiani deleted the oidc_okta_testing branch November 14, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants