Transfer ownership path already shared #26523

Closed
PVince81 opened this Issue Nov 2, 2016 · 6 comments

Projects

None yet

1 participant

@PVince81
Collaborator
PVince81 commented Nov 2, 2016

Steps

  1. Create three users "user1", "user2" and "user3"
  2. Add "user2" and "user3" in "group1"
  3. Login as "user1"
  4. Create a folder "test"
  5. Share "test" with "group1" and "user2" and "user3"
  6. Run ./occ files:transfer-ownership user1 user2

Expected result

Resulting share: "user2" shares with "group1" and "user3".

Actual result

Analysing files of user1 ...
    1 [============================]
Collecting all share information for files and folder of user1 ...
    3 [============================]
Transferring files to user2/files/transferred from user1 on 2016-11-02T12:07:16+00:00 ...
Restoring shares ...
 1/3 [=========>------------------]  33%
                                      
  [Exception]                         
  Path already shared with this user  

Versions

Observed on OC 9.0.5, 9.0.6RC2, 9.1.1, 9.1.2RC2, 9.2 pre-alpha (master)

Seems it will need more fine-grained checks.

@DeepDiver1975 @owncloud/qa

@PVince81 PVince81 added this to the 9.1.3 milestone Nov 2, 2016
@PVince81
Collaborator
PVince81 commented Nov 2, 2016

also it seems that the share in question completely disappears as the original file/folder from "user1" was already deleted but the share wasn't moved properly
The share itself still has DB entries but those would get cleant up as part of cron's "clean up orphaned shares"

@PVince81
Collaborator
PVince81 commented Nov 2, 2016

Interestingly when rerunning the command, even though the owner has no files any more, it will still gather the stray shares and try to transfer them. Not sure if that could cause side effects considering that the files are gone...

@PVince81
Collaborator
PVince81 commented Nov 2, 2016

Okay, seems that the stray shares do point to the new files because when moving files we keep the fileid, so if the error can be solved then those stray shares should get migrated properly.

@PVince81
Collaborator
PVince81 commented Nov 2, 2016

I debugged this and the reason the error kicks in here https://github.com/owncloud/core/blob/v9.1.2RC2/lib/private/Share20/Manager.php#L382 is because we're iterating over the shares and we haven't moved all the shares to the new owner.

So what happens is that the validation code finds that the owner is different which makes it an invalid share. But if we'd first migrate the other share and then this one it might work.

Here are the two shares from the debugger:


// by property_get
$existingShare                   = (object|OC\Share20\Share[19]);
  $existingShare->id             = (string[1]) '1';
  $existingShare->providerId     = (string[10]) 'ocinternal';
  $existingShare->node           = (null);
  $existingShare->fileId         = (int) '31';
  $existingShare->nodeType       = (string[6]) 'folder';
  $existingShare->shareType      = (int) '1';
  $existingShare->sharedWith     = (string[6]) 'group1';
  $existingShare->sharedBy       = (string[5]) 'user1';
  $existingShare->shareOwner     = (string[5]) 'user1';
  $existingShare->permissions    = (int) '31';
  $existingShare->expireDate     = (null);
  $existingShare->password       = (null);
  $existingShare->token          = (null);
  $existingShare->parent         = (null);
  $existingShare->target         = (string[5]) '/test';
  $existingShare->shareTime      = (object|DateTime[3])+;
  $existingShare->mailSend       = (bool) '0';
  $existingShare->rootFolder     = (object|OC\Files\Node\LazyRoot[2])+;
  $existingShare->userManager    = (object|OC\User\Manager[4])+;


// by property_get
$share                           = (object|OC\Share20\Share[19]);
  $share->id                     = (string[1]) '3';
  $share->providerId             = (string[10]) 'ocinternal';
  $share->node                   = (object|OC\Files\Node\Folder[4])+;
  $share->fileId                 = (int) '31';
  $share->nodeType               = (string[6]) 'folder';
  $share->shareType              = (int) '0';
  $share->sharedWith             = (string[5]) 'user3';
  $share->sharedBy               = (string[5]) 'user2';
  $share->shareOwner             = (string[5]) 'user2';
  $share->permissions            = (int) '31';
  $share->expireDate             = (null);
  $share->password               = (null);
  $share->token                  = (null);
  $share->parent                 = (null);
  $share->target                 = (string[5]) '/test';
  $share->shareTime              = (object|DateTime[3])+;
  $share->mailSend               = (bool) '0';
  $share->rootFolder             = (object|OC\Files\Node\LazyRoot[2])+;
  $share->userManager            = (object|OC\User\Manager[4])+;
@PVince81
Collaborator
PVince81 commented Nov 2, 2016

I found a solution: change the order of share processing and first transfer the group shares and then the user shares.

@PVince81 PVince81 referenced this issue Nov 2, 2016
Merged

First process group shares and then user shares #26526

4 of 11 tasks complete
@PVince81
Collaborator
PVince81 commented Nov 2, 2016

Fix is here #26526

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