Fix account console for usage with secure-session client-policy (#37447) #39539
Conversation
8620bd0 to
78545a0
Compare
...quillian/tests/base/src/test/java/org/keycloak/testsuite/actions/TermsAndConditionsTest.java
Outdated
Show resolved
Hide resolved
...quillian/tests/base/src/test/java/org/keycloak/testsuite/actions/TermsAndConditionsTest.java
Outdated
Show resolved
Hide resolved
|
@pedroigor @thomasdarimont It is the behaviour of keycloak.js integration of account console that when it sees the login URL with the unknown BTW. This happens during every successful login to the account console as well and it was the case before my changes as well. So during every login to account console (with or without In other words, with this PR, when user press "decline", Keycloak will redirect to the URL like this which contains When you omit the To me, the options are:
TBH my preference is (1) as regarding usability, it is OK to redirect user directly to the login screen. When user rejected callback, he is aware that he is not able to login. So looks like the error page would be just another "splash screen" displayed to the user? But if you prefer to do (2), please let me know and I can update the PR to not need to do any changes in Not sure if we can do something else? To me, it seems that this whole combination of redirecting to the login screen from Java ( |
|
@thomasdarimont When you want to test with latest chromedriver, you can download it manually https://googlechromelabs.github.io/chrome-for-testing/ and unpack somewhere in your laptop. Then you can run the test with something like: At least, that is what I am usually doing. |
From a UX perspective, the user should be informed about any error returned by Keycloak to the application. In this case, inform that the term was declined. I vote to keep the previous behavior.
Yer ... And we discussed a lot this approach when we introduced it. The decision was basically a tradeoff between making deep changes to the Account App - as discussed with @jonkoops at that time - and a simpler/hacky change to handle things from the backend. I'm also not happy with the current behavior but the old behavior where you have the Account App being loaded when the user is not even authenticated is worse, I think .... |
…loak#37447) Fixes keycloak#37447 Signed-off-by: Thomas Darimont <thomas.darimont@googlemail.com> (cherry picked from commit a465416) Co-authored-by: mposolda <mposolda@gmail.com>
430670d to
0c7a84c
Compare
…msAndConditionsTest Signed-off-by: mposolda <mposolda@gmail.com>
0c7a84c to
c49b13f
Compare
@pedroigor Yeah, I see the point. For the UX, still not 100% sure as when user declined terms-and-conditions, it should be clear to him that login not possible and hence redirect to login screen might be OK. On the other hand, there could be some other errors (not caused by user himself), which may be clearer to display to the user... So I've updated the PR, so no change needed in |
Fixes #37447
(cherry picked from commit a465416)
This is an extension of the PR #38476 , which fixes
TermsAndConditionsTest(as after the error after user rejects terms-and-conditions, there is another request sent automatically and hence login page is displayed) .Also added the automated test in
ClientPoliciesExecutorTestfor the scenario from the report.