Browse Source

Fixes after code review

Signed-off-by: Vitor Mattos <vitor@php.rio>
pull/8492/head
Vitor Mattos 3 years ago
parent
commit
0d3a83b7e9
No known key found for this signature in database GPG Key ID: B7AB4B76A7CA7318
  1. 8
      docs/recording.md
  2. 76
      docs/settings.md
  3. 15
      lib/Service/RecordingService.php
  4. 23
      tests/php/Service/RecordingServiceTest.php

8
docs/recording.md

@ -46,10 +46,10 @@
* Header:
| field | type | Description |
| ------------------------- | ------ | ------------------------------------------------------------------------------------------------------------------------- |
| `TALK_SIPBRIDGE_RANDOM` | string | Random string that need to be concatenated with room token to generate the checksum using the `sip_bridge_shared_secret`. |
| `TALK_SIPBRIDGE_CHECKSUM` | string | The checksum generated with `TALK_SIPBRIDGE_RANDOM`. |
| field | type | Description |
| ------------------------- | ------ | -------------------------------------------------------------------------------------------------------------------------- |
| `TALK_SIPBRIDGE_RANDOM` | string | Random string that needs to be concatenated with room token to generate the checksum using the `sip_bridge_shared_secret`. |
| `TALK_SIPBRIDGE_CHECKSUM` | string | The checksum generated with `TALK_SIPBRIDGE_RANDOM`. |
* Data:

76
docs/settings.md

