From bb833dd2fab0787336d925dca698f76eb6661eda Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 23 Oct 2024 06:57:23 +0200 Subject: [PATCH 1/6] feat(email): Allow inviting email guests to private conversations Signed-off-by: Joas Schilling --- lib/Controller/RoomController.php | 9 +-------- lib/Service/RoomService.php | 6 ++++-- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php index 7370b207987..e5861bdb0d2 100644 --- a/lib/Controller/RoomController.php +++ b/lib/Controller/RoomController.php @@ -1426,13 +1426,6 @@ public function addParticipantToRoom(string $newParticipant, string $source = 'u $this->participantService->addCircle($this->room, $circle, $participants); } elseif ($source === 'emails') { - $data = []; - try { - $this->roomService->setType($this->room, Room::TYPE_PUBLIC); - $data = ['type' => $this->room->getType()]; - } catch (TypeException) { - } - $email = strtolower($newParticipant); $actorId = hash('sha256', $email); try { @@ -1442,7 +1435,7 @@ public function addParticipantToRoom(string $newParticipant, string $source = 'u $this->guestManager->sendEmailInvitation($this->room, $participant); } - return new DataResponse($data); + return new DataResponse([]); } elseif ($source === 'federated_users') { if (!$this->talkConfig->isFederationEnabled()) { return new DataResponse(['error' => 'federation'], Http::STATUS_NOT_IMPLEMENTED); diff --git a/lib/Service/RoomService.php b/lib/Service/RoomService.php index a53a5979310..c1ce4253c03 100644 --- a/lib/Service/RoomService.php +++ b/lib/Service/RoomService.php @@ -686,11 +686,13 @@ public function setType(Room $room, int $newType, bool $allowSwitchingOneToOne = $room->setType($newType); if ($oldType === Room::TYPE_PUBLIC) { - // Kick all guests and users that were not invited + // Kick all guests that are not email invited + // and all users that joined the public link $delete = $this->db->getQueryBuilder(); $delete->delete('talk_attendees') ->where($delete->expr()->eq('room_id', $delete->createNamedParameter($room->getId(), IQueryBuilder::PARAM_INT))) - ->andWhere($delete->expr()->in('participant_type', $delete->createNamedParameter([Participant::GUEST, Participant::GUEST_MODERATOR, Participant::USER_SELF_JOINED], IQueryBuilder::PARAM_INT_ARRAY))); + ->andWhere($delete->expr()->in('participant_type', $delete->createNamedParameter([Participant::GUEST, Participant::GUEST_MODERATOR, Participant::USER_SELF_JOINED], IQueryBuilder::PARAM_INT_ARRAY))) + ->andWhere($delete->expr()->neq('actor_type', $delete->createNamedParameter(Attendee::ACTOR_EMAILS, IQueryBuilder::PARAM_INT))); $delete->executeStatement(); } From b095f8714c606cf460dd7304aabcee5f59520866 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 12 May 2026 13:39:20 +0200 Subject: [PATCH 2/6] fix(session): Store and read the email auth without tabId This is kind of required as the frontpage which writes the session is the HTML request which can not send the header Signed-off-by: Joas Schilling --- lib/TalkSession.php | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/lib/TalkSession.php b/lib/TalkSession.php index 5dfb87d4feb..d41803885fe 100644 --- a/lib/TalkSession.php +++ b/lib/TalkSession.php @@ -63,11 +63,15 @@ public function removeGuestActorIdForRoom(string $token): void { } public function getAuthedEmailActorIdForRoom(string $token): ?string { - return $this->getValue('spreed-authed-email', $token); + return $this->getValue('spreed-authed-email', $token, false); } public function setAuthedEmailActorIdForRoom(string $token, string $actorId): void { - $this->setValue('spreed-authed-email', $token, $actorId); + $this->setValue('spreed-authed-email', $token, $actorId, false); + } + + public function removeAuthedEmailActorIdForRoom(string $token): void { + $this->removeValue('spreed-authed-email', $token, false); } public function getFileShareTokenForRoom(string $roomToken): ?string { @@ -108,14 +112,14 @@ protected function getValues(string $key): array { return $values; } - protected function getValue(string $key, string $token): ?string { - $token .= $this->getTabId(); + protected function getValue(string $key, string $token, bool $useTabId = true): ?string { + $token .= $useTabId ? $this->getTabId() : ''; $values = $this->getValues($key); return $values[$token] ?? null; } - protected function setValue(string $key, string $token, string $value): void { - $token .= $this->getTabId(); + protected function setValue(string $key, string $token, string $value, bool $useTabId = true): void { + $token .= $useTabId ? $this->getTabId() : ''; $reopened = $this->session->reopen(); $values = $this->getValues($key); @@ -128,11 +132,11 @@ protected function setValue(string $key, string $token, string $value): void { } } - protected function removeValue(string $key, string $token): void { + protected function removeValue(string $key, string $token, bool $useTabId = true): void { $reopened = $this->session->reopen(); $values = $this->getValues($key); - $tabId = $this->getTabId(); + $tabId = $useTabId ? $this->getTabId() : ''; if ($tabId !== '') { unset($values[$token . $tabId]); } else { From 36aca137c1613c398163d1230b2be9df6e9d9273 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 12 May 2026 13:43:02 +0200 Subject: [PATCH 3/6] feat(email): Fix authentication of email users Co-Authored-By: Claude Opus 4.7 Signed-off-by: Joas Schilling --- lib/Authenticator.php | 129 ++++++++++++++++++ lib/Chat/Parser/SystemMessage.php | 2 +- lib/Config.php | 1 - lib/Controller/CallController.php | 2 +- lib/Controller/ChatController.php | 6 +- lib/Controller/RoomController.php | 60 ++++++-- lib/Controller/SignalingController.php | 30 +++- lib/Federation/Authenticator.php | 115 ---------------- lib/Manager.php | 7 +- lib/Middleware/InjectionMiddleware.php | 29 +++- tests/php/Chat/Parser/SystemMessageTest.php | 2 +- tests/php/Controller/ChatControllerTest.php | 2 +- .../Controller/SignalingControllerTest.php | 2 +- tests/php/Recording/BackendNotifierTest.php | 2 +- tests/psalm-baseline.xml | 10 +- 15 files changed, 246 insertions(+), 153 deletions(-) create mode 100644 lib/Authenticator.php delete mode 100644 lib/Federation/Authenticator.php diff --git a/lib/Authenticator.php b/lib/Authenticator.php new file mode 100644 index 00000000000..15c15273c91 --- /dev/null +++ b/lib/Authenticator.php @@ -0,0 +1,129 @@ +resolved) { + return; + } + $this->resolved = true; + + if ((bool)$this->request->getHeader('x-nextcloud-federation')) { + $authUser = urldecode($this->request->server['PHP_AUTH_USER'] ?? ''); + try { + $cloudId = $this->cloudIdManager->resolveCloudId($authUser); + $this->isFederationRequest = true; + $this->actorType = Attendee::ACTOR_FEDERATED_USERS; + $this->actorId = $cloudId->getId(); + $this->accessToken = $this->request->server['PHP_AUTH_PW'] ?? ''; + return; + } catch (\InvalidArgumentException) { + // Fall through to other authentication shapes + } + } + + $token = $this->request->getParam('token'); + if (is_string($token) && $token !== '') { + $emailActorId = $this->talkSession->getAuthedEmailActorIdForRoom($token); + if ($emailActorId !== null) { + $this->isAuthenticatedEmailGuest = true; + $this->actorType = Attendee::ACTOR_EMAILS; + $this->actorId = $emailActorId; + } + } + } + + public function isFederationRequest(): bool { + $this->resolve(); + return $this->isFederationRequest; + } + + public function isAuthenticatedEmailGuest(): bool { + $this->resolve(); + return $this->isAuthenticatedEmailGuest; + } + + public function isAuthenticatedRequest(): bool { + $this->resolve(); + return $this->isFederationRequest || $this->isAuthenticatedEmailGuest; + } + + public function getActorType(): string { + $this->resolve(); + return $this->actorType; + } + + public function getActorId(): string { + $this->resolve(); + return $this->actorId; + } + + /** + * Federation-only alias for {@see getActorId()}. Returns an empty string + * when the current request is not a federation request, even if another + * authenticated actor (e.g. email guest) is present. + */ + public function getCloudId(): string { + return $this->isFederationRequest() ? $this->getActorId() : ''; + } + + public function getAccessToken(): string { + $this->resolve(); + return $this->accessToken; + } + + public function authenticated(Room $room, Participant $participant): void { + $this->room = $room; + $this->participant = $participant; + } + + /** + * @throws RoomNotFoundException + */ + public function getRoom(): Room { + if ($this->room === null) { + throw new RoomNotFoundException(); + } + return $this->room; + } + + /** + * @throws ParticipantNotFoundException + */ + public function getParticipant(): Participant { + if ($this->participant === null) { + throw new ParticipantNotFoundException(); + } + return $this->participant; + } +} diff --git a/lib/Chat/Parser/SystemMessage.php b/lib/Chat/Parser/SystemMessage.php index 8cd63326f5d..7f22c821094 100644 --- a/lib/Chat/Parser/SystemMessage.php +++ b/lib/Chat/Parser/SystemMessage.php @@ -10,11 +10,11 @@ use OCA\Circles\CirclesManager; use OCA\DAV\CardDAV\PhotoCache; +use OCA\Talk\Authenticator; use OCA\Talk\Chat\ChatManager; use OCA\Talk\Events\MessageParseEvent; use OCA\Talk\Events\OverwritePublicSharePropertiesEvent; use OCA\Talk\Exceptions\ParticipantNotFoundException; -use OCA\Talk\Federation\Authenticator; use OCA\Talk\Model\Attendee; use OCA\Talk\Model\Message; use OCA\Talk\Participant; diff --git a/lib/Config.php b/lib/Config.php index 30df5dc9432..e6c3b78a58c 100644 --- a/lib/Config.php +++ b/lib/Config.php @@ -10,7 +10,6 @@ use OCA\Talk\AppInfo\Application; use OCA\Talk\Events\BeforeTurnServersGetEvent; -use OCA\Talk\Federation\Authenticator; use OCA\Talk\Model\Attendee; use OCA\Talk\Service\RecordingService; use OCA\Talk\Settings\UserPreference; diff --git a/lib/Controller/CallController.php b/lib/Controller/CallController.php index fa75a086df1..8a81cc7ad48 100644 --- a/lib/Controller/CallController.php +++ b/lib/Controller/CallController.php @@ -8,10 +8,10 @@ namespace OCA\Talk\Controller; +use OCA\Talk\Authenticator; use OCA\Talk\Config; use OCA\Talk\Exceptions\DialOutFailedException; use OCA\Talk\Exceptions\ParticipantNotFoundException; -use OCA\Talk\Federation\Authenticator; use OCA\Talk\Manager; use OCA\Talk\Middleware\Attribute\FederationSupported; use OCA\Talk\Middleware\Attribute\RequireCallEnabled; diff --git a/lib/Controller/ChatController.php b/lib/Controller/ChatController.php index 4db08153185..e29c2c556c1 100644 --- a/lib/Controller/ChatController.php +++ b/lib/Controller/ChatController.php @@ -9,6 +9,7 @@ namespace OCA\Talk\Controller; use OCA\Talk\AppInfo\Application; +use OCA\Talk\Authenticator; use OCA\Talk\Chat\AutoComplete\SearchPlugin; use OCA\Talk\Chat\AutoComplete\Sorter; use OCA\Talk\Chat\ChatManager; @@ -19,7 +20,6 @@ use OCA\Talk\Exceptions\CannotReachRemoteException; use OCA\Talk\Exceptions\ChatSummaryException; use OCA\Talk\Exceptions\ParticipantNotFoundException; -use OCA\Talk\Federation\Authenticator; use OCA\Talk\GuestManager; use OCA\Talk\Manager; use OCA\Talk\MatterbridgeManager; @@ -251,8 +251,8 @@ private function resolveReplyTo(int $replyTo, string $replyToToken, string $acto * @return list{0: Attendee::ACTOR_*, 1: string} */ protected function getActorInfo(string $actorDisplayName = ''): array { - $remoteCloudId = $this->federationAuthenticator->getCloudId(); - if ($remoteCloudId !== '') { + if ($this->federationAuthenticator->getActorType() === Attendee::ACTOR_FEDERATED_USERS) { + $remoteCloudId = $this->federationAuthenticator->getActorId(); if ($actorDisplayName) { $this->participantService->updateDisplayNameForActor(Attendee::ACTOR_FEDERATED_USERS, $remoteCloudId, $actorDisplayName); } diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php index e5861bdb0d2..a8eba4efd07 100644 --- a/lib/Controller/RoomController.php +++ b/lib/Controller/RoomController.php @@ -9,6 +9,7 @@ namespace OCA\Talk\Controller; use OCA\DAV\CalDAV\TimezoneService; +use OCA\Talk\Authenticator; use OCA\Talk\Capabilities; use OCA\Talk\Config; use OCA\Talk\Events\AAttendeeRemovedEvent; @@ -36,7 +37,6 @@ use OCA\Talk\Exceptions\RoomProperty\SipConfigurationException; use OCA\Talk\Exceptions\RoomProperty\TypeException; use OCA\Talk\Exceptions\UnauthorizedException; -use OCA\Talk\Federation\Authenticator; use OCA\Talk\Federation\FederationManager; use OCA\Talk\Federation\Proxy\TalkV1\ProxyRequest; use OCA\Talk\GuestManager; @@ -456,15 +456,32 @@ public function getSingleRoom(string $token): DataResponse { $isTalkFederation = $this->request->getHeader('x-nextcloud-federation'); if (!$isTalkFederation) { - $sessionId = $this->session->getSessionForRoom($token); - $room = $this->manager->getRoomForUserByToken($token, $this->userId, $sessionId, $includeLastMessage, $isSIPBridgeRequest); + if ($this->userId === null && $this->federationAuthenticator->isAuthenticatedEmailGuest()) { + // Authenticated email guest: the user/public-room check in + // getRoomForUserByToken() would reject non-public rooms, so + // trust the session-stored actor id and require the email + // attendee to still exist. + $room = $this->manager->getRoomByToken($token); + try { + $participant = $this->participantService->getParticipantByActor( + $room, + Attendee::ACTOR_EMAILS, + $this->federationAuthenticator->getActorId(), + ); + } catch (ParticipantNotFoundException) { + throw new RoomNotFoundException(); + } + } else { + $sessionId = $this->session->getSessionForRoom($token); + $room = $this->manager->getRoomForUserByToken($token, $this->userId, $sessionId, $includeLastMessage, $isSIPBridgeRequest); - try { - $participant = $this->participantService->getParticipant($room, $this->userId, $sessionId); - } catch (ParticipantNotFoundException) { try { - $participant = $this->participantService->getParticipantBySession($room, $sessionId); + $participant = $this->participantService->getParticipant($room, $this->userId, $sessionId); } catch (ParticipantNotFoundException) { + try { + $participant = $this->participantService->getParticipantBySession($room, $sessionId); + } catch (ParticipantNotFoundException) { + } } } } else { @@ -2013,9 +2030,20 @@ public function markConversationAsInsensitive(): DataResponse { ])] public function joinRoom(string $token, string $password = '', bool $force = true): DataResponse { $sessionId = $this->session->getSessionForRoom($token); + $authenticatedEmailGuest = null; + if ($this->userId === null && $this->federationAuthenticator->isAuthenticatedEmailGuest()) { + $authenticatedEmailGuest = $this->federationAuthenticator->getActorId(); + } + try { - // The participant is just joining, so enforce to not load any session - $room = $this->manager->getRoomForUserByToken($token, $this->userId, null); + // The participant is just joining, so enforce to not load any session. + // Authenticated email guests bypass the user/public-room access check; + // the email attendee lookup below is the trust anchor instead. + if ($authenticatedEmailGuest !== null) { + $room = $this->manager->getRoomByToken($token); + } else { + $room = $this->manager->getRoomForUserByToken($token, $this->userId, null); + } } catch (RoomNotFoundException $e) { $response = new DataResponse(null, Http::STATUS_NOT_FOUND); $response->throttle(['token' => $token, 'action' => 'talkRoomToken']); @@ -2065,8 +2093,6 @@ public function joinRoom(string $token, string $password = '', bool $force = tru } } - $authenticatedEmailGuest = $this->session->getAuthedEmailActorIdForRoom($token); - $headers = []; if ($authenticatedEmailGuest !== null || $room->isFederatedConversation() || ($previousParticipant instanceof Participant && $previousParticipant->isGuest())) { @@ -2088,6 +2114,12 @@ public function joinRoom(string $token, string $password = '', bool $force = tru try { $previousParticipant = $this->participantService->getParticipantByActor($room, Attendee::ACTOR_EMAILS, $authenticatedEmailGuest); } catch (ParticipantNotFoundException) { + // Stale spreed-authed-email session: the email attendee was + // removed (e.g. by a moderator) since the invite link was opened. + $this->session->removeAuthedEmailActorIdForRoom($token); + $response = new DataResponse(null, Http::STATUS_NOT_FOUND); + $response->throttle(['token' => $token, 'action' => 'talkRoomToken']); + return $response; } } $participant = $this->participantService->joinRoomAsNewGuest($this->roomService, $room, $password, $result['result'], $previousParticipant); @@ -2537,7 +2569,11 @@ public function leaveRoom(string $token): DataResponse { $this->session->removeSessionForRoom($token); try { - $room = $this->manager->getRoomForUserByToken($token, $this->userId, $sessionId); + if ($this->userId === null && $this->federationAuthenticator->isAuthenticatedEmailGuest()) { + $room = $this->manager->getRoomByToken($token); + } else { + $room = $this->manager->getRoomForUserByToken($token, $this->userId, $sessionId); + } $participant = $this->participantService->getParticipantBySession($room, $sessionId); if ($room->isFederatedConversation()) { diff --git a/lib/Controller/SignalingController.php b/lib/Controller/SignalingController.php index afc77a0d879..cb2d0a1d16e 100644 --- a/lib/Controller/SignalingController.php +++ b/lib/Controller/SignalingController.php @@ -8,13 +8,13 @@ namespace OCA\Talk\Controller; +use OCA\Talk\Authenticator; use OCA\Talk\Config; use OCA\Talk\Events\BeforeSignalingResponseSentEvent; use OCA\Talk\Exceptions\ForbiddenException; use OCA\Talk\Exceptions\ParticipantNotFoundException; use OCA\Talk\Exceptions\RoomNotFoundException; use OCA\Talk\Exceptions\UnauthorizedException; -use OCA\Talk\Federation\Authenticator; use OCA\Talk\Manager; use OCA\Talk\Model\Attendee; use OCA\Talk\Model\Session; @@ -161,6 +161,14 @@ public function getSettings(string $token = ''): DataResponse { $this->federationAuthenticator->getCloudId() ); $this->federationAuthenticator->authenticated($room, $participant); + } elseif ($token !== '' && $this->userId === null && $this->federationAuthenticator->isAuthenticatedEmailGuest()) { + $room = $this->manager->getRoomByToken($token); + $participant = $this->participantService->getParticipantByActor( + $room, + Attendee::ACTOR_EMAILS, + $this->federationAuthenticator->getActorId(), + ); + $this->federationAuthenticator->authenticated($room, $participant); } elseif ($token !== '') { $room = $this->manager->getRoomForUserByToken($token, $this->userId); } elseif ($this->userId !== null || $isRecordingRequest) { @@ -600,11 +608,21 @@ public function pullMessages(string $token): DataResponse { // Was the session killed or the complete conversation? try { - $room = $this->manager->getRoomForUserByToken($token, $this->userId); - if ($this->userId) { - // For logged in users we check if they are still part of the public conversation, - // if not they were removed instead of having a conflict. - $this->participantService->getParticipant($room, $this->userId, false); + if ($this->userId === null && $this->federationAuthenticator->isAuthenticatedEmailGuest()) { + $room = $this->manager->getRoomByToken($token); + // Verify the email attendee still exists; otherwise treat as removed. + $this->participantService->getParticipantByActor( + $room, + Attendee::ACTOR_EMAILS, + $this->federationAuthenticator->getActorId(), + ); + } else { + $room = $this->manager->getRoomForUserByToken($token, $this->userId); + if ($this->userId) { + // For logged in users we check if they are still part of the public conversation, + // if not they were removed instead of having a conflict. + $this->participantService->getParticipant($room, $this->userId, false); + } } // Session was killed, make the UI redirect to an error diff --git a/lib/Federation/Authenticator.php b/lib/Federation/Authenticator.php deleted file mode 100644 index 4631e1b6392..00000000000 --- a/lib/Federation/Authenticator.php +++ /dev/null @@ -1,115 +0,0 @@ -isFederationRequest = (bool)$this->request->getHeader('x-nextcloud-federation'); - if (!$this->isFederationRequest) { - $this->federationCloudId = ''; - $this->accessToken = ''; - return; - } - - $authUser = $this->request->server['PHP_AUTH_USER'] ?? ''; - $authUser = urldecode($authUser); - - try { - $cloudId = $this->cloudIdManager->resolveCloudId($authUser); - $this->federationCloudId = $cloudId->getId(); - $this->accessToken = $this->request->server['PHP_AUTH_PW'] ?? ''; - } catch (\InvalidArgumentException) { - $this->isFederationRequest = false; - $this->federationCloudId = ''; - $this->accessToken = ''; - } - } - - public function isFederationRequest(): bool { - if ($this->isFederationRequest === null) { - $this->readHeaders(); - - if ($this->isFederationRequest === null) { - return false; - } - } - - return $this->isFederationRequest; - } - - public function getCloudId(): string { - if ($this->federationCloudId === null) { - $this->readHeaders(); - - if ($this->federationCloudId === null) { - return ''; - } - } - - return $this->federationCloudId; - } - - public function getAccessToken(): string { - if ($this->accessToken === null) { - $this->readHeaders(); - - if ($this->accessToken === null) { - return ''; - } - } - - return $this->accessToken; - } - - public function authenticated(Room $room, Participant $participant): void { - $this->room = $room; - $this->participant = $participant; - } - - /** - * @throws RoomNotFoundException - */ - public function getRoom(): Room { - if ($this->room === null) { - throw new RoomNotFoundException(); - } - return $this->room; - } - - /** - * @throws ParticipantNotFoundException - */ - public function getParticipant(): Participant { - if ($this->participant === null) { - throw new ParticipantNotFoundException(); - } - return $this->participant; - - } -} diff --git a/lib/Manager.php b/lib/Manager.php index 9b33f9f3e27..85eea1fa6f9 100644 --- a/lib/Manager.php +++ b/lib/Manager.php @@ -13,7 +13,6 @@ use OCA\Talk\Events\RoomCreatedEvent; use OCA\Talk\Exceptions\ParticipantNotFoundException; use OCA\Talk\Exceptions\RoomNotFoundException; -use OCA\Talk\Federation\Authenticator; use OCA\Talk\Model\Attachment; use OCA\Talk\Model\Attendee; use OCA\Talk\Model\AttendeeMapper; @@ -1110,7 +1109,11 @@ public function getRoomForSession(?string $userId, ?string $sessionId): Room { $this->participantService->cacheParticipant($room, $participant); $room->setParticipant($row['actor_id'], $participant); - if ($room->getType() === Room::TYPE_PUBLIC || !in_array($participant->getAttendee()->getParticipantType(), [Participant::GUEST, Participant::GUEST_MODERATOR, Participant::USER_SELF_JOINED], true)) { + if ($room->getType() === Room::TYPE_PUBLIC + || $row['actor_type'] === Attendee::ACTOR_EMAILS + || !in_array($participant->getAttendee()->getParticipantType(), [Participant::GUEST, Participant::GUEST_MODERATOR, Participant::USER_SELF_JOINED], true)) { + // Email attendees were directly invited via their email address; + // the attendee row itself is the authorization to access the room. return $room; } diff --git a/lib/Middleware/InjectionMiddleware.php b/lib/Middleware/InjectionMiddleware.php index 9ae74444ef1..22839ba0055 100644 --- a/lib/Middleware/InjectionMiddleware.php +++ b/lib/Middleware/InjectionMiddleware.php @@ -8,13 +8,13 @@ namespace OCA\Talk\Middleware; +use OCA\Talk\Authenticator; use OCA\Talk\Controller\AEnvironmentAwareOCSController; use OCA\Talk\Exceptions\CannotReachRemoteException; use OCA\Talk\Exceptions\ForbiddenException; use OCA\Talk\Exceptions\ParticipantNotFoundException; use OCA\Talk\Exceptions\PermissionsException; use OCA\Talk\Exceptions\RoomNotFoundException; -use OCA\Talk\Federation\Authenticator; use OCA\Talk\Manager; use OCA\Talk\Middleware\Attribute\AllowWithoutParticipantWhenPendingInvitation; use OCA\Talk\Middleware\Attribute\FederationSupported; @@ -220,7 +220,7 @@ protected function getLoggedInOrGuest( bool $requireFederationWhenNotLoggedIn = false, ?string $sessionIdParameter = null, ): void { - if ($requireFederationWhenNotLoggedIn && $this->userId === null && !$this->federationAuthenticator->isFederationRequest()) { + if ($requireFederationWhenNotLoggedIn && $this->userId === null && !$this->federationAuthenticator->isAuthenticatedRequest()) { throw new ParticipantNotFoundException(); } @@ -232,7 +232,30 @@ protected function getLoggedInOrGuest( $sessionId = null; if (!$room instanceof Room) { $token = $this->request->getParam('token'); - if (!$this->federationAuthenticator->isFederationRequest()) { + if ($this->userId === null && $this->federationAuthenticator->isAuthenticatedEmailGuest()) { + $room = $this->manager->getRoomByToken($token); + + // After joinRoom() the email guest owns a regular spreed-session for the room. + // Prefer the session lookup so the resolved Participant carries its Session + // (joinCall/chat/etc. require it). Sanity-check that the session actually + // belongs to the authenticated email attendee. + $sessionId = $this->talkSession->getSessionForRoom($token); + if ($sessionId !== null) { + $participant = $this->participantService->getParticipantBySession($room, $sessionId); + $attendee = $participant->getAttendee(); + if ($attendee->getActorType() !== Attendee::ACTOR_EMAILS + || $attendee->getActorId() !== $this->federationAuthenticator->getActorId()) { + throw new ParticipantNotFoundException(); + } + } else { + // No session yet (e.g. between landing on the page and joining); trust the + // session-stored actor id only as long as the email attendee still exists. + $participant = $this->participantService->getParticipantByActor($room, Attendee::ACTOR_EMAILS, $this->federationAuthenticator->getActorId()); + } + + $this->federationAuthenticator->authenticated($room, $participant); + $controller->setParticipant($participant); + } elseif (!$this->federationAuthenticator->isFederationRequest()) { $sessionId = $this->talkSession->getSessionForRoom($token); $room = $this->manager->getRoomForUserByToken($token, $this->userId, $sessionId); } else { diff --git a/tests/php/Chat/Parser/SystemMessageTest.php b/tests/php/Chat/Parser/SystemMessageTest.php index dd8a0413831..2d9da4e639b 100644 --- a/tests/php/Chat/Parser/SystemMessageTest.php +++ b/tests/php/Chat/Parser/SystemMessageTest.php @@ -9,10 +9,10 @@ namespace OCA\Talk\Tests\php\Chat\Parser; use OCA\DAV\CardDAV\PhotoCache; +use OCA\Talk\Authenticator; use OCA\Talk\Chat\ChatManager; use OCA\Talk\Chat\Parser\SystemMessage; use OCA\Talk\Exceptions\ParticipantNotFoundException; -use OCA\Talk\Federation\Authenticator; use OCA\Talk\Model\Attendee; use OCA\Talk\Model\Message; use OCA\Talk\Model\Session; diff --git a/tests/php/Controller/ChatControllerTest.php b/tests/php/Controller/ChatControllerTest.php index a5f285b4dd1..c1a3561f2b6 100644 --- a/tests/php/Controller/ChatControllerTest.php +++ b/tests/php/Controller/ChatControllerTest.php @@ -8,6 +8,7 @@ namespace OCA\Talk\Tests\php\Controller; +use OCA\Talk\Authenticator; use OCA\Talk\Chat\AutoComplete\SearchPlugin; use OCA\Talk\Chat\ChatManager; use OCA\Talk\Chat\MessageParser; @@ -15,7 +16,6 @@ use OCA\Talk\Chat\ReactionManager; use OCA\Talk\Config; use OCA\Talk\Controller\ChatController; -use OCA\Talk\Federation\Authenticator; use OCA\Talk\GuestManager; use OCA\Talk\Manager; use OCA\Talk\MatterbridgeManager; diff --git a/tests/php/Controller/SignalingControllerTest.php b/tests/php/Controller/SignalingControllerTest.php index 971756ac395..bfc0f0890c2 100644 --- a/tests/php/Controller/SignalingControllerTest.php +++ b/tests/php/Controller/SignalingControllerTest.php @@ -8,13 +8,13 @@ namespace OCA\Talk\Tests\php\Controller; +use OCA\Talk\Authenticator; use OCA\Talk\Chat\CommentsManager; use OCA\Talk\Config; use OCA\Talk\Controller\SignalingController; use OCA\Talk\Events\BeforeSignalingResponseSentEvent; use OCA\Talk\Exceptions\ParticipantNotFoundException; use OCA\Talk\Exceptions\RoomNotFoundException; -use OCA\Talk\Federation\Authenticator; use OCA\Talk\Manager; use OCA\Talk\Model\Attendee; use OCA\Talk\Model\AttendeeMapper; diff --git a/tests/php/Recording/BackendNotifierTest.php b/tests/php/Recording/BackendNotifierTest.php index cba83431212..1a9e12cb581 100644 --- a/tests/php/Recording/BackendNotifierTest.php +++ b/tests/php/Recording/BackendNotifierTest.php @@ -8,9 +8,9 @@ namespace OCA\Talk\Tests\php\Recording; +use OCA\Talk\Authenticator; use OCA\Talk\Chat\CommentsManager; use OCA\Talk\Config; -use OCA\Talk\Federation\Authenticator; use OCA\Talk\Manager; use OCA\Talk\Model\AttendeeMapper; use OCA\Talk\Model\SessionMapper; diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index a4d85acfd27..48dd33304db 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -8,6 +8,11 @@ + + + request->server]]> + + @@ -82,11 +87,6 @@ - - - request->server]]> - - From 68a1df5d964a345e18877b5dd92a19dbea38197a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 12 May 2026 13:34:17 +0200 Subject: [PATCH 4/6] test(email): Add integration test Signed-off-by: Joas Schilling --- .../features/bootstrap/FeatureContext.php | 44 ++++++++++++++++--- .../conversation-3/invite-email.feature | 36 +++++++++++++++ .../spreedcheats/appinfo/routes.php | 1 + .../lib/Controller/ApiController.php | 22 ++++++++++ 4 files changed, 97 insertions(+), 6 deletions(-) diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 395e6adad6d..7810183b21d 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -173,6 +173,10 @@ public static function getSessionIdForUser(string $user): string { } public function getAttendeeId(string $type, string $id, string $room, ?string $user = null): int { + if ($type === 'emails' && str_contains($id, '@')) { + $id = hash('sha256', $id); + } + if ($type === 'federated_users') { if (!str_contains($id, '@')) { $id .= '@' . $this->remoteServerUrl; @@ -1363,17 +1367,45 @@ public function userSessionState(string $user, int $state, string $identifier, i #[Then('/^user "([^"]*)" views call-URL of room "([^"]*)" with (\d+)$/')] public function userViewsCallURL(string $user, string $identifier, int $statusCode, ?TableNode $formData = null): void { + $queryParams = []; + $assertions = []; + if ($formData instanceof TableNode) { + foreach ($formData->getRows() as $row) { + if (count($row) === 2) { + $queryParams[$row[0]] = $row[1]; + } else { + $assertions[] = $row[0]; + } + } + } + + $token = self::$identifierToToken[$identifier] ?? $identifier; + + if (isset($queryParams['email']) && !isset($queryParams['access'])) { + $previousUser = $this->setCurrentUser('admin'); + $this->sendRequest('GET', '/apps/spreedcheats/email/access?' . http_build_query([ + 'token' => $token, + 'email' => $queryParams['email'], + ])); + $this->assertStatusCode($this->response, 200); + $data = $this->getDataFromResponse($this->response); + $queryParams['access'] = $data['access_token']; + $this->setCurrentUser($previousUser); + } + $this->setCurrentUser($user); - $this->sendFrontpageRequest( - 'GET', '/call/' . (self::$identifierToToken[$identifier] ?? $identifier) - ); + $url = '/call/' . $token; + if (!empty($queryParams)) { + $url .= '?' . http_build_query($queryParams); + } + $this->sendFrontpageRequest('GET', $url); $this->assertStatusCode($this->response, $statusCode); - if ($formData instanceof TableNode) { + if (!empty($assertions)) { $content = $this->response->getBody()->getContents(); - foreach ($formData->getRows() as $line) { - Assert::assertStringContainsString($line[0], $content); + foreach ($assertions as $assertion) { + Assert::assertStringContainsString($assertion, $content); } } } diff --git a/tests/integration/features/conversation-3/invite-email.feature b/tests/integration/features/conversation-3/invite-email.feature index 6c024b2f5c1..ec9726dae6a 100644 --- a/tests/integration/features/conversation-3/invite-email.feature +++ b/tests/integration/features/conversation-3/invite-email.feature @@ -22,3 +22,39 @@ Feature: conversation-3/invite-email # Reinvite failure When user "participant1" resends invite for room "room" with 404 (v4) | attendeeId | not-found@example.tld | + + Scenario: Email guest joins a private conversation through their invite link + Given user "participant1" creates room "room" (v4) + | roomType | 2 | + | roomName | room | + And user "participant1" adds email "test@example.tld" to room "room" with 200 (v4) + And user "participant1" is participant of room "room" (v4) + | type | + | 2 | + And user "participant1" sees the following attendees in room "room" with 200 (v4) + | participantType | actorType | actorId | invitedActorId | + | 4 | emails | SHA256(test@example.tld) | test@example.tld | + | 1 | users | participant1 | ABSENT | + When user "guest" views call-URL of room "room" with 200 + | email | test@example.tld | + Then user "guest" joins room "room" with 200 (v4) + Then user "guest" joins call "room" with 200 (v4) + | flags | 1 | + Then user "participant1" sees 1 peers in call "room" with 200 (v4) + Then user "guest" leaves room "room" with 200 (v4) + + Scenario: Email guest cannot rejoin after being removed from a private conversation + Given user "participant1" creates room "room" (v4) + | roomType | 2 | + | roomName | room | + And user "participant1" adds email "test@example.tld" to room "room" with 200 (v4) + And user "participant1" sees the following attendees in room "room" with 200 (v4) + | participantType | actorType | actorId | invitedActorId | + | 4 | emails | SHA256(test@example.tld) | test@example.tld | + | 1 | users | participant1 | ABSENT | + And user "guest" views call-URL of room "room" with 200 + | email | test@example.tld | + And user "guest" joins room "room" with 200 (v4) + And user "guest" leaves room "room" with 200 (v4) + When user "participant1" removes email "test@example.tld" from room "room" with 200 (v4) + Then user "guest" joins room "room" with 404 (v4) diff --git a/tests/integration/spreedcheats/appinfo/routes.php b/tests/integration/spreedcheats/appinfo/routes.php index 740d80ade91..0b5b1114f53 100644 --- a/tests/integration/spreedcheats/appinfo/routes.php +++ b/tests/integration/spreedcheats/appinfo/routes.php @@ -10,6 +10,7 @@ 'ocs' => [ ['name' => 'Api#resetSpreed', 'url' => '/', 'verb' => 'DELETE'], ['name' => 'Api#ageChat', 'url' => '/age', 'verb' => 'POST'], + ['name' => 'Api#emailAccessToken', 'url' => '/email/access', 'verb' => 'GET'], ['name' => 'Api#forgedFederationLeave', 'url' => '/forged/federation/active', 'verb' => 'DELETE'], ['name' => 'Api#createEventInCalendar', 'url' => '/calendar', 'verb' => 'POST'], ['name' => 'Api#createDashboardEvents', 'url' => '/dashboardEvents', 'verb' => 'POST'], diff --git a/tests/integration/spreedcheats/lib/Controller/ApiController.php b/tests/integration/spreedcheats/lib/Controller/ApiController.php index 3d27daa680b..aefcc384740 100644 --- a/tests/integration/spreedcheats/lib/Controller/ApiController.php +++ b/tests/integration/spreedcheats/lib/Controller/ApiController.php @@ -215,6 +215,28 @@ public function ageChat(string $token, int $hours): DataResponse { return new DataResponse(); } + public function emailAccessToken(string $token, string $email): DataResponse { + $query = $this->db->getQueryBuilder(); + $query->select('a.access_token') + ->from('talk_rooms', 'r') + ->leftJoin('r', 'talk_attendees', 'a', $query->expr()->andX( + $query->expr()->eq('a.room_id', 'r.id'), + $query->expr()->eq('a.actor_type', $query->createNamedParameter(Attendee::ACTOR_EMAILS)), + $query->expr()->eq('a.actor_id', $query->createNamedParameter(hash('sha256', $email))), + )) + ->where($query->expr()->eq('r.token', $query->createNamedParameter($token))); + + $result = $query->executeQuery(); + $accessToken = $result->fetchOne(); + $result->closeCursor(); + + if (!$accessToken) { + return new DataResponse(null, Http::STATUS_NOT_FOUND); + } + + return new DataResponse(['access_token' => $accessToken]); + } + public function forgedFederationLeave(string $token, string $actingUser, string $targetUser): DataResponse { $query = $this->db->getQueryBuilder(); $query->select('id') From 94d0f0e0265cb052f31c84c689e20b9a4e01b8f4 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 12 May 2026 14:39:00 +0200 Subject: [PATCH 5/6] fix(email): Fix missing conversation name in invitation email Signed-off-by: Joas Schilling --- lib/GuestManager.php | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/GuestManager.php b/lib/GuestManager.php index 4c4757df761..07ccfa80532 100644 --- a/lib/GuestManager.php +++ b/lib/GuestManager.php @@ -222,27 +222,28 @@ public function sendEmailInvitation(Room $room, Participant $participant): void $user = $this->userSession->getUser(); $invitee = $user instanceof IUser ? $user->getDisplayName() : ''; + $roomName = $room->getDisplayName('', true); $template = $this->mailer->createEMailTemplate('Talk.InviteByEmail', [ 'invitee' => $invitee, - 'roomName' => $room->getDisplayName(''), + 'roomName' => $roomName, 'roomLink' => $link, 'email' => $email, 'pin' => $pin, ]); if ($user instanceof IUser) { - $subject = $this->l->t('%1$s invited you to conversation "%2$s".', [$user->getDisplayName(), $room->getDisplayName('')]); + $subject = $this->l->t('%1$s invited you to conversation "%2$s".', [$user->getDisplayName(), $roomName]); $message->setFrom([Util::getDefaultEmailAddress('no-reply') => $user->getDisplayName()]); } else { - $subject = $this->l->t('You were invited to conversation "%s".', $room->getDisplayName('')); + $subject = $this->l->t('You were invited to conversation "%s".', $roomName); $message->setFrom([Util::getDefaultEmailAddress('no-reply') => $this->defaults->getName()]); } $template->setSubject($subject); $template->addHeader(); $template->addHeading( - htmlspecialchars($room->getDisplayName('')), + htmlspecialchars($roomName), $this->l->t('Conversation invitation') ); $template->addBodyText( @@ -298,7 +299,7 @@ public function sendEmailInvitation(Room $room, Participant $participant): void $this->l->t('Click the link below to join the lobby now.') ); $template->addBodyButton( - $this->l->t('Join lobby for "%s"', [$room->getDisplayName('')]), + $this->l->t('Join lobby for "%s"', [$roomName]), $link ); } else { @@ -308,7 +309,7 @@ public function sendEmailInvitation(Room $room, Participant $participant): void $this->l->t('Click the link below to join the conversation now.') ); $template->addBodyButton( - $this->l->t('Join "%s"', [$room->getDisplayName('')]), + $this->l->t('Join "%s"', [$roomName]), $link ); } From 572ebb47326048c4bf26e8c5d02b1157264c7408 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 13 May 2026 13:15:43 +0200 Subject: [PATCH 6/6] fix(email): Fix reloading the page as an email guest Signed-off-by: Joas Schilling --- lib/Controller/ChatController.php | 2 +- lib/Middleware/InjectionMiddleware.php | 15 ++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/Controller/ChatController.php b/lib/Controller/ChatController.php index e29c2c556c1..332780748e3 100644 --- a/lib/Controller/ChatController.php +++ b/lib/Controller/ChatController.php @@ -1280,7 +1280,7 @@ public function getMessageContext( } } - $currentUser = $this->userManager->get($this->userId); + $currentUser = $this->userId !== null ? $this->userManager->getExistingUser($this->userId) : null; if ($messageId === 0) { // Guest in a fully expired chat, no history, just loading the chat from beginning for now $commentsHistory = $commentsFuture = []; diff --git a/lib/Middleware/InjectionMiddleware.php b/lib/Middleware/InjectionMiddleware.php index 22839ba0055..ae95d5a255c 100644 --- a/lib/Middleware/InjectionMiddleware.php +++ b/lib/Middleware/InjectionMiddleware.php @@ -241,11 +241,16 @@ protected function getLoggedInOrGuest( // belongs to the authenticated email attendee. $sessionId = $this->talkSession->getSessionForRoom($token); if ($sessionId !== null) { - $participant = $this->participantService->getParticipantBySession($room, $sessionId); - $attendee = $participant->getAttendee(); - if ($attendee->getActorType() !== Attendee::ACTOR_EMAILS - || $attendee->getActorId() !== $this->federationAuthenticator->getActorId()) { - throw new ParticipantNotFoundException(); + try { + $participant = $this->participantService->getParticipantBySession($room, $sessionId); + $attendee = $participant->getAttendee(); + if ($attendee->getActorType() !== Attendee::ACTOR_EMAILS + || $attendee->getActorId() !== $this->federationAuthenticator->getActorId()) { + throw new ParticipantNotFoundException('3'); + } + } catch (ParticipantNotFoundException) { + // Guest might be reloading so the session has the old Talk session while the join happens in parallel + $participant = $this->participantService->getParticipantByActor($room, Attendee::ACTOR_EMAILS, $this->federationAuthenticator->getActorId()); } } else { // No session yet (e.g. between landing on the page and joining); trust the