klausbetz / apple-identity-provider-keycloak

An extension for Keycloak, that enables web-based sign in with Apple and token exchange
Apache License 2.0
193 stars 27 forks source link

Apple icon based on font-awesome #10

Closed EvgeniGordeev closed 1 year ago

EvgeniGordeev commented 2 years ago

Font awesome bundled with keycloak provides an icon for Apple products https://fontawesome.com/icons/apple?s=solid&f=brands. Can it be incorporated into the plugin?

image

Workaround:

klausbetz commented 2 years ago

Thank you for this great improvement, @EvgeniGordeev 😀 I'll see what I can do here. kcLogoIdP-apple=fa fa-apple is already a good hint 👍

klausbetz commented 2 years ago

@stianst: Do you have an idea how we can achieve this directly in this extension?

stianst commented 2 years ago

@klausbetz Not right now, but is something we should support.

Thinking perhaps to extend https://github.com/keycloak/keycloak/blob/main/server-spi/src/main/java/org/keycloak/models/IdentityProviderModel.java to have a "displayIconClasses", with a default implementation (so it doesn't break backwards compatiblity) that just returns null. Then a custom provider can implement this method to set the default classes.

Next step would be to update https://github.com/keycloak/keycloak/blob/main/services/src/main/java/org/keycloak/forms/login/freemarker/model/IdentityProviderBean.java#L76 to use IdentityProviderModel#getDisplayIconClasses() if one isn't set by a theme.

Would be more than happy to review/merge a PR if you'd be interested in contributing that?

klausbetz commented 2 years ago

Thank you, @stianst! I'll check it out 👍

klausbetz commented 2 years ago

@stianst I created a PR for the changes you proposed. It was way easier to implement than I thought it would be. 😁
Feel free to add your 2 cents to the PR.

https://github.com/keycloak/keycloak/pull/14826

klausbetz commented 2 years ago

As soon as there's a new version of the keycloak dependencies, I'll release a new version of this package including the icon.

klausbetz commented 2 years ago

Keycloak 20.0.0 is out! 😄

However, it seems like this feature is not as easy to implement as we thought it is. 🤔
I implemented a work-around (some would call this a fckn dirty hack and I'm not proud of it 😁) in this PR https://github.com/klausbetz/apple-identity-provider-keycloak/pull/14, which messes around with the "social"-data of the login form.
⚠️ This work-around with the custom LoginFormsProvider only works on v19.0.2 and above (cause the constructor of FreeMarkerLoginFormsProvider was changed in v19.0.2). Compatibility-wise not ideal.

The main issue is that the new method IdentityProviderModel.getDisplayIconClasses() is ignored, cause AppleIdentityProviderConfig.java (which contains the override for getDisplayIconClasses()) is not instantiated for the login form. https://github.com/klausbetz/apple-identity-provider-keycloak/blob/72af348ee433dee16e3136f157507f0bbcea1fd4/src/main/java/at/klausbetz/provider/AppleIdentityProviderConfig.java#L50-L53 See RealmAdapter.java which instantiates IdentityProviderModel.

@stianst I'm afraid this PR https://github.com/keycloak/keycloak/pull/14826 for keycloak did not improve the situation. Do you have any idea how the icon can be displayed on the standard login page? If there's no easy way, I'd rather skip this issue and revert the changes from https://github.com/keycloak/keycloak/pull/14826.
To be honest, from a functional point of view this feature has no priority. It's way easier to address this UI feature in a custom theme which the respective keycloak administrator implements.
Moreover, there are visual guidelines from apple which are not covered in this extension as well (which may lead to rejected App reviews from Apple in the worst case).

klausbetz commented 1 year ago

Yeah!
With the next release of Keycloak the icon should be visible in the standard theme 😀
Just make sure to use 1.3.0 or any later version.

klausbetz commented 1 year ago

Works, using Keycloak 21. Tested using v1.4.1.

klausbetz commented 1 year ago

Closing this ticket is cheap since no one can use this extension with Keycloak 21 effectively, but the icon is still shown on the standard theme.