@ -56,41 +56,41 @@ Option legend:
* 🖌️ - UI option in the admin settings available
* 💻 - Dedicated OCC command available
| Key | Internal type | Default | Option | Valid values |
|--------------------------------------|------------------------------------------------------------------|---------------------------------------------------------------------------|--------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `allowed_groups` | string[] | `[]` | 🖌️ | List of group ids that are allowed to use Talk |
| `sip_bridge_groups` | string[] | `[]` | 🖌️ | List of group ids that are allowed to enable SIP dial-in in a conversation |
| `start_conversations` | string[] | `[]` | 🖌️ | List of group ids that are allowed to create conversations |
| `hosted-signaling-server-account` | array | `{}` | 🖌️ | Account information of the hosted signaling server |
| `stun_servers` | array[] | `[]` | 🖌💻️ | List of STUN servers, should be configured via the web interface or the OCC commands |
| `turn_servers` | array[] | `[]` | 🖌️💻 | List of TURN servers, should be configured via the web interface or the OCC commands |
| `signaling_servers` | array[] | `[]` | 🖌️💻 | List of signaling servers, should be configured via the web interface or the OCC commands |
| `signaling_mode` | string<br>`internal` or `external` or `conversation_cluster` | `internal` | | `internal` when no HPB is configured, `external` when configured, `conversation_cluster` is an experimental flag that is deprecated |
| `sip_bridge_dialin_info` | string | | 🖌️ | Additional information added in the SIP dial-in invitation mail and sidebar |
| `sip_bridge_shared_secret` | string | | 🖌️ | Shared secret allowing the SIP bridge to authenticate on the Nextcloud server |
| `signaling_ticket_secret` | string | | | Secret used to secure the signaling tickets for guests (255 character random string) |
| `signaling_token_alg` | string<br>`ES256`, `ES384`, `RS256`, `RS384`, `RS512` or `EdDSA` | `ES256` | | Algorithm for the signaling tickets |
| `signaling_token_privkey_*` | string | * | | Private key for the signaling ticket creation by the server |
| `signaling_token_pubkey_*` | string | * | | Public key for the signaling ticket creation by the server |
| `hosted-signaling-server-nonce` | string | | | Temporary nonce while configuring the hosted signaling server |
| `hosted-signaling-server-account-id` | string | | | Account identifier of the hosted signaling server |
| `matterbridge_binary` | string | | | Path to the matterbridge binary file |
| `bridge_bot_password` | string | | | Automatically generated password of the matterbridge bot user profile |
| `start_calls` | int | `0` | 🖌️ | Who can start a call, see [constants list](constants.md#start-call) |
| `max-gif-size` | int | `3145728` | | Maximum file size for clients to render gifs previews with animation |
| `session-ping-limit` | int | `200` | | Number of sessions the HPB can ping in a single request |
| `token_entropy` | int | `8` | | Length of conversation tokens, can be increased to make tokens harder to guess but reduces readability and dial-in comfort |
| `default_group_notification` | int | `2` | 🖌️ | Default notification level for group conversations [constants list](constants.md#participant-notification-levels) |
| `default_permissions` | int | `246` | | Default permissions for non-moderators (see [constants list](constants.md#attendee-permissions) for bit flags) |
| `grid_videos_limit` | int | `19` | | Maximum number of videos to show (additional to the own video) |
| `grid_videos_limit_enforced` | string<br>`yes` or `no` | `no` | | Whether the number of grid videos should be enforced |
| `changelog` | string<br>`yes` or `no` | `yes` | | Whether the changelog conversation is updated with new features on major releases |
| `has_reference_id` | string<br>`yes` or `no` | `no` | | Indicator whether the clients can use the reference value to identify their message, will be automatically set to `yes` when the repair steps are executed |
| `hide_signaling_warning` | string<br>`yes` or `no` | `no` | 🖌️ | Flag that allows to suppress the warning that an HPB should be configured |
| `signaling_dev` | string<br>`yes` or `no` | `no` | | Developer flag that allows to suppress various requirements like a Redis server when using the HPB |
| `breakout_rooms` | string<br>`yes` or `no` | `yes` | | Whether or not breakout rooms are allowed (Will only prevent creating new breakout rooms. Existing conversations are not modified.) |
| `call_recording` | string<br>`yes` or `no` | `yes` | | Enable call recording |
| `federation_enabled` | string<br>`yes` or `no` | `no` | | 🏗️ *Work in progress:* Whether or not federation with this instance is allowed |
| `conversations_files` | string<br>`1` or `0` | `1` | 🖌️ | Whether the files app integration is enabled allowing to start conversations in the right sidebar |
| `conversations_files_public_shares` | string<br>`1` or `0` | `1` | 🖌️ | Whether the public share integration is enabled allowing to start conversations in the right sidebar on the public share page (Requires `conversations_files` also to be enabled) |
| `enable_matterbridge` | string<br>`1` or `0` | `0` | 🖌️ | Whether the matterbridge integration is enabled and can be configured |
| Key | Internal type | Default | Option | Valid values |
|--------------------------------------|------------------------------------------------------------------|------------|--------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `allowed_groups` | string[] | `[]` | 🖌️ | List of group ids that are allowed to use Talk |
| `sip_bridge_groups` | string[] | `[]` | 🖌️ | List of group ids that are allowed to enable SIP dial-in in a conversation |
| `start_conversations` | string[] | `[]` | 🖌️ | List of group ids that are allowed to create conversations |
| `hosted-signaling-server-account` | array | `{}` | 🖌️ | Account information of the hosted signaling server |
| `stun_servers` | array[] | `[]` | 🖌💻️ | List of STUN servers, should be configured via the web interface or the OCC commands |
| `turn_servers` | array[] | `[]` | 🖌️💻 | List of TURN servers, should be configured via the web interface or the OCC commands |
| `signaling_servers` | array[] | `[]` | 🖌️💻 | List of signaling servers, should be configured via the web interface or the OCC commands |
| `signaling_mode` | string<br>`internal` or `external` or `conversation_cluster` | `internal` | | `internal` when no HPB is configured, `external` when configured, `conversation_cluster` is an experimental flag that is deprecated |
| `sip_bridge_dialin_info` | string | | 🖌️ | Additional information added in the SIP dial-in invitation mail and sidebar |
| `sip_bridge_shared_secret` | string | | 🖌️ | Shared secret allowing the SIP bridge to authenticate on the Nextcloud server |
| `signaling_ticket_secret` | string | | | Secret used to secure the signaling tickets for guests (255 character random string) |
| `signaling_token_alg` | string<br>`ES256`, `ES384`, `RS256`, `RS384`, `RS512` or `EdDSA` | `ES256` | | Algorithm for the signaling tickets |
| `signaling_token_privkey_*` | string | * | | Private key for the signaling ticket creation by the server |
| `signaling_token_pubkey_*` | string | * | | Public key for the signaling ticket creation by the server |
| `hosted-signaling-server-nonce` | string | | | Temporary nonce while configuring the hosted signaling server |
| `hosted-signaling-server-account-id` | string | | | Account identifier of the hosted signaling server |
| `matterbridge_binary` | string | | | Path to the matterbridge binary file |
| `bridge_bot_password` | string | | | Automatically generated password of the matterbridge bot user profile |
| `start_calls` | int | `0` | 🖌️ | Who can start a call, see [constants list](constants.md#start-call) |
| `max-gif-size` | int | `3145728` | | Maximum file size for clients to render gifs previews with animation |
| `session-ping-limit` | int | `200` | | Number of sessions the HPB can ping in a single request |
| `token_entropy` | int | `8` | | Length of conversation tokens, can be increased to make tokens harder to guess but reduces readability and dial-in comfort |
| `default_group_notification` | int | `2` | 🖌️ | Default notification level for group conversations [constants list](constants.md#participant-notification-levels) |
| `default_permissions` | int | `246` | | Default permissions for non-moderators (see [constants list](constants.md#attendee-permissions) for bit flags) |
| `grid_videos_limit` | int | `19` | | Maximum number of videos to show (additional to the own video) |
| `grid_videos_limit_enforced` | string<br>`yes` or `no` | `no` | | Whether the number of grid videos should be enforced |
| `changelog` | string<br>`yes` or `no` | `yes` | | Whether the changelog conversation is updated with new features on major releases |
| `has_reference_id` | string<br>`yes` or `no` | `no` | | Indicator whether the clients can use the reference value to identify their message, will be automatically set to `yes` when the repair steps are executed |
| `hide_signaling_warning` | string<br>`yes` or `no` | `no` | 🖌️ | Flag that allows to suppress the warning that an HPB should be configured |
| `signaling_dev` | string<br>`yes` or `no` | `no` | | Developer flag that allows to suppress various requirements like a Redis server when using the HPB |
| `breakout_rooms` | string<br>`yes` or `no` | `yes` | | Whether or not breakout rooms are allowed (Will only prevent creating new breakout rooms. Existing conversations are not modified.) |
| `call_recording` | string<br>`yes` or `no` | `yes` | | Enable call recording |
| `federation_enabled` | string<br>`yes` or `no` | `no` | | 🏗️ *Work in progress:* Whether or not federation with this instance is allowed |
| `conversations_files` | string<br>`1` or `0` | `1` | 🖌️ | Whether the files app integration is enabled allowing to start conversations in the right sidebar |
| `conversations_files_public_shares` | string<br>`1` or `0` | `1` | 🖌️ | Whether the public share integration is enabled allowing to start conversations in the right sidebar on the public share page (Requires `conversations_files` also to be enabled) |
| `enable_matterbridge` | string<br>`1` or `0` | `0` | 🖌️ | Whether the matterbridge integration is enabled and can be configured |

15
lib/Service/RecordingService.php

@ -62,8 +62,8 @@ class RecordingService {
public function store(Room $room, string $owner, array $file): void {
$content = $this->getContentFromFileArray($file);
$this->validateFileName($file['name']);
$this->validateFileFormat($file['name'], $content);
$fileName = basename($file['name']);
$this->validateFileFormat($fileName, $content);
try {
$this->participantService->getParticipant($room, $owner);
@ -73,7 +73,7 @@ class RecordingService {
try {
$recordingFolder = $this->getRecordingFolder($owner, $room->getToken());
$recordingFolder->newFile($file['name'], $content);
$recordingFolder->newFile($fileName, $content);
} catch (NoUserException $e) {
throw new InvalidArgumentException('owner_invalid');
} catch (NotPermittedException $e) {
@ -111,15 +111,6 @@ class RecordingService {
}
}
public function validateFileName(string $fileName): string {
$recordFileName = escapeshellcmd($fileName);
$recordFileName = pathinfo($recordFileName, PATHINFO_BASENAME);
if ($recordFileName !== $fileName) {
throw new InvalidArgumentException('file_name');
}
return $recordFileName;
}
private function getRecordingFolder(string $owner, string $token): Folder {
$userFolder = $this->rootFolder->getUserFolder($owner);
$recordingRootFolderName = $this->config->getRecordingFolder($owner);

23
tests/php/Service/RecordingServiceTest.php

@ -71,29 +71,6 @@ class RecordingServiceTest extends TestCase {
);
}
/**
* @dataProvider dataValidateFileName
*/
public function testValidateFileName(string $name, string $expected, string $exceptionMessage): void {
if ($exceptionMessage) {
$this->expectExceptionMessage($exceptionMessage);
}
$actual = $this->recordingService->validateFileName($name);
$this->assertEquals($expected, $actual);
}
public function dataValidateFileName(): array {
return [
['a/b', '', 'file_name'],
['a`b', '', 'file_name'],
['a\b', '', 'file_name'],
['../ab', '', 'file_name'],
['{}ab', '', 'file_name'],
['[]ab', '', 'file_name'],
['a.b', 'a.b', ''],
];
}
/** @dataProvider dataValidateFileFormat */
public function testValidateFileFormat(string $fileName, string $content, string $exceptionMessage):void {
if ($exceptionMessage) {

Loading…
Cancel
Save