From 40584f1dee6010ff8a6786d58cc3c2428ade23d5 Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Fri, 18 Aug 2023 10:09:13 -0100 Subject: [PATCH 1/2] admin have no special rights on users' entries Signed-off-by: Maxence Lange --- .../lib/Controller/AjaxController.php | 2 +- .../tests/Controller/AjaxControllerTest.php | 34 +++++-------------- 2 files changed, 9 insertions(+), 27 deletions(-) diff --git a/apps/files_external/lib/Controller/AjaxController.php b/apps/files_external/lib/Controller/AjaxController.php index db23ecd709d63..e41a75a62bc09 100644 --- a/apps/files_external/lib/Controller/AjaxController.php +++ b/apps/files_external/lib/Controller/AjaxController.php @@ -108,7 +108,7 @@ public function saveGlobalCredentials($uid, $user, $password) { $currentUser = $this->userSession->getUser(); // Non-admins can only edit their own credentials - $allowedToEdit = ($this->groupManager->isAdmin($currentUser->getUID()) || $currentUser->getUID() === $uid); + $allowedToEdit = ($currentUser->getUID() === $uid); if ($allowedToEdit) { $this->globalAuth->saveAuth($uid, $user, $password); diff --git a/apps/files_external/tests/Controller/AjaxControllerTest.php b/apps/files_external/tests/Controller/AjaxControllerTest.php index 2ddd64f0e073a..88efe9b1c04d4 100644 --- a/apps/files_external/tests/Controller/AjaxControllerTest.php +++ b/apps/files_external/tests/Controller/AjaxControllerTest.php @@ -102,17 +102,11 @@ public function testSaveGlobalCredentialsAsAdminForAnotherUser() { ->expects($this->once()) ->method('getUser') ->willReturn($user); - $this->groupManager - ->expects($this->once()) - ->method('isAdmin') - ->with('MyAdminUid') - ->willReturn(true); $this->globalAuth - ->expects($this->once()) - ->method('saveAuth') - ->with('UidOfTestUser', 'test', 'password'); + ->expects($this->never()) + ->method('saveAuth'); - $this->assertSame(true, $this->ajaxController->saveGlobalCredentials('UidOfTestUser', 'test', 'password')); + $this->assertSame(false, $this->ajaxController->saveGlobalCredentials('UidOfTestUser', 'test', 'password')); } public function testSaveGlobalCredentialsAsAdminForSelf() { @@ -125,11 +119,6 @@ public function testSaveGlobalCredentialsAsAdminForSelf() { ->expects($this->once()) ->method('getUser') ->willReturn($user); - $this->groupManager - ->expects($this->once()) - ->method('isAdmin') - ->with('MyAdminUid') - ->willReturn(true); $this->globalAuth ->expects($this->once()) ->method('saveAuth') @@ -141,18 +130,13 @@ public function testSaveGlobalCredentialsAsAdminForSelf() { public function testSaveGlobalCredentialsAsNormalUserForSelf() { $user = $this->createMock(IUser::class); $user - ->expects($this->exactly(2)) + ->expects($this->once()) ->method('getUID') ->willReturn('MyUserUid'); $this->userSession ->expects($this->once()) ->method('getUser') ->willReturn($user); - $this->groupManager - ->expects($this->once()) - ->method('isAdmin') - ->with('MyUserUid') - ->willReturn(false); $this->globalAuth ->expects($this->once()) ->method('saveAuth') @@ -164,18 +148,16 @@ public function testSaveGlobalCredentialsAsNormalUserForSelf() { public function testSaveGlobalCredentialsAsNormalUserForAnotherUser() { $user = $this->createMock(IUser::class); $user - ->expects($this->exactly(2)) + ->expects($this->once()) ->method('getUID') ->willReturn('MyUserUid'); $this->userSession ->expects($this->once()) ->method('getUser') ->willReturn($user); - $this->groupManager - ->expects($this->once()) - ->method('isAdmin') - ->with('MyUserUid') - ->willReturn(false); + $this->globalAuth + ->expects($this->never()) + ->method('saveAuth'); $this->assertSame(false, $this->ajaxController->saveGlobalCredentials('AnotherUserUid', 'test', 'password')); } From a969cfad8ac55a9818ca1767ae1446bc8eeb2d01 Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Fri, 18 Aug 2023 10:17:48 -0100 Subject: [PATCH 2/2] syntactic migration Signed-off-by: Maxence Lange --- .../lib/Controller/AjaxController.php | 62 +++++++------------ 1 file changed, 22 insertions(+), 40 deletions(-) diff --git a/apps/files_external/lib/Controller/AjaxController.php b/apps/files_external/lib/Controller/AjaxController.php index e41a75a62bc09..ce83fbccbe02f 100644 --- a/apps/files_external/lib/Controller/AjaxController.php +++ b/apps/files_external/lib/Controller/AjaxController.php @@ -1,10 +1,14 @@ * @author Lukas Reschke * @author Martin Mattel + * @author Maxence Lange * @author Morris Jobke * @author Robin Appelman * @author Robin McCorkell @@ -26,52 +30,32 @@ * along with this program. If not, see * */ + namespace OCA\Files_External\Controller; use OCA\Files_External\Lib\Auth\Password\GlobalAuth; use OCA\Files_External\Lib\Auth\PublicKey\RSA; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\JSONResponse; -use OCP\IGroupManager; use OCP\IRequest; use OCP\IUserSession; class AjaxController extends Controller { - /** @var RSA */ - private $rsaMechanism; - /** @var GlobalAuth */ - private $globalAuth; - /** @var IUserSession */ - private $userSession; - /** @var IGroupManager */ - private $groupManager; - - /** - * @param string $appName - * @param IRequest $request - * @param RSA $rsaMechanism - * @param GlobalAuth $globalAuth - * @param IUserSession $userSession - * @param IGroupManager $groupManager - */ - public function __construct($appName, - IRequest $request, - RSA $rsaMechanism, - GlobalAuth $globalAuth, - IUserSession $userSession, - IGroupManager $groupManager) { + public function __construct( + string $appName, + IRequest $request, + private RSA $rsaMechanism, + private GlobalAuth $globalAuth, + private IUserSession $userSession + ) { parent::__construct($appName, $request); - $this->rsaMechanism = $rsaMechanism; - $this->globalAuth = $globalAuth; - $this->userSession = $userSession; - $this->groupManager = $groupManager; } /** * @param int $keyLength * @return array */ - private function generateSshKeys($keyLength) { + private function generateSshKeys(int $keyLength): array { $key = $this->rsaMechanism->createKey($keyLength); // Replace the placeholder label with a more meaningful one $key['publickey'] = str_replace('phpseclib-generated-key', gethostname(), $key['publickey']); @@ -83,9 +67,11 @@ private function generateSshKeys($keyLength) { * Generates an SSH public/private key pair. * * @NoAdminRequired + * * @param int $keyLength + * @return JSONResponse */ - public function getSshKeys($keyLength = 1024) { + public function getSshKeys(int $keyLength = 1024): JSONResponse { $key = $this->generateSshKeys($keyLength); return new JSONResponse( ['data' => [ @@ -104,17 +90,13 @@ public function getSshKeys($keyLength = 1024) { * @param string $password * @return bool */ - public function saveGlobalCredentials($uid, $user, $password) { - $currentUser = $this->userSession->getUser(); - - // Non-admins can only edit their own credentials - $allowedToEdit = ($currentUser->getUID() === $uid); - - if ($allowedToEdit) { - $this->globalAuth->saveAuth($uid, $user, $password); - return true; - } else { + public function saveGlobalCredentials(string $uid, string $user, string $password): bool { + if ($this->userSession->getUser()->getUID() !== $uid) { return false; } + + $this->globalAuth->saveAuth($uid, $user, $password); + + return true; } }