Skip to content

Commit

Permalink
Only expose storage location to admins
Browse files Browse the repository at this point in the history
Co-authored-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com>
Signed-off-by: Vincent Petry <vincent@nextcloud.com>
  • Loading branch information
PVince81 and come-nc committed Jan 13, 2023
1 parent b88864b commit c24884d
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 17 deletions.
21 changes: 12 additions & 9 deletions apps/provisioning_api/lib/Controller/AUserData.php
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ public function __construct(string $appName,
*/
protected function getUserData(string $userId, bool $includeScopes = false): array {
$currentLoggedInUser = $this->userSession->getUser();
assert($currentLoggedInUser !== null, 'No user logged in');

$data = [];

Expand All @@ -113,8 +114,8 @@ protected function getUserData(string $userId, bool $includeScopes = false): arr
throw new OCSNotFoundException('User does not exist');
}

// Should be at least Admin Or SubAdmin!
if ($this->groupManager->isAdmin($currentLoggedInUser->getUID())
$isAdmin = $this->groupManager->isAdmin($currentLoggedInUser->getUID());
if ($isAdmin
|| $this->groupManager->getSubAdmin()->isUserAccessible($currentLoggedInUser, $targetUserObject)) {
$data['enabled'] = $this->config->getUserValue($targetUserObject->getUID(), 'core', 'enabled', 'true') === 'true';
} else {
Expand All @@ -132,13 +133,15 @@ protected function getUserData(string $userId, bool $includeScopes = false): arr
$gids[] = $group->getGID();
}

try {
# might be thrown by LDAP due to handling of users disappears
# from the external source (reasons unknown to us)
# cf. https://github.com/nextcloud/server/issues/12991
$data['storageLocation'] = $targetUserObject->getHome();
} catch (NoUserException $e) {
throw new OCSNotFoundException($e->getMessage(), $e);
if ($isAdmin) {
try {
# might be thrown by LDAP due to handling of users disappears
# from the external source (reasons unknown to us)
# cf. https://github.com/nextcloud/server/issues/12991
$data['storageLocation'] = $targetUserObject->getHome();
} catch (NoUserException $e) {
throw new OCSNotFoundException($e->getMessage(), $e);
}
}

// Find the data
Expand Down
12 changes: 4 additions & 8 deletions apps/provisioning_api/tests/Controller/UsersControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1165,9 +1165,8 @@ public function testGetUserDataAsSubAdminAndUserIsAccessible() {
->method('getDisplayName')
->willReturn('Demo User');
$targetUser
->expects($this->once())
->method('getHome')
->willReturn('/var/www/newtcloud/data/UID');
->expects($this->never())
->method('getHome');
$targetUser
->expects($this->once())
->method('getLastLogin')
Expand Down Expand Up @@ -1206,7 +1205,6 @@ public function testGetUserDataAsSubAdminAndUserIsAccessible() {
$expected = [
'id' => 'UID',
'enabled' => true,
'storageLocation' => '/var/www/newtcloud/data/UID',
'lastLogin' => 1521191471000,
'backend' => 'Database',
'subadmin' => [],
Expand Down Expand Up @@ -1349,9 +1347,8 @@ public function testGetUserDataAsSubAdminSelfLookup() {
->method('getUID')
->willReturn('UID');
$targetUser
->expects($this->once())
->method('getHome')
->willReturn('/var/www/newtcloud/data/UID');
->expects($this->never())
->method('getHome');
$targetUser
->expects($this->once())
->method('getLastLogin')
Expand Down Expand Up @@ -1385,7 +1382,6 @@ public function testGetUserDataAsSubAdminSelfLookup() {

$expected = [
'id' => 'UID',
'storageLocation' => '/var/www/newtcloud/data/UID',
'lastLogin' => 1521191471000,
'backend' => 'Database',
'subadmin' => [],
Expand Down

2 comments on commit c24884d

@mvalois
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there any reason for this? Users shall not retrieve their own storageLocation value?

@PVince81
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, security purposes

regular users should not know about the underlying installation path as it's internal information

Please sign in to comment.