Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Check remove server connectivity #4361

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat: Check remove server connectivity #4361

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions lib/Command/ActivateConfig.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,15 @@ protected function execute(InputInterface $input, OutputInterface $output): int
return 1;
}

try {
$this->connectivityService->testCallback($output);
} catch (\Throwable $e) {
// FIXME: Optional when allowing generic WOPI servers
$output->writeln('<error>Failed to fetch wopi');
$output->writeln($e->getMessage());
return 1;
}

// Summarize URLs for easier debugging

$output->writeln('');
Expand Down
2 changes: 2 additions & 0 deletions lib/Controller/SettingsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public function checkSettings(): DataResponse {
$output = new NullOutput();
$this->connectivityService->testDiscovery($output);
$this->connectivityService->testCapabilities($output);
$this->connectivityService->testCallback($output);
} catch (\Exception $e) {
$this->logger->error($e->getMessage(), ['exception' => $e]);
return new DataResponse([
Expand Down Expand Up @@ -181,6 +182,7 @@ public function setSettings(
$this->connectivityService->testDiscovery($output);
$this->connectivityService->testCapabilities($output);
$this->connectivityService->autoConfigurePublicUrl();
$this->connectivityService->testCallback($output);
} catch (\Throwable $e) {
return new JSONResponse([
'status' => 'error',
Expand Down
36 changes: 36 additions & 0 deletions lib/Service/ConnectivityService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
use Exception;
use OCA\Richdocuments\AppConfig;
use OCA\Richdocuments\WOPI\Parser;
use OCP\Http\Client\IClientService;
use OCP\IURLGenerator;
use Symfony\Component\Console\Output\OutputInterface;

class ConnectivityService {
Expand All @@ -17,6 +19,8 @@
private DiscoveryService $discoveryService,
private CapabilitiesService $capabilitiesService,
private Parser $parser,
private IClientService $clientService,
private IURLGenerator $urlGenerator,
) {
}

Expand Down Expand Up @@ -47,6 +51,38 @@
$output->writeln('<info>✓ Detected WOPI server: ' . $this->capabilitiesService->getServerProductName() . ' ' . $this->capabilitiesService->getProductVersion() . '</info>');
}

public function testCallback(OutputInterface $output): void {

Check failure on line 54 in lib/Service/ConnectivityService.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

UndefinedClass 

lib/Service/ConnectivityService.php:54:31: UndefinedClass: Class, interface or enum named Symfony\Component\Console\Output\OutputInterface does not exist (see https://psalm.dev/019)
$url = $this->parser->getUrlSrcValue('Capabilities');
if ($url === '') {
// Fixme can we skip early if the collabora version does not have the wopiAccessCheck endpoint, maybe it can be exposed in discovery
Copy link

@meven meven Dec 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was suggested will probably add to discovery.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be available with CollaboraOnline/online#10840 with hasWopiAccessCheck key in the /hosting/capabilities json answer.

return;
}

$url = str_replace('/hosting/capabilities', '/hosting/wopiAccessCheck', $url);

$callbackUrl = $this->urlGenerator->getAbsoluteURL('/status.php');

$client = $this->clientService->newClient();
try {
$response = $client->post($url, [
...RemoteOptionsService::getDefaultOptions(),
'body' => json_encode([
'callbackUrl' => $callbackUrl,
]),
'headers' => [
'Content-Type' => 'application/json',
],
]);
$result = json_decode($response->getBody(), true, 512, JSON_THROW_ON_ERROR);
if ($result['status'] === 'CheckStatus::WopiHostNotAllowed') {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ($result['status'] === 'CheckStatus::WopiHostNotAllowed') {
if ($result['status'] === 'WopiHostNotAllowed') {

I have simplified this to just be WopiHostNotAllowed in CollaboraOnline/online#10807

I am taking the time window until mid-january and next COOL stable version to make small API corrections before they are completely set in stone.
You can see the MR related at the end of CollaboraOnline/online#9202

throw new \Exception('WOPI host not allowed by Collabora');
}
} catch (Exception $e) {
throw $e;
}
$output->writeln('<info>✓ URL Callback ok</info>');
}

/**
* Detect public URL of the WOPI server for setting CSP on Nextcloud
*
Expand Down
Loading