Consider file to be encrypted only if it has encrypted property set

[VicDeo]

Description

Consider a file to be encrypted only if it has encrypted property set to true

Related Issue

https://github.com/owncloud/enterprise/issues/1739#issuecomment-393223048

Motivation and Context

It's not possible to download file that is encrypted externally

How Has This Been Tested?

0. Setup encryption on a separate instance
1. upload any encrypted file to another ownCloud instance where the encryption is not active
2. Try to download this file

Expected

File is downloaded

Actual

Web UI shows an error:

Forbidden
Encryption not ready: Module with id: OC_DEFAULT_MODULE does not exist.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Database schema changes (next release will require increase of minor version instead of patch)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:
• Changelog item, see TEMPLATE

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[codecov]

Codecov Report

Merging #36921 into master will increase coverage by 0.46%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master   #36921      +/-   ##
============================================
+ Coverage     64.29%   64.75%   +0.46%     
- Complexity    19135    19137       +2     
============================================
  Files          1270     1270              
  Lines         74872    74909      +37     
  Branches       1329     1329              
============================================
+ Hits          48137    48506     +369     
+ Misses        26344    26012     -332     
  Partials        391      391              

Flag	Coverage Δ	Complexity Δ
#javascript	54.17% <ø> (ø)	0.00 <ø> (ø)	⬆️
#phpunit	65.93% <100.00%> (+0.51%)	19137.00 <0.00> (+2.00)	⬇️

Impacted Files	Coverage Δ	Complexity Δ
apps/files/lib/Command/Scan.php	71.92% <0.00%> (-9.75%)	61.00% <0.00%> (ø%)
lib/private/Files/Cache/HomePropagator.php	77.77% <0.00%> (-9.73%)	3.00% <0.00%> (ø%)
lib/private/Files/View.php	84.83% <0.00%> (+0.29%)	389.00% <0.00%> (ø%)	⬆️
apps/dav/lib/Connector/Sabre/File.php	84.19% <0.00%> (+0.64%)	115.00% <0.00%> (ø%)	⬆️
lib/private/DB/Connection.php	67.64% <0.00%> (+2.20%)	49.00% <0.00%> (ø%)	⬆️
lib/private/Share/Share.php	71.15% <0.00%> (+2.20%)	501.00% <0.00%> (ø%)	⬆️
apps/files_versions/lib/Storage.php	72.39% <0.00%> (+2.53%)	95.00% <0.00%> (ø%)	⬆️
lib/private/legacy/util.php	73.59% <0.00%> (+2.55%)	219.00% <0.00%> (ø%)	⬆️
lib/private/Files/Storage/Common.php	85.01% <0.00%> (+3.36%)	138.00% <0.00%> (ø%)	⬆️
lib/private/Files/Storage/Wrapper/Encryption.php	69.43% <0.00%> (+4.57%)	158.00% <0.00%> (+2.00%)	⬇️
... and 18 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 01aedc0...baff6ec. Read the comment docs.

[jvillafanez]

$data = $this->storage->getCache()->get($path);
if (isset($data['encrypted']) && $data['encrypted'] === true) {
  $encryptionModule = $this->encryptionManager->getEncryptionModule($encryptionModuleId);
}

It seems a bit better to me. I'm not sure if we need the $targetIsEncrypted but we can assign it inside the condition if needed.

We might also need a comment with a little explanation, because accessing the storage cache could be expensive. I don't think we can avoid it at the moment, but maybe someone could have a better idea in the future.

[VicDeo]

@jvillafanez

accessing the storage cache could be expensive

True. But it happens only if we have found an encrypted header in the line 383
As for replacing $targetIsEncrypted with $encryptionModule - this var is matches it's content perfectly.
In addition it was not added by me and is used below this point several times.

[jvillafanez]

As for replacing $targetIsEncrypted with $encryptionModule - this var is matches it's content perfectly.

I didn't want to replace it. I just didn't see where the $targetIsEncrypted was being used

$data = $this->storage->getCache()->get($path);
if (isset($data['encrypted']) && $data['encrypted'] === true) {
  $targetIsEncrypted = true;
  $encryptionModule = $this->encryptionManager->getEncryptionModule($encryptionModuleId);
}

I can live with the above

[VicDeo]

thanks for the clarification. updated.