Skip to content

reset encryptionVersion to '1' if a file was stream copied - #9018

Merged
schiessle merged 1 commit into
masterfrom
fix-copy-encrypted-files
Apr 3, 2018
Merged

reset encryptionVersion to '1' if a file was stream copied#9018
schiessle merged 1 commit into
masterfrom
fix-copy-encrypted-files

Conversation

@schiessle

@schiessleschiessle commented Mar 28, 2018

Copy link
Copy Markdown
Member

reset encryptionVersion to '1' if a file was stream copied, because this means that we basically write the file from scratch

Steps to test:

  • enable server side encryption
  • relogin
  • create a non-empty text file
  • copy the file to a sub folder with the copy operation of the web interface
  • try to open the copy (fails if the file was copied without this PR and works if the file was copied with this PR)

fix#8989

@rullzerrullzer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐘

@MorrisJobkeMorrisJobke added the 3. to review Waiting for reviews label Apr 3, 2018

@MorrisJobkeMorrisJobke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works 👍

@MorrisJobke

Copy link
Copy Markdown
Member

Unit tests fail:

1) Test\Files\Storage\Wrapper\EncryptionTest::testCopyBetweenStorage with data set #0 (true, true, true)
Expectation failed for method name is equal to "put" when invoked 1 time(s)
Parameter 1 for invocation OC\Files\Cache\Cache::put('file.txt', Array (...)) does not match expected value.
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
Array (
'encrypted' => true
- 'encryptedVersion' => 12345
+ 'encryptedVersion' => 1
/drone/src/github.com/nextcloud/server/lib/private/Files/Storage/Wrapper/Encryption.php:682
/drone/src/github.com/nextcloud/server/lib/private/Files/Storage/Wrapper/Encryption.php:761
/drone/src/github.com/nextcloud/server/tests/lib/TestCase.php:210
/drone/src/github.com/nextcloud/server/tests/lib/Files/Storage/Wrapper/EncryptionTest.php:814

@MorrisJobkeMorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Apr 3, 2018
…his means that we basically write the file from scratch
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
@schiessle
schiessleforce-pushed the fix-copy-encrypted-files branch from 47568ae to a0923d9CompareApril 3, 2018 16:01
@codecov

codecovBot commented Apr 3, 2018

Copy link
Copy Markdown

Codecov Report

Merging #9018 into master will decrease coverage by 16.93%.
The diff coverage is 0%.

@@ Coverage Diff @@## master #9018 +/- ##
=============================================
- Coverage 51.89% 34.95% -16.94% + Complexity 25282 25281 -1 
=============================================
Files 1601 1601 Lines 94959 94957 -2 Branches 1388 1388 =============================================
- Hits 49278 33196 -16082 - Misses 45681 61761 +16080
Impacted FilesCoverage ΔComplexity Δ
lib/private/Files/Storage/Wrapper/Encryption.php49.76% <0%> (-21.47%)144 <7> (+1)
apps/user_ldap/lib/GroupPluginManager.php0% <0%> (-100%)17% <0%> (ø)
core/Command/TwoFactorAuth/Disable.php0% <0%> (-100%)4% <0%> (ø)
.../Contacts/ContactsMenu/Providers/EMailProvider.php0% <0%> (-100%)4% <0%> (ø)
lib/public/RichObjectStrings/Definitions.php0% <0%> (-100%)2% <0%> (ø)
lib/private/App/AppStore/Version/Version.php0% <0%> (-100%)3% <0%> (ø)
...s/dav/lib/CalDAV/BirthdayCalendar/EnablePlugin.php0% <0%> (-100%)7% <0%> (ø)
...ware/Security/Exceptions/NotConfirmedException.php0% <0%> (-100%)1% <0%> (ø)
...curity/Exceptions/StrictCookieMissingException.php0% <0%> (-100%)1% <0%> (ø)
apps/files/lib/Activity/Settings/FileDeleted.php0% <0%> (-100%)8% <0%> (ø)
... and 522 more

@schiessleschiessle added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Apr 3, 2018
@schiessle

Copy link
Copy Markdown
MemberAuthor

test should be fixed now... if all tests passed this can be merged

@schiessle
schiessle merged commit c3900c9 into masterApr 3, 2018
@schiessle
schiessle deleted the fix-copy-encrypted-files branch April 3, 2018 16:24
@MorrisJobke

Copy link
Copy Markdown
Member

@schiessle Please also remove the backport-request label once you opened the backport PR ;)

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to releaseReady to be released and/or waiting for tests to finishbugfeature: encryption (server-side)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

copying shared files on server-side-encrypted setup broken

3 participants

@schiessle@MorrisJobke@rullzer