Repository navigation
Transfer ownership must work with master key #27427
Description
Transfer ownership should be possible because it doesn't require any user's specific password.
Currently it doesn't:
Steps:
cd tests/integration- Edit transfer-ownership.feature and remove "no_encryption" tags
sudo -u wwwrun OC_TEST_ENCRYPTION_MASTER_KEY_ENABLED=1 ./run.sh features/transfer-ownership.feature
Expected:
All tests passing.
Actual:
Transfer ownership refuses to work because it detects encryption.
- adjust
isReadyForUserto return true if master key is used - somehow detect that encrypted files are ok when master key is enabled instead of asking for decryption. This is a bit tricky because it requires calling non-specific encryption manager APIs.
Maybe add a method to the encryption modules to let them tell whether moving files between users can be done without their passwords ?
@pmaier1 will become more important when we start fading out the default encryption
Activity
PVince81 commentedon May 31, 2017
@sharidas please take care of this, thanks
sharidas commentedon May 31, 2017
Sure
butonic commentedon Jun 6, 2017
@pmaier1 @PVince81 we fade out the individual user key based encryption?
PVince81 commentedon Aug 7, 2017
While the merged PRs seem to work, there is a risk that it will not always work correctly in case the storage wrappers are setup differently. The problem is here: https://github.com/owncloud/core/pull/28107/files#diff-938b56891f3079e628cfda1c331911b7R366
This adds an argument to
fopenwhich at first look seemed fine but in hindsight I remembered that if the encryption wrapper is itself wrapped by anotherfopenwrapper, the latter will likely not forward the value to the encryption wrapper, so it would break.So far we are lucky that it works fine for transfer ownership but could break with third party apps.
I'm moving this ticket to "planned" now to look into other ways to pass around that value. This is quite tricky with the current architecture without resorting to globals...
PVince81 commentedon Sep 14, 2017
@sharidas please link to the PRs where you cleant up the tech debt mentioned above, then close.
sharidas commentedon Sep 14, 2017
The PR which addresses issue referred #27427 (comment) is here #28774 . This is another PR which got addressed #28820 and merged.
lock commentedon Aug 1, 2019
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.