Re: [keycloak-dev] How to handle empty strings returned by Social login providers in user info - KEYCLOAK-1182

From: "Vlastimil Elias" <velias(a)redhat.com&gt; To: "Stian Thorgersen" <stian(a)redhat.com&gt; Cc: keycloak-dev(a)lists.jboss.org Sent: Tuesday, 7 April, 2015 12:49:08 PM Subject: Re: [keycloak-dev] How to handle empty strings returned by Social login providers in user info - KEYCLOAK-1182 Hi, sure, I'll patch the second problem also. I'd also like to add unit test (based on junit) directly into keycloak-broker-oidc project to cover behaviour of getJsonProperty() method, as I think that is is better place than keycloak-testsuite-integration project used for integration tests.

Vl. On 7.4.2015 12:22, Stian Thorgersen wrote: > Sounds good. I guess you'll fix the null -> "null" issue at the same time? > > ----- Original Message ----- >> From: "Vlastimil Elias" <velias(a)redhat.com&gt; >> To: keycloak-dev(a)lists.jboss.org >> Sent: Tuesday, 7 April, 2015 10:57:16 AM >> Subject: [keycloak-dev] How to handle empty strings returned by Social >> login providers in user info - KEYCLOAK-1182 >> >> Hi, >> >> during latest testing I find problem with empty string returned in email >> field from GitHub social provider, which causes http 500 error in later >> processing (but seems under some other circumstances only, not for all >> cases), see https://issues.jboss.org/browse/KEYCLOAK-1182 >> >> When I look into the code used to take used profile informations (email, >> name, id) from Social provider REST responses, it simply takes what is >> returned and do not care too much what is here. >> >> But other Keycloak code (eg search user by email etc) typically only >> check for null values when testing "existence" of information. If value >> is not null then it takes it as existing one, so empty strings may bring >> problems here as it is used as valid email later. >> >> I believe KC should look at what is returned from Social providers and >> convert empty strings to null values. >> It is only small change at one place - >> AbstractOAuth2IdentityProvider.getJsonProperty() which resolves this >> problem. >> >> What do you think about this solution? >> >> I have patch prepared and it works, I can post it as pull request after >> some additional testing. >> >> Vl. >> >> -- >> Vlastimil Elias >> Principal Software Engineer >> jboss.org Development Team >> >> _______________________________________________ >> keycloak-dev mailing list >> keycloak-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/keycloak-dev >> -- Vlastimil Elias Principal Software Engineer jboss.org Development Team