From a21376480df10fdb96685d0eb2e663d494aed16f Mon Sep 17 00:00:00 2001 From: kondou Date: Fri, 6 Sep 2013 08:05:07 +0200 Subject: [PATCH 1/6] Split personal and user-mgmt password change logic --- settings/ajax/changepassword.php | 36 ++++++++++++------------ settings/ajax/changepersonalpassword.php | 24 ++++++++++++++++ settings/js/personal.js | 11 +++++--- settings/js/users.js | 2 +- settings/routes.php | 2 ++ 5 files changed, 52 insertions(+), 23 deletions(-) create mode 100644 settings/ajax/changepersonalpassword.php diff --git a/settings/ajax/changepassword.php b/settings/ajax/changepassword.php index 47ceb5ab873..41f0fa2f2fd 100644 --- a/settings/ajax/changepassword.php +++ b/settings/ajax/changepassword.php @@ -1,34 +1,34 @@ array('message' => $l->t('No user supplied')) )); + exit(); +} + +$password = isset($_POST['password']) ? $_POST['password'] : null; $recoveryPassword = isset($_POST['recoveryPassword']) ? $_POST['recoveryPassword'] : null; -$userstatus = null; if (OC_User::isAdminUser(OC_User::getUser())) { $userstatus = 'admin'; -} -if (OC_SubAdmin::isUserAccessible(OC_User::getUser(), $username)) { +} elseif (OC_SubAdmin::isUserAccessible(OC_User::getUser(), $username)) { $userstatus = 'subadmin'; -} -if (OC_User::getUser() === $username && OC_User::checkPassword($username, $oldPassword)) { - $userstatus = 'user'; -} - -if (is_null($userstatus)) { - OC_JSON::error(array('data' => array('message' => 'Authentication error'))); +} else { + $l = new \OC_L10n('settings'); + OC_JSON::error(array('data' => array('message' => $l->t('Authentication error')) )); exit(); } -if (\OCP\App::isEnabled('files_encryption') && $userstatus !== 'user') { +if (\OC_App::isEnabled('files_encryption')) { //handle the recovery case $util = new \OCA\Encryption\Util(new \OC_FilesystemView('/'), $username); $recoveryAdminEnabled = OC_Appconfig::getValue('files_encryption', 'recoveryAdminEnabled'); @@ -55,7 +55,7 @@ if (\OCP\App::isEnabled('files_encryption') && $userstatus !== 'user') { } } -} else { // if user changes his own password or if encryption is disabled, proceed +} else { // if encryption is disabled, proceed if (!is_null($password) && OC_User::setPassword($username, $password)) { OC_JSON::success(array('data' => array('username' => $username))); } else { diff --git a/settings/ajax/changepersonalpassword.php b/settings/ajax/changepersonalpassword.php new file mode 100644 index 00000000000..6c3f5d599ac --- /dev/null +++ b/settings/ajax/changepersonalpassword.php @@ -0,0 +1,24 @@ + array("message" => $l->t("Wrong password")) )); + exit(); +} +if (!is_null($password) && OC_User::setPassword($username, $password)) { + OC_JSON::success(); +} else { + OC_JSON::error(); +} diff --git a/settings/js/personal.js b/settings/js/personal.js index 8ad26c086b5..8cf4754f793 100644 --- a/settings/js/personal.js +++ b/settings/js/personal.js @@ -52,14 +52,17 @@ $(document).ready(function(){ $('#passwordchanged').hide(); $('#passworderror').hide(); // Ajax foo - $.post( 'ajax/changepassword.php', post, function(data){ + $.post(OC.Router.generate('settings_ajax_changepersonalpassword'), post, function(data){ if( data.status === "success" ){ $('#pass1').val(''); $('#pass2').val(''); $('#passwordchanged').show(); - } - else{ - $('#passworderror').html( data.data.message ); + } else{ + if (typeof(data.data) !== "undefined") { + $('#passworderror').html(data.data.message); + } else { + $('#passworderror').html(t('Unable to change password')); + } $('#passworderror').show(); } }); diff --git a/settings/js/users.js b/settings/js/users.js index ab08d7099c6..e3e749a312e 100644 --- a/settings/js/users.js +++ b/settings/js/users.js @@ -361,7 +361,7 @@ $(document).ready(function () { if ($(this).val().length > 0) { var recoveryPasswordVal = $('input:password[id="recoveryPassword"]').val(); $.post( - OC.filePath('settings', 'ajax', 'changepassword.php'), + OC.Router.generate('settings_ajax_changepassword'), {username: uid, password: $(this).val(), recoveryPassword: recoveryPasswordVal}, function (result) { if (result.status != 'success') { diff --git a/settings/routes.php b/settings/routes.php index 73ee70d1d5c..af1c70ea44d 100644 --- a/settings/routes.php +++ b/settings/routes.php @@ -39,6 +39,8 @@ $this->create('settings_ajax_removegroup', '/settings/ajax/removegroup.php') ->actionInclude('settings/ajax/removegroup.php'); $this->create('settings_ajax_changepassword', '/settings/ajax/changepassword.php') ->actionInclude('settings/ajax/changepassword.php'); +$this->create('settings_ajax_changepersonalpassword', '/settings/ajax/changepersonalpassword.php') + ->actionInclude('settings/ajax/changepersonalpassword.php'); $this->create('settings_ajax_changedisplayname', '/settings/ajax/changedisplayname.php') ->actionInclude('settings/ajax/changedisplayname.php'); // personel From 4aa84047fe5c499c56b723006c8acaf5891c5df4 Mon Sep 17 00:00:00 2001 From: kondou Date: Fri, 6 Sep 2013 17:05:10 +0200 Subject: [PATCH 2/6] Remove $recoveryPassword from changepersonalpassword & fix indent --- settings/ajax/changepassword.php | 20 ++++++++++++-------- settings/ajax/changepersonalpassword.php | 1 - 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/settings/ajax/changepassword.php b/settings/ajax/changepassword.php index 41f0fa2f2fd..67b23d2a19c 100644 --- a/settings/ajax/changepassword.php +++ b/settings/ajax/changepassword.php @@ -45,14 +45,18 @@ if (\OC_App::isEnabled('files_encryption')) { } elseif ($recoveryEnabledForUser && ! $validRecoveryPassword) { OC_JSON::error(array('data' => array('message' => 'Wrong admin recovery password. Please check the password and try again.'))); } else { // now we know that everything is fine regarding the recovery password, let's try to change the password - $result = OC_User::setPassword($username, $password, $recoveryPassword); - if (!$result && $recoveryPasswordSupported) { - OC_JSON::error(array("data" => array( "message" => "Back-end doesn't support password change, but the users encryption key was successfully updated." ))); - } elseif (!$result && !$recoveryPasswordSupported) { - OC_JSON::error(array("data" => array( "message" => "Unable to change password" ))); - } else { - OC_JSON::success(array("data" => array( "username" => $username ))); - } + $result = OC_User::setPassword($username, $password, $recoveryPassword); + if (!$result && $recoveryPasswordSupported) { + OC_JSON::error(array( + "data" => array( + "message" => "Back-end doesn't support password change, but the users encryption key was successfully updated." + ) + )); + } elseif (!$result && !$recoveryPasswordSupported) { + OC_JSON::error(array("data" => array( "message" => "Unable to change password" ))); + } else { + OC_JSON::success(array("data" => array( "username" => $username ))); + } } } else { // if encryption is disabled, proceed diff --git a/settings/ajax/changepersonalpassword.php b/settings/ajax/changepersonalpassword.php index 6c3f5d599ac..44ede3f9cc6 100644 --- a/settings/ajax/changepersonalpassword.php +++ b/settings/ajax/changepersonalpassword.php @@ -10,7 +10,6 @@ OC_App::loadApps(); $username = OC_User::getUser(); $password = isset($_POST['personal-password']) ? $_POST['personal-password'] : null; $oldPassword = isset($_POST['oldpassword']) ? $_POST['oldpassword'] : ''; -$recoveryPassword = isset($_POST['recoveryPassword']) ? $_POST['recoveryPassword'] : null; if (!OC_User::checkPassword($username, $oldPassword)) { $l = new \OC_L10n('settings'); From f6faec0e0bfddb14cc17f4a7f60900438215dd35 Mon Sep 17 00:00:00 2001 From: kondou Date: Wed, 11 Sep 2013 16:35:13 +0200 Subject: [PATCH 3/6] Use a controller instead of two files for changepassword.php --- settings/ajax/changepassword.php | 138 ++++++++++++++--------- settings/ajax/changepersonalpassword.php | 23 ---- settings/routes.php | 15 ++- 3 files changed, 94 insertions(+), 82 deletions(-) delete mode 100644 settings/ajax/changepersonalpassword.php diff --git a/settings/ajax/changepassword.php b/settings/ajax/changepassword.php index 67b23d2a19c..53bd69a2cd0 100644 --- a/settings/ajax/changepassword.php +++ b/settings/ajax/changepassword.php @@ -1,68 +1,98 @@ array('message' => $l->t('No user supplied')) )); - exit(); -} + // Manually load apps to ensure hooks work correctly (workaround for issue 1503) + \OC_App::loadApps(); -$password = isset($_POST['password']) ? $_POST['password'] : null; -$recoveryPassword = isset($_POST['recoveryPassword']) ? $_POST['recoveryPassword'] : null; + $username = \OC_User::getUser(); + $password = isset($_POST['personal-password']) ? $_POST['personal-password'] : null; + $oldPassword = isset($_POST['oldpassword']) ? $_POST['oldpassword'] : ''; -if (OC_User::isAdminUser(OC_User::getUser())) { - $userstatus = 'admin'; -} elseif (OC_SubAdmin::isUserAccessible(OC_User::getUser(), $username)) { - $userstatus = 'subadmin'; -} else { - $l = new \OC_L10n('settings'); - OC_JSON::error(array('data' => array('message' => $l->t('Authentication error')) )); - exit(); -} + if (!\OC_User::checkPassword($username, $oldPassword)) { + $l = new \OC_L10n('settings'); + \OC_JSON::error(array("data" => array("message" => $l->t("Wrong password")) )); + exit(); + } + if (!is_null($password) && \OC_User::setPassword($username, $password)) { + \OC_JSON::success(); + } else { + \OC_JSON::error(); + } + } -if (\OC_App::isEnabled('files_encryption')) { - //handle the recovery case - $util = new \OCA\Encryption\Util(new \OC_FilesystemView('/'), $username); - $recoveryAdminEnabled = OC_Appconfig::getValue('files_encryption', 'recoveryAdminEnabled'); + public static function changeUserPassword($args) { + // Check if we are an user + \OC_JSON::callCheck(); + \OC_JSON::checkLoggedIn(); - $validRecoveryPassword = false; - $recoveryPasswordSupported = false; - if ($recoveryAdminEnabled) { - $validRecoveryPassword = $util->checkRecoveryPassword($recoveryPassword); - $recoveryEnabledForUser = $util->recoveryEnabledForUser(); - } + // Manually load apps to ensure hooks work correctly (workaround for issue 1503) + \OC_App::loadApps(); - if ($recoveryEnabledForUser && $recoveryPassword === '') { - OC_JSON::error(array('data' => array('message' => 'Please provide a admin recovery password, otherwise all user data will be lost'))); - } elseif ($recoveryEnabledForUser && ! $validRecoveryPassword) { - OC_JSON::error(array('data' => array('message' => 'Wrong admin recovery password. Please check the password and try again.'))); - } else { // now we know that everything is fine regarding the recovery password, let's try to change the password - $result = OC_User::setPassword($username, $password, $recoveryPassword); - if (!$result && $recoveryPasswordSupported) { - OC_JSON::error(array( - "data" => array( - "message" => "Back-end doesn't support password change, but the users encryption key was successfully updated." - ) - )); - } elseif (!$result && !$recoveryPasswordSupported) { - OC_JSON::error(array("data" => array( "message" => "Unable to change password" ))); + if (isset($_POST['username'])) { + $username = $_POST['username']; } else { - OC_JSON::success(array("data" => array( "username" => $username ))); + $l = new \OC_L10n('settings'); + \OC_JSON::error(array('data' => array('message' => $l->t('No user supplied')) )); + exit(); } - } -} else { // if encryption is disabled, proceed - if (!is_null($password) && OC_User::setPassword($username, $password)) { - OC_JSON::success(array('data' => array('username' => $username))); - } else { - OC_JSON::error(array('data' => array('message' => 'Unable to change password'))); + $password = isset($_POST['password']) ? $_POST['password'] : null; + $recoveryPassword = isset($_POST['recoveryPassword']) ? $_POST['recoveryPassword'] : null; + + if (\OC_User::isAdminUser(\OC_User::getUser())) { + $userstatus = 'admin'; + } elseif (\OC_SubAdmin::isUserAccessible(\OC_User::getUser(), $username)) { + $userstatus = 'subadmin'; + } else { + $l = new \OC_L10n('settings'); + \OC_JSON::error(array('data' => array('message' => $l->t('Authentication error')) )); + exit(); + } + + if (\OC_App::isEnabled('files_encryption')) { + //handle the recovery case + $util = new \OCA\Encryption\Util(new \OC_FilesystemView('/'), $username); + $recoveryAdminEnabled = \OC_Appconfig::getValue('files_encryption', 'recoveryAdminEnabled'); + + $validRecoveryPassword = false; + $recoveryPasswordSupported = false; + if ($recoveryAdminEnabled) { + $validRecoveryPassword = $util->checkRecoveryPassword($recoveryPassword); + $recoveryEnabledForUser = $util->recoveryEnabledForUser(); + } + + if ($recoveryEnabledForUser && $recoveryPassword === '') { + \OC_JSON::error(array('data' => array('message' => 'Please provide a admin recovery password, otherwise all user data will be lost'))); + } elseif ($recoveryEnabledForUser && ! $validRecoveryPassword) { + \OC_JSON::error(array('data' => array('message' => 'Wrong admin recovery password. Please check the password and try again.'))); + } else { // now we know that everything is fine regarding the recovery password, let's try to change the password + $result = \OC_User::setPassword($username, $password, $recoveryPassword); + if (!$result && $recoveryPasswordSupported) { + \OC_JSON::error(array( + "data" => array( + "message" => "Back-end doesn't support password change, but the users encryption key was successfully updated." + ) + )); + } elseif (!$result && !$recoveryPasswordSupported) { + \OC_JSON::error(array("data" => array( "message" => "Unable to change password" ))); + } else { + \OC_JSON::success(array("data" => array( "username" => $username ))); + } + + } + } else { // if encryption is disabled, proceed + if (!is_null($password) && \OC_User::setPassword($username, $password)) { + \OC_JSON::success(array('data' => array('username' => $username))); + } else { + \OC_JSON::error(array('data' => array('message' => 'Unable to change password'))); + } + } } } diff --git a/settings/ajax/changepersonalpassword.php b/settings/ajax/changepersonalpassword.php deleted file mode 100644 index 44ede3f9cc6..00000000000 --- a/settings/ajax/changepersonalpassword.php +++ /dev/null @@ -1,23 +0,0 @@ - array("message" => $l->t("Wrong password")) )); - exit(); -} -if (!is_null($password) && OC_User::setPassword($username, $password)) { - OC_JSON::success(); -} else { - OC_JSON::error(); -} diff --git a/settings/routes.php b/settings/routes.php index af1c70ea44d..71de81aa6c4 100644 --- a/settings/routes.php +++ b/settings/routes.php @@ -6,6 +6,9 @@ * See the COPYING-README file. */ +// Necessary to include changepassword controller +OC::$CLASSPATH['OC\Settings\ChangePassword\Controller'] = 'settings/ajax/changepassword.php'; + // Settings pages $this->create('settings_help', '/settings/help') ->actionInclude('settings/help.php'); @@ -37,13 +40,15 @@ $this->create('settings_ajax_togglesubadmins', '/settings/ajax/togglesubadmins.p ->actionInclude('settings/ajax/togglesubadmins.php'); $this->create('settings_ajax_removegroup', '/settings/ajax/removegroup.php') ->actionInclude('settings/ajax/removegroup.php'); -$this->create('settings_ajax_changepassword', '/settings/ajax/changepassword.php') - ->actionInclude('settings/ajax/changepassword.php'); -$this->create('settings_ajax_changepersonalpassword', '/settings/ajax/changepersonalpassword.php') - ->actionInclude('settings/ajax/changepersonalpassword.php'); +$this->create('settings_ajax_changepassword', '/settings/users/changepassword') + ->post() + ->action('OC\Settings\ChangePassword\Controller', 'changeUserPassword'); $this->create('settings_ajax_changedisplayname', '/settings/ajax/changedisplayname.php') ->actionInclude('settings/ajax/changedisplayname.php'); -// personel +// personal +$this->create('settings_ajax_changepersonalpassword', '/settings/personal/changepassword') + ->post() + ->action('OC\Settings\ChangePassword\Controller', 'changePersonalPassword'); $this->create('settings_ajax_lostpassword', '/settings/ajax/lostpassword.php') ->actionInclude('settings/ajax/lostpassword.php'); $this->create('settings_ajax_setlanguage', '/settings/ajax/setlanguage.php') From 306a8681c5a4699d2f9e0375922000c85501def3 Mon Sep 17 00:00:00 2001 From: kondou Date: Fri, 13 Sep 2013 17:03:13 +0200 Subject: [PATCH 4/6] Move ajax/changepassword to changepassword/controller to use autoloading --- .../{ajax/changepassword.php => changepassword/controller.php} | 0 settings/routes.php | 3 --- 2 files changed, 3 deletions(-) rename settings/{ajax/changepassword.php => changepassword/controller.php} (100%) diff --git a/settings/ajax/changepassword.php b/settings/changepassword/controller.php similarity index 100% rename from settings/ajax/changepassword.php rename to settings/changepassword/controller.php diff --git a/settings/routes.php b/settings/routes.php index 71de81aa6c4..6778a2ab828 100644 --- a/settings/routes.php +++ b/settings/routes.php @@ -6,9 +6,6 @@ * See the COPYING-README file. */ -// Necessary to include changepassword controller -OC::$CLASSPATH['OC\Settings\ChangePassword\Controller'] = 'settings/ajax/changepassword.php'; - // Settings pages $this->create('settings_help', '/settings/help') ->actionInclude('settings/help.php'); From 18da2f9cf76815faeaa6ae162fa8af2d80aaeb3e Mon Sep 17 00:00:00 2001 From: kondou Date: Fri, 13 Sep 2013 17:07:23 +0200 Subject: [PATCH 5/6] Improve changepassword route naming --- settings/js/personal.js | 2 +- settings/js/users.js | 2 +- settings/routes.php | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/settings/js/personal.js b/settings/js/personal.js index e4284c2e8c6..74620f39810 100644 --- a/settings/js/personal.js +++ b/settings/js/personal.js @@ -52,7 +52,7 @@ $(document).ready(function(){ $('#passwordchanged').hide(); $('#passworderror').hide(); // Ajax foo - $.post(OC.Router.generate('settings_ajax_changepersonalpassword'), post, function(data){ + $.post(OC.Router.generate('settings_personal_changepassword'), post, function(data){ if( data.status === "success" ){ $('#pass1').val(''); $('#pass2').val(''); diff --git a/settings/js/users.js b/settings/js/users.js index e3e749a312e..d800de73f5b 100644 --- a/settings/js/users.js +++ b/settings/js/users.js @@ -361,7 +361,7 @@ $(document).ready(function () { if ($(this).val().length > 0) { var recoveryPasswordVal = $('input:password[id="recoveryPassword"]').val(); $.post( - OC.Router.generate('settings_ajax_changepassword'), + OC.Router.generate('settings_users_changepassword'), {username: uid, password: $(this).val(), recoveryPassword: recoveryPasswordVal}, function (result) { if (result.status != 'success') { diff --git a/settings/routes.php b/settings/routes.php index 6778a2ab828..60f9d8e1001 100644 --- a/settings/routes.php +++ b/settings/routes.php @@ -37,13 +37,13 @@ $this->create('settings_ajax_togglesubadmins', '/settings/ajax/togglesubadmins.p ->actionInclude('settings/ajax/togglesubadmins.php'); $this->create('settings_ajax_removegroup', '/settings/ajax/removegroup.php') ->actionInclude('settings/ajax/removegroup.php'); -$this->create('settings_ajax_changepassword', '/settings/users/changepassword') +$this->create('settings_users_changepassword', '/settings/users/changepassword') ->post() ->action('OC\Settings\ChangePassword\Controller', 'changeUserPassword'); $this->create('settings_ajax_changedisplayname', '/settings/ajax/changedisplayname.php') ->actionInclude('settings/ajax/changedisplayname.php'); // personal -$this->create('settings_ajax_changepersonalpassword', '/settings/personal/changepassword') +$this->create('settings_personal_changepassword', '/settings/personal/changepassword') ->post() ->action('OC\Settings\ChangePassword\Controller', 'changePersonalPassword'); $this->create('settings_ajax_lostpassword', '/settings/ajax/lostpassword.php') From 18a2c48ceb2206fbc871dc0c28e5fb233b4fc0fc Mon Sep 17 00:00:00 2001 From: kondou Date: Wed, 18 Sep 2013 16:47:27 +0200 Subject: [PATCH 6/6] Translate errormsgs in settings/changepassword/controller --- settings/changepassword/controller.php | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/settings/changepassword/controller.php b/settings/changepassword/controller.php index 53bd69a2cd0..1ecb644a96c 100644 --- a/settings/changepassword/controller.php +++ b/settings/changepassword/controller.php @@ -69,19 +69,27 @@ class Controller { } if ($recoveryEnabledForUser && $recoveryPassword === '') { - \OC_JSON::error(array('data' => array('message' => 'Please provide a admin recovery password, otherwise all user data will be lost'))); + $l = new \OC_L10n('settings'); + \OC_JSON::error(array('data' => array( + 'message' => $l->t('Please provide an admin recovery password, otherwise all user data will be lost') + ))); } elseif ($recoveryEnabledForUser && ! $validRecoveryPassword) { - \OC_JSON::error(array('data' => array('message' => 'Wrong admin recovery password. Please check the password and try again.'))); + $l = new \OC_L10n('settings'); + \OC_JSON::error(array('data' => array( + 'message' => $l->t('Wrong admin recovery password. Please check the password and try again.') + ))); } else { // now we know that everything is fine regarding the recovery password, let's try to change the password $result = \OC_User::setPassword($username, $password, $recoveryPassword); if (!$result && $recoveryPasswordSupported) { + $l = new \OC_L10n('settings'); \OC_JSON::error(array( "data" => array( - "message" => "Back-end doesn't support password change, but the users encryption key was successfully updated." + "message" => $l->t("Back-end doesn't support password change, but the users encryption key was successfully updated.") ) )); } elseif (!$result && !$recoveryPasswordSupported) { - \OC_JSON::error(array("data" => array( "message" => "Unable to change password" ))); + $l = new \OC_L10n('settings'); + \OC_JSON::error(array("data" => array( $l->t("message" => "Unable to change password" ) ))); } else { \OC_JSON::success(array("data" => array( "username" => $username ))); } @@ -91,7 +99,8 @@ class Controller { if (!is_null($password) && \OC_User::setPassword($username, $password)) { \OC_JSON::success(array('data' => array('username' => $username))); } else { - \OC_JSON::error(array('data' => array('message' => 'Unable to change password'))); + $l = new \OC_L10n('settings'); + \OC_JSON::error(array('data' => array('message' => $l->t('Unable to change password')))); } } }