[keycloak-dev] SerializedBrokeredIdentityContext ProviderID is filled with IdP alias

[Chris Brandhorst]

Hi,

I agree that the alias should be used, just not that the method used to get the alias is put in a field called identityProviderId, which is an existing but different property.

Regards,
Chris

> On 13 Nov 2018, at 21:45, Marek Posolda <mposolda at redhat.com> wrote:
> 
> Hi,
> 
> the alias is here on purpose. Alias of identityProvider is guaranteed to be unique across the realm. This is not the case for providerId. For example you can have 3 SAML identity providers configured in your realm. Then all those 3 providers will have same providerId, so you won't know which one you want to work with.
> 
> On the other hand, when you have alias, you can always lookup the providerId from it.
> 
> Marek
> 
> On 09/11/18 19:11, Chris Brandhorst wrote:
>> Hi all,
>> 
>> Redirect by Bruno from https://issues.jboss.org/browse/KEYCLOAK-8773:
>> 
>> We came across the following. In SerializedBrokeredIdentityContext#serialize, the identityProviderId property is filled with the alias of the IdentityProviderModel, instead of (what we would expect) its providerId.
>> 
>> Relevant line:
>> https://github.com/keycloak/keycloak/blob/b478472b3578b8980d7b5f1642e91e75d1e78d16/services/src/main/java/org/keycloak/authentication/authenticators/broker/util/SerializedBrokeredIdentityContext.java#L300
>> 
>> We feel this behaviour is semantically incorrect: we were checking against this property in one of our authenticators, but our code did not work for another identity provider of the same type. After some digging we thus found that we were expecting the providerId (coded value) but were actually reading the alias (configured value).
>> 
>> Simply throwing this in as a possible improvement. What do you think?
>> 
>> Regards,
>> Chris Brandhorst
>> _______________________________________________
>> keycloak-dev mailing list
>> keycloak-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
> 
>