From 2daff2ddaedb573a3cc3b073eaa37ae0e966b573 Mon Sep 17 00:00:00 2001 From: Jana Peper Date: Fri, 17 Oct 2025 16:23:24 +0200 Subject: [PATCH] fix: Apply suggestions from code review Signed-off-by: Jana Peper --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../lib/BackgroundJobs/WebhookCall.php | 29 +------ .../lib/Controller/WebhooksController.php | 4 +- .../lib/Db/WebhookListener.php | 29 +------ .../lib/Service/TokenService.php | 83 +++++++++++++++++++ 6 files changed, 91 insertions(+), 56 deletions(-) create mode 100644 apps/webhook_listeners/lib/Service/TokenService.php diff --git a/apps/webhook_listeners/composer/composer/autoload_classmap.php b/apps/webhook_listeners/composer/composer/autoload_classmap.php index fec930f65a6..97d77e4f13b 100644 --- a/apps/webhook_listeners/composer/composer/autoload_classmap.php +++ b/apps/webhook_listeners/composer/composer/autoload_classmap.php @@ -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', ); diff --git a/apps/webhook_listeners/composer/composer/autoload_static.php b/apps/webhook_listeners/composer/composer/autoload_static.php index 0380f51c721..58589f49005 100644 --- a/apps/webhook_listeners/composer/composer/autoload_static.php +++ b/apps/webhook_listeners/composer/composer/autoload_static.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', ); diff --git a/apps/webhook_listeners/lib/BackgroundJobs/WebhookCall.php b/apps/webhook_listeners/lib/BackgroundJobs/WebhookCall.php index 438cb9e2eea..1a03f5ab725 100644 --- a/apps/webhook_listeners/lib/BackgroundJobs/WebhookCall.php +++ b/apps/webhook_listeners/lib/BackgroundJobs/WebhookCall.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; - } } diff --git a/apps/webhook_listeners/lib/Controller/WebhooksController.php b/apps/webhook_listeners/lib/Controller/WebhooksController.php index 7956085d4ba..a6c02933bdb 100644 --- a/apps/webhook_listeners/lib/Controller/WebhooksController.php +++ b/apps/webhook_listeners/lib/Controller/WebhooksController.php @@ -113,7 +113,7 @@ class WebhooksController extends OCSController { * @param "none"|"header"|null $authMethod Authentication method to use * @param ?array $authData Array of data for authentication * @param ?array $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 @@ -186,7 +186,7 @@ class WebhooksController extends OCSController { * @param "none"|"header"|null $authMethod Authentication method to use * @param ?array $authData Array of data for authentication * @param ?array $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 diff --git a/apps/webhook_listeners/lib/Db/WebhookListener.php b/apps/webhook_listeners/lib/Db/WebhookListener.php index 023a27db8ea..e628e9458b2 100644 --- a/apps/webhook_listeners/lib/Db/WebhookListener.php +++ b/apps/webhook_listeners/lib/Db/WebhookListener.php @@ -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); - } } diff --git a/apps/webhook_listeners/lib/Service/TokenService.php b/apps/webhook_listeners/lib/Service/TokenService.php new file mode 100644 index 00000000000..32eb0d7c454 --- /dev/null +++ b/apps/webhook_listeners/lib/Service/TokenService.php @@ -0,0 +1,83 @@ + ['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); + } +}