diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 289a6827..caaa0fb0 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -61,6 +61,13 @@ jobs: run: | docker-compose exec -T web ./vendor/bin/phpcs -s + - name: Generate proof key + run: | + set -x + docker-compose exec -T collabora coolconfig generate-proof-key + # Collabora becomes unavailable after the above command. + docker-compose restart collabora + - name: Drupal site install run: | docker-compose exec -T web ./vendor/bin/run drupal:site-install diff --git a/README.md b/README.md index 602f3eff..fc321040 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,14 @@ For a local demo or test installation, [see below](#development--demo-installati - (optional) For the 'Collabora Online Group' sub-module, you also need the [Group](https://drupa.org/project/group) and [Group Media](https://drupa.org/project/groupmedia) modules. -### Installation steps +### Collabora Online server preparation + +It is recommended to generate a proof key as documented here:\ +https://sdk.collaboraonline.com/docs/advanced_integration.html#wopi-proof + +If not, the proof mechanism needs to be disabled in the module settings. + +### Module installation steps See the [Drupal guide to install modules](https://www.drupal.org/docs/extending-drupal/installing-modules). @@ -39,7 +46,7 @@ field in the `composer.json` of your project. Then you can go into Drupal logged as an admin and go to _Extend_. In the list you should be able to find _Collabora Online_ and enable it. -From there you can access the module specific configuration. +From there you can access the module-specific configuration. Please check the "Configuration" section below! @@ -69,6 +76,9 @@ docker-compose up -d docker-compose exec web composer install docker-compose exec web ./vendor/bin/run drupal:site-install + +docker-compose exec collabora coolconfig generate-proof-key +docker-compose restart collabora ``` Optionally, generate an admin login link. diff --git a/collabora_online.post_update.php b/collabora_online.post_update.php new file mode 100644 index 00000000..02de2850 --- /dev/null +++ b/collabora_online.post_update.php @@ -0,0 +1,24 @@ +getEditable('collabora_online.settings'); + $cool_settings = $config->get('cool') ?? []; + $cool_settings['wopi_proof'] ??= TRUE; + $config->set('cool', $cool_settings); + $config->save(); +} diff --git a/collabora_online.routing.yml b/collabora_online.routing.yml index d0619ee0..c1fb763e 100644 --- a/collabora_online.routing.yml +++ b/collabora_online.routing.yml @@ -47,7 +47,8 @@ collabora-online.wopi.info: action: 'info' methods: [ GET ] requirements: - _permission: 'access content' + _collabora_online_wopi_access: 'TRUE' + _format: 'collabora_online_wopi' options: parameters: action: @@ -62,7 +63,8 @@ collabora-online.wopi.contents: action: 'content' methods: [ GET ] requirements: - _permission: 'access content' + _collabora_online_wopi_access: 'TRUE' + _format: 'collabora_online_wopi' options: parameters: action: @@ -77,7 +79,8 @@ collabora-online.wopi.save: action: 'save' methods: [ POST ] requirements: - _permission: 'access content' + _collabora_online_wopi_access: 'TRUE' + _format: 'collabora_online_wopi' options: parameters: action: diff --git a/collabora_online.services.yml b/collabora_online.services.yml index 98e6eaf1..c9c7d982 100644 --- a/collabora_online.services.yml +++ b/collabora_online.services.yml @@ -12,3 +12,7 @@ services: class: Drupal\collabora_online\Jwt\JwtTranscoder Drupal\collabora_online\MediaHelperInterface: class: Drupal\collabora_online\MediaHelper + Drupal\collabora_online\EventSubscriber\ExceptionWopiSubscriber: { } + Drupal\collabora_online\Access\WopiProofAccessCheck: + tags: + - { name: access_check, applies_to: _collabora_online_wopi_access } diff --git a/composer.json b/composer.json index 80947b68..dc32a09e 100644 --- a/composer.json +++ b/composer.json @@ -12,7 +12,9 @@ "require": { "php": ">=8.1", "drupal/key": "^1.19", - "firebase/php-jwt": "^6.10" + "firebase/php-jwt": "^6.10", + "phpseclib/phpseclib": "^3.0", + "symfony/error-handler": "^6.4|^7.1" }, "require-dev": { "composer/installers": "^2", diff --git a/config/install/collabora_online.settings.yml b/config/install/collabora_online.settings.yml index 60e09ee9..17f2890f 100644 --- a/config/install/collabora_online.settings.yml +++ b/config/install/collabora_online.settings.yml @@ -10,5 +10,6 @@ cool: access_token_ttl: 86400 # Disable cert checks. disable_cert_check: false + wopi_proof: true # Allow fullscreen - Enabled by default for functionality. allowfullscreen: true diff --git a/config/schema/collabora_online.schema.yml b/config/schema/collabora_online.schema.yml index d187e2f5..2704cb1e 100644 --- a/config/schema/collabora_online.schema.yml +++ b/config/schema/collabora_online.schema.yml @@ -21,6 +21,9 @@ collabora_online.settings: disable_cert_check: type: boolean label: 'Disable cert checks.' + wopi_proof: + type: boolean + label: 'Verify WOPI proof header and timestamp.' allowfullscreen: type: boolean label: 'Allow full-screen.' diff --git a/src/Access/WopiProofAccessCheck.php b/src/Access/WopiProofAccessCheck.php new file mode 100644 index 00000000..a58257d6 --- /dev/null +++ b/src/Access/WopiProofAccessCheck.php @@ -0,0 +1,281 @@ +configFactory->get('collabora_online.settings'); + if (!($config->get('cool')['wopi_proof'] ?? TRUE)) { + return AccessResult::allowed() + ->addCacheableDependency($config); + } + // Each incoming request will have a different proof and timestamp, so there + // is no point in caching. + return $this->doCheckAccess($request) + ->setCacheMaxAge(0); + } + + /** + * Checks the WOPI proof and the timeout, without adding cache metadata. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The request. + * + * @return \Drupal\Core\Access\AccessResult + * An access result without cache metadata. + * Instead, calling code should set cache max age 0. + */ + protected function doCheckAccess(Request $request): AccessResult { + $timeout_access = $this->checkTimeout($request); + if (!$timeout_access->isAllowed()) { + return $timeout_access; + } + // There is no need for ->andIf(), because there is no cache metadata to + // merge. + return $this->checkProof($request); + } + + /** + * Checks if the X-WOPI-Timestamp is expired, without cache metadata. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The request. + * + * @return \Drupal\Core\Access\AccessResult + * An access result without cache metadata. + */ + protected function checkTimeout(Request $request): AccessResult { + $wopi_ticks_str = $request->headers->get('X-WOPI-Timestamp', ''); + if (!is_numeric($wopi_ticks_str)) { + return AccessResult::forbidden('The X-WOPI-Timestamp header is missing, empty or invalid.'); + } + $wopi_timestamp = DotNetTime::ticksToTimestamp((float) $wopi_ticks_str); + $now_timestamp = $this->time->getRequestTime(); + $wopi_age_seconds = $now_timestamp - $wopi_timestamp; + if ($wopi_age_seconds > $this->ttlSeconds) { + return AccessResult::forbidden(sprintf( + 'The X-WOPI-Timestamp header is %s seconds old, which is more than the %s seconds TTL.', + $wopi_age_seconds, + $this->ttlSeconds, + )); + } + return AccessResult::allowed(); + } + + /** + * Checks the WOPI proof, without adding cache metadata. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The request. + * + * @return \Drupal\Core\Access\AccessResult + * An access result without cache metadata. + */ + protected function checkProof(Request $request): AccessResult { + $keys = $this->getKeys(); + if (!isset($keys['current'])) { + return AccessResult::forbidden('Missing or incomplete WOPI proof keys.'); + } + $signatures = $this->getSignatures($request); + if (!isset($signatures['current'])) { + return AccessResult::forbidden('Missing or incomplete WOPI proof headers.'); + } + $subject = $this->getSubject($request); + + // Try different key and signature combinations. + foreach ($keys as $key_name => $key) { + foreach ($signatures as $signature_name => $signature) { + if ($key_name === 'old' && $signature_name === 'old') { + // Don't verify an old signature with an old key. + continue; + } + $success = $key->verify($subject, $signature); + if ($success) { + return AccessResult::allowed(); + } + } + } + return AccessResult::forbidden('WOPI proof mismatch.'); + } + + /** + * Gets the message to be signed. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The request. + * + * @return string + * The message to be signed. + */ + protected function getSubject(Request $request): string { + // This class is not responsible for checking the expiration, but it still + // needs the WOPI timestamp to build the message for the signature. + $timestamp_ticks = $request->headers->get('X-WOPI-Timestamp'); + $token = $request->query->get('access_token', ''); + $url = $request->getUri(); + return sprintf( + '%s%s%s%s%s%s', + pack('N', strlen($token)), + $token, + pack('N', strlen($url)), + strtoupper($url), + pack('N', 8), + pack('J', $timestamp_ticks) + ); + } + + /** + * Gets RSA public keys from the discovery.xml. + * + * The discovery.xml has a current and an old key. + * This is to support situations when the key has been recently changed, but + * the incoming request was signed with the older key. + * + * @return array<'current'|'old', \phpseclib3\Crypt\RSA\PublicKey> + * Current and old public key, or just the current if they are the same, or + * empty array if none found. + */ + protected function getKeys(): array { + // Get current and old key. + // Remove empty values. + // If both are the same, keep only the current one. + $public_keys = array_unique(array_filter([ + 'current' => $this->discovery->getProofKey(), + 'old' => $this->discovery->getProofKeyOld(), + ])); + $key_objects = []; + foreach ($public_keys as $key_name => $key_str) { + $key_obj = $this->prepareKey($key_str, $key_name); + if ($key_obj === NULL) { + continue; + } + $key_objects[$key_name] = $key_obj; + } + return $key_objects; + } + + /** + * Gets an RSA key object based on a string value. + * + * @param string $key_str + * Key string value from discovery.xml. + * @param 'current'|'old' $key_name + * Key name, only used for logging. + * + * @return \phpseclib3\Crypt\RSA\PublicKey|null + * An RSA public key object, or NULL on failure. + */ + protected function prepareKey(string $key_str, string $key_name): ?PublicKey { + try { + $key_object = PublicKeyLoader::loadPublicKey($key_str); + } + catch (\Throwable $e) { + $log_message = "Problem with the @name key from discovery.yml:
\n" + . Error::DEFAULT_ERROR_MESSAGE; + $log_args = Error::decodeException($e); + $log_args['@name'] = $key_name; + $this->logger->error($log_message, $log_args); + return NULL; + } + if (!$key_object instanceof PublicKey) { + $log_message = "Problem with the @name key from discovery.yml:
\n" + . "Expected RSA public key, found @type."; + $log_args = [ + '@name' => $key_name, + '@type' => get_debug_type($key_object), + ]; + $this->logger->error($log_message, $log_args); + return NULL; + } + return $key_object + ->withHash('sha256') + ->withPadding(RSA::SIGNATURE_RELAXED_PKCS1); + } + + /** + * Gets the current and old signature from the request. + * + * The request will have a current and an old signature. + * This is to support situations when the key has been recently changed, but + * the cached discovery.xml still has the old key. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * Incoming request that may have a signature to be verified. + * + * @return array{current?: string, old?: string} + * Current and old signature from the request, decoded and ready for use. + * If they are the same, only one of them is returned. + * If no signatures are found, an empty array is returned. + */ + protected function getSignatures(Request $request): array { + // Get the current and old proof header. + // Remove empty values. + // If both are the same, keep only the current one. + $proof_headers = array_unique(array_filter([ + 'current' => $request->headers->get('X-WOPI-Proof'), + 'old' => $request->headers->get('X-WOPI-ProofOld'), + ])); + $decoded_proof_headers = array_map( + fn (string $header_value) => base64_decode($header_value, TRUE), + $proof_headers, + ); + // Remove false values where decoding failed. + return array_filter($decoded_proof_headers); + } + +} diff --git a/src/Controller/WopiController.php b/src/Controller/WopiController.php index 1d8178f2..3921df13 100644 --- a/src/Controller/WopiController.php +++ b/src/Controller/WopiController.php @@ -30,6 +30,7 @@ use Symfony\Component\HttpFoundation\BinaryFileResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; /** * Provides WOPI route responses for the Collabora module. @@ -54,20 +55,6 @@ public function __construct( $this->setStringTranslation($string_translation); } - /** - * Creates a failure response that is understood by Collabora. - * - * @return \Symfony\Component\HttpFoundation\Response - * Response object. - */ - public static function permissionDenied(): Response { - return new Response( - 'Authentication failed.', - Response::HTTP_FORBIDDEN, - ['content-type' => 'text/plain'], - ); - } - /** * Handles the WOPI 'info' request for a media entity. * @@ -81,21 +68,21 @@ public static function permissionDenied(): Response { */ public function wopiCheckFileInfo(string $id, Request $request): Response { $token = $request->query->get('access_token'); + if ($token === NULL) { + throw new AccessDeniedHttpException('Missing access token.'); + } $jwt_payload = $this->verifyTokenForMediaId($token, $id); - if ($jwt_payload === NULL) { - return static::permissionDenied(); - } /** @var \Drupal\media\MediaInterface|null $media */ $media = $this->entityTypeManager->getStorage('media')->load($id); if ($media === NULL) { - return static::permissionDenied(); + throw new AccessDeniedHttpException('Media not found.'); } $file = $this->mediaHelper->getFileForMedia($media); if ($file === NULL) { - return static::permissionDenied(); + throw new AccessDeniedHttpException('No file attached to media.'); } $mtime = date_create_immutable_from_format('U', $file->getChangedTime()); @@ -103,13 +90,13 @@ public function wopiCheckFileInfo(string $id, Request $request): Response { /** @var \Drupal\user\UserInterface|null $user */ $user = $this->entityTypeManager->getStorage('user')->load($jwt_payload['uid']); if ($user === NULL) { - return static::permissionDenied(); + throw new AccessDeniedHttpException('User not found.'); } $can_write = $jwt_payload['wri']; if ($can_write && !$media->access('edit in collabora', $user)) { $this->logger->error('Token and user permissions do not match.'); - return static::permissionDenied(); + throw new AccessDeniedHttpException('The user does not have collabora edit access for this media.'); } $payload = [ @@ -153,9 +140,6 @@ public function wopiGetFile(string $id, Request $request): Response { $token = $request->query->get('access_token'); $jwt_payload = $this->verifyTokenForMediaId($token, $id); - if ($jwt_payload === NULL) { - return static::permissionDenied(); - } /** @var \Drupal\user\UserInterface|null $user */ $user = $this->entityTypeManager->getStorage('user')->load($jwt_payload['uid']); @@ -164,12 +148,12 @@ public function wopiGetFile(string $id, Request $request): Response { /** @var \Drupal\media\MediaInterface|null $media */ $media = $this->entityTypeManager->getStorage('media')->load($id); if ($media === NULL) { - return static::permissionDenied(); + throw new AccessDeniedHttpException('Media not found.'); } $file = $this->mediaHelper->getFileForMedia($media); if ($file === NULL) { - return static::permissionDenied(); + throw new AccessDeniedHttpException('No file attached to media.'); } $mimetype = $file->getMimeType(); @@ -201,16 +185,20 @@ public function wopiPutFile(string $id, Request $request): Response { $exitsave = $request->headers->get('x-cool-wopi-isexitsave') == 'true'; $jwt_payload = $this->verifyTokenForMediaId($token, $id); - if ($jwt_payload == NULL || empty($jwt_payload['wri'])) { - return static::permissionDenied(); + if (empty($jwt_payload['wri'])) { + throw new AccessDeniedHttpException('The token only grants read access.'); } /** @var \Drupal\media\MediaInterface|null $media */ $media = $this->entityTypeManager->getStorage('media')->load($id); + if ($media === NULL) { + throw new AccessDeniedHttpException('Media not found.'); + } + /** @var \Drupal\user\UserInterface|null $user */ $user = $this->entityTypeManager->getStorage('user')->load($jwt_payload['uid']); - if ($media === NULL || $user === NULL) { - return static::permissionDenied(); + if ($user === NULL) { + throw new AccessDeniedHttpException('User not found.'); } $this->accountSwitcher->switchTo($user); @@ -333,27 +321,35 @@ public function wopi(string $action, string $id, Request $request): Response { * Media id expected to be in the token payload. * This could be a stringified integer like '123'. * - * @return array|null - * Data decoded from the token, or NULL on failure or if the token has - * expired. + * @return array + * Data decoded from the token. + * + * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException + * The token is malformed, invalid or has expired. */ protected function verifyTokenForMediaId( #[\SensitiveParameter] string $token, int|string $expected_media_id, - ): array|null { + ): array { try { $values = $this->jwtTranscoder->decode($token); } catch (CollaboraNotAvailableException $e) { $this->logger->warning('A token cannot be decoded: @message', ['@mesage' => $e->getMessage()]); - return NULL; + throw new AccessDeniedHttpException('Malformed token'); } if ($values === NULL) { - return NULL; + throw new AccessDeniedHttpException('Empty token values'); } - if ($values['fid'] !== $expected_media_id) { - return NULL; + if ((string) $values['fid'] !== (string) $expected_media_id) { + throw new AccessDeniedHttpException(sprintf( + // The token payload is not encrypted, just encoded. + // It is ok to reveal its values in the response for logging. + 'Found fid %s in request path, but fid %s in token payload', + $expected_media_id, + $values['fid'], + )); } return $values; } diff --git a/src/Cool/CollaboraDiscovery.php b/src/Cool/CollaboraDiscovery.php index e11adb2e..5b828b84 100644 --- a/src/Cool/CollaboraDiscovery.php +++ b/src/Cool/CollaboraDiscovery.php @@ -15,9 +15,10 @@ namespace Drupal\collabora_online\Cool; use Drupal\collabora_online\Exception\CollaboraNotAvailableException; +use Symfony\Component\ErrorHandler\ErrorHandler; /** - * Service to get a WOPI client url for a given MIME type. + * Service to get values from the discovery.xml. */ class CollaboraDiscovery implements CollaboraDiscoveryInterface { @@ -35,12 +36,7 @@ public function __construct( * {@inheritdoc} */ public function getWopiClientURL(string $mimetype = 'text/plain'): string { - $xml = $this->discoveryFetcher->getDiscoveryXml(); - - $discovery_parsed = simplexml_load_string($xml); - if (!$discovery_parsed) { - throw new CollaboraNotAvailableException('The retrieved discovery.xml file is not a valid XML file.'); - } + $discovery_parsed = $this->getParsedXml(); $result = $discovery_parsed->xpath(sprintf('/wopi-discovery/net-zone/app[@name=\'%s\']/action', $mimetype)); if (empty($result[0]['urlsrc'][0])) { @@ -50,4 +46,70 @@ public function getWopiClientURL(string $mimetype = 'text/plain'): string { return (string) $result[0]['urlsrc'][0]; } + /** + * {@inheritdoc} + */ + public function getProofKey(): ?string { + $discovery_parsed = $this->getParsedXml(); + $attribute = $discovery_parsed->xpath('/wopi-discovery/proof-key/@value')[0] ?? NULL; + return $attribute?->__toString(); + } + + /** + * {@inheritdoc} + */ + public function getProofKeyOld(): ?string { + $discovery_parsed = $this->getParsedXml(); + $attribute = $discovery_parsed->xpath('/wopi-discovery/proof-key/@oldvalue')[0] ?? NULL; + return $attribute?->__toString(); + } + + /** + * Fetches the discovery.xml, and gets the parsed contents. + * + * @return \SimpleXMLElement + * Parsed xml from the discovery.xml. + * + * @throws \Drupal\collabora_online\Exception\CollaboraNotAvailableException + * Fetching the discovery.xml failed, or the result is not valid xml. + */ + protected function getParsedXml(): \SimpleXMLElement { + $xml = $this->discoveryFetcher->getDiscoveryXml(); + return $this->parseXml($xml); + } + + /** + * Parses an XML string. + * + * @param string $xml + * XML string. + * + * @return \SimpleXMLElement + * Parsed XML. + * + * @throws \Drupal\collabora_online\Exception\CollaboraNotAvailableException + * The XML is invalid or empty. + */ + protected function parseXml(string $xml): \SimpleXMLElement { + try { + // Avoid errors from XML parsing hitting the regular error handler. + // An alternative would be libxml_use_internal_errors(), but then we would + // have to deal with the results from libxml_get_errors(). + $parsed_xml = ErrorHandler::call( + fn () => simplexml_load_string($xml), + ); + } + catch (\ErrorException $e) { + throw new CollaboraNotAvailableException('Error in the retrieved discovery.xml file: ' . $e->getMessage(), previous: $e); + } + if ($parsed_xml === FALSE) { + // The parser returned FALSE, but no error was raised. + // This is known to happen when $xml is an empty string. + // Instead we could check for $xml === '' earlier, but we don't know for + // sure if this is, and always will be, the only such case. + throw new CollaboraNotAvailableException('The discovery.xml file is empty.'); + } + return $parsed_xml; + } + } diff --git a/src/Cool/CollaboraDiscoveryInterface.php b/src/Cool/CollaboraDiscoveryInterface.php index b8aae932..e9472c8d 100644 --- a/src/Cool/CollaboraDiscoveryInterface.php +++ b/src/Cool/CollaboraDiscoveryInterface.php @@ -15,7 +15,7 @@ namespace Drupal\collabora_online\Cool; /** - * Service to get a WOPI client url for a given MIME type. + * Service to get a WOPI client URL for a given MIME type. */ interface CollaboraDiscoveryInterface { @@ -23,15 +23,34 @@ interface CollaboraDiscoveryInterface { * Gets the URL for the WOPI client. * * @param string $mimetype - * Mime type for which to get the WOPI client url. + * Mime type for which to get the WOPI client URL. * This refers to config entries in the discovery.xml file. * * @return string - * The WOPI client url. + * The WOPI client URL. * * @throws \Drupal\collabora_online\Exception\CollaboraNotAvailableException - * The client url cannot be retrieved. + * The client URL cannot be retrieved. */ public function getWopiClientURL(string $mimetype = 'text/plain'): string; + /** + * Gets the public key used for proofing. + * + * @return string|null + * The recent key, or NULL if none found. + */ + public function getProofKey(): ?string; + + /** + * Gets the old public key for proofing. + * + * This covers the case when the public key was already updated, but an + * incoming request has a proof that was generated with the previous key. + * + * @return string|null + * The old key, or NULL if none found. + */ + public function getProofKeyOld(): ?string; + } diff --git a/src/EventSubscriber/ExceptionWopiSubscriber.php b/src/EventSubscriber/ExceptionWopiSubscriber.php new file mode 100644 index 00000000..70a1f045 --- /dev/null +++ b/src/EventSubscriber/ExceptionWopiSubscriber.php @@ -0,0 +1,54 @@ +getThrowable(); + + // If the exception is cacheable, generate a cacheable response. + if ($exception instanceof CacheableDependencyInterface) { + $response = new CacheableResponse($exception->getMessage(), $exception->getStatusCode(), $exception->getHeaders()); + $response->addCacheableDependency($exception); + } + else { + $response = new Response($exception->getMessage(), $exception->getStatusCode(), $exception->getHeaders()); + } + + $event->setResponse($response); + } + +} diff --git a/src/Form/ConfigForm.php b/src/Form/ConfigForm.php index 05a20e84..f5a0c3a1 100644 --- a/src/Form/ConfigForm.php +++ b/src/Form/ConfigForm.php @@ -86,6 +86,12 @@ public function buildForm(array $form, FormStateInterface $form_state): array { '#default_value' => $cool_settings['disable_cert_check'] ?? FALSE, ]; + $form['wopi_proof'] = [ + '#type' => 'checkbox', + '#title' => $this->t('Verify proof header and timestamp in incoming WOPI requests.'), + '#default_value' => $cool_settings['wopi_proof'] ?? TRUE, + ]; + $form['allowfullscreen'] = [ '#type' => 'checkbox', '#title' => $this->t('Allow COOL to use fullscreen mode.'), @@ -116,6 +122,7 @@ public function submitForm(array &$form, FormStateInterface $form_state): void { ->set('cool.key_id', $form_state->getValue('key_id')) ->set('cool.access_token_ttl', $form_state->getValue('access_token_ttl')) ->set('cool.disable_cert_check', $form_state->getValue('disable_cert_check')) + ->set('cool.wopi_proof', $form_state->getValue('wopi_proof')) ->set('cool.allowfullscreen', $form_state->getValue('allowfullscreen')) ->save(); diff --git a/src/Jwt/JwtTranscoderBase.php b/src/Jwt/JwtTranscoderBase.php index de906e93..0474a025 100644 --- a/src/Jwt/JwtTranscoderBase.php +++ b/src/Jwt/JwtTranscoderBase.php @@ -45,8 +45,8 @@ public function decode( $this->logger->error($e->getMessage()); return NULL; } - if (!isset($payload['exp']) || $payload['exp'] < gettimeofday(TRUE)) { - // The token is expired, or no timeout was set. + if (!isset($payload['exp'])) { + // The token does not have an expiration timestamp. return NULL; } return $payload; diff --git a/src/Util/DotNetTime.php b/src/Util/DotNetTime.php new file mode 100644 index 00000000..163c4e05 --- /dev/null +++ b/src/Util/DotNetTime.php @@ -0,0 +1,54 @@ + + + + + + + + + + + + + + diff --git a/tests/fixtures/discovery.proof-key.xml b/tests/fixtures/discovery.proof-key.xml new file mode 100644 index 00000000..0dd9f537 --- /dev/null +++ b/tests/fixtures/discovery.proof-key.xml @@ -0,0 +1,11 @@ + + + + diff --git a/tests/src/Functional/ConfigFormTest.php b/tests/src/Functional/ConfigFormTest.php index 58a03a62..23542ebe 100644 --- a/tests/src/Functional/ConfigFormTest.php +++ b/tests/src/Functional/ConfigFormTest.php @@ -60,8 +60,9 @@ public function testConfigForm(): void { $assert_session->fieldValueEquals('WOPI host URL', 'https://localhost/'); $assert_session->fieldValueEquals('JWT private key', ''); $assert_session->fieldValueEquals('Access Token Expiration (in seconds)', '86400'); - $assert_session->fieldValueEquals('Disable TLS certificate check for COOL.', ''); - $assert_session->fieldValueEquals('Allow COOL to use fullscreen mode.', '1'); + $assert_session->checkboxNotChecked('Disable TLS certificate check for COOL.'); + $assert_session->checkboxChecked('Verify proof header and timestamp in incoming WOPI requests.'); + $assert_session->checkboxChecked('Allow COOL to use fullscreen mode.'); // The key select element has no options, because no compatible key exists. $this->assertSame( @@ -94,6 +95,8 @@ public function testConfigForm(): void { ->setValue('3600'); $assert_session->fieldExists('Disable TLS certificate check for COOL.') ->check(); + $assert_session->fieldExists('Verify proof header and timestamp in incoming WOPI requests.') + ->uncheck(); // Since default is checked we disable the full screen option. $assert_session->fieldExists('Allow COOL to use fullscreen mode.') ->uncheck(); @@ -107,8 +110,9 @@ public function testConfigForm(): void { $assert_session->fieldValueEquals('WOPI host URL', 'http://wopihost.com'); $assert_session->fieldValueEquals('JWT private key', 'collabora_test'); $assert_session->fieldValueEquals('Access Token Expiration (in seconds)', '3600'); - $assert_session->fieldValueEquals('Disable TLS certificate check for COOL.', '1'); - $assert_session->fieldValueEquals('Allow COOL to use fullscreen mode.', ''); + $assert_session->checkboxChecked('Disable TLS certificate check for COOL.'); + $assert_session->checkboxNotChecked('Verify proof header and timestamp in incoming WOPI requests.'); + $assert_session->checkboxNotChecked('Allow COOL to use fullscreen mode.'); // Test validation of required fields. $this->drupalGet(Url::fromRoute('collabora-online.settings')); @@ -143,8 +147,9 @@ public function testConfigForm(): void { $assert_session->fieldValueEquals('WOPI host URL', ''); $assert_session->fieldValueEquals('JWT private key', ''); $assert_session->fieldValueEquals('Access Token Expiration (in seconds)', '0'); - $assert_session->fieldValueEquals('Disable TLS certificate check for COOL.', ''); - $assert_session->fieldValueEquals('Allow COOL to use fullscreen mode.', ''); + $assert_session->checkboxNotChecked('Disable TLS certificate check for COOL.'); + $assert_session->checkboxChecked('Verify proof header and timestamp in incoming WOPI requests.'); + $assert_session->checkboxNotChecked('Allow COOL to use fullscreen mode.'); $assert_session->buttonExists('Save configuration'); } diff --git a/tests/src/Kernel/Controller/WopiControllerProofTest.php b/tests/src/Kernel/Controller/WopiControllerProofTest.php new file mode 100644 index 00000000..3eaf375b --- /dev/null +++ b/tests/src/Kernel/Controller/WopiControllerProofTest.php @@ -0,0 +1,191 @@ +getProofKey(). + * + * @var string + */ + protected string $wopiProofKey = self::PROOF_KEY; + + /** + * Mock return value for $discovery->getProofKeyOld(). + * + * @var string + */ + protected string $wopiProofKeyOld = self::PROOF_KEY; + + /** + * {@inheritdoc} + */ + protected function setUp(): void { + parent::setUp(); + + // Set a mock discovery with custom proof keys. + $mock_discovery = $this->createMock(CollaboraDiscoveryInterface::class); + $this->container->set(CollaboraDiscoveryInterface::class, $mock_discovery); + $mock_discovery->method('getProofKey')->willReturnReference($this->wopiProofKey); + $mock_discovery->method('getProofKeyOld')->willReturnReference($this->wopiProofKeyOld); + } + + /** + * Tests access check based on WOPI proof and timestamp. + */ + public function testWopiProofAccess(): void { + $requests = $this->createRequests(); + // The values below have been recorded from real WOPI requests coming from + // a Collabora client. + $wopi_tick_times = [ + 'info' => '638701624451393010', + 'file' => '638701624452984064', + 'save' => '638701624725606559', + ]; + $wopi_proofs = [ + 'info' => 'MQSnbW+hu4z18psc+EfHkNBP1oTMTkcTYSOy5bAouB24XKaiaPb/BRF05ds1oq8GIvSjxL0nczIYMOxZyiNzGg3BvkKOm2eGPIMNCJvk7QZyIF/FI6E7uTg4zp7K/x6S+Dph/nVrNT37xjvHsf0MuKjfGdJKDMz8WzhXsKs/yAJVxErrFcabJRuva48fLNmMYkO/c9lLqxpdNvlASBOajeoECuKYwMJitBgMMDwrFuPA7a+RjWIjtYkkHu4oyZXTdWskU6P6LFSE4jWe9rPAxYJsOAXDZpefDYDXUSryazSeK+AgaA3p69ZrFTD5M/FH1hEDKhLqWOll49n7oTnL8w==', + 'file' => 'Ka9G+FAyjzf04Sk2Z1DSX0f0Xk4a9qtUGhKIfF/DPTuJOHykw38XGZy+v045EIsOpVZ6CFnlToI/h6hbstYhBG78O0xMV0L/o65HO8jCkuFPvU4yTXSAfgnKSVuQB3bbti2KnzG57dp6UwwnIUb2kNnCV8W4LTGuCaCBR2Z9Ydwk09UHHaIBo57g6oHGKpLoiMLhOMq4PDW3RBis98Vmc+p6qDipj4f1c7HarBlxew4BLMC0ubRFwXsBxMWMn9E3xJdMdUZOIAyidtghoOSuLbSKyCixsxQ0ONMHRFKq4K//ozoWac+8V8h7IzTbD6LIQwNITJqUW5PwPuzt/dNdTg==', + 'save' => 'Pj5Ak7sBsI4Pa0RVkn+0jJNLhIuGJRKjRi//cv1lHE4O1d3VlgT1WsFc5tRr1IW/OfiLVg6yJielgzSpjNRhushUATq/YdRx4lA61Cx9KQ6Y9hr2SZdg4sNgFjOZSFAfIBBx0P1Hfqgl1olv3EjIO/Fb+7/YNSsSG+rtpjPt8fGdaRxVUa4vCUjVCoJl+uaTY8CohGE4Aj5llXUmL2ZuctA8M/Ts+yOWEPfJ/nTwI0o6oG/2BtrQMQxChM7Lk59W+iGHh/AbxwRU+K0t7bdktzwtYbRmWarJwSIE/7pZ0zVbVj92hFNqtqKzR52+ACTqLB/qQnpSMl3Yu3Z5FkcS9g==', + ]; + foreach ($requests as $name => $request) { + $wopi_tick_time = $wopi_tick_times[$name]; + $wopi_proof = $wopi_proofs[$name]; + $this->doTestWopiProofAccess($request, $wopi_tick_time, $wopi_proof, $name); + } + } + + /** + * Tests a single WOPI route with different proof combinations. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * A basic request for the route. + * @param string $wopi_tick_time + * Proof time in DotNet ticks, as for the X-WOPI-Timestamp header. + * @param string $wopi_proof + * WOPI proof value, as for the X-WOPI-Proof header. + * @param string $message + * Message to use in assertions. + */ + protected function doTestWopiProofAccess(Request $request, string $wopi_tick_time, string $wopi_proof, string $message): void { + $token = 'eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJmaWQiOiIxMDAwIiwidWlkIjoiMSIsIndyaSI6dHJ1ZSwiZXhwIjoxNzM0NjUyMDQ1LjA0NjY3NX0.tM4aHamo7sQA3pkqiNbABlMM5mi-Z9vODaSm847hxIA'; + $request->query->set('access_token', $token); + $request->query->set('access_token_ttl', '0'); + $request->server->set('QUERY_STRING', http_build_query($request->query->all())); + // The test proof values were generated in an environment with + // 'web.test:8080' as the host name. + $request->headers->set('HOST', 'web.test:8080'); + $request->headers->set('X-WOPI-Timestamp', $wopi_tick_time); + $set_state = function ( + ?int $offset_seconds = NULL, + ?string $proof_recent = NULL, + ?string $proof_old = NULL, + ?string $key_recent = NULL, + ?string $key_old = NULL, + ) use ($request, $wopi_tick_time, $wopi_proof) { + // By default, set a fake request time 18 minutes after the WOPI time. + $_SERVER['REQUEST_TIME'] = DotNetTime::ticksToTimestamp((float) $wopi_tick_time) + + ($offset_seconds ?? 18 * 20); + $request->headers->set('X-WOPI-Proof', $proof_recent ?? $wopi_proof); + $request->headers->set('X-WOPI-ProofOld', $proof_old ?? $wopi_proof); + $this->wopiProofKey = $key_recent ?? self::PROOF_KEY; + $this->wopiProofKeyOld = $key_old ?? self::PROOF_KEY; + // Set a timestamp that is earlier than the timeout. + // For now there is no parameter to manipulate this, but it is still part + // of the "state", so it makes sense to setup in this function. + JWT::$timestamp = 1734605245; + }; + + // With all values at their default, access is granted. + $set_state(); + // Clone the request to avoid side effects. + $this->assertRequestSuccess(clone $request, $message); + + // A single bad proof is ok, if the old proof is valid. + $set_state(proof_recent: self::BAD_PROOF); + $this->assertRequestSuccess(clone $request, $message); + + // A single bad old proof is ok, if the recent proof is valid. + $set_state(proof_old: self::BAD_PROOF); + $this->assertRequestSuccess(clone $request, $message); + + // Two bad proofs lead to failure. + $set_state(proof_recent: self::BAD_PROOF, proof_old: self::BAD_PROOF); + $this->assertAccessDeniedResponse('WOPI proof mismatch.', clone $request, $message); + + // A non-matching recent proof key is ok, if the old key still matches the + // recent proof from the request. + $set_state(key_recent: self::BAD_PROOF_KEY); + $this->assertRequestSuccess(clone $request, $message); + + // Two non-matching proof keys lead to failure. + $set_state(key_recent: self::BAD_PROOF_KEY, key_old: self::BAD_PROOF_KEY); + $this->assertAccessDeniedResponse('WOPI proof mismatch.', clone $request, $message); + + // The old proof matching the old key does not work. + $set_state(proof_recent: self::BAD_PROOF, key_recent: self::BAD_PROOF_KEY); + $this->assertAccessDeniedResponse('WOPI proof mismatch.', clone $request, $message); + + // Set a fake request time 22 minutes after the WOPI time. + $set_state(offset_seconds: 22 * 60); + $this->assertAccessDeniedResponse('The X-WOPI-Timestamp header is 1320 seconds old, which is more than the 1200 seconds TTL.', clone $request, $message); + + // Reset everything to clean up. + $set_state(); + } + + /** + * Asserts that a request results in a successful response. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The request. + * @param string $message + * A message to distinguish from other assertions. + */ + protected function assertRequestSuccess(Request $request, string $message): void { + $response = $this->handleRequest($request); + $this->assertEquals( + Response::HTTP_OK, + $response->getStatusCode(), + // Print the failure message if this is not 200. + $message . "\n" . substr((string) $response->getContent(), 0, 3000), + ); + } + +} diff --git a/tests/src/Kernel/Controller/WopiControllerTest.php b/tests/src/Kernel/Controller/WopiControllerTest.php index 758ded7d..9a71981e 100644 --- a/tests/src/Kernel/Controller/WopiControllerTest.php +++ b/tests/src/Kernel/Controller/WopiControllerTest.php @@ -14,84 +14,24 @@ namespace Drupal\Tests\collabora_online\Kernel\Controller; -use ColinODell\PsrTestLogger\TestLogger; -use Drupal\collabora_online\Jwt\JwtTranscoderInterface; -use Drupal\Component\Serialization\Json; -use Drupal\file\Entity\File; -use Drupal\file\FileInterface; -use Drupal\media\MediaInterface; -use Drupal\Tests\collabora_online\Kernel\CollaboraKernelTestBase; -use Drupal\user\UserInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; /** * @coversDefaultClass \Drupal\collabora_online\Controller\WopiController */ -class WopiControllerTest extends CollaboraKernelTestBase { - - /** - * {@inheritdoc} - */ - protected static $modules = [ - 'collabora_online_test', - ]; - - /** - * The user with access to perform operations. - * - * @var \Drupal\user\UserInterface - */ - protected UserInterface $user; - - /** - * The media where to perform operations. - * - * @var \Drupal\media\MediaInterface - */ - protected MediaInterface $media; - - /** - * The source file. - * - * @var \Drupal\file\FileInterface - */ - protected FileInterface $file; - - /** - * The test logger channel. - * - * @var \ColinODell\PsrTestLogger\TestLogger - */ - protected TestLogger $logger; +class WopiControllerTest extends WopiControllerTestBase { /** * {@inheritdoc} */ protected function setUp(): void { parent::setUp(); - $this->logger = new TestLogger(); - \Drupal::service('logger.factory')->addLogger($this->logger); - - $collabora_settings = \Drupal::configFactory()->getEditable('collabora_online.settings'); - $cool = $collabora_settings->get('cool'); - $cool['key_id'] = 'collabora'; - $collabora_settings->set('cool', $cool)->save(); - - // Make sure that ids for different entity types are distinguishable. - // This will reveal bugs where one id gets mixed up for another. - \Drupal::database()->query("ALTER TABLE {media} AUTO_INCREMENT = 1000"); - \Drupal::database()->query("ALTER TABLE {file_managed} AUTO_INCREMENT = 2000"); - $this->media = $this->createMediaEntity('document'); - $this->user = $this->createUser([ - 'access content', - 'edit any document in collabora', - ]); - $fid = $this->media->getSource()->getSourceFieldValue($this->media); - $this->file = File::load($fid); - - $this->setCurrentUser($this->user); + // Disable the WOPI proof for this test. + $this->config('collabora_online.settings') + ->set('cool.wopi_proof', FALSE) + ->save(); } /** @@ -208,7 +148,11 @@ public function testBadToken(): void { foreach ($requests as $name => $request) { // Replace the token with a value that is not in the JWT format. $request->query->set('access_token', 'bad_token'); - $this->assertAccessDeniedResponse($request, $name); + $this->assertAccessDeniedResponse( + 'Empty token values', + $request, + $name, + ); } } @@ -223,7 +167,11 @@ public function testWrongTokenPayload(): void { // Inject a bad value into the token payload. $requests = $this->createRequests(token_payload: ['fid' => 4321]); foreach ($requests as $name => $request) { - $this->assertAccessDeniedResponse($request, $name); + $this->assertAccessDeniedResponse( + sprintf('Found fid %s in request path, but fid 4321 in token payload', $this->media->id()), + $request, + $name, + ); } } @@ -237,7 +185,11 @@ public function testWrongTokenPayload(): void { public function testMediaNotFound(): void { $requests = $this->createRequests(media_id: 555); foreach ($requests as $name => $request) { - $this->assertAccessDeniedResponse($request, $name); + $this->assertAccessDeniedResponse( + 'Media not found.', + $request, + $name, + ); } } @@ -252,199 +204,12 @@ public function testUserNotFound(): void { $requests = $this->createRequests(user_id: 555); unset($requests['file']); foreach ($requests as $name => $request) { - $this->assertAccessDeniedResponse($request, $name); + $this->assertAccessDeniedResponse( + 'User not found.', + $request, + $name, + ); } } - /** - * Creates WOPI requests for different routes, with some shared parameters. - * - * This can be used for tests where each route is expected to have the same - * response. - * - * @param int|null $media_id - * Media entity id, if different from the default. - * @param int|null $user_id - * User id, if different from the default. - * @param array $token_payload - * Explicit token payload values. - * This can be used to cause a bad token. - * - * @return array - * Requests keyed by a distinguishable name. - */ - protected function createRequests(?int $media_id = NULL, ?int $user_id = NULL, array $token_payload = []): array { - $create_request = fn (string $uri_suffix, string $method = 'GET', bool $write = FALSE) => $this->createRequest( - $uri_suffix, - $method, - $media_id, - $user_id, - $write, - $token_payload, - ); - return [ - 'info' => $create_request(''), - 'file' => $create_request('/contents'), - 'save' => $create_request('/contents', 'POST', TRUE), - ]; - } - - /** - * Creates a WOPI request. - * - * @param string $uri_suffix - * Suffix to append to the WOPI media url. - * @param string $method - * E.g. 'GET' or 'POST'. - * @param int|null $media_id - * Media entity id, if different from the default. - * @param int|null $user_id - * User id, if different from the default. - * @param bool $write - * TRUE if write access is requested. - * @param array $token_payload - * Explicit token payload values. - * This can be used to cause a bad token. - * - * @return \Symfony\Component\HttpFoundation\Request - * The request. - */ - protected function createRequest( - string $uri_suffix = '', - string $method = 'GET', - ?int $media_id = NULL, - ?int $user_id = NULL, - bool $write = FALSE, - array $token_payload = [], - ): Request { - $media_id ??= (int) $this->media->id(); - $user_id ??= (int) $this->user->id(); - $uri = '/cool/wopi/files/' . $media_id . $uri_suffix; - $token = $this->createAccessToken($media_id, $user_id, $write, $token_payload); - $parameters = [ - 'id' => $media_id, - 'access_token' => $token, - ]; - return Request::create($uri, $method, $parameters); - } - - /** - * Retrieves an encoded access token. - * - * @param int|null $fid - * The file id. - * @param int|null $uid - * The user id. - * @param bool $write - * The write permission. - * @param array $payload - * Explicit payload values. - * This can be used to cause a bad token. - * - * @return string - * The enconded token. - */ - protected function createAccessToken(?int $fid = NULL, ?int $uid = NULL, bool $write = FALSE, array $payload = []): string { - /** @var \Drupal\collabora_online\Jwt\JwtTranscoderInterface $transcoder */ - $transcoder = \Drupal::service(JwtTranscoderInterface::class); - $expire_timestamp = gettimeofday(TRUE) + 1000; - $payload += [ - 'fid' => (string) ($fid ?? $this->media->id()), - 'uid' => (string) ($uid ?? $this->user->id()), - 'wri' => $write, - 'exp' => $expire_timestamp, - ]; - return $transcoder->encode($payload, $expire_timestamp); - } - - /** - * Asserts status code and content in a response given a request. - * - * @param array $expected_data - * The expected response JSON data. - * @param \Symfony\Component\HttpFoundation\Request $request - * The request to perform. - * @param string $message - * Message to distinguish this from other assertions. - */ - protected function assertJsonResponseOk(array $expected_data, Request $request, string $message = ''): void { - $this->assertJsonResponse(Response::HTTP_OK, $expected_data, $request, $message); - } - - /** - * Asserts a json response given a request. - * - * @param int $expected_code - * The expected response status code. - * @param array $expected_data - * The expected response JSON data. - * @param \Symfony\Component\HttpFoundation\Request $request - * The request to perform. - * @param string $message - * Message to distinguish this from other assertions. - */ - protected function assertJsonResponse(int $expected_code, array $expected_data, Request $request, string $message = ''): void { - $response = $this->handleRequest($request); - $this->assertEquals($expected_code, $response->getStatusCode(), $message); - $this->assertEquals('application/json', $response->headers->get('Content-Type'), $message); - $content = $response->getContent(); - $data = Json::decode($content); - $this->assertSame($expected_data, $data, $message); - } - - /** - * Asserts an access denied response given a request. - * - * @param \Symfony\Component\HttpFoundation\Request $request - * The request to perform. - * @param string $message - * Message to distinguish this from other assertions. - */ - protected function assertAccessDeniedResponse(Request $request, string $message = ''): void { - $this->assertResponse( - Response::HTTP_FORBIDDEN, - 'Authentication failed.', - 'text/plain', - $request, - $message, - ); - } - - /** - * Asserts status code and content in a response given a request. - * - * @param int $expected_code - * The expected response status code. - * @param string $expected_content - * The expected response content. - * @param string $expected_content_type - * The type of content of the response. - * @param \Symfony\Component\HttpFoundation\Request $request - * The request to perform. - * @param string $message - * Message to distinguish this from other assertions. - */ - protected function assertResponse(int $expected_code, string $expected_content, string $expected_content_type, Request $request, string $message = ''): void { - $response = $this->handleRequest($request); - - $this->assertEquals($expected_code, $response->getStatusCode(), $message); - $this->assertEquals($expected_content, $response->getContent(), $message); - $this->assertEquals($expected_content_type, $response->headers->get('Content-Type'), $message); - } - - /** - * Handles a request and gets the response. - * - * @param \Symfony\Component\HttpFoundation\Request $request - * Incoming request. - * - * @return \Symfony\Component\HttpFoundation\Response - * The response. - */ - protected function handleRequest(Request $request): Response { - /** @var \Symfony\Component\HttpKernel\HttpKernelInterface $kernel */ - $kernel = \Drupal::service('http_kernel'); - return $kernel->handle($request); - } - } diff --git a/tests/src/Kernel/Controller/WopiControllerTestBase.php b/tests/src/Kernel/Controller/WopiControllerTestBase.php new file mode 100644 index 00000000..ba5c2e59 --- /dev/null +++ b/tests/src/Kernel/Controller/WopiControllerTestBase.php @@ -0,0 +1,280 @@ +logger = new TestLogger(); + \Drupal::service('logger.factory')->addLogger($this->logger); + + $collabora_settings = \Drupal::configFactory()->getEditable('collabora_online.settings'); + $cool = $collabora_settings->get('cool'); + $cool['key_id'] = 'collabora'; + $collabora_settings->set('cool', $cool)->save(); + + // Make sure that ids for different entity types are distinguishable. + // This will reveal bugs where one id gets mixed up for another. + \Drupal::database()->query("ALTER TABLE {media} AUTO_INCREMENT = 1000"); + \Drupal::database()->query("ALTER TABLE {file_managed} AUTO_INCREMENT = 2000"); + + $this->media = $this->createMediaEntity('document'); + $this->user = $this->createUser([ + 'access content', + 'edit any document in collabora', + ]); + $fid = $this->media->getSource()->getSourceFieldValue($this->media); + $this->file = File::load($fid); + + $this->setCurrentUser($this->user); + } + + /** + * Creates WOPI requests for different routes, with some shared parameters. + * + * This can be used for tests where each route is expected to have the same + * response. + * + * @param int|null $media_id + * Media entity id, if different from the default. + * @param int|null $user_id + * User id, if different from the default. + * @param array $token_payload + * Explicit token payload values. + * This can be used to cause a bad token. + * + * @return array + * Requests keyed by a distinguishable name. + */ + protected function createRequests(?int $media_id = NULL, ?int $user_id = NULL, array $token_payload = []): array { + $create_request = fn (string $uri_suffix, string $method = 'GET', bool $write = FALSE) => $this->createRequest( + $uri_suffix, + $method, + $media_id, + $user_id, + $write, + $token_payload, + ); + return [ + 'info' => $create_request(''), + 'file' => $create_request('/contents'), + 'save' => $create_request('/contents', 'POST', TRUE), + ]; + } + + /** + * Creates a WOPI request. + * + * @param string $uri_suffix + * Suffix to append to the WOPI media url. + * @param string $method + * E.g. 'GET' or 'POST'. + * @param int|null $media_id + * Media entity id, if different from the default. + * @param int|null $user_id + * User id, if different from the default. + * @param bool $write + * TRUE if write access is requested. + * @param array $token_payload + * Explicit token payload values. + * This can be used to cause a bad token. + * + * @return \Symfony\Component\HttpFoundation\Request + * The request. + */ + protected function createRequest( + string $uri_suffix = '', + string $method = 'GET', + ?int $media_id = NULL, + ?int $user_id = NULL, + bool $write = FALSE, + array $token_payload = [], + ): Request { + $media_id ??= (int) $this->media->id(); + $user_id ??= (int) $this->user->id(); + $uri = '/cool/wopi/files/' . $media_id . $uri_suffix; + $token = $this->createAccessToken($media_id, $user_id, $write, $token_payload); + $parameters = [ + 'access_token' => $token, + 'access_token_ttl' => '0', + ]; + return Request::create($uri, $method, $parameters); + } + + /** + * Retrieves an encoded access token. + * + * @param int|null $fid + * The file id. + * @param int|null $uid + * The user id. + * @param bool $write + * The write permission. + * @param array $payload + * Explicit payload values. + * This can be used to cause a bad token. + * + * @return string + * The enconded token. + */ + protected function createAccessToken(?int $fid = NULL, ?int $uid = NULL, bool $write = FALSE, array $payload = []): string { + /** @var \Drupal\collabora_online\Jwt\JwtTranscoderInterface $transcoder */ + $transcoder = \Drupal::service(JwtTranscoderInterface::class); + $expire_timestamp = gettimeofday(TRUE) + 1000; + $payload += [ + 'fid' => (string) ($fid ?? $this->media->id()), + 'uid' => (string) ($uid ?? $this->user->id()), + 'wri' => $write, + 'exp' => $expire_timestamp, + ]; + return $transcoder->encode($payload, $expire_timestamp); + } + + /** + * Asserts a successful json response given a request. + * + * @param array $expected_data + * The expected response JSON data. + * @param \Symfony\Component\HttpFoundation\Request $request + * The request to perform. + * @param string $message + * Message to distinguish this from other assertions. + */ + protected function assertJsonResponseOk(array $expected_data, Request $request, string $message = ''): void { + $this->assertJsonResponse(Response::HTTP_OK, $expected_data, $request, $message); + } + + /** + * Asserts a json response given a request. + * + * @param int $expected_code + * The expected response status code. + * @param array $expected_data + * The expected response JSON data. + * @param \Symfony\Component\HttpFoundation\Request $request + * The request to perform. + * @param string $message + * Message to distinguish this from other assertions. + */ + protected function assertJsonResponse(int $expected_code, array $expected_data, Request $request, string $message = ''): void { + $response = $this->handleRequest($request); + $this->assertEquals($expected_code, $response->getStatusCode(), $message); + $this->assertEquals('application/json', $response->headers->get('Content-Type'), $message); + $content = $response->getContent(); + $data = Json::decode($content); + $this->assertSame($expected_data, $data, $message); + } + + /** + * Asserts an access denied response given a request. + * + * @param string $expected_response_message + * Message expected to be in the response. + * @param \Symfony\Component\HttpFoundation\Request $request + * The request to perform. + * @param string $assertion_message + * Message to distinguish this from other assertions. + */ + protected function assertAccessDeniedResponse(string $expected_response_message, Request $request, string $assertion_message = ''): void { + $this->assertResponse( + Response::HTTP_FORBIDDEN, + $expected_response_message, + 'text/plain', + $request, + $assertion_message, + ); + } + + /** + * Asserts status code and content in a response given a request. + * + * @param int $expected_code + * The expected response status code. + * @param string $expected_content + * The expected response content. + * @param string $expected_content_type + * The type of content of the response. + * @param \Symfony\Component\HttpFoundation\Request $request + * The request to perform. + * @param string $message + * Message to distinguish this from other assertions. + */ + protected function assertResponse(int $expected_code, string $expected_content, string $expected_content_type, Request $request, string $message = ''): void { + $response = $this->handleRequest($request); + + $this->assertEquals($expected_code, $response->getStatusCode(), $message); + $this->assertEquals($expected_content, $response->getContent(), $message); + $this->assertEquals($expected_content_type, $response->headers->get('Content-Type'), $message); + } + + /** + * Handles a request and gets the response. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * Incoming request. + * + * @return \Symfony\Component\HttpFoundation\Response + * The response. + */ + protected function handleRequest(Request $request): Response { + /** @var \Symfony\Component\HttpKernel\HttpKernelInterface $kernel */ + $kernel = \Drupal::service('http_kernel'); + return $kernel->handle($request); + } + +} diff --git a/tests/src/Unit/CollaboraDiscoveryTest.php b/tests/src/Unit/CollaboraDiscoveryTest.php new file mode 100644 index 00000000..b6959953 --- /dev/null +++ b/tests/src/Unit/CollaboraDiscoveryTest.php @@ -0,0 +1,152 @@ +getDiscoveryFromFile($file); + $this->assertSame( + 'http://collabora.test:9980/browser/61cf2b4/cool.html?', + $discovery->getWopiClientURL(), + ); + $this->assertSame( + 'http://spreadsheet.collabora.test:9980/browser/61cf2b4/cool.html?', + $discovery->getWopiClientURL('text/spreadsheet'), + ); + // Test unknown mime type. + $this->expectException(CollaboraNotAvailableException::class); + $this->expectExceptionMessage('The requested mime type is not handled.'); + $discovery->getWopiClientURL('text/unknown'); + } + + /** + * Tests reading proof keys from the discovery.xml. + * + * @covers ::getProofKey + * @covers ::getProofKeyOld + */ + public function testProofKey(): void { + $file = dirname(__DIR__, 2) . '/fixtures/discovery.proof-key.xml'; + $discovery = $this->getDiscoveryFromFile($file); + $this->assertSame( + 'BgIAAACkAABSU0ExAAgAAAEAAQCxJYuefceQ4XI3/iUQvL9+ebLFZSRdM1n6fkB+OtILJexHUsD/aItTWgzB/G6brdxLlyHXoPjbJl4QoWZVrr1XY+ZHQ/a9Yzf/VN2mPLKFB9hmnUPI580VpFfkC3gCgpqwFwMpAkQSzYSDFQ/7W4ryPP6irvVzhg16IqQ9oEhZWmwy6caKcqh4BK31oI8SrI6bsZLBMTli70197UWHmgIGk4JJbeC8cBFb6uZDaidAcRn1HSAF2JnaEscUNMIsiNMM/71BT6U6hVSv5Qk0oISMLfVOeCPQZ6OmYo4M42wDKBpaJGMOpgoeQX6Feq+agf7uBvd8S/ITGZ8WinQfHZaQ', + $discovery->getProofKey(), + ); + $this->assertSame( + 'BgIAAACkAABSU0ExAAgAAAEAAQDj9QjZQ9bOOw5LfAMxMLMDTLgHsNvBcdRpYQ8S9qK9ylJevgp+j66k9/uyKXSwI9WTVHW+XLTCPq6aId+XqB5e8+H5rov7e4Itkpnr6eXZ1jAu9TW2jEnqCYdGqG6Pv0kbRv1gUFEsjciy8i9UAQ12Ons7J58nQLd3tJ4WATANoCyVJLfA7BQ6IRSq8/K3jqmSE8xu3HDLX+lnMrsK2KL4lYcjerGZpmOKI5tPZbC5xSMkB9alE5NhTYeYw25CyG4FHoss2AwNgvSQDaf6d/icNg5ZoGQwtISGKL6IFc4oogFHFdvR4FQCQ61wdz7RmHjJUpsPFio8htuSeMjbC7fS', + $discovery->getProofKeyOld(), + ); + } + + /** + * Tests behavior if discovery.xml does not have proof keys. + * + * @covers ::getProofKey + * @covers ::getProofKeyOld + */ + public function testNoProofKey(): void { + $xml = ''; + $discovery = $this->getDiscoveryFromXml($xml); + $this->assertNull($discovery->getProofKey()); + $this->assertNull($discovery->getProofKeyOld()); + } + + /** + * Tests error behavior for blank xml content. + * + * @covers ::getWopiClientURL + * @covers ::getProofKey + * @covers ::getProofKeyOld + * + * @dataProvider provideAllMethodNames + */ + public function testBlankXml(string $method): void { + $discovery = $this->getDiscoveryFromXml(''); + + $this->expectException(CollaboraNotAvailableException::class); + $this->expectExceptionMessage('The discovery.xml file is empty.'); + $discovery->$method(); + } + + /** + * Tests error behavior for malformed xml content. + * + * @covers ::getWopiClientURL + * @covers ::getProofKey + * @covers ::getProofKeyOld + * + * @dataProvider provideAllMethodNames + */ + public function testBrokenXml(string $method): void { + $xml = 'This file does not contain valid xml.'; + $discovery = $this->getDiscoveryFromXml($xml); + + $this->expectException(CollaboraNotAvailableException::class); + $this->expectExceptionMessageMatches('#^Error in the retrieved discovery.xml file: #'); + $discovery->$method(); + } + + /** + * Provides all methods as callbacks. + * + * This is used for tests where each method will throw the same exception. + * + * @return list + * Argument tuples with method names. + */ + public static function provideAllMethodNames(): array { + return [ + 'getWopiClientURL' => ['getWopiClientURL'], + 'getProofKey' => ['getProofKey'], + 'getProofKeyOld' => ['getProofKeyOld'], + ]; + } + + /** + * Gets a discovery instance based on a test xml file. + * + * @param string $file + * A test xml file. + * + * @return \Drupal\collabora_online\Cool\CollaboraDiscoveryInterface + * Discovery instance. + */ + protected function getDiscoveryFromFile(string $file): CollaboraDiscoveryInterface { + $xml = file_get_contents($file); + return $this->getDiscoveryFromXml($xml); + } + + /** + * Gets a discovery instance based on test xml. + * + * @param string $xml + * Explicit XML content. + * + * @return \Drupal\collabora_online\Cool\CollaboraDiscoveryInterface + * Discovery instance. + */ + protected function getDiscoveryFromXml(string $xml): CollaboraDiscoveryInterface { + $fetcher = $this->createMock(CollaboraDiscoveryFetcherInterface::class); + $fetcher->method('getDiscoveryXml')->willReturn($xml); + return new CollaboraDiscovery($fetcher); + } + +}