Skip to content

Commit 8572f9a

Browse files
janepiejanepie
committed
fix: Apply suggestions from code review
Signed-off-by: Jana Peper <[email protected]>
1 parent b58d620 commit 8572f9a

File tree

6 files changed

+91
-56
lines changed

6 files changed

+91
-56
lines changed

‎apps/webhook_listeners/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
'OCA\\WebhookListeners\\Migration\\Version1500Date20251007130000' => $baseDir . '/../lib/Migration/Version1500Date20251007130000.php',
2121
'OCA\\WebhookListeners\\ResponseDefinitions' => $baseDir . '/../lib/ResponseDefinitions.php',
2222
'OCA\\WebhookListeners\\Service\\PHPMongoQuery' => $baseDir . '/../lib/Service/PHPMongoQuery.php',
23+
'OCA\\WebhookListeners\\Service\\TokenService' => $baseDir . '/../lib/Service/TokenService.php',
2324
'OCA\\WebhookListeners\\Settings\\Admin' => $baseDir . '/../lib/Settings/Admin.php',
2425
'OCA\\WebhookListeners\\Settings\\AdminSection' => $baseDir . '/../lib/Settings/AdminSection.php',
2526
);

‎apps/webhook_listeners/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ class ComposerStaticInitWebhookListeners
3535
'OCA\\WebhookListeners\\Migration\\Version1500Date20251007130000' => __DIR__ . '/..' . '/../lib/Migration/Version1500Date20251007130000.php',
3636
'OCA\\WebhookListeners\\ResponseDefinitions' => __DIR__ . '/..' . '/../lib/ResponseDefinitions.php',
3737
'OCA\\WebhookListeners\\Service\\PHPMongoQuery' => __DIR__ . '/..' . '/../lib/Service/PHPMongoQuery.php',
38+
'OCA\\WebhookListeners\\Service\\TokenService' => __DIR__ . '/..' . '/../lib/Service/TokenService.php',
3839
'OCA\\WebhookListeners\\Settings\\Admin' => __DIR__ . '/..' . '/../lib/Settings/Admin.php',
3940
'OCA\\WebhookListeners\\Settings\\AdminSection' => __DIR__ . '/..' . '/../lib/Settings/AdminSection.php',
4041
);

