It looks that we should support latest Keycloak server with older versions of Keycloak adapters. So for some corner scenarios, I wonder if we should add the switch to the ClientModel and admin console like "Adapter version" . This switch will be available for both OIDC and SAML clients, but will be useful just for the clients, which uses Keycloak adapter. It will be useful to specify the version of Keycloak client adapter, which particular client application is using. WDYT? The reason why I felt into this is a reported RHSSO bug. Long-story short: When Keycloak SAML 1.9.8 adapter is used with "isPassive=true", then Keycloak 2.5.4 server returns him the valid error response. However 1.9.8 adapter has a bug https://issues.jboss.org/browse/KEYCLOAK-4264 and it throws NPE when it receives such response. With SAML 1.9.8 adapter + 1.9.8 server, the Keycloak server returned invalid error response, however 1.9.8 adapter was able to handle this invalid response without throwing any exception. By adding the switch to the ClientModel, we defacto allow adapter to say: "Please return me broken response, because I am not able to handle valid response." Note that this is bug in adapter, so it will be better to ask customers to rather upgrade their SAML adapters to newest version. On the other hand, we claim to support backwards compatibility. So should we add the switch or not? WDYT? Marek _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev

Add switch IMO. It should have a select box that defaults to "latest". On 3/2/17 9:44 AM, Marek Posolda wrote: > It looks that we should support latest Keycloak server with older > versions of Keycloak adapters. > > So for some corner scenarios, I wonder if we should add the switch to > the ClientModel and admin console like "Adapter version" . This switch > will be available for both OIDC and SAML clients, but will be useful > just for the clients, which uses Keycloak adapter. It will be useful to > specify the version of Keycloak client adapter, which particular client > application is using. WDYT? > > The reason why I felt into this is a reported RHSSO bug. > > Long-story short: When Keycloak SAML 1.9.8 adapter is used with > "isPassive=true", then Keycloak 2.5.4 server returns him the valid error > response. However 1.9.8 adapter has a bug > https://issues.jboss.org/browse/KEYCLOAK-4264 and it throws NPE when it > receives such response. > > With SAML 1.9.8 adapter + 1.9.8 server, the Keycloak server returned > invalid error response, however 1.9.8 adapter was able to handle this > invalid response without throwing any exception. > > > By adding the switch to the ClientModel, we defacto allow adapter to > say: "Please return me broken response, because I am not able to handle > valid response." > > Note that this is bug in adapter, so it will be better to ask customers > to rather upgrade their SAML adapters to newest version. On the other > hand, we claim to support backwards compatibility. > > So should we add the switch or not? WDYT? > > Marek > > _______________________________________________ > keycloak-dev mailing list > keycloak-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/keycloak-dev _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev

Ah yes. I was thinking about the client message vs. switch, but it seems that switch be cleaner then. Thanks all for the feedback! Marek On 03/03/17 09:15, Hynek Mlnarik wrote: > Determination of client version from client message would not work for > IdP-initiated SSO (there is no client message to determine version > from), so +1. > > On Thu, Mar 2, 2017 at 8:28 PM, Bill Burke <bburke(a)redhat.com&gt; wrote: >> Add switch IMO. It should have a select box that defaults to "latest". >> >> >> On 3/2/17 9:44 AM, Marek Posolda wrote: >>> It looks that we should support latest Keycloak server with older >>> versions of Keycloak adapters. >>> >>> So for some corner scenarios, I wonder if we should add the switch to >>> the ClientModel and admin console like "Adapter version" . This switch >>> will be available for both OIDC and SAML clients, but will be useful >>> just for the clients, which uses Keycloak adapter. It will be useful to >>> specify the version of Keycloak client adapter, which particular client >>> application is using. WDYT? >>> >>> The reason why I felt into this is a reported RHSSO bug. >>> >>> Long-story short: When Keycloak SAML 1.9.8 adapter is used with >>> "isPassive=true", then Keycloak 2.5.4 server returns him the valid error >>> response. However 1.9.8 adapter has a bug >>> https://issues.jboss.org/browse/KEYCLOAK-4264 and it throws NPE when it >>> receives such response. >>> >>> With SAML 1.9.8 adapter + 1.9.8 server, the Keycloak server returned >>> invalid error response, however 1.9.8 adapter was able to handle this >>> invalid response without throwing any exception. >>> >>> >>> By adding the switch to the ClientModel, we defacto allow adapter to >>> say: "Please return me broken response, because I am not able to handle >>> valid response." >>> >>> Note that this is bug in adapter, so it will be better to ask customers >>> to rather upgrade their SAML adapters to newest version. On the other >>> hand, we claim to support backwards compatibility. >>> >>> So should we add the switch or not? WDYT? >>> >>> Marek >>> >>> _______________________________________________ >>> keycloak-dev mailing list >>> keycloak-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/keycloak-dev >> _______________________________________________ >> keycloak-dev mailing list >> keycloak-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/keycloak-dev > > _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev

