Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature proposal] Event when user isn't found & ability to externally override the user (LDAP example) #11645

Merged
merged 7 commits into from
Jul 26, 2022

Conversation

bertoost
Copy link
Contributor

Description

In Craft 2, I had developed a LDAP plugin which created users when not found locally in the Craft CMS system. This plugin also had interface settings and options to configure which groups from LDAP should have which permissions in Craft etc.

When a user is not in the Craft CMS system, there is no way (as far I know of) to check an external source, create the user and continue the authentication process, since the user is immediately told the user was not found.

The idea of adding this event is to give plugins the ability to hook in and create the Craft user and continue authentication.

With this update I have managed to create the local user, validate the external password and login to Craft like a normal user.

Explanation

  • When a user isn't found in Craft, fire new event
  • Plugins can hook in to check their external source, like LDAP
  • When found, create the local Craft user (with all their necessary settings)
  • Set the user back to the event, and tell Craft; it's handled!
  • Then the "before authenticate" is triggered...
  • The plugin can check the password in the external source
  • When validated, update the local Craft user's password (for local fallback purposes)
  • And the user will be logged in with their account.

Further development...

Probably you have some thoughts, better ideas, better naming etc.
This is like a PoC proposal to get you involved and to work on improvements.

Like to hear again. Thanks!

Regards, Bert

bertoost added 2 commits July 20, 2022 21:29
…over

This will make it possible to create the user based on external sources like LDAP, to create the user in the Craft system
@bertoost bertoost requested a review from a team as a code owner July 20, 2022 20:12
Comment on lines 175 to 180
if (!$user || $user->password === null) {
// Delay again to match $user->authenticate()'s delay
Craft::$app->getSecurity()->validatePassword('p@ss1w0rd', '$2y$13$nj9aiBeb7RfEfYP3Cum6Revyu14QelGGxwcnFUKXIrQUitSodEPRi');
return $this->_handleLoginFailure(User::AUTH_INVALID_CREDENTIALS);

$event = new LoginUserNotFoundEvent([
'loginName' => $loginName,
]);
$this->trigger(self::EVENT_LOGIN_USER_NOT_FOUND, $event);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think this should be triggered in the event that $user was found but $user->password === null. Maybe just check for if (!$user) here, and then add a separate if ($user->password === null) check after it, keeping the original conditional block inside it.

Copy link
Contributor Author

@bertoost bertoost Jul 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I understand.. Change on it's way

@brandonkelly brandonkelly changed the base branch from develop to 4.2 July 25, 2022 23:19
@brandonkelly brandonkelly merged commit 7ab889f into craftcms:4.2 Jul 26, 2022
@brandonkelly
Copy link
Member

Made a bunch of tweaks – there’s now two events: EVENT_BEFORE_FIND_LOGIN_USER and EVENT_AFTER_FIND_LOGIN_USER, which will each get triggered regardless of whether a user is found.

And merged for 4.2!

@bertoost
Copy link
Contributor Author

Thanks @brandonkelly ! That was the idea of my PoC and I'm thrilled this is going to be in 4.2.
Will continue to develop my LDAP plugin 😄

@bertoost bertoost deleted the feature/login-user-source branch July 26, 2022 05:43
@brandonkelly
Copy link
Member

4.2.0 is out now with those events ✨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants