Skip to content

Add Keycloak socialite provider#2755

Closed
cyber-simon wants to merge 4 commits intoBookStackApp:masterfrom
cyber-simon:master
Closed

Add Keycloak socialite provider#2755
cyber-simon wants to merge 4 commits intoBookStackApp:masterfrom
cyber-simon:master

Conversation

@cyber-simon
Copy link

Simple change to add the Keycloak provider from the existing Socialite providers.

@ssddanbrown
Copy link
Member

Thanks for offering this @cyber-simon, But I've come to the decision I don't want to grow out the supported third-party social provides much further, at least without a high level of benefit/request from the user-base. It's not a large amount of work to initially implement but it gets troublesome attempting to support people here when issues eventually arise.

Within the latest release, v21.04, I've added a system where you can perform some level of back-end customization. Details on this system can be seen here: https://github.com/BookStackApp/BookStack/blob/release/dev/docs/logical-theme-system.md
It's still in the testing stages, and I'm looking for feedback on it, but it should be possible to use additional socialite providers via this system, An example of this is shown at the bottom using Reddit which I tested during development of that system.

@Meteorus
Copy link

@ssddanbrown this is really a sad decision. I'd like to use this feature for OpenID. So i would be happy too, if you merge this feature into the master branch.

@etgocode
Copy link

etgocode commented May 19, 2021

That's unfortunate, this feature would have saved me at least 2 working days. :(

@cyber-simon
Copy link
Author

cyber-simon commented May 20, 2021

@ssddanbrown Hey Dan! I tried your new solution, and this seems to be a nice way of doing this. Unfortunetly i ran into one bigger problem:

The Keycloak socialite doesn't work, without a scope set (in the getDriver method):

if ($driverName === 'keycloak') {
    $driver->scopes(['openid']);        
}

If this one is not set, the redirect parameter scope will be empty, which violates the OpenID Connect standard. Keycloak won't accept the request.

Would it be okay, to take these lines in? Could be configurable like config('services.keycloak.scope'). Or would it be better, to define a getDriver hook in the funtions.php?

@ssddanbrown
Copy link
Member

Thanks for the feedback @cyber-simon.
I've opened #2759 to allow you to do that via some level of configureDriver (Or similar) callback using the theme system. Have assigned to the next patch release.

@ssddanbrown
Copy link
Member

Hi @cyber-simon,
I've just released v21.04.6 with the additional option to configure the driver.
Please see the very bottom example on the updated docs page: https://github.com/BookStackApp/BookStack/blob/master/dev/docs/logical-theme-system.md

Just an advisory, in the future I may be looking to include https://github.com/wikimedia/composer-merge-plugin as a method to handle multiple composer.json's so you don't have to manually import the required classes from the Socialite provider, just have them installed within your theme's composer usage. This should be better in the long run but could be a breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants