Skip to content

Commit

Permalink
Added webhook secrets
Browse files Browse the repository at this point in the history
  • Loading branch information
vtsykun committed Jan 12, 2023
1 parent bc05dd8 commit 751248e
Show file tree
Hide file tree
Showing 7 changed files with 205 additions and 55 deletions.
78 changes: 57 additions & 21 deletions src/Form/Type/WebhookType.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Packeton\Form\Type;

use Cron\CronExpression;
use Packeton\DBAL\OpensslCrypter;
use Packeton\Entity\User;
use Packeton\Entity\Webhook;
use Packeton\Validator\Constraint\ValidRegex;
Expand All @@ -25,25 +26,11 @@

class WebhookType extends AbstractType
{
/**
* @var TokenStorageInterface
*/
private $tokenStorage;

/**
* @var PayloadRenderer
*/
private $renderer;

/**
* @param TokenStorageInterface $tokenStorage
* @param PayloadRenderer $renderer
*/
public function __construct(TokenStorageInterface $tokenStorage, PayloadRenderer $renderer)
{
$this->tokenStorage = $tokenStorage;
$this->renderer = $renderer;
}
public function __construct(
private readonly TokenStorageInterface $tokenStorage,
private readonly PayloadRenderer $renderer,
private readonly OpensslCrypter $crypter,
) {}

/**
* {@inheritdoc}
Expand Down Expand Up @@ -123,7 +110,21 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
'required' => false,
]);

$builder->addEventListener(FormEvents::POST_SET_DATA, [$this, 'onSetData']);
$builder->addEventListener(FormEvents::POST_SET_DATA, $this->onSetData(...));
$builder->addEventListener(FormEvents::POST_SUBMIT, $this->postSubmit(...));
}

public function postSubmit(FormEvent $event): void
{
$data = $event->getData();
if (!$data instanceof Webhook || empty($secrets = $data->getOptions()['secrets'] ?? null)) {
return;
}

if (is_array($secrets)) {
$secrets = $this->crypter->encryptData(json_encode($secrets));
$data->setOptions(array_merge($data->getOptions(), ['secrets' => $secrets]));
}
}

/**
Expand Down Expand Up @@ -157,6 +158,7 @@ public function configureOptions(OptionsResolver $resolver): void
{
$resolver->setDefaults([
'data_class' => Webhook::class,
'constraints' => [new Callback([$this, 'validateWebhook'])],
]);
}

Expand All @@ -178,6 +180,29 @@ public function checkPayload($value, ExecutionContextInterface $context): void
}
}

public function validateWebhook($value, ExecutionContextInterface $context)
{
if (!$value instanceof Webhook) {
return;
}

if ($options = $value->getOptions() and $diff = array_diff(array_keys($options), $this->allowedClientOptions())) {
$context->addViolation(sprintf('This options is not allowed. "%s"', implode(",", $diff)));
}

if (!$hostname = parse_url($value->getUrl(), PHP_URL_HOST)) {
$context->addViolation("Hostname can not be is empty.");
}

if (filter_var($hostname, FILTER_VALIDATE_IP)) {
$flag = \FILTER_FLAG_IPV4 | \FILTER_FLAG_NO_PRIV_RANGE | \FILTER_FLAG_NO_RES_RANGE;
if (!filter_var($hostname, FILTER_VALIDATE_IP, $flag)) {
$context->addViolation("This is not a valid IP address $hostname");
}
}
}


/**
* @param string|null $value
* @param ExecutionContextInterface $context
Expand All @@ -204,12 +229,23 @@ public static function getEventsChoices(): array
'Update tag/branch' => Webhook::HOOK_PUSH_UPDATE,
'Created a new repository' => Webhook::HOOK_REPO_NEW,
'Remove repository' => Webhook::HOOK_REPO_DELETE,
'By HTTP requests to https://APP_URL/api/webhook-invoke/{name}' => Webhook::HOOK_HTTP_REQUEST,
'By HTTP requests to https://APP_URL/api/webhook-invoke/{name}' => Webhook::HOOK_HTTP_REQUEST,
'User login event' => Webhook::HOOK_USER_LOGIN,
'By cron' => Webhook::HOOK_CRON,
];
}

/**
* Get allowed options for client https://symfony.com/doc/current/http_client.html
*/
protected function allowedClientOptions(): array
{
return [
'headers', 'verify_peer', 'verify_host', 'secrets',
'auth_ntlm', 'auth_basic', 'auth_bearer',
];
}

/**
* @return object|null|User
*/
Expand Down
11 changes: 3 additions & 8 deletions src/Webhook/HookRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,12 @@ public function getBody()
return $this->body;
}

