From 930a0a43aa844a2e55889d175af7535f9375a4c2 Mon Sep 17 00:00:00 2001 From: dartcafe Date: Sun, 22 Mar 2026 13:50:17 +0100 Subject: [PATCH 1/5] Shares refactoring and optimizations Signed-off-by: dartcafe --- lib/Command/Share/Add.php | 10 +- lib/Command/Share/Remove.php | 14 +- lib/Controller/PublicController.php | 15 ++ lib/Db/Comment.php | 2 +- lib/Db/EntityWithUser.php | 23 ++- lib/Db/Poll.php | 7 + lib/Db/Share.php | 59 +++--- lib/Db/UserMapper.php | 93 ++++----- lib/Helper/NameGenerator.php | 85 +++++++++ lib/Model/Mail/MailBase.php | 2 +- lib/Model/User/Admin.php | 5 - lib/Model/User/Anon.php | 9 +- lib/Model/User/Contact.php | 18 +- lib/Model/User/Ghost.php | 11 +- lib/Model/UserBase.php | 54 ++---- lib/Notification/Notifier.php | 14 +- lib/Provider/ActivityProvider.php | 24 ++- lib/Service/ShareService.php | 283 ++++++++++++++-------------- lib/Service/SystemService.php | 22 ++- src/components/Shares/ShareItem.vue | 18 -- src/components/Shares/ShareMenu.vue | 13 +- src/components/User/UserBubble.vue | 2 +- src/components/User/UserItem.vue | 6 +- tests/Unit/Db/UserMapperTest.php | 48 +++-- 24 files changed, 491 insertions(+), 346 deletions(-) create mode 100644 lib/Helper/NameGenerator.php diff --git a/lib/Command/Share/Add.php b/lib/Command/Share/Add.php index 87f7124dd2..10c6daca8f 100644 --- a/lib/Command/Share/Add.php +++ b/lib/Command/Share/Add.php @@ -12,9 +12,7 @@ use OCA\Polls\AppInfo\Application; use OCA\Polls\Db\Poll; use OCA\Polls\Exceptions\ShareAlreadyExistsException; -use OCA\Polls\Model\Group\Group; -use OCA\Polls\Model\User\Email; -use OCA\Polls\Model\User\User; +use OCA\Polls\Model\UserBase; use OCP\AppFramework\Db\DoesNotExistException; use Stecman\Component\Symfony\Console\BashCompletion\CompletionContext; use Symfony\Component\Console\Input\InputArgument; @@ -86,7 +84,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int private function inviteUsers(Poll $poll, array $userIds): void { foreach ($userIds as $userId) { try { - $share = $this->shareService->add($poll->getId(), User::TYPE, $userId); + $share = $this->shareService->add($poll->getId(), UserBase::TYPE_USER, $userId); $this->shareService->sendInvitation($share); } catch (ShareAlreadyExistsException $e) { // silently ignore already existing shares @@ -102,7 +100,7 @@ private function inviteUsers(Poll $poll, array $userIds): void { private function inviteGroups(Poll $poll, array $groupIds): void { foreach ($groupIds as $groupId) { try { - $share = $this->shareService->add($poll->getId(), Group::TYPE, $groupId); + $share = $this->shareService->add($poll->getId(), UserBase::TYPE_GROUP, $groupId); $this->shareService->sendInvitation($share); } catch (ShareAlreadyExistsException $e) { // silently ignore already existing shares @@ -118,7 +116,7 @@ private function inviteGroups(Poll $poll, array $groupIds): void { private function inviteEmails(Poll $poll, array $emails): void { foreach ($emails as $email) { try { - $share = $this->shareService->add($poll->getId(), Email::TYPE, $email); + $share = $this->shareService->add($poll->getId(), UserBase::TYPE_EMAIL, $email); $this->shareService->sendInvitation($share); } catch (ShareAlreadyExistsException $e) { // silently ignore already existing shares diff --git a/lib/Command/Share/Remove.php b/lib/Command/Share/Remove.php index ac4120cf3a..422dde3042 100644 --- a/lib/Command/Share/Remove.php +++ b/lib/Command/Share/Remove.php @@ -12,11 +12,7 @@ use OCA\Polls\AppInfo\Application; use OCA\Polls\Db\Poll; use OCA\Polls\Db\Share; -use OCA\Polls\Model\Group\Group; -use OCA\Polls\Model\User\Contact; -use OCA\Polls\Model\User\Email; -use OCA\Polls\Model\User\GenericUser; -use OCA\Polls\Model\User\User; +use OCA\Polls\Model\UserBase; use OCP\AppFramework\Db\DoesNotExistException; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -126,7 +122,7 @@ private function removeEmails(Poll $poll, array $emails): void { private function getUserShares(Poll $poll): array { $shares = $this->shareMapper->findByPoll($poll->getId()); return array_values(array_filter($shares, static function (Share $share): bool { - return ($share->getType() === User::TYPE); + return ($share->getType() === UserBase::TYPE_USER); })); } @@ -138,7 +134,7 @@ private function getUserShares(Poll $poll): array { private function getGroupShares(Poll $poll): array { $shares = $this->shareMapper->findByPoll($poll->getId()); return array_values(array_filter($shares, static function (Share $share): bool { - return ($share->getType() === Group::TYPE); + return ($share->getType() === UserBase::TYPE_GROUP); })); } @@ -150,11 +146,11 @@ private function getGroupShares(Poll $poll): array { private function getEmailShares(Poll $poll): array { $shares = $this->shareMapper->findByPoll($poll->getId()); return array_values(array_filter($shares, static function (Share $share): bool { - if (($share->getType() === GenericUser::TYPE) && $share->getEmailAddress()) { + if (($share->getType() === UserBase::TYPE_GENERIC) && $share->getEmailAddress()) { return true; } - return (($share->getType() === Email::TYPE) || ($share->getType() === Contact::TYPE)); + return (($share->getType() === UserBase::TYPE_EMAIL) || ($share->getType() === UserBase::TYPE_CONTACT)); })); } } diff --git a/lib/Controller/PublicController.php b/lib/Controller/PublicController.php index bddda0e157..1799088a46 100644 --- a/lib/Controller/PublicController.php +++ b/lib/Controller/PublicController.php @@ -403,6 +403,21 @@ public function validatePublicDisplayName(string $displayName, string $token): J ]); } + /** + * Validate it the user name is reserved + * return false, if this username already exists as a user or as a participant of the poll + * @param string $token Share token + */ + #[PublicPage] + #[ShareTokenRequired] + #[OpenAPI(OpenAPI::SCOPE_IGNORE)] + #[FrontpageRoute(verb: 'POST', url: '/randomname')] + public function getRandomUserName(string $token): JSONResponse { + return $this->response(fn () => [ + 'name' => $this->systemService->getRandomDisplayName($token) + ]); + } + /** * Validate email address (simple validation) * @param string $emailAddress Email address string to check for validation diff --git a/lib/Db/Comment.php b/lib/Db/Comment.php index 53d8741c8e..165928ace0 100644 --- a/lib/Db/Comment.php +++ b/lib/Db/Comment.php @@ -67,7 +67,7 @@ public function getRecipientUser() : ?UserBase { try { /* @var UserMapper $userMapper */ $userMapper = (Container::queryClass(UserMapper::class)); - $user = $userMapper->getParticipant($this->getRecipient(), $this->getPollId()); + $user = $userMapper->getUser($this->getRecipient(), $this->getPollId()); } catch (Exception $e) { return null; } diff --git a/lib/Db/EntityWithUser.php b/lib/Db/EntityWithUser.php index 449152a91a..072b217d9d 100644 --- a/lib/Db/EntityWithUser.php +++ b/lib/Db/EntityWithUser.php @@ -91,30 +91,29 @@ protected function getEntityAnonymization(): bool { return false; } + /** + * get the pollId, if relevant for user identification, otherwise return null + * Overwrite if user identificaton needs another pollId than the one of the entity + */ + public function getUserRelevantPollId(): ?int { + return $this->getPollId(); + } /** - * @return UserBase Gets owner of the entity + * @return UserBase Gets user of the entity */ public function getUser(): UserBase { if ($this->getEntityAnonymization()) { $user = new Anon($this->getUserId()); return $user; } - /** @var UserMapper */ - $userMapper = (Container::queryClass(UserMapper::class)); + $userMapper = Container::queryClass(UserMapper::class); try { - $pollId = $this->getPollId(); - if ($pollId === null) { - // If pollId is not set, we assume that the user is not a participant of a poll - return $userMapper->getUserFromUserBase($this->getUserId()); - } - $user = $userMapper->getParticipant($this->getUserId(), $pollId); - // Get user from userbase + return $userMapper->getUser($this->getUserId(), $this->getUserRelevantPollId()); } catch (Exception) { - $user = new Ghost($this->getUserId()); + return new Ghost($this->getUserId()); } - return $user; } } diff --git a/lib/Db/Poll.php b/lib/Db/Poll.php index 098b48c9a7..aa722f1e8a 100644 --- a/lib/Db/Poll.php +++ b/lib/Db/Poll.php @@ -516,6 +516,13 @@ public function getUserId(): string { return $this->getOwner(); } + /** + * Return null for user identification for the poll user to support caching + */ + public function getUserRelevantPollId(): ?int { + return null; + } + // alias of setOwner($value) public function setUserId(string $userId): void { $this->setOwner($userId); diff --git a/lib/Db/Share.php b/lib/Db/Share.php index 8592d51b0f..241dc8431e 100644 --- a/lib/Db/Share.php +++ b/lib/Db/Share.php @@ -48,6 +48,8 @@ * @method void setLocked(int $value) * @method string getDisplayName() * @method void setDisplayName(string $value) + * @method string getLabel() + * @method void setLabel(string $value) * @method string getMiscSettings() * @method void setMiscSettings(string $value) * @method ?int getAnonymizedVotes() @@ -63,17 +65,17 @@ class Share extends EntityWithUser implements JsonSerializable { public const EMAIL_DISABLED = 'disabled'; // Only authenticated access - public const TYPE_USER = 'user'; - public const TYPE_ADMIN = 'admin'; - public const TYPE_GROUP = 'group'; + public const TYPE_USER = UserBase::TYPE_USER; + public const TYPE_ADMIN = UserBase::TYPE_ADMIN; + public const TYPE_GROUP = UserBase::TYPE_GROUP; // Public and authenticated Access - public const TYPE_PUBLIC = 'public'; + public const TYPE_PUBLIC = UserBase::TYPE_PUBLIC; // Only public access - public const TYPE_EMAIL = 'email'; - public const TYPE_CONTACT = 'contact'; - public const TYPE_EXTERNAL = 'external'; + public const TYPE_EMAIL = UserBase::TYPE_EMAIL; + public const TYPE_CONTACT = UserBase::TYPE_CONTACT; + public const TYPE_EXTERNAL = UserBase::TYPE_EXTERNAL; // no direct Access public const TYPE_TEAM = 'circle'; @@ -178,7 +180,7 @@ public function jsonSerialize(): array { } public function resolveUser(): Ghost|Group|Team|Contact|ContactGroup|User|Email|GenericUser { - return UserMapper::getUserObject( + return UserMapper::createUserObject( $this->getType(), $this->getUserId(), $this->getDisplayName(), @@ -242,22 +244,13 @@ public function setPublicPollEmail(string $value): void { $this->setMiscSettingsByKey('publicPollEmail', $value); } - /** - * Share label for public shares, now stored as displayName - * TODO: remove after migration was introduced - */ - public function setLabel(string $label): void { - $this->setDisplayName($label); - $this->label = $label; - } - /** * Share label for public shares * TODO: remove after migrating labels to displayName */ public function getLabel(): string { // In case of public poll use label as fallback for displayName - return $this->displayName ?? $this->label ?? ''; + return $this->label ?? $this->displayName ?? ''; } /** @@ -265,12 +258,36 @@ public function getLabel(): string { * TODO: remove public poll check after migration labels to displayName */ public function getDisplayName(): string { - if ($this->getType() === self::TYPE_PUBLIC) { - return $this->getLabel(); - } return $this->displayName ?? ''; } + public function setFromUserObject(UserBase $userGroup, string $token, string $purpose = 'poll'): void { + // Contacts are converted: with email → email share, without email → external share + $type = $userGroup->getType(); + if ($type === self::TYPE_CONTACT || $type === self::TYPE_EMAIL) { + $type = self::TYPE_EXTERNAL; + } + + $this->setType($type); + $this->setDisplayName($userGroup->getDisplayName()); + $this->setEmailAddress($userGroup->getEmailAddress()); + + // ignore if share is for poll Groups + if ($purpose === 'poll') { + if ($type === self::TYPE_PUBLIC) { + // public share to create, set token as userId + $this->setUserId($token); + } else { + $this->setUserId($userGroup->getShareUserId()); + } + if ($type === self::TYPE_EXTERNAL) { + $this->setLanguage($userGroup->getLanguageCode()); + $this->setTimeZoneName($userGroup->getTimeZoneName()); + } + } + } + + public function getTimeZoneName(): string { return $this->getMiscSettingsArray()['timeZone'] ?? ''; } diff --git a/lib/Db/UserMapper.php b/lib/Db/UserMapper.php index 9085101bc1..1de36bc656 100644 --- a/lib/Db/UserMapper.php +++ b/lib/Db/UserMapper.php @@ -36,6 +36,9 @@ class UserMapper extends QBMapper { public const TABLE = Share::TABLE; + /** @var array */ + private static array $userCache = []; + /** @psalm-suppress PossiblyUnusedMethod */ public function __construct( IDBConnection $db, @@ -50,32 +53,43 @@ public function __construct( * Returns a UserBase child from share determined by userId and pollId or from userbase by userId * * @param string $userId - * @param int|string|null $pollId Can only be used together with $userId and will return the internal user or the share user + * @param int|null $pollId Can only be used together with $userId and will return the internal user or the share user * @return UserBase **/ - public function getParticipant(string $userId, int|string|null $pollId): UserBase { - $pollId = is_string($pollId) ? (int)$pollId : $pollId; + public function getUser(string $userId, ?int $pollId = null): UserBase { if ($userId === '') { return new UserBase($userId, UserBase::TYPE_EMPTY); } try { - return $this->getUserFromUserBase($userId, $pollId); - } catch (UserNotFoundException $e) { - // just catch and continue if not found and try to find user by share; + return $this->getCachedUser($userId, $pollId); + } catch (UserNotFoundException) { + // Not found in cache, continue to fetch from userbase or share } try { - if ($pollId === null) { - throw new ShareNotFoundException('PollId is required to get share user'); + $user = $this->getUserFromUserBase($userId); + try { + if ($pollId !== null && $this->getShareByPollAndUser($userId, $pollId)->getType() === Share::TYPE_ADMIN) { + $user = new Admin($userId); + } + } catch (ShareNotFoundException) { + // No admin share found + } + } catch (UserNotFoundException) { + try { + if ($pollId === null) { + throw new ShareNotFoundException('PollId is required to get share user'); + } + $user = $this->getShareByPollAndUser($userId, $pollId)->resolveUser(); + } catch (ShareNotFoundException) { + $user = new Ghost($userId); } - $share = $this->getShareByPollAndUser($userId, $pollId); - return $share->resolveUser(); - } catch (ShareNotFoundException $e) { - // User seems to be probaly deleted, use fake share } - return new Ghost($userId); + $cacheKey = $userId . ':' . ($pollId ?? 'null'); + self::$userCache[$cacheKey] = $user; + return $user; } /** @@ -88,40 +102,35 @@ public function getParticipants(int $pollId): array { $participants = $this->findParticipantsByPoll($pollId); foreach ($participants as &$participant) { - $users[] = $this->getParticipant($participant->getUserId(), $pollId); + $users[] = $this->getUser($participant->getUserId(), $pollId); } return $users; } /** - * Get a user from the userbase - * - * Returns a User child build from the userId and - * if pollId is given, it will check if the user has admin rights for the poll + * Get a user from the NC userbase * * @param string $userId - * @param int|null $pollId * @return User * @throws UserNotFoundException */ - public function getUserFromUserBase(string $userId, ?int $pollId = null): User { + public function getUserFromUserBase(string $userId): User { $user = $this->userManager->get($userId); + if (!$user instanceof IUser) { + throw new UserNotFoundException(); + } + return new User($userId); + } - if ($user instanceof IUser) { - try { - // check if we find a share, where the user got admin rights for the particular poll - if ($pollId !== null && $this->getShareByPollAndUser($userId, $pollId)->getType() === Share::TYPE_ADMIN) { - return new Admin($userId); - } - } catch (ShareNotFoundException $e) { - // No share found, user has no admin delegation granted by share - // silent catch - } - return new User($userId); + private function getCachedUser(string $userId, ?int $pollId = null): UserBase { + $cacheKey = $userId . ':' . ($pollId ?? 'null'); + if (isset(self::$userCache[$cacheKey])) { + return self::$userCache[$cacheKey]; } - throw new UserNotFoundException(); + throw new UserNotFoundException('User not found in cache'); } + public function getUserFromShareToken(string $token): UserBase { $share = $this->getShareByToken($token); @@ -154,7 +163,7 @@ private function getShareByPollAndUser(string $userId, int $pollId): Share { try { return $this->findEntity($qb); - } catch (DoesNotExistException $e) { + } catch (DoesNotExistException) { throw new ShareNotFoundException('Share not found by userId and pollId'); } } @@ -162,17 +171,17 @@ private function getShareByPollAndUser(string $userId, int $pollId): Share { /** * @throws InvalidShareTypeException */ - public static function getUserObject(string $type, string $id, string $displayName = '', string $emailAddress = '', string $language = '', string $locale = '', string $timeZoneName = ''): Ghost|Group|Team|Contact|ContactGroup|User|Email|GenericUser { + public static function createUserObject(string $type, string $id, string $displayName = '', string $emailAddress = '', string $language = '', string $locale = '', string $timeZoneName = ''): Ghost|Group|Team|Contact|ContactGroup|User|Email|GenericUser { try { return match ($type) { - Ghost::TYPE => new Ghost($id), - Group::TYPE => new Group($id), - Team::TYPE => new Team($id), - Contact::TYPE => new Contact($id), - ContactGroup::TYPE => new ContactGroup($id), - User::TYPE => new User($id), - Admin::TYPE => new Admin($id), - Email::TYPE => new Email($id, $displayName, $emailAddress, $language), + UserBase::TYPE_GHOST => new Ghost($id), + UserBase::TYPE_GROUP => new Group($id), + UserBase::TYPE_TEAM => new Team($id), + UserBase::TYPE_CONTACT => new Contact($id), + UserBase::TYPE_CONTACTGROUP => new ContactGroup($id), + UserBase::TYPE_USER => new User($id), + UserBase::TYPE_ADMIN => new Admin($id), + UserBase::TYPE_EMAIL => new Email($id, $displayName, $emailAddress, $language), UserBase::TYPE_EXTERNAL => new GenericUser($id, UserBase::TYPE_EXTERNAL, $displayName, $emailAddress, $language, $locale, $timeZoneName), UserBase::TYPE_PUBLIC => new GenericUser($id, UserBase::TYPE_PUBLIC, $displayName), default => throw new InvalidShareTypeException('Invalid user type (' . $type . ')'), diff --git a/lib/Helper/NameGenerator.php b/lib/Helper/NameGenerator.php new file mode 100644 index 0000000000..950c50f17d --- /dev/null +++ b/lib/Helper/NameGenerator.php @@ -0,0 +1,85 @@ +userMapper->getParticipant($userId, $this->poll->getId()); + return $this->userMapper->getUser($userId, $this->poll->getId()); } protected function getRichDescription() : string { diff --git a/lib/Model/User/Admin.php b/lib/Model/User/Admin.php index 386a81e04c..93a6abb66f 100644 --- a/lib/Model/User/Admin.php +++ b/lib/Model/User/Admin.php @@ -15,9 +15,4 @@ class Admin extends User { public function __construct(string $id) { parent::__construct($id, self::TYPE); } - - public function getPrincipalUri(): string { - return 'principals/users/' . $this->getId(); - } - } diff --git a/lib/Model/User/Anon.php b/lib/Model/User/Anon.php index b5ebf3ccfb..2c37946a65 100644 --- a/lib/Model/User/Anon.php +++ b/lib/Model/User/Anon.php @@ -9,6 +9,7 @@ namespace OCA\Polls\Model\User; use DateTimeZone; +use OCA\Polls\Helper\NameGenerator; use OCA\Polls\Model\UserBase; class Anon extends UserBase { @@ -43,11 +44,11 @@ public function getTimeZoneName(): string { } public function getDisplayName(): string { - return 'Anon'; + return NameGenerator::generate($this->id); } public function getDescription(): string { - return $this->getDisplayName(); + return 'Anonymous'; } public function getEmailAddress(): string { @@ -70,10 +71,6 @@ public function jsonSerialize(): array { return $this->getSimpleUserArray(); } - public function getSafeDisplayName(): string { - return $this->getDisplayName(); - } - public function getSafeEmailAddress(): string { return $this->getEmailAddress(); } diff --git a/lib/Model/User/Contact.php b/lib/Model/User/Contact.php index 492cd489c1..f7362a8910 100644 --- a/lib/Model/User/Contact.php +++ b/lib/Model/User/Contact.php @@ -44,16 +44,6 @@ public function getSafeId(): string { return $this->displayName; } - /** - * In shares, contact users are used as email users - * return Email::TYPE (Share), - * - * @return string - **/ - public function getShareType(): string { - return UserBase::TYPE_EMAIL; - } - /** * In shares, contact users are used as email users * return Email::TYPE (Share), @@ -80,6 +70,14 @@ public function getShareUserId(): string { private function loadContact(): void { $contacts = self::listRaw($this->id, ['UID', 'FN']); + if (empty($contacts)) { + $contacts = self::listRaw($this->id, ['EMAIL']); + } + + if (empty($contacts)) { + throw new \Exception('Contact not found: ' . $this->id); + } + // workaround fur multiple found UIDs // Don't throw an error, log the error and take the first entry if (count($contacts) > 1) { diff --git a/lib/Model/User/Ghost.php b/lib/Model/User/Ghost.php index baed89c2d1..e71971c021 100644 --- a/lib/Model/User/Ghost.php +++ b/lib/Model/User/Ghost.php @@ -8,6 +8,7 @@ namespace OCA\Polls\Model\User; +use OCA\Polls\Helper\NameGenerator; use OCA\Polls\Model\UserBase; class Ghost extends UserBase { @@ -23,7 +24,15 @@ public function __construct(?string $id = null) { } public function getDisplayName(): string { - return 'Deleted User'; + return NameGenerator::generate($this->id); + } + + public function getDescription(): string { + return 'Deleted user'; + } + + public function getIsNoUser(): bool { + return true; } } diff --git a/lib/Model/UserBase.php b/lib/Model/UserBase.php index a642201cbd..2bf899b973 100644 --- a/lib/Model/UserBase.php +++ b/lib/Model/UserBase.php @@ -31,7 +31,9 @@ class UserBase implements JsonSerializable { /** @var string */ - public const TYPE = 'generic'; + public const TYPE_GENERIC = 'generic'; + /** @var string */ + public const TYPE = self::TYPE_GENERIC; /** @var string */ public const TYPE_PUBLIC = 'public'; /** @var string */ @@ -116,15 +118,15 @@ public function getHashedUserId(): string { * Returns the users' type * * Returned Type will be one of - * Contact::TYPE (Share), - * Email::TYPE (Share), - * Ghost::TYPE (deleted user), - * User::TYPE (NC User), - * Admin::TYPE (NC user with admin rights), - * Group::TYPE (NC Group), - * Team::TYPE (Share), - * ContactGroup::TYPE (Share) - * Anon::TYPE (Anonymized User) + * UserBase::TYPE_CONTACT (Share), + * UserBase::TYPE_EMAIL (Share), + * UserBase::TYPE_GHOST (deleted user), + * UserBase::TYPE_USER (NC User), + * UserBase::TYPE_ADMIN (NC user with admin rights), + * UserBase::TYPE_GROUP (NC Group), + * UserBase::TYPE_TEAM (Share), + * UserBase::TYPE_CONTACTGROUP (Share) + * UserBase::TYPE_ANON (Anonymized User) * * @return string **/ @@ -132,33 +134,14 @@ public function getType(): string { return $this->type; } - /** - * Returns the users' type used in shares - * - * Returned Type will be one of - * Email::TYPE (Share), - * Ghost::TYPE (deleted user), - * User::TYPE (NC User), - * Admin::TYPE (NC user with admin rights), - * Group::TYPE (NC Group), - * Team::TYPE (Share), - * ContactGroup::TYPE (Share) - * Anon::TYPE (Anonymized User) - * - * @return string - **/ - public function getShareType(): string { - return $this->type; - } - public function getIsGuest(): bool { - return !in_array($this->type, [User::TYPE, Admin::TYPE]); + return !in_array($this->type, [self::TYPE_USER, self::TYPE_ADMIN]); } /** * used for telling internal from guest users */ public function getSimpleType(): string { - if (in_array($this->type, [User::TYPE, Admin::TYPE])) { + if (in_array($this->type, [self::TYPE_USER, self::TYPE_ADMIN])) { return self::TYPE_USER; } @@ -360,7 +343,7 @@ protected function getSimpleUserArray(): array { 'array' => 'simpleArray', 'categories' => '', 'desc' => '', - 'displayName' => $this->getSafeDisplayName(), + 'displayName' => $this->getDisplayName(), 'emailAddress' => $this->getSafeEmailAddress(), 'id' => $this->getSafeId(), 'user' => null, @@ -397,13 +380,6 @@ public function getSafeId(): string { return $this->getId(); } - /** - * anonymize the displayname in case of anonymous settings - */ - public function getSafeDisplayName(): string { - return $this->displayName; - } - // Function for obfuscating mail adresses; Default return the email address public function getSafeEmailAddress(): string { // always return real email address if this user object is the current user diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php index cc6f2e7ded..8d8f76e7d4 100644 --- a/lib/Notification/Notifier.php +++ b/lib/Notification/Notifier.php @@ -12,6 +12,7 @@ use OCA\Polls\Db\Poll; use OCA\Polls\Db\PollMapper; use OCA\Polls\Db\UserMapper; +use OCA\Polls\Model\User\Ghost; use OCA\Polls\Service\NotificationService; use OCP\AppFramework\Db\DoesNotExistException; use OCP\IURLGenerator; @@ -134,7 +135,16 @@ public function prepare(INotification $notification, string $languageCode): INot } $actorId = $this->extractActorId($notification) ?? $poll->getOwner(); - $actor = $this->userMapper->getUserFromUserBase($actorId); + $actor = $this->userMapper->getUser($actorId); + + if ($actor instanceof Ghost) { + // no valid actor, withdraw notification + $this->logger->info('Notification silently removed, actor not found', [ + 'notification' => $notification->getObjectId(), + 'actorId' => $actorId, + ]); + $this->notificationService->removeNotification($notification); + } $pollTitle = $poll->getTitle(); $notification->setLink($poll->getVoteUrl()); @@ -172,7 +182,7 @@ public function prepare(INotification $notification, string $languageCode): INot switch ($notification->getSubject()) { case self::NOTIFY_POLL_CHANGED_OWNER: - $newOwner = $this->userMapper->getUserFromUserBase($parameters['newOwner']); + $newOwner = $this->userMapper->getUser($parameters['newOwner']); // overwrite the subject with the new owner $notification->setParsedSubject( $l->t('%s is the new owner of your poll.', $newOwner->getDisplayName()) diff --git a/lib/Provider/ActivityProvider.php b/lib/Provider/ActivityProvider.php index 91b6938e17..993d55f698 100644 --- a/lib/Provider/ActivityProvider.php +++ b/lib/Provider/ActivityProvider.php @@ -11,6 +11,7 @@ use OCA\Polls\AppInfo\Application; use OCA\Polls\Db\ShareMapper; use OCA\Polls\Db\UserMapper; +use OCA\Polls\Model\User\Ghost; use OCA\Polls\Model\UserBase; use OCA\Polls\Service\ActivityService; use OCP\Activity\Exceptions\UnknownActivityException; @@ -69,21 +70,18 @@ private function patchParameters(array $parameters) : array { protected function setSubjects(IEvent $event, string $subject): void { $parameters = $this->patchParameters($event->getSubjectParameters()); - try { - $actor = $this->userMapper->getParticipant($event->getAuthor(), $event->getObjectId()); - $parameters['actor'] = [ - 'type' => $actor->getSimpleType(), - 'id' => $actor->getId(), - 'name' => $actor->getDisplayName(), - ]; - } catch (\Exception $e) { - $parameters['actor'] = [ - 'type' => UserBase::TYPE_GUEST, - 'id' => $event->getAuthor(), - 'name' => 'An unknown participant', - ]; + /** @psalm-suppress RedundantCast */ + $actor = $this->userMapper->getUser($event->getAuthor(), (int)$event->getObjectId()); + + if ($actor instanceof Ghost) { + $actor = new UserBase($actor->getId(), UserBase::TYPE_GUEST, $actor->getDisplayName(), 'An unknown participant'); } + $parameters['actor'] = [ + 'type' => $actor->getSimpleType(), + 'id' => $actor->getId(), + 'name' => $actor->getDisplayName(), + ]; $placeholders = $replacements = []; foreach ($parameters as $placeholder => $parameter) { diff --git a/lib/Service/ShareService.php b/lib/Service/ShareService.php index 8f1d0ad436..b4bbdb0600 100644 --- a/lib/Service/ShareService.php +++ b/lib/Service/ShareService.php @@ -28,17 +28,14 @@ use OCA\Polls\Event\ShareRegistrationEvent; use OCA\Polls\Event\ShareTypeChangedEvent; use OCA\Polls\Exceptions\ForbiddenException; +use OCA\Polls\Exceptions\InvalidEmailAddress; use OCA\Polls\Exceptions\InvalidShareTypeException; use OCA\Polls\Exceptions\InvalidUsernameException; use OCA\Polls\Exceptions\NotFoundException; use OCA\Polls\Exceptions\ShareAlreadyExistsException; use OCA\Polls\Exceptions\ShareNotFoundException; -use OCA\Polls\Model\Group\ContactGroup; use OCA\Polls\Model\SentResult; use OCA\Polls\Model\Settings\AppSettings; -use OCA\Polls\Model\User\Contact; -use OCA\Polls\Model\User\Email; -use OCA\Polls\Model\User\Ghost; use OCA\Polls\Model\UserBase; use OCA\Polls\UserSession; use OCP\AppFramework\Db\DoesNotExistException; @@ -120,30 +117,6 @@ public function listNotInvited(int $pollId): array { return $this->shares; } - /** - * Creates a new personal share based on a public share for the current user - * or returns the existing personal share, if it already exists - */ - private function convertPublicShareToPersonalShare(): Share { - try { - // Create a new new personal share - $this->share = $this->createNewShare( - $this->share->getPollIdOrFail(), - $this->userSession->getCurrentUser(), - preventInvitation: true - ); - } catch (ShareAlreadyExistsException $e) { - // return existing personal share - $this->share = $this->shareMapper->findByPollAndUser( - $this->share->getPollIdOrFail(), - $this->userSession->getCurrentUserId() - ); - } - // remove the public token from session - $this->userSession->setShareToken($this->share->getToken()); - return $this->share; - } - /** * Request access to a share by share token * Returns the share for the given token, if the user has access to it. @@ -177,7 +150,7 @@ public function getEffectiveShare(?string $token = null): Share { // Logged-in user on public share: create/retrieve personal share $this->share->getType() === Share::TYPE_PUBLIC && $this->userSession->getIsLoggedIn() - => $this->convertPublicShareToPersonalShare(), + => $this->registerInternalUser($this->share->getToken()), // Email/contact share: convert to external share in_array($this->share->getType(), Share::CONVERATABLE_PUBLIC_SHARES, true) @@ -217,7 +190,13 @@ public function setType(string $token, string $type): Share { } /** - * Change share type + * Set email requirement for public poll registration + * @param string $token Token of share to update + * @param string $value One of the Share::EMAIL_* constants + * @return Share + * @throws NotFoundException if token does not exist + * @throws ForbiddenException if user has no permission to edit the poll + * @throws ShareNotFoundException if token does not exist in shareMapper */ public function setPublicPollEmail(string $token, string $value): Share { try { @@ -237,6 +216,13 @@ public function setPublicPollEmail(string $token, string $value): Share { /** * Set emailAddress of personal public share * + * @param Share $share Share to update + * @param string $emailAddress new email address to set + * @param bool $emptyIsValid Set true, if empty email address should be accepted, otherwise an exception will be thrown + * @throws InvalidShareTypeException if share type is not external + * @throws InvalidEmailAddress if email address is not valid + * + * * @return Share */ public function setEmailAddress(Share $share, string $emailAddress, bool $emptyIsValid = false): Share { @@ -375,13 +361,7 @@ private function convertDependingObjects(string $userId, string $replacementId, } /** - * Create a personal share from a public share - * or update an email share with the displayName - * @param string $publicShareToken - * @param string $displayName - * @param string $emailAddress - * @param string $timeZone - * @return Share + * @deprecated Use registerGuest() instead */ public function register( string $publicShareToken, @@ -389,46 +369,7 @@ public function register( string $emailAddress = '', string $timeZone = '', ): Share { - $this->share = $this->get($publicShareToken); - $displayName = $this->systemService->validatePublicUsername($displayName, share: $this->share); - - if ($this->share->getPublicPollEmail() !== Share::EMAIL_DISABLED) { - MailService::validateEmailAddress($emailAddress, $this->share->getPublicPollEmail() !== Share::EMAIL_MANDATORY); - } - - $language = $this->systemService->getGenericLanguage(); - $userId = $this->generatePublicUserId(); - - if ($this->share->getType() === Share::TYPE_PUBLIC) { - // Create new external share for user, who entered the poll via public link, - // prevent invtation sending, when no email address is given - $this->createNewShare( - $this->share->getPollIdOrFail(), - UserMapper::getUserObject(Share::TYPE_EXTERNAL, $userId, $displayName, $emailAddress, $language, $language, $timeZone), - !$emailAddress, - $timeZone - ); - $this->eventDispatcher->dispatchTyped(new ShareRegistrationEvent($this->share)); - - } elseif (in_array($this->share->getType(), Share::CONVERATABLE_PUBLIC_SHARES, true)) { - // Convert email and contact shares to external share, if user registers - // this should be avoided by the actual use cases, but keep code in case of later changes - $this->convertPersonalPublicShareToExternalShare($userId, $displayName, $emailAddress, $timeZone, $language); - - } else { - throw new ForbiddenException('Share does not allow registering for poll'); - } - - // send invitation mail, if invitationSent has no timestamp - try { - if (!$this->share->getInvitationSent()) { - $this->mailService->sendInvitation($this->share); - } - } catch (\Exception $e) { - $this->logger->error('Error sending Mail', ['emailAddress' => $this->share->getEmailAddress()]); - } - - return $this->share; + return $this->registerGuest($publicShareToken, $displayName, $emailAddress, $timeZone); } /** @@ -534,9 +475,9 @@ private function resolveGroup(Share $share): array { throw new InvalidShareTypeException('Cannot resolve members from share type ' . $share->getType()); } - foreach (UserMapper::getUserObject($share->getType(), $share->getUserId())->getMembers() as $member) { + foreach (UserMapper::createUserObject($share->getType(), $share->getUserId())->getMembers() as $member) { try { - $newShare = $this->add($share->getPollIdOrFail(), $member->getType(), $member->getId()); + $newShare = $this->createShare($share->getPollIdOrFail(), $member); $shares[] = $newShare; } catch (ForbiddenException $e) { // skip, if user is not allowed to add share, usually because of forbidden share type @@ -591,67 +532,154 @@ private function generatePublicUserId(string $prefix = 'ex_'): string { } /** - * Add share - * - * @param int $pollOrPollGroupId Poll or PollGroup id - * @param string $type Type of share, e.g. UserBase::TYPE_EXTERNAL, UserBase::TYPE_PUBLIC, Ghost::TYPE, Contact::TYPE, ContactGroup::TYPE, Email::TYPE - * @return Share + * Use case 1a: Owner creates a public share link for a poll */ - public function add( - int $pollOrPollGroupId, - string $type, - string $userId = '', - string $displayName = '', - string $emailAddress = '', - string $purpose = 'poll', - ): Share { + public function createPublicShare(int $pollId): Share { + $this->pollMapper->get($pollId) + ->request(Poll::PERMISSION_POLL_EDIT) + ->request(Poll::PERMISSION_SHARE_ADD) + ->request(Poll::PERMISSION_SHARE_ADD_EXTERNAL); + $this->appSettings->getPublicSharesAllowed(); + + $shareUser = UserMapper::createUserObject(UserBase::TYPE_PUBLIC, ''); + + try { + $share = $this->createNewShare($pollId, $shareUser); + } catch (ShareAlreadyExistsException $e) { + $share = $e->getShare(); + } + + $this->eventDispatcher->dispatchTyped(new ShareCreateEvent($share)); + return $share; + } + + /** + * Use case 1b: Owner shares poll with a specific user object (User, Group, Team, Contact, External) + * Contacts are automatically converted to external shares (with email) in Share::setFromUserObject() + */ + public function createShare(int $pollOrPollGroupId, UserBase $userGroup, string $purpose = 'poll'): Share { if ($purpose === 'poll') { $poll = $this->pollMapper->get($pollOrPollGroupId) ->request(Poll::PERMISSION_POLL_EDIT) ->request(Poll::PERMISSION_SHARE_ADD); - if ($type === UserBase::TYPE_PUBLIC) { - $this->appSettings->getPublicSharesAllowed(); - $poll->request(Poll::PERMISSION_SHARE_ADD_EXTERNAL); - } - - // validate user type for external types and check, if user is allowed to create this type of share - if (match ($type) { - Ghost::TYPE => true, - Contact::TYPE => true, - ContactGroup::TYPE => true, - Email::TYPE => true, - UserBase::TYPE_EXTERNAL => true, - default => false, - }) { + if (in_array($userGroup->getType(), [ + UserBase::TYPE_GHOST, + UserBase::TYPE_CONTACT, + UserBase::TYPE_CONTACTGROUP, + UserBase::TYPE_EMAIL, + UserBase::TYPE_EXTERNAL, + ], true)) { $poll->request(Poll::PERMISSION_SHARE_ADD_EXTERNAL); } - $shareUser = UserMapper::getUserObject($type, $userId, $displayName, $emailAddress); $preventInvitation = false; } else { - $shareUser = $this->userMapper->getUserFromUserBase($userId); $preventInvitation = true; } try { - $share = $this->createNewShare( - pollOrPollGroupId: $pollOrPollGroupId, - userGroup: $shareUser, - preventInvitation: $preventInvitation, - timeZone: '', - purpose: $purpose - ); + $share = $this->createNewShare($pollOrPollGroupId, $userGroup, $preventInvitation, $purpose); } catch (ShareAlreadyExistsException $e) { $share = $e->getShare(); } - // TODO: extend event for pollGroups if ($share->getPollId() !== null) { $this->eventDispatcher->dispatchTyped(new ShareCreateEvent($share)); } return $share; } + /** + * Use case 2a: Guest user registers via a public share link + * Creates a new external share with displayName and optional email address + */ + public function registerGuest( + string $token, + string $displayName, + string $emailAddress = '', + string $timeZone = '', + ): Share { + $this->share = $this->get($token); + + if ($this->share->getType() !== Share::TYPE_PUBLIC) { + throw new ForbiddenException('Registration is only allowed for public shares'); + } + + $displayName = $this->systemService->validatePublicUsername($displayName, share: $this->share); + + if ($this->share->getPublicPollEmail() !== Share::EMAIL_DISABLED) { + MailService::validateEmailAddress($emailAddress, $this->share->getPublicPollEmail() !== Share::EMAIL_MANDATORY); + } + + $language = $this->systemService->getGenericLanguage(); + $userId = $this->generatePublicUserId(); + $user = UserMapper::createUserObject(Share::TYPE_EXTERNAL, $userId, $displayName, $emailAddress, $language, $language, $timeZone); + + $this->createNewShare( + $this->share->getPollIdOrFail(), + $user, + $user->getEmailAddress() !== '', + ); + $this->eventDispatcher->dispatchTyped(new ShareRegistrationEvent($this->share)); + + try { + if (!$this->share->getInvitationSent()) { + $this->mailService->sendInvitation($this->share); + } + } catch (\Exception $e) { + $this->logger->error('Error sending Mail', ['emailAddress' => $this->share->getEmailAddress()]); + } + + return $this->share; + } + + /** + * Use case 2b: Logged-in NC user accesses poll via public share + * Automatically creates a personal user share if not already involved + */ + public function registerInternalUser(string $token): Share { + $this->share = $this->get($token); + + if ($this->share->getType() !== Share::TYPE_PUBLIC) { + throw new ForbiddenException('Internal user registration is only allowed for public shares'); + } + + try { + $this->share = $this->createNewShare( + $this->share->getPollIdOrFail(), + $this->userSession->getCurrentUser(), + preventInvitation: true + ); + } catch (ShareAlreadyExistsException) { + $this->share = $this->shareMapper->findByPollAndUser( + $this->share->getPollIdOrFail(), + $this->userSession->getCurrentUserId() + ); + } + $this->userSession->setShareToken($this->share->getToken()); + return $this->share; + } + + /** + * @deprecated Use createPublicShare(), createShare(), registerGuest(), or registerInternalUser() instead + */ + public function add( + int $pollOrPollGroupId, + string $type, + string $userId = '', + string $displayName = '', + string $emailAddress = '', + string $purpose = 'poll', + ): Share { + if ($type === UserBase::TYPE_PUBLIC) { + return $this->createPublicShare($pollOrPollGroupId); + } + $shareUser = $purpose === 'poll' + ? UserMapper::createUserObject($type, $userId, $displayName, $emailAddress) + : $this->userMapper->getUserFromUserBase($userId); + return $this->createShare($pollOrPollGroupId, $shareUser, $purpose); + } + private function sortByCategory(): void { $sortedShares = []; foreach (Share::TYPE_SORT_ARRAY as $shareType) { @@ -725,14 +753,12 @@ private function generateToken(): string { * @param int $pollOrPollGroupId Poll or PollGroup id * @param UserBase $userGroup UserBase object of the user group to share with * @param bool $preventInvitation Set true, if no invitation should be sent - * @param string $timeZone Timezone of the user, used for external users * @param string $purpose Purpose of the share, either 'poll' or 'pollGroup' */ private function createNewShare( int $pollOrPollGroupId, UserBase $userGroup, bool $preventInvitation = false, - string $timeZone = '', string $purpose = 'poll', ): Share { $preventInvitation = $userGroup->getType() === UserBase::TYPE_PUBLIC ?: $preventInvitation; @@ -751,27 +777,8 @@ private function createNewShare( } $this->share->setToken($token); - $this->share->setType($userGroup->getShareType()); - $this->share->setDisplayName($userGroup->getDisplayName()); + $this->share->setFromUserObject($userGroup, $token, $purpose); $this->share->setInvitationSent($preventInvitation ? time() : 0); - $this->share->setEmailAddress($userGroup->getEmailAddress()); - $this->share->setUserId($userGroup->getShareUserId()); - - // skip for poll groups - if ($purpose === 'poll') { - // normal continuation - // public share to create, set token as userId - if ($userGroup->getType() === UserBase::TYPE_PUBLIC) { - $this->share->setUserId($token); - } - - // user is created from public share. Store locale information for - // usage in server side actions, i.e. scheduled emails - if ($userGroup->getType() === UserBase::TYPE_EXTERNAL) { - $this->share->setLanguage($userGroup->getLanguageCode()); - $this->share->setTimeZoneName($timeZone); - } - } try { $this->share = $this->shareMapper->insert($this->share); diff --git a/lib/Service/SystemService.php b/lib/Service/SystemService.php index b908dcff4f..f0d70fdfd4 100644 --- a/lib/Service/SystemService.php +++ b/lib/Service/SystemService.php @@ -18,6 +18,7 @@ use OCA\Polls\Exceptions\ForbiddenException; use OCA\Polls\Exceptions\InvalidUsernameException; use OCA\Polls\Exceptions\TooShortException; +use OCA\Polls\Helper\NameGenerator; use OCA\Polls\Model\Group\ContactGroup; use OCA\Polls\Model\Group\Group; use OCA\Polls\Model\Group\Team; @@ -25,6 +26,7 @@ use OCA\Polls\Model\User\Contact; use OCA\Polls\Model\User\Email; use OCA\Polls\Model\User\User; +use OCA\Polls\Model\UserBase; use OCP\BackgroundJob\IJob; use OCP\BackgroundJob\IJobList; use OCP\Collaboration\Collaborators\ISearch; @@ -207,11 +209,11 @@ private function search(string $query, array $types): array { } foreach (($result['circles'] ?? []) as $item) { - $items[] = UserMapper::getUserObject(Team::TYPE, $item['value']['shareWith'])->getRichUserArray(); + $items[] = UserMapper::createUserObject(UserBase::TYPE_TEAM, $item['value']['shareWith'])->getRichUserArray(); } foreach (($result['exact']['circles'] ?? []) as $item) { - $items[] = UserMapper::getUserObject(Team::TYPE, $item['value']['shareWith'])->getRichUserArray(); + $items[] = UserMapper::createUserObject(UserBase::TYPE_TEAM, $item['value']['shareWith'])->getRichUserArray(); } } @@ -261,6 +263,20 @@ public function validatePublicUsernameByToken(string $userName, string $token): return $this->validatePublicUsername($userName, $share); } + public function getRandomDisplayName(string $token): string { + $share = $this->shareMapper->findByToken($token); + for ($i = 0; $i < 10; $i++) { + $userName = NameGenerator::generateRandom(); + try { + $this->validatePublicUsername($userName, $share); + return $userName; + } catch (InvalidUsernameException|TooShortException) { + $userName = ''; + } + } + return $userName; + } + /** * Run a cron job immediately * @param class-string $className The class name of the cron job to run @@ -377,7 +393,7 @@ public function validatePublicUsername(string $userName, Share $share): string { // get all shares for this poll foreach ($this->shareMapper->findByPoll($share->getPollIdOrFail()) as $share) { - if ($share->getType() !== Team::TYPE && $share->getUser()->hasName($compareUserName)) { + if ($share->getType() !== UserBase::TYPE_TEAM && $share->getUser()->hasName($compareUserName)) { throw new InvalidUsernameException; } } diff --git a/src/components/Shares/ShareItem.vue b/src/components/Shares/ShareItem.vue index 9191dd7f73..a371d15e9b 100644 --- a/src/components/Shares/ShareItem.vue +++ b/src/components/Shares/ShareItem.vue @@ -95,24 +95,6 @@ const typeIconComponent = computed(() => { } }) -// const votedIndicator = computed(() => { -// if (share.voted) { -// return { -// class: 'vote-status voted', -// component: VotedIcon, -// name: t('polls', 'Has voted'), -// } -// } -// if (!share.voted) { -// return { -// class: 'vote-status unvoted', -// component: UnvotedIcon, -// name: t('polls', 'Has not voted'), -// } -// } -// return { class: 'vote-status empty', component: null, name: '' } -// }) - const votedIndicator = computed(() => { if (share.groupId || ['public', 'group'].includes(share.type)) { return { class: 'vote-status empty', component: 'div', name: '' } diff --git a/src/components/Shares/ShareMenu.vue b/src/components/Shares/ShareMenu.vue index b7baab4ada..6cc15f5fef 100644 --- a/src/components/Shares/ShareMenu.vue +++ b/src/components/Shares/ShareMenu.vue @@ -41,6 +41,8 @@ const sharesStore = useSharesStore() const pollGroupsStore = usePollGroupsStore() const pollStore = usePollStore() +const actionsRef = ref | null>(null) + const isDirectShare = computed( () => share.groupId === pollGroupsStore.currentPollGroup?.id @@ -177,9 +179,13 @@ const switchAdmin = computed(() => ({ const copyLinkButton = computed(() => ({ activate: !share.groupId && !share.deleted && !!share.URL, name: t('polls', 'Copy link to clipboard'), - action: () => { + action: async () => { + // Close menu first to deactivate focus trap (active for public shares + // due to NcActionInput). On HTTP, the browser aborts a pending clipboard + // write when the focus trap deactivates simultaneously. + await actionsRef.value?.closeMenu(false) try { - navigator.clipboard.writeText(share.URL) + await navigator.clipboard.writeText(share.URL) showSuccess(t('polls', 'Link copied to clipboard')) } catch { showError(t('polls', 'Error while copying link to clipboard')) @@ -259,7 +265,7 @@ async function submitLabel() {