On 3/3/2017 3:15 AM, Hynek Mlnarik wrote: > Determination of client version from client message would not work for > IdP-initiated SSO (there is no client message to determine version > from), so +1. I don't understand this. I don't know exactly how we implement Idp-initiated SSO, but if you are talking to a client then, by definition, you are exchanging messages. In every protocol I can remember, part of the handshake includes transmitting the protocol version. If you don't do this, it leads to problems. With the switch, somebody has to manually manage the protocol version of potentially thousands of clients. If somebody sets the switch wrong, the client can't communicate. Out of the plethora of possible issues, how long will it take before somebody realizes that the version switch is wrong? Also, you might have the switch set incorrectly, but the client still seems to communicate fine because it doesn't run into unexpected messages. But you never know that the client is really using the wrong version. So what happens when there is a serious security problem in a version and you need to upgrade or disable certain clients? Then you don't know for sure what version each client is running. For the security reason alone, you need to know for sure what software the client is running. If the client always tells you its version, the server ALWAYS knows what to do. Otherwise, it's hit or miss. Granted, I haven't been "into" protocols for many, many years. But this seems like fundamental stuff. Feel free to talk me down. > On Thu, Mar 2, 2017 at 8:28 PM, Bill Burke <bburke(a)redhat.com&gt; wrote: >> Add switch IMO. It should have a select box that defaults to "latest". >> >> >> On 3/2/17 9:44 AM, Marek Posolda wrote: >>> It looks that we should support latest Keycloak server with older >>> versions of Keycloak adapters. >>> >>> So for some corner scenarios, I wonder if we should add the switch to >>> the ClientModel and admin console like "Adapter version" . This switch >>> will be available for both OIDC and SAML clients, but will be useful >>> just for the clients, which uses Keycloak adapter. It will be useful to >>> specify the version of Keycloak client adapter, which particular client >>> application is using. WDYT? >>> >>> The reason why I felt into this is a reported RHSSO bug. >>> >>> Long-story short: When Keycloak SAML 1.9.8 adapter is used with >>> "isPassive=true", then Keycloak 2.5.4 server returns him the valid error >>> response. However 1.9.8 adapter has a bug >>> https://issues.jboss.org/browse/KEYCLOAK-4264 and it throws NPE when it >>> receives such response. >>> >>> With SAML 1.9.8 adapter + 1.9.8 server, the Keycloak server returned >>> invalid error response, however 1.9.8 adapter was able to handle this >>> invalid response without throwing any exception. >>> >>> >>> By adding the switch to the ClientModel, we defacto allow adapter to >>> say: "Please return me broken response, because I am not able to handle >>> valid response." >>> >>> Note that this is bug in adapter, so it will be better to ask customers >>> to rather upgrade their SAML adapters to newest version. On the other >>> hand, we claim to support backwards compatibility. >>> >>> So should we add the switch or not? WDYT? >>> >>> Marek >>> >>> _______________________________________________ >>> keycloak-dev mailing list >>> keycloak-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/keycloak-dev >> _______________________________________________ >> keycloak-dev mailing list >> keycloak-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/keycloak-dev > _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev

I agree with you in principle stan, but there are other issues: * For OIDC it is a get request with simple query parameters. There is no request object you can hide the version information in and you'd have to add another query param.

* As Hynek said, for SAML IDP initiated SSO, there is no client request. User logs in then assertion is sent to client.

* Keycloak 1.x and 2.x are already out the door and don't submit a version. 2.5.x fixes the problem of the OP, 1.x doesn't so there is no way to tell the difference.

* Client templates can be used to manage large sets of clients.

