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

Password validation extension #298

Merged
merged 3 commits into from Jun 10, 2014

Conversation

czolnowski
Copy link
Contributor

Extended password validation based on hooks and PasswordValidator interface.

@mattab
Copy link
Member

mattab commented Jun 2, 2014

Thanks for the suggestion!

In this case, i think it is over-engineered to add logic of password validators in core. Why generalise password validators, when we have only one in core? The goal of course is to let plugins add more validators. But In this case, IMHO it is enough to add a new event called 'UsersManager.checkPassword' in this method. Then any plugin could throw an exception when password is not valid. (and plugins can decide for themselves to implement password validators).

@@ -444,7 +447,7 @@ public function updateUser($userLogin, $password = false, $email = false, $alias
*
* @param string $userLogin The user's login handle.
*/
Piwik::postEvent('UsersManager.updateUser.end', array($userLogin));
Piwik::postEvent('UsersManager.updateUser.end', array($userLogin, $passwordHasBeenUpdated));
Copy link
Member

Choose a reason for hiding this comment

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

add $passwordHasBeenUpdated flag to the Event comment just above, so it shows up in the auto generated doc: http://developer.piwik.org/api-reference/events

…mplement in hook result. (in separate plugins)

Added events documentation.
@mattab
Copy link
Member

mattab commented Jun 10, 2014

Looks good to me!

mattab pushed a commit that referenced this pull request Jun 10, 2014
@mattab mattab merged commit 3b21c5d into matomo-org:master Jun 10, 2014
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.

None yet

2 participants