‎apps/webhook_listeners/lib/BackgroundJobs/WebhookCall.php‎

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use OCA\AppAPI\PublicFunctions;
1313
use OCA\WebhookListeners\Db\AuthMethod;
1414
use OCA\WebhookListeners\Db\WebhookListenerMapper;
15+
use OCA\WebhookListeners\Service\TokenService;
1516
use OCP\App\IAppManager;
1617
use OCP\AppFramework\Utility\ITimeFactory;
1718
use OCP\BackgroundJob\QueuedJob;
@@ -30,6 +31,7 @@ public function __construct(
3031
private WebhookListenerMapper $mapper,
3132
private LoggerInterface $logger,
3233
private IAppManager $appManager,
34+
private TokenService $tokenService,
3335
ITimeFactory $timeFactory,
3436
) {
3537
parent::__construct($timeFactory);
@@ -44,7 +46,7 @@ protected function run($argument): void {
4446
$client = $this->clientService->newClient();
4547

4648
// adding temporary auth tokens to the call
47-
$data['tokens'] = $this->getTokens($webhookListener, $data['user']['uid']);
49+
$data['tokens'] = $this->tokenService->getTokens($webhookListener, $data['user']['uid']);
4850
$options = [
4951
'verify' => $this->certificateManager->getAbsoluteBundlePath(),
5052
'headers' => $webhookListener->getHeaders() ?? [],
@@ -96,30 +98,5 @@ protected function run($argument): void {
9698
}
9799
}
98100

99-
private function getTokens($webhookListener, $triggerUserId): array {
100-
$tokens = [];
101-
$tokenNeeded = $webhookListener->getTokenNeeded();
102-
if (isset($tokenNeeded['users'])) {
103-
foreach ($tokenNeeded['users'] as $userId) {
104-
$tokens['users'][$userId] = $webhookListener->createTemporaryToken($userId);
105-
}
106-
}
107-
if (isset($tokenNeeded['users'])) {
108-
foreach ($tokenNeeded['functions'] as $function) {
109-
switch ($function) {
110-
case 'owner':
111-
// token for the person who created the flow
112-
$functionId = $webhookListener->getUserId();
113-
$tokens['functions']['owner'][$functionId] = $webhookListener->createTemporaryToken($functionId);
114-
break;
115-
case 'trigger':
116-
// token for the person who triggered the webhook
117-
$tokens['functions']['trigger'][$triggerUserId] = $webhookListener->createTemporaryToken($triggerUserId);
118-
break;
119-
}
120-
}
121-
}
122101

123-
return $tokens;
124-
}
125102
}

‎apps/webhook_listeners/lib/Controller/WebhooksController.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ public function show(int $id): DataResponse {
113113
* @param "none"|"header"|null $authMethod Authentication method to use
114114
* @param ?array<string,mixed> $authData Array of data for authentication
115115
* @param ?array<string,mixed> $tokenNeeded List of user ids for which to include auth tokens in the event.
116-
* Has to fields: "users" lists uids of users for which tokens are needed, "functions" lists functions for which tokens can be included.
116+
* Has two fields: "users" list of user uids for which tokens are needed, "functions" list of functions for which tokens can be included.
117117
* Possible functions: "owner" for the user creating the webhook, "trigger" for the user triggering the webhook call
118118
*
119119
* @return DataResponse<Http::STATUS_OK, WebhookListenersWebhookInfo, array{}>
@@ -186,7 +186,7 @@ public function create(
186186
* @param "none"|"header"|null $authMethod Authentication method to use
187187
* @param ?array<string,mixed> $authData Array of data for authentication
188188
* @param ?array<string,mixed> $tokenNeeded List of user ids for which to include auth tokens in the event.
189-
* Has to fields: "users" lists uids of users for which tokens are needed, "functions" lists functions for which tokens can be included.
189+
* Has two fields: "users" list of user uids for which tokens are needed, "functions" list of functions for which tokens can be included.
190190
* Possible functions: "owner" for the user creating the webhook, "trigger" for the user triggering the webhook call
191191
*
192192
* @return DataResponse<Http::STATUS_OK, WebhookListenersWebhookInfo, array{}>

‎apps/webhook_listeners/lib/Db/WebhookListener.php‎

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,8 @@
99

1010
namespace OCA\WebhookListeners\Db;
1111

12-
use OC\Authentication\Token\IProvider;
1312
use OCP\AppFramework\Db\Entity;
14-
use OCP\Authentication\Token\IToken;
1513
use OCP\Security\ICrypto;
16-
use OCP\Security\ISecureRandom;
1714
use OCP\Server;
1815

1916
/**
@@ -96,24 +93,14 @@ class WebhookListener extends Entity implements \JsonSerializable {
9693

9794
private ICrypto $crypto;
9895

99-
private IProvider $tokenProvider;
96+
10097
public function __construct(
10198
?ICrypto $crypto = null,
102-
?IProvider $tokenProvider = null,
103-
private ?ISecureRandom $random = null,
10499
) {
105100
if ($crypto === null) {
106101
$crypto = Server::get(ICrypto::class);
107102
}
108103
$this->crypto = $crypto;
109-
if ($tokenProvider === null) {
110-
$tokenProvider = Server::get(IProvider::class);
111-
}
112-
$this->tokenProvider = $tokenProvider;
113-
if ($random === null) {
114-
$random = Server::get(ISecureRandom::class);
115-
}
116-
$this->random = $random;
117104
$this->addType('appId', 'string');
118105
$this->addType('userId', 'string');
119106
$this->addType('httpMethod', 'string');
@@ -169,19 +156,5 @@ public function getAppId(): ?string {
169156
}
170157

171158

172-
public function createTemporaryToken($userId) {
173-
$token = $this->generateRandomDeviceToken();
174-
$name = 'Authentication for Webhook';
175-
$password = null;
176-
$deviceToken = $this->tokenProvider->generateToken($token, $userId, $userId, $password, $name, IToken::PERMANENT_TOKEN);
177-
return $token;
178-
}
179159

180-
private function generateRandomDeviceToken() {
181-
$groups = [];
182-
for ($i = 0; $i < 5; $i++) {
183-
$groups[] = $this->random->generate(5, ISecureRandom::CHAR_HUMAN_READABLE);
184-
}
185-
return implode('-', $groups);
186-
}
187160
}

‎apps/webhook_listeners/lib/Service/TokenService.php‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
<?php
2+
3+
/**
4+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-License-Identifier: AGPL-3.0-or-later
6+
*/
7+
8+
namespace OCA\WebhookListeners\Service;
9+
10+
use OC\Authentication\Token\IProvider;
11+
use OCA\WebhookListeners\Db\WebhookListener;
12+
use OCP\Authentication\Token\IToken;
13+
use OCP\Security\ISecureRandom;
14+
15+
class TokenService {
16+
public function __construct(
17+
private IProvider $tokenProvider,
18+
private ISecureRandom $random,
19+
) {
20+
}
21+
22+
23+
/**
24+
* creates an array which includes two arrays of tokens: 'users' and 'functions'
25+
* The array ['users' => ['jane', 'bob'], 'functions' => ['owner', 'trigger']]
26+
* as requested tokens in the registered webhook produces a result like
27+
* ['users' => [['jane' => 'abcdtokenabcd1'], ['bob','=> 'abcdtokenabcd2']], 'functions' => [['owner' => ['admin' => 'abcdtokenabcd3']], ['trigger' => ['user1' => 'abcdtokenabcd4']]]]
28+
*
29+
* @param WebhookListener $webhookListener
30+
* @param string|null $triggerUserId the user that triggered the webhook call
31+
* @return array
32+
*/
33+
public function getTokens(WebhookListener $webhookListener, ?string $triggerUserId): array {
34+
$tokens = [
35+
'users' => [],
36+
'functions' => [],
37+
];
38+
$tokenNeeded = $webhookListener->getTokenNeeded();
39+
if (isset($tokenNeeded['users'])) {
40+
foreach ($tokenNeeded['users'] as $userId) {
41+
$tokens['users'][$userId] = $webhookListener->createTemporaryToken($userId);
42+
}
43+
}
44+
if (isset($tokenNeeded['users'])) {
45+
foreach ($tokenNeeded['functions'] as $function) {
46+
switch ($function) {
47+
case 'owner':
48+
// token for the person who created the flow
49+
$functionId = $webhookListener->getUserId();
50+
$tokens['functions']['owner'] = [
51+
$functionId => $webhookListener->createTemporaryToken($functionId)
52+
];
53+
break;
54+
case 'trigger':
55+
// token for the person who triggered the webhook
56+
$tokens['functions']['trigger'] = [
57+
$triggerUserId => $webhookListener->createTemporaryToken($triggerUserId)
58+
];
59+
break;
60+
}
61+
}
62+
}
63+
64+
return $tokens;
65+
}
66+
67+
68+
public function createTemporaryToken(string $userId): string {
69+
$token = $this->generateRandomDeviceToken();
70+
$name = 'Ephemeral webhook authentication';
71+
$password = null;
72+
$deviceToken = $this->tokenProvider->generateToken($token, $userId, $userId, $password, $name, IToken::PERMANENT_TOKEN);
73+
return $token;
74+
}
75+
76+
private function generateRandomDeviceToken(): string {
77+
$groups = [];
78+
for ($i = 0; $i < 5; $i++) {
79+
$groups[] = $this->random->generate(5, ISecureRandom::CHAR_HUMAN_READABLE);
80+
}
81+
return implode('-', $groups);
82+
}
83+
}

0 commit comments

Comments
 (0)