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

[Stian Thorgersen]

----- Original Message -----
> From: "Vlastimil Elias" <velias at redhat.com>
> To: "Stian Thorgersen" <stian at redhat.com>
> Cc: keycloak-dev at 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.

We love tests, just slightly less keen on writing them ;)

> 
> 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 at redhat.com>
> >> To: keycloak-dev at 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 at lists.jboss.org
> >> https://lists.jboss.org/mailman/listinfo/keycloak-dev
> >>
> 
> --
> Vlastimil Elias
> Principal Software Engineer
> jboss.org Development Team
> 
>