fix: Apply suggestions from code review

Signed-off-by: Jana Peper <jana.peper@nextcloud.com>
pull/55790/head
Jana Peper 2025-10-17 16:23:24 +07:00 committed by janepie
parent db158ce413
commit 2daff2ddae
6 changed files with 91 additions and 56 deletions

@ -20,6 +20,7 @@ return array(
'OCA\\WebhookListeners\\Migration\\Version1500Date20251007130000' => $baseDir . '/../lib/Migration/Version1500Date20251007130000.php',
'OCA\\WebhookListeners\\ResponseDefinitions' => $baseDir . '/../lib/ResponseDefinitions.php',
'OCA\\WebhookListeners\\Service\\PHPMongoQuery' => $baseDir . '/../lib/Service/PHPMongoQuery.php',
'OCA\\WebhookListeners\\Service\\TokenService' => $baseDir . '/../lib/Service/TokenService.php',
'OCA\\WebhookListeners\\Settings\\Admin' => $baseDir . '/../lib/Settings/Admin.php',
'OCA\\WebhookListeners\\Settings\\AdminSection' => $baseDir . '/../lib/Settings/AdminSection.php',
);

@ -35,6 +35,7 @@ class ComposerStaticInitWebhookListeners
'OCA\\WebhookListeners\\Migration\\Version1500Date20251007130000' => __DIR__ . '/..' . '/../lib/Migration/Version1500Date20251007130000.php',
'OCA\\WebhookListeners\\ResponseDefinitions' => __DIR__ . '/..' . '/../lib/ResponseDefinitions.php',
'OCA\\WebhookListeners\\Service\\PHPMongoQuery' => __DIR__ . '/..' . '/../lib/Service/PHPMongoQuery.php',
'OCA\\WebhookListeners\\Service\\TokenService' => __DIR__ . '/..' . '/../lib/Service/TokenService.php',
'OCA\\WebhookListeners\\Settings\\Admin' => __DIR__ . '/..' . '/../lib/Settings/Admin.php',
'OCA\\WebhookListeners\\Settings\\AdminSection' => __DIR__ . '/..' . '/../lib/Settings/AdminSection.php',
);

