Skip to content

Properly clean up configkey values from oc_appconfig table when disabling encryption#35756

Closed
pako81 wants to merge 6145 commits intomasterfrom
reactivating-encryption-fix
Closed

Properly clean up configkey values from oc_appconfig table when disabling encryption#35756
pako81 wants to merge 6145 commits intomasterfrom
reactivating-encryption-fix

Conversation

@pako81
Copy link
Copy Markdown

@pako81 pako81 commented Jul 3, 2019

Description

Currently, we have the problem that we do not correctly clean up the configkey values in the oc_appconfig table when disabling encryption.

Specifically the useMasterKey key for the encryption app, which stays set to 1. This seems to lead to the encryption_enabled key for core to remain set to no so when trying to reenable the encryption app and running encryption:select-encryption-type masterkey, this will fail.

The same has been observed for user-keys encryption. This PR fixes this behavior by properly cleaning up the configkey values and making re-enabling of encryption possible.

Related Issue

How Has This Been Tested?

  • Manually by first activating encryption by running: occ app:enable encryption, occ encryption:enable, occ encryption:select-encryption-type masterkey -y and encryption:encrypt-all

  • Then disable it with occ app:disable encryption, occ encryption:disable

  • Re-enabling encryption once again and check that when running occ encryption:select-encryption-type masterkey, no error is triggered.

Tested the same with user-keys encryption.

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:

Open tasks:

  • Backport (if applicable set "backport-request" label and remove when the backport was done)

phil-davis and others added 30 commits June 6, 2019 10:16
Make create user steps specify explicitly that the user has skeleton files
…th-tolowercase-inshare-api

Revert "Convert shareWith to lower case"
Remove deprecated default attributes test steps
Ignore share hooks for masterkey encryption
Send requests in loop rather than scenario outline to speed up CI
The recipient of internal share, should not be able
to increase the permission.

Signed-off-by: Sujith H <sharidasan@owncloud.com>
skip failing test on ldap and files_primary_s3
The recipient of internal share should not increase permission
…matching-file

api test for federation sharing when the receiver has matching file/folder
acceptance test for shares received from multiple users with the same name file/folder
Add no skeleton steps in webUIRestrictSharing
…abled

Fixup check_apache_module_enabled in run.sh
@pako81 pako81 self-assigned this Jul 3, 2019
@pako81 pako81 added this to the development milestone Jul 3, 2019
@karakayasemi
Copy link
Copy Markdown
Contributor

@pako81 you have some style errors, run make test-php-style-fix to get rid of from them. Also, some unit tests are failing you need to adjust them.

@pako81 pako81 force-pushed the reactivating-encryption-fix branch from 77d19e7 to bd3c517 Compare July 3, 2019 18:24
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 3, 2019

Codecov Report

Merging #35756 into master will decrease coverage by 16.72%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #35756       +/-   ##
=============================================
- Coverage     65.81%   49.09%   -16.73%     
=============================================
  Files          1228      109     -1119     
  Lines         70982    10535    -60447     
  Branches       1289     1289               
=============================================
- Hits          46716     5172    -41544     
+ Misses        23888     4985    -18903     
  Partials        378      378
Flag Coverage Δ Complexity Δ
#javascript 53.7% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 38.78% <ø> (-28.42%) 0 <ø> (-18818)
Impacted Files Coverage Δ Complexity Δ
lib/private/Files/Storage/DAV.php 59.45% <0%> (-21.64%) 0% <0%> (ø)
apps/updatenotification/templates/admin.php
lib/private/Encryption/Keys/Storage.php
lib/private/App/CodeChecker/NodeVisitor.php
lib/private/RedisFactory.php
apps/dav/lib/Avatars/AvatarNode.php
...s/dav/appinfo/Migrations/Version20170202213905.php
apps/dav/lib/Upload/ChunkLocationProvider.php
apps/files/lib/AppInfo/Application.php
apps/systemtags/list.php
... and 1109 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 367cc9d...bd3c517. Read the comment docs.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 3, 2019

Codecov Report

Merging #35756 into master will decrease coverage by 16.72%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #35756       +/-   ##
=============================================
- Coverage     65.81%   49.09%   -16.73%     
=============================================
  Files          1228      109     -1119     
  Lines         70982    10535    -60447     
  Branches       1289     1289               
=============================================
- Hits          46716     5172    -41544     
+ Misses        23888     4985    -18903     
  Partials        378      378
Flag Coverage Δ Complexity Δ
#javascript 53.7% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 38.78% <ø> (-28.42%) 0 <ø> (-18818)
Impacted Files Coverage Δ Complexity Δ
lib/private/Files/Storage/DAV.php 59.45% <0%> (-21.64%) 0% <0%> (ø)
apps/updatenotification/templates/admin.php
lib/private/Encryption/Keys/Storage.php
lib/private/App/CodeChecker/NodeVisitor.php
lib/private/RedisFactory.php
apps/dav/lib/Avatars/AvatarNode.php
...s/dav/appinfo/Migrations/Version20170202213905.php
apps/dav/lib/Upload/ChunkLocationProvider.php
apps/files/lib/AppInfo/Application.php
apps/systemtags/list.php
... and 1109 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 f38b42e...51f25cb. Read the comment docs.

@pako81 pako81 force-pushed the reactivating-encryption-fix branch from bd3c517 to 51f25cb Compare July 4, 2019 14:34
@micbar
Copy link
Copy Markdown
Contributor

micbar commented Jul 5, 2019

@pako81 We will work together on this test next week :-)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.