From dcd82100278bb78dbb814e5e130705a5b07cd348 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 11 Sep 2017 17:17:13 +0200 Subject: [PATCH 01/19] Start implementing room passwords Signed-off-by: Joas Schilling --- lib/Controller/CallController.php | 24 ++++--- lib/Exceptions/InvalidPasswordException.php | 27 +++++++ lib/Manager.php | 3 +- .../Version2001003Date20170911145301.php | 70 +++++++++++++++++++ lib/Room.php | 53 +++++++++++++- 5 files changed, 164 insertions(+), 13 deletions(-) create mode 100644 lib/Exceptions/InvalidPasswordException.php create mode 100644 lib/Migration/Version2001003Date20170911145301.php diff --git a/lib/Controller/CallController.php b/lib/Controller/CallController.php index 2cc21f3756..f076cb55a2 100644 --- a/lib/Controller/CallController.php +++ b/lib/Controller/CallController.php @@ -25,6 +25,7 @@ namespace OCA\Spreed\Controller; +use OCA\Spreed\Exceptions\InvalidPasswordException; use OCA\Spreed\Exceptions\RoomNotFoundException; use OCA\Spreed\Manager; use OCA\Spreed\Signalling\Messages; @@ -118,24 +119,29 @@ class CallController extends OCSController { * @UseSession * * @param string $token + * @param string $password * @return DataResponse */ - public function joinCall($token) { + public function joinCall($token, $password) { try { $room = $this->manager->getRoomForParticipantByToken($token, $this->userId); } catch (RoomNotFoundException $e) { return new DataResponse([], Http::STATUS_NOT_FOUND); } - if ($this->userId !== null) { - $sessionIds = $this->manager->getSessionIdsForUser($this->userId); - $newSessionId = $room->enterRoomAsUser($this->userId); - - if (!empty($sessionIds)) { - $this->messages->deleteMessages($sessionIds); + try { + if ($this->userId !== null) { + $sessionIds = $this->manager->getSessionIdsForUser($this->userId); + $newSessionId = $room->enterRoomAsUser($this->userId, $password); + + if (!empty($sessionIds)) { + $this->messages->deleteMessages($sessionIds); + } + } else { + $newSessionId = $room->enterRoomAsGuest($password); } - } else { - $newSessionId = $room->enterRoomAsGuest(); + } catch (InvalidPasswordException $e) { + return new DataResponse([], Http::STATUS_FORBIDDEN); } $this->session->set('spreed-session', $newSessionId); diff --git a/lib/Exceptions/InvalidPasswordException.php b/lib/Exceptions/InvalidPasswordException.php new file mode 100644 index 0000000000..44cfe11b94 --- /dev/null +++ b/lib/Exceptions/InvalidPasswordException.php @@ -0,0 +1,27 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + + +namespace OCA\Spreed\Exceptions; + +class InvalidPasswordException extends \Exception { + +} diff --git a/lib/Manager.php b/lib/Manager.php index 440b83d0cb..a031b7e79d 100644 --- a/lib/Manager.php +++ b/lib/Manager.php @@ -60,7 +60,7 @@ class Manager { * @return Room */ protected function createRoomObject(array $row) { - return new Room($this->db, $this->secureRandom, $this->dispatcher, (int) $row['id'], (int) $row['type'], $row['token'], $row['name']); + return new Room($this->db, $this->secureRandom, $this->dispatcher, (int) $row['id'], (int) $row['type'], $row['token'], $row['name'], $row['password']); } /** @@ -358,6 +358,7 @@ class Manager { 'type' => $type, 'token' => $token, 'name' => $name, + 'password' => '', ]); } diff --git a/lib/Migration/Version2001003Date20170911145301.php b/lib/Migration/Version2001003Date20170911145301.php new file mode 100644 index 0000000000..724df26725 --- /dev/null +++ b/lib/Migration/Version2001003Date20170911145301.php @@ -0,0 +1,70 @@ + + * + * @author Joas Schilling + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +namespace OCA\Spreed\Migration; + +use Doctrine\DBAL\Schema\Schema; +use Doctrine\DBAL\Types\Type; +use OCP\Migration\SimpleMigrationStep; +use OCP\Migration\IOutput; + +class Version2001003Date20170911145301 extends SimpleMigrationStep { + + /** + * @param IOutput $output + * @param \Closure $schemaClosure The `\Closure` returns a `Schema` + * @param array $options + * @since 13.0.0 + */ + public function preSchemaChange(IOutput $output, \Closure $schemaClosure, array $options) { + } + + /** + * @param IOutput $output + * @param \Closure $schemaClosure The `\Closure` returns a `Schema` + * @param array $options + * @return null|Schema + * @since 13.0.0 + */ + public function changeSchema(IOutput $output, \Closure $schemaClosure, array $options) { + /** @var Schema $schema */ + $schema = $schemaClosure(); + + $table = $schema->getTable('spreedme_rooms'); + $table->addColumn('password', Type::STRING, [ + 'notnull' => false, + 'length' => 64, + 'default' => '', + ]); + + return $schema; + } + + /** + * @param IOutput $output + * @param \Closure $schemaClosure The `\Closure` returns a `Schema` + * @param array $options + * @since 13.0.0 + */ + public function postSchemaChange(IOutput $output, \Closure $schemaClosure, array $options) { + } +} diff --git a/lib/Room.php b/lib/Room.php index a5f8f43b21..6277e55a22 100644 --- a/lib/Room.php +++ b/lib/Room.php @@ -25,6 +25,7 @@ namespace OCA\Spreed; +use OCA\Spreed\Exceptions\InvalidPasswordException; use OCA\Spreed\Exceptions\ParticipantNotFoundException; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; @@ -53,6 +54,8 @@ class Room { private $token; /** @var string */ private $name; + /** @var string */ + private $password; /** @var string */ protected $currentUser; @@ -69,8 +72,9 @@ class Room { * @param int $type * @param string $token * @param string $name + * @param string $password */ - public function __construct(IDBConnection $db, ISecureRandom $secureRandom, EventDispatcherInterface $dispatcher, $id, $type, $token, $name) { + public function __construct(IDBConnection $db, ISecureRandom $secureRandom, EventDispatcherInterface $dispatcher, $id, $type, $token, $name, $password) { $this->db = $db; $this->secureRandom = $secureRandom; $this->dispatcher = $dispatcher; @@ -78,6 +82,7 @@ class Room { $this->type = $type; $this->token = $token; $this->name = $name; + $this->password = $password; } /** @@ -108,6 +113,13 @@ class Room { return $this->name; } + /** + * @return string + */ + public function getPassword() { + return $this->password; + } + /** * @param string $userId * @param Participant $participant @@ -215,6 +227,29 @@ class Room { return true; } + /** + * @param string $password Currently it is only allowed to have a password for Room::PUBLIC_CALL + * @return bool True when the change was valid, false otherwise + */ + public function setPassword($password) { + if ($password === $this->getPassword()) { + return true; + } + + if ($this->getType() !== self::PUBLIC_CALL) { + return false; + } + + $query = $this->db->getQueryBuilder(); + $query->update('spreedme_rooms') + ->set('password', $query->createNamedParameter($password)) + ->where($query->expr()->eq('id', $query->createNamedParameter($this->getId(), IQueryBuilder::PARAM_INT))); + $query->execute(); + $this->password = $password; + + return true; + } + /** * @param int $newType Currently it is only allowed to change to: Room::GROUP_CALL, Room::PUBLIC_CALL * @return bool True when the change was valid, false otherwise @@ -314,9 +349,11 @@ class Room { /** * @param string $userId + * @param string $password * @return string + * @throws InvalidPasswordException */ - public function enterRoomAsUser($userId) { + public function enterRoomAsUser($userId, $password) { $this->dispatcher->dispatch(self::class . '::preUserEnterRoom', new GenericEvent($this)); $query = $this->db->getQueryBuilder(); @@ -330,6 +367,10 @@ class Room { $result = $query->execute(); if ($result === 0) { + if ($this->getPassword() !== '' && $this->getPassword() !== $password) { + throw new InvalidPasswordException(); + } + // User joining a public room, without being invited $this->addParticipant($userId, Participant::USER_SELF_JOINED, $sessionId); } @@ -353,11 +394,17 @@ class Room { } /** + * @param string $password * @return string + * @throws InvalidPasswordException */ - public function enterRoomAsGuest() { + public function enterRoomAsGuest($password) { $this->dispatcher->dispatch(self::class . '::preGuestEnterRoom', new GenericEvent($this)); + if ($this->getPassword() !== '' && $this->getPassword() !== $password) { + throw new InvalidPasswordException(); + } + $sessionId = $this->secureRandom->generate(255); while (!$this->db->insertIfNotExist('*PREFIX*spreedme_room_participants', [ 'userId' => '', From 504acc5b8565a5e098c2f94a8cce4148d6c11b83 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 20 Sep 2017 09:37:30 +0200 Subject: [PATCH 02/19] Add endpoint to set the password Signed-off-by: Joas Schilling --- appinfo/routes.php | 9 +++++++++ lib/Controller/RoomController.php | 29 +++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/appinfo/routes.php b/appinfo/routes.php index 3b586de6ee..e5076e0d0e 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -139,6 +139,15 @@ return [ 'token' => '^[a-z0-9]{4,30}$', ], ], + [ + 'name' => 'Room#setPassword', + 'url' => '/api/{apiVersion}/room/{token}/password', + 'verb' => 'PUT', + 'requirements' => [ + 'apiVersion' => 'v1', + 'token' => '^[a-z0-9]{4,30}$', + ], + ], [ 'name' => 'Room#getParticipants', 'url' => '/api/{apiVersion}/room/{token}/participants', diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php index 0e8810b781..0c54187fbd 100644 --- a/lib/Controller/RoomController.php +++ b/lib/Controller/RoomController.php @@ -675,6 +675,35 @@ class RoomController extends OCSController { return new DataResponse(); } + /** + * @NoAdminRequired + * + * @param string $token + * @param string $password + * @return DataResponse + */ + public function setPassword($token, $password) { + try { + $room = $this->manager->getRoomForParticipantByToken($token, $this->userId); + $participant = $room->getParticipant($this->userId); + } catch (RoomNotFoundException $e) { + return new DataResponse([], Http::STATUS_NOT_FOUND); + } catch (\RuntimeException $e) { + return new DataResponse([], Http::STATUS_NOT_FOUND); + } + + if (!in_array($participant->getParticipantType(), [Participant::OWNER, Participant::MODERATOR], true)) { + return new DataResponse([], Http::STATUS_FORBIDDEN); + } + + if ($room->getType() !== Room::PUBLIC_CALL) { + return new DataResponse([], Http::STATUS_FORBIDDEN); + } + + $room->setPassword($password); + return new DataResponse(); + } + /** * @NoAdminRequired * From 834a13c1efaa63ab302c332a38f5f86e5b3eea76 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 20 Sep 2017 14:28:00 +0200 Subject: [PATCH 03/19] Add docs about the new endpoint Signed-off-by: Joas Schilling --- docs/api-v1.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docs/api-v1.md b/docs/api-v1.md index 0a1226013e..ffa35ce0cb 100644 --- a/docs/api-v1.md +++ b/docs/api-v1.md @@ -157,6 +157,23 @@ Base endpoint is: `/ocs/v2.php/apps/spreed/api/v1` + `403 Forbidden` When the current user is not a moderator/owner + `404 Not Found` When the room could not be found for the participant +### Set room password + +* Method: `PUT` +* Endpoint: `/room/{token}/password` +* Data: + + field | type | Description + ------|------|------------ + `password` | string | Set a new password for the room + +* Response: + - Header: + + `200 OK` + + `403 Forbidden` When the current user is not a moderator/owner + + `403 Forbidden` When the room is not a public room + + `404 Not Found` When the room could not be found for the participant + ## Participant management From 411ff3dea96ad3377f51d291b9f68911b4ee40b0 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 20 Sep 2017 15:58:14 +0200 Subject: [PATCH 04/19] Only do signaling while we ping (after successful join) Signed-off-by: Joas Schilling --- js/signaling.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/js/signaling.js b/js/signaling.js index ef46600bd9..fac798e5da 100644 --- a/js/signaling.js +++ b/js/signaling.js @@ -93,7 +93,6 @@ function InternalSignaling() { SignalingBase.prototype.constructor.apply(this, arguments); this.spreedArrayConnection = []; - this._openEventSource(); this.pingFails = 0; this.pingInterval = null; @@ -178,6 +177,7 @@ this.sessionId = result.ocs.data.sessionId; this.currentCallToken = token; this._startPingCall(); + this._openEventSource(); this._getCallPeers(token).then(function(peers) { var callDescription = { 'clients': {} @@ -192,6 +192,10 @@ }.bind(this)); callback('', callDescription); }.bind(this)); + }.bind(this), + error: function (result) { + // TODO request password and try again... + console.log("error", result); }.bind(this) }); }; @@ -199,6 +203,7 @@ InternalSignaling.prototype.leaveCall = function(token) { if (token === this.currentCallToken) { this._stopPingCall(); + this._closeEventSource(); } $.ajax({ url: OC.linkToOCS('apps/spreed/api/v1/call', 2) + token, @@ -278,6 +283,16 @@ }.bind(this)); }; + /** + * @private + */ + InternalSignaling.prototype._closeEventSource = function() { + if (this.source) { + this.source.close(); + this.source = null; + } + }; + /** * @private */ @@ -297,6 +312,7 @@ */ InternalSignaling.prototype._startPingCall = function() { this._pingCall(); + // Send a ping to the server all 5 seconds to ensure that the connection // is still alive. this.pingInterval = window.setInterval(function() { From eb7546ea0eaca94bc60b3aeb82afbaf1df39e24b Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 20 Sep 2017 16:03:13 +0200 Subject: [PATCH 05/19] Increase version to run migrations Signed-off-by: Joas Schilling --- appinfo/info.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appinfo/info.xml b/appinfo/info.xml index 848fe2dd9d..99c19080e3 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -42,7 +42,7 @@ And in the works for the [coming versions](https://github.com/nextcloud/spreed/m https://github.com/nextcloud/spreed/issues https://github.com/nextcloud/spreed.git - 2.1.3 + 2.1.4 From f8a9f124f8d5a3ae0ee8d5de2dcd7d45db6b12e1 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 21 Sep 2017 10:54:48 +0200 Subject: [PATCH 06/19] Add option for the moderator to set a password Signed-off-by: Joas Schilling --- css/style.css | 7 ++-- js/views/roomlistview.js | 56 +++++++++++++++++++++++++++++-- lib/Controller/RoomController.php | 1 + 3 files changed, 59 insertions(+), 5 deletions(-) diff --git a/css/style.css b/css/style.css index 2d2a05a20a..78f1a7b1a3 100644 --- a/css/style.css +++ b/css/style.css @@ -126,21 +126,24 @@ opacity: .5; } +.password-input, .rename-input { margin-top: 0px !important; margin-bottom: 4px !important; } +.icon-confirm.password-confirm, .icon-confirm.rename-confirm { background-size: 16px; background-color: transparent; border: none; position: absolute; - right: -2px; - top: 42px; + right: 0; + bottom: 3px; padding: 16px; opacity: .5; } +.icon-confirm.password-confirm:hover, .icon-confirm.rename-confirm:hover { opacity: 1; } diff --git a/js/views/roomlistview.js b/js/views/roomlistview.js index ee31c4b5bb..492a997210 100644 --- a/js/views/roomlistview.js +++ b/js/views/roomlistview.js @@ -55,9 +55,9 @@ ''+ ''+t('spreed', 'Rename')+''+ ''+ + ''+ + '
'+ ''+ - ''+ - ''+ '{{/if}}'+ '
  • '+ '
    • '+ + '
  • '+ + ''+ + ''+ + '
    '+ + '
    • '+ '{{/if}}'+ '{{#if showShareLink}}'+ '
  • '+ @@ -198,6 +206,9 @@ 'click .app-navigation-entry-menu .rename-room-button': 'showRenameInput', 'click .app-navigation-entry-menu .rename-confirm': 'confirmRoomRename', 'keyup .rename-input': 'renameKeyUp', + 'click .app-navigation-entry-menu .password-room-button': 'showPasswordInput', + 'click .app-navigation-entry-menu .password-confirm': 'confirmRoomPassword', + 'keyup .password-input': 'passwordKeyUp', 'click .app-navigation-entry-menu .share-link-button': 'shareGroup', 'click .app-navigation-entry-menu .leave-room-button': 'leaveRoom', 'click .app-navigation-entry-menu .delete-room-button': 'deleteRoom', @@ -222,10 +233,12 @@ toggleMenuClass: function() { this.ui.menu.toggleClass('open', this.menuShown); - // Hide rename input and show button when opening menu + // Hide rename and password input and show button when opening menu if (this.menuShown) { this.$el.find('.rename-element').addClass('hidden-important'); this.$el.find('.rename-room-button').removeClass('hidden-important'); + this.$el.find('.password-element').addClass('hidden-important'); + this.$el.find('.password-room-button').removeClass('hidden-important'); } }, checkSharingStatus: function() { @@ -312,6 +325,43 @@ }); } }, + showPasswordInput: function() { + this.$el.find('.password-element').removeClass('hidden-important'); + this.$el.find('.password-room-button').addClass('hidden-important'); + + this.$el.find('.password-input').focus(); + this.$el.find('.password-input').select(); + }, + hidePasswordInput: function() { + this.$el.find('.password-element').addClass('hidden-important'); + this.$el.find('.password-room-button').removeClass('hidden-important'); + }, + confirmRoomPassword: function() { + var newRoomPassword = $.trim(this.$el.find('.password-input').val()); + this.passwordRoom(newRoomPassword); + this.hidePasswordInput(); + }, + passwordKeyUp: function(e) { + if (e.keyCode === 13) { + this.confirmRoomPassword(); + } else if (e.keyCode === 27) { + this.hidePasswordInput(); + } + }, + passwordRoom: function(roomPassword) { + var app = OCA.SpreedMe.app; + + if (this.model.get('type') === ROOM_TYPE_PUBLIC_CALL) { + $.ajax({ + url: OC.linkToOCS('apps/spreed/api/v1/room', 2) + this.model.get('token') + '/password', + type: 'PUT', + data: 'password='+roomPassword, + success: function() { + app.syncRooms(); + } + }); + } + }, shareGroup: function() { var app = OCA.SpreedMe.app; diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php index 0c54187fbd..fa56e0b1f7 100644 --- a/lib/Controller/RoomController.php +++ b/lib/Controller/RoomController.php @@ -184,6 +184,7 @@ class RoomController extends OCSController { 'sessionId' => isset($participants['users'][$this->userId]['sessionId']) ? $participants['users'][$this->userId]['sessionId'] : '0', 'participants' => $participantList, 'numGuests' => $numActiveGuests, + 'hasPassword' => $room->getPassword() !== '', ]; if ($this->userId !== null) { From 1eb766eb01d8bc8b4a6e11ef5d7cf59cf235c306 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 21 Sep 2017 10:58:20 +0200 Subject: [PATCH 07/19] Hash the passwords Signed-off-by: Joas Schilling --- lib/Controller/RoomController.php | 2 +- lib/Manager.php | 9 +++++-- ....php => Version2001Date20170921145301.php} | 4 +-- lib/Room.php | 27 ++++++++++--------- 4 files changed, 25 insertions(+), 17 deletions(-) rename lib/Migration/{Version2001003Date20170911145301.php => Version2001Date20170921145301.php} (95%) diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php index fa56e0b1f7..c8896c8bca 100644 --- a/lib/Controller/RoomController.php +++ b/lib/Controller/RoomController.php @@ -184,7 +184,7 @@ class RoomController extends OCSController { 'sessionId' => isset($participants['users'][$this->userId]['sessionId']) ? $participants['users'][$this->userId]['sessionId'] : '0', 'participants' => $participantList, 'numGuests' => $numActiveGuests, - 'hasPassword' => $room->getPassword() !== '', + 'hasPassword' => $room->hasPassword(), ]; if ($this->userId !== null) { diff --git a/lib/Manager.php b/lib/Manager.php index a031b7e79d..d7bee894ba 100644 --- a/lib/Manager.php +++ b/lib/Manager.php @@ -26,6 +26,7 @@ use OCA\Spreed\Exceptions\RoomNotFoundException; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IConfig; use OCP\IDBConnection; +use OCP\Security\IHasher; use OCP\Security\ISecureRandom; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -39,6 +40,8 @@ class Manager { private $secureRandom; /** @var EventDispatcherInterface */ private $dispatcher; + /** @var IHasher */ + private $hasher; /** * Manager constructor. @@ -47,12 +50,14 @@ class Manager { * @param IConfig $config * @param ISecureRandom $secureRandom * @param EventDispatcherInterface $dispatcher + * @param IHasher $hasher */ - public function __construct(IDBConnection $db, IConfig $config, ISecureRandom $secureRandom, EventDispatcherInterface $dispatcher) { + public function __construct(IDBConnection $db, IConfig $config, ISecureRandom $secureRandom, EventDispatcherInterface $dispatcher, IHasher $hasher) { $this->db = $db; $this->config = $config; $this->secureRandom = $secureRandom; $this->dispatcher = $dispatcher; + $this->hasher = $hasher; } /** @@ -60,7 +65,7 @@ class Manager { * @return Room */ protected function createRoomObject(array $row) { - return new Room($this->db, $this->secureRandom, $this->dispatcher, (int) $row['id'], (int) $row['type'], $row['token'], $row['name'], $row['password']); + return new Room($this->db, $this->secureRandom, $this->dispatcher, $this->hasher, (int) $row['id'], (int) $row['type'], $row['token'], $row['name'], $row['password']); } /** diff --git a/lib/Migration/Version2001003Date20170911145301.php b/lib/Migration/Version2001Date20170921145301.php similarity index 95% rename from lib/Migration/Version2001003Date20170911145301.php rename to lib/Migration/Version2001Date20170921145301.php index 724df26725..efefbfd6cf 100644 --- a/lib/Migration/Version2001003Date20170911145301.php +++ b/lib/Migration/Version2001Date20170921145301.php @@ -27,7 +27,7 @@ use Doctrine\DBAL\Types\Type; use OCP\Migration\SimpleMigrationStep; use OCP\Migration\IOutput; -class Version2001003Date20170911145301 extends SimpleMigrationStep { +class Version2001Date20170921145301 extends SimpleMigrationStep { /** * @param IOutput $output @@ -52,7 +52,7 @@ class Version2001003Date20170911145301 extends SimpleMigrationStep { $table = $schema->getTable('spreedme_rooms'); $table->addColumn('password', Type::STRING, [ 'notnull' => false, - 'length' => 64, + 'length' => 255, 'default' => '', ]); diff --git a/lib/Room.php b/lib/Room.php index 6277e55a22..beb3c9693b 100644 --- a/lib/Room.php +++ b/lib/Room.php @@ -30,6 +30,7 @@ use OCA\Spreed\Exceptions\ParticipantNotFoundException; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; use OCP\IUser; +use OCP\Security\IHasher; use OCP\Security\ISecureRandom; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\EventDispatcher\GenericEvent; @@ -45,6 +46,8 @@ class Room { private $secureRandom; /** @var EventDispatcherInterface */ private $dispatcher; + /** @var IHasher */ + private $hasher; /** @var int */ private $id; @@ -68,16 +71,18 @@ class Room { * @param IDBConnection $db * @param ISecureRandom $secureRandom * @param EventDispatcherInterface $dispatcher + * @param IHasher $hasher * @param int $id * @param int $type * @param string $token * @param string $name * @param string $password */ - public function __construct(IDBConnection $db, ISecureRandom $secureRandom, EventDispatcherInterface $dispatcher, $id, $type, $token, $name, $password) { + public function __construct(IDBConnection $db, ISecureRandom $secureRandom, EventDispatcherInterface $dispatcher, IHasher $hasher, $id, $type, $token, $name, $password) { $this->db = $db; $this->secureRandom = $secureRandom; $this->dispatcher = $dispatcher; + $this->hasher = $hasher; $this->id = $id; $this->type = $type; $this->token = $token; @@ -114,10 +119,10 @@ class Room { } /** - * @return string + * @return bool */ - public function getPassword() { - return $this->password; + public function hasPassword() { + return $this->password !== ''; } /** @@ -232,20 +237,18 @@ class Room { * @return bool True when the change was valid, false otherwise */ public function setPassword($password) { - if ($password === $this->getPassword()) { - return true; - } - if ($this->getType() !== self::PUBLIC_CALL) { return false; } + $hash = $this->hasher->hash($password); + $query = $this->db->getQueryBuilder(); $query->update('spreedme_rooms') - ->set('password', $query->createNamedParameter($password)) + ->set('password', $query->createNamedParameter($hash)) ->where($query->expr()->eq('id', $query->createNamedParameter($this->getId(), IQueryBuilder::PARAM_INT))); $query->execute(); - $this->password = $password; + $this->password = $hash; return true; } @@ -367,7 +370,7 @@ class Room { $result = $query->execute(); if ($result === 0) { - if ($this->getPassword() !== '' && $this->getPassword() !== $password) { + if ($this->hasPassword() && $this->hasher->verify($password, $this->password)) { throw new InvalidPasswordException(); } @@ -401,7 +404,7 @@ class Room { public function enterRoomAsGuest($password) { $this->dispatcher->dispatch(self::class . '::preGuestEnterRoom', new GenericEvent($this)); - if ($this->getPassword() !== '' && $this->getPassword() !== $password) { + if ($this->hasPassword() && $this->hasher->verify($password, $this->password)) { throw new InvalidPasswordException(); } From 6117e5ae8d98b9c92c260a7aab41f26825371d4d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 21 Sep 2017 11:16:38 +0200 Subject: [PATCH 08/19] Doc paassword on "join call" Signed-off-by: Joas Schilling --- docs/api-v1.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/api-v1.md b/docs/api-v1.md index ffa35ce0cb..dbc943f918 100644 --- a/docs/api-v1.md +++ b/docs/api-v1.md @@ -313,10 +313,16 @@ Base endpoint is: `/ocs/v2.php/apps/spreed/api/v1` * Method: `POST` * Endpoint: `/call/{token}` +* Data: + + field | type | Description + ------|------|------------ + `password` | string | Optional: Password is only required for users which are of type `4` or `5` and only when the room has `hasPassword` set to true. * Response: - Header: + `200 OK` + + `403 Forbidden` When the password is required and didn't match + `404 Not Found` When the room could not be found for the participant - Data: From aee0e22052933174785cefa1da67dd3638c3f5a2 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 21 Sep 2017 14:36:47 +0200 Subject: [PATCH 09/19] Request the password when joining a call with password Signed-off-by: Joas Schilling --- js/signaling.js | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/js/signaling.js b/js/signaling.js index fac798e5da..f67b2405e2 100644 --- a/js/signaling.js +++ b/js/signaling.js @@ -156,7 +156,7 @@ }.bind(this)); }; - InternalSignaling.prototype.joinCall = function(token, callback) { + InternalSignaling.prototype.joinCall = function(token, callback, password) { // The client is joining a new call, in this case we need // to do the following: // @@ -172,6 +172,9 @@ beforeSend: function (request) { request.setRequestHeader('Accept', 'application/json'); }, + data: { + password: password + }, success: function (result) { console.log("Joined", result); this.sessionId = result.ocs.data.sessionId; @@ -194,8 +197,33 @@ }.bind(this)); }.bind(this), error: function (result) { - // TODO request password and try again... - console.log("error", result); + if (result.status === 404 || result.status === 503) { + // Room not found or maintenance mode + OC.redirect(OC.generateUrl('apps/spreed')) + } + + if (result.status === 403) { + // Invalid password + OC.dialogs.prompt( + t('spreed', 'Please enter the password for this call'), + t('spreed','Password required'), + function (result, password) { + if (result && password !== '') { + this.joinCall(token, callback, password); + } + }.bind(this), + true, + t('spreed','Password'), + true + ).then(function() { + var $dialog = $('.oc-dialog:visible'); + $dialog.find('.ui-icon').remove(); + + var $buttons = $dialog.find('button'); + $buttons.eq(0).text(t('core', 'Cancel')); + $buttons.eq(1).text(t('core', 'Submit')); + }); + } }.bind(this) }); }; From 8d780764e3ef0b6718fca94cdc5b1b6689e8e89a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 21 Sep 2017 14:47:48 +0200 Subject: [PATCH 10/19] Remove empty methods Signed-off-by: Joas Schilling --- lib/Migration/Version2001Date20170921145301.php | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/lib/Migration/Version2001Date20170921145301.php b/lib/Migration/Version2001Date20170921145301.php index efefbfd6cf..5619174300 100644 --- a/lib/Migration/Version2001Date20170921145301.php +++ b/lib/Migration/Version2001Date20170921145301.php @@ -29,15 +29,6 @@ use OCP\Migration\IOutput; class Version2001Date20170921145301 extends SimpleMigrationStep { - /** - * @param IOutput $output - * @param \Closure $schemaClosure The `\Closure` returns a `Schema` - * @param array $options - * @since 13.0.0 - */ - public function preSchemaChange(IOutput $output, \Closure $schemaClosure, array $options) { - } - /** * @param IOutput $output * @param \Closure $schemaClosure The `\Closure` returns a `Schema` @@ -59,12 +50,4 @@ class Version2001Date20170921145301 extends SimpleMigrationStep { return $schema; } - /** - * @param IOutput $output - * @param \Closure $schemaClosure The `\Closure` returns a `Schema` - * @param array $options - * @since 13.0.0 - */ - public function postSchemaChange(IOutput $output, \Closure $schemaClosure, array $options) { - } } From ff6bddb29df93670117b75f9360c6ef4c933844f Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 22 Sep 2017 11:29:07 +0200 Subject: [PATCH 11/19] Only return limited information before password verification Signed-off-by: Joas Schilling --- lib/Controller/CallController.php | 14 ++++- lib/Controller/RoomController.php | 85 ++++++++++++++++++++++--------- lib/Manager.php | 2 +- 3 files changed, 74 insertions(+), 27 deletions(-) diff --git a/lib/Controller/CallController.php b/lib/Controller/CallController.php index f076cb55a2..e15a898b99 100644 --- a/lib/Controller/CallController.php +++ b/lib/Controller/CallController.php @@ -79,12 +79,20 @@ class CallController extends OCSController { * @return DataResponse */ public function getPeersForCall($token) { + if (!$this->session->exists('spreed-session')) { + return new DataResponse([], Http::STATUS_NOT_FOUND); + } + try { - $room = $this->manager->getRoomForParticipantByToken($token, $this->userId); + $room = $this->manager->getRoomForSession($this->userId, $this->session->get('spreed-session')); } catch (RoomNotFoundException $e) { return new DataResponse([], Http::STATUS_NOT_FOUND); } + if ($room->getToken() !== $token) { + return new DataResponse([], Http::STATUS_NOT_FOUND); + } + /** @var array[] $participants */ $participants = $room->getParticipants(time() - 30); $result = []; @@ -159,6 +167,10 @@ class CallController extends OCSController { * @return DataResponse */ public function pingCall($token) { + if (!$this->session->exists('spreed-session')) { + return new DataResponse([], Http::STATUS_NOT_FOUND); + } + try { $room = $this->manager->getRoomForParticipantByToken($token, $this->userId); } catch (RoomNotFoundException $e) { diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php index c8896c8bca..dec0ae5d05 100644 --- a/lib/Controller/RoomController.php +++ b/lib/Controller/RoomController.php @@ -37,6 +37,7 @@ use OCP\AppFramework\OCSController; use OCP\IL10N; use OCP\ILogger; use OCP\IRequest; +use OCP\ISession; use OCP\IUser; use OCP\IUserManager; use OCP\IGroup; @@ -46,6 +47,8 @@ use OCP\Notification\IManager as INotificationManager; class RoomController extends OCSController { /** @var string */ private $userId; + /** @var ISession */ + private $session; /** @var IUserManager */ private $userManager; /** @var IGroupManager */ @@ -65,6 +68,7 @@ class RoomController extends OCSController { * @param string $appName * @param string $UserId * @param IRequest $request + * @param ISession $session * @param IUserManager $userManager * @param IGroupManager $groupManager * @param ILogger $logger @@ -76,6 +80,7 @@ class RoomController extends OCSController { public function __construct($appName, $UserId, IRequest $request, + ISession $session, IUserManager $userManager, IGroupManager $groupManager, ILogger $logger, @@ -84,6 +89,7 @@ class RoomController extends OCSController { IActivityManager $activityManager, IL10N $l10n) { parent::__construct($appName, $request); + $this->session = $session; $this->userId = $UserId; $this->userManager = $userManager; $this->groupManager = $groupManager; @@ -107,8 +113,9 @@ class RoomController extends OCSController { $return = []; foreach ($rooms as $room) { try { - $return[] = $this->formatRoom($room); + $return[] = $this->formatRoom($room, $room->getParticipant($this->userId)); } catch (RoomNotFoundException $e) { + } catch (\RuntimeException $e) { } } @@ -124,7 +131,18 @@ class RoomController extends OCSController { public function getRoom($token) { try { $room = $this->manager->getRoomForParticipantByToken($token, $this->userId); - return new DataResponse($this->formatRoom($room)); + + $participant = null; + try { + $participant = $room->getParticipant($this->userId); + } catch (ParticipantNotFoundException $e) { + try { + $participant = $room->getParticipantBySession($this->session->get('spreed-session')); + } catch (ParticipantNotFoundException $e) { + } + } + + return new DataResponse($this->formatRoom($room, $participant)); } catch (RoomNotFoundException $e) { return new DataResponse([], Http::STATUS_NOT_FOUND); } @@ -132,10 +150,42 @@ class RoomController extends OCSController { /** * @param Room $room + * @param Participant $participant * @return array * @throws RoomNotFoundException */ - protected function formatRoom(Room $room) { + protected function formatRoom(Room $room, Participant $participant = null) { + + if ($participant instanceof Participant) { + $participantType = $participant->getParticipantType(); + } else { + $participantType = Participant::GUEST; + } + + $roomData = [ + 'id' => $room->getId(), + 'token' => $room->getToken(), + 'type' => $room->getType(), + 'name' => $room->getName(), + 'displayName' => $room->getName(), + 'participantType' => $participantType, + 'count' => $room->getNumberOfParticipants(time() - 30), + 'hasPassword' => $room->hasPassword(), + ]; + + if (!$participant instanceof Participant) { + // Unauthenticated user without a session (didn't enter password) + $roomData = array_merge($roomData, [ + 'lastPing' => 0, + 'sessionId' => '0', + 'participants' => [], + 'numGuests' => 0, + 'guestList' => '', + ]); + + return $roomData; + } + // Sort by lastPing /** @var array[] $participants */ $participants = $room->getParticipants(); @@ -146,23 +196,16 @@ class RoomController extends OCSController { uasort($participants['guests'], $sortParticipants); $participantList = []; - foreach ($participants['users'] as $participant => $data) { - $user = $this->userManager->get($participant); + foreach ($participants['users'] as $userId => $data) { + $user = $this->userManager->get($userId); if ($user instanceof IUser) { - $participantList[$participant] = [ + $participantList[$user->getUID()] = [ 'name' => $user->getDisplayName(), 'type' => $data['participantType'], ]; } } - try { - $participant = $room->getParticipant($this->userId); - $participantType = $participant->getParticipantType(); - } catch (ParticipantNotFoundException $e) { - $participantType = Participant::GUEST; - } - $activeGuests = array_filter($participants['guests'], function($data) { return $data['lastPing'] > time() - 30; }); @@ -172,20 +215,12 @@ class RoomController extends OCSController { $room->cleanGuestParticipants(); } - $roomData = [ - 'id' => $room->getId(), - 'token' => $room->getToken(), - 'type' => $room->getType(), - 'name' => $room->getName(), - 'displayName' => $room->getName(), - 'participantType' => $participantType, - 'count' => $room->getNumberOfParticipants(time() - 30), - 'lastPing' => isset($participants['users'][$this->userId]['lastPing']) ? $participants['users'][$this->userId]['lastPing'] : 0, - 'sessionId' => isset($participants['users'][$this->userId]['sessionId']) ? $participants['users'][$this->userId]['sessionId'] : '0', + $roomData = array_merge($roomData, [ + 'lastPing' => $participant->getLastPing(), + 'sessionId' => $participant->getSessionId(), 'participants' => $participantList, 'numGuests' => $numActiveGuests, - 'hasPassword' => $room->hasPassword(), - ]; + ]); if ($this->userId !== null) { unset($participantList[$this->userId]); diff --git a/lib/Manager.php b/lib/Manager.php index d7bee894ba..37ea884ce5 100644 --- a/lib/Manager.php +++ b/lib/Manager.php @@ -268,7 +268,7 @@ class Manager { throw new RoomNotFoundException(); } - if ($userId !== $row['userId']) { + if ((string) $userId !== $row['userId']) { throw new RoomNotFoundException(); } From ff26dc022c2a1225255aa4771809dc8ac2341b34 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 22 Sep 2017 14:19:58 +0200 Subject: [PATCH 12/19] Allow accessing the peers for users which are part of the room Signed-off-by: Joas Schilling --- lib/Controller/CallController.php | 15 ++++++++++----- lib/Manager.php | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/Controller/CallController.php b/lib/Controller/CallController.php index e15a898b99..25c492a852 100644 --- a/lib/Controller/CallController.php +++ b/lib/Controller/CallController.php @@ -79,14 +79,19 @@ class CallController extends OCSController { * @return DataResponse */ public function getPeersForCall($token) { - if (!$this->session->exists('spreed-session')) { - return new DataResponse([], Http::STATUS_NOT_FOUND); - } - try { $room = $this->manager->getRoomForSession($this->userId, $this->session->get('spreed-session')); } catch (RoomNotFoundException $e) { - return new DataResponse([], Http::STATUS_NOT_FOUND); + if ($this->userId === null) { + return new DataResponse([], Http::STATUS_NOT_FOUND); + } + + // For logged in users we search for rooms where they are real participants + try { + $room = $this->manager->getRoomForParticipantByToken($token, $this->userId); + } catch (RoomNotFoundException $e) { + return new DataResponse([], Http::STATUS_NOT_FOUND); + } } if ($room->getToken() !== $token) { diff --git a/lib/Manager.php b/lib/Manager.php index 37ea884ce5..9df4750e0f 100644 --- a/lib/Manager.php +++ b/lib/Manager.php @@ -247,7 +247,7 @@ class Manager { * @throws RoomNotFoundException */ public function getRoomForSession($userId, $sessionId) { - if ($sessionId === '' || $sessionId === '0') { + if ((string) $sessionId === '' || $sessionId === '0') { throw new RoomNotFoundException(); } From 5ba66c0de0e6b99abf60faec1b313d7c2e4fe309 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 22 Sep 2017 14:25:42 +0200 Subject: [PATCH 13/19] Remove self-joined users from the list after leaving Signed-off-by: Joas Schilling --- lib/Controller/CallController.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/Controller/CallController.php b/lib/Controller/CallController.php index 25c492a852..28d1de4f65 100644 --- a/lib/Controller/CallController.php +++ b/lib/Controller/CallController.php @@ -28,6 +28,7 @@ namespace OCA\Spreed\Controller; use OCA\Spreed\Exceptions\InvalidPasswordException; use OCA\Spreed\Exceptions\RoomNotFoundException; use OCA\Spreed\Manager; +use OCA\Spreed\Participant; use OCA\Spreed\Signalling\Messages; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; @@ -197,7 +198,18 @@ class CallController extends OCSController { */ public function leaveCall($token) { if ($this->userId !== null) { - // TODO: Currently we ignore $token, should be fixed at some point + try { + $room = $this->manager->getRoomForParticipantByToken($token, $this->userId); + $participant = $room->getParticipant($this->userId); + + if ($participant->getParticipantType() === Participant::USER_SELF_JOINED) { + $room->removeParticipantBySession($participant); + } + } catch (RoomNotFoundException $e) { + } catch (\RuntimeException $e) { + } + + // As a pre-caution we simply disconnect the user from all rooms $this->manager->disconnectUserFromAllRooms($this->userId); } else { $sessionId = $this->session->get('spreed-session'); From 12332361d638781d7b8041d5f27381779720d6e1 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 22 Sep 2017 14:39:13 +0200 Subject: [PATCH 14/19] ES lint Signed-off-by: Joas Schilling --- js/signaling.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/signaling.js b/js/signaling.js index f67b2405e2..8be9dba48d 100644 --- a/js/signaling.js +++ b/js/signaling.js @@ -199,7 +199,7 @@ error: function (result) { if (result.status === 404 || result.status === 503) { // Room not found or maintenance mode - OC.redirect(OC.generateUrl('apps/spreed')) + OC.redirect(OC.generateUrl('apps/spreed')); } if (result.status === 403) { From 87bfc885bd70f23569a7c08c66c23ffe089a8818 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 22 Sep 2017 15:34:59 +0200 Subject: [PATCH 15/19] Add integration tests for setting the password and joining without it Signed-off-by: Joas Schilling --- .../features/bootstrap/FeatureContext.php | 17 ++++++ .../integration/features/set-password.feature | 58 +++++++++++++++++++ 2 files changed, 75 insertions(+) create mode 100644 tests/integration/features/set-password.feature diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 5d3122baa8..d70e6cf6d6 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -221,6 +221,23 @@ class FeatureContext implements Context, SnippetAcceptingContext { $this->assertStatusCode($this->response, $statusCode); } + /** + * @When /^user "([^"]*)" sets password "([^"]*)" for room "([^"]*)" with (\d+)$/ + * + * @param string $user + * @param string $password + * @param string $identifier + * @param string $statusCode + */ + public function userSetsTheRoomPassword($user, $password, $identifier, $statusCode) { + $this->setCurrentUser($user); + $this->sendRequest( + 'PUT', '/apps/spreed/api/v1/room/' . self::$identifierToToken[$identifier] . '/password', + new TableNode([['password', $password]]) + ); + $this->assertStatusCode($this->response, $statusCode); + } + /** * @Then /^user "([^"]*)" makes room "([^"]*)" (public|private) with (\d+)$/ * diff --git a/tests/integration/features/set-password.feature b/tests/integration/features/set-password.feature new file mode 100644 index 0000000000..f37f5035e7 --- /dev/null +++ b/tests/integration/features/set-password.feature @@ -0,0 +1,58 @@ +Feature: public + Background: + Given user "participant1" exists + Given user "participant2" exists + Given user "participant3" exists + + Scenario: Owner sets a room password + Given user "participant1" creates room "room" + | roomType | 3 | + And user "participant1" is participant of the following rooms + | id | type | participantType | participants | + | room | 3 | 1 | participant1-displayname | + When user "participant1" sets password "foobar" for room "room" with 200 + Then user "participant3" joins call "room" with 403 + When user "participant1" sets password "" for room "room" with 200 + Then user "participant3" joins call "room" with 200 + + Scenario: Moderator sets a room password + Given user "participant1" creates room "room" + | roomType | 3 | + And user "participant1" is participant of the following rooms + | id | type | participantType | participants | + | room | 3 | 1 | participant1-displayname | + And user "participant1" adds "participant2" to room "room" with 200 + And user "participant1" promotes "participant2" in room "room" with 200 + When user "participant2" sets password "foobar" for room "room" with 200 + Then user "participant3" joins call "room" with 403 + When user "participant2" sets password "" for room "room" with 200 + Then user "participant3" joins call "room" with 200 + + Scenario: User sets a room password + Given user "participant1" creates room "room" + | roomType | 3 | + And user "participant1" is participant of the following rooms + | id | type | participantType | participants | + | room | 3 | 1 | participant1-displayname | + And user "participant1" adds "participant2" to room "room" with 200 + When user "participant2" sets password "foobar" for room "room" with 403 + Then user "participant3" joins call "room" with 200 + And user "participant3" leaves call "room" with 200 + When user "participant1" sets password "foobar" for room "room" with 200 + Then user "participant3" joins call "room" with 403 + When user "participant2" sets password "" for room "room" with 403 + Then user "participant3" joins call "room" with 403 + + Scenario: Stranger sets a room password + Given user "participant1" creates room "room" + | roomType | 3 | + And user "participant1" is participant of the following rooms + | id | type | participantType | participants | + | room | 3 | 1 | participant1-displayname | + When user "participant2" sets password "foobar" for room "room" with 404 + Then user "participant3" joins call "room" with 200 + And user "participant3" leaves call "room" with 200 + When user "participant1" sets password "foobar" for room "room" with 200 + Then user "participant3" joins call "room" with 403 + When user "participant2" sets password "" for room "room" with 404 + Then user "participant3" joins call "room" with 403 From fc2e8645ec64d32296c24bd99b3f6b80368b7a78 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 22 Sep 2017 15:35:53 +0200 Subject: [PATCH 16/19] Yeah... of course the password must be wrong to fail, not correct... Signed-off-by: Joas Schilling --- lib/Room.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Room.php b/lib/Room.php index beb3c9693b..dee33590c3 100644 --- a/lib/Room.php +++ b/lib/Room.php @@ -370,7 +370,7 @@ class Room { $result = $query->execute(); if ($result === 0) { - if ($this->hasPassword() && $this->hasher->verify($password, $this->password)) { + if ($this->hasPassword() && !$this->hasher->verify($password, $this->password)) { throw new InvalidPasswordException(); } @@ -404,7 +404,7 @@ class Room { public function enterRoomAsGuest($password) { $this->dispatcher->dispatch(self::class . '::preGuestEnterRoom', new GenericEvent($this)); - if ($this->hasPassword() && $this->hasher->verify($password, $this->password)) { + if ($this->hasPassword() && !$this->hasher->verify($password, $this->password)) { throw new InvalidPasswordException(); } From c8354fb2501b0398ee8984dd4f01034f74ebf499 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 22 Sep 2017 15:43:44 +0200 Subject: [PATCH 17/19] Make sure join with password works :) Signed-off-by: Joas Schilling --- .../features/bootstrap/FeatureContext.php | 9 +++++++-- tests/integration/features/set-password.feature | 12 ++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index d70e6cf6d6..e75450fdf8 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -228,6 +228,7 @@ class FeatureContext implements Context, SnippetAcceptingContext { * @param string $password * @param string $identifier * @param string $statusCode + * @param TableNode */ public function userSetsTheRoomPassword($user, $password, $identifier, $statusCode) { $this->setCurrentUser($user); @@ -310,10 +311,14 @@ class FeatureContext implements Context, SnippetAcceptingContext { * @param string $user * @param string $identifier * @param string $statusCode + * @param TableNode|null $formData */ - public function userJoinsCall($user, $identifier, $statusCode) { + public function userJoinsCall($user, $identifier, $statusCode, TableNode $formData = null) { $this->setCurrentUser($user); - $this->sendRequest('POST', '/apps/spreed/api/v1/call/' . self::$identifierToToken[$identifier]); + $this->sendRequest( + 'POST', '/apps/spreed/api/v1/call/' . self::$identifierToToken[$identifier], + $formData + ); $this->assertStatusCode($this->response, $statusCode); } diff --git a/tests/integration/features/set-password.feature b/tests/integration/features/set-password.feature index f37f5035e7..c30c8b4a3d 100644 --- a/tests/integration/features/set-password.feature +++ b/tests/integration/features/set-password.feature @@ -12,6 +12,9 @@ Feature: public | room | 3 | 1 | participant1-displayname | When user "participant1" sets password "foobar" for room "room" with 200 Then user "participant3" joins call "room" with 403 + Then user "participant3" joins call "room" with 200 + | password | foobar | + And user "participant3" leaves call "room" with 200 When user "participant1" sets password "" for room "room" with 200 Then user "participant3" joins call "room" with 200 @@ -25,6 +28,9 @@ Feature: public And user "participant1" promotes "participant2" in room "room" with 200 When user "participant2" sets password "foobar" for room "room" with 200 Then user "participant3" joins call "room" with 403 + Then user "participant3" joins call "room" with 200 + | password | foobar | + And user "participant3" leaves call "room" with 200 When user "participant2" sets password "" for room "room" with 200 Then user "participant3" joins call "room" with 200 @@ -40,6 +46,9 @@ Feature: public And user "participant3" leaves call "room" with 200 When user "participant1" sets password "foobar" for room "room" with 200 Then user "participant3" joins call "room" with 403 + Then user "participant3" joins call "room" with 200 + | password | foobar | + And user "participant3" leaves call "room" with 200 When user "participant2" sets password "" for room "room" with 403 Then user "participant3" joins call "room" with 403 @@ -54,5 +63,8 @@ Feature: public And user "participant3" leaves call "room" with 200 When user "participant1" sets password "foobar" for room "room" with 200 Then user "participant3" joins call "room" with 403 + Then user "participant3" joins call "room" with 200 + | password | foobar | + And user "participant3" leaves call "room" with 200 When user "participant2" sets password "" for room "room" with 404 Then user "participant3" joins call "room" with 403 From e0613c7330cee2baa2186a0b2e3883b98600c666 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 22 Sep 2017 15:49:54 +0200 Subject: [PATCH 18/19] Add integration tests for call handling with passwords Signed-off-by: Joas Schilling --- .../features/callapi/password.feature | 130 ++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 tests/integration/features/callapi/password.feature diff --git a/tests/integration/features/callapi/password.feature b/tests/integration/features/callapi/password.feature new file mode 100644 index 0000000000..a88d9d866e --- /dev/null +++ b/tests/integration/features/callapi/password.feature @@ -0,0 +1,130 @@ +Feature: callapi/public + Background: + Given user "participant1" exists + And user "participant2" exists + And user "participant3" exists + + Scenario: User has no rooms + Then user "participant1" is participant of the following rooms + Then user "participant2" is participant of the following rooms + Then user "participant3" is participant of the following rooms + + Scenario: User1 invites user2 to a public room and they can do everything + When user "participant1" creates room "room" + | roomType | 3 | + And user "participant1" sets password "foobar" for room "room" with 200 + And user "participant1" adds "participant2" to room "room" with 200 + Then user "participant1" is participant of room "room" + And user "participant2" is participant of room "room" + Then user "participant1" sees 0 peers in call "room" with 200 + And user "participant2" sees 0 peers in call "room" with 200 + Then user "participant1" joins call "room" with 200 + Then user "participant1" sees 1 peers in call "room" with 200 + And user "participant2" sees 1 peers in call "room" with 200 + And user "participant2" joins call "room" with 200 + Then user "participant1" sees 2 peers in call "room" with 200 + And user "participant2" sees 2 peers in call "room" with 200 + Then user "participant1" pings call "room" with 200 + And user "participant2" pings call "room" with 200 + Then user "participant1" leaves call "room" with 200 + Then user "participant1" sees 1 peers in call "room" with 200 + And user "participant2" sees 1 peers in call "room" with 200 + Then user "participant2" leaves call "room" with 200 + Then user "participant1" sees 0 peers in call "room" with 200 + And user "participant2" sees 0 peers in call "room" with 200 + + Scenario: User1 invites user2 to a public room and user3 can not join without password + When user "participant1" creates room "room" + | roomType | 3 | + And user "participant1" sets password "foobar" for room "room" with 200 + And user "participant1" adds "participant2" to room "room" with 200 + Then user "participant1" is participant of room "room" + Then user "participant3" is not participant of room "room" + And user "participant3" sees 0 peers in call "room" with 404 + Then user "participant1" joins call "room" with 200 + Then user "participant1" sees 1 peers in call "room" with 200 + And user "participant3" sees 0 peers in call "room" with 404 + And user "participant3" joins call "room" with 403 + Then user "participant1" sees 1 peers in call "room" with 200 + And user "participant3" sees 0 peers in call "room" with 404 + And user "participant3" pings call "room" with 404 + Then user "participant1" sees 1 peers in call "room" with 200 + And user "participant3" sees 0 peers in call "room" with 404 + Then user "participant3" leaves call "room" with 200 + Then user "participant1" sees 1 peers in call "room" with 200 + And user "participant3" sees 0 peers in call "room" with 404 + Then user "participant1" leaves call "room" with 200 + Then user "participant1" sees 0 peers in call "room" with 200 + And user "participant3" sees 0 peers in call "room" with 404 + + Scenario: User1 invites user2 to a public room and user3 can join with password + When user "participant1" creates room "room" + | roomType | 3 | + And user "participant1" sets password "foobar" for room "room" with 200 + And user "participant1" adds "participant2" to room "room" with 200 + Then user "participant1" is participant of room "room" + Then user "participant3" is not participant of room "room" + And user "participant3" sees 0 peers in call "room" with 404 + Then user "participant1" joins call "room" with 200 + Then user "participant1" sees 1 peers in call "room" with 200 + And user "participant3" sees 0 peers in call "room" with 404 + And user "participant3" joins call "room" with 200 + | password | foobar | + Then user "participant1" sees 2 peers in call "room" with 200 + And user "participant3" sees 2 peers in call "room" with 200 + And user "participant3" pings call "room" with 200 + Then user "participant1" sees 2 peers in call "room" with 200 + And user "participant3" sees 2 peers in call "room" with 200 + Then user "participant3" leaves call "room" with 200 + Then user "participant1" sees 1 peers in call "room" with 200 + And user "participant3" sees 0 peers in call "room" with 404 + Then user "participant1" leaves call "room" with 200 + Then user "participant1" sees 0 peers in call "room" with 200 + And user "participant3" sees 0 peers in call "room" with 404 + + Scenario: User1 invites user2 to a public room and guest can not join without password + When user "participant1" creates room "room" + | roomType | 3 | + And user "participant1" sets password "foobar" for room "room" with 200 + And user "participant1" adds "participant2" to room "room" with 200 + Then user "participant1" is participant of room "room" + And user "guest" sees 0 peers in call "room" with 404 + Then user "participant1" joins call "room" with 200 + Then user "participant1" sees 1 peers in call "room" with 200 + And user "guest" sees 0 peers in call "room" with 404 + And user "guest" joins call "room" with 403 + Then user "participant1" sees 1 peers in call "room" with 200 + And user "guest" sees 0 peers in call "room" with 404 + And user "guest" pings call "room" with 404 + Then user "participant1" sees 1 peers in call "room" with 200 + And user "guest" sees 0 peers in call "room" with 404 + Then user "guest" leaves call "room" with 200 + Then user "participant1" sees 1 peers in call "room" with 200 + And user "guest" sees 0 peers in call "room" with 404 + Then user "participant1" leaves call "room" with 200 + Then user "participant1" sees 0 peers in call "room" with 200 + And user "guest" sees 0 peers in call "room" with 404 + + Scenario: User1 invites user2 to a public room and guest can join with password + When user "participant1" creates room "room" + | roomType | 3 | + And user "participant1" sets password "foobar" for room "room" with 200 + And user "participant1" adds "participant2" to room "room" with 200 + Then user "participant1" is participant of room "room" + And user "guest" sees 0 peers in call "room" with 404 + Then user "participant1" joins call "room" with 200 + Then user "participant1" sees 1 peers in call "room" with 200 + And user "guest" sees 0 peers in call "room" with 404 + And user "guest" joins call "room" with 200 + | password | foobar | + Then user "participant1" sees 2 peers in call "room" with 200 + And user "guest" sees 2 peers in call "room" with 200 + And user "guest" pings call "room" with 200 + Then user "participant1" sees 2 peers in call "room" with 200 + And user "guest" sees 2 peers in call "room" with 200 + Then user "guest" leaves call "room" with 200 + Then user "participant1" sees 1 peers in call "room" with 200 + And user "guest" sees 0 peers in call "room" with 404 + Then user "participant1" leaves call "room" with 200 + Then user "participant1" sees 0 peers in call "room" with 200 + And user "guest" sees 0 peers in call "room" with 404 From 925761577cfc75923a50df9eb45e67ad27777c67 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 22 Sep 2017 16:15:10 +0200 Subject: [PATCH 19/19] Fail when the user is not a participant Signed-off-by: Joas Schilling --- lib/Controller/CallController.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/Controller/CallController.php b/lib/Controller/CallController.php index 28d1de4f65..32d9d071ed 100644 --- a/lib/Controller/CallController.php +++ b/lib/Controller/CallController.php @@ -26,6 +26,7 @@ namespace OCA\Spreed\Controller; use OCA\Spreed\Exceptions\InvalidPasswordException; +use OCA\Spreed\Exceptions\ParticipantNotFoundException; use OCA\Spreed\Exceptions\RoomNotFoundException; use OCA\Spreed\Manager; use OCA\Spreed\Participant; @@ -90,8 +91,11 @@ class CallController extends OCSController { // For logged in users we search for rooms where they are real participants try { $room = $this->manager->getRoomForParticipantByToken($token, $this->userId); + $room->getParticipant($this->userId); } catch (RoomNotFoundException $e) { return new DataResponse([], Http::STATUS_NOT_FOUND); + } catch (ParticipantNotFoundException $e) { + return new DataResponse([], Http::STATUS_NOT_FOUND); } }