On 3/3/17 9:11 AM, Stan Silvert wrote: > On 3/3/2017 3:15 AM, Hynek Mlnarik wrote: >> Determination of client version from client message would not work for >> IdP-initiated SSO (there is no client message to determine version >> from), so +1. > I don't understand this. I don't know exactly how we implement > Idp-initiated SSO, but if you are talking to a client then, by > definition, you are exchanging messages. In every protocol I can > remember, part of the handshake includes transmitting the protocol version. > > If you don't do this, it leads to problems. With the switch, somebody > has to manually manage the protocol version of potentially thousands of > clients. If somebody sets the switch wrong, the client can't > communicate. Out of the plethora of possible issues, how long will it > take before somebody realizes that the version switch is wrong? > > Also, you might have the switch set incorrectly, but the client still > seems to communicate fine because it doesn't run into unexpected > messages. But you never know that the client is really using the wrong > version. So what happens when there is a serious security problem in a > version and you need to upgrade or disable certain clients? Then you > don't know for sure what version each client is running. > > For the security reason alone, you need to know for sure what software > the client is running. If the client always tells you its version, the > server ALWAYS knows what to do. Otherwise, it's hit or miss. > > Granted, I haven't been "into" protocols for many, many years. But this > seems like fundamental stuff. Feel free to talk me down. >> On Thu, Mar 2, 2017 at 8:28 PM, Bill Burke <bburke(a)redhat.com&gt; wrote: >>> Add switch IMO. It should have a select box that defaults to "latest". >>> >>> >>> On 3/2/17 9:44 AM, Marek Posolda wrote: >>>> It looks that we should support latest Keycloak server with older >>>> versions of Keycloak adapters. >>>> >>>> So for some corner scenarios, I wonder if we should add the switch to >>>> the ClientModel and admin console like "Adapter version" . This switch >>>> will be available for both OIDC and SAML clients, but will be useful >>>> just for the clients, which uses Keycloak adapter. It will be useful to >>>> specify the version of Keycloak client adapter, which particular client >>>> application is using. WDYT? >>>> >>>> The reason why I felt into this is a reported RHSSO bug. >>>> >>>> Long-story short: When Keycloak SAML 1.9.8 adapter is used with >>>> "isPassive=true", then Keycloak 2.5.4 server returns him the valid error >>>> response. However 1.9.8 adapter has a bug >>>> https://issues.jboss.org/browse/KEYCLOAK-4264 and it throws NPE when it >>>> receives such response. >>>> >>>> With SAML 1.9.8 adapter + 1.9.8 server, the Keycloak server returned >>>> invalid error response, however 1.9.8 adapter was able to handle this >>>> invalid response without throwing any exception. >>>> >>>> >>>> By adding the switch to the ClientModel, we defacto allow adapter to >>>> say: "Please return me broken response, because I am not able to handle >>>> valid response." >>>> >>>> Note that this is bug in adapter, so it will be better to ask customers >>>> to rather upgrade their SAML adapters to newest version. On the other >>>> hand, we claim to support backwards compatibility. >>>> >>>> So should we add the switch or not? WDYT? >>>> >>>> Marek >>>> >>>> _______________________________________________ >>>> keycloak-dev mailing list >>>> keycloak-dev(a)lists.jboss.org >>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev >>> _______________________________________________ >>> keycloak-dev mailing list >>> keycloak-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/keycloak-dev > _______________________________________________ > keycloak-dev mailing list > keycloak-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/keycloak-dev _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev

