Merge lp:~mterry/deja-dup/gio-mount-password into lp:deja-dup/22

[Michael Terry]

If duplicity is run under sudo (as happens when non-home data is backed up), it can't use the user's remote mount. So it previously just aborted. This branch works around that.

To test:
 * Setup Deja Dup like so:
   - Include "/bin"
   - Make your backup location a remote GVFS location that needs a password, like an FTP server or SSH server
 * Make a backup
 * Try to restore (./deja-dup/deja-dup --restore)

Before the patch:
 * You should get prompted for your root password and it will fail with "Connection failed, please check your password: Login dialog cancelled"

After the patch:
 * You should get *not* get prompted for your root password and it will restore, even though nothing got restored (this is a bug -- I need to make it warn about all the files that didn't get restored)

[Michael Terry (mterry)]

Hrm, this needs some more thinking. If a user mounts the location first, but does not save the mount info in the keyring, we have no way to get to it. Seems awkward that we're being punished for using the standard GNOME mount infrastructure, but there ya go.

[Michael Terry (mterry)]

This is still a work in progress because duplicity doesn't currently report permission errors when restoring... This yak is big.

[Ken VanDine (ken-vandine)]

The diff looks OK to me, however following your steps to test this I got slightly different results. I chose a remote storage location, SSH and backed up /TEST which was only writtable by root. I removed all the files in /TEST and attempted a restore. I didn't get prompted for a password and it reported a successful restore with no warnings. However, no files were actually restored.

[Michael Terry (mterry)]

Oh! I thought I edited the description of this merge to change what I expected. What you describe is what I expect. :)

The big difference in this branch is just not asking for a password. The restore will have no effect, but will say it succeeded (which is a whole 'nother problem).

[Ken VanDine (ken-vandine)]

OK, it looks good then. Certainly an improvement over the previous behavior :)

review: Approve

=== modified file 'common/Backend.vala'
--- common/Backend.vala	2012-03-21 02:54:21 +0000
+++ common/Backend.vala	2012-03-21 03:04:22 +0000
@@ -31,7 +31,7 @@
   public abstract bool is_native(); // must be callable when nothing is mounted, nothing is prepared
   public virtual Icon? get_icon() {return null;}
 
-  public abstract string get_location();
+  public abstract string get_location(ref bool as_root);
   public abstract string get_location_pretty(); // short description for user
 
   public virtual async bool is_ready(out string when) {when = null; return true;} // must be callable when nothing is mounted, nothing is prepared

=== modified file 'common/BackendAuto.vala'
--- common/BackendAuto.vala	2012-03-21 02:54:21 +0000
+++ common/BackendAuto.vala	2012-03-21 03:04:22 +0000
@@ -40,7 +40,7 @@
     return false;
   }
 
-  public override string get_location() {
+  public override string get_location(ref bool as_root) {
     return "invalid";
   }
 

=== modified file 'common/BackendFile.vala'
--- common/BackendFile.vala	2012-03-21 02:54:21 +0000
+++ common/BackendFile.vala	2012-03-21 03:04:22 +0000
@@ -64,9 +64,52 @@
   }
 
   // Location will be mounted by this time
-  public override string get_location()
+  public override string get_location(ref bool as_root)
   {
     var file = get_file_from_settings();
+
+    if (as_root && !file.is_native()) {
+      // OK...  Root can't use GVFS URIs as-is because it would need access to
+      // our GVFS mounts which are only available on our session dbus.  Which
+      // root can't talk to.  Possible workarounds:
+      //  * Some magic to let root talk to our gvfs daemons (haven't found yet)
+      //  * Use FUSE local paths (root also isn't given access to these mounts)
+      //  * Have duplicity notice that it needs root to write to a file, and
+      //    then restart itself under sudo.  But then we'd just hit the same
+      //    problem again but now duplicity has to solve it...
+      //  * Restore to a temporary folder and move files over with sudo.  This
+      //    is what we used to always do in older deja-dup.  But it had
+      //    several problems with consuming the hard drive, especially if the
+      //    user had partitioned in ways we didn't expect.  Hard to know where
+      //    a safe spot is to hoard all the files.
+      //  * Pass mount username/password to duplicity as environment variables
+      //    and have root do the mount itself.  This could work...  if we had
+      //    a reliable way to get the username/password.  We could get it from
+      //    keyring (even then, only a guess, since the daemon could have set
+      //    the 'object' or 'authtype' fields, which we don't know if it did)
+      //    or from a MountOperation.  But a user could have mounted it earlier
+      //    in session without saving password in keyring.  And we can't force
+      //    an unmount on the user just so we can remount it.
+      //  * Have duplicity try to mount and ask user for password.  We'd need
+      //    to add functionality to duplicity to allow a conversation between a
+      //    driving app like deja-dup and itself, to be able to proxy these
+      //    prompts and questions to the user.  This would work nicely, but is
+      //    a very different interaction model than duplicity uses today.
+      //    Much more deja-dup-focused.  If we're going down this direction,
+      //    there are all sorts of two-way interactions that we could stand to
+      //    benefit from.  Would require a deep rethink of our driving model.
+      //
+      // So in the absence of an actually good solution, we'll just disable
+      // running under sudo if the location is remote.  :(  Maybe our
+      // over-eager needs-root algorithm got it wrong anyway. Regardless, this
+      // way the user will get a permissions denied error that will point them
+      // in the direction of trying to restore in a new folder rather than on
+      // top of their running system, which, let's be honest, is probably not
+      // a good idea anyway.  BTW, where does Napolean keep his armies?
+      // In his sleevies!
+      as_root = false;
+    }
+
     return file.get_uri();
   }
 

