BenjaminFavre / keycloak-apple-social-identity-provider

Apple Social Identity Provider for Keycloak
Apache License 2.0
108 stars 53 forks source link

Accept full content of private key file in Client Secret field #14

Open iudex- opened 3 years ago

iudex- commented 3 years ago

Please accept this humble PR to simplify the configuration and pottentially save hours of others in the future...

After rushing over the setup and spending hours trying to figure out why the login just wouldn't work it turned out that the result of cat *.p8 | base64 -w 0 just wasn't the value the client secret field expected :facepalm:

With this PR merged the Client Secret field will accept the full content of the private key file. Already correctly configured private key fields aren't affected by the change, as their values will survive replaceAll("[\r\n\t\f ]|-----.+?-----", "") (Regex test) unharmed.


Thank you so much for this extension! :slightly_smiling_face: I'm migrating to it from a custom build keycloak with the Apple IDP PR merged and all i needed to do was replace the client secret field. The approach with generating the client secret on the fly is neat. Previously it happened that our client secret expired...

BenjaminFavre commented 3 years ago

Hi @iudex- , you’re welcome, I’m glad this extension helped you! And thank you for your PR.

You are right saying the way the private key has to be set in not optimal. When I wrote this extension, I did not intend to share it; so I took the shortest path to my goal, neglecting UI. I think the most convenient would be a file input, because this is the way we get the key from Apple. If you take the time to replace the text input by a file input, I would happily merge it.

But I prefer not to merge your current contribution for this reason : some tutorials I don’t own have been written, helping people enable Apple login in Keycloak with my extension. A lot of people reading the tutorials instead of my doc may not notice the change if the UI do not warn them. Plus the README makes it very clear how to set the private key! It happens to misread the doc, I plead guilty too often :)

I did not invest a lot of time in this extension because I thought Keycloak would have promptly enabled Apple login natively. It’s not so evident now, they have trouble to automate testing. If confirmed, I will take time to improve it and set the file input myself.