diff --git a/src/app/Http/Controllers/API/V4/OpenViduController.php b/src/app/Http/Controllers/API/V4/OpenViduController.php --- a/src/app/Http/Controllers/API/V4/OpenViduController.php +++ b/src/app/Http/Controllers/API/V4/OpenViduController.php @@ -105,6 +105,37 @@ } /** + * Create a connection for screen sharing. + * + * @param string $id Room identifier (name) + * + * @return \Illuminate\Http\JsonResponse + */ + public function createConnection($id) + { + $room = Room::where('name', $id)->first(); + + // This isn't a room, bye bye + if (!$room) { + return $this->errorResponse(404, \trans('meet.room-not-found')); + } + + $connection = $this->getConnectionFromRequest(); + + if ( + !$connection + || $connection->session_id != $room->session_id + || ($connection->role & Room::ROLE_PUBLISHER) == 0 + ) { + return $this->errorResponse(403); + } + + $response = $room->getSessionToken(Room::ROLE_SCREEN); + + return response()->json(['status' => 'success', 'token' => $response['token']]); + } + + /** * Dismiss the participant/connection from the session. * * @param string $id Room identifier (name) @@ -134,7 +165,7 @@ } /** - * Listing of rooms that belong to the current user. + * Listing of rooms that belong to the authenticated user. * * @return \Illuminate\Http\JsonResponse */ @@ -291,13 +322,6 @@ return $this->errorResponse(500, \trans('meet.session-join-error')); } - // Create session token for screen sharing connection - if (($role & Room::ROLE_PUBLISHER) && !empty(request()->input('screenShare'))) { - $add_token = $room->getSessionToken(Room::ROLE_SCREEN); - - $response['shareToken'] = $add_token['token']; - } - // Get up-to-date connections metadata $response['connections'] = $room->getSessionConnections(); @@ -471,19 +495,37 @@ } // Moderator's authentication via the extra request header + if ( + ($connection = $this->getConnectionFromRequest()) + && $connection->session_id === $room->session_id + && $connection->role & Room::ROLE_MODERATOR + ) { + return true; + } + + return false; + } + + /** + * Get the connection object for the token in current request headers. + * It will also validate the token. + * + * @return \App\OpenVidu\Connection|null Connection (if exists and the token is valid) + */ + protected function getConnectionFromRequest() + { + // Authenticate the user via the extra request header if ($token = request()->header(self::AUTH_HEADER)) { list($connId, ) = explode(':', base64_decode($token), 2); if ( ($connection = Connection::find($connId)) - && $connection->session_id === $room->session_id && $connection->metadata['authToken'] === $token - && $connection->role & Room::ROLE_MODERATOR ) { - return true; + return $connection; } } - return false; + return null; } } diff --git a/src/resources/js/meet/app.js b/src/resources/js/meet/app.js --- a/src/resources/js/meet/app.js +++ b/src/resources/js/meet/app.js @@ -44,7 +44,6 @@ let subscribersContainer OV = new OpenVidu() - screenOV = new OpenVidu() // If there's anything to do, do it here. //OV.setAdvancedConfiguration(config) @@ -131,11 +130,14 @@ }) session.on('connectionDestroyed', event => { - let conn = connections[event.connection.connectionId] + let connectionId = event.connection.connectionId + let conn = connections[connectionId] + if (conn) { $(conn.element).remove() - delete connections[event.connection.connectionId] + delete connections[connectionId] } + resize() }) @@ -689,13 +691,16 @@ */ function switchScreen(callback) { if (screenPublisher) { + // Note: This is what the original openvidu-call app does. + // It is probably better for performance reasons to close the connection, + // than to use unpublish() and keep the connection open. screenSession.disconnect() screenSession = null screenPublisher = null if (callback) { - // Note: Disconnecting invalidates the token. The callback should request - // a new token for the next screen sharing session. + // Note: Disconnecting invalidates the token, we have to inform the vue component + // to update UI state (and be prepared to request a new token). callback(false) } @@ -1201,6 +1206,10 @@ let gotSession = !!screenSession + if (!screenOV) { + screenOV = new OpenVidu() + } + // Init screen sharing session if (!gotSession) { screenSession = screenOV.initSession(); @@ -1208,6 +1217,13 @@ let successFunc = function() { screenSession.publish(screenPublisher) + + screenSession.on('sessionDisconnected', event => { + callback(false) + screenSession = null + screenPublisher = null + }) + if (callback) { callback(true) } @@ -1216,7 +1232,7 @@ let errorFunc = function() { screenPublisher = null if (callback) { - callback(false) + callback(false, true) } } diff --git a/src/resources/vue/Meet/Room.vue b/src/resources/vue/Meet/Room.vue --- a/src/resources/vue/Meet/Room.vue +++ b/src/resources/vue/Meet/Room.vue @@ -625,23 +625,26 @@ this.setMenuItem('video', enabled) }, switchScreen() { - this.meet.switchScreen(enabled => { - this.setMenuItem('screen', enabled) - - // After one screen sharing session ended request a new token - // for the next screen sharing session - if (!enabled) { - // TODO: This might need to be a different route. E.g. the room password might have - // changed since user joined the session - // Also because it creates a redundant connection (token) - axios.post('/api/v4/openvidu/rooms/' + this.room, this.post, { ignoreErrors: true }) - .then(response => { - // Response data contains: session, token and shareToken - this.session.shareToken = response.data.shareToken - this.meet.updateSession(this.session) - }) - } - }) + const switchScreenAction = () => { + this.meet.switchScreen((enabled, error) => { + this.setMenuItem('screen', enabled) + if (!enabled && !error) { + // Closing a screen sharing connection invalidates the token + delete this.session.shareToken + } + }) + } + + if (this.session.shareToken || !$('#meet-session-menu').find('.link-screen').is('.text-danger')) { + switchScreenAction() + } else { + axios.post('/api/v4/openvidu/rooms/' + this.room + '/connections') + .then(response => { + this.session.shareToken = response.data.token + this.meet.updateSession(this.session) + switchScreenAction() + }) + } }, updateParticipant(connId, params) { if (this.isModerator()) { diff --git a/src/routes/api.php b/src/routes/api.php --- a/src/routes/api.php +++ b/src/routes/api.php @@ -101,6 +101,7 @@ ], function () { Route::post('openvidu/rooms/{id}', 'API\V4\OpenViduController@joinRoom'); + Route::post('openvidu/rooms/{id}/connections', 'API\V4\OpenViduController@createConnection'); // FIXME: I'm not sure about this one, should we use DELETE request maybe? Route::post('openvidu/rooms/{id}/connections/{conn}/dismiss', 'API\V4\OpenViduController@dismissConnection'); Route::put('openvidu/rooms/{id}/connections/{conn}', 'API\V4\OpenViduController@updateConnection'); diff --git a/src/tests/Browser/Meet/RoomControlsTest.php b/src/tests/Browser/Meet/RoomControlsTest.php --- a/src/tests/Browser/Meet/RoomControlsTest.php +++ b/src/tests/Browser/Meet/RoomControlsTest.php @@ -273,6 +273,7 @@ ->assertElementsCount('@chat-list .message', 0) ->keys('@chat-input', 'test1', '{enter}') ->assertValue('@chat-input', '') + ->waitFor('@chat-list .message') ->assertElementsCount('@chat-list .message', 1) ->assertSeeIn('@chat-list .message .nickname', 'john') ->assertSeeIn('@chat-list .message div:last-child', 'test1'); diff --git a/src/tests/Feature/Controller/OpenViduTest.php b/src/tests/Feature/Controller/OpenViduTest.php --- a/src/tests/Feature/Controller/OpenViduTest.php +++ b/src/tests/Feature/Controller/OpenViduTest.php @@ -119,7 +119,6 @@ $this->assertSame($session_id, $json['session']); $this->assertTrue(is_string($session_id) && !empty($session_id)); $this->assertTrue(strpos($json['token'], 'wss://') === 0); - $this->assertTrue(!array_key_exists('shareToken', $json)); $john_token = $json['token']; @@ -131,7 +130,6 @@ $this->assertSame(322, $json['code']); $this->assertTrue(empty($json['token'])); - $this->assertTrue(empty($json['shareToken'])); // Non-owner, now the session exists, with 'init', but no 'canPublish' argument $response = $this->actingAs($jack)->post("api/v4/openvidu/rooms/{$room->name}", ['init' => 1]); @@ -143,7 +141,6 @@ $this->assertSame($session_id, $json['session']); $this->assertTrue(strpos($json['token'], 'wss://') === 0); $this->assertTrue($json['token'] != $john_token); - $this->assertTrue(empty($json['shareToken'])); // Non-owner, now the session exists, with 'init', and with 'role=PUBLISHER' $post = ['canPublish' => true, 'init' => 1]; @@ -156,7 +153,6 @@ $this->assertSame($session_id, $json['session']); $this->assertTrue(strpos($json['token'], 'wss://') === 0); $this->assertTrue($json['token'] != $john_token); - $this->assertTrue(!array_key_exists('shareToken', $json)); $this->assertEmpty($json['config']['password']); $this->assertEmpty($json['config']['requires_password']); @@ -359,8 +355,6 @@ $this->assertSame(Room::ROLE_PUBLISHER, $json['role']); $this->assertSame($room->session_id, $json['session']); $this->assertTrue(strpos($json['token'], 'wss://') === 0); - $this->assertTrue(strpos($json['shareToken'], 'wss://') === 0); - $this->assertTrue($json['shareToken'] != $json['token']); } /** @@ -417,6 +411,76 @@ } /** + * Test creating an extra connection for screen sharing + * + * @group openvidu + */ + public function testCreateConnection(): void + { + $john = $this->getTestUser('john@kolab.org'); + $jack = $this->getTestUser('jack@kolab.org'); + $room = Room::where('name', 'john')->first(); + $room->session_id = null; + $room->save(); + + $this->assignMeetEntitlement($john); + + // First we create the session + $post = ['init' => 1, 'canPublish' => 1]; + $response = $this->actingAs($john)->post("api/v4/openvidu/rooms/{$room->name}", $post); + $response->assertStatus(200); + + $json = $response->json(); + $owner_auth_token = $json['authToken']; + + // And the other user connection + $response = $this->actingAs($jack)->post("api/v4/openvidu/rooms/{$room->name}", ['init' => 1]); + $response->assertStatus(200); + + $json = $response->json(); + + $conn_id = $json['connectionId']; + $auth_token = $json['authToken']; + + // Non-existing room name + $response = $this->post("api/v4/openvidu/rooms/non-existing/connections", []); + $response->assertStatus(404); + + // No connection token provided + $response = $this->post("api/v4/openvidu/rooms/{$room->name}/connections", []); + $response->assertStatus(403); + + // Invalid token + $response = $this->actingAs($jack) + ->withHeaders([OpenViduController::AUTH_HEADER => '123']) + ->post("api/v4/openvidu/rooms/{$room->name}/connections", []); + + $response->assertStatus(403); + + // Subscriber can't get the screen-sharing connection + // Note: We're acting as Jack because there's no easy way to unset the 'actingAs' user + // throughout the test + $response = $this->actingAs($jack) + ->withHeaders([OpenViduController::AUTH_HEADER => $auth_token]) + ->post("api/v4/openvidu/rooms/{$room->name}/connections", []); + + $response->assertStatus(403); + + // Publisher can get the connection + $response = $this->actingAs($jack) + ->withHeaders([OpenViduController::AUTH_HEADER => $owner_auth_token]) + ->post("api/v4/openvidu/rooms/{$room->name}/connections", []); + + $response->assertStatus(200); + + $json = $response->json(); + + $this->assertSame('success', $json['status']); + $this->assertTrue(strpos($json['token'], 'wss://') === 0); + $this->assertTrue(strpos($json['token'], 'role=PUBLISHER') !== false); + } + + /** * Test dismissing a participant (closing a connection) * * @group openvidu @@ -679,7 +743,7 @@ /** * Create a moderator connection to the room session. * - * @param \App\Room $room The room + * @param \App\OpenVidu\Room $room The room * * @return string The connection authentication token */