From fe7d9a7ca07bb21905c6483dee49bf37dd131674 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 4 Dec 2014 14:15:55 +0100 Subject: [PATCH 1/5] Add REST route for user & group management First step of a somewhat testable user management. - I know, the JSON returns are in an ugly format but the JS expects it that way. So let's keep it that way until we have time to fix the JS in the future. --- lib/private/group.php | 22 +- lib/private/group/group.php | 5 + lib/private/group/metadata.php | 7 +- lib/private/server.php | 22 +- lib/private/user.php | 25 +- lib/private/user/manager.php | 2 +- lib/private/user/user.php | 18 + settings/ajax/creategroup.php | 21 -- settings/ajax/createuser.php | 59 ---- settings/ajax/grouplist.php | 46 --- settings/ajax/removegroup.php | 14 - settings/ajax/removeuser.php | 26 -- settings/ajax/userlist.php | 92 ------ settings/application.php | 71 +++- settings/controller/groupscontroller.php | 140 ++++++++ settings/controller/userscontroller.php | 251 ++++++++++++++ settings/js/settings.js | 2 +- settings/js/users/deleteHandler.js | 5 +- settings/js/users/groups.js | 6 +- settings/js/users/users.js | 6 +- settings/middleware/subadminmiddleware.php | 65 ++++ settings/routes.php | 39 +-- .../controller/groupscontrollertest.php | 217 ++++++++++++ .../controller/userscontrollertest.php | 310 ++++++++++++++++++ .../middleware/subadminmiddlewaretest.php | 91 +++++ 25 files changed, 1222 insertions(+), 340 deletions(-) delete mode 100644 settings/ajax/creategroup.php delete mode 100644 settings/ajax/createuser.php delete mode 100644 settings/ajax/grouplist.php delete mode 100644 settings/ajax/removegroup.php delete mode 100644 settings/ajax/removeuser.php delete mode 100644 settings/ajax/userlist.php create mode 100644 settings/controller/groupscontroller.php create mode 100644 settings/controller/userscontroller.php create mode 100644 settings/middleware/subadminmiddleware.php create mode 100644 tests/settings/controller/groupscontrollertest.php create mode 100644 tests/settings/controller/userscontrollertest.php create mode 100644 tests/settings/middleware/subadminmiddlewaretest.php diff --git a/lib/private/group.php b/lib/private/group.php index 49f683c411..d6e6e17f88 100644 --- a/lib/private/group.php +++ b/lib/private/group.php @@ -37,6 +37,7 @@ class OC_Group { /** * @return \OC\Group\Manager + * @deprecated Use \OC::$server->getGroupManager(); */ public static function getManager() { return \OC::$server->getGroupManager(); @@ -44,6 +45,7 @@ class OC_Group { /** * @return \OC\User\Manager + * @deprecated Use \OC::$server->getUserManager() */ private static function getUserManager() { return \OC::$server->getUserManager(); @@ -73,12 +75,10 @@ class OC_Group { * * Tries to create a new group. If the group name already exists, false will * be returned. Basic checking of Group name + * @deprecated Use \OC::$server->getGroupManager()->createGroup() instead */ public static function createGroup($gid) { - OC_Hook::emit("OC_Group", "pre_createGroup", array("run" => true, "gid" => $gid)); - if (self::getManager()->createGroup($gid)) { - OC_Hook::emit("OC_User", "post_createGroup", array("gid" => $gid)); return true; } else { return false; @@ -91,19 +91,12 @@ class OC_Group { * @return bool * * Deletes a group and removes it from the group_user-table + * @deprecated Use \OC::$server->getGroupManager()->delete() instead */ public static function deleteGroup($gid) { - // Prevent users from deleting group admin - if ($gid == "admin") { - return false; - } - - OC_Hook::emit("OC_Group", "pre_deleteGroup", array("run" => true, "gid" => $gid)); - $group = self::getManager()->get($gid); if ($group) { if ($group->delete()) { - OC_Hook::emit("OC_User", "post_deleteGroup", array("gid" => $gid)); return true; } } @@ -117,6 +110,7 @@ class OC_Group { * @return bool * * Checks whether the user is member of a group or not. + * @deprecated Use \OC::$server->getGroupManager->inGroup($user); */ public static function inGroup($uid, $gid) { $group = self::getManager()->get($gid); @@ -134,14 +128,13 @@ class OC_Group { * @return bool * * Adds a user to a group. + * @deprecated Use \OC::$server->getGroupManager->addUser(); */ public static function addToGroup($uid, $gid) { $group = self::getManager()->get($gid); $user = self::getUserManager()->get($uid); if ($group and $user) { - OC_Hook::emit("OC_Group", "pre_addToGroup", array("run" => true, "uid" => $uid, "gid" => $gid)); $group->addUser($user); - OC_Hook::emit("OC_User", "post_addToGroup", array("uid" => $uid, "gid" => $gid)); return true; } else { return false; @@ -176,6 +169,7 @@ class OC_Group { * * This function fetches all groups a user belongs to. It does not check * if the user exists at all. + * @deprecated Use \OC::$server->getGroupManager->getuserGroupIds($user) */ public static function getUserGroups($uid) { $user = self::getUserManager()->get($uid); @@ -209,6 +203,7 @@ class OC_Group { * * @param string $gid * @return bool + * @deprecated Use \OC::$server->getGroupManager->groupExists($gid) */ public static function groupExists($gid) { return self::getManager()->groupExists($gid); @@ -260,6 +255,7 @@ class OC_Group { * @param int $limit * @param int $offset * @return array an array of display names (value) and user ids(key) + * @deprecated Use \OC::$server->getGroupManager->displayNamesInGroup($gid, $search, $limit, $offset) */ public static function displayNamesInGroup($gid, $search = '', $limit = -1, $offset = 0) { return self::getManager()->displayNamesInGroup($gid, $search, $limit, $offset); diff --git a/lib/private/group/group.php b/lib/private/group/group.php index 6111051ea0..5f439e91cd 100644 --- a/lib/private/group/group.php +++ b/lib/private/group/group.php @@ -229,6 +229,11 @@ class Group implements IGroup { * @return bool */ public function delete() { + // Prevent users from deleting group admin + if ($this->getGID() === 'admin') { + return false; + } + $result = false; if ($this->emitter) { $this->emitter->emit('\OC\Group', 'preDelete', array($this)); diff --git a/lib/private/group/metadata.php b/lib/private/group/metadata.php index 687a735347..c702c924ff 100644 --- a/lib/private/group/metadata.php +++ b/lib/private/group/metadata.php @@ -29,7 +29,7 @@ class MetaData { protected $metaData = array(); /** - * @var \OC\Group\Manager $groupManager + * @var \OCP\IGroupManager $groupManager */ protected $groupManager; @@ -41,12 +41,12 @@ class MetaData { /** * @param string $user the uid of the current user * @param bool $isAdmin whether the current users is an admin - * @param \OC\Group\Manager $groupManager + * @param \OCP\IGroupManager $groupManager */ public function __construct( $user, $isAdmin, - \OC\Group\Manager $groupManager + \OCP\IGroupManager $groupManager ) { $this->user = $user; $this->isAdmin = (bool)$isAdmin; @@ -168,6 +168,7 @@ class MetaData { if($this->isAdmin) { return $this->groupManager->search($search); } else { + // FIXME: Remove static method call $groupIds = \OC_SubAdmin::getSubAdminsGroups($this->user); /* \OC_SubAdmin::getSubAdminsGroups() returns an array of GIDs, but this diff --git a/lib/private/server.php b/lib/private/server.php index 7bd7f8ca45..a08014fa6f 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -104,8 +104,26 @@ class Server extends SimpleContainer implements IServerContainer { return new \OC\User\Manager($config); }); $this->registerService('GroupManager', function (Server $c) { - $userManager = $c->getUserManager(); - return new \OC\Group\Manager($userManager); + $groupManager = new \OC\Group\Manager($this->getUserManager()); + $groupManager->listen('\OC\Group', 'preCreate', function ($gid) { + \OC_Hook::emit('OC_Group', 'pre_createGroup', array('run' => true, 'gid' => $gid)); + }); + $groupManager->listen('\OC\Group', 'postCreate', function (\OC\Group\Group $gid) { + \OC_Hook::emit('OC_User', 'post_createGroup', array('gid' => $gid->getGID())); + }); + $groupManager->listen('\OC\Group', 'preDelete', function (\OC\Group\Group $group) { + \OC_Hook::emit('OC_Group', 'pre_deleteGroup', array('run' => true, 'gid' => $group->getGID())); + }); + $groupManager->listen('\OC\Group', 'postDelete', function (\OC\Group\Group $group) { + \OC_Hook::emit('OC_User', 'post_deleteGroup', array('gid' => $group->getGID())); + }); + $groupManager->listen('\OC\Group', 'preAddUser', function (\OC\Group\Group $group, \OC\User\User $user) { + \OC_Hook::emit('OC_Group', 'pre_addToGroup', array('run' => true, 'uid' => $user->getUID(), 'gid' => $group->getGID())); + }); + $groupManager->listen('\OC\Group', 'postAddUser', function (\OC\Group\Group $group, \OC\User\User $user) { + \OC_Hook::emit('OC_Group', 'post_addToGroup', array('uid' => $user->getUID(), 'gid' => $group->getGID())); + }); + return $groupManager; }); $this->registerService('UserSession', function (Server $c) { $manager = $c->getUserManager(); diff --git a/lib/private/user.php b/lib/private/user.php index b2a235425c..f93b76a3a6 100644 --- a/lib/private/user.php +++ b/lib/private/user.php @@ -47,6 +47,7 @@ class OC_User { /** * @return \OC\User\Manager + * @deprecated Use \OC::$server->getUserManager() */ public static function getManager() { return OC::$server->getUserManager(); @@ -179,6 +180,7 @@ class OC_User { * itself, not in its subclasses. * * Allowed characters in the username are: "a-z", "A-Z", "0-9" and "_.@-" + * @deprecated Use \OC::$server->getUserManager->createUser($uid, $password) */ public static function createUser($uid, $password) { return self::getManager()->createUser($uid, $password); @@ -190,30 +192,12 @@ class OC_User { * @return bool * * Deletes a user + * @deprecated Use \OC::$server->getUserManager->delete() */ public static function deleteUser($uid) { $user = self::getManager()->get($uid); if ($user) { - $result = $user->delete(); - - // if delete was successful we clean-up the rest - if ($result) { - - // We have to delete the user from all groups - foreach (OC_Group::getUserGroups($uid) as $i) { - OC_Group::removeFromGroup($uid, $i); - } - // Delete the user's keys in preferences - OC_Preferences::deleteUser($uid); - - // Delete user files in /data/ - OC_Helper::rmdirr(\OC_User::getHome($uid)); - - // Delete the users entry in the storage table - \OC\Files\Cache\Storage::remove('home::' . $uid); - } - - return true; + return $user->delete(); } else { return false; } @@ -525,6 +509,7 @@ class OC_User { * @return string * * returns the path to the users home directory + * @deprecated Use \OC::$server->getUserManager->getHome() */ public static function getHome($uid) { $user = self::getManager()->get($uid); diff --git a/lib/private/user/manager.php b/lib/private/user/manager.php index 0c01f957bd..2403f45aa2 100644 --- a/lib/private/user/manager.php +++ b/lib/private/user/manager.php @@ -220,7 +220,7 @@ class Manager extends PublicEmitter implements IUserManager { * @param string $uid * @param string $password * @throws \Exception - * @return bool|\OC\User\User the created user of false + * @return bool|\OC\User\User the created user or false */ public function createUser($uid, $password) { $l = \OC::$server->getL10N('lib'); diff --git a/lib/private/user/user.php b/lib/private/user/user.php index 9ad2f5f0d3..ad85337f62 100644 --- a/lib/private/user/user.php +++ b/lib/private/user/user.php @@ -153,6 +153,24 @@ class User implements IUser { $this->emitter->emit('\OC\User', 'preDelete', array($this)); } $result = $this->backend->deleteUser($this->uid); + if ($result) { + + // FIXME: Feels like an hack - suggestions? + + // We have to delete the user from all groups + foreach (\OC_Group::getUserGroups($this->uid) as $i) { + \OC_Group::removeFromGroup($this->uid, $i); + } + // Delete the user's keys in preferences + \OC_Preferences::deleteUser($this->uid); + + // Delete user files in /data/ + \OC_Helper::rmdirr(\OC_User::getHome($this->uid)); + + // Delete the users entry in the storage table + \OC\Files\Cache\Storage::remove('home::' . $this->uid); + } + if ($this->emitter) { $this->emitter->emit('\OC\User', 'postDelete', array($this)); } diff --git a/settings/ajax/creategroup.php b/settings/ajax/creategroup.php deleted file mode 100644 index be376bea9d..0000000000 --- a/settings/ajax/creategroup.php +++ /dev/null @@ -1,21 +0,0 @@ -getL10N('settings'); - -// Does the group exist? -if( in_array( $groupname, OC_Group::getGroups())) { - OC_JSON::error(array("data" => array( "message" => $l->t("Group already exists") ))); - exit(); -} - -// Return Success story -if( OC_Group::createGroup( $groupname )) { - OC_JSON::success(array("data" => array( "groupname" => $groupname ))); -} -else{ - OC_JSON::error(array("data" => array( "message" => $l->t("Unable to add group") ))); -} diff --git a/settings/ajax/createuser.php b/settings/ajax/createuser.php deleted file mode 100644 index 463c15d59e..0000000000 --- a/settings/ajax/createuser.php +++ /dev/null @@ -1,59 +0,0 @@ - array( 'message' => 'User creation failed for '.$username ))); - exit(); - } - foreach( $groups as $i ) { - if(!OC_Group::groupExists($i)) { - OC_Group::createGroup($i); - } - OC_Group::addToGroup( $username, $i ); - } - - $userManager = \OC_User::getManager(); - $user = $userManager->get($username); - OCP\JSON::success(array("data" => - array( - // returns whether the home already existed - "homeExists" => $homeExists, - "username" => $username, - "groups" => OC_Group::getUserGroups( $username ), - 'storageLocation' => $user->getHome()))); -} catch (Exception $exception) { - OCP\JSON::error(array("data" => array( "message" => $exception->getMessage()))); -} diff --git a/settings/ajax/grouplist.php b/settings/ajax/grouplist.php deleted file mode 100644 index 93bb510773..0000000000 --- a/settings/ajax/grouplist.php +++ /dev/null @@ -1,46 +0,0 @@ - - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE - * License as published by the Free Software Foundation; either - * version 3 of the License, or any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU AFFERO GENERAL PUBLIC LICENSE for more details. - * - * You should have received a copy of the GNU Affero General Public - * License along with this library. If not, see . - * - */ - -OC_JSON::callCheck(); -OC_JSON::checkSubAdminUser(); -if (isset($_GET['pattern']) && !empty($_GET['pattern'])) { - $pattern = $_GET['pattern']; -} else { - $pattern = ''; -} -if (isset($_GET['filterGroups']) && !empty($_GET['filterGroups'])) { - $filterGroups = intval($_GET['filterGroups']) === 1; -} else { - $filterGroups = false; -} -$groupPattern = $filterGroups ? $pattern : ''; -$groups = array(); -$adminGroups = array(); -$groupManager = \OC_Group::getManager(); -$isAdmin = OC_User::isAdminUser(OC_User::getUser()); - -$groupsInfo = new \OC\Group\MetaData(OC_User::getUser(), $isAdmin, $groupManager); -$groupsInfo->setSorting($groupsInfo::SORT_USERCOUNT); -list($adminGroups, $groups) = $groupsInfo->get($groupPattern, $pattern); - -OC_JSON::success( - array('data' => array('adminGroups' => $adminGroups, 'groups' => $groups))); diff --git a/settings/ajax/removegroup.php b/settings/ajax/removegroup.php deleted file mode 100644 index 798d7916e6..0000000000 --- a/settings/ajax/removegroup.php +++ /dev/null @@ -1,14 +0,0 @@ - array( "groupname" => $name ))); -} -else{ - OC_JSON::error(array("data" => array( "message" => $l->t("Unable to delete group") ))); -} diff --git a/settings/ajax/removeuser.php b/settings/ajax/removeuser.php deleted file mode 100644 index eda8523878..0000000000 --- a/settings/ajax/removeuser.php +++ /dev/null @@ -1,26 +0,0 @@ -getL10N('core'); - OC_JSON::error(array( 'data' => array( 'message' => $l->t('Authentication error') ))); - exit(); -} - -// Return Success story -if( OC_User::deleteUser( $username )) { - OC_JSON::success(array("data" => array( "username" => $username ))); -} -else{ - $l = \OC::$server->getL10N('core'); - OC_JSON::error(array("data" => array( "message" => $l->t("Unable to delete user") ))); -} diff --git a/settings/ajax/userlist.php b/settings/ajax/userlist.php deleted file mode 100644 index 807cf5f189..0000000000 --- a/settings/ajax/userlist.php +++ /dev/null @@ -1,92 +0,0 @@ -. - * - */ - -OC_JSON::callCheck(); -OC_JSON::checkSubAdminUser(); -if (isset($_GET['offset'])) { - $offset = $_GET['offset']; -} else { - $offset = 0; -} -if (isset($_GET['limit'])) { - $limit = $_GET['limit']; -} else { - $limit = 10; -} -if (isset($_GET['gid']) && !empty($_GET['gid'])) { - $gid = $_GET['gid']; - if ($gid === '_everyone') { - $gid = false; - } -} else { - $gid = false; -} -if (isset($_GET['pattern']) && !empty($_GET['pattern'])) { - $pattern = $_GET['pattern']; -} else { - $pattern = ''; -} -$users = array(); -$userManager = \OC_User::getManager(); -if (OC_User::isAdminUser(OC_User::getUser())) { - if($gid !== false) { - $batch = OC_Group::displayNamesInGroup($gid, $pattern, $limit, $offset); - } else { - $batch = OC_User::getDisplayNames($pattern, $limit, $offset); - } - foreach ($batch as $uid => $displayname) { - $user = $userManager->get($uid); - $users[] = array( - 'name' => $uid, - 'displayname' => $displayname, - 'groups' => OC_Group::getUserGroups($uid), - 'subadmin' => OC_SubAdmin::getSubAdminsGroups($uid), - 'quota' => OC_Preferences::getValue($uid, 'files', 'quota', 'default'), - 'storageLocation' => $user->getHome(), - 'lastLogin' => $user->getLastLogin(), - ); - } -} else { - $groups = OC_SubAdmin::getSubAdminsGroups(OC_User::getUser()); - if($gid !== false && in_array($gid, $groups)) { - $groups = array($gid); - } elseif($gid !== false) { - //don't you try to investigate loops you must not know about - $groups = array(); - } - $batch = OC_Group::usersInGroups($groups, $pattern, $limit, $offset); - foreach ($batch as $uid) { - $user = $userManager->get($uid); - - // Only add the groups, this user is a subadmin of - $userGroups = array_intersect(OC_Group::getUserGroups($uid), OC_SubAdmin::getSubAdminsGroups(OC_User::getUser())); - $users[] = array( - 'name' => $uid, - 'displayname' => $user->getDisplayName(), - 'groups' => $userGroups, - 'quota' => OC_Preferences::getValue($uid, 'files', 'quota', 'default'), - 'storageLocation' => $user->getHome(), - 'lastLogin' => $user->getLastLogin(), - ); - } -} -OC_JSON::success(array('data' => $users)); diff --git a/settings/application.php b/settings/application.php index 64aa467122..0a80bd8b1e 100644 --- a/settings/application.php +++ b/settings/application.php @@ -10,11 +10,14 @@ namespace OC\Settings; -use OC\AppFramework\Utility\SimpleContainer; use OC\Settings\Controller\AppSettingsController; +use OC\Settings\Controller\GroupsController; use OC\Settings\Controller\MailSettingsController; use OC\Settings\Controller\SecuritySettingsController; +use OC\Settings\Controller\UsersController; +use OC\Settings\Middleware\SubadminMiddleware; use \OCP\AppFramework\App; +use OCP\IContainer; use \OCP\Util; /** @@ -34,7 +37,7 @@ class Application extends App { /** * Controllers */ - $container->registerService('MailSettingsController', function(SimpleContainer $c) { + $container->registerService('MailSettingsController', function(IContainer $c) { return new MailSettingsController( $c->query('AppName'), $c->query('Request'), @@ -46,7 +49,7 @@ class Application extends App { $c->query('DefaultMailAddress') ); }); - $container->registerService('AppSettingsController', function(SimpleContainer $c) { + $container->registerService('AppSettingsController', function(IContainer $c) { return new AppSettingsController( $c->query('AppName'), $c->query('Request'), @@ -54,33 +57,81 @@ class Application extends App { $c->query('Config') ); }); - $container->registerService('SecuritySettingsController', function(SimpleContainer $c) { + $container->registerService('SecuritySettingsController', function(IContainer $c) { return new SecuritySettingsController( $c->query('AppName'), $c->query('Request'), $c->query('Config') ); }); + $container->registerService('GroupsController', function(IContainer $c) { + return new GroupsController( + $c->query('AppName'), + $c->query('Request'), + $c->query('GroupManager'), + $c->query('UserSession'), + $c->query('IsAdmin'), + $c->query('L10N') + ); + }); + $container->registerService('UsersController', function(IContainer $c) { + return new UsersController( + $c->query('AppName'), + $c->query('Request'), + $c->query('UserManager'), + $c->query('GroupManager'), + $c->query('UserSession'), + $c->query('Config'), + $c->query('IsAdmin'), + $c->query('L10N') + ); + }); + + /** + * Middleware + */ + $container->registerService('SubadminMiddleware', function(IContainer $c){ + return new SubadminMiddleware( + $c->query('ControllerMethodReflector'), + $c->query('IsSubAdmin') + ); + }); + // Execute middlewares + $container->registerMiddleware('SubadminMiddleware'); /** * Core class wrappers */ - $container->registerService('Config', function(SimpleContainer $c) { + $container->registerService('Config', function(IContainer $c) { return $c->query('ServerContainer')->getConfig(); }); - $container->registerService('L10N', function(SimpleContainer $c) { + $container->registerService('L10N', function(IContainer $c) { return $c->query('ServerContainer')->getL10N('settings'); }); - $container->registerService('UserSession', function(SimpleContainer $c) { + $container->registerService('GroupManager', function(IContainer $c) { + return $c->query('ServerContainer')->getGroupManager(); + }); + $container->registerService('UserManager', function(IContainer $c) { + return $c->query('ServerContainer')->getUserManager(); + }); + $container->registerService('UserSession', function(IContainer $c) { return $c->query('ServerContainer')->getUserSession(); }); - $container->registerService('Mail', function(SimpleContainer $c) { + /** FIXME: Remove once OC_User is non-static and mockable */ + $container->registerService('IsAdmin', function(IContainer $c) { + return \OC_User::isAdminUser(\OC_User::getUser()); + }); + /** FIXME: Remove once OC_SubAdmin is non-static and mockable */ + $container->registerService('IsSubAdmin', function(IContainer $c) { + return \OC_Subadmin::isSubAdmin(\OC_User::getUser()); + }); + $container->registerService('Mail', function(IContainer $c) { return new \OC_Mail; }); - $container->registerService('Defaults', function(SimpleContainer $c) { + $container->registerService('Defaults', function(IContainer $c) { return new \OC_Defaults; }); - $container->registerService('DefaultMailAddress', function(SimpleContainer $c) { + $container->registerService('DefaultMailAddress', function(IContainer $c) { return Util::getDefaultEmailAddress('no-reply'); }); } diff --git a/settings/controller/groupscontroller.php b/settings/controller/groupscontroller.php new file mode 100644 index 0000000000..6e6ab89460 --- /dev/null +++ b/settings/controller/groupscontroller.php @@ -0,0 +1,140 @@ +groupManager = $groupManager; + $this->userSession = $userSession; + $this->isAdmin = $isAdmin; + $this->l10n = $l10n; + } + + /** + * @NoAdminRequired + * + * @param string $pattern + * @param bool $filterGroups + * @return DataResponse + */ + public function index($pattern = '', $filterGroups = false) { + $groupPattern = $filterGroups ? $pattern : ''; + + $groupsInfo = new \OC\Group\MetaData($this->userSession->getUser()->getUID(), + $this->isAdmin, $this->groupManager); + $groupsInfo->setSorting($groupsInfo::SORT_USERCOUNT); + list($adminGroups, $groups) = $groupsInfo->get($groupPattern, $pattern); + + return new DataResponse( + array( + 'data' => array('adminGroups' => $adminGroups, 'groups' => $groups) + ) + ); + } + + /** + * @param string $id + * @return DataResponse + */ + public function create($id) { + if($this->groupManager->groupExists($id)) { + return new DataResponse( + array( + 'status' => 'error', + 'data' => array( + 'message' => (string)$this->l10n->t('Group already exists.') + ) + ) + ); + } + if($this->groupManager->createGroup($id)) { + return new DataResponse( + array( + 'status' => 'success', + 'data' => array( + 'groupname' => $id + ) + ) + ); + } + + return new DataResponse( + array( + 'status' => 'error', + 'data' => array( + 'message' => (string)$this->l10n->t('Unable to add group.') + ) + ) + ); + } + + /** + * @param string $id + * @return DataResponse + */ + public function destroy($id) { + $group = $this->groupManager->get($id); + if ($group) { + if ($group->delete()) { + return new DataResponse( + array( + 'status' => 'success', + 'data' => array( + 'groupname' => $id + ) + ) + ); + } + } + return new DataResponse( + array( + 'status' => 'error', + 'data' => array( + 'message' => (string)$this->l10n->t('Unable to delete group.') + ) + ) + ); + } + +} diff --git a/settings/controller/userscontroller.php b/settings/controller/userscontroller.php new file mode 100644 index 0000000000..d61d19f8cb --- /dev/null +++ b/settings/controller/userscontroller.php @@ -0,0 +1,251 @@ +userManager = $userManager; + $this->groupManager = $groupManager; + $this->userSession = $userSession; + $this->config = $config; + $this->isAdmin = $isAdmin; + $this->l10n = $l10n; + } + + /** + * @NoAdminRequired + * @NoCSRFRequired + * @param int $offset + * @param int $limit + * @param string $gid + * @param string $pattern + * @return DataResponse + * + * TODO: Tidy up and write unit tests - code is mainly static method calls + */ + public function index($offset = 0, $limit = 10, $gid = '', $pattern = '') { + // FIXME: The JS sends the group '_everyone' instead of no GID for the "all users" group. + if($gid === '_everyone') { + $gid = ''; + } + $users = array(); + if ($this->isAdmin) { + if($gid !== '') { + $batch = $this->groupManager->displayNamesInGroup($gid, $pattern, $limit, $offset); + } else { + // FIXME: Remove static method call + $batch = \OC_User::getDisplayNames($pattern, $limit, $offset); + } + + foreach ($batch as $uid => $displayname) { + $user = $this->userManager->get($uid); + $users[] = array( + 'name' => $uid, + 'displayname' => $displayname, + 'groups' => $this->groupManager->getUserGroupIds($user), + 'subadmin' => \OC_SubAdmin::getSubAdminsGroups($uid), + 'quota' => $this->config->getUserValue($uid, 'files', 'quota', 'default'), + 'storageLocation' => $user->getHome(), + 'lastLogin' => $user->getLastLogin(), + ); + } + } else { + $groups = \OC_SubAdmin::getSubAdminsGroups($this->userSession->getUser()->getUID()); + if($gid !== '' && in_array($gid, $groups)) { + $groups = array($gid); + } elseif($gid !== '') { + //don't you try to investigate loops you must not know about + $groups = array(); + } + $batch = \OC_Group::usersInGroups($groups, $pattern, $limit, $offset); + foreach ($batch as $uid) { + $user = $this->userManager->get($uid); + + // Only add the groups, this user is a subadmin of + $userGroups = array_intersect($this->groupManager->getUserGroupIds($user), \OC_SubAdmin::getSubAdminsGroups($this->userSession->getUser()->getUID())); + $users[] = array( + 'name' => $uid, + 'displayname' => $user->getDisplayName(), + 'groups' => $userGroups, + 'quota' => $this->config->getUserValue($uid, 'files', 'quota', 'default'), + 'storageLocation' => $user->getHome(), + 'lastLogin' => $user->getLastLogin(), + ); + } + } + + // FIXME: That assignment on "data" is uneeded here - JS should be adjusted + return new DataResponse(array('data' => $users, 'status' => 'success')); + } + + /** + * @NoAdminRequired + * + * @param string $username + * @param string $password + * @param array $groups + * @return DataResponse + * + * TODO: Tidy up and write unit tests - code is mainly static method calls + */ + public function create($username, $password, array $groups) { + + if (!$this->isAdmin) { + if (!empty($groups)) { + foreach ($groups as $key => $group) { + if (!\OC_SubAdmin::isGroupAccessible($this->userSession->getUser()->getUID(), $group)) { + unset($groups[$key]); + } + } + } + if (empty($groups)) { + $groups = \OC_SubAdmin::getSubAdminsGroups($this->userSession->getUser()->getUID()); + } + } + + try { + $user = $this->userManager->createUser($username, $password); + } catch (\Exception $exception) { + return new DataResponse( + array( + 'status' => 'error', + 'data' => array( + 'message' => (string)$this->l10n->t('Unable to create user.') + ) + ) + ); + } + + if($user instanceof User) { + foreach( $groups as $groupName ) { + $group = $this->groupManager->get($groupName); + + if(empty($group)) { + $group = $this->groupManager->createGroup($groupName); + } + $group->addUser($user); + } + } + + return new DataResponse( + array( + 'status' => 'success', + 'data' => array( + 'username' => $username, + 'groups' => $this->groupManager->getUserGroupIds($user), + 'storageLocation' => $user->getHome() + ) + ) + ); + + } + + /** + * @NoAdminRequired + * + * @param string $id + * @return DataResponse + * + * TODO: Tidy up and write unit tests - code is mainly static method calls + */ + public function destroy($id) { + if($this->userSession->getUser()->getUID() === $id) { + return new DataResponse( + array( + 'status' => 'error', + 'data' => array( + 'message' => (string)$this->l10n->t('Unable to delete user.') + ) + ) + ); + } + + // FIXME: Remove this static function call at some point… + if(!$this->isAdmin && !\OC_SubAdmin::isUserAccessible($this->userSession->getUser()->getUID(), $id)) { + return new DataResponse( + array( + 'status' => 'error', + 'data' => array( + 'message' => (string)$this->l10n->t('Authentication error')) + ) + ); + } + + $user = $this->userManager->get($id); + if($user) { + if($user->delete()) { + return new DataResponse( + array( + 'status' => 'success', + 'data' => array( + 'username' => $id + ) + ) + ); + } + } + + return new DataResponse( + array( + 'status' => 'error', + 'data' => array( + 'message' => (string)$this->l10n->t('Unable to delete user.') + ) + ) + ); + + } + +} diff --git a/settings/js/settings.js b/settings/js/settings.js index 13c56a8f53..e98bd2cc89 100644 --- a/settings/js/settings.js +++ b/settings/js/settings.js @@ -41,7 +41,7 @@ OC.Settings = _.extend(OC.Settings, { }; } $.ajax({ - url: OC.generateUrl('/settings/ajax/grouplist'), + url: OC.generateUrl('/settings/users/groups'), data: queryData, dataType: 'json', success: function(data) { diff --git a/settings/js/users/deleteHandler.js b/settings/js/users/deleteHandler.js index c89a844044..942bae91cd 100644 --- a/settings/js/users/deleteHandler.js +++ b/settings/js/users/deleteHandler.js @@ -189,11 +189,10 @@ DeleteHandler.prototype.deleteEntry = function(keepNotification) { var payload = {}; payload[dh.ajaxParamID] = dh.oidToDelete; $.ajax({ - type: 'POST', - url: OC.filePath('settings', 'ajax', dh.ajaxEndpoint), + type: 'DELETE', + url: OC.generateUrl(dh.ajaxEndpoint+'/'+this.oidToDelete), // FIXME: do not use synchronous ajax calls as they block the browser ! async: false, - data: payload, success: function (result) { if (result.status === 'success') { // Remove undo option, & remove user from table diff --git a/settings/js/users/groups.js b/settings/js/users/groups.js index 081842734f..284d5598ed 100644 --- a/settings/js/users/groups.js +++ b/settings/js/users/groups.js @@ -84,9 +84,9 @@ GroupList = { createGroup: function (groupname) { $.post( - OC.filePath('settings', 'ajax', 'creategroup.php'), + OC.generateUrl('/settings/users/groups'), { - groupname: groupname + id: groupname }, function (result) { if (result.status !== 'success') { @@ -221,7 +221,7 @@ GroupList = { }, initDeleteHandling: function () { //set up handler - GroupDeleteHandler = new DeleteHandler('removegroup.php', 'groupname', + GroupDeleteHandler = new DeleteHandler('/settings/users/groups', 'groupname', GroupList.hide, GroupList.remove); //configure undo diff --git a/settings/js/users/users.js b/settings/js/users/users.js index 5e0c0cac18..6de8b7029e 100644 --- a/settings/js/users/users.js +++ b/settings/js/users/users.js @@ -292,7 +292,7 @@ var UserList = { }, initDeleteHandling: function() { //set up handler - UserDeleteHandler = new DeleteHandler('removeuser.php', 'username', + UserDeleteHandler = new DeleteHandler('/settings/users/users', 'username', UserList.markRemove, UserList.remove); //configure undo @@ -326,7 +326,7 @@ var UserList = { UserList.currentGid = gid; var pattern = filter.getPattern(); $.get( - OC.generateUrl('/settings/ajax/userlist'), + OC.generateUrl('/settings/users/users'), { offset: UserList.offset, limit: UserList.usersToLoad, gid: gid, pattern: pattern }, function (result) { var loadedUsers = 0; @@ -667,7 +667,7 @@ $(document).ready(function () { var groups = $('#newusergroups').val(); $('#newuser').get(0).reset(); $.post( - OC.filePath('settings', 'ajax', 'createuser.php'), + OC.generateUrl('/settings/users/users'), { username: username, password: password, diff --git a/settings/middleware/subadminmiddleware.php b/settings/middleware/subadminmiddleware.php new file mode 100644 index 0000000000..a5c005e314 --- /dev/null +++ b/settings/middleware/subadminmiddleware.php @@ -0,0 +1,65 @@ +reflector = $reflector; + $this->isSubAdmin = $isSubAdmin; + } + + /** + * Check if sharing is enabled before the controllers is executed + * @param \OCP\AppFramework\Controller $controller + * @param string $methodName + * @throws \Exception + */ + public function beforeController($controller, $methodName) { + if(!$this->reflector->hasAnnotation('NoSubadminRequired')) { + if(!$this->isSubAdmin) { + throw new \Exception('Logged in user must be a subadmin'); + } + } + } + + /** + * Return 403 page in case of an exception + * @param \OCP\AppFramework\Controller $controller + * @param string $methodName + * @param \Exception $exception + * @return TemplateResponse + */ + public function afterException($controller, $methodName, \Exception $exception) { + return new TemplateResponse('core', '403', array(), 'guest'); + } + +} diff --git a/settings/routes.php b/settings/routes.php index 7ca33fc274..1b7a918fa7 100644 --- a/settings/routes.php +++ b/settings/routes.php @@ -9,17 +9,22 @@ namespace OC\Settings; $application = new Application(); -$application->registerRoutes($this, array('routes' =>array( - array('name' => 'MailSettings#setMailSettings', 'url' => '/settings/admin/mailsettings', 'verb' => 'POST'), - array('name' => 'MailSettings#storeCredentials', 'url' => '/settings/admin/mailsettings/credentials', 'verb' => 'POST'), - array('name' => 'MailSettings#sendTestMail', 'url' => '/settings/admin/mailtest', 'verb' => 'POST'), - array('name' => 'AppSettings#listCategories', 'url' => '/settings/apps/categories', 'verb' => 'GET'), - array('name' => 'AppSettings#listApps', 'url' => '/settings/apps/list', 'verb' => 'GET'), - array('name' => 'SecuritySettings#enforceSSL', 'url' => '/settings/admin/security/ssl', 'verb' => 'POST'), - array('name' => 'SecuritySettings#enforceSSLForSubdomains', 'url' => '/settings/admin/security/ssl/subdomains', 'verb' => 'POST'), - array('name' => 'SecuritySettings#trustedDomains', 'url' => '/settings/admin/security/trustedDomains', 'verb' => 'POST'), - -))); +$application->registerRoutes($this, array( + 'resources' => array( + 'groups' => array('url' => '/settings/users/groups'), + 'users' => array('url' => '/settings/users/users') + ), + 'routes' =>array( + array('name' => 'MailSettings#setMailSettings', 'url' => '/settings/admin/mailsettings', 'verb' => 'POST'), + array('name' => 'MailSettings#storeCredentials', 'url' => '/settings/admin/mailsettings/credentials', 'verb' => 'POST'), + array('name' => 'MailSettings#sendTestMail', 'url' => '/settings/admin/mailtest', 'verb' => 'POST'), + array('name' => 'AppSettings#listCategories', 'url' => '/settings/apps/categories', 'verb' => 'GET'), + array('name' => 'AppSettings#listApps', 'url' => '/settings/apps/list', 'verb' => 'GET'), + array('name' => 'SecuritySettings#enforceSSL', 'url' => '/settings/admin/security/ssl', 'verb' => 'POST'), + array('name' => 'SecuritySettings#enforceSSLForSubdomains', 'url' => '/settings/admin/security/ssl/subdomains', 'verb' => 'POST'), + array('name' => 'SecuritySettings#trustedDomains', 'url' => '/settings/admin/security/trustedDomains', 'verb' => 'POST'), + ) +)); /** @var $this \OCP\Route\IRouter */ @@ -38,26 +43,14 @@ $this->create('settings_admin', '/settings/admin') ->actionInclude('settings/admin.php'); // Settings ajax actions // users -$this->create('settings_ajax_userlist', '/settings/ajax/userlist') - ->actionInclude('settings/ajax/userlist.php'); -$this->create('settings_ajax_grouplist', '/settings/ajax/grouplist') - ->actionInclude('settings/ajax/grouplist.php'); $this->create('settings_ajax_everyonecount', '/settings/ajax/geteveryonecount') ->actionInclude('settings/ajax/geteveryonecount.php'); -$this->create('settings_ajax_createuser', '/settings/ajax/createuser.php') - ->actionInclude('settings/ajax/createuser.php'); -$this->create('settings_ajax_removeuser', '/settings/ajax/removeuser.php') - ->actionInclude('settings/ajax/removeuser.php'); $this->create('settings_ajax_setquota', '/settings/ajax/setquota.php') ->actionInclude('settings/ajax/setquota.php'); -$this->create('settings_ajax_creategroup', '/settings/ajax/creategroup.php') - ->actionInclude('settings/ajax/creategroup.php'); $this->create('settings_ajax_togglegroups', '/settings/ajax/togglegroups.php') ->actionInclude('settings/ajax/togglegroups.php'); $this->create('settings_ajax_togglesubadmins', '/settings/ajax/togglesubadmins.php') ->actionInclude('settings/ajax/togglesubadmins.php'); -$this->create('settings_ajax_removegroup', '/settings/ajax/removegroup.php') - ->actionInclude('settings/ajax/removegroup.php'); $this->create('settings_users_changepassword', '/settings/users/changepassword') ->post() ->action('OC\Settings\ChangePassword\Controller', 'changeUserPassword'); diff --git a/tests/settings/controller/groupscontrollertest.php b/tests/settings/controller/groupscontrollertest.php new file mode 100644 index 0000000000..ac4619cfdb --- /dev/null +++ b/tests/settings/controller/groupscontrollertest.php @@ -0,0 +1,217 @@ +container = $app->getContainer(); + $this->container['AppName'] = 'settings'; + $this->container['GroupManager'] = $this->getMockBuilder('\OCP\IGroupManager') + ->disableOriginalConstructor()->getMock(); + $this->container['UserSession'] = $this->getMockBuilder('\OC\User\Session') + ->disableOriginalConstructor()->getMock(); + $this->container['L10N'] = $this->getMockBuilder('\OCP\IL10N') + ->disableOriginalConstructor()->getMock(); + $this->container['IsAdmin'] = true; + $this->container['L10N'] + ->expects($this->any()) + ->method('t') + ->will($this->returnCallback(function($text, $parameters = array()) { + return vsprintf($text, $parameters); + })); + $this->groupsController = $this->container['GroupsController']; + + } + + /** + * TODO: Since GroupManager uses the static OC_Subadmin class it can't be mocked + * to test for subadmins. Thus the test always assumes you have admin permissions... + */ + public function testIndex() { + $firstGroup = $this->getMockBuilder('\OC\Group\Group') + ->disableOriginalConstructor()->getMock(); + $firstGroup + ->method('getGID') + ->will($this->returnValue('firstGroup')); + $firstGroup + ->method('count') + ->will($this->returnValue(12)); + $secondGroup = $this->getMockBuilder('\OC\Group\Group') + ->disableOriginalConstructor()->getMock(); + $secondGroup + ->method('getGID') + ->will($this->returnValue('secondGroup')); + $secondGroup + ->method('count') + ->will($this->returnValue(25)); + $thirdGroup = $this->getMockBuilder('\OC\Group\Group') + ->disableOriginalConstructor()->getMock(); + $thirdGroup + ->method('getGID') + ->will($this->returnValue('thirdGroup')); + $thirdGroup + ->method('count') + ->will($this->returnValue(14)); + $fourthGroup = $this->getMockBuilder('\OC\Group\Group') + ->disableOriginalConstructor()->getMock(); + $fourthGroup + ->method('getGID') + ->will($this->returnValue('admin')); + $fourthGroup + ->method('count') + ->will($this->returnValue(18)); + /** @var \OC\Group\Group[] $groups */ + $groups = array(); + $groups[] = $firstGroup; + $groups[] = $secondGroup; + $groups[] = $thirdGroup; + $groups[] = $fourthGroup; + + $user = $this->getMockBuilder('\OC\User\User') + ->disableOriginalConstructor()->getMock(); + $this->container['UserSession'] + ->expects($this->once()) + ->method('getUser') + ->will($this->returnValue($user)); + $user + ->expects($this->once()) + ->method('getUID') + ->will($this->returnValue('MyAdminUser')); + $this->container['GroupManager'] + ->method('search') + ->will($this->returnValue($groups)); + + $expectedResponse = new DataResponse( + array( + 'data' => array( + 'adminGroups' => array( + 0 => array( + 'id' => 'admin', + 'name' => 'admin', + 'usercount' => 18 + ) + ), + 'groups' => + array( + 0 => array( + 'id' => 'secondGroup', + 'name' => 'secondGroup', + 'usercount' => 25 + ), + 1 => array( + 'id' => 'thirdGroup', + 'name' => 'thirdGroup', + 'usercount' => 14 + ), + 2 => array( + 'id' => 'firstGroup', + 'name' => 'firstGroup', + 'usercount' => 12 + ) + ) + ) + ) + ); + $response = $this->groupsController->index(); + $this->assertEquals($expectedResponse, $response); + } + + public function testCreateWithExistingGroup() { + $this->container['GroupManager'] + ->expects($this->once()) + ->method('groupExists') + ->with('ExistingGroup') + ->will($this->returnValue(true)); + + $expectedResponse = new DataResponse(array('status' => 'error', 'data' => array('message' => 'Group already exists.'))); + $response = $this->groupsController->create('ExistingGroup'); + $this->assertEquals($expectedResponse, $response); + } + + public function testCreateSuccessful() { + $this->container['GroupManager'] + ->expects($this->once()) + ->method('groupExists') + ->with('NewGroup') + ->will($this->returnValue(false)); + $this->container['GroupManager'] + ->expects($this->once()) + ->method('createGroup') + ->with('NewGroup') + ->will($this->returnValue(true)); + + $expectedResponse = new DataResponse(array('status' => 'success', 'data' => array('groupname' => 'NewGroup'))); + $response = $this->groupsController->create('NewGroup'); + $this->assertEquals($expectedResponse, $response); + } + + public function testCreateUnsuccessful() { + $this->container['GroupManager'] + ->expects($this->once()) + ->method('groupExists') + ->with('NewGroup') + ->will($this->returnValue(false)); + $this->container['GroupManager'] + ->expects($this->once()) + ->method('createGroup') + ->with('NewGroup') + ->will($this->returnValue(false)); + + $expectedResponse = new DataResponse(array('status' => 'error', 'data' => array('message' => 'Unable to add group.'))); + $response = $this->groupsController->create('NewGroup'); + $this->assertEquals($expectedResponse, $response); + } + + public function testDestroySuccessful() { + $group = $this->getMockBuilder('\OC\Group\Group') + ->disableOriginalConstructor()->getMock(); + $this->container['GroupManager'] + ->expects($this->once()) + ->method('get') + ->with('ExistingGroup') + ->will($this->returnValue($group)); + $group + ->expects($this->once()) + ->method('delete') + ->will($this->returnValue(true)); + + $expectedResponse = new DataResponse(array('status' => 'success', 'data' => array('groupname' => 'ExistingGroup'))); + $response = $this->groupsController->destroy('ExistingGroup'); + $this->assertEquals($expectedResponse, $response); + } + + public function testDestroyUnsuccessful() { + $this->container['GroupManager'] + ->expects($this->once()) + ->method('get') + ->with('ExistingGroup') + ->will($this->returnValue(null)); + + $expectedResponse = new DataResponse(array('status' => 'error', 'data' => array('message' => 'Unable to delete group.'))); + $response = $this->groupsController->destroy('ExistingGroup'); + $this->assertEquals($expectedResponse, $response); + } + +} diff --git a/tests/settings/controller/userscontrollertest.php b/tests/settings/controller/userscontrollertest.php new file mode 100644 index 0000000000..8aee2b2dd5 --- /dev/null +++ b/tests/settings/controller/userscontrollertest.php @@ -0,0 +1,310 @@ +container = $app->getContainer(); + $this->container['AppName'] = 'settings'; + $this->container['GroupManager'] = $this->getMockBuilder('\OCP\IGroupManager') + ->disableOriginalConstructor()->getMock(); + $this->container['UserManager'] = $this->getMockBuilder('\OCP\IUserManager') + ->disableOriginalConstructor()->getMock(); + $this->container['UserSession'] = $this->getMockBuilder('\OC\User\Session') + ->disableOriginalConstructor()->getMock(); + $this->container['L10N'] = $this->getMockBuilder('\OCP\IL10N') + ->disableOriginalConstructor()->getMock(); + $this->container['Config'] = $this->getMockBuilder('\OCP\IConfig') + ->disableOriginalConstructor()->getMock(); + $this->container['IsAdmin'] = true; + $this->container['L10N'] + ->expects($this->any()) + ->method('t') + ->will($this->returnCallback(function($text, $parameters = array()) { + return vsprintf($text, $parameters); + })); + $this->usersController = $this->container['UsersController']; + + } + + /** + * TODO: Since the function uses the static OC_Subadmin class it can't be mocked + * to test for subadmins. Thus the test always assumes you have admin permissions... + */ + public function testIndex() { + $admin = $this->getMockBuilder('\OC\User\User') + ->disableOriginalConstructor()->getMock(); + $admin + ->method('getLastLogin') + ->will($this->returnValue(12)); + $admin + ->method('getHome') + ->will($this->returnValue('/home/admin')); + $foo = $this->getMockBuilder('\OC\User\User') + ->disableOriginalConstructor()->getMock(); + $foo + ->method('getLastLogin') + ->will($this->returnValue(500)); + $foo + ->method('getHome') + ->will($this->returnValue('/home/foo')); + $bar = $this->getMockBuilder('\OC\User\User') + ->disableOriginalConstructor()->getMock(); + $bar + ->method('getLastLogin') + ->will($this->returnValue(3999)); + $bar + ->method('getHome') + ->will($this->returnValue('/home/bar')); + + $this->container['GroupManager'] + ->expects($this->once()) + ->method('displayNamesInGroup') + ->will($this->returnValue(array('foo' => 'M. Foo', 'admin' => 'S. Admin', 'bar' => 'B. Ar'))); + $this->container['GroupManager'] + ->expects($this->exactly(3)) + ->method('getUserGroupIds') + ->will($this->onConsecutiveCalls(array('Users', 'Support'), array('admins', 'Support'), array('External Users'))); + $this->container['UserManager'] + ->expects($this->exactly(3)) + ->method('get') + ->will($this->onConsecutiveCalls($foo, $admin, $bar)); + $this->container['Config'] + ->expects($this->exactly(3)) + ->method('getUserValue') + ->will($this->onConsecutiveCalls(1024, 404, 2323)); + + $expectedResponse = new DataResponse( + array( + 'status' => 'success', + 'data' => array( + 0 => array( + 'name' => 'foo', + 'displayname' => 'M. Foo', + 'groups' => array('Users', 'Support'), + 'subadmin' => array(), + 'quota' => 1024, + 'storageLocation' => '/home/foo', + 'lastLogin' => 500 + ), + 1 => array( + 'name' => 'admin', + 'displayname' => 'S. Admin', + 'groups' => array('admins', 'Support'), + 'subadmin' => array(), + 'quota' => 404, + 'storageLocation' => '/home/admin', + 'lastLogin' => 12 + ), + 2 => array( + 'name' => 'bar', + 'displayname' => 'B. Ar', + 'groups' => array('External Users'), + 'subadmin' => array(), + 'quota' => 2323, + 'storageLocation' => '/home/bar', + 'lastLogin' => 3999 + ), + ) + ) + ); + $response = $this->usersController->index(0, 10, 'pattern'); + $this->assertEquals($expectedResponse, $response); + } + + /** + * TODO: Since the function uses the static OC_Subadmin class it can't be mocked + * to test for subadmins. Thus the test always assumes you have admin permissions... + */ + public function testCreateSuccessfulWithoutGroup() { + $user = $this->getMockBuilder('\OC\User\User') + ->disableOriginalConstructor()->getMock(); + $user + ->method('getHome') + ->will($this->returnValue('/home/user')); + + $this->container['UserManager'] + ->expects($this->once()) + ->method('createUser') + ->will($this->onConsecutiveCalls($user)); + + + $expectedResponse = new DataResponse( + array( + 'status' => 'success', + 'data' => array( + 'username' => 'foo', + 'groups' => null, + 'storageLocation' => '/home/user' + ) + ) + ); + $response = $this->usersController->create('foo', 'password', array()); + $this->assertEquals($expectedResponse, $response); + } + + /** + * TODO: Since the function uses the static OC_Subadmin class it can't be mocked + * to test for subadmins. Thus the test always assumes you have admin permissions... + */ + public function testCreateSuccessfulWithGroup() { + $user = $this->getMockBuilder('\OC\User\User') + ->disableOriginalConstructor()->getMock(); + $user + ->method('getHome') + ->will($this->returnValue('/home/user')); + $user + ->method('getHome') + ->will($this->returnValue('/home/user')); + $existingGroup = $this->getMockBuilder('\OCP\IGroup') + ->disableOriginalConstructor()->getMock(); + $existingGroup + ->expects($this->once()) + ->method('addUser') + ->with($user); + $newGroup = $this->getMockBuilder('\OCP\IGroup') + ->disableOriginalConstructor()->getMock(); + $newGroup + ->expects($this->once()) + ->method('addUser') + ->with($user); + + $this->container['UserManager'] + ->expects($this->once()) + ->method('createUser') + ->will($this->onConsecutiveCalls($user)); + $this->container['GroupManager'] + ->expects($this->exactly(2)) + ->method('get') + ->will($this->onConsecutiveCalls(null, $existingGroup)); + $this->container['GroupManager'] + ->expects($this->once()) + ->method('createGroup') + ->with('NewGroup') + ->will($this->onConsecutiveCalls($newGroup)); + $this->container['GroupManager'] + ->expects($this->once()) + ->method('getUserGroupIds') + ->with($user) + ->will($this->onConsecutiveCalls(array('NewGroup', 'ExistingGroup'))); + + $expectedResponse = new DataResponse( + array( + 'status' => 'success', + 'data' => array( + 'username' => 'foo', + 'groups' => array('NewGroup', 'ExistingGroup'), + 'storageLocation' => '/home/user' + ) + ) + ); + $response = $this->usersController->create('foo', 'password', array('NewGroup', 'ExistingGroup')); + $this->assertEquals($expectedResponse, $response); + } + + /** + * TODO: Since the function uses the static OC_Subadmin class it can't be mocked + * to test for subadmins. Thus the test always assumes you have admin permissions... + */ + public function testCreateUnsuccessful() { + $this->container['UserManager'] + ->method('createUser') + ->will($this->throwException(new \Exception())); + + $expectedResponse = new DataResponse( + array( + 'status' => 'error', + 'data' => array( + 'message' => 'Unable to create user.' + ) + ) + ); + $response = $this->usersController->create('foo', 'password', array()); + $this->assertEquals($expectedResponse, $response); + } + + /** + * TODO: Since the function uses the static OC_Subadmin class it can't be mocked + * to test for subadmins. Thus the test always assumes you have admin permissions... + */ + public function testDestroySelf() { + $user = $this->getMockBuilder('\OC\User\User') + ->disableOriginalConstructor()->getMock(); + $user + ->expects($this->once()) + ->method('getUID') + ->will($this->returnValue('myself')); + $this->container['UserSession'] + ->method('getUser') + ->will($this->returnValue($user)); + + $expectedResponse = new DataResponse( + array( + 'status' => 'error', + 'data' => array( + 'message' => 'Unable to delete user.' + ) + ) + ); + $response = $this->usersController->destroy('myself'); + $this->assertEquals($expectedResponse, $response); + } + + /** + * TODO: Since the function uses the static OC_Subadmin class it can't be mocked + * to test for subadmins. Thus the test always assumes you have admin permissions... + */ + public function testDestroy() { + $user = $this->getMockBuilder('\OC\User\User') + ->disableOriginalConstructor()->getMock(); + $user + ->expects($this->once()) + ->method('getUID') + ->will($this->returnValue('Admin')); + $toDeleteUser = $this->getMockBuilder('\OC\User\User') + ->disableOriginalConstructor()->getMock(); + $toDeleteUser + ->expects($this->once()) + ->method('delete') + ->will($this->returnValue(true)); + $this->container['UserSession'] + ->method('getUser') + ->will($this->returnValue($user)); + $this->container['UserManager'] + ->method('get') + ->with('UserToDelete') + ->will($this->returnValue($toDeleteUser)); + + $expectedResponse = new DataResponse( + array( + 'status' => 'success', + 'data' => array( + 'username' => 'UserToDelete' + ) + ) + ); + $response = $this->usersController->destroy('UserToDelete'); + $this->assertEquals($expectedResponse, $response); + } +} diff --git a/tests/settings/middleware/subadminmiddlewaretest.php b/tests/settings/middleware/subadminmiddlewaretest.php new file mode 100644 index 0000000000..e5572cfba5 --- /dev/null +++ b/tests/settings/middleware/subadminmiddlewaretest.php @@ -0,0 +1,91 @@ +reflector = $this->getMockBuilder('\OC\AppFramework\Utility\ControllerMethodReflector') + ->disableOriginalConstructor()->getMock(); + $this->controller = $this->getMockBuilder('\OCP\AppFramework\Controller') + ->disableOriginalConstructor()->getMock(); + + $this->subadminMiddlewareAsSubAdmin = new SubadminMiddleware($this->reflector, true); + $this->subadminMiddleware = new SubadminMiddleware($this->reflector, false); + } + + /** + * @expectedException \Exception + * @expectedExceptionMessage Logged in user must be a subadmin + */ + public function testBeforeControllerAsUserWithExemption() { + $this->reflector + ->expects($this->once()) + ->method('hasAnnotation') + ->with('NoSubadminRequired') + ->will($this->returnValue(false)); + $this->subadminMiddleware->beforeController($this->controller, 'foo'); + } + + + public function testBeforeControllerAsUserWithoutExemption() { + $this->reflector + ->expects($this->once()) + ->method('hasAnnotation') + ->with('NoSubadminRequired') + ->will($this->returnValue(true)); + $this->subadminMiddleware->beforeController($this->controller, 'foo'); + } + + public function testBeforeControllerAsSubAdminWithoutExemption() { + $this->reflector + ->expects($this->once()) + ->method('hasAnnotation') + ->with('NoSubadminRequired') + ->will($this->returnValue(false)); + $this->subadminMiddlewareAsSubAdmin->beforeController($this->controller, 'foo'); + } + + public function testBeforeControllerAsSubAdminWithExemption() { + $this->reflector + ->expects($this->once()) + ->method('hasAnnotation') + ->with('NoSubadminRequired') + ->will($this->returnValue(true)); + $this->subadminMiddlewareAsSubAdmin->beforeController($this->controller, 'foo'); + } + + + + + public function testAfterException() { + $expectedResponse = new TemplateResponse('core', '403', array(), 'guest'); + $this->assertEquals($expectedResponse, $this->subadminMiddleware->afterException($this->controller, 'foo', new \Exception())); + } +} \ No newline at end of file From eac0131d5ab6f5cb5f98595731d2bd8a17fe9b36 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Mon, 8 Dec 2014 15:05:03 +0100 Subject: [PATCH 2/5] fix updated URL schema in JS unit tests --- settings/tests/js/users/deleteHandlerSpec.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/settings/tests/js/users/deleteHandlerSpec.js b/settings/tests/js/users/deleteHandlerSpec.js index 6b6328be80..c6d88b3241 100644 --- a/settings/tests/js/users/deleteHandlerSpec.js +++ b/settings/tests/js/users/deleteHandlerSpec.js @@ -85,7 +85,7 @@ describe('DeleteHandler tests', function() { // previous one was delete expect(fakeServer.requests.length).toEqual(1); var request = fakeServer.requests[0]; - expect(request.url).toEqual(OC.webroot + '/index.php/settings/ajax/dummyendpoint.php'); + expect(request.url).toEqual(OC.webroot + '/index.php/dummyendpoint.php/some_uid'); }); it('automatically deletes after timeout', function() { var handler = init(markCallback, removeCallback, undoCallback); @@ -98,7 +98,7 @@ describe('DeleteHandler tests', function() { clock.tick(3000); expect(fakeServer.requests.length).toEqual(1); var request = fakeServer.requests[0]; - expect(request.url).toEqual(OC.webroot + '/index.php/settings/ajax/dummyendpoint.php'); + expect(request.url).toEqual(OC.webroot + '/index.php/dummyendpoint.php/some_uid'); }); it('deletes when deleteEntry is called', function() { var handler = init(markCallback, removeCallback, undoCallback); @@ -107,7 +107,7 @@ describe('DeleteHandler tests', function() { handler.deleteEntry(); expect(fakeServer.requests.length).toEqual(1); var request = fakeServer.requests[0]; - expect(request.url).toEqual(OC.webroot + '/index.php/settings/ajax/dummyendpoint.php'); + expect(request.url).toEqual(OC.webroot + '/index.php/dummyendpoint.php/some_uid'); }); it('cancels deletion when undo is clicked', function() { var handler = init(markCallback, removeCallback, undoCallback); @@ -135,7 +135,7 @@ describe('DeleteHandler tests', function() { expect(fakeServer.requests.length).toEqual(0); }); it('calls removeCallback after successful server side deletion', function() { - fakeServer.respondWith(/\/index\.php\/settings\/ajax\/dummyendpoint.php/, [ + fakeServer.respondWith(/\/index\.php\/dummyendpoint.php\/some_uid/, [ 200, { 'Content-Type': 'application/json' }, JSON.stringify({status: 'success'}) @@ -148,7 +148,6 @@ describe('DeleteHandler tests', function() { expect(fakeServer.requests.length).toEqual(1); var request = fakeServer.requests[0]; var query = OC.parseQueryString(request.requestBody); - expect(query.paramid).toEqual('some_uid'); expect(removeCallback.calledOnce).toEqual(true); expect(undoCallback.notCalled).toEqual(true); @@ -157,7 +156,7 @@ describe('DeleteHandler tests', function() { it('calls undoCallback and shows alert after failed server side deletion', function() { // stub t to avoid extra calls var tStub = sinon.stub(window, 't').returns('text'); - fakeServer.respondWith(/\/index\.php\/settings\/ajax\/dummyendpoint.php/, [ + fakeServer.respondWith(/\/index\.php\/dummyendpoint.php\/some_uid/, [ 200, { 'Content-Type': 'application/json' }, JSON.stringify({status: 'error', data: {message: 'test error'}}) @@ -171,7 +170,6 @@ describe('DeleteHandler tests', function() { expect(fakeServer.requests.length).toEqual(1); var request = fakeServer.requests[0]; var query = OC.parseQueryString(request.requestBody); - expect(query.paramid).toEqual('some_uid'); expect(removeCallback.notCalled).toEqual(true); expect(undoCallback.calledOnce).toEqual(true); From 3a49411051b3ad194b7296658c9177cbee82a74d Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 8 Dec 2014 15:32:53 +0100 Subject: [PATCH 3/5] Fix url --- settings/js/users/groups.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/settings/js/users/groups.js b/settings/js/users/groups.js index 284d5598ed..40872785e3 100644 --- a/settings/js/users/groups.js +++ b/settings/js/users/groups.js @@ -115,7 +115,7 @@ GroupList = { } GroupList.updating = true; $.get( - OC.generateUrl('/settings/ajax/grouplist'), + OC.generateUrl('/settings/users/groups'), { pattern: filter.getPattern(), filterGroups: filter.filterGroups ? 1 : 0 From 8b3e3890628fa00d50c94af1131cbe3cd63e4a12 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 8 Dec 2014 15:32:59 +0100 Subject: [PATCH 4/5] Add statuscodes --- settings/controller/groupscontroller.php | 18 ++++--- settings/controller/userscontroller.php | 22 +++++--- .../controller/groupscontrollertest.php | 43 +++++++++++++-- .../controller/userscontrollertest.php | 53 +++++++++++++++++-- 4 files changed, 113 insertions(+), 23 deletions(-) diff --git a/settings/controller/groupscontroller.php b/settings/controller/groupscontroller.php index 6e6ab89460..8236d5507f 100644 --- a/settings/controller/groupscontroller.php +++ b/settings/controller/groupscontroller.php @@ -10,6 +10,7 @@ namespace OC\Settings\Controller; +use OC\AppFramework\Http; use \OCP\AppFramework\Controller; use OCP\AppFramework\Http\DataResponse; use OCP\IGroupManager; @@ -85,7 +86,8 @@ class GroupsController extends Controller { 'data' => array( 'message' => (string)$this->l10n->t('Group already exists.') ) - ) + ), + Http::STATUS_CONFLICT ); } if($this->groupManager->createGroup($id)) { @@ -95,7 +97,8 @@ class GroupsController extends Controller { 'data' => array( 'groupname' => $id ) - ) + ), + Http::STATUS_CREATED ); } @@ -105,7 +108,8 @@ class GroupsController extends Controller { 'data' => array( 'message' => (string)$this->l10n->t('Unable to add group.') ) - ) + ), + Http::STATUS_FORBIDDEN ); } @@ -123,7 +127,8 @@ class GroupsController extends Controller { 'data' => array( 'groupname' => $id ) - ) + ), + Http::STATUS_NO_CONTENT ); } } @@ -132,8 +137,9 @@ class GroupsController extends Controller { 'status' => 'error', 'data' => array( 'message' => (string)$this->l10n->t('Unable to delete group.') - ) - ) + ), + ), + Http::STATUS_FORBIDDEN ); } diff --git a/settings/controller/userscontroller.php b/settings/controller/userscontroller.php index d61d19f8cb..aa16574221 100644 --- a/settings/controller/userscontroller.php +++ b/settings/controller/userscontroller.php @@ -10,6 +10,7 @@ namespace OC\Settings\Controller; +use OC\AppFramework\Http; use OC\User\User; use \OCP\AppFramework\Controller; use OCP\AppFramework\Http\DataResponse; @@ -164,7 +165,8 @@ class UsersController extends Controller { 'data' => array( 'message' => (string)$this->l10n->t('Unable to create user.') ) - ) + ), + Http::STATUS_FORBIDDEN ); } @@ -187,7 +189,8 @@ class UsersController extends Controller { 'groups' => $this->groupManager->getUserGroupIds($user), 'storageLocation' => $user->getHome() ) - ) + ), + Http::STATUS_CREATED ); } @@ -208,7 +211,8 @@ class UsersController extends Controller { 'data' => array( 'message' => (string)$this->l10n->t('Unable to delete user.') ) - ) + ), + Http::STATUS_FORBIDDEN ); } @@ -218,8 +222,10 @@ class UsersController extends Controller { array( 'status' => 'error', 'data' => array( - 'message' => (string)$this->l10n->t('Authentication error')) - ) + 'message' => (string)$this->l10n->t('Authentication error') + ) + ), + Http::STATUS_FORBIDDEN ); } @@ -232,7 +238,8 @@ class UsersController extends Controller { 'data' => array( 'username' => $id ) - ) + ), + Http::STATUS_NO_CONTENT ); } } @@ -243,7 +250,8 @@ class UsersController extends Controller { 'data' => array( 'message' => (string)$this->l10n->t('Unable to delete user.') ) - ) + ), + Http::STATUS_FORBIDDEN ); } diff --git a/tests/settings/controller/groupscontrollertest.php b/tests/settings/controller/groupscontrollertest.php index ac4619cfdb..003fb70b81 100644 --- a/tests/settings/controller/groupscontrollertest.php +++ b/tests/settings/controller/groupscontrollertest.php @@ -11,6 +11,7 @@ namespace OC\Settings\Controller; use OC\Group\Group; use \OC\Settings\Application; +use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; /** @@ -145,7 +146,15 @@ class GroupsControllerTest extends \Test\TestCase { ->with('ExistingGroup') ->will($this->returnValue(true)); - $expectedResponse = new DataResponse(array('status' => 'error', 'data' => array('message' => 'Group already exists.'))); + $expectedResponse = new DataResponse( + array( + 'status' => 'error', + 'data' => array( + 'message' => 'Group already exists.' + ) + ), + Http::STATUS_CONFLICT + ); $response = $this->groupsController->create('ExistingGroup'); $this->assertEquals($expectedResponse, $response); } @@ -162,7 +171,13 @@ class GroupsControllerTest extends \Test\TestCase { ->with('NewGroup') ->will($this->returnValue(true)); - $expectedResponse = new DataResponse(array('status' => 'success', 'data' => array('groupname' => 'NewGroup'))); + $expectedResponse = new DataResponse( + array( + 'status' => 'success', + 'data' => array('groupname' => 'NewGroup') + ), + Http::STATUS_CREATED + ); $response = $this->groupsController->create('NewGroup'); $this->assertEquals($expectedResponse, $response); } @@ -179,7 +194,13 @@ class GroupsControllerTest extends \Test\TestCase { ->with('NewGroup') ->will($this->returnValue(false)); - $expectedResponse = new DataResponse(array('status' => 'error', 'data' => array('message' => 'Unable to add group.'))); + $expectedResponse = new DataResponse( + array( + 'status' => 'error', + 'data' => array('message' => 'Unable to add group.') + ), + Http::STATUS_FORBIDDEN + ); $response = $this->groupsController->create('NewGroup'); $this->assertEquals($expectedResponse, $response); } @@ -197,7 +218,13 @@ class GroupsControllerTest extends \Test\TestCase { ->method('delete') ->will($this->returnValue(true)); - $expectedResponse = new DataResponse(array('status' => 'success', 'data' => array('groupname' => 'ExistingGroup'))); + $expectedResponse = new DataResponse( + array( + 'status' => 'success', + 'data' => array('groupname' => 'ExistingGroup') + ), + Http::STATUS_NO_CONTENT + ); $response = $this->groupsController->destroy('ExistingGroup'); $this->assertEquals($expectedResponse, $response); } @@ -209,7 +236,13 @@ class GroupsControllerTest extends \Test\TestCase { ->with('ExistingGroup') ->will($this->returnValue(null)); - $expectedResponse = new DataResponse(array('status' => 'error', 'data' => array('message' => 'Unable to delete group.'))); + $expectedResponse = new DataResponse( + array( + 'status' => 'error', + 'data' => array('message' => 'Unable to delete group.') + ), + Http::STATUS_FORBIDDEN + ); $response = $this->groupsController->destroy('ExistingGroup'); $this->assertEquals($expectedResponse, $response); } diff --git a/tests/settings/controller/userscontrollertest.php b/tests/settings/controller/userscontrollertest.php index 8aee2b2dd5..0f1dfb4e68 100644 --- a/tests/settings/controller/userscontrollertest.php +++ b/tests/settings/controller/userscontrollertest.php @@ -10,6 +10,7 @@ namespace OC\Settings\Controller; use \OC\Settings\Application; +use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; /** @@ -158,7 +159,8 @@ class UsersControllerTest extends \Test\TestCase { 'groups' => null, 'storageLocation' => '/home/user' ) - ) + ), + Http::STATUS_CREATED ); $response = $this->usersController->create('foo', 'password', array()); $this->assertEquals($expectedResponse, $response); @@ -217,7 +219,8 @@ class UsersControllerTest extends \Test\TestCase { 'groups' => array('NewGroup', 'ExistingGroup'), 'storageLocation' => '/home/user' ) - ) + ), + Http::STATUS_CREATED ); $response = $this->usersController->create('foo', 'password', array('NewGroup', 'ExistingGroup')); $this->assertEquals($expectedResponse, $response); @@ -238,7 +241,8 @@ class UsersControllerTest extends \Test\TestCase { 'data' => array( 'message' => 'Unable to create user.' ) - ) + ), + Http::STATUS_FORBIDDEN ); $response = $this->usersController->create('foo', 'password', array()); $this->assertEquals($expectedResponse, $response); @@ -265,7 +269,8 @@ class UsersControllerTest extends \Test\TestCase { 'data' => array( 'message' => 'Unable to delete user.' ) - ) + ), + Http::STATUS_FORBIDDEN ); $response = $this->usersController->destroy('myself'); $this->assertEquals($expectedResponse, $response); @@ -302,7 +307,45 @@ class UsersControllerTest extends \Test\TestCase { 'data' => array( 'username' => 'UserToDelete' ) - ) + ), + Http::STATUS_NO_CONTENT + ); + $response = $this->usersController->destroy('UserToDelete'); + $this->assertEquals($expectedResponse, $response); + } + /** + * TODO: Since the function uses the static OC_Subadmin class it can't be mocked + * to test for subadmins. Thus the test always assumes you have admin permissions... + */ + public function testDestroyUnsuccessful() { + $user = $this->getMockBuilder('\OC\User\User') + ->disableOriginalConstructor()->getMock(); + $user + ->expects($this->once()) + ->method('getUID') + ->will($this->returnValue('Admin')); + $toDeleteUser = $this->getMockBuilder('\OC\User\User') + ->disableOriginalConstructor()->getMock(); + $toDeleteUser + ->expects($this->once()) + ->method('delete') + ->will($this->returnValue(false)); + $this->container['UserSession'] + ->method('getUser') + ->will($this->returnValue($user)); + $this->container['UserManager'] + ->method('get') + ->with('UserToDelete') + ->will($this->returnValue($toDeleteUser)); + + $expectedResponse = new DataResponse( + array( + 'status' => 'error', + 'data' => array( + 'message' => 'Unable to delete user.' + ) + ), + Http::STATUS_FORBIDDEN ); $response = $this->usersController->destroy('UserToDelete'); $this->assertEquals($expectedResponse, $response); From c23957811d31812cc9193eae0c83f6d7648b971b Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 8 Dec 2014 16:35:13 +0100 Subject: [PATCH 5/5] React on other statuscodes than 200 --- settings/controller/groupscontroller.php | 10 +-- settings/controller/userscontroller.php | 14 ++--- settings/js/users/groups.js | 27 ++++---- settings/js/users/users.js | 61 +++++++++---------- .../controller/groupscontrollertest.php | 8 +-- .../controller/userscontrollertest.php | 23 +++---- 6 files changed, 54 insertions(+), 89 deletions(-) diff --git a/settings/controller/groupscontroller.php b/settings/controller/groupscontroller.php index 8236d5507f..82e72821c3 100644 --- a/settings/controller/groupscontroller.php +++ b/settings/controller/groupscontroller.php @@ -82,10 +82,7 @@ class GroupsController extends Controller { if($this->groupManager->groupExists($id)) { return new DataResponse( array( - 'status' => 'error', - 'data' => array( - 'message' => (string)$this->l10n->t('Group already exists.') - ) + 'message' => (string)$this->l10n->t('Group already exists.') ), Http::STATUS_CONFLICT ); @@ -93,10 +90,7 @@ class GroupsController extends Controller { if($this->groupManager->createGroup($id)) { return new DataResponse( array( - 'status' => 'success', - 'data' => array( - 'groupname' => $id - ) + 'groupname' => $id ), Http::STATUS_CREATED ); diff --git a/settings/controller/userscontroller.php b/settings/controller/userscontroller.php index aa16574221..5bd4b55510 100644 --- a/settings/controller/userscontroller.php +++ b/settings/controller/userscontroller.php @@ -161,10 +161,7 @@ class UsersController extends Controller { } catch (\Exception $exception) { return new DataResponse( array( - 'status' => 'error', - 'data' => array( - 'message' => (string)$this->l10n->t('Unable to create user.') - ) + 'message' => (string)$this->l10n->t('Unable to create user.') ), Http::STATUS_FORBIDDEN ); @@ -183,12 +180,9 @@ class UsersController extends Controller { return new DataResponse( array( - 'status' => 'success', - 'data' => array( - 'username' => $username, - 'groups' => $this->groupManager->getUserGroupIds($user), - 'storageLocation' => $user->getHome() - ) + 'username' => $username, + 'groups' => $this->groupManager->getUserGroupIds($user), + 'storageLocation' => $user->getHome() ), Http::STATUS_CREATED ); diff --git a/settings/js/users/groups.js b/settings/js/users/groups.js index 40872785e3..c06bc5ff14 100644 --- a/settings/js/users/groups.js +++ b/settings/js/users/groups.js @@ -89,24 +89,19 @@ GroupList = { id: groupname }, function (result) { - if (result.status !== 'success') { - OC.dialogs.alert(result.data.message, - t('settings', 'Error creating group')); - } - else { - if (result.data.groupname) { - var addedGroup = result.data.groupname; - UserList.availableGroups = $.unique($.merge(UserList.availableGroups, [addedGroup])); - GroupList.addGroup(result.data.groupname); + if (result.groupname) { + var addedGroup = result.groupname; + UserList.availableGroups = $.unique($.merge(UserList.availableGroups, [addedGroup])); + GroupList.addGroup(result.groupname); - $('.groupsselect, .subadminsselect') - .append($('