jhass / nextcloud-keeweb

Integrate Keeweb into Nextcloud
Other
415 stars 49 forks source link

Fixed Keeweb application for NC 28.x.x #231

Closed florian-forestier closed 10 months ago

florian-forestier commented 10 months ago

Hi,

This MR aim to fix #229 and #230.

The most important part of this MR is the viewer.js file. As Nextcloud removed the OCA.Files API, I had to use the new one. The original problem (CORS issue) was solved by adding $csp->addAllowedScriptDomain("'unsafe-inline'"); to lib/Controller/PageController.php. Tested on Nextcloud 28.0.1.

I also seen #221. Please note that I am interested in continuing your work on this Nextcloud plugin, as I use it on a daily basis, for personal and professional use.

I also also take the opportunity of this PR to thank you for the job you've done over the years on this project, having access to my keepass files directly on my Nextcloud saved me several times :smile:

Note: don't be affraid of the +6,515 added lines, 99.9% of them are in the package-lock.json file...

:wave:

arnowelzel commented 10 months ago

Thanks for your help, great!

Did you check, if that version still works with NC 27 as well? If not, we need to increase the minimum supported version to 28 as well and keep the older version for NC 27 and below unchanged.

florian-forestier commented 10 months ago

Hi,

It partially works for NC 27 (file association is broken due to changes in viewer.js). It would be better to increase the minimum supported version.

arnowelzel commented 10 months ago

Hi,

It partially works for NC 27 (file association is broken due to changes in viewer.js). It would be better to increase the minimum supported version.

Ok, thanks for the feedback. I'll change the supported version since NC 27 is still maintained and the older version works fine there.

arnowelzel commented 10 months ago

Hmm - it does not work here with NC 28 either :-(

Clicking a KDBX file will open Keeweb - but the file is not opened then.

Edit: the URL called then is https://myserver.example/apps/keeweb/?open=/test.kdbx - which is correct, since test.kdbx is in the main folder. But Keeweb does not open it - it will just present the start page without the file.

florian-forestier commented 10 months ago

Looking on it, I didn't had this problem on my previous test

arnowelzel commented 10 months ago

May it helps to know, that my server responds with HTTP 404 to the request, eventhough the file exists:

image

image

florian-forestier commented 10 months ago

Okay, it seems there is two issues:

The following patch should solve these issues:

diff --git a/keeweb/lib/Controller/PageController.php b/keeweb/lib/Controller/PageController.php
index 005ced5..847afac 100644
--- a/keeweb/lib/Controller/PageController.php
+++ b/keeweb/lib/Controller/PageController.php
@@ -87,6 +87,7 @@ class PageController extends Controller {
    public function config($file) {
        $csrfToken = \OC::$server->getCSRFTokenManager()->getToken()->getEncryptedValue();
        $webdavBase = \OCP\Util::linkToRemote('webdav');
+        $webdavBase = str_replace("http://", "https://", $webdavBase);
        $config = ['settings' => (object) null];
        if (isset($file)) {
            $config['files'] = [
@@ -136,6 +137,7 @@ class PageController extends Controller {
        $csp->addAllowedConnectDomain("'self'");
        $csp->addAllowedConnectDomain('https://services.keeweb.info');
        $csp->addAllowedScriptDomain('https://plugins.keeweb.info');
+        $csp->addAllowedScriptDomain("blob:");
        $csp->addAllowedScriptDomain("'unsafe-inline'");
        $csp->addAllowedConnectDomain('https://plugins.keeweb.info');
         $csp->allowEvalScript(true);

Tested on a fresh NC28 install. @arnowelzel can you try on your side and confirm ?

arnowelzel commented 10 months ago

Sorry - still does not work here. Same effect: KeeWeb is loaded but does not open the file. Server is using HTTPS (as it was before).

Same error in the console:

GET https://myserver.example/apps/keeweb/?open=/test.kdbx 404 (Not Found)

About this line - I don't think we need that. When the instance is not using HTTPS the resource can not be loaded anyway, since it will not be available using HTTPS:

$webdavBase = str_replace("http://", "https://", $webdavBase);
florian-forestier commented 10 months ago

I can't reproduce from my side on a fresh and a old Nextcloud server, using Firefox 115.6.0esr (normal & private browsing), and Chromium 120.0.6099.129 (normal & private browsing)... Also checked using Chrome and Brave on mobile (last version available on Play Store) without issue.

My server is deployed using the official docker image, with a Traefik acting as reverse-proxy. Can you give more information on how your Nextcloud is deployed, so I could create the same environment to check ?

arnowelzel commented 10 months ago

I don't use Docker but a plain server setup with NC 28 using Apache 2.4, PHP-FPM 8.3 and MariaDB.

To be sure I repeated the test with a fresh setup - and now it works. The replacement of http with https is not needed since KeeWeb would not work when loaded via HTTP anyway.

I will apply the change $csp->addAllowedScriptDomain("blob:"); and then prepare a new release.

arnowelzel commented 10 months ago

Now the release action does not work any longer :-(.

bin/release: line 65: hub: command not found
Error: Process completed with exit code 127.

@jhass Can you have a look what is missing? I can't update the workflow actions, just trigger them.

jhass commented 10 months ago

Github deprecated hub in favor of gh a while ago, I guess they removed the former from the GHA runner images now. I fixed the release script to use gh over hub.

Thanks guys!

arnowelzel commented 10 months ago

Thanks for the quick response and help!

bkraul commented 10 months ago

@florian-forestier is da real MVP. Thank you for this!