diff --git a/lib/Service/HostedSignalingServerService.php b/lib/Service/HostedSignalingServerService.php index 891a4966c9d..d570e68c4bd 100644 --- a/lib/Service/HostedSignalingServerService.php +++ b/lib/Service/HostedSignalingServerService.php @@ -81,8 +81,7 @@ public function registerAccount(RegisterAccountData $registerAccountData): Accou $status = $response->getStatusCode(); switch ($status) { case Http::STATUS_UNAUTHORIZED: - $body = $response->getBody()->getContents(); - $this->logger->error('Requesting hosted signaling server trial failed: unauthorized - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Requesting hosted signaling server trial failed: unauthorized - HTTP status: ' . $status); $message = $this->l10n->t('There is a problem with the authentication of this instance. Maybe it is not reachable from the outside to verify it\'s URL.'); throw new HostedSignalingServerAPIException($message, $status); @@ -91,7 +90,7 @@ public function registerAccount(RegisterAccountData $registerAccountData): Accou if ($body) { $parsedBody = json_decode((string)$body, true); if (json_last_error() !== JSON_ERROR_NONE) { - $this->logger->error('Requesting hosted signaling server trial failed: cannot parse JSON response - JSON error: ' . json_last_error() . ' ' . json_last_error_msg() . ' HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Requesting hosted signaling server trial failed: cannot parse JSON response - JSON error: ' . json_last_error() . ' ' . json_last_error_msg() . ' HTTP status: ' . $status); $message = $this->l10n->t('Something unexpected happened.'); throw new HostedSignalingServerAPIException($message, $status); @@ -156,26 +155,22 @@ public function registerAccount(RegisterAccountData $registerAccountData): Accou $message = $this->l10n->t('Something unexpected happened.'); throw new HostedSignalingServerAPIException($message, $status); case Http::STATUS_TOO_MANY_REQUESTS: - $body = $response->getBody()->getContents(); - $this->logger->error('Requesting hosted signaling server trial failed: too many requests - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Requesting hosted signaling server trial failed: too many requests - HTTP status: ' . $status); $message = $this->l10n->t('Too many requests are send from your servers address. Please try again later.'); throw new HostedSignalingServerInputException($message, $status); case Http::STATUS_CONFLICT: - $body = $response->getBody()->getContents(); - $this->logger->error('Requesting hosted signaling server trial failed: already registered - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Requesting hosted signaling server trial failed: already registered - HTTP status: ' . $status); $message = $this->l10n->t('There is already a trial registered for this Nextcloud instance.'); throw new HostedSignalingServerInputException($message, $status); case Http::STATUS_INTERNAL_SERVER_ERROR: - $body = $response->getBody()->getContents(); - $this->logger->error('Requesting hosted signaling server trial failed: internal server error - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Requesting hosted signaling server trial failed: internal server error - HTTP status: ' . $status); $message = $this->l10n->t('Something unexpected happened. Please try again later.'); throw new HostedSignalingServerAPIException($message, $status); default: - $body = $response->getBody()->getContents(); - $this->logger->error('Requesting hosted signaling server trial failed: something else happened - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Requesting hosted signaling server trial failed: something else happened - HTTP status: ' . $status); $message = $this->l10n->t('Failed to request trial because the trial server behaved wrongly. Please try again later.'); throw new HostedSignalingServerAPIException($message, $status); @@ -194,8 +189,7 @@ public function registerAccount(RegisterAccountData $registerAccountData): Accou $status = $response->getStatusCode(); if ($status !== Http::STATUS_CREATED) { - $body = $response->getBody(); - $this->logger->error('Requesting hosted signaling server trial failed: something else happened - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Requesting hosted signaling server trial failed: something else happened - HTTP status: ' . $status); $message = $this->l10n->t('Something unexpected happened.'); throw new HostedSignalingServerAPIException($message, $status); @@ -205,14 +199,14 @@ public function registerAccount(RegisterAccountData $registerAccountData): Accou $data = json_decode($body, true); if (json_last_error() !== JSON_ERROR_NONE) { - $this->logger->error('Requesting hosted signaling server trial failed: cannot parse JSON response - JSON error: ' . json_last_error() . ' ' . json_last_error_msg() . ' HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Requesting hosted signaling server trial failed: cannot parse JSON response - JSON error: ' . json_last_error() . ' ' . json_last_error_msg() . ' HTTP status: ' . $status); $message = $this->l10n->t('Something unexpected happened.'); throw new HostedSignalingServerAPIException($message, Http::STATUS_INTERNAL_SERVER_ERROR); } if (!isset($data['account_id'])) { - $this->logger->error('Requesting hosted signaling server trial failed: no account ID transfered - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Requesting hosted signaling server trial failed: no account ID transfered - HTTP status: ' . $status); $message = $this->l10n->t('Something unexpected happened.'); throw new HostedSignalingServerAPIException($message, Http::STATUS_INTERNAL_SERVER_ERROR); @@ -264,8 +258,7 @@ public function fetchAccountInfo(AccountId $accountId) { switch ($status) { case Http::STATUS_UNAUTHORIZED: - $body = $response->getBody()->getContents(); - $this->logger->error('Getting the account information failed: unauthorized - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Getting the account information failed: unauthorized - HTTP status: ' . $status); $message = $this->l10n->t('There is a problem with the authentication of this request. Maybe it is not reachable from the outside to verify it\'s URL.'); throw new HostedSignalingServerAPIException($message, $status); @@ -274,7 +267,7 @@ public function fetchAccountInfo(AccountId $accountId) { if ($body) { $parsedBody = json_decode((string)$body, true); if (json_last_error() !== JSON_ERROR_NONE) { - $this->logger->error('Getting the account information failed: cannot parse JSON response - JSON error: ' . json_last_error() . ' ' . json_last_error_msg() . ' HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Getting the account information failed: cannot parse JSON response - JSON error: ' . json_last_error() . ' ' . json_last_error_msg() . ' HTTP status: ' . $status); $message = $this->l10n->t('Something unexpected happened.'); throw new HostedSignalingServerAPIException($message, $status); @@ -285,8 +278,7 @@ public function fetchAccountInfo(AccountId $accountId) { $log = 'The account ID is missing.'; break; default: - $body = $response->getBody()->getContents(); - $this->logger->error('Getting the account information failed: something else happened - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Getting the account information failed: something else happened - HTTP status: ' . $status); $message = $this->l10n->t('Failed to fetch account information because the trial server behaved wrongly. Please check back later.'); throw new HostedSignalingServerAPIException($message, $status); @@ -301,26 +293,22 @@ public function fetchAccountInfo(AccountId $accountId) { $message = $this->l10n->t('Something unexpected happened.'); throw new HostedSignalingServerAPIException($message, $status); case Http::STATUS_TOO_MANY_REQUESTS: - $body = $response->getBody()->getContents(); - $this->logger->error('Getting the account information failed: too many requests - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Getting the account information failed: too many requests - HTTP status: ' . $status); $message = $this->l10n->t('Too many requests are send from your servers address. Please try again later.'); throw new HostedSignalingServerAPIException($message, $status); case Http::STATUS_NOT_FOUND: - $body = $response->getBody()->getContents(); - $this->logger->error('Getting the account information failed: account not found - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Getting the account information failed: account not found - HTTP status: ' . $status); $message = $this->l10n->t('There is no such account registered.'); throw new HostedSignalingServerAPIException($message, $status); case Http::STATUS_INTERNAL_SERVER_ERROR: - $body = $response->getBody()->getContents(); - $this->logger->error('Getting the account information failed: internal server error - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Getting the account information failed: internal server error - HTTP status: ' . $status); $message = $this->l10n->t('Something unexpected happened. Please try again later.'); throw new HostedSignalingServerAPIException($message, $status); default: - $body = $response->getBody()->getContents(); - $this->logger->error('Getting the account information failed: something else happened - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Getting the account information failed: something else happened - HTTP status: ' . $status); $message = $this->l10n->t('Failed to fetch account information because the trial server behaved wrongly. Please check back later.'); throw new HostedSignalingServerAPIException($message, $status); @@ -339,8 +327,7 @@ public function fetchAccountInfo(AccountId $accountId) { $status = $response->getStatusCode(); if ($status !== Http::STATUS_OK) { - $body = $response->getBody(); - $this->logger->error('Getting the account information failed: something else happened - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Getting the account information failed: something else happened - HTTP status: ' . $status); $message = $this->l10n->t('Something unexpected happened.'); throw new HostedSignalingServerAPIException($message, $status); @@ -350,7 +337,7 @@ public function fetchAccountInfo(AccountId $accountId) { $data = (array)json_decode($body, true); if (json_last_error() !== JSON_ERROR_NONE) { - $this->logger->error('Getting the account information failed: cannot parse JSON response - JSON error: ' . json_last_error() . ' ' . json_last_error_msg() . ' HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Getting the account information failed: cannot parse JSON response - JSON error: ' . json_last_error() . ' ' . json_last_error_msg() . ' HTTP status: ' . $status); $message = $this->l10n->t('Something unexpected happened.'); throw new HostedSignalingServerAPIException($message, Http::STATUS_INTERNAL_SERVER_ERROR); @@ -381,7 +368,7 @@ public function fetchAccountInfo(AccountId $accountId) { || (in_array($accountStatus, ['error', 'blocked'], true) && !isset($data['reason'])) || !in_array($accountStatus, ['error', 'blocked', 'pending', 'active', 'expired'], true) ) { - $this->logger->error('Getting the account information failed: response is missing mandatory field - data: ' . json_encode($data)); + $this->logger->error('Getting the account information failed: response is missing mandatory field - account status: ' . json_encode($accountStatus)); $message = $this->l10n->t('Something unexpected happened.'); throw new HostedSignalingServerAPIException($message, Http::STATUS_INTERNAL_SERVER_ERROR); @@ -418,8 +405,7 @@ public function deleteAccount(AccountId $accountId): void { switch ($status) { case Http::STATUS_UNAUTHORIZED: - $body = $response->getBody()->getContents(); - $this->logger->error('Deleting the hosted signaling server account failed: unauthorized - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Deleting the hosted signaling server account failed: unauthorized - HTTP status: ' . $status); $message = $this->l10n->t('There is a problem with the authentication of this request. Maybe it is not reachable from the outside to verify it\'s URL.'); throw new HostedSignalingServerAPIException($message, $status); @@ -428,7 +414,7 @@ public function deleteAccount(AccountId $accountId): void { if ($body) { $parsedBody = json_decode((string)$body, true); if (json_last_error() !== JSON_ERROR_NONE) { - $this->logger->error('Deleting the hosted signaling server account failed: cannot parse JSON response - JSON error: ' . json_last_error() . ' ' . json_last_error_msg() . ' HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Deleting the hosted signaling server account failed: cannot parse JSON response - JSON error: ' . json_last_error() . ' ' . json_last_error_msg() . ' HTTP status: ' . $status); $message = $this->l10n->t('Something unexpected happened.'); throw new HostedSignalingServerAPIException($message, $status); @@ -439,8 +425,7 @@ public function deleteAccount(AccountId $accountId): void { $log = 'The account ID is missing.'; break; default: - $body = $response->getBody()->getContents(); - $this->logger->error('Deleting the hosted signaling server account failed: something else happened - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Deleting the hosted signaling server account failed: something else happened - HTTP status: ' . $status); $message = $this->l10n->t('Failed to delete the account because the trial server behaved wrongly. Please check back later.'); throw new HostedSignalingServerAPIException($message, $status); @@ -455,26 +440,22 @@ public function deleteAccount(AccountId $accountId): void { $message = $this->l10n->t('Something unexpected happened.'); throw new HostedSignalingServerAPIException($message, $status); case Http::STATUS_TOO_MANY_REQUESTS: - $body = $response->getBody()->getContents(); - $this->logger->error('Deleting the hosted signaling server account failed: too many requests - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Deleting the hosted signaling server account failed: too many requests - HTTP status: ' . $status); $message = $this->l10n->t('Too many requests are sent from your servers address. Please try again later.'); throw new HostedSignalingServerAPIException($message, $status); case Http::STATUS_NOT_FOUND: - $body = $response->getBody()->getContents(); - $this->logger->error('Deleting the hosted signaling server account failed: account not found - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Deleting the hosted signaling server account failed: account not found - HTTP status: ' . $status); $message = $this->l10n->t('There is no such account registered.'); throw new HostedSignalingServerAPIException($message, $status); case Http::STATUS_INTERNAL_SERVER_ERROR: - $body = $response->getBody()->getContents(); - $this->logger->error('Deleting the hosted signaling server account failed: internal server error - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Deleting the hosted signaling server account failed: internal server error - HTTP status: ' . $status); $message = $this->l10n->t('Something unexpected happened. Please try again later.'); throw new HostedSignalingServerAPIException($message, $status); default: - $body = $response->getBody()->getContents(); - $this->logger->error('Deleting the hosted signaling server account failed: something else happened - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Deleting the hosted signaling server account failed: something else happened - HTTP status: ' . $status); $message = $this->l10n->t('Failed to delete the account because the trial server behaved wrongly. Please check back later.'); throw new HostedSignalingServerAPIException($message, $status); @@ -493,8 +474,7 @@ public function deleteAccount(AccountId $accountId): void { $status = $response->getStatusCode(); if ($status !== Http::STATUS_NO_CONTENT) { - $body = $response->getBody(); - $this->logger->error('Deleting the hosted signaling server account failed: something else happened - HTTP status: ' . $status . ' Response body: ' . $body); + $this->logger->error('Deleting the hosted signaling server account failed: something else happened - HTTP status: ' . $status); $message = $this->l10n->t('Something unexpected happened.'); throw new HostedSignalingServerAPIException($message, $status); diff --git a/tests/php/Service/HostedSignalingServerServiceTest.php b/tests/php/Service/HostedSignalingServerServiceTest.php index ba9aedb4b24..ea192f49d51 100644 --- a/tests/php/Service/HostedSignalingServerServiceTest.php +++ b/tests/php/Service/HostedSignalingServerServiceTest.php @@ -10,6 +10,7 @@ use OCA\Talk\DataObjects\AccountId; use OCA\Talk\DataObjects\RegisterAccountData; +use OCA\Talk\Exceptions\HostedSignalingServerAPIException; use OCA\Talk\Service\HostedSignalingServerService; use OCP\AppFramework\Http; use OCP\AppFramework\Services\IAppConfig; @@ -28,6 +29,7 @@ class HostedSignalingServerServiceTest extends TestCase { protected IClient&MockObject $client; protected IEventDispatcher&MockObject $dispatcher; + protected LoggerInterface&MockObject $logger; protected HostedSignalingServerService $service; public function setUp(): void { @@ -39,12 +41,13 @@ public function setUp(): void { $clientService = $this->createMock(IClientService::class); $clientService->method('newClient')->willReturn($this->client); $this->dispatcher = $this->createMock(IEventDispatcher::class); + $this->logger = $this->createMock(LoggerInterface::class); $this->service = new HostedSignalingServerService( $config, $this->createMock(IAppConfig::class), $clientService, - $this->createMock(LoggerInterface::class), + $this->logger, $this->createMock(IL10N::class), $this->createMock(ISecureRandom::class), $this->dispatcher, @@ -69,6 +72,25 @@ public function testRegisterAccountLogsCriticalActionWithoutContactData(): void $this->service->registerAccount(new RegisterAccountData('https://cloud.example', 'Admin', 'admin@example.com', 'en', 'DE')); } + public function testFetchAccountInfoDoesNotLogSecretsOrBodies(): void { + $this->logger->expects($this->once()) + ->method('error') + ->with($this->logicalNot($this->logicalOr( + $this->stringContains('signaling-secret'), + $this->stringContains('admin@example.com'), + ))); + + $this->client->method('get')->willReturn($this->createResponse(Http::STATUS_OK, json_encode([ + 'status' => 'active', + 'created' => 'now', + 'signaling' => ['url' => 'wss://signaling.example', 'secret' => 'signaling-secret'], + 'owner' => ['email' => 'admin@example.com'], + ]))); + + $this->expectException(HostedSignalingServerAPIException::class); + $this->service->fetchAccountInfo(new AccountId('account-1')); + } + public function testDeleteAccountLogsCriticalAction(): void { $this->client->method('delete')->willReturn($this->createResponse(Http::STATUS_NO_CONTENT));