Skip to content

Commit

Permalink
fix(signed-request): removing unstable from public
Browse files Browse the repository at this point in the history
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
  • Loading branch information
ArtificialOwl committed Dec 2, 2024
1 parent 5ac1e9a commit ff175a1
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 71 deletions.
21 changes: 4 additions & 17 deletions lib/private/OCM/OCMSignatoryManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -144,26 +144,13 @@ private function generateKeyId(): string {
*/
public function getRemoteSignatory(string $remote): ?Signatory {
try {
return $this->getRemoteSignatoryFromHost($remote);
$ocmProvider = $this->ocmDiscoveryService->discover($remote, true);
$signatory = $ocmProvider->getSignatory();

Check failure on line 148 in lib/private/OCM/OCMSignatoryManager.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

UndefinedInterfaceMethod

lib/private/OCM/OCMSignatoryManager.php:148:31: UndefinedInterfaceMethod: Method OCP\OCM\IOCMProvider::getSignatory does not exist (see https://psalm.dev/181)
$signatory?->setSignatoryType(SignatoryType::TRUSTED);
return $signatory;
} catch (OCMProviderException $e) {
$this->logger->warning('fail to get remote signatory', ['exception' => $e, 'remote' => $remote]);
return null;
}
}

/**
* As host is enough to generate signatory using OCMDiscoveryService
*
* @param string $host
*
* @return Signatory|null
* @throws OCMProviderException on fail to discover ocm services
* @since 31.0.0
*/
public function getRemoteSignatoryFromHost(string $host): ?Signatory {
$ocmProvider = $this->ocmDiscoveryService->discover($host, true);
$signatory = $ocmProvider->getSignatory();
$signatory?->setSignatoryType(SignatoryType::TRUSTED);
return $signatory;
}
}
51 changes: 20 additions & 31 deletions lib/private/Security/Signature/Model/IncomingSignedRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@ public function __construct(
$this->verifyHeaders();
$this->extractSignatureHeader();
$this->reconstructSignatureData();

try {
// we set origin based on the keyId defined in the Signature header of the request
$this->setOrigin(Signatory::extractIdentityFromUri($this->getSigningElement('keyId')));
} catch (IdentityNotFoundException $e) {
throw new IncomingRequestException($e->getMessage());
}
}

/**
Expand All @@ -66,21 +73,22 @@ public function __construct(
* @throws SignatureNotFoundException
*/
private function verifyHeaders(): void {
if ($this->request->getHeader('Signature') === '') {
throw new SignatureNotFoundException('missing Signature in header');
}

// confirm presence of date, content-length, digest and Signature
$date = $this->getRequest()->getHeader('date');
$date = $this->request->getHeader('date');
if ($date === '') {
throw new SignatureNotFoundException('missing date in header');
throw new IncomingRequestException('missing date in header');
}
$contentLength = $this->getRequest()->getHeader('content-length');
$contentLength = $this->request->getHeader('content-length');
if ($contentLength === '') {
throw new SignatureNotFoundException('missing content-length in header');
throw new IncomingRequestException('missing content-length in header');
}
$digest = $this->getRequest()->getHeader('digest');
$digest = $this->request->getHeader('digest');
if ($digest === '') {
throw new SignatureNotFoundException('missing digest in header');
}
if ($this->getRequest()->getHeader('Signature') === '') {
throw new SignatureNotFoundException('missing Signature in header');
throw new IncomingRequestException('missing digest in header');
}

// confirm date
Expand Down Expand Up @@ -113,7 +121,7 @@ private function verifyHeaders(): void {
*/
private function extractSignatureHeader(): void {
$details = [];
foreach (explode(',', $this->getRequest()->getHeader('Signature')) as $entry) {
foreach (explode(',', $this->request->getHeader('Signature')) as $entry) {
if ($entry === '' || !strpos($entry, '=')) {
continue;
}
Expand All @@ -139,6 +147,8 @@ private function extractSignatureHeader(): void {
}

/**
* reconstruct signature data based on signature's metadata stored in the 'Signature' header
*
* @throws SignatureException
* @throws SignatureElementNotFoundException
*/
Expand Down Expand Up @@ -178,27 +188,6 @@ public function getRequest(): IRequest {
return $this->request;
}

/**
* @inheritDoc
*
* @param Signatory $signatory
*
* @return $this
* @throws IdentityNotFoundException
* @throws IncomingRequestException
* @throws SignatoryException
* @since 31.0.0
*/
public function setSignatory(Signatory $signatory): self {
$identity = \OCP\Server::get(ISignatureManager::class)->extractIdentityFromUri($signatory->getKeyId());
if ($identity !== $this->getOrigin()) {
throw new SignatoryException('keyId from provider is different from the one from signed request');
}

parent::setSignatory($signatory);
return $this;
}

/**
* @inheritDoc
*
Expand Down
6 changes: 0 additions & 6 deletions lib/private/Security/Signature/SignatureManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,6 @@ public function getIncomingSignedRequest(

// generate IncomingSignedRequest based on body and request
$signedRequest = new IncomingSignedRequest($body, $this->request, $options);
try {
// we set origin based on the keyId defined in the Signature header of the request
$signedRequest->setOrigin($this->extractIdentityFromUri($signedRequest->getSigningElement('keyId')));
} catch (IdentityNotFoundException $e) {
throw new IncomingRequestException($e->getMessage());
}

try {
// confirm the validity of content and identity of the incoming request
Expand Down
31 changes: 15 additions & 16 deletions lib/public/OCM/IOCMProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
namespace OCP\OCM;

use JsonSerializable;
use NCU\Security\Signature\Model\Signatory;
use OCP\OCM\Exceptions\OCMArgumentException;
use OCP\OCM\Exceptions\OCMProviderException;

Expand Down Expand Up @@ -120,21 +119,21 @@ public function getResourceTypes(): array;
*/
public function extractProtocolEntry(string $resourceName, string $protocol): string;

/**
* store signatory (public/private key pair) to sign outgoing/incoming request
*
* @param Signatory $signatory
* @since 31.0.0
*/
public function setSignatory(Signatory $signatory): void;

/**
* signatory (public/private key pair) used to sign outgoing/incoming request
*
* @return Signatory|null returns null if no Signatory available
* @since 31.0.0
*/
public function getSignatory(): ?Signatory;
// /**
// * store signatory (public/private key pair) to sign outgoing/incoming request
// *
// * @param Signatory $signatory
// * @experimental 31.0.0
// */
// public function setSignatory(Signatory $signatory): void;

// /**
// * signatory (public/private key pair) used to sign outgoing/incoming request
// *
// * @return Signatory|null returns null if no Signatory available
// * @experimental 31.0.0
// */
// public function getSignatory(): ?Signatory;

/**
* import data from an array
Expand Down
2 changes: 1 addition & 1 deletion lib/unstable/Security/Signature/Model/Signatory.php
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ public function jsonSerialize(): array {
*
* @return string
* @throws IdentityNotFoundException if identity cannot be extracted
* @since 31.0.0
* @experimental 31.0.0
*/
public static function extractIdentityFromUri(string $uri): string {
$identity = parse_url($uri, PHP_URL_HOST);
Expand Down

0 comments on commit ff175a1

Please sign in to comment.