E2EE. Do not generate keypair without user request.

[allexzander]

Signed-off-by: allexzander blackslayer4@gmail.com

Mnemonic is now displayed in mono font and text can't be typed in the dialog (readonly).
Keys are not generated automatically (see the video), but only after the user clicks "Enable encryption".

e2ee_enable_display_mnemonic_reconnect.mp4

[allexzander]

@jancborchardt see the description for the screenshot, video, and explanation of how this now works. Does it look good?

[jancborchardt]

Nice! Some feedback:

• The dialog takes a bit long to appear after clicking "Enable encryption", can that be sped up?
• Can the dialog be a child of the settings dialog, like a modal overlay centered on top instead of yet another dialog?
• The input still looks editable, is there no default grey background or " disabled" style?
• The input is too high, can it be sized according to content?

[allexzander]

@jancborchardt I have fixed the 3-rd and 4-th remarks.

Regarding the first too.

The dialog takes a bit long to appear after clicking "Enable encryption", can that be sped up?

I can't control this as this is not a UI-caused delay. The desktop clients talk to the server and then to a Windows credentials store to generate, validate and store new keypair. Before, we had this happening in the background because it was always being triggered automatically, without the user clicking that "Enable encryption button". Now, as the user initiates the E2E keys generation, the delay is visible.

Can the dialog be a child of the settings dialog, like a modal overlay centered on top instead of yet another dialog?

It is already modal. You can't close the Settings dialog or, otherwise, interact with it without closing the mnemonic dialog.

[jancborchardt]

I can't control this as this is not a UI-caused delay. The desktop clients talk to the server and then to a Windows credentials store to generate, validate and store new keypair.

Thank you for the explanation! :) So could we show the dialog already, but show a spinner / loading message inside the input saying "Generating mnemonic"?

[codecov]

Codecov Report

Merging #5067 (7ad4c72) into master (1dbdd88) will decrease coverage by 0.13%.
The diff coverage is 0.00%.

❗ Current head 7ad4c72 differs from pull request most recent head 9ab89da. Consider uploading reports for the commit 9ab89da to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5067      +/-   ##
==========================================
- Coverage   57.22%   57.09%   -0.14%     
==========================================
  Files         138      138              
  Lines       17441    17444       +3     
==========================================
- Hits         9980     9959      -21     
- Misses       7461     7485      +24     

Impacted Files	Coverage Δ
src/libsync/account.cpp	37.15% <0.00%> (-0.44%)	⬇️
src/libsync/account.h	33.33% <ø> (ø)
src/libsync/clientsideencryption.cpp	26.40% <0.00%> (+0.07%)	⬆️
src/libsync/clientsideencryption.h	38.46% <ø> (ø)
src/libsync/vfs/cfapi/hydrationjob.cpp	52.38% <0.00%> (-3.71%)	⬇️
src/libsync/vfs/cfapi/vfs_cfapi.cpp	83.00% <0.00%> (-2.38%)	⬇️
src/libsync/vfs/cfapi/cfapiwrapper.cpp	72.50% <0.00%> (-1.82%)	⬇️
src/libsync/syncengine.cpp	83.71% <0.00%> (-0.46%)	⬇️
src/libsync/discovery.cpp	84.05% <0.00%> (+0.28%)	⬆️

[allexzander]

I can't control this as this is not a UI-caused delay. The desktop clients talk to the server and then to a Windows credentials store to generate, validate and store new keypair.

Thank you for the explanation! :) So could we show the dialog already, but show a spinner / loading message inside the input saying "Generating mnemonic"?

So, we've discussed that with Tobias and this thing is turning into a new feature and is a bit out of the scope of this PR. There is also a plan to enhance the UX of E2E, and then this can be done alongside.

[mgallien]

can you make it a property registered to Qt metaobject system ?

[mgallien]

please fix this and clean history of commits if needed
that said, I approve it works fine !
congratulations

[nextcloud-desktop-bot]

AppImage file: nextcloud-PR-5067-9ab89daedd867465e6c86fd420994f54a03fbee4-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.

[sonarqubecloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
17 Code Smells

0.0% Coverage
0.0% Duplication

[allexzander]

/backport to stable-3.6