PasswordsettingsAction aligned with FormAction

Also made some changes in the password "munging" function call
common_munge_password to accept a profile instead of user ID (which
was only there because stoneage StatusNet used the ID to generate a
not-very-random salt, but nowadays we primarily use AuthCrypt plugin).

diff --git a/EVENTS.txt b/EVENTS.txt
index cfba97403b20e1918193cb34483aabc5063d6750..2f91a305f0cce8eaec4ac19c21f4f898a855af4c 100644 (file)
--- a/EVENTS.txt
+++ b/EVENTS.txt
@@ -615,12 +615,12 @@ EndCheckPassword: After checking a username/password pair
 - $authenticatedUser: User object if credentials match a user, else null.
 
 StartChangePassword: Before changing a password
-- $user: user
+- Profile $target: The profile of the User that is changing password
 - $oldpassword: the user's old password
 - $newpassword: the desired new password
 
 EndChangePassword: After changing a password
-- $user: user
+- Profile $target: The profile of the User that just changed its password
 
 StartHashPassword: Generate a hashed version of the password (like a salted crypt)
 - &$hashed: Hashed version of the password, later put in the database

diff --git a/actions/passwordsettings.php b/actions/passwordsettings.php
index db36b612a29becfa806733c885e57ff955504f48..cfdb6c78176e221fcef24a1cd3e7d04feb48584b 100644 (file)
--- a/actions/passwordsettings.php
+++ b/actions/passwordsettings.php
@@ -28,11 +28,7 @@
  * @link      http://status.net/
  */
 
