kirby-oauth icon indicating copy to clipboard operation
kirby-oauth copied to clipboard

Map groups between Kirby and ResourceOwner

Open pReya opened this issue 1 year ago • 8 comments

Hey @thathoff,

thanks for your work and keeping this plugin updated <3.

I'd like to implement a use case, where SSO/OAuth users have different groups on the Identity Provider (Nextcloud via https://github.com/bahuma20/oauth2-nextcloud in my case).

Any chance you see a way to implement a simple mapping between groups? E.g. if $provider->getResourceOwner($token)->getGroups() (documentation on the Provider side) contains admin, then assign Kirby group admin.

I'm not entirely sure how this could be configured in a nice and clean way. But this is quite a common use case for SSO/Oauth, right?

EDIT: For my use case a groupWhitelist could be enough, but it seems a proper mapping would still make sense as a general feature.

pReya avatar Feb 14 '23 16:02 pReya

You’re welcome! And it is a common use case indeed! I already thought about having a hook inside the loginUser() (here) method to implement custom modifications to the Kirby user based on the OAuth ResourceOwner.

thathoff avatar Feb 14 '23 16:02 thathoff

Yeah, I guess a hook basically covers all use cases and is more flexible than hard coding specific user props. Any chance you can put the hook functionality on your roadmap?

pReya avatar Feb 14 '23 16:02 pReya

Since there is no roadmap for this plug-in I can't add it. But I think this makes the plug-in a lot more flexible this will be the next feature I'm going to implement.

If that's not fast enough for your case or someone else likes to help here: Pull requests are very welcome... 😉

thathoff avatar Feb 14 '23 17:02 thathoff

What would you suggest the function signature for this hook should look like? What are its parameters?

pReya avatar Feb 15 '23 11:02 pReya

I’m thinking about having a hook that gets the $oauthUser (ResourceOwnerInterface) as a first parameter and the second one is the Kirby user. Then you can modify the user or even implement a gate by throwing an exception to prevent the user login. I’m not sure if it should replace/bypass the whole creation and check logic currently I would say it should be an addition.

thathoff avatar Feb 16 '23 09:02 thathoff

I’m not sure if it should replace/bypass the whole creation and check logic currently I would say it should be an addition.

Would probably be nice if this was optional – maybe a config option? Or could this somehow be conifgured/decided within the hook function? Could call the existing login logic inside the hook (if wanted)?

pReya avatar Feb 16 '23 17:02 pReya

@thathoff In the last year I've learned a lot more about Oauth, and I have an upcoming project, where I need to distinguish users based on their groups in the Identity Provider again.

I have thought about implementing this, but I'm currently a little lost regarding the PHP League Oauth Interface. The interface doesn't have any functions for getting custom claim (like groups) from the token. So even with your suggested hook solution, we'd only have the ResourceOwnerInterface which doesn't offer anything to access custom claims from the token.

pReya avatar Jan 10 '24 08:01 pReya

This depends largely on your Oauth provider. I remember from a project that fetching the groups for a Google user is pretty hard to do. Have you tried to fetch the groups with the $provider->getAuthenticatedRequest() method and maybe an API providing the groups of the user? (see https://oauth2-client.thephpleague.com/usage/ for an example).

thathoff avatar Jan 11 '24 19:01 thathoff