/**
* @return array
*/
public function getHeaders(): array
public function getHeaders(array $options = []): array
{
$headers = $this->options['headers'] ?? [];
$headers = $options ? ($options['headers'] ?? []) : ($this->options['headers'] ?? []);
$headers = is_array($headers) ? $headers : [];

return array_filter($headers, function ($v) {
return is_scalar($v);
});
return array_filter($headers, fn ($v) => is_scalar($v));
}

/**
Expand Down
125 changes: 108 additions & 17 deletions src/Webhook/HookRequestExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,25 @@

namespace Packeton\Webhook;

use Packeton\DBAL\OpensslCrypter;
use Packeton\Entity\Webhook;
use Packeton\Webhook\Twig\ContextAwareInterface;
use Packeton\Webhook\Twig\InterruptException;
use Packeton\Webhook\Twig\WebhookContext;
use Psr\Log\LoggerAwareInterface;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpClient\HttpClient;
use Symfony\Component\HttpClient\CurlHttpClient;
use Symfony\Component\HttpClient\NoPrivateNetworkHttpClient;
use Symfony\Contracts\HttpClient\HttpClientInterface;

class HookRequestExecutor implements ContextAwareInterface, LoggerAwareInterface
{
private $requestResolver;
private $logger;
private ?LoggerInterface $logger;

public function __construct(RequestResolver $requestResolver)
{
$this->requestResolver = $requestResolver;
}
public function __construct(
private readonly RequestResolver $requestResolver,
private readonly OpensslCrypter $crypter,
) {}

/**
* @param Webhook $webhook
Expand All @@ -33,7 +34,7 @@ public function executeWebhook(Webhook $webhook, array $variables = [], HttpClie
{
$variables['webhook'] = $webhook;
if (null === $client) {
$client = HttpClient::create(['max_duration' => 60]);
$client = new NoPrivateNetworkHttpClient(new CurlHttpClient(['max_duration' => 60]));
}

$maxAttempt = 3;
Expand All @@ -48,34 +49,53 @@ public function executeWebhook(Webhook $webhook, array $variables = [], HttpClie
$requests = array_slice($requests, 0, 50);
foreach ($requests as $request) {
$options = $request->getOptions();

$secrets = $options['secrets'] ?? null;
unset($options['secrets']);

if ($body = $request->getBody()) {
$options['body'] = $request->getBody();
try {
if ($json = @json_decode($body, true)) {
if (is_array($json = @json_decode($body, true))) {
$options['json'] = $json;
unset($options['body']);
}
} catch (\Throwable $e) {}
} catch (\Throwable) {}
}

try {
$response = $client->request($request->getMethod(), $request->getUrl(), $options);
if (is_string($secrets)) {
if (!$this->crypter->isEncryptData($secrets)) {
throw new \InvalidArgumentException('Invalid decrypt secrets parameter.');
}

$secrets = $this->crypter->decryptData($secrets);
$secrets = $secrets ? json_decode($secrets, true) : null;
if (empty($secrets) || !is_array($secrets)) {
throw new \InvalidArgumentException('Unable to decrypt secrets, probably enc key was changed, please check you configuration.');
}
}

$cloneRequestOpts = $options;
[$url, $options] = $secrets ? $this->processSecrets($secrets, $request->getUrl(), $options) : [$request->getUrl(), $options];

$response = $client->request($request->getMethod(), $url, $options);
$headers = array_map(fn ($item) => $item[0] ?? $item, $response->getHeaders(false));
$options = [
'status_code' => $response->getStatusCode(),
'headers' => $headers,
];
if ($info = $response->getInfo()) {
$options['request_headers'] = $this->getRequestHeaders($info, $request->getHeaders());
$options['request_headers'] = $this->getRequestHeaders($info, $request->getHeaders($cloneRequestOpts));
$options = array_merge($info, $options);
}

$responses[] = new HookResponse(
$request,
substr($response->getContent(false), 0, 8192), // Save only first 8k bytes to database
$options
);
[$content, $options] = $this->hideSensitiveParameters(substr($response->getContent(false), 0, 8192), $options, $secrets);
$request = $this->hideSensitiveRequest($request, $secrets);

$responses[] = new HookResponse($request, $content, $options);
} catch (\Exception $exception) {
$request = $this->hideSensitiveRequest($request, $secrets);
$responses[] = $this->createFailsResponse($webhook, $exception, $request);
$maxAttempt--;
}
Expand All @@ -88,6 +108,77 @@ public function executeWebhook(Webhook $webhook, array $variables = [], HttpClie
return $responses;
}

private function processSecrets(array $secrets, string $url, array $options): array
{
if (!isset($secrets['allowed-domains'])) {
$this->logger?->warning('Using secrets without restriction "allowed-domains" is not secure. Please set allowed domains parameters for secrets options');
} else {
$domains = (array)$secrets['allowed-domains'];
$hostname = parse_url($url, PHP_URL_HOST);

if (empty($hostname) || !in_array($hostname, $domains, true)) {
throw new \InvalidArgumentException("This domain $hostname is not allowed, only allowed-domains: " . implode(',', $domains));
}
}

$secretWrapper = function(&$node) use ($secrets) {
if (is_string($node)) {
foreach ($secrets as $name => $secret) {
if (is_string($secret)) {
$name = 'secrets.' . $name;
$node = str_replace('${' . $name . '}', $secret, $node);
$node = str_replace('${ ' . $name . ' }', $secret, $node);
}
}
}
return $node;
};

array_walk_recursive($options, $secretWrapper);
return [$secretWrapper($url), $options];
}

private function hideSensitiveRequest(HookRequest $request, mixed $secrets = null): HookRequest
{
$opts = $this->hideSensitiveParameters(null, $request->jsonSerialize(), $secrets)[1];
if (is_array($headers = $opts['options']['headers'] ?? null)) {
foreach ($headers as $name => $value) {
if (str_contains(mb_strtolower($name), 'authorization')) {
$headers[$name] = '***';
}
}
$opts['options']['headers'] = $headers;
}

return HookRequest::fromArray($opts);
}

private function hideSensitiveParameters(mixed $content, array $options, mixed $secrets = null): array
{
$secretHider = function(&$content) use ($secrets) {
if (is_string($content)) {
foreach ($secrets as $value) {
$content = is_string($value) ? str_replace($value, '***', $content) : $content;
}
}
return $content;
};

if (is_array($secrets)) {
$content = $secretHider($content);
array_walk_recursive($options, $secretHider);
}

if (isset($options['request_headers']['Authorization'])) {
$options['request_headers']['Authorization'] = '***';
}
if (isset($options['request_headers']['authorization'])) {
$options['request_headers']['authorization'] = '***';
}

return [$content, $options];
}

private function getRequestHeaders($info, array $baseHeaders = [])
{
if (!isset($info['debug']) || !is_string($info['debug'])) {
Expand Down
8 changes: 2 additions & 6 deletions src/Webhook/HookResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,7 @@ public function getHeaders(): array
$headers = $this->options['headers'] ?? [];
$headers = is_array($headers) ? $headers : [];

return array_filter($headers, function ($v) {
return is_scalar($v);
});
return array_filter($headers, fn ($v) => is_scalar($v));
}

/**
Expand All @@ -108,9 +106,7 @@ public function getRequestHeaders()
$headers = $this->options['request_headers'] ?? [];
$headers = is_array($headers) ? $headers : [];

return array_filter($headers, function ($v) {
return is_scalar($v);
});
return array_filter($headers, fn ($v) => is_scalar($v));
}

return $this->request->getHeaders();
Expand Down
2 changes: 1 addition & 1 deletion src/Webhook/HookTestAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ private function processChildWebhook(Webhook $webhook, array $context, HttpClien

if (isset($runtimeContext[WebhookContext::CHILD_WEBHOOK])) {
/** @var Webhook $childHook */
foreach ($runtimeContext[WebhookContext::CHILD_WEBHOOK] as list($childHook, $childContext)) {
foreach ($runtimeContext[WebhookContext::CHILD_WEBHOOK] as [$childHook, $childContext]) {
if (null !== $childHook->getOwner() && $childHook->getVisibility() === Webhook::USER_VISIBLE && $childHook->getOwner() !== $webhook->getOwner()) {
$response[] = new HookErrorResponse('You can not call private webhooks of another user owner, please check nesting webhook visibility');
continue;
Expand Down
3 changes: 2 additions & 1 deletion src/Webhook/Twig/WebhookExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Psr\Log\LoggerAwareTrait;
use Psr\Log\NullLogger;
use Symfony\Component\HttpClient\HttpClient;
use Symfony\Component\HttpClient\NoPrivateNetworkHttpClient;
use Twig\Extension\AbstractExtension;
use Twig\TwigFunction;

Expand Down Expand Up @@ -166,7 +167,7 @@ public function hook_function_http_request(string $url, array $options = [])
$isRaw = $options['raw'] ?? false;

unset($options['method'], $options['raw']);
$client = HttpClient::create(['max_duration' => 60]);
$client = new NoPrivateNetworkHttpClient(HttpClient::create(['max_duration' => 60]));

try {
$response = $client->request($method, $url, $options);
Expand Down
Loading

0 comments on commit 751248e

Please sign in to comment.