Add support for compiling without QtWebEngine (webflow / flow2 support) -- rebased

[theova]

A rebase of @sroracle's branch, see #1808 :

Fixes #932. I opted to make it default to Basic authentication in the absence of WebEngine but maybe it could be OAuth as well (maybe I can make that configurable too?)

[veddox]

A version that doesn't use the QtWebEngine would also solve #587

[er-vin]

Personally I'm not much in love with yet another build time option which will mean either another entry in our CI matrix which would need to be checked (unlikely) or a combination which will break after a while (more likely).

My understanding is that what pulls the QtWebEngineWidgets dependency really is the wizard bits, not really the flow2 auth itself. Redesigning that wizard step to not depend on QtWebEngineWidgets would look like a better path forward to me.

[veddox]

My understanding is that what pulls the QtWebEngineWidgets dependency really is the wizard bits, not really the flow2 auth itself. Redesigning that wizard step to not depend on QtWebEngineWidgets would look like a better path forward to me.

As far as I see, the wizard simply displays the server's standard login page, which is a) an easy option and b) preserves the server's theming, which is nice. How would you suggest changing that?

[bodograumann]

kubelogin opens oauth in the user’s webbrowser. Maybe a similar approach could be used here?

[er-vin]

My understanding is that what pulls the QtWebEngineWidgets dependency really is the wizard bits, not really the flow2 auth itself. Redesigning that wizard step to not depend on QtWebEngineWidgets would look like a better path forward to me.

As far as I see, the wizard simply displays the server's standard login page, which is a) an easy option and b) preserves the server's theming, which is nice. How would you suggest changing that?

Well, I guess we got some confusion to untangle... AFAIK the old webflow indeed displays the server's login page, so it'd be harder to replace. Flow2 shouldn't need it though since it opens the browser instead.

[theova]

I think it would be good to have an official statement whether:

• it is generally desirable to have an option to disable the Qt-webengine dependency at build time.
• this PR has a chance to be merged.

Personally I would like to see this PR merged, or otherwise there may be another way to get rid of the webengine dependency.

[er-vin]

Well, as I mentioned earlier in its current form I don't think it's going into the right direction. It's taking to much of an axe through the auth schemes. At least it should preserve the flow2 auth. I'd understand why it'd need to disable webflow (where the browser had to be in the client) but not flow2 (where it triggers an outside browser using QDesktopServices).

[maxcrees]

I'll take another look at flow2 when I get the chance then. I believe it was still pulling in webengine somehow, but I could be mistaken or that may no longer be the case.

[er-vin]

I'll take another look at flow2 when I get the chance then. I believe it was still pulling in webengine somehow, but I could be mistaken or that may no longer be the case.

You might be right that it still pulls webengine somehow, if that's the case I wonder why and I suspect it shouldn't so we'd better get that fixed.

[theova]

Allright, thanks for the feedback!

I see that src/gui/wizard/flow2authcredspage.cpp pulls #include "creds/webflowcredentials.h" and uses it for

AbstractCredentials *Flow2AuthCredsPage::getCredentials() const
{
    auto *ocWizard = qobject_cast<OwncloudWizard *>(wizard());
    Q_ASSERT(ocWizard);
    return new WebFlowCredentials(
                _user,
                _appPassword,
                ocWizard->_clientSslCertificate,
                ocWizard->_clientSslKey,
                ocWizard->_clientSslCaCertificates
    );
}

I'm not sure how to handle this...

Edit: I applied the commit 8460c8c to this branch.

[er-vin]

Allright, thanks for the feedback!

I see that src/gui/wizard/flow2authcredspage.cpp pulls #include "creds/webflowcredentials.h" and uses it for

AbstractCredentials *Flow2AuthCredsPage::getCredentials() const
{
    auto *ocWizard = qobject_cast<OwncloudWizard *>(wizard());
    Q_ASSERT(ocWizard);
    return new WebFlowCredentials(
                _user,
                _appPassword,
                ocWizard->_clientSslCertificate,
                ocWizard->_clientSslKey,
                ocWizard->_clientSslCaCertificates
    );
}

I'm not sure how to handle this...

Yes, well... it should never create the WebView in there. This credentials class is reused between webflow and flow2 because the credentials storage is the same between the two approaches. It should never get in any of the branches leading to the creation of a WebView though (this needs to be verified of course). In that case said branches can likely be ifndef'd, that should be the last bit linking to QtWebEngine in your case I think.

Once you get there I think you can reenable LoginFlow2 in networkjobs.cpp (I see it is still replaced with Basic but that shouldn't be necessary anymore and that's what I'm trying to avoid).

[theova]

Ok, I will check it once I find the time.

Everybody is welcome to contribute to this issue. Just ask for access to my branch.

[nextcloud-desktop-bot]

AppImage file: Nextcloud-PR-2204-1f577e539ac8d7894a8090a6d54abf88c3e37a26-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

[gilou]

Hi, the included appimage binary works in a vnc / xfce4 setup that wouldn't work otherwise in a similar setup as the one mentionned on #587 !

And so, it would be good to have a non-qtwebengine binary available more generally (or through compile option…)

[shuber2]

In my opinion this should be called something like WITH_WEBFLOW, because this is the feature to be enabled/disabled, with ON as default. The consequence of being enabled is that webengine is pulled in. Thinking as a Gentoo user the use-flag of the nextcloud-client would be something like webflow. A feature like webengine would suggest that there is an alternative, like qtwebkit.

[shuber2]

I just saw that there is also a NO_SHIBBOLETH and hence the negative-form NO_WEBFLOW should be preserved for consistency.

[FlexW]

@theova Are you still interested in that pull request? I think this needs some rebase as there have been quite some changes in that area. @er-vin @mgallien Maybe want to do a separate pr that removes the flow1 authentication? I think it's broken anyway. This would simplify that pr a bit.

[theova]

@theova Are you still interested in that pull request? I think this needs some rebase as there have been quite some changes in that area. @er-vin @mgallien Maybe want to do a separate pr that removes the flow1 authentication? I think it's broken anyway. This would simplify that pr a bit.

Thanks for asking. I don't have time to work on this issue. Maybe I'll come back to it. Then a separate pr removing flow2 would definitely help!

So this PR can be closed for me...

[FlexW]

@theova Thanks for your reply :) I was speaking of removing flow1, not flow2. flow2 is the mein authentication mechanism :)