On 03/03/17 16:29, Stan Silvert wrote: > Versioning is always a pain. But it's much better for us to manage the > pain than leave it up to customers. > > On 3/3/2017 9:35 AM, Bill Burke wrote: >> I agree with you in principle stan, but there are other issues: >> >> * For OIDC it is a get request with simple query parameters. There >> is no >> request object you can hide the version information in and you'd >> have to >> add another query param. > I don't think you need to hide the version. Just add another query > param. Then if version is not present, you at least know it's an old > client. Another query param for OIDC should work and some similar extension for SAML should work too though. The backwards compatibility is supported just for Keycloak adapters, so we can "extend" the protocols here IMO. Maybe it's even not too late to fix this in 2.x :) But still, for 1.x the version won't never be present. So you still won't be able to differentiate between 1.9.8 adapter (where we need to send "broken" response) and between external SAML adapter (where we need to send valid response). As external OIDC and SAML adapters will obviously never send us the version as that would be Keycloak specific extension. >> * As Hynek said, for SAML IDP initiated SSO, there is no client >> request. User logs in then assertion is sent to client. > Would it make sense to send client version as part of the login? > > At some point, the client has to send a message to the server. You can > always piggyback the version onto whatever message is sent. Maybe the possibility here is to send the message at the deployment time (ServletContextListener?). But that has some other potential issues (eg. application deployed at the time when KC server is not yet running, which would mean error etc..). There are request from adapters to server to download keys, but not sure whether to mix with the version (and keys can still be hardcoded in the adapter config, so this one is not reliable too). In summary, I am also more keen for the switch. Note that we're talking about version of Keycloak (RHSSO) but not about version of protocol (OIDC or SAML). So in practice, Keycloak server should always send the correct response according to the protocol. Only exception, I can see, is some bug in the old adapter, which is not able to handle correct responses according to OIDC or SAML protocols. Which is exactly the case of this particular RHSSO bug. People should be told to rather update their adapters and the switch will be a backup just in case that updating adapters is too much pain for them. In case there is security issue in the adapter, customers should be told to update their adapters anyway. Marek >> * Keycloak 1.x and 2.x are already out the door and don't submit a >> version. 2.5.x fixes the problem of the OP, 1.x doesn't so there is no >> way to tell the difference. > Yea, I raised this issue in Brno last year. It's not too late to fix > the problem going forward though. >> * Client templates can be used to manage large sets of clients. > That might help, but it's not really a solution. >> >> >> On 3/3/17 9:11 AM, Stan Silvert wrote: >>> On 3/3/2017 3:15 AM, Hynek Mlnarik wrote: >>>> Determination of client version from client message would not work >>>> for >>>> IdP-initiated SSO (there is no client message to determine version >>>> from), so +1. >>> I don't understand this. I don't know exactly how we implement >>> Idp-initiated SSO, but if you are talking to a client then, by >>> definition, you are exchanging messages. In every protocol I can >>> remember, part of the handshake includes transmitting the protocol >>> version. >>> >>> If you don't do this, it leads to problems. With the switch, somebody >>> has to manually manage the protocol version of potentially >>> thousands of >>> clients. If somebody sets the switch wrong, the client can't >>> communicate. Out of the plethora of possible issues, how long will it >>> take before somebody realizes that the version switch is wrong? >>> >>> Also, you might have the switch set incorrectly, but the client still >>> seems to communicate fine because it doesn't run into unexpected >>> messages. But you never know that the client is really using the >>> wrong >>> version. So what happens when there is a serious security problem >>> in a >>> version and you need to upgrade or disable certain clients? Then you >>> don't know for sure what version each client is running. >>> >>> For the security reason alone, you need to know for sure what software >>> the client is running. If the client always tells you its version, >>> the >>> server ALWAYS knows what to do. Otherwise, it's hit or miss. >>> >>> Granted, I haven't been "into" protocols for many, many years. But >>> this >>> seems like fundamental stuff. Feel free to talk me down. >>>> On Thu, Mar 2, 2017 at 8:28 PM, Bill Burke <bburke(a)redhat.com&gt; wrote: >>>>> Add switch IMO. It should have a select box that defaults to >>>>> "latest". >>>>> >>>>> >>>>> On 3/2/17 9:44 AM, Marek Posolda wrote: >>>>>> It looks that we should support latest Keycloak server with older >>>>>> versions of Keycloak adapters. >>>>>> >>>>>> So for some corner scenarios, I wonder if we should add the >>>>>> switch to >>>>>> the ClientModel and admin console like "Adapter version" . This >>>>>> switch >>>>>> will be available for both OIDC and SAML clients, but will be >>>>>> useful >>>>>> just for the clients, which uses Keycloak adapter. It will be >>>>>> useful to >>>>>> specify the version of Keycloak client adapter, which particular >>>>>> client >>>>>> application is using. WDYT? >>>>>> >>>>>> The reason why I felt into this is a reported RHSSO bug. >>>>>> >>>>>> Long-story short: When Keycloak SAML 1.9.8 adapter is used with >>>>>> "isPassive=true", then Keycloak 2.5.4 server returns him the >>>>>> valid error >>>>>> response. However 1.9.8 adapter has a bug >>>>>> https://issues.jboss.org/browse/KEYCLOAK-4264 and it throws NPE >>>>>> when it >>>>>> receives such response. >>>>>> >>>>>> With SAML 1.9.8 adapter + 1.9.8 server, the Keycloak server >>>>>> returned >>>>>> invalid error response, however 1.9.8 adapter was able to handle >>>>>> this >>>>>> invalid response without throwing any exception. >>>>>> >>>>>> >>>>>> By adding the switch to the ClientModel, we defacto allow >>>>>> adapter to >>>>>> say: "Please return me broken response, because I am not able to >>>>>> handle >>>>>> valid response." >>>>>> >>>>>> Note that this is bug in adapter, so it will be better to ask >>>>>> customers >>>>>> to rather upgrade their SAML adapters to newest version. On the >>>>>> other >>>>>> hand, we claim to support backwards compatibility. >>>>>> >>>>>> So should we add the switch or not? WDYT? >>>>>> >>>>>> Marek >>>>>> >>>>>> _______________________________________________ >>>>>> keycloak-dev mailing list >>>>>> keycloak-dev(a)lists.jboss.org >>>>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev >>>>> _______________________________________________ >>>>> keycloak-dev mailing list >>>>> keycloak-dev(a)lists.jboss.org >>>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev >>> _______________________________________________ >>> keycloak-dev mailing list >>> keycloak-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/keycloak-dev >> _______________________________________________ >> keycloak-dev mailing list >> keycloak-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/keycloak-dev > _______________________________________________ > keycloak-dev mailing list > keycloak-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/keycloak-dev

