-
Notifications
You must be signed in to change notification settings - Fork 95
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
[FIXES #2971] MapStore - SSO keycloak kerberos #356
Conversation
… a successfull login
From my testing I have this error. Here is what I did.
mvn version:
When I try to login I have this issue. screencast-localhost_8080-2024.06.19-10_44_22.webmwith
I don't know why you don't have this error in this case. Anyway, because the issue involves only the Here the doLogin in Here the doLogin in keycloak: Line 61 in 16bbce6
As you can see the keycloak version has a flag, it is used only in that place: https://github.com/search?q=repo%3Ageosolutions-it%2Fgeostore%20KEYCLOAK_REDIRECT&type=code and set only here: Line 148 in 16bbce6
if This looks to me the key point of the problem. I think the problem behind is that kerberos uses the same cookie of the internal library causing the problem. In the filter the user results authenticated, but in fact it is not, because the client is not aware of the cookie (that is done by callback + token endpoints calls, see this chart) So we have anyway to complete the login workflow.
|
@offtherailz so far I have been trying your suggestion but what happens is that whenever the I still remain convinced that the cleanset and correct solution is to correctly set the I have done few improvements to the Please give it a try. If you have a |
@offtherailz waiting for your review here |
as achieved with @tdipisa if possible let's split the part for the scopes from the issue with keycloak SSO + kerberos. |
… set by configuration" This reverts commit 59b7b63.
@offtherailz done, oidc scopes PR here; commit reverted on this one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The solution looks to be working in the conditions you provided.
I don't know if there is other way to avoid to set the cookie and risk the header size issue.
Anyway the solution is working, in our test environment. We have to advice the client, in case of issues, about this https://github.com/geosolutions-it/docker-mapstore-integrations/commit/0f187d6899cd5bad320d8ccffb70aed722b19b8f
I think we should try this solution on the client server first, to avoid to reach the end and see that it doesn't apply. In fact we had to do an hard workaround to replicate the issue, so to be 100% that this fixes the clients problem a test have to be done @tdipisa
@afabiani thank you for this improvement. We need to also backport this to 2.0.x for testing on the involved project. Let me know if you see any kind of issues for this, it doesn't seems to me. |
@tdipisa nope, no issues, it's safe. |
ref.: https://github.com/geosolutions-it/support/issues/2971