chore(http-sig): Simplify implementation

Signed-off-by: Micke Nordin <kano@sunet.se>

Author: mickenordin

apps/cloud_federation_api/lib/Controller/OCMRequestController.php

@@ -9,6 +9,7 @@
 
 namespace OCA\CloudFederationAPI\Controller;
 
+use JsonException;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Attribute\BruteForceProtection;
@@ -17,7 +18,6 @@
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\AppFramework\Http\Response;
 use OCP\EventDispatcher\IEventDispatcher;
-use OCP\Federation\ICloudFederationProviderManager;
 use OCP\IRequest;
 use OCP\OCM\Events\OCMEndpointRequestEvent;
 use OCP\OCM\Exceptions\OCMArgumentException;
@@ -31,7 +31,6 @@ public function __construct(
 		IRequest $request,
 		private readonly IEventDispatcher $eventDispatcher,
 		private readonly IOCMDiscoveryService $ocmDiscoveryService,
-		private readonly ICloudFederationProviderManager $cloudFederationProviderManager,
 		private readonly LoggerInterface $logger,
 	) {
 		parent::__construct($appName, $request);
@@ -57,29 +56,33 @@ public function manageOCMRequests(string $ocmPath): Response {
 			throw new OCMArgumentException('path is not UTF-8');
 		}
 
-		// Resolve the signer origin from the payload before verification.
-		$payload = $this->request->getParams();
-		$origin = null;
-		if ($payload !== []) {
-			$identity = $this->cloudFederationProviderManager->resolveSenderIdentity($payload);
-			if ($identity !== null) {
-				try {
-					$origin = $this->ocmDiscoveryService->getHostFromOcmAddress($identity);
-				} catch (IncomingRequestException) {
-					// unresolvable origin; verification will fail without one
-				}
+		$ocmAddress = null;
+		$params = $this->request->getParams();
+		foreach (['owner', 'sender', 'sharedBy'] as $field) {
+			if (is_string($params[$field] ?? null) && $params[$field] !== '') {
+				$ocmAddress = $params[$field];
+				break;
 			}
 		}
 
 		try {
-			$signedRequest = $this->ocmDiscoveryService->getIncomingSignedRequest($origin);
+			$signedRequest = $this->ocmDiscoveryService->getIncomingSignedRequest($ocmAddress);
 		} catch (IncomingRequestException $e) {
 			$this->logger->warning('incoming ocm request exception', ['exception' => $e]);
 			$response = new JSONResponse(['message' => $e->getMessage(), 'validationErrors' => []], Http::STATUS_BAD_REQUEST);
 			$response->throttle();
 			return $response;
 		}
 
+		// assuming that ocm request contains a json array
+		$payload = $signedRequest?->getBody() ?? file_get_contents('php://input');
+		try {
+			$payload = ($payload) ? json_decode($payload, true, 512, JSON_THROW_ON_ERROR) : null;
+		} catch (JsonException $e) {
+			$this->logger->debug('json decode error', ['exception' => $e]);
+			$payload = null;
+		}
+
 		$event = new OCMEndpointRequestEvent(
 			$this->request->getMethod(),
 			preg_replace('@/+@', '/', $ocmPath),

apps/cloud_federation_api/lib/Controller/RequestHandlerController.php

@@ -110,8 +110,7 @@ public function addShare($shareWith, $name, $description, $providerId, $owner, $
 			try {
 				// if request is signed and well signed, no exceptions are thrown
 				// if request is not signed and host is known for not supporting signed request, no exception are thrown
-				$origin = $this->ocmDiscoveryService->getHostFromOcmAddress($owner);
-				$signedRequest = $this->ocmDiscoveryService->getIncomingSignedRequest($origin);
+				$signedRequest = $this->ocmDiscoveryService->getIncomingSignedRequest($owner);
 				$this->confirmSignedOrigin($signedRequest, 'owner', $owner);
 			} catch (IncomingRequestException $e) {
 				$this->logger->warning('incoming request exception', ['exception' => $e]);
@@ -309,11 +308,7 @@ public function receiveNotification($notificationType, $resourceType, $providerI
 		if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) {
 			try {
 				$identity = $this->resolveNotificationIdentity($resourceType, $notification);
-				$origin = null;
-				if ($identity !== '') {
-					$origin = $this->ocmDiscoveryService->getHostFromOcmAddress($identity);
-				}
-				$signedRequest = $this->ocmDiscoveryService->getIncomingSignedRequest($origin);
+				$signedRequest = $this->ocmDiscoveryService->getIncomingSignedRequest($identity !== '' ? $identity : null);
 				if ($identity !== '') {
 					$this->ocmDiscoveryService->confirmRequestOrigin($signedRequest?->getOrigin(), $identity);
 				}

apps/cloud_federation_api/lib/Controller/TokenController.php

@@ -23,17 +23,19 @@
 use OCP\Authentication\Exceptions\ExpiredTokenException;
 use OCP\Authentication\Exceptions\InvalidTokenException;
 use OCP\Authentication\Token\IToken;
+use OCP\Federation\ICloudIdManager;
 use OCP\IAppConfig;
 use OCP\IRequest;
-use OCP\OCM\IOCMDiscoveryService;
 use OCP\Security\ISecureRandom;
+use OCP\Security\Signature\Exceptions\IdentityNotFoundException;
 use OCP\Security\Signature\Exceptions\IncomingRequestException;
 use OCP\Security\Signature\Exceptions\SignatoryNotFoundException;
 use OCP\Security\Signature\Exceptions\SignatureException;
 use OCP\Security\Signature\Exceptions\SignatureNotFoundException;
 use OCP\Security\Signature\IIncomingSignedRequest;
 use OCP\Security\Signature\ISignatureManager;
 use OCP\Security\Signature\Model\Signatory;
+use OCP\Share\Exceptions\ShareNotFound;
 use OCP\Share\IManager as IShareManager;
 use Psr\Log\LoggerInterface;
 
@@ -53,7 +55,7 @@ public function __construct(
 		private readonly IAppConfig $appConfig,
 		private readonly OcmTokenMapMapper $ocmTokenMapMapper,
 		private readonly IShareManager $shareManager,
-		private readonly IOCMDiscoveryService $ocmDiscoveryService,
+		private readonly ICloudIdManager $cloudIdManager,
 	) {
 		parent::__construct('cloud_federation_api', $request);
 	}
@@ -74,16 +76,17 @@ private function resolveOriginFromRefreshToken(string $code): ?string {
 			if ($sharedWith === null || $sharedWith === '') {
 				return null;
 			}
-			return $this->ocmDiscoveryService->getHostFromOcmAddress($sharedWith);
-		} catch (\Throwable) {
+			$remote = $this->cloudIdManager->resolveCloudId($sharedWith)->getRemote();
+			return $this->signatureManager->extractIdentityFromUri($remote);
+		} catch (ShareNotFound|IdentityNotFoundException|\InvalidArgumentException) {
 			return null;
 		}
 	}
 
 	/**
 	 * Verify the signature of incoming request if available
 	 *
-	 * @param string|null $origin the origin of the request, or null if unknown
+	 * @param string|null $origin sender origin, or null if unknown
 	 *
 	 * @return IIncomingSignedRequest|null null if remote does not support signed requests
 	 * @throws IncomingRequestException if signature is required but invalid
@@ -148,9 +151,7 @@ private function resolveJwtSigningKey(string $privateKeyPem): array {
 	public function jwks(): JSONResponse {
 		$keys = [];
 		try {
-			foreach ($this->signatoryManager->getLocalJwks() as $jwk) {
-				$keys[] = $jwk;
-			}
+			$keys = $this->signatoryManager->getLocalJwks();
 		} catch (\Throwable $e) {
 			$this->logger->warning('failed to build local JWKs', ['exception' => $e]);
 		}

apps/cloud_federation_api/tests/Controller/TokenControllerTest.php

@@ -21,9 +21,10 @@
 use OCP\Authentication\Exceptions\ExpiredTokenException;
 use OCP\Authentication\Exceptions\InvalidTokenException;
 use OCP\Authentication\Token\IToken;
+use OCP\Federation\ICloudId;
+use OCP\Federation\ICloudIdManager;
 use OCP\IAppConfig;
 use OCP\IRequest;
-use OCP\OCM\IOCMDiscoveryService;
 use OCP\Security\ISecureRandom;
 use OCP\Security\Signature\Exceptions\SignatoryNotFoundException;
 use OCP\Security\Signature\Exceptions\SignatureException;
@@ -48,7 +49,7 @@ class TokenControllerTest extends TestCase {
 	private IAppConfig&MockObject $appConfig;
 	private OcmTokenMapMapper&MockObject $ocmTokenMapMapper;
 	private IShareManager&MockObject $shareManager;
-	private IOCMDiscoveryService&MockObject $ocmDiscoveryService;
+	private ICloudIdManager&MockObject $cloudIdManager;
 
 	private TokenController $controller;
 
@@ -65,11 +66,13 @@ protected function setUp(): void {
 		$this->timeFactory = $this->createMock(ITimeFactory::class);
 		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->signatureManager = $this->createMock(ISignatureManager::class);
+		$this->signatureManager->method('extractIdentityFromUri')
+			->willReturnCallback(static fn (string $uri): string => (string)parse_url($uri, PHP_URL_HOST));
 		$this->signatoryManager = $this->createMock(OCMSignatoryManager::class);
 		$this->appConfig = $this->createMock(IAppConfig::class);
 		$this->ocmTokenMapMapper = $this->createMock(OcmTokenMapMapper::class);
 		$this->shareManager = $this->createMock(IShareManager::class);
-		$this->ocmDiscoveryService = $this->createMock(IOCMDiscoveryService::class);
+		$this->cloudIdManager = $this->createMock(ICloudIdManager::class);
 
 		$this->controller = new TokenController(
 			$this->request,
@@ -82,7 +85,7 @@ protected function setUp(): void {
 			$this->appConfig,
 			$this->ocmTokenMapMapper,
 			$this->shareManager,
-			$this->ocmDiscoveryService,
+			$this->cloudIdManager,
 		);
 	}
 
@@ -133,6 +136,11 @@ private function configureHappyPath(
 		$this->shareManager->method('getShareByToken')
 			->with($refreshToken)
 			->willReturn($share);
+		$cloudId = $this->createMock(ICloudId::class);
+		$cloudId->method('getRemote')->willReturn('https://remote.example.com');
+		$this->cloudIdManager->method('resolveCloudId')
+			->with($sharedWith)
+			->willReturn($cloudId);
 
 		$signatory = new Signatory();
 		$signatory->setKeyId('https://local.example.com/index.php/ocm#signature');
@@ -153,10 +161,10 @@ public function testAccessTokenSuccess(): void {
 		$signedRequest = $this->createMock(IIncomingSignedRequest::class);
 		$signedRequest->method('getOrigin')->willReturn('remote.example.com');
 		$this->signatureManager->method('getIncomingSignedRequest')
-			->with($this->signatoryManager)
+			->with($this->signatoryManager, null, 'remote.example.com')
 			->willReturn($signedRequest);
 
-		$this->configureHappyPath('valid-refresh-token', 123, 'testuser', 'owner', 'sharee@remote.example.com', 'fixedjtivalue00');
+		$this->configureHappyPath('valid-refresh-token', 123, 'testuser', 'owner', 'sharee@department@remote.example.com', 'fixedjtivalue00');
 
 		$this->ocmTokenMapMapper->expects($this->once())
 			->method('insert')
@@ -181,7 +189,7 @@ public function testAccessTokenSuccess(): void {
 		$decoded = JWT::decode($data['access_token'], new Key($this->publicKeyPem, 'RS256'));
 		$this->assertSame('https://local.example.com', $decoded->iss);
 		$this->assertSame('owner', $decoded->sub);
-		$this->assertSame('sharee@remote.example.com', $decoded->aud);
+		$this->assertSame('sharee@department@remote.example.com', $decoded->aud);
 		$this->assertSame('789', $decoded->client_id);
 		$this->assertSame('fixedjtivalue00', $decoded->jti);
 		$this->assertSame(1000000, $decoded->iat);

lib/private/AppFramework/Http/Attributes/FederationRateLimit.php

@@ -13,7 +13,6 @@
 use OC\OCM\OCMDiscoveryService;
 use OCA\Federation\TrustedServers;
 use OCP\AppFramework\Http\Attribute\AnonRateLimit;
-use OCP\Federation\ICloudFederationProviderManager;
 use OCP\IRequest;
 use OCP\Server;
 
@@ -25,14 +24,12 @@
  */
 #[Attribute(Attribute::TARGET_METHOD)]
 class FederationRateLimit extends AnonRateLimit {
-	private readonly ICloudFederationProviderManager $federationProviderManager;
 	private readonly OCMDiscoveryService $discoveryService;
 	private readonly ?TrustedServers $trustedServers;
 
 	public function __construct(int $limit, int $period) {
 		parent::__construct($limit, $period);
 
-		$this->federationProviderManager = Server::get(ICloudFederationProviderManager::class);
 		$this->discoveryService = Server::get(OCMDiscoveryService::class);
 		$this->trustedServers = Server::get(TrustedServers::class);
 	}
@@ -44,16 +41,8 @@ public function shouldApply(IRequest $request): bool {
 		}
 
 		try {
-			// Resolve the signer origin from the payload so trusted servers
-			// can be exempted.
-			$parsed = $request->getParams();
-			$identity = $this->federationProviderManager->resolveSenderIdentity($parsed);
-			$origin = null;
-			if ($identity !== null) {
-				$origin = $this->discoveryService->getHostFromOcmAddress($identity);
-			}
-
-			$signedRequest = $this->discoveryService->getIncomingSignedRequest($origin);
+			$owner = $request->getParam('owner');
+			$signedRequest = $this->discoveryService->getIncomingSignedRequest(is_string($owner) ? $owner : null);
 			if (!$signedRequest) {
 				return true;
 			}

lib/private/Federation/CloudFederationProviderManager.php

@@ -18,7 +18,6 @@
 use OCP\Federation\ICloudFederationProviderManager;
 use OCP\Federation\ICloudFederationShare;
 use OCP\Federation\ICloudIdManager;
-use OCP\Federation\ISignedCloudFederationProvider;
 use OCP\Http\Client\IClient;
 use OCP\Http\Client\IClientService;
 use OCP\Http\Client\IResponse;
@@ -106,39 +105,6 @@ public function getCloudFederationProvider($resourceType) {
 		}
 	}
 
-	/**
-	 * @inheritDoc
-	 *
-	 * Notifications resolve via sharedSecret; shares via owner/sender.
-	 */
-	#[\Override]
-	public function resolveSenderIdentity(array $body): ?string {
-		$resourceType = $body['resourceType'] ?? '';
-		if ($resourceType !== '') {
-			$notification = $body['notification'] ?? null;
-			$sharedSecret = is_array($notification) ? ($notification['sharedSecret'] ?? '') : '';
-			if ($sharedSecret !== '') {
-				try {
-					$provider = $this->getCloudFederationProvider($resourceType);
-					if ($provider instanceof ISignedCloudFederationProvider || $provider instanceof \NCU\Federation\ISignedCloudFederationProvider) {
-						$identity = $provider->getFederationIdFromSharedSecret($sharedSecret, is_array($notification) ? $notification : []);
-						if ($identity !== '') {
-							return $identity;
-						}
-					}
-				} catch (\Exception) {
-					// unresolved; fall through to share-style fields
-				}
-			}
-		}
-		foreach (['owner', 'sender', 'sharedBy'] as $field) {
-			if (isset($body[$field]) && is_string($body[$field]) && $body[$field] !== '') {
-				return $body[$field];
-			}
-		}
-		return null;
-	}
-
 	/**
 	 * @deprecated 29.0.0 - Use {@see sendCloudShare()} instead and handle errors manually
 	 */

lib/private/OCM/OCMDiscoveryService.php

@@ -259,7 +259,8 @@ public function getLocalOCMProvider(bool $fullDetails = true): IOCMProvider {
 	 * @since 33.0.0
 	 */
 	#[\Override]
-	public function getIncomingSignedRequest(?string $origin = null): ?IIncomingSignedRequest {
+	public function getIncomingSignedRequest(?string $ocmAddress = null): ?IIncomingSignedRequest {
+		$origin = $ocmAddress !== null ? $this->getHostFromOcmAddress($ocmAddress) : null;
 		try {
 			$signedRequest = $this->signatureManager->getIncomingSignedRequest($this->signatoryManager, null, $origin);
 			$this->logger->debug('signed request available', ['signedRequest' => $signedRequest]);
@@ -316,8 +317,7 @@ public function confirmRequestOrigin(?string $signedOrigin, string $ocmAddress):
 	 * @return string the host (with port) of the OCM address
 	 * @throws IncomingRequestException on malformed address or unresolvable host
 	 */
-	#[\Override]
-	public function getHostFromOcmAddress(string $entry): string {
+	private function getHostFromOcmAddress(string $entry): string {
 		try {
 			$cloudId = $this->cloudIdManager->resolveCloudId(trim($entry, '@'));
 			return $this->signatureManager->extractIdentityFromUri($cloudId->getRemote());

lib/private/OCM/OCMSignatoryManager.php

@@ -404,6 +404,11 @@ private function buildLocalUrl(string $path): string {
 			return 'https://' . $identity . $path;
 		}
 
+		try {
+			return $this->signatureManager->generateKeyIdFromConfig($path);
+		} catch (IdentityNotFoundException) {
+		}
+
 		return $this->urlGenerator->getAbsoluteURL($path);
 	}
 
@@ -522,13 +527,20 @@ private function fetchJwks(string $origin): ?array {
 	}
 
 	/**
-	 * The peer's `jwksUri` from its discovery response. Must be https, or
-	 * http from an http-only peer (the spec's testing fallback): an https
-	 * peer pointing at an http jwksUri would downgrade the key fetch.
+	 * Resolve the peer's `jwksUri`, preferring HTTPS discovery. HTTP is accepted
+	 * only when HTTPS discovery fails and the discovery document is fetched over
+	 * HTTP as well.
 	 */
 	private function resolveJwksUri(string $origin): ?string {
 		try {
-			$provider = Server::get(IOCMDiscoveryService::class)->discover($origin);
+			$discoveryService = Server::get(IOCMDiscoveryService::class);
+			try {
+				$provider = $discoveryService->discover('https://' . $origin);
+				$discoveryScheme = 'https';
+			} catch (OCMProviderException) {
+				$provider = $discoveryService->discover('http://' . $origin);
+				$discoveryScheme = 'http';
+			}
 		} catch (NotFoundExceptionInterface|ContainerExceptionInterface|OCMProviderException $e) {
 			$this->logger->warning('cannot discover remote OCM provider for JWKS', ['exception' => $e, 'origin' => $origin]);
 			return null;
@@ -541,10 +553,14 @@ private function resolveJwksUri(string $origin): ?string {
 			}
 			return null;
 		}
-		$httpFromHttpPeer = str_starts_with($jwksUri, 'http://')
-			&& str_starts_with($provider->getEndPoint(), 'http://');
-		if (!str_starts_with($jwksUri, 'https://') && !$httpFromHttpPeer) {
-			$this->logger->warning('refusing jwksUri: https is required unless the peer itself is http-only', ['origin' => $origin, 'jwksUri' => $jwksUri]);
+		$secureJwks = str_starts_with($jwksUri, 'https://');
+		$httpJwksFromHttpDiscovery = $discoveryScheme === 'http' && str_starts_with($jwksUri, 'http://');
+		if (!$secureJwks && !$httpJwksFromHttpDiscovery) {
+			$this->logger->warning('refusing jwksUri for the OCM discovery transport', [
+				'origin' => $origin,
+				'jwksUri' => $jwksUri,
+				'discoveryScheme' => $discoveryScheme,
+			]);
 			return null;
 		}
 		return $jwksUri;