diff --git a/apps/provisioning_api/appinfo/routes.php b/apps/provisioning_api/appinfo/routes.php index 08411856e7..00a362864e 100644 --- a/apps/provisioning_api/appinfo/routes.php +++ b/apps/provisioning_api/appinfo/routes.php @@ -26,10 +26,13 @@ namespace OCA\Provisioning_API\AppInfo; +use OCA\Provisioning_API\Apps; +use OCA\Provisioning_API\Groups; +use OCA\Provisioning_API\Users; use OCP\API; // Users -$users = new \OCA\Provisioning_API\Users( +$users = new Users( \OC::$server->getUserManager(), \OC::$server->getConfig(), \OC::$server->getGroupManager(), @@ -41,6 +44,8 @@ API::register('post', '/cloud/users', [$users, 'addUser'], 'provisioning_api', A API::register('get', '/cloud/users/{userid}', [$users, 'getUser'], 'provisioning_api', API::USER_AUTH); API::register('put', '/cloud/users/{userid}', [$users, 'editUser'], 'provisioning_api', API::USER_AUTH); API::register('delete', '/cloud/users/{userid}', [$users, 'deleteUser'], 'provisioning_api', API::SUBADMIN_AUTH); +API::register('put', '/cloud/users/{userid}/enable', [$users, 'enableUser'], 'provisioning_api', API::SUBADMIN_AUTH); +API::register('put', '/cloud/users/{userid}/disable', [$users, 'disableUser'], 'provisioning_api', API::SUBADMIN_AUTH); API::register('get', '/cloud/users/{userid}/groups', [$users, 'getUsersGroups'], 'provisioning_api', API::USER_AUTH); API::register('post', '/cloud/users/{userid}/groups', [$users, 'addToGroup'], 'provisioning_api', API::SUBADMIN_AUTH); API::register('delete', '/cloud/users/{userid}/groups', [$users, 'removeFromGroup'], 'provisioning_api', API::SUBADMIN_AUTH); @@ -49,7 +54,7 @@ API::register('delete', '/cloud/users/{userid}/subadmins', [$users, 'removeSubAd API::register('get', '/cloud/users/{userid}/subadmins', [$users, 'getUserSubAdminGroups'], 'provisioning_api', API::ADMIN_AUTH); // Groups -$groups = new \OCA\Provisioning_API\Groups( +$groups = new Groups( \OC::$server->getGroupManager(), \OC::$server->getUserSession(), \OC::$server->getRequest() @@ -61,7 +66,7 @@ API::register('delete', '/cloud/groups/{groupid}', [$groups, 'deleteGroup'], 'pr API::register('get', '/cloud/groups/{groupid}/subadmins', [$groups, 'getSubAdminsOfGroup'], 'provisioning_api', API::ADMIN_AUTH); // Apps -$apps = new \OCA\Provisioning_API\Apps( +$apps = new Apps( \OC::$server->getAppManager(), \OC::$server->getOcsClient() ); diff --git a/apps/provisioning_api/lib/users.php b/apps/provisioning_api/lib/users.php index 68c89e41f6..2749372c39 100644 --- a/apps/provisioning_api/lib/users.php +++ b/apps/provisioning_api/lib/users.php @@ -31,32 +31,36 @@ namespace OCA\Provisioning_API; use \OC_OCS_Result; use \OC_Helper; use OCP\Files\NotFoundException; +use OCP\IConfig; +use OCP\IGroupManager; use OCP\ILogger; +use OCP\IUserManager; +use OCP\IUserSession; class Users { - /** @var \OCP\IUserManager */ + /** @var IUserManager */ private $userManager; - /** @var \OCP\IConfig */ + /** @var IConfig */ private $config; - /** @var \OCP\IGroupManager */ + /** @var IGroupManager */ private $groupManager; - /** @var \OCP\IUserSession */ + /** @var IUserSession */ private $userSession; /** @var ILogger */ private $logger; /** - * @param \OCP\IUserManager $userManager - * @param \OCP\IConfig $config - * @param \OCP\IGroupManager $groupManager - * @param \OCP\IUserSession $userSession + * @param IUserManager $userManager + * @param IConfig $config + * @param IGroupManager $groupManager + * @param IUserSession $userSession * @param ILogger $logger */ - public function __construct(\OCP\IUserManager $userManager, - \OCP\IConfig $config, - \OCP\IGroupManager $groupManager, - \OCP\IUserSession $userSession, + public function __construct(IUserManager $userManager, + IConfig $config, + IGroupManager $groupManager, + IUserSession $userSession, ILogger $logger) { $this->userManager = $userManager; $this->config = $config; @@ -329,6 +333,50 @@ class Users { } } + /** + * @param array $parameters + * @return OC_OCS_Result + */ + public function disableUser($parameters) { + return $this->setEnabled($parameters, false); + } + + /** + * @param array $parameters + * @return OC_OCS_Result + */ + public function enableUser($parameters) { + return $this->setEnabled($parameters, true); + } + + /** + * @param array $parameters + * @param bool $value + * @return OC_OCS_Result + */ + private function setEnabled($parameters, $value) { + // Check if user is logged in + $currentLoggedInUser = $this->userSession->getUser(); + if ($currentLoggedInUser === null) { + return new OC_OCS_Result(null, \OCP\API::RESPOND_UNAUTHORISED); + } + + $targetUser = $this->userManager->get($parameters['userid']); + if($targetUser === null || $targetUser->getUID() === $currentLoggedInUser->getUID()) { + return new OC_OCS_Result(null, 101); + } + + // If not permitted + $subAdminManager = $this->groupManager->getSubAdmin(); + if(!$this->groupManager->isAdmin($currentLoggedInUser->getUID()) && !$subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)) { + return new OC_OCS_Result(null, 997); + } + + // enable/disable the user now + $targetUser->setEnabled($value); + return new OC_OCS_Result(null, 100); + } + /** * @param array $parameters * @return OC_OCS_Result diff --git a/apps/provisioning_api/tests/userstest.php b/apps/provisioning_api/tests/userstest.php index 020071bcfa..8f463ec8b8 100644 --- a/apps/provisioning_api/tests/userstest.php +++ b/apps/provisioning_api/tests/userstest.php @@ -58,8 +58,8 @@ class UsersTest extends OriginalTest { parent::tearDown(); } - protected function setup() { - parent::setup(); + protected function setUp() { + parent::setUp(); $this->userManager = $this->getMock('\OCP\IUserManager'); $this->config = $this->getMock('\OCP\IConfig'); @@ -540,7 +540,7 @@ class UsersTest extends OriginalTest { ->expects($this->once()) ->method('isSubAdminOfGroup') ->with($loggedInUser, $existingGroup) - ->wilLReturn(false); + ->willReturn(false); $this->groupManager ->expects($this->once()) ->method('getSubAdmin') @@ -642,7 +642,7 @@ class UsersTest extends OriginalTest { [$loggedInUser, $existingGroup1], [$loggedInUser, $existingGroup2] ) - ->wilLReturn(true); + ->willReturn(true); $expected = new \OC_OCS_Result(null, 100); @@ -2295,4 +2295,60 @@ class UsersTest extends OriginalTest { $expected = new \OC_OCS_Result(null, 102, 'Unknown error occurred'); $this->assertEquals($expected, $this->api->getUserSubAdminGroups(['userid' => 'RequestedUser'])); } + + public function testEnableUser() { + $targetUser = $this->getMock('\OCP\IUser'); + $targetUser->expects($this->once()) + ->method('setEnabled') + ->with(true); + $this->userManager + ->expects($this->once()) + ->method('get') + ->with('RequestedUser') + ->will($this->returnValue($targetUser)); + $loggedInUser = $this->getMock('\OCP\IUser'); + $loggedInUser + ->expects($this->exactly(2)) + ->method('getUID') + ->will($this->returnValue('admin')); + $this->userSession + ->expects($this->once()) + ->method('getUser') + ->will($this->returnValue($loggedInUser)); + $this->groupManager + ->expects($this->once()) + ->method('isAdmin') + ->will($this->returnValue(true)); + + $expected = new \OC_OCS_Result(null, 100); + $this->assertEquals($expected, $this->api->enableUser(['userid' => 'RequestedUser'])); + } + + public function testDisableUser() { + $targetUser = $this->getMock('\OCP\IUser'); + $targetUser->expects($this->once()) + ->method('setEnabled') + ->with(false); + $this->userManager + ->expects($this->once()) + ->method('get') + ->with('RequestedUser') + ->will($this->returnValue($targetUser)); + $loggedInUser = $this->getMock('\OCP\IUser'); + $loggedInUser + ->expects($this->exactly(2)) + ->method('getUID') + ->will($this->returnValue('admin')); + $this->userSession + ->expects($this->once()) + ->method('getUser') + ->will($this->returnValue($loggedInUser)); + $this->groupManager + ->expects($this->once()) + ->method('isAdmin') + ->will($this->returnValue(true)); + + $expected = new \OC_OCS_Result(null, 100); + $this->assertEquals($expected, $this->api->disableUser(['userid' => 'RequestedUser'])); + } } diff --git a/build/integration/features/bootstrap/Provisioning.php b/build/integration/features/bootstrap/Provisioning.php index feeb850ae7..6cf5751448 100644 --- a/build/integration/features/bootstrap/Provisioning.php +++ b/build/integration/features/bootstrap/Provisioning.php @@ -35,7 +35,6 @@ trait Provisioning { } $this->userExists($user); PHPUnit_Framework_Assert::assertEquals(200, $this->response->getStatusCode()); - } /** @@ -230,6 +229,20 @@ trait Provisioning { } } + /** + * @When /^assure user "([^"]*)" is disabled$/ + */ + public function assureUserIsDisabled($user) { + $fullUrl = $this->baseUrl . "v{$this->apiVersion}.php/cloud/users/$user/disable"; + $client = new Client(); + $options = []; + if ($this->currentUser === 'admin') { + $options['auth'] = $this->adminUser; + } + + $this->response = $client->send($client->createRequest("PUT", $fullUrl, $options)); + } + /** * @When /^Deleting the user "([^"]*)"$/ * @param string $user @@ -363,6 +376,25 @@ trait Provisioning { PHPUnit_Framework_Assert::assertEquals(200, $this->response->getStatusCode()); } + /** + * @Given /^Assure user "([^"]*)" is subadmin of group "([^"]*)"$/ + * @param string $user + * @param string $group + */ + public function assureUserIsSubadminOfGroup($user, $group) { + $fullUrl = $this->baseUrl . "v{$this->apiVersion}.php/cloud/users/$user/subadmins"; + $client = new Client(); + $options = []; + if ($this->currentUser === 'admin') { + $options['auth'] = $this->adminUser; + } + $options['body'] = [ + 'groupid' => $group + ]; + $this->response = $client->send($client->createRequest("POST", $fullUrl, $options)); + PHPUnit_Framework_Assert::assertEquals(200, $this->response->getStatusCode()); + } + /** * @Given /^user "([^"]*)" is not a subadmin of group "([^"]*)"$/ * @param string $user @@ -528,6 +560,38 @@ trait Provisioning { PHPUnit_Framework_Assert::assertEquals(200, $this->response->getStatusCode()); } + /** + * @Then /^user "([^"]*)" is disabled$/ + * @param string $user + */ + public function userIsDisabled($user) { + $fullUrl = $this->baseUrl . "v{$this->apiVersion}.php/cloud/users/$user"; + $client = new Client(); + $options = []; + if ($this->currentUser === 'admin') { + $options['auth'] = $this->adminUser; + } + + $this->response = $client->get($fullUrl, $options); + PHPUnit_Framework_Assert::assertEquals("false", $this->response->xml()->data[0]->enabled); + } + + /** + * @Then /^user "([^"]*)" is enabled$/ + * @param string $user + */ + public function userIsEnabled($user) { + $fullUrl = $this->baseUrl . "v{$this->apiVersion}.php/cloud/users/$user"; + $client = new Client(); + $options = []; + if ($this->currentUser === 'admin') { + $options['auth'] = $this->adminUser; + } + + $this->response = $client->get($fullUrl, $options); + PHPUnit_Framework_Assert::assertEquals("true", $this->response->xml()->data[0]->enabled); + } + /** * @Given user :user has a quota of :quota * @param string $user @@ -588,4 +652,5 @@ trait Provisioning { } $this->usingServer($previousServer); } + } diff --git a/build/integration/features/bootstrap/WebDav.php b/build/integration/features/bootstrap/WebDav.php index 79c447ac57..cd9584ad18 100644 --- a/build/integration/features/bootstrap/WebDav.php +++ b/build/integration/features/bootstrap/WebDav.php @@ -424,5 +424,15 @@ trait WebDav { } + /** + * @Given /^Downloading file "([^"]*)" as "([^"]*)"$/ + */ + public function downloadingFileAs($fileName, $user) { + try { + $this->response = $this->makeDavRequest($user, 'GET', $fileName, []); + } catch (\GuzzleHttp\Exception\ServerException $ex) { + $this->response = $ex->getResponse(); + } + } } diff --git a/build/integration/features/provisioning-v1.feature b/build/integration/features/provisioning-v1.feature index 8c32c04523..d44903c274 100644 --- a/build/integration/features/provisioning-v1.feature +++ b/build/integration/features/provisioning-v1.feature @@ -315,3 +315,197 @@ Feature: provisioning Then the OCS status code should be "100" And the HTTP status code should be "200" And app "files_external" is disabled + + Scenario: disable an user + Given As an "admin" + And user "user1" exists + When sending "PUT" to "/cloud/users/user1/disable" + Then the OCS status code should be "100" + And the HTTP status code should be "200" + And user "user1" is disabled + + Scenario: enable an user + Given As an "admin" + And user "user1" exists + And assure user "user1" is disabled + When sending "PUT" to "/cloud/users/user1/enable" + Then the OCS status code should be "100" + And the HTTP status code should be "200" + And user "user1" is enabled + + Scenario: Subadmin should be able to enable or disable an user in their group + Given As an "admin" + And user "subadmin" exists + And user "user1" exists + And group "new-group" exists + And user "subadmin" belongs to group "new-group" + And user "user1" belongs to group "new-group" + And Assure user "subadmin" is subadmin of group "new-group" + And As an "subadmin" + When sending "PUT" to "/cloud/users/user1/disable" + Then the OCS status code should be "100" + Then the HTTP status code should be "200" + And As an "admin" + And user "user1" is disabled + + Scenario: Subadmin should not be able to enable or disable an user not in their group + Given As an "admin" + And user "subadmin" exists + And user "user1" exists + And group "new-group" exists + And group "another-group" exists + And user "subadmin" belongs to group "new-group" + And user "user1" belongs to group "another-group" + And Assure user "subadmin" is subadmin of group "new-group" + And As an "subadmin" + When sending "PUT" to "/cloud/users/user1/disable" + Then the OCS status code should be "997" + Then the HTTP status code should be "401" + And As an "admin" + And user "user1" is enabled + + Scenario: Subadmins should not be able to disable users that have admin permissions in their group + Given As an "admin" + And user "another-admin" exists + And user "subadmin" exists + And group "new-group" exists + And user "another-admin" belongs to group "admin" + And user "subadmin" belongs to group "new-group" + And user "another-admin" belongs to group "new-group" + And Assure user "subadmin" is subadmin of group "new-group" + And As an "subadmin" + When sending "PUT" to "/cloud/users/another-admin/disable" + Then the OCS status code should be "997" + Then the HTTP status code should be "401" + And As an "admin" + And user "another-admin" is enabled + + Scenario: Admin can disable another admin user + Given As an "admin" + And user "another-admin" exists + And user "another-admin" belongs to group "admin" + When sending "PUT" to "/cloud/users/another-admin/disable" + Then the OCS status code should be "100" + Then the HTTP status code should be "200" + And user "another-admin" is disabled + + Scenario: Admin can enable another admin user + Given As an "admin" + And user "another-admin" exists + And user "another-admin" belongs to group "admin" + And assure user "another-admin" is disabled + When sending "PUT" to "/cloud/users/another-admin/enable" + Then the OCS status code should be "100" + Then the HTTP status code should be "200" + And user "another-admin" is enabled + + Scenario: Admin can disable subadmins in the same group + Given As an "admin" + And user "subadmin" exists + And group "new-group" exists + And user "subadmin" belongs to group "new-group" + And user "admin" belongs to group "new-group" + And Assure user "subadmin" is subadmin of group "new-group" + When sending "PUT" to "/cloud/users/subadmin/disable" + Then the OCS status code should be "100" + Then the HTTP status code should be "200" + And user "subadmin" is disabled + + Scenario: Admin can enable subadmins in the same group + Given As an "admin" + And user "subadmin" exists + And group "new-group" exists + And user "subadmin" belongs to group "new-group" + And user "admin" belongs to group "new-group" + And Assure user "subadmin" is subadmin of group "new-group" + And assure user "another-admin" is disabled + When sending "PUT" to "/cloud/users/subadmin/disable" + Then the OCS status code should be "100" + Then the HTTP status code should be "200" + And user "subadmin" is disabled + + Scenario: Admin user cannot disable himself + Given As an "admin" + And user "another-admin" exists + And user "another-admin" belongs to group "admin" + And As an "another-admin" + When sending "PUT" to "/cloud/users/another-admin/disable" + Then the OCS status code should be "101" + And the HTTP status code should be "200" + And As an "admin" + And user "another-admin" is enabled + + Scenario:Admin user cannot enable himself + Given As an "admin" + And user "another-admin" exists + And user "another-admin" belongs to group "admin" + And assure user "another-admin" is disabled + And As an "another-admin" + When sending "PUT" to "/cloud/users/another-admin/enable" + And As an "admin" + Then user "another-admin" is disabled + + Scenario: disable an user with a regular user + Given As an "admin" + And user "user1" exists + And user "user2" exists + And As an "user1" + When sending "PUT" to "/cloud/users/user2/disable" + Then the OCS status code should be "997" + And the HTTP status code should be "401" + And As an "admin" + And user "user2" is enabled + + Scenario: enable an user with a regular user + Given As an "admin" + And user "user1" exists + And user "user2" exists + And assure user "user2" is disabled + And As an "user1" + When sending "PUT" to "/cloud/users/user2/enable" + Then the OCS status code should be "997" + And the HTTP status code should be "401" + And As an "admin" + And user "user2" is disabled + + Scenario: Subadmin should not be able to disable himself + Given As an "admin" + And user "subadmin" exists + And group "new-group" exists + And user "subadmin" belongs to group "new-group" + And Assure user "subadmin" is subadmin of group "new-group" + And As an "subadmin" + When sending "PUT" to "/cloud/users/subadmin/disable" + Then the OCS status code should be "101" + Then the HTTP status code should be "200" + And As an "admin" + And user "subadmin" is enabled + + Scenario: Subadmin should not be able to enable himself + Given As an "admin" + And user "subadmin" exists + And group "new-group" exists + And user "subadmin" belongs to group "new-group" + And Assure user "subadmin" is subadmin of group "new-group" + And assure user "subadmin" is disabled + And As an "subadmin" + When sending "PUT" to "/cloud/users/subadmin/enabled" + And As an "admin" + And user "subadmin" is disabled + + Scenario: Making a web request with an enabled user + Given As an "admin" + And user "user0" exists + And As an "user0" + When sending "GET" to "/index.php/apps/files" + Then the HTTP status code should be "200" + + Scenario: Making a web request with a disabled user + Given As an "admin" + And user "user0" exists + And assure user "user0" is disabled + And As an "user0" + When sending "GET" to "/index.php/apps/files" + Then the OCS status code should be "999" + And the HTTP status code should be "200" + diff --git a/build/integration/features/sharing-v1.feature b/build/integration/features/sharing-v1.feature index d67afede95..79d7617f02 100644 --- a/build/integration/features/sharing-v1.feature +++ b/build/integration/features/sharing-v1.feature @@ -696,3 +696,16 @@ Feature: sharing Then user "user2" should see following elements | /foo/ | | /foo%20(2)/ | + + Scenario: Creating a new share with a disabled user + Given As an "admin" + And user "user0" exists + And user "user1" exists + And assure user "user0" is disabled + And As an "user0" + When sending "POST" to "/apps/files_sharing/api/v1/shares" with + | path | welcome.txt | + | shareWith | user1 | + | shareType | 0 | + Then the OCS status code should be "997" + And the HTTP status code should be "401" diff --git a/build/integration/features/webdav-related.feature b/build/integration/features/webdav-related.feature index abdc63935e..f4d40615fa 100644 --- a/build/integration/features/webdav-related.feature +++ b/build/integration/features/webdav-related.feature @@ -286,3 +286,11 @@ Feature: webdav-related When As an "user0" And Downloading file "/files/user0/myChunkedFile.txt" Then Downloaded content should be "AAAAABBBBBCCCCC" + + Scenario: A disabled user cannot use webdav + Given user "userToBeDisabled" exists + And As an "admin" + And assure user "userToBeDisabled" is disabled + When Downloading file "/welcome.txt" as "userToBeDisabled" + Then the HTTP status code should be "503" + diff --git a/core/Command/User/Disable.php b/core/Command/User/Disable.php new file mode 100644 index 0000000000..018f11190d --- /dev/null +++ b/core/Command/User/Disable.php @@ -0,0 +1,64 @@ + + * + * @copyright Copyright (c) 2016, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program 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, version 3, + * along with this program. If not, see + * + */ + +namespace OC\Core\Command\User; + +use OCP\IUser; +use OCP\IUserManager; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Console\Input\InputArgument; + +class Disable extends Command { + /** @var IUserManager */ + protected $userManager; + + /** + * @param IUserManager $userManager + */ + public function __construct(IUserManager $userManager) { + $this->userManager = $userManager; + parent::__construct(); + } + + protected function configure() { + $this + ->setName('user:disable') + ->setDescription('disables the specified user') + ->addArgument( + 'uid', + InputArgument::REQUIRED, + 'the username' + ); + } + + protected function execute(InputInterface $input, OutputInterface $output) { + $user = $this->userManager->get($input->getArgument('uid')); + if (is_null($user)) { + $output->writeln('User does not exist'); + return; + } + + $user->setEnabled(false); + $output->writeln('The specified user is disabled'); + } +} diff --git a/core/Command/User/Enable.php b/core/Command/User/Enable.php new file mode 100644 index 0000000000..ffe2e40d65 --- /dev/null +++ b/core/Command/User/Enable.php @@ -0,0 +1,64 @@ + + * + * @copyright Copyright (c) 2016, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program 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, version 3, + * along with this program. If not, see + * + */ + +namespace OC\Core\Command\User; + +use OCP\IUser; +use OCP\IUserManager; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Console\Input\InputArgument; + +class Enable extends Command { + /** @var IUserManager */ + protected $userManager; + + /** + * @param IUserManager $userManager + */ + public function __construct(IUserManager $userManager) { + $this->userManager = $userManager; + parent::__construct(); + } + + protected function configure() { + $this + ->setName('user:enable') + ->setDescription('enables the specified user') + ->addArgument( + 'uid', + InputArgument::REQUIRED, + 'the username' + ); + } + + protected function execute(InputInterface $input, OutputInterface $output) { + $user = $this->userManager->get($input->getArgument('uid')); + if (is_null($user)) { + $output->writeln('User does not exist'); + return; + } + + $user->setEnabled(true); + $output->writeln('The specified user is enabled'); + } +} diff --git a/core/register_command.php b/core/register_command.php index 798497d97d..3dab37ce5a 100644 --- a/core/register_command.php +++ b/core/register_command.php @@ -121,6 +121,8 @@ if (\OC::$server->getConfig()->getSystemValue('installed', false)) { $application->add(new OC\Core\Command\User\Add(\OC::$server->getUserManager(), \OC::$server->getGroupManager())); $application->add(new OC\Core\Command\User\Delete(\OC::$server->getUserManager())); + $application->add(new OC\Core\Command\User\Disable(\OC::$server->getUserManager())); + $application->add(new OC\Core\Command\User\Enable(\OC::$server->getUserManager())); $application->add(new OC\Core\Command\User\LastSeen(\OC::$server->getUserManager())); $application->add(new OC\Core\Command\User\Report(\OC::$server->getUserManager())); $application->add(new OC\Core\Command\User\ResetPassword(\OC::$server->getUserManager())); diff --git a/lib/private/legacy/api.php b/lib/private/legacy/api.php index bab879c95f..702b9df192 100644 --- a/lib/private/legacy/api.php +++ b/lib/private/legacy/api.php @@ -356,7 +356,11 @@ class OC_API { if(isset($_SERVER['PHP_AUTH_USER']) && isset($_SERVER['PHP_AUTH_PW']) ) { $authUser = $_SERVER['PHP_AUTH_USER']; $authPw = $_SERVER['PHP_AUTH_PW']; - $return = OC_User::login($authUser, $authPw); + try { + $return = OC_User::login($authUser, $authPw); + } catch (\OC\User\LoginException $e) { + return false; + } if ($return === true) { self::$logoutRequired = true; diff --git a/lib/private/legacy/user.php b/lib/private/legacy/user.php index 78eb4bab12..18a4c369d5 100644 --- a/lib/private/legacy/user.php +++ b/lib/private/legacy/user.php @@ -63,8 +63,6 @@ class OC_User { return OC::$server->getUserSession(); } - private static $_backends = array(); - private static $_usedBackends = array(); private static $_setupedBackends = array(); @@ -105,7 +103,7 @@ class OC_User { break; default: \OCP\Util::writeLog('core', 'Adding default user backend ' . $backend . '.', \OCP\Util::DEBUG); - $className = 'OC_USER_' . strToUpper($backend); + $className = 'OC_USER_' . strtoupper($backend); self::$_usedBackends[$backend] = new $className(); \OC::$server->getUserManager()->registerBackend(self::$_usedBackends[$backend]); break; @@ -183,6 +181,7 @@ class OC_User { /** * Try to login a user using the magic cookie (remember login) * + * @deprecated use \OCP\IUserSession::loginWithCookie() * @param string $uid The username of the user to log in * @param string $token * @return bool @@ -249,6 +248,8 @@ class OC_User { /** * Sets user id for session and triggers emit + * + * @param string $uid */ public static function setUserId($uid) { $userSession = \OC::$server->getUserSession(); @@ -304,14 +305,11 @@ class OC_User { /** * Check if the user is logged in, considers also the HTTP basic credentials * + * @deprecated use \OC::$server->getUserSession()->isLoggedIn() * @return bool */ public static function isLoggedIn() { - if (\OC::$server->getSession()->get('user_id') !== null && self::$incognitoMode === false) { - return self::userExists(\OC::$server->getSession()->get('user_id')); - } - - return false; + return \OC::$server->getUserSession()->isLoggedIn(); } /** diff --git a/lib/private/user/session.php b/lib/private/user/session.php index 5402c5cf74..c7f8a6920d 100644 --- a/lib/private/user/session.php +++ b/lib/private/user/session.php @@ -32,6 +32,8 @@ namespace OC\User; use OC\Hooks\Emitter; +use OCP\ISession; +use OCP\IUserManager; use OCP\IUserSession; /** @@ -53,26 +55,20 @@ use OCP\IUserSession; * @package OC\User */ class Session implements IUserSession, Emitter { - /** - * @var \OC\User\Manager $manager - */ + /** @var \OC\User\Manager $manager */ private $manager; - /** - * @var \OC\Session\Session $session - */ + /** @var \OC\Session\Session $session */ private $session; - /** - * @var \OC\User\User $activeUser - */ + /** @var \OC\User\User $activeUser */ protected $activeUser; /** - * @param \OCP\IUserManager $manager - * @param \OCP\ISession $session + * @param IUserManager $manager + * @param ISession $session */ - public function __construct(\OCP\IUserManager $manager, \OCP\ISession $session) { + public function __construct(IUserManager $manager, ISession $session) { $this->manager = $manager; $this->session = $session; } @@ -107,7 +103,7 @@ class Session implements IUserSession, Emitter { /** * get the session object * - * @return \OCP\ISession + * @return ISession */ public function getSession() { return $this->session; @@ -116,10 +112,10 @@ class Session implements IUserSession, Emitter { /** * set the session object * - * @param \OCP\ISession $session + * @param ISession $session */ - public function setSession(\OCP\ISession $session) { - if ($this->session instanceof \OCP\ISession) { + public function setSession(ISession $session) { + if ($this->session instanceof ISession) { $this->session->close(); } $this->session = $session; @@ -170,7 +166,12 @@ class Session implements IUserSession, Emitter { * @return bool if logged in */ public function isLoggedIn() { - return $this->getUser() !== null; + $user = $this->getUser(); + if (is_null($user)) { + return false; + } + + return $user->isEnabled(); } /** @@ -226,15 +227,18 @@ class Session implements IUserSession, Emitter { if ($this->isLoggedIn()) { return true; } else { - throw new LoginException('Login canceled by app'); + // injecting l10n does not work - there is a circular dependency between session and \OCP\L10N\IFactory + $message = \OC::$server->getL10N('lib')->t('Login canceled by app'); + throw new LoginException($message); } } else { - return false; + // injecting l10n does not work - there is a circular dependency between session and \OCP\L10N\IFactory + $message = \OC::$server->getL10N('lib')->t('User disabled'); + throw new LoginException($message); } } - } else { - return false; } + return false; } /** diff --git a/lib/private/user/user.php b/lib/private/user/user.php index 3199790dba..3668043676 100644 --- a/lib/private/user/user.php +++ b/lib/private/user/user.php @@ -417,5 +417,4 @@ class User implements IUser { $this->emitter->emit('\OC\User', 'changeUser', array($this, $feature, $value)); } } - } diff --git a/lib/public/iuser.php b/lib/public/iuser.php index b0dd8dc35b..16617a2f2f 100644 --- a/lib/public/iuser.php +++ b/lib/public/iuser.php @@ -32,6 +32,7 @@ namespace OCP; * @since 8.0.0 */ interface IUser { + /** * get the user id * diff --git a/tests/lib/user/session.php b/tests/lib/user/session.php index 1c042dec9f..5a8ea57cb8 100644 --- a/tests/lib/user/session.php +++ b/tests/lib/user/session.php @@ -11,19 +11,25 @@ namespace Test\User; use OC\Session\Memory; use OC\User\User; +use OCP\ISession; +use OCP\IUserManager; +use OCP\UserInterface; +use Test\TestCase; /** * @group DB * @package Test\User */ -class Session extends \Test\TestCase { +class Session extends TestCase { public function testGetUser() { + /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->once()) ->method('get') ->with('user_id') ->will($this->returnValue('foo')); + /** @var UserInterface | \PHPUnit_Framework_MockObject_MockObject $backend */ $backend = $this->getMock('\Test\Util\User\Dummy'); $backend->expects($this->once()) ->method('userExists') @@ -39,12 +45,14 @@ class Session extends \Test\TestCase { } public function testIsLoggedIn() { + /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->once()) ->method('get') ->with('user_id') ->will($this->returnValue('foo')); + /** @var UserInterface | \PHPUnit_Framework_MockObject_MockObject $backend */ $backend = $this->getMock('\Test\Util\User\Dummy'); $backend->expects($this->once()) ->method('userExists') @@ -60,12 +68,14 @@ class Session extends \Test\TestCase { } public function testNotLoggedIn() { + /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->once()) ->method('get') ->with('user_id') ->will($this->returnValue(null)); + /** @var UserInterface | \PHPUnit_Framework_MockObject_MockObject $backend */ $backend = $this->getMock('\Test\Util\User\Dummy'); $backend->expects($this->never()) ->method('userExists'); @@ -79,15 +89,18 @@ class Session extends \Test\TestCase { } public function testSetUser() { + /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->once()) ->method('set') ->with('user_id', 'foo'); + /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject $manager */ $manager = $this->getMock('\OC\User\Manager'); $backend = $this->getMock('\Test\Util\User\Dummy'); + /** @var User | \PHPUnit_Framework_MockObject_MockObject $user */ $user = $this->getMock('\OC\User\User', array(), array('foo', $backend)); $user->expects($this->once()) ->method('getUID') @@ -98,6 +111,7 @@ class Session extends \Test\TestCase { } public function testLoginValidPasswordEnabled() { + /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->once()) ->method('regenerateId'); @@ -126,12 +140,13 @@ class Session extends \Test\TestCase { unset($managerMethods[$i]); } } + /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject $manager */ $manager = $this->getMock('\OC\User\Manager', $managerMethods, array()); $backend = $this->getMock('\Test\Util\User\Dummy'); $user = $this->getMock('\OC\User\User', array(), array('foo', $backend)); - $user->expects($this->once()) + $user->expects($this->exactly(2)) ->method('isEnabled') ->will($this->returnValue(true)); $user->expects($this->any()) @@ -150,7 +165,12 @@ class Session extends \Test\TestCase { $this->assertEquals($user, $userSession->getUser()); } + /** + * @expectedException \OC\User\LoginException + * @expectedExceptionMessage User disabled + */ public function testLoginValidPasswordDisabled() { + /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->never()) ->method('set'); @@ -167,6 +187,7 @@ class Session extends \Test\TestCase { unset($managerMethods[$i]); } } + /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject $manager */ $manager = $this->getMock('\OC\User\Manager', $managerMethods, array()); $backend = $this->getMock('\Test\Util\User\Dummy'); @@ -188,6 +209,7 @@ class Session extends \Test\TestCase { } public function testLoginInvalidPassword() { + /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->never()) ->method('set'); @@ -204,6 +226,7 @@ class Session extends \Test\TestCase { unset($managerMethods[$i]); } } + /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject $manager */ $manager = $this->getMock('\OC\User\Manager', $managerMethods, array()); $backend = $this->getMock('\Test\Util\User\Dummy'); @@ -224,16 +247,16 @@ class Session extends \Test\TestCase { } public function testLoginNonExisting() { + /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->never()) ->method('set'); $session->expects($this->once()) ->method('regenerateId'); + /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject $manager */ $manager = $this->getMock('\OC\User\Manager'); - $backend = $this->getMock('\Test\Util\User\Dummy'); - $manager->expects($this->once()) ->method('checkPassword') ->with('foo', 'bar') @@ -244,6 +267,7 @@ class Session extends \Test\TestCase { } public function testRememberLoginValidToken() { + /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->exactly(1)) ->method('set') @@ -290,6 +314,7 @@ class Session extends \Test\TestCase { $token = 'goodToken'; \OC::$server->getConfig()->setUserValue('foo', 'login_token', $token, time()); + /** @var \OC\User\Session $userSession */ $userSession = $this->getMock( '\OC\User\Session', //override, otherwise tests will fail because of setcookie() @@ -303,6 +328,7 @@ class Session extends \Test\TestCase { } public function testRememberLoginInvalidToken() { + /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->never()) ->method('set'); @@ -319,6 +345,7 @@ class Session extends \Test\TestCase { unset($managerMethods[$i]); } } + /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject $manager */ $manager = $this->getMock('\OC\User\Manager', $managerMethods, array()); $backend = $this->getMock('\Test\Util\User\Dummy'); @@ -347,6 +374,7 @@ class Session extends \Test\TestCase { } public function testRememberLoginInvalidUser() { + /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->never()) ->method('set'); @@ -363,6 +391,7 @@ class Session extends \Test\TestCase { unset($managerMethods[$i]); } } + /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject $manager */ $manager = $this->getMock('\OC\User\Manager', $managerMethods, array()); $backend = $this->getMock('\Test\Util\User\Dummy'); @@ -395,6 +424,7 @@ class Session extends \Test\TestCase { 'bar' => new User('bar', null) ); + /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject $manager */ $manager = $this->getMockBuilder('\OC\User\Manager') ->disableOriginalConstructor() ->getMock();