Added plugin hooks before and after the kirby user gets logged in

[Greakz]

Added hooks before and after the kirby user gets logged in. This was suggested in https://github.com/thathoff/kirby-oauth/issues/30#issuecomment-1432739791

An example use-case is to process the given roles in the oAuthUserData to assign kirby roles. It would be nice if something like this could be added to the plugin. Feel free to change anything.

[thathoff]

Thanks for your pull request. I would prefer to have the second parameter of the hook require a ResourceOwnerInterface as this is typed and could make things more flexible (so in code it would be $oauthUser).

I would also move the before hook before the whole gate logic starting here so it's possible to prevent user creation with the hook.

[Greakz]

So, for my understanding: Instead of the oauthUserData you want to pass the oauthUser to provide the full ResourceOwnerInterface. This would change the first parameter of the hook. The second is the kirby user, so that an additional plugin can work with the oauthUser and the kirby-user.

You suggest the before hook in front of line 148 to make it possible to prevent user creation? Or do we need to put that hook inside the If-clause to only trigger when the user does not exist (in this case, before line 149)? This hook would not be a login:before-hook in my opinion because it's before the user-creation, so this should be a third hook with only the oauthUser I think.

Something like: $this->kirby->trigger('thathoff.oauth.user-creation:before', ['oauthUser' => $oauthUser]);

Do you agree, or did I misunderstand something?

[thathoff]

Sorry for my late reply, I’ve been on vacation.

Instead of the oauthUserData you want to pass the oauthUser to provide the full ResourceOwnerInterface. This would change the first parameter of the hook. The second is the kirby user, so that an additional plugin can work with the oauthUser and the kirby-user.

Yes you’re right.

You suggest the before hook in front of line 148 to make it possible to prevent user creation? Or do we need to put that hook inside the If-clause to only trigger when the user does not exist (in this case, before line 149)? This hook would not be a login:before-hook in my opinion because it's before the user-creation, so this should be a third hook with only the oauthUser I think.

Then i would go with:

$createResult = $this->kirby->trigger('thathoff.oauth.user-create:before', ['oauthUser' => $oauthUser]);
$kirbyUser = null;

// if the hook returns a kirby user we take this one
if ($createResult instanceof \Kirby\Cms\User) {
   $kirbyUser = $createResult
}

// if the hook returns null or true the user is created
if ($createResult === true || $createResult === null) {
   // go on with user creation
}

if (!$kirbyUser) {
    $this->error("User cannot be created.");
}

and

$this->kirby->trigger('thathoff.oauth.user-create:after', ['oauthUser' => $oauthUser, 'user' => $kirbyUser]);

[pReya]

@Greakz Will you have time to implement the changes suggested by @thathoff ?

[Greakz]

Sorry for my late reply, I’ve been on vacation.

Same here, sorry.

@Greakz Will you have time to implement the changes suggested by @thathoff ?

Yes, the changes should be implemented. The only thing left to do is change the version in composer.json (I guess this will be 3.1.0?) but i think this is up to you @thathoff

Tell me if you want other changes in this PullRequest or if you are fine with what we got now @thathoff

[thathoff]

Thank you! I’ll check everything and maybe include this feature right into the next release.