Skip to content

Commit

Permalink
fix(signed-url): add non-standard http ports to final url if passed
Browse files Browse the repository at this point in the history 
Fix: #162
  • Loading branch information
@calvinalkan
calvinalkan committed Dec 6, 2022
1 parent c8ee18e commit 1368c80
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 64 deletions.
136 changes: 72 additions & 64 deletions src/Snicco/Component/signed-url/src/UrlSigner.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,21 @@

final class UrlSigner
{

private SignedUrlStorage $storage;

private HMAC $hasher;

public function __construct(SignedUrlStorage $storage, HMAC $hasher)
{
$this->storage = $storage;
$this->hasher = $hasher;
}

/**
* @param non-empty-string $target
* @param positive-int $lifetime_in_sec
* @param positive-int $max_usage
* @param non-empty-string $target
* @param positive-int $lifetime_in_sec
* @param positive-int $max_usage
*
* @throws Exception if random_bytes can't be generated
* @throws UnavailableStorage
Expand All @@ -45,150 +46,157 @@ public function sign(
int $lifetime_in_sec,
int $max_usage = 1,
string $request_context = ''
): SignedUrl {
) :SignedUrl {
Assert::notEmpty($target);

$target = $this->normalizeTarget($target);

$parts = $this->parseUrl($target);

$path = $this->getPath($parts);
$query = $this->getQueryString($parts);
$expires_at = $this->getExpiryTimestamp($lifetime_in_sec);

$path_with_query = $query ? ($path . '?' . $query) : $path;

$path_with_query = $query ? ($path.'?'.$query) : $path;
// We create a random identifier for storing the signed url usage limit.
// We don't store the signature or a hash of it.
$identifier = Base64UrlSafe::encode(random_bytes(16));

// The signature consists of the identifier, the request context the developer passed
// (such as ip address or user agent) and the protected path with an expires_at query parameter.
$plain_text_signature =
$identifier .
$request_context .
$identifier.
$request_context.
$this->appendExpiryQueryParam($path_with_query, $expires_at);

$signature = Base64UrlSafe::encode($this->hasher->create($plain_text_signature));

// We append the expires_at and signature timestamp to the path, so that it can be easily validated.
// If any of the plain text parts have been tampered validation wil fail.
$path_with_query =
$this->appendExpiryQueryParam($path_with_query, $expires_at)
. '&'
. SignedUrl::SIGNATURE_KEY
. '='
. $identifier
. $signature;

$url = ($domain_and_scheme = $this->getDomainAndSchema($parts))
? $domain_and_scheme . $path_with_query
.'&'
.SignedUrl::SIGNATURE_KEY
.'='
.$identifier
.$signature;
$url = ($domain_and_scheme = $this->getHostAndScheme($parts))
? $domain_and_scheme.$path_with_query
: $path_with_query;

$signed_url = SignedUrl::create($url, $target, $identifier, $expires_at, $max_usage);

$this->storage->store($signed_url);

return $signed_url;
}

private function normalizeTarget(string $protect): string
private function normalizeTarget(string $protect) :string
{
if (0 === strpos($protect, 'http')) {
return $protect;
}

return '/' . ltrim($protect, '/');
return '/'.ltrim($protect, '/');
}

private function parseUrl(string $protect): array
private function parseUrl(string $protect) :array
{
$parts = ($parts = parse_url($protect)) ? $parts : [];

if (! isset($parts['path']) && isset($parts['host'], $parts['scheme'])) {
if ( ! isset($parts['path']) && isset($parts['host'], $parts['scheme'])) {
$parts['path'] = '/';
}

$this->validateUrlParts($parts, $protect);

return $parts;
}

private function validateUrlParts(array $parsed, string $target): void
private function validateUrlParts(array $parsed, string $target) :void
{
if ('/' === $parsed['path'] && '/' !== $target && 0 !== strpos($target, 'http')) {
throw new InvalidArgumentException(sprintf('%s is not a valid path.', $target));
}

/** @var string $qs */
$qs = $parsed['query'] ?? '';

parse_str($qs, $query);

if (isset($query[SignedUrl::EXPIRE_KEY])) {
throw new LogicException(
"The expires query parameter is reserved.\nPlease rename your query parameter."
);
}

if (isset($query[SignedUrl::SIGNATURE_KEY])) {
throw new LogicException(
"The signature query parameter is reserved.\nPlease rename your query parameter."
);
}
}

/**
* @psalm-suppress MixedReturnStatement
* @psalm-suppress MixedInferredReturnType
*/
private function getPath(array $parts): string
private function getPath(array $parts) :string
{
if (isset($parts['path'])) {
return $parts['path'];
}

// Should not be possible ever.
// @codeCoverageIgnoreStart
throw new InvalidArgumentException('Invalid path provided.');
// @codeCoverageIgnoreEnd
}

/**
* @psalm-suppress MixedArgument
*/
private function getQueryString(array $parts): ?string
private function getQueryString(array $parts) :?string
{
if (! isset($parts['query'])) {
if ( ! isset($parts['query'])) {
return null;
}

return rtrim($parts['query'], '&');
}

private function getExpiryTimestamp(int $lifetime_in_sec): int
private function getExpiryTimestamp(int $lifetime_in_sec) :int
{
return time() + $lifetime_in_sec;
}

private function appendExpiryQueryParam(string $path, int $expires_at): string
private function appendExpiryQueryParam(string $path, int $expires_at) :string
{
if (! strpos($path, '?')) {
return $path . '?' . SignedUrl::EXPIRE_KEY . '=' . (string) $expires_at;
if ( ! strpos($path, '?')) {
return $path.'?'.SignedUrl::EXPIRE_KEY.'='.(string)$expires_at;
}

return rtrim($path, '&') . '&' . SignedUrl::EXPIRE_KEY . '=' . (string) $expires_at;
return rtrim($path, '&').'&'.SignedUrl::EXPIRE_KEY.'='.(string)$expires_at;
}

/**
* @psalm-suppress MixedOperand
*/
private function getDomainAndSchema(array $parts): ?string
private function getHostAndScheme(array $parts) :?string
{
if (isset($parts['host'], $parts['scheme'])) {
return $parts['scheme'] . '://' . $parts['host'];
$scheme_and_domain = $parts['scheme'].'://'.$parts['host'];

if (isset($parts['port'])) {
$scheme_and_domain .= ':'.$parts['port'];
}

return $scheme_and_domain;
}

return null;
}

}
14 changes: 14 additions & 0 deletions src/Snicco/Component/signed-url/tests/UrlSignerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,20 @@ public function a_signed_url_can_be_created_for_a_full_url(): void
$this->assertStringContainsString('signature=', $magic_link->asString());
$this->assertSame('https://foo.com/foo/', $magic_link->protects());
}

/**
* @test
*/
public function a_signed_url_can_be_created_with_non_standard_ports(): void
{
$magic_link = $this->url_signer->sign('https://foo.com:8443/foo/', 10);
$this->assertInstanceOf(SignedUrl::class, $magic_link);

$this->assertStringStartsWith('https://foo.com:8443/foo/', $magic_link->asString());
$this->assertStringContainsString('expires=', $magic_link->asString());
$this->assertStringContainsString('signature=', $magic_link->asString());
$this->assertSame('https://foo.com:8443/foo/', $magic_link->protects());
}

/**
* @test
Expand Down

0 comments on commit 1368c80

Please sign in to comment.