fix: Banned user cannot be added to the room

lib/Controller/RoomController.php

@@ -1070,7 +1070,7 @@ protected function formatParticipantList(array $participants, bool $includeStatu
 	 * @return DataResponse<Http::STATUS_OK, array{type: int}|array<empty>, array{}>|DataResponse<Http::STATUS_NOT_FOUND|Http::STATUS_NOT_IMPLEMENTED, array<empty>, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array{error?: string}, array{}>
 	 *
 	 * 200: Participant successfully added
-	 * 400: Adding participant is not possible
+	 * 400: Adding participant is not possible, e.g. when the user is banned (check error attribute of response for detail key)
 	 * 404: User, group or other target to invite was not found
 	 * 501: SIP dial-out is not configured
 	 */
@@ -1117,6 +1117,11 @@ public function addParticipantToRoom(string $newParticipant, string $source = 'u
 				return new DataResponse([], Http::STATUS_NOT_FOUND);
 			}
 
+			//Check if the user is banned
+			if ($this->banService->isActorBanned($this->room, Attendee::ACTOR_USERS, $newUser->getUID())) {
+				return new DataResponse(['error' => 'ban'], Http::STATUS_BAD_REQUEST);
+			}
+
 			$participantsToAdd[] = [
 				'actorType' => Attendee::ACTOR_USERS,
 				'actorId' => $newUser->getUID(),

lib/Listener/AMembershipListener.php

@@ -16,6 +16,7 @@
 use OCA\Talk\Model\Attendee;
 use OCA\Talk\Participant;
 use OCA\Talk\Room;
+use OCA\Talk\Service\BanService;
 use OCA\Talk\Service\ParticipantService;
 use OCP\App\IAppManager;
 use OCP\EventDispatcher\Event;
@@ -35,6 +36,8 @@ public function __construct(
 		protected IAppManager $appManager,
 		protected IGroupManager $groupManager,
 		protected ParticipantService $participantService,
+		protected BanService $banService,
+		protected LoggerInterface $logger,
 	) {
 	}
 

lib/Listener/CircleMembershipListener.php

@@ -15,13 +15,15 @@
 use OCA\Talk\Manager;
 use OCA\Talk\Model\Attendee;
 use OCA\Talk\Participant;
+use OCA\Talk\Service\BanService;
 use OCA\Talk\Service\ParticipantService;
 use OCP\App\IAppManager;
 use OCP\EventDispatcher\Event;
 use OCP\IGroupManager;
 use OCP\ISession;
 use OCP\IUser;
 use OCP\IUserManager;
+use Psr\Log\LoggerInterface;
 
 class CircleMembershipListener extends AMembershipListener {
 
@@ -30,14 +32,18 @@ public function __construct(
 		IAppManager $appManager,
 		IGroupManager $groupManager,
 		ParticipantService $participantService,
+		BanService $banService,
+		LoggerInterface $logger,
 		private IUserManager $userManager,
 		private ISession $session,
 	) {
 		parent::__construct(
 			$manager,
 			$appManager,
 			$groupManager,
-			$participantService
+			$participantService,
+			$banService,
+			$logger,
 		);
 	}
 
@@ -101,6 +107,10 @@ protected function addingCircleMemberEvent(AddingCircleMemberEvent $event): void
 	}
 
 	protected function addNewMemberToRooms(array $rooms, Member $member): void {
+		if (empty($rooms)) {
+			return;
+		}
+
 		if ($member->getUserType() !== Member::TYPE_USER || $member->getUserId() === '') {
 			// Not a user?
 			return;
@@ -111,7 +121,13 @@ protected function addNewMemberToRooms(array $rooms, Member $member): void {
 			return;
 		}
 
+		$bannedRoomIds = $this->banService->getBannedRoomsForUserId($user->getUID());
 		foreach ($rooms as $room) {
+			if (isset($bannedRoomIds[$room->getId()])) {
+				$this->logger->warning('User ' . $user->getUID() . ' is banned from conversation ' . $room->getToken() . ' and was skipped while adding them to circle ' . $member->getCircle()->getDisplayName());
+				continue;
+			}
+
 			try {
 				$participant = $room->getParticipant($member->getUserId());
 				if ($participant->getAttendee()->getParticipantType() === Participant::USER_SELF_JOINED) {

lib/Listener/GroupMembershipListener.php

@@ -30,7 +30,17 @@ public function handle(Event $event): void {
 	protected function addNewMemberToRooms(IGroup $group, IUser $user): void {
 		$rooms = $this->manager->getRoomsForActor(Attendee::ACTOR_GROUPS, $group->getGID());
 
+		if (empty($rooms)) {
+			return;
+		}
+
+		$bannedRoomIds = $this->banService->getBannedRoomsForUserId($user->getUID());
 		foreach ($rooms as $room) {
+			if (isset($bannedRoomIds[$room->getId()])) {
+				$this->logger->warning('User ' . $user->getUID() . ' is banned from conversation ' . $room->getToken() . ' and was skipped while adding them to group ' . $group->getDisplayName());
+				continue;
+			}
+
 			try {
 				$participant = $this->participantService->getParticipant($room, $user->getUID());
 				if ($participant->getAttendee()->getParticipantType() === Participant::USER_SELF_JOINED) {

lib/Model/BanMapper.php

@@ -53,6 +53,17 @@ public function findByRoomId(int $roomId, ?string $bannedActorType = null): arra
 		return $this->findEntities($query);
 	}
 
+	public function findByUserId(string $userId): array {
+		$query = $this->db->getQueryBuilder();
+		$query->select('*')
+			->from($this->getTableName())
+			->where($query->expr()->eq('banned_actor_type', $query->createNamedParameter(Attendee::ACTOR_USERS, IQueryBuilder::PARAM_STR)))
+			->andWhere($query->expr()->eq('banned_actor_id', $query->createNamedParameter($userId, IQueryBuilder::PARAM_STR)))
+			->orderBy('id', 'ASC');
+
+		return $this->findEntities($query);
+	}
+
 	/**
 	 * @throws DoesNotExistException
 	 */

lib/Service/BanService.php

@@ -188,6 +188,20 @@ public function throwIfActorIsBanned(Room $room, ?string $userId): void {
 		}
 	}
 
+	/**
+	 * Check if the actor is banned without logging
+	 *
+	 * @return bool True if the actor is banned, false otherwise
+	 */
+	public function isActorBanned(Room $room, string $actorType, string $actorId): bool {
+		try {
+			$this->banMapper->findForBannedActorAndRoom($actorType, $actorId, $room->getId());
+			return true;
+		} catch (DoesNotExistException) {
+			return false;
+		}
+	}
+
 	/**
 	 * Retrieve all bans for a specific room.
 	 *
@@ -197,6 +211,26 @@ public function getBansForRoom(int $roomId): array {
 		return $this->banMapper->findByRoomId($roomId);
 	}
 
+	/**
+	 * Retrieve all banned userIDs for a specific room.
+	 *
+	 * @return array<string, mixed> Key is the user ID
+	 */
+	public function getBannedUserIdsForRoom(int $roomId): array {
+		$bans = $this->banMapper->findByRoomId($roomId, Attendee::ACTOR_USERS);
+		return array_flip(array_map(static fn (Ban $ban) => $ban->getBannedActorId(), $bans));
+	}
+
+	/**
+	 * Retrieve all room IDs a user is banned from
+	 *
+	 * @return array<int, mixed> Key is the room ID
+	 */
+	public function getBannedRoomsForUserId(string $userId): array {
+		$bans = $this->banMapper->findByUserId($userId);
+		return array_flip(array_map(static fn (Ban $ban) => $ban->getRoomId(), $bans));
+	}
+
 	/**
 	 * Retrieve a ban by its ID and delete it.
 	 */

lib/Service/ParticipantService.php

@@ -47,6 +47,7 @@
 use OCA\Talk\Manager;
 use OCA\Talk\Model\Attendee;
 use OCA\Talk\Model\AttendeeMapper;
+use OCA\Talk\Model\Ban;
 use OCA\Talk\Model\BreakoutRoom;
 use OCA\Talk\Model\SelectHelper;
 use OCA\Talk\Model\Session;
@@ -73,6 +74,7 @@
 use OCP\Server;
 use OCP\UserStatus\IManager as IUserStatusManager;
 use OCP\UserStatus\IUserStatus;
+use Psr\Log\LoggerInterface;
 
 class ParticipantService {
 
@@ -98,6 +100,7 @@ public function __construct(
 		private ITimeFactory $timeFactory,
 		private ICacheFactory $cacheFactory,
 		private IUserStatusManager $userStatusManager,
+		private LoggerInterface $logger,
 	) {
 	}
 
@@ -456,7 +459,7 @@ public function joinRoomAsNewGuest(RoomService $roomService, Room $room, string
 	 * @throws CannotReachRemoteException thrown when sending the federation request didn't work
 	 * @throws \Exception thrown if $addedBy is not set when adding a federated user
 	 */
-	public function addUsers(Room $room, array $participants, ?IUser $addedBy = null): array {
+	public function addUsers(Room $room, array $participants, ?IUser $addedBy = null, bool $bansAlreadyChecked = false): array {
 		if (empty($participants)) {
 			return [];
 		}
@@ -466,10 +469,20 @@ public function addUsers(Room $room, array $participants, ?IUser $addedBy = null
 			$lastMessage = (int) $room->getLastMessage()->getId();
 		}
 
+		$bannedUserIds = [];
+		if (!$bansAlreadyChecked) {
+			$banService = Server::get(BanService::class);
+			$bannedUserIds = $banService->getBannedUserIdsForRoom($room->getId());
+		}
 		$attendees = [];
 		foreach ($participants as $participant) {
 			$readPrivacy = Participant::PRIVACY_PUBLIC;
 			if ($participant['actorType'] === Attendee::ACTOR_USERS) {
+				if (isset($bannedUserIds[$participant['actorId']])) {
+					$this->logger->warning('User ' . $participant['actorId'] . ' is banned from conversation ' . $room->getToken() . ' and was skipped while adding users');
+					continue;
+				}
+
 				$readPrivacy = $this->talkConfig->getUserReadPrivacy($participant['actorId']);
 			} elseif ($participant['actorType'] === Attendee::ACTOR_FEDERATED_USERS) {
 				if ($addedBy === null) {
@@ -603,6 +616,8 @@ public function addGroup(Room $room, IGroup $group, array $existingParticipants
 			$existingParticipants = $this->getParticipantsForRoom($room);
 		}
 
+		$banService = Server::get(BanService::class);
+		$bannedUserIds = $banService->getBannedUserIdsForRoom($room->getId());
 		$participantsByUserId = [];
 		foreach ($existingParticipants as $participant) {
 			if ($participant->getAttendee()->getActorType() === Attendee::ACTOR_USERS) {
@@ -622,6 +637,11 @@ public function addGroup(Room $room, IGroup $group, array $existingParticipants
 				continue;
 			}
 
+			if (isset($bannedUserIds[$user->getUID()])) {
+				$this->logger->warning('User ' . $user->getUID() . ' is banned from conversation ' . $room->getToken() . ' and was skipped while adding group ' . $group->getDisplayName());
+				continue;
+			}
+
 			$newParticipants[] = [
 				'actorType' => Attendee::ACTOR_USERS,
 				'actorId' => $user->getUID(),
@@ -646,7 +666,7 @@ public function addGroup(Room $room, IGroup $group, array $existingParticipants
 			$this->dispatcher->dispatchTyped($attendeeEvent);
 		}
 
-		$this->addUsers($room, $newParticipants);
+		$this->addUsers($room, $newParticipants, bansAlreadyChecked: true);
 	}
 
 	/**
@@ -688,6 +708,8 @@ public function addCircle(Room $room, Circle $circle, array $existingParticipant
 			$existingParticipants = $this->getParticipantsForRoom($room);
 		}
 
+		$banService = Server::get(BanService::class);
+		$bannedUserIds = $banService->getBannedUserIdsForRoom($room->getId());
 		$participantsByUserId = [];
 		foreach ($existingParticipants as $participant) {
 			if ($participant->getAttendee()->getActorType() === Attendee::ACTOR_USERS) {
@@ -723,6 +745,11 @@ public function addCircle(Room $room, Circle $circle, array $existingParticipant
 				continue;
 			}
 
+			if (isset($bannedUserIds[$user->getUID()])) {
+				$this->logger->warning('User ' . $user->getUID() . ' is banned from conversation ' . $room->getToken() . ' and was skipped while adding circle ' . $circle->getDisplayName());
+				continue;
+			}
+
 			$newParticipants[] = [
 				'actorType' => Attendee::ACTOR_USERS,
 				'actorId' => $user->getUID(),
@@ -747,7 +774,7 @@ public function addCircle(Room $room, Circle $circle, array $existingParticipant
 			$this->dispatcher->dispatchTyped($attendeeEvent);
 		}
 
-		$this->addUsers($room, $newParticipants);
+		$this->addUsers($room, $newParticipants, bansAlreadyChecked: true);
 	}
 
 	/**

openapi-full.json

@@ -12557,7 +12557,7 @@
                         }
                     },
                     "400": {
-                        "description": "Adding participant is not possible",
+                        "description": "Adding participant is not possible, e.g. when the user is banned (check error attribute of response for detail key)",
                         "content": {
                             "application/json": {
                                 "schema": {

openapi.json

@@ -12677,7 +12677,7 @@
                         }
                     },
                     "400": {
-                        "description": "Adding participant is not possible",
+                        "description": "Adding participant is not possible, e.g. when the user is banned (check error attribute of response for detail key)",
                         "content": {
                             "application/json": {
                                 "schema": {

src/types/openapi/openapi-full.ts

@@ -6759,7 +6759,7 @@ export interface operations {
                     };
                 };
             };
-            /** @description Adding participant is not possible */
+            /** @description Adding participant is not possible, e.g. when the user is banned (check error attribute of response for detail key) */
             400: {
                 headers: {
                     [name: string]: unknown;

src/types/openapi/openapi.ts

@@ -6337,7 +6337,7 @@ export interface operations {
                     };
                 };
             };
-            /** @description Adding participant is not possible */
+            /** @description Adding participant is not possible, e.g. when the user is banned (check error attribute of response for detail key) */
             400: {
                 headers: {
                     [name: string]: unknown;

tests/integration/features/bootstrap/FeatureContext.php

@@ -900,6 +900,7 @@ protected function assertAttendeeList(string $identifier, ?TableNode $formData,
 				}
 				return $attendee;
 			}, $formData->getHash(), $result);
+			$expected = array_filter($expected);
 
 			$result = array_map(function ($attendee) {
 				if (isset($attendee['permissions'])) {
@@ -918,7 +919,7 @@ protected function assertAttendeeList(string $identifier, ?TableNode $formData,
 				'expected' => $expected,
 				'actual' => $attendees,
 				'result' => $result,
-			]));
+			], true));
 		} else {
 			Assert::assertNull($formData);
 		}