From fee42647fb06b30cf35e2b21e2e8b5c8ef72bcc8 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 24 Jan 2017 14:07:52 +0100 Subject: [PATCH 1/2] add data from the users profile to the provisioning api Signed-off-by: Bjoern Schiessle --- .../lib/Controller/UsersController.php | 18 +++++-- .../tests/Controller/UsersControllerTest.php | 51 ++++++++++++++++++- 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 2e8a2ffe5e..e659b49443 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -29,6 +29,7 @@ namespace OCA\Provisioning_API\Controller; +use OC\Accounts\AccountManager; use \OC_Helper; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCS\OCSException; @@ -53,6 +54,8 @@ class UsersController extends OCSController { private $groupManager; /** @var IUserSession */ private $userSession; + /** @var AccountManager */ + private $accountManager; /** @var ILogger */ private $logger; @@ -63,6 +66,7 @@ class UsersController extends OCSController { * @param IConfig $config * @param IGroupManager $groupManager * @param IUserSession $userSession + * @param AccountManager $accountManager * @param ILogger $logger */ public function __construct($appName, @@ -71,6 +75,7 @@ class UsersController extends OCSController { IConfig $config, IGroupManager $groupManager, IUserSession $userSession, + AccountManager $accountManager, ILogger $logger) { parent::__construct($appName, $request); @@ -78,6 +83,7 @@ class UsersController extends OCSController { $this->config = $config; $this->groupManager = $groupManager; $this->userSession = $userSession; + $this->accountManager = $accountManager; $this->logger = $logger; } @@ -107,7 +113,7 @@ class UsersController extends OCSController { } if($offset === null) { - $offset = 0; + $offset = 0; } $users = []; @@ -159,7 +165,7 @@ class UsersController extends OCSController { throw new OCSException('no group specified (required for subadmins)', 106); } } - + try { $newUser = $this->userManager->createUser($userid, $password); $this->logger->info('Successful addUser call with userid: '.$userid, ['app' => 'ocs_api']); @@ -209,10 +215,16 @@ class UsersController extends OCSController { } } + $userAccount = $this->accountManager->getUser($targetUserObject); + // Find the data $data['quota'] = $this->fillStorageInfo($userId); $data['email'] = $targetUserObject->getEMailAddress(); $data['displayname'] = $targetUserObject->getDisplayName(); + $data['phone'] = $userAccount[\OC\Accounts\AccountManager::PROPERTY_PHONE]['value']; + $data['address'] = $userAccount[\OC\Accounts\AccountManager::PROPERTY_ADDRESS]['value']; + $data['webpage'] = $userAccount[\OC\Accounts\AccountManager::PROPERTY_WEBSITE]['value']; + $data['twitter'] = $userAccount[\OC\Accounts\AccountManager::PROPERTY_TWITTER]['value']; return new DataResponse($data); } @@ -436,7 +448,7 @@ class UsersController extends OCSController { throw new OCSException('', \OCP\API::RESPOND_UNAUTHORISED); } } - + } /** diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index 4d3da5fd33..1565407b83 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -29,6 +29,7 @@ namespace OCA\Provisioning_API\Tests\Controller; +use OC\Accounts\AccountManager; use OCA\Provisioning_API\Controller\UsersController; use OCP\AppFramework\Http\DataResponse; use OCP\IGroup; @@ -41,7 +42,7 @@ use Test\TestCase as OriginalTest; use OCP\ILogger; class UsersControllerTest extends OriginalTest { - + /** @var IUserManager | PHPUnit_Framework_MockObject_MockObject */ protected $userManager; /** @var IConfig | PHPUnit_Framework_MockObject_MockObject */ @@ -54,6 +55,8 @@ class UsersControllerTest extends OriginalTest { protected $logger; /** @var UsersController | PHPUnit_Framework_MockObject_MockObject */ protected $api; + /** @var AccountManager | PHPUnit_Framework_MockObject_MockObject */ + protected $accountManager; protected function tearDown() { parent::tearDown(); @@ -80,6 +83,9 @@ class UsersControllerTest extends OriginalTest { $request = $this->getMockBuilder('OCP\IRequest') ->disableOriginalConstructor() ->getMock(); + $this->accountManager = $this->getMockBuilder(AccountManager::class) + ->disableOriginalConstructor() + ->getMock(); $this->api = $this->getMockBuilder('OCA\Provisioning_API\Controller\UsersController') ->setConstructorArgs([ 'provisioning_api', @@ -88,6 +94,7 @@ class UsersControllerTest extends OriginalTest { $this->config, $this->groupManager, $this->userSession, + $this->accountManager, $this->logger, ]) ->setMethods(['fillStorageInfo']) @@ -652,6 +659,16 @@ class UsersControllerTest extends OriginalTest { ->method('isAdmin') ->with('admin') ->will($this->returnValue(true)); + $this->accountManager->expects($this->any())->method('getUser') + ->with($targetUser) + ->willReturn( + [ + AccountManager::PROPERTY_ADDRESS => ['value' => 'address'], + AccountManager::PROPERTY_PHONE => ['value' => 'phone'], + AccountManager::PROPERTY_TWITTER => ['value' => 'twitter'], + AccountManager::PROPERTY_WEBSITE => ['value' => 'website'], + ] + ); $this->config ->expects($this->at(0)) ->method('getUserValue') @@ -672,6 +689,10 @@ class UsersControllerTest extends OriginalTest { 'quota' => ['DummyValue'], 'email' => 'demo@owncloud.org', 'displayname' => 'Demo User', + 'phone' => 'phone', + 'address' => 'address', + 'webpage' => 'website', + 'twitter' => 'twitter' ]; $this->assertEquals($expected, $this->api->getUser('UserToGet')->getData()); } @@ -731,12 +752,26 @@ class UsersControllerTest extends OriginalTest { ->expects($this->once()) ->method('getDisplayName') ->will($this->returnValue('Demo User')); + $this->accountManager->expects($this->any())->method('getUser') + ->with($targetUser) + ->willReturn( + [ + AccountManager::PROPERTY_ADDRESS => ['value' => 'address'], + AccountManager::PROPERTY_PHONE => ['value' => 'phone'], + AccountManager::PROPERTY_TWITTER => ['value' => 'twitter'], + AccountManager::PROPERTY_WEBSITE => ['value' => 'website'], + ] + ); $expected = [ 'enabled' => 'true', 'quota' => ['DummyValue'], 'email' => 'demo@owncloud.org', 'displayname' => 'Demo User', + 'phone' => 'phone', + 'address' => 'address', + 'webpage' => 'website', + 'twitter' => 'twitter' ]; $this->assertEquals($expected, $this->api->getUser('UserToGet')->getData()); } @@ -837,11 +872,25 @@ class UsersControllerTest extends OriginalTest { ->expects($this->once()) ->method('getEMailAddress') ->will($this->returnValue('subadmin@owncloud.org')); + $this->accountManager->expects($this->any())->method('getUser') + ->with($targetUser) + ->willReturn( + [ + AccountManager::PROPERTY_ADDRESS => ['value' => 'address'], + AccountManager::PROPERTY_PHONE => ['value' => 'phone'], + AccountManager::PROPERTY_TWITTER => ['value' => 'twitter'], + AccountManager::PROPERTY_WEBSITE => ['value' => 'website'], + ] + ); $expected = [ 'quota' => ['DummyValue'], 'email' => 'subadmin@owncloud.org', 'displayname' => 'Subadmin User', + 'phone' => 'phone', + 'address' => 'address', + 'webpage' => 'website', + 'twitter' => 'twitter' ]; $this->assertEquals($expected, $this->api->getUser('subadmin')->getData()); } From 5086335643b6181284ee50f57b95525002842992 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 24 Jan 2017 15:45:55 +0100 Subject: [PATCH 2/2] unify endpoints form core and the the provisioning api Signed-off-by: Bjoern Schiessle --- apps/provisioning_api/appinfo/routes.php | 1 + .../lib/Controller/UsersController.php | 25 ++++++ .../tests/Controller/UsersControllerTest.php | 88 ++++++++++++++++++- core/Controller/OCSController.php | 14 --- core/routes.php | 1 - tests/Core/Controller/OCSControllerTest.php | 18 ---- 6 files changed, 112 insertions(+), 35 deletions(-) diff --git a/apps/provisioning_api/appinfo/routes.php b/apps/provisioning_api/appinfo/routes.php index 04a34fba90..baa4e475be 100644 --- a/apps/provisioning_api/appinfo/routes.php +++ b/apps/provisioning_api/appinfo/routes.php @@ -45,6 +45,7 @@ return [ ['root' => '/cloud', 'name' => 'Users#getUsers', 'url' => '/users', 'verb' => 'GET'], ['root' => '/cloud', 'name' => 'Users#addUser', 'url' => '/users', 'verb' => 'POST'], ['root' => '/cloud', 'name' => 'Users#getUser', 'url' => '/users/{userId}', 'verb' => 'GET'], + ['root' => '/cloud', 'name' => 'Users#getCurrentUser', 'url' => '/user', 'verb' => 'GET'], ['root' => '/cloud', 'name' => 'Users#editUser', 'url' => '/users/{userId}', 'verb' => 'PUT'], ['root' => '/cloud', 'name' => 'Users#deleteUser', 'url' => '/users/{userId}', 'verb' => 'DELETE'], ['root' => '/cloud', 'name' => 'Users#enableUser', 'url' => '/users/{userId}/enable', 'verb' => 'PUT'], diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index e659b49443..45839cf4f8 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -218,6 +218,7 @@ class UsersController extends OCSController { $userAccount = $this->accountManager->getUser($targetUserObject); // Find the data + $data['id'] = $targetUserObject->getUID(); $data['quota'] = $this->fillStorageInfo($userId); $data['email'] = $targetUserObject->getEMailAddress(); $data['displayname'] = $targetUserObject->getDisplayName(); @@ -229,6 +230,30 @@ class UsersController extends OCSController { return new DataResponse($data); } + /** + * @NoAdminRequired + * @NoSubAdminRequired + * + * gets user info from the currently logged in user + * + * @return DataResponse + * @throws OCSException + */ + public function getCurrentUser() { + $user = $this->userSession->getUser(); + if ($user) { + $result = $this->getUser($user->getUID()); + // rename "displayname" to "display-name" only for this call to keep + // the API stable. + $result['display-name'] = $result['displayname']; + unset($result['displayname']); + return $result; + + } + + throw new OCSException('', \OCP\API::RESPOND_UNAUTHORISED); + } + /** * @NoAdminRequired * @NoSubAdminRequired diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index 1565407b83..52f7f391c9 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -33,6 +33,7 @@ use OC\Accounts\AccountManager; use OCA\Provisioning_API\Controller\UsersController; use OCP\AppFramework\Http\DataResponse; use OCP\IGroup; +use OCP\IRequest; use OCP\IUser; use OCP\IUserManager; use OCP\IConfig; @@ -57,6 +58,8 @@ class UsersControllerTest extends OriginalTest { protected $api; /** @var AccountManager | PHPUnit_Framework_MockObject_MockObject */ protected $accountManager; + /** @var IRequest | PHPUnit_Framework_MockObject_MockObject */ + protected $request; protected function tearDown() { parent::tearDown(); @@ -80,7 +83,7 @@ class UsersControllerTest extends OriginalTest { $this->logger = $this->getMockBuilder('OCP\ILogger') ->disableOriginalConstructor() ->getMock(); - $request = $this->getMockBuilder('OCP\IRequest') + $this->request = $this->getMockBuilder('OCP\IRequest') ->disableOriginalConstructor() ->getMock(); $this->accountManager = $this->getMockBuilder(AccountManager::class) @@ -89,7 +92,7 @@ class UsersControllerTest extends OriginalTest { $this->api = $this->getMockBuilder('OCA\Provisioning_API\Controller\UsersController') ->setConstructorArgs([ 'provisioning_api', - $request, + $this->request, $this->userManager, $this->config, $this->groupManager, @@ -683,8 +686,13 @@ class UsersControllerTest extends OriginalTest { ->expects($this->once()) ->method('getDisplayName') ->will($this->returnValue('Demo User')); + $targetUser + ->expects($this->once()) + ->method('getUID') + ->will($this->returnValue('UID')); $expected = [ + 'id' => 'UID', 'enabled' => 'true', 'quota' => ['DummyValue'], 'email' => 'demo@owncloud.org', @@ -752,6 +760,10 @@ class UsersControllerTest extends OriginalTest { ->expects($this->once()) ->method('getDisplayName') ->will($this->returnValue('Demo User')); + $targetUser + ->expects($this->once()) + ->method('getUID') + ->will($this->returnValue('UID')); $this->accountManager->expects($this->any())->method('getUser') ->with($targetUser) ->willReturn( @@ -764,6 +776,7 @@ class UsersControllerTest extends OriginalTest { ); $expected = [ + 'id' => 'UID', 'enabled' => 'true', 'quota' => ['DummyValue'], 'email' => 'demo@owncloud.org', @@ -872,6 +885,10 @@ class UsersControllerTest extends OriginalTest { ->expects($this->once()) ->method('getEMailAddress') ->will($this->returnValue('subadmin@owncloud.org')); + $targetUser + ->expects($this->once()) + ->method('getUID') + ->will($this->returnValue('UID')); $this->accountManager->expects($this->any())->method('getUser') ->with($targetUser) ->willReturn( @@ -884,6 +901,7 @@ class UsersControllerTest extends OriginalTest { ); $expected = [ + 'id' => 'UID', 'quota' => ['DummyValue'], 'email' => 'subadmin@owncloud.org', 'displayname' => 'Subadmin User', @@ -2534,4 +2552,70 @@ class UsersControllerTest extends OriginalTest { $this->assertEquals([], $this->api->disableUser('RequestedUser')->getData()); } + + public function testGetCurrentUserLoggedIn() { + + $user = $this->getMock(IUser::class); + $user->expects($this->once())->method('getUID')->willReturn('UID'); + + $this->userSession->expects($this->once())->method('getUser') + ->willReturn($user); + + /** @var UsersController | PHPUnit_Framework_MockObject_MockObject $api */ + $api = $this->getMockBuilder('OCA\Provisioning_API\Controller\UsersController') + ->setConstructorArgs([ + 'provisioning_api', + $this->request, + $this->userManager, + $this->config, + $this->groupManager, + $this->userSession, + $this->accountManager, + $this->logger, + ]) + ->setMethods(['getUser']) + ->getMock(); + + $api->expects($this->once())->method('getUser')->with('UID') + ->willReturn( + [ + 'id' => 'UID', + 'enabled' => 'true', + 'quota' => ['DummyValue'], + 'email' => 'demo@owncloud.org', + 'displayname' => 'Demo User', + 'phone' => 'phone', + 'address' => 'address', + 'webpage' => 'website', + 'twitter' => 'twitter' + ] + ); + + $expected = [ + 'id' => 'UID', + 'enabled' => 'true', + 'quota' => ['DummyValue'], + 'email' => 'demo@owncloud.org', + 'phone' => 'phone', + 'address' => 'address', + 'webpage' => 'website', + 'twitter' => 'twitter', + 'display-name' => 'Demo User' + ]; + + $this->assertSame($expected, $api->getCurrentUser()); + } + + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + */ + public function testGetCurrentUserNotLoggedIn() { + + $this->userSession->expects($this->once())->method('getUser') + ->willReturn(null); + + $this->api->getCurrentUser(); + } + + } diff --git a/core/Controller/OCSController.php b/core/Controller/OCSController.php index dc9775f260..1deb5e958b 100644 --- a/core/Controller/OCSController.php +++ b/core/Controller/OCSController.php @@ -105,20 +105,6 @@ class OCSController extends \OCP\AppFramework\OCSController { return new DataResponse($result); } - /** - * @NoAdminRequired - * @return DataResponse - */ - public function getCurrentUser() { - $userObject = $this->userSession->getUser(); - $data = [ - 'id' => $userObject->getUID(), - 'display-name' => $userObject->getDisplayName(), - 'email' => $userObject->getEMailAddress(), - ]; - return new DataResponse($data); - } - /** * @PublicPage * diff --git a/core/routes.php b/core/routes.php index 6f1892d19a..5d61d58e03 100644 --- a/core/routes.php +++ b/core/routes.php @@ -59,7 +59,6 @@ $application->registerRoutes($this, [ ], 'ocs' => [ ['root' => '/cloud', 'name' => 'OCS#getCapabilities', 'url' => '/capabilities', 'verb' => 'GET'], - ['root' => '/cloud', 'name' => 'OCS#getCurrentUser', 'url' => '/user', 'verb' => 'GET'], ['root' => '', 'name' => 'OCS#getConfig', 'url' => '/config', 'verb' => 'GET'], ['root' => '/person', 'name' => 'OCS#personCheck', 'url' => '/check', 'verb' => 'POST'], ['root' => '/identityproof', 'name' => 'OCS#getIdentityProof', 'url' => '/key/{cloudId}', 'verb' => 'GET'], diff --git a/tests/Core/Controller/OCSControllerTest.php b/tests/Core/Controller/OCSControllerTest.php index 6c47521786..7241df9317 100644 --- a/tests/Core/Controller/OCSControllerTest.php +++ b/tests/Core/Controller/OCSControllerTest.php @@ -116,24 +116,6 @@ class OCSControllerTest extends TestCase { $this->assertEquals($expected, $this->controller->getCapabilities()); } - public function testGetCurrentUser() { - $user = $this->createMock(IUser::class); - $user->method('getUID')->willReturn('uid'); - $user->method('getDisplayName')->willReturn('displayName'); - $user->method('getEMailAddress')->willReturn('e@mail.com'); - - - $this->userSession->method('getUser') - ->willReturn($user); - - $expected = new DataResponse([ - 'id' => 'uid', - 'display-name' => 'displayName', - 'email' => 'e@mail.com', - ]); - $this->assertEquals($expected, $this->controller->getCurrentUser()); - } - public function testPersonCheckValid() { $this->request->method('getRemoteAddress') ->willReturn('1.2.3.4');