=== modified file 'common/BackendRackspace.vala'
--- common/BackendRackspace.vala	2012-03-21 02:54:21 +0000
+++ common/BackendRackspace.vala	2012-03-21 03:04:22 +0000
@@ -50,7 +50,7 @@
     return yield Network.get().can_reach ("http://%s/".printf(RACKSPACE_SERVER));
   }
 
-  public override string get_location()
+  public override string get_location(ref bool as_root)
   {
     var settings = get_settings(RACKSPACE_ROOT);
     var container = get_folder_key(settings, RACKSPACE_CONTAINER_KEY);

=== modified file 'common/BackendS3.vala'
--- common/BackendS3.vala	2012-03-21 02:54:21 +0000
+++ common/BackendS3.vala	2012-03-21 03:04:22 +0000
@@ -60,7 +60,7 @@
     return yield Network.get().can_reach ("http://%s/".printf(S3_SERVER));
   }
 
-  public override string get_location()
+  public override string get_location(ref bool as_root)
   {
     var settings = get_settings(S3_ROOT);
     

=== modified file 'common/BackendU1.vala'
--- common/BackendU1.vala	2012-03-21 02:54:21 +0000
+++ common/BackendU1.vala	2012-03-21 03:04:22 +0000
@@ -151,7 +151,7 @@
     return yield Network.get().can_reach ("https://one.ubuntu.com/");
   }
 
-  public override string get_location()
+  public override string get_location(ref bool as_root)
   {
     var settings = get_settings(U1_ROOT);
     var folder = get_folder_key(settings, U1_FOLDER_KEY);

=== modified file 'common/Duplicity.vala'
--- common/Duplicity.vala	2012-03-21 02:54:21 +0000
+++ common/Duplicity.vala	2012-03-21 03:04:22 +0000
@@ -82,7 +82,6 @@
   
   DuplicityInstance inst;
   
-  string remote;
   List<string> backend_argv;
   List<string> saved_argv;
   List<string> saved_envp;
@@ -166,7 +165,6 @@
   {
     // save arguments for calling duplicity again later
     mode = original_mode;
-    this.remote = backend.get_location();
     this.backend = backend;
     saved_argv = new List<string>();
     saved_envp = new List<string>();
@@ -206,6 +204,11 @@
       return 0;
   }
 
+  string get_remote ()
+  {
+    return backend.get_location(ref needs_root);
+  }
+
   void expand_links_in_file(File file, ref List<File> all, bool include, List<File>? seen = null)
   {
     // For symlinks, we want to add the link and its target to the list.
@@ -558,7 +561,7 @@
     var cleanup_argv = new List<string>();
     cleanup_argv.append("cleanup");
     cleanup_argv.append("--force");
-    cleanup_argv.append(this.remote);
+    cleanup_argv.append(get_remote());
     
     set_status(_("Cleaning up…"));
     connect_and_start(null, null, cleanup_argv);
@@ -572,7 +575,7 @@
     argv.append("remove-all-but-n-full");
     argv.append("%d".printf(cutoff));
     argv.append("--force");
-    argv.append(this.remote);
+    argv.append(get_remote());
     
     set_status(_("Cleaning up…"));
     connect_and_start(null, null, argv);
@@ -1013,8 +1016,7 @@
     case "S3CreateError":
       if (text.contains("<Code>BucketAlreadyExists</Code>")) {
         if (((BackendS3)backend).bump_bucket()) {
-          remote = backend.get_location();
-          if (restart())
+          if (restart()) // get_remote() will eventually grab new bucket name
             return;
         }
         
@@ -1375,21 +1377,21 @@
           argv.prepend("full");
         argv.append("--volsize=%d".printf(get_volsize()));
         argv.append(local_arg.get_path());
-        argv.append(remote);
+        argv.append(get_remote());
         break;
       case Operation.Mode.RESTORE:
         argv.prepend("restore");
         argv.append("--force");
-        argv.append(remote);
+        argv.append(get_remote());
         argv.append(local_arg.get_path());
         break;
       case Operation.Mode.STATUS:
         argv.prepend("collection-status");
-        argv.append(remote);
+        argv.append(get_remote());
         break;
       case Operation.Mode.LIST:
         argv.prepend("list-current-files");
-        argv.append(remote);
+        argv.append(get_remote());
         break;
       }
     }