-if (!defined('STATUSNET') && !defined('LACONICA')) {
-    exit(1);
-}
-
-
+if (!defined('STATUSNET')) { exit(1); }
 
 /**
  * Change password
@@ -77,18 +73,8 @@ class PasswordsettingsAction extends SettingsAction
         $this->autofocus('oldpassword');
     }
 
-    /**
-     * Content area of the page
-     *
-     * Shows a form for changing the password
-     *
-     * @return void
-     */
-
     function showContent()
     {
-        $user = common_current_user();
-
         $this->elementStart('form', array('method' => 'POST',
                                           'id' => 'form_password',
                                           'class' => 'form_settings',
@@ -102,7 +88,7 @@ class PasswordsettingsAction extends SettingsAction
 
         $this->elementStart('ul', 'form_data');
         // Users who logged in with OpenID won't have a pwd
-        if ($user->password) {
+        if ($this->scoped->hasPassword()) {
             $this->elementStart('li');
             // TRANS: Field label on page where to change password.
             $this->password('oldpassword', _('Old password'));
@@ -129,29 +115,8 @@ class PasswordsettingsAction extends SettingsAction
         $this->elementEnd('form');
     }
 
-    /**
-     * Handle a post
-     *
-     * Validate input and save changes. Reload the form with a success
-     * or error message.
-     *
-     * @return void
-     */
-    function handlePost()
+    protected function doPost()
     {
-        // CSRF protection
-
-        $token = $this->trimmed('token');
-        if (!$token || $token != common_session_token()) {
-            // TRANS: Client error displayed when the session token does not match or is not given.
-            $this->showForm(_('There was a problem with your session token. '.
-                               'Try again, please.'));
-            return;
-        }
-
-        $user = common_current_user();
-        assert(!is_null($user)); // should already be checked
-
         // FIXME: scrub input
 
         $newpassword = $this->arg('newpassword');
@@ -161,49 +126,44 @@ class PasswordsettingsAction extends SettingsAction
 
         if (strlen($newpassword) < 6) {
             // TRANS: Form validation error on page where to change password.
-            $this->showForm(_('Password must be 6 or more characters.'));
-            return;
+            throw new ClientException(_('Password must be 6 or more characters.'));
         } else if (0 != strcmp($newpassword, $confirm)) {
             // TRANS: Form validation error on password change when password confirmation does not match.
-            $this->showForm(_('Passwords do not match.'));
-            return;
+            throw new ClientException(_('Passwords do not match.'));
         }
 
-        if ($user->password) {
+        $oldpassword = null;
+        if ($this->scoped->hasPassword()) {
             $oldpassword = $this->arg('oldpassword');
 
-            if (!common_check_user($user->nickname, $oldpassword)) {
+            if (!common_check_user($this->scoped->getNickname(), $oldpassword)) {
                 // TRANS: Form validation error on page where to change password.
-                $this->showForm(_('Incorrect old password.'));
-                return;
+                throw new ClientException(_('Incorrect old password.'));
             }
-        }else{
-            $oldpassword = null;
         }
 
-        $success = false;
-        if(Event::handle('StartChangePassword', array($user, $oldpassword, $newpassword))){
+        if (Event::handle('StartChangePassword', array($this->scoped, $oldpassword, $newpassword))) {
             //no handler changed the password, so change the password internally
+            $user = $this->scoped->getUser();
             $original = clone($user);
 
-            $user->password = common_munge_password($newpassword, $user->id);
+            $user->password = common_munge_password($newpassword, $this->scoped);
 
             $val = $user->validate();
             if ($val !== true) {
                 // TRANS: Form validation error on page where to change password.
-                $this->showForm(_('Error saving user; invalid.'));
-                return;
+                throw new ServerException(_('Error saving user; invalid.'));
             }
 
             if (!$user->update($original)) {
                 // TRANS: Server error displayed on page where to change password when password change
                 // TRANS: could not be made because of a server error.
-                $this->serverError(_('Cannot save new password.'));
+                throw new ServerException(_('Cannot save new password.'));
             }
-            Event::handle('EndChangePassword', array($user));
+            Event::handle('EndChangePassword', array($this->scoped));
         }
 
         // TRANS: Form validation notice on page where to change password.
-        $this->showForm(_('Password saved.'), true);
+        return _('Password saved.');
     }
 }

diff --git a/actions/recoverpassword.php b/actions/recoverpassword.php
index 060ba83510b44fb82fc870e7f7d7f74e49e6a5c3..1cbb73fd4a21fa5528fcabcaf6506dfb4848b083 100644 (file)
--- a/actions/recoverpassword.php
+++ b/actions/recoverpassword.php
@@ -325,7 +325,7 @@ class RecoverpasswordAction extends Action
 
         $original = clone($user);
 
-        $user->password = common_munge_password($newpassword, $user->id);
+        $user->password = common_munge_password($newpassword, $user->getProfile());
 
         if (!$user->update($original)) {
             common_log_db_error($user, 'UPDATE', __FILE__);

diff --git a/classes/Profile.php b/classes/Profile.php
index 384eaa07263e7414056ee7c8445ecaff5bdf0964..5709a15d01060513b7b63b956d63f227edafdf56 100644 (file)
--- a/classes/Profile.php
+++ b/classes/Profile.php
@@ -138,6 +138,18 @@ class Profile extends Managed_DataObject
         return true;
     }
 
+    // Returns false if the user has no password (which will always
+    // be the case for remote users). This can be the case for OpenID
+    // logins or other mechanisms which don't store a password hash.
+    public function hasPassword()
+    {
+        try {
+            return !empty($this->getUser()->hasPassword());
+        } catch (NoSuchUserException $e) {
+            return false;
+        }
+    }
+
     public function getObjectType()
     {
         // FIXME: More types... like peopletags and whatever

diff --git a/classes/User.php b/classes/User.php
index e1ce33be181dad1e387e3b0143301abd2dfe0c6d..175d945401e2e25538a8ec5526ea1ff50234e743 100644 (file)
--- a/classes/User.php
+++ b/classes/User.php
@@ -299,7 +299,7 @@ class User extends Managed_DataObject
             }
 
             if (!empty($password)) { // may not have a password for OpenID users
-                $user->password = common_munge_password($password, $id);
+                $user->password = common_munge_password($password);
             }
 
             $result = $user->insert();
@@ -1015,6 +1015,11 @@ class User extends Managed_DataObject
         return $this->getProfile()->isPrivateStream();
     }
 
+    public function hasPassword()
+    {
+        return !empty($this->password);
+    }
+
     public function delPref($namespace, $topic)
     {
         return $this->getProfile()->delPref($namespace, $topic);

diff --git a/lib/authenticationplugin.php b/lib/authenticationplugin.php
index 66f11ca1a9507a5850bdb11418314b8407a39020..c33b0ef8a8b3dac054248cbb8390668482c41f87 100644 (file)
--- a/lib/authenticationplugin.php
+++ b/lib/authenticationplugin.php
@@ -201,13 +201,13 @@ abstract class AuthenticationPlugin extends Plugin
         }
     }
 
-    function onStartChangePassword($user,$oldpassword,$newpassword)
+    function onStartChangePassword(Profile $target ,$oldpassword, $newpassword)
     {
         if($this->password_changeable){
             $user_username = new User_username();
-            $user_username->user_id=$user->id;
+            $user_username->user_id = $target->getID();
             $user_username->provider_name=$this->provider_name;
-            if($user_username->find() && $user_username->fetch()){
+            if ($user_username->find(true)) {
                 $authenticated = $this->checkPassword($user_username->username, $oldpassword);
                 if($authenticated){
                     $result = $this->changePassword($user_username->username,$oldpassword,$newpassword);

diff --git a/lib/util.php b/lib/util.php
index 4949231c520545a7cb9295dfa982a696ec010fd5..1ff5b13b938b0bff8cb61d482119ba275b8523e5 100644 (file)
--- a/lib/util.php
+++ b/lib/util.php
@@ -210,7 +210,7 @@ function common_language()
 /**
  * Salted, hashed passwords are stored in the DB.
  */
-function common_munge_password($password, $id, Profile $profile=null)
+function common_munge_password($password, Profile $profile=null)
 {
     $hashed = null;
 
@@ -245,8 +245,7 @@ function common_check_user($nickname, $password)
         }
 
         if ($user instanceof User && !empty($password)) {
-            if (0 == strcmp(common_munge_password($password, $user->id),
-                            $user->password)) {
+            if (0 == strcmp(common_munge_password($password, $user->getProfile()), $user->password)) {
                 //internal checking passed
                 $authenticatedUser = $user;
             }

diff --git a/plugins/AuthCrypt/AuthCryptPlugin.php b/plugins/AuthCrypt/AuthCryptPlugin.php
index c6695667725d904683e38d926568ee59dfd881d2..540019f9c25955f28aa9d864182e8861dd2998c9 100644 (file)
--- a/plugins/AuthCrypt/AuthCryptPlugin.php
+++ b/plugins/AuthCrypt/AuthCryptPlugin.php
@@ -110,17 +110,17 @@ class AuthCryptPlugin extends AuthenticationPlugin
      * EVENTS
      */
 
-    public function onStartChangePassword($user, $oldpassword, $newpassword)
+    public function onStartChangePassword(Profile $target, $oldpassword, $newpassword)
     {
-        if (!$this->checkPassword($user->nickname, $oldpassword)) {
+        if (!$this->checkPassword($target->getNickname(), $oldpassword)) {
             // if we ARE in overwrite mode, test password with common_check_user
-            if (!$this->overwrite || !common_check_user($user->nickname, $oldpassword)) {
+            if (!$this->overwrite || !common_check_user($target->getNickname(), $oldpassword)) {
                 // either we're not in overwrite mode, or the password was incorrect
                 return !$this->authoritative;
             }
             // oldpassword was apparently ok
         }
-        $changed = $this->changePassword($user->nickname, $oldpassword, $newpassword);
+        $changed = $this->changePassword($target->getNickname(), $oldpassword, $newpassword);
 
         return (!$changed && empty($this->authoritative));
     }

diff --git a/plugins/RequireValidatedEmail/actions/confirmfirstemail.php b/plugins/RequireValidatedEmail/actions/confirmfirstemail.php
index cc4b646f6b024c37e84fb01d370af8f8687b7498..d0d1893739bd03400d91cef554bce9b5aa5ab67e 100644 (file)
--- a/plugins/RequireValidatedEmail/actions/confirmfirstemail.php
+++ b/plugins/RequireValidatedEmail/actions/confirmfirstemail.php
@@ -157,7 +157,7 @@ class ConfirmfirstemailAction extends Action
 
         $orig = clone($this->user);
 
-        $this->user->password = common_munge_password($this->password, $this->user->id);
+        $this->user->password = common_munge_password($this->password, $this->user->getProfile());
 
         $this->user->update($orig);