Skip to content

Commit

Permalink
feat(users): Store and load a user's manager
Browse files Browse the repository at this point in the history
Co-Authored-By: hamza221 <hamzamahjoubi221@gmail.com>
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
  • Loading branch information
ChristophWurst and hamza221 committed May 12, 2023
1 parent 1399c88 commit 1381c4c
Show file tree
Hide file tree
Showing 19 changed files with 270 additions and 19 deletions.
20 changes: 19 additions & 1 deletion apps/dav/lib/CardDAV/Converter.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,19 @@
use OCP\Accounts\IAccountManager;
use OCP\IImage;
use OCP\IUser;
use OCP\IUserManager;
use Sabre\VObject\Component\VCard;
use Sabre\VObject\Property\Text;

class Converter {
/** @var IAccountManager */
private $accountManager;
private IUserManager $userManager;

public function __construct(IAccountManager $accountManager) {
public function __construct(IAccountManager $accountManager,
IUserManager $userManager) {
$this->accountManager = $accountManager;
$this->userManager = $userManager;
}

public function createCardFromUser(IUser $user): ?VCard {
Expand Down Expand Up @@ -102,6 +106,20 @@ public function createCardFromUser(IUser $user): ?VCard {
}
}

// Local properties
$managers = $user->getManagerUids();
// X-MANAGERSNAME only allows a single value, so we take the first manager
if (isset($managers[0])) {
$displayName = $this->userManager->getDisplayName($managers[0]);
// Only set the manager if a user object is found
if ($displayName !== null) {
$vCard->add(new Text($vCard, 'X-MANAGERSNAME', $displayName, [
'uid' => $managers[0],
'X-NC-SCOPE' => IAccountManager::SCOPE_LOCAL,
]));
}
}

if ($publish && !empty($cloudId)) {
$vCard->add(new Text($vCard, 'CLOUD', $cloudId));
$vCard->validate();
Expand Down
34 changes: 32 additions & 2 deletions apps/dav/tests/unit/CardDAV/ConverterTest.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
<?php

declare(strict_types=1);

/**
* @copyright Copyright (c) 2016, ownCloud, Inc.
*
Expand Down Expand Up @@ -33,18 +36,22 @@
use OCP\Accounts\IAccountProperty;
use OCP\IImage;
use OCP\IUser;
use OCP\IUserManager;
use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase;

class ConverterTest extends TestCase {

/** @var IAccountManager|\PHPUnit\Framework\MockObject\MockObject */
private $accountManager;
/** @var IUserManager|(IUserManager&MockObject)|MockObject */
private IUserManager|MockObject $userManager;

protected function setUp(): void {
parent::setUp();

$this->accountManager = $this->createMock(IAccountManager::class);
$this->userManager = $this->createMock(IUserManager::class);
}

/**
Expand Down Expand Up @@ -96,7 +103,7 @@ public function testCreation($expectedVCard, $displayName = null, $eMailAddress
$user = $this->getUserMock((string)$displayName, $eMailAddress, $cloudId);
$accountManager = $this->getAccountManager($user);

$converter = new Converter($accountManager);
$converter = new Converter($accountManager, $this->userManager);
$vCard = $converter->createCardFromUser($user);
if ($expectedVCard !== null) {
$this->assertInstanceOf('Sabre\VObject\Component\VCard', $vCard);
Expand All @@ -107,6 +114,29 @@ public function testCreation($expectedVCard, $displayName = null, $eMailAddress
}
}

public function testManagerProp(): void {
$user = $this->getUserMock("user", "user@domain.tld", "user@cloud.domain.tld");
$user->method('getManagerUids')
->willReturn(['mgr']);
$this->userManager->expects(self::once())
->method('getDisplayName')
->with('mgr')
->willReturn('Manager');
$accountManager = $this->getAccountManager($user);

$converter = new Converter($accountManager, $this->userManager);
$vCard = $converter->createCardFromUser($user);

$this->compareData(
[
'cloud' => 'user@cloud.domain.tld',
'email' => 'user@domain.tld',
'x-managersname' => 'Manager',
],
$vCard->jsonSerialize()
);
}

protected function compareData($expected, $data) {
foreach ($expected as $key => $value) {
$found = false;
Expand Down Expand Up @@ -182,7 +212,7 @@ public function providesNewUsers() {
* @param $fullName
*/
public function testNameSplitter($expected, $fullName): void {
$converter = new Converter($this->accountManager);
$converter = new Converter($this->accountManager, $this->userManager);
$r = $converter->splitFullName($fullName);
$r = implode(';', $r);
$this->assertEquals($expected, $r);
Expand Down
3 changes: 3 additions & 0 deletions apps/provisioning_api/lib/Controller/AUserData.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ abstract class AUserData extends OCSController {
public const USER_FIELD_LOCALE = 'locale';
public const USER_FIELD_PASSWORD = 'password';
public const USER_FIELD_QUOTA = 'quota';
public const USER_FIELD_MANAGER = 'manager';
public const USER_FIELD_NOTIFICATION_EMAIL = 'notify_email';

/** @var IUserManager */
Expand Down Expand Up @@ -151,6 +152,8 @@ protected function getUserData(string $userId, bool $includeScopes = false): arr
$data['backend'] = $targetUserObject->getBackendClassName();
$data['subadmin'] = $this->getUserSubAdminGroupsData($targetUserObject->getUID());
$data[self::USER_FIELD_QUOTA] = $this->fillStorageInfo($targetUserObject->getUID());
$managerUids = $targetUserObject->getManagerUids();
$data[self::USER_FIELD_MANAGER] = empty($managerUids) ? '' : $managerUids[0];

try {
if ($includeScopes) {
Expand Down
20 changes: 18 additions & 2 deletions apps/provisioning_api/lib/Controller/UsersController.php
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,8 @@ public function addUser(
array $groups = [],
array $subadmin = [],
string $quota = '',
string $language = ''
string $language = '',
?string $manager = null,
): DataResponse {
$user = $this->userSession->getUser();
$isAdmin = $this->groupManager->isAdmin($user->getUID());
Expand Down Expand Up @@ -447,6 +448,15 @@ public function addUser(
$this->editUser($userid, self::USER_FIELD_LANGUAGE, $language);
}

/**
* null -> nothing sent
* '' -> unset manager
* else -> set manager
*/
if ($manager !== null) {
$this->editUser($userid, self::USER_FIELD_MANAGER, $manager);
}

// Send new user mail only if a mail is set
if ($email !== '') {
$newUser->setEMailAddress($email);
Expand Down Expand Up @@ -800,9 +810,11 @@ public function editUser(string $userId, string $key, string $value): DataRespon

$permittedFields[] = IAccountManager::PROPERTY_AVATAR . self::SCOPE_SUFFIX;

// If admin they can edit their own quota
// If admin they can edit their own quota and manager
if ($this->groupManager->isAdmin($currentLoggedInUser->getUID())) {
$permittedFields[] = self::USER_FIELD_QUOTA;
$permittedFields[] = self::USER_FIELD_MANAGER;

}
} else {
// Check if admin / subadmin
Expand Down Expand Up @@ -836,6 +848,7 @@ public function editUser(string $userId, string $key, string $value): DataRespon
$permittedFields[] = IAccountManager::PROPERTY_PROFILE_ENABLED;
$permittedFields[] = self::USER_FIELD_QUOTA;
$permittedFields[] = self::USER_FIELD_NOTIFICATION_EMAIL;
$permittedFields[] = self::USER_FIELD_MANAGER;
} else {
// No rights
throw new OCSException('', OCSController::RESPOND_NOT_FOUND);
Expand Down Expand Up @@ -885,6 +898,9 @@ public function editUser(string $userId, string $key, string $value): DataRespon
}
$targetUser->setQuota($quota);
break;
case self::USER_FIELD_MANAGER:
$targetUser->setManagerUids([$value]);
break;
case self::USER_FIELD_PASSWORD:
try {
if (strlen($value) > IUserManager::MAX_PASSWORD_LENGTH) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1093,6 +1093,7 @@ public function testGetUserDataAsAdmin() {
'biography' => 'biography',
'profile_enabled' => '1',
'notify_email' => null,
'manager' => '',
];
$this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['UID']));
}
Expand Down Expand Up @@ -1233,6 +1234,7 @@ public function testGetUserDataAsSubAdminAndUserIsAccessible() {
'biography' => 'biography',
'profile_enabled' => '1',
'notify_email' => null,
'manager' => '',
];
$this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['UID']));
}
Expand Down Expand Up @@ -1411,6 +1413,7 @@ public function testGetUserDataAsSubAdminSelfLookup() {
'biography' => 'biography',
'profile_enabled' => '1',
'notify_email' => null,
'manager' => '',
];
$this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['UID']));
}
Expand Down
2 changes: 1 addition & 1 deletion apps/settings/css/settings.css

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion apps/settings/css/settings.css.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions apps/settings/css/settings.scss
Original file line number Diff line number Diff line change
Expand Up @@ -1350,6 +1350,8 @@ doesnotexist:-o-prefocus, .strengthify-wrapper {
minmax($grid-col-min-width, 1fr) // email
minmax(1.5*$grid-col-min-width, 1fr) // groups
minmax(1.5*$grid-col-min-width, 1fr) // group admins
minmax($grid-col-min-width, 1fr) // quota
minmax(1.5*$grid-col-min-width, 1fr) // manager
repeat(auto-fit, minmax($grid-col-min-width, 1fr));
border-bottom: var(--color-border) 1px solid;

Expand Down Expand Up @@ -1394,6 +1396,7 @@ doesnotexist:-o-prefocus, .strengthify-wrapper {
}
}

.managers,
.groups,
.subadmins,
.quota {
Expand Down
Loading

0 comments on commit 1381c4c

Please sign in to comment.