From 541ee13780df4a73878ab408d16f9ff641e6368d Mon Sep 17 00:00:00 2001 From: chandi Langecker Date: Mon, 21 Nov 2022 20:01:53 +0100 Subject: [PATCH] sessions: ignore self-emitted update events Signed-off-by: chandi Langecker --- lib/Db/SessionMapper.php | 2 +- lib/Listeners/LiveUpdateListener.php | 24 +++++++++++---- lib/Service/SessionService.php | 32 ++++++++++++++++--- src/sessions.js | 46 +++++++++++++++++----------- 4 files changed, 74 insertions(+), 30 deletions(-) diff --git a/lib/Db/SessionMapper.php b/lib/Db/SessionMapper.php index b4424a499..c5606db71 100644 --- a/lib/Db/SessionMapper.php +++ b/lib/Db/SessionMapper.php @@ -56,7 +56,7 @@ class SessionMapper extends QBMapper { public function findAllActive($boardId) { $qb = $this->db->getQueryBuilder(); - $qb->select('id', 'board_id', 'last_contact', 'user_id') + $qb->select('id', 'board_id', 'last_contact', 'user_id', 'token') ->from($this->getTableName()) ->where($qb->expr()->eq('board_id', $qb->createNamedParameter($boardId))) ->andWhere($qb->expr()->gt('last_contact', $qb->createNamedParameter(time() - SessionService::SESSION_VALID_TIME))) diff --git a/lib/Listeners/LiveUpdateListener.php b/lib/Listeners/LiveUpdateListener.php index 06eecfe4f..ec2f57750 100644 --- a/lib/Listeners/LiveUpdateListener.php +++ b/lib/Listeners/LiveUpdateListener.php @@ -33,25 +33,30 @@ use OCA\Deck\Service\SessionService; use OCA\NotifyPush\Queue\IQueue; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; +use OCP\IRequest; use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; class LiveUpdateListener implements IEventListener { - private string $userId; private LoggerInterface $logger; private SessionService $sessionService; + private IRequest $request; private $queue; - public function __construct(ContainerInterface $container, SessionService $sessionService, $userId) { + public function __construct( + ContainerInterface $container, + IRequest $request, + SessionService $sessionService + ) { try { $this->queue = $container->get(IQueue::class); } catch (\Exception $e) { // most likely notify_push is not installed. return; } - $this->userId = $userId; $this->logger = $container->get(LoggerInterface::class); $this->sessionService = $sessionService; + $this->request = $request; } public function handle(Event $event): void { @@ -61,10 +66,17 @@ class LiveUpdateListener implements IEventListener { } try { - if ($event instanceof SessionCreatedEvent || $event instanceof SessionClosedEvent) { - $this->sessionService->notifyAllSessions($this->queue, $event->getBoardId(), NotifyPushEvents::DeckBoardUpdate, $event->getUserId(), [ + // the web frontend is adding the Session-ID as a header on every request + // TODO: verify the token! this currently allows to spoof a token from someone + // else, preventing this person from getting any live updates + $causingSessionToken = $this->request->getHeader('x-nc-deck-session'); + if ( + $event instanceof SessionCreatedEvent || + $event instanceof SessionClosedEvent + ) { + $this->sessionService->notifyAllSessions($this->queue, $event->getBoardId(), NotifyPushEvents::DeckBoardUpdate, [ 'id' => $event->getBoardId() - ]); + ], $causingSessionToken); } } catch (\Exception $e) { $this->logger->error('Error when handling live update event', ['exception' => $e]); diff --git a/lib/Service/SessionService.php b/lib/Service/SessionService.php index 2132f6e4f..a6287a050 100644 --- a/lib/Service/SessionService.php +++ b/lib/Service/SessionService.php @@ -57,11 +57,11 @@ class SessionService { $this->eventDispatcher = $eventDispatcher; } - public function initSession($boardId): Session { + public function initSession(int $boardId): Session { $session = new Session(); $session->setBoardId($boardId); $session->setUserId($this->userId); - $session->setToken($this->secureRandom->generate(64)); + $session->setToken($this->secureRandom->generate(32)); $session->setLastContact($this->timeFactory->getTime()); $session = $this->sessionMapper->insert($session); @@ -91,12 +91,34 @@ class SessionService { return $this->sessionMapper->deleteInactive(); } - public function notifyAllSessions(IQueue $queue, int $boardId, $event, $excludeUserId, $body) { + public function notifyAllSessions(IQueue $queue, int $boardId, $event, $body, $causingSessionToken = null) { $activeSessions = $this->sessionMapper->findAllActive($boardId); - + $userIds = []; foreach ($activeSessions as $session) { + if ($causingSessionToken && $session->getToken() === $causingSessionToken) { + // Limitation: + // If the same user has a second session active, the session $causingSessionToken + // still gets notified due to the current fact, that notify_push only supports + // to specify users, not individual sessions + // https://github.com/nextcloud/notify_push/issues/195 + continue; + } + + // don't notify the same user multiple times + if (!in_array($session->getUserId(), $userIds)) { + $userIds[] = $session->getUserId(); + } + } + + if ($causingSessionToken) { + // we only send a slice of the session token to everyone + // since knowledge of the full token allows everyone + // to close the session maliciously + $body['_causingSessionToken'] = substr($causingSessionToken, 0, 12); + } + foreach ($userIds as $userId) { $queue->push('notify_custom', [ - 'user' => $session->getUserId(), + 'user' => $userId, 'message' => $event, 'body' => $body ]); diff --git a/src/sessions.js b/src/sessions.js index b5a2f96f6..408eaeba7 100644 --- a/src/sessions.js +++ b/src/sessions.js @@ -21,13 +21,36 @@ import { listen } from '@nextcloud/notify_push' import { sessionApi } from './services/SessionApi.js' import store from './store/main.js' +import axios from '@nextcloud/axios' const SESSION_INTERVAL = 90 // in seconds let hasPush = false +/** + * used to verify, whether an event is originated by ourselves + * + * @param token + */ +function isOurSessionToken(token) { + if (axios.defaults.headers['x-nc-deck-session'] + && axios.defaults.headers['x-nc-deck-session'].startsWith(token)) { + return true + } else { + return false + } +} + hasPush = listen('deck_board_update', (name, body) => { - triggerDeckReload(body.id) + // ignore update events which we have triggered ourselves + if (isOurSessionToken(body._causingSessionToken)) return + + // only handle update events for the currently open board + const currentBoardId = store.state.currentBoard?.id + if (body.id !== currentBoardId) return + + store.dispatch('refreshBoard', currentBoardId) +}) }) /** @@ -38,21 +61,6 @@ export function isNotifyPushEnabled() { return hasPush } -/** - * Triggers a reload of the deck, if the provided id - * matches the current open deck - * - * @param triggeredBoardId - */ -export function triggerDeckReload(triggeredBoardId) { - const currentBoardId = store.state.currentBoard?.id - - // only handle update events for the currently open board - if (triggeredBoardId !== currentBoardId) return - - store.dispatch('refreshBoard', currentBoardId) -} - /** * * @param boardId @@ -74,6 +82,7 @@ export function createSession(boardId) { tokenPromise = sessionApi.createSession(boardId).then(res => res.token) tokenPromise.then((t) => { token = t + axios.defaults.headers['x-nc-deck-session'] = t }) } create() @@ -95,12 +104,13 @@ export function createSession(boardId) { // periodically notify the server that we are still here let interval = setInterval(ensureSession, SESSION_INTERVAL * 1000) - // close session when + // close session when tab gets hidden/inactive const visibilitychangeListener = () => { if (document.visibilityState === 'hidden') { sessionApi.closeSessionViaBeacon(boardId, token) tokenPromise = null token = null + delete axios.defaults.headers['x-nc-deck-session'] // stop session refresh interval clearInterval(interval) @@ -110,7 +120,7 @@ export function createSession(boardId) { // we must assume that the websocket connection was // paused and we have missed updates in the meantime. - triggerDeckReload() + store.dispatch('refreshBoard', store.state.currentBoard?.id) // restart session refresh interval interval = setInterval(ensureSession, SESSION_INTERVAL * 1000)