Hi Mike, I just tested the recent change you proposed with guacOpenIDTransformToken(), and can confirm that it works well in my case, with “Exclude Session State From Authentication Response” in Keycloak setting ON or OFF.
Do you think it is a good idea to report this issue to Keycloak team? I am not sure whether it is really the case. And, if the Keycloak team add the “?” as I thought, do we still need guacOpenIDTransformToken()? The logout issue is still there. Thanks, Yang > On May 31, 2019, at 09:53, Yang Yang <[email protected]> wrote: > > Hi Mike and Nick, > > Both of you are right. I just got a workaround for my initial case, not > really a fix to the issue. And, I met the looping issue again when turning > off “Exclude Session State From Authentication Response” in Keycloak setting, > which results with “../#/&session_state=….&id_token=…” instead of > “../#/&id_token=…” > > From my understanding, this is actually a bug on Keycloak side that it misses > the “?” before parameters such as “id_token=…” or others as > “session_state=…”. The solution currently available (Transform > "/#/id_token=..." to "/#/?id_token=…”) or the one I made is to help, in some > very specific case, parameter “id_token=...” be properly handled. > > By the way, I found another issue with Keycloak, or any other OpenID > integration I believe, is that users cannot logout from Guacamole. When a > user logout, they will be directed to the login page, and then will be > redirected to Keycloak; however, as the login session on Keycloak side is > still there, the user will be login again. > > > Sorry that I got very limited knowledge in programming, never used Java or > JavaScript before, and could not help more… > > Thanks, > Yang > >> On May 31, 2019, at 07:58, Mike Jumper <[email protected] >> <mailto:[email protected]>> wrote: >> >> On Thu, May 30, 2019 at 4:34 PM Nick Couchman <[email protected] >> <mailto:[email protected]>> wrote: >> On Thu, May 30, 2019 at 02:09 Yang Yang <[email protected] >> <mailto:[email protected]>> wrote: >> Hello, >> >> I solved this issue by making a small change to two files for the openid >> extension and rebuild it: >> ~/guacamole-client-1.0.0/extensions/guacamole-auth-openid/src/main/resources/config/openidConfig.js >> ~/guacamole-client-1.0.0/extensions/guacamole-auth-openid/target/classes/config/openidConfig.js >> >> /** >> * Config block which augments the existing routing, providing special >> handling >> * for the "id_token=" fragments provided by OpenID Connect. >> */ >> angular.module('index').config(['$routeProvider', >> function indexRouteConfig($routeProvider) { >> >> // Transform "/#/id_token=..." to "/#/?id_token=…" >> -- $routeProvider.when('/id_token=:response', { >> ++ $routeProvider.when(‘/&id_token=:response', { >> >> >> While this change fixes things for your use case, I suspect that it may >> break things for other instances of OpenID providers. At the very least >> such a change would need to be thoroughly tested with multiple OpenID >> providers. >> >> The above change to the regex would indeed break things for any IDP that >> places the id_token at the beginning of the string. >> >> The fact that the id_token parameter will not always occur at the beginning >> of the parameter list was identified in an older thread: >> >> https://lists.apache.org/thread.html/cc0a9300086c55e25d59d73d025d6e0be07b42cc8903f4de1c1b48a5@%3Cuser.guacamole.apache.org%3E >> >> <https://lists.apache.org/thread.html/cc0a9300086c55e25d59d73d025d6e0be07b42cc8903f4de1c1b48a5@%3Cuser.guacamole.apache.org%3E> >> >> That thread resulted in the following proposed change, with the intent to >> open a JIRA issue and PR if it could be confirmed to solve the issue: >> >> https://github.com/apache/guacamole-client/compare/master...mike-jumper:openid-token >> >> <https://github.com/apache/guacamole-client/compare/master...mike-jumper:openid-token> >> >> That confirmation never came. If the above could be confirmed, I would >> suggest moving forward with the changes I proposed in that older thread. >> They are intended to handle the id_token parameter regardless of where it's >> located, not just at the beginning (current implementation) or anywhere BUT >> the beginning (the patch in this latest thread), however I'm not 100% sure >> the regex in my changes is correct, either. >> >> - Mike >