@ -12,6 +12,7 @@ namespace OCA\WebhookListeners\BackgroundJobs;
use OCA\AppAPI\PublicFunctions;
use OCA\WebhookListeners\Db\AuthMethod;
use OCA\WebhookListeners\Db\WebhookListenerMapper;
use OCA\WebhookListeners\Service\TokenService;
use OCP\App\IAppManager;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\QueuedJob;
@ -30,6 +31,7 @@ class WebhookCall extends QueuedJob {
private WebhookListenerMapper $mapper,
private LoggerInterface $logger,
private IAppManager $appManager,
private TokenService $tokenService,
ITimeFactory $timeFactory,
) {
parent::__construct($timeFactory);
@ -44,7 +46,7 @@ class WebhookCall extends QueuedJob {
$client = $this->clientService->newClient();
// adding temporary auth tokens to the call
$data['tokens'] = $this->getTokens($webhookListener, $data['user']['uid']);
$data['tokens'] = $this->tokenService->getTokens($webhookListener, $data['user']['uid']);
$options = [
'verify' => $this->certificateManager->getAbsoluteBundlePath(),
'headers' => $webhookListener->getHeaders() ?? [],
@ -96,30 +98,5 @@ class WebhookCall extends QueuedJob {
}
}
private function getTokens($webhookListener, $triggerUserId): array {
$tokens = [];
$tokenNeeded = $webhookListener->getTokenNeeded();
if (isset($tokenNeeded['users'])) {
foreach ($tokenNeeded['users'] as $userId) {
$tokens['users'][$userId] = $webhookListener->createTemporaryToken($userId);
}
}
if (isset($tokenNeeded['users'])) {
foreach ($tokenNeeded['functions'] as $function) {
switch ($function) {
case 'owner':
// token for the person who created the flow
$functionId = $webhookListener->getUserId();
$tokens['functions']['owner'][$functionId] = $webhookListener->createTemporaryToken($functionId);
break;
case 'trigger':
// token for the person who triggered the webhook
$tokens['functions']['trigger'][$triggerUserId] = $webhookListener->createTemporaryToken($triggerUserId);
break;
}
}
}
return $tokens;
}
}

@ -113,7 +113,7 @@ class WebhooksController extends OCSController {
* @param "none"|"header"|null $authMethod Authentication method to use
* @param ?array<string,mixed> $authData Array of data for authentication
* @param ?array<string,mixed> $tokenNeeded List of user ids for which to include auth tokens in the event.
* Has to fields: "users" lists uids of users for which tokens are needed, "functions" lists functions for which tokens can be included.
* Has two fields: "users" list of user uids for which tokens are needed, "functions" list of functions for which tokens can be included.
* Possible functions: "owner" for the user creating the webhook, "trigger" for the user triggering the webhook call
*
* @return DataResponse<Http::STATUS_OK, WebhookListenersWebhookInfo, array{}>
@ -186,7 +186,7 @@ class WebhooksController extends OCSController {
* @param "none"|"header"|null $authMethod Authentication method to use
* @param ?array<string,mixed> $authData Array of data for authentication
* @param ?array<string,mixed> $tokenNeeded List of user ids for which to include auth tokens in the event.
* Has to fields: "users" lists uids of users for which tokens are needed, "functions" lists functions for which tokens can be included.
* Has two fields: "users" list of user uids for which tokens are needed, "functions" list of functions for which tokens can be included.
* Possible functions: "owner" for the user creating the webhook, "trigger" for the user triggering the webhook call
*
* @return DataResponse<Http::STATUS_OK, WebhookListenersWebhookInfo, array{}>

@ -9,11 +9,8 @@ declare(strict_types=1);
namespace OCA\WebhookListeners\Db;
use OC\Authentication\Token\IProvider;
use OCP\AppFramework\Db\Entity;
use OCP\Authentication\Token\IToken;
use OCP\Security\ICrypto;
use OCP\Security\ISecureRandom;
use OCP\Server;
/**
@ -96,24 +93,14 @@ class WebhookListener extends Entity implements \JsonSerializable {
private ICrypto $crypto;
private IProvider $tokenProvider;
public function __construct(
?ICrypto $crypto = null,
?IProvider $tokenProvider = null,
private ?ISecureRandom $random = null,
) {
if ($crypto === null) {
$crypto = Server::get(ICrypto::class);
}
$this->crypto = $crypto;
if ($tokenProvider === null) {
$tokenProvider = Server::get(IProvider::class);
}
$this->tokenProvider = $tokenProvider;
if ($random === null) {
$random = Server::get(ISecureRandom::class);
}
$this->random = $random;
$this->addType('appId', 'string');
$this->addType('userId', 'string');
$this->addType('httpMethod', 'string');
@ -169,19 +156,5 @@ class WebhookListener extends Entity implements \JsonSerializable {
}
public function createTemporaryToken($userId) {
$token = $this->generateRandomDeviceToken();
$name = 'Authentication for Webhook';
$password = null;
$deviceToken = $this->tokenProvider->generateToken($token, $userId, $userId, $password, $name, IToken::PERMANENT_TOKEN);
return $token;
}
private function generateRandomDeviceToken() {
$groups = [];
for ($i = 0; $i < 5; $i++) {
$groups[] = $this->random->generate(5, ISecureRandom::CHAR_HUMAN_READABLE);
}
return implode('-', $groups);
}
}

@ -0,0 +1,83 @@
<?php
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCA\WebhookListeners\Service;
use OC\Authentication\Token\IProvider;
use OCA\WebhookListeners\Db\WebhookListener;
use OCP\Authentication\Token\IToken;
use OCP\Security\ISecureRandom;
class TokenService {
public function __construct(
private IProvider $tokenProvider,
private ISecureRandom $random,
) {
}
/**
* creates an array which includes two arrays of tokens: 'users' and 'functions'
* The array ['users' => ['jane', 'bob'], 'functions' => ['owner', 'trigger']]
* as requested tokens in the registered webhook produces a result like
* ['users' => [['jane' => 'abcdtokenabcd1'], ['bob','=> 'abcdtokenabcd2']], 'functions' => [['owner' => ['admin' => 'abcdtokenabcd3']], ['trigger' => ['user1' => 'abcdtokenabcd4']]]]
*
* @param WebhookListener $webhookListener
* @param string|null $triggerUserId the user that triggered the webhook call
* @return array
*/
public function getTokens(WebhookListener $webhookListener, ?string $triggerUserId): array {
$tokens = [
'users' => [],
'functions' => [],
];
$tokenNeeded = $webhookListener->getTokenNeeded();
if (isset($tokenNeeded['users'])) {
foreach ($tokenNeeded['users'] as $userId) {
$tokens['users'][$userId] = $webhookListener->createTemporaryToken($userId);
}
}
if (isset($tokenNeeded['users'])) {
foreach ($tokenNeeded['functions'] as $function) {
switch ($function) {
case 'owner':
// token for the person who created the flow
$functionId = $webhookListener->getUserId();
$tokens['functions']['owner'] = [
$functionId => $webhookListener->createTemporaryToken($functionId)
];
break;
case 'trigger':
// token for the person who triggered the webhook
$tokens['functions']['trigger'] = [
$triggerUserId => $webhookListener->createTemporaryToken($triggerUserId)
];
break;
}
}
}
return $tokens;
}
public function createTemporaryToken(string $userId): string {
$token = $this->generateRandomDeviceToken();
$name = 'Ephemeral webhook authentication';
$password = null;
$deviceToken = $this->tokenProvider->generateToken($token, $userId, $userId, $password, $name, IToken::PERMANENT_TOKEN);
return $token;
}
private function generateRandomDeviceToken(): string {
$groups = [];
for ($i = 0; $i < 5; $i++) {
$groups[] = $this->random->generate(5, ISecureRandom::CHAR_HUMAN_READABLE);
}
return implode('-', $groups);
}
}