Skip to content

fix: Don't log hosted HPB responses in the log #20019

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sudormant merged 1 commit into from
Oct 27, 2026
Merged

fix: Don't log hosted HPB responses in the log #20019

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 27 additions & 47 deletions lib/Service/HostedSignalingServerService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
24 changes: 23 additions & 1 deletion tests/php/Service/HostedSignalingServerServiceTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand All @@ -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,
Expand All @@ -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));

Expand Down
Loading