occ encryption:encrypt-all should not auto-enable user-key encryption

[mrow4a]

Description

With OC10.7 the user-key encyption has been deprecated. This PR makes it not possible to auto-enable user-key encryption when no module is selected when executing occ encryption:encrypt-all

Related Issue

• Implements https://doc.owncloud.com/docs/next/server_release_notes.html#deprecation-note-for-user-key-storage-encryption
• Implements https://github.com/owncloud/enterprise/issues/4939

Types of changes

• Technical debt

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket (not needed)
• Changelog item, see TEMPLATE

[pako81]

Makes sense - @mrow4a maybe this is also a good time to start thinking about removing user-key encryption as an option in the webUI.

[mrow4a]

Makes sense - @mrow4a maybe this is also a good time to start thinking about removing user-key encryption as an option in the webUI.

yes, I work on bigger change in encryption app to make it work. but not yet finished.

[jvillafanez]

I think it's better to point the user to what he should do instead of just saying "None of the encryption modules is enabled". You can also add something like "Set master encryption through the web UI" or something like that (not sure if it's possible through command line).

[mrow4a]

I think it's better to point the user to what he should do instead of just saying "None of the encryption modules is enabled". You can also add something like "Set master encryption through the web UI" or something like that (not sure if it's possible through command line).

it is core repo, this should not be aware of any encryption modules or even the apps that are used, right ? What I did here was to more maintain backwards compatibility with previous development here.

[phil-davis]

LGTM - I suppose that the encryption app should be the place that tries to guide admins if they enable the app but have not yet chosen the master or user key encryption.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

100.0% Coverage
0.0% Duplication

[jvillafanez]

it is core repo, this should not be aware of any encryption modules or even the apps that are used, right ? What I did here was to more maintain backwards compatibility with previous development here.

That's right. It shouldn't but now it is checking if user key or master key encryption is enabled. I'm ok if this is addressed in the future, otherwise the easiest solution is to adjust the error message (making the code module-agnostic will likely take a while)

[mrow4a]

@jvillafanez now I start thinking, that this part of the code was only to make sure that default encryption mode - use-key encryption - gets enabled by default. I think the best option is just to remove that block instead, as it was hacky anyways. If none of modules is enabled this explodes anyways.

[jvillafanez]

As long as it explodes with a meaningful message...

From what I understand, there is only one encryption module, which is the default one. "master key" and "user key" are options of such module, which won't be available in other modules.

I think a reasonable approach is to get the encryption module earlier, and show a message with the module id or displayname before starting doing anything. This way, we can give the encryption module a change of throwing an exception if it isn't properly configured (such as not having selected any mode in the default encryption module). There should be a proper method to check if the module is ready, but there isn't any and it's late to add one in the interface.

[DeepDiver1975]

related: owncloud/encryption#389

new installations will only run master key encryption ...