On 3/3/17 10:29 AM, Stan Silvert wrote: > Versioning is always a pain. But it's much better for us to manage the > pain than leave it up to customers. > > On 3/3/2017 9:35 AM, Bill Burke wrote: >> I agree with you in principle stan, but there are other issues: >> >> * For OIDC it is a get request with simple query parameters. There is no >> request object you can hide the version information in and you'd have to >> add another query param. > I don't think you need to hide the version. Just add another query > param. Then if version is not present, you at least know it's an old > client. >> * As Hynek said, for SAML IDP initiated SSO, there is no client >> request. User logs in then assertion is sent to client. > Would it make sense to send client version as part of the login? > > At some point, the client has to send a message to the server. You can > always piggyback the version onto whatever message is sent. Untrue. With SAML IDP initiated SSO, the client does not have to send a message to the server, ever. Also, this isn't a communication protocol, its an authentication protocol, so once the protocol is finished, there's no more piggybacking that can be done. >> * Keycloak 1.x and 2.x are already out the door and don't submit a >> version. 2.5.x fixes the problem of the OP, 1.x doesn't so there is no >> way to tell the difference. > Yea, I raised this issue in Brno last year. It's not too late to fix > the problem going forward though. We can and should propagate version, but it still doesn't solve the problem of a 1.9.x adapter talking to Keycoak 2.5.x, nor does it solve idp initiated SSO issues.

Bill _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev

I'm not on the Keycloak dev team (obviously) but can't you backport a patch to RH-SSO 7.0 to fix the broken adapter behavior rather than building in a very specialized flag that will be present in every single SAML client configuration going forward where it's completely unnecessary a large majority of the time? It doesn't make sense to me to build a workaround into the IDP code just to accommodate one specific version of one specific client library. If this was any other client library with this problem I feel like the response would be that the client library needs to be updated rather than maintaining a workaround in the IDP. If RH-SSO 7.0 is still a supported product then it should still be eligible for receiving updates. Jared On Fri, Mar 3, 2017 at 2:37 PM, Stan Silvert <ssilvert(a)redhat.com&gt; wrote: > On 3/3/2017 2:11 PM, Bill Burke wrote: > > > > On 3/3/17 10:29 AM, Stan Silvert wrote: > >> Versioning is always a pain. But it's much better for us to manage the > >> pain than leave it up to customers. > >> > >> On 3/3/2017 9:35 AM, Bill Burke wrote: > >>> I agree with you in principle stan, but there are other issues: > >>> > >>> * For OIDC it is a get request with simple query parameters. There is > no > >>> request object you can hide the version information in and you'd have > to > >>> add another query param. > >> I don't think you need to hide the version. Just add another query > >> param. Then if version is not present, you at least know it's an old > >> client. > >>> * As Hynek said, for SAML IDP initiated SSO, there is no client > >>> request. User logs in then assertion is sent to client. > >> Would it make sense to send client version as part of the login? > >> > >> At some point, the client has to send a message to the server. You can > >> always piggyback the version onto whatever message is sent. > > Untrue. With SAML IDP initiated SSO, the client does not have to send a > > message to the server, ever. Also, this isn't a communication protocol, > > its an authentication protocol, so once the protocol is finished, > > there's no more piggybacking that can be done. > > > >>> * Keycloak 1.x and 2.x are already out the door and don't submit a > >>> version. 2.5.x fixes the problem of the OP, 1.x doesn't so there is no > >>> way to tell the difference. > >> Yea, I raised this issue in Brno last year. It's not too late to fix > >> the problem going forward though. > > We can and should propagate version, but it still doesn't solve the > > problem of a 1.9.x adapter talking to Keycoak 2.5.x, nor does it solve > > idp initiated SSO issues. > I'm on board with that. Let the client send a version whenever possible. > > > > Bill > > _______________________________________________ > > keycloak-dev mailing list > > keycloak-dev(a)lists.jboss.org > > https://lists.jboss.org/mailman/listinfo/keycloak-dev > > _______________________________________________ > keycloak-dev mailing list > keycloak-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/keycloak-dev > _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev