Merge lp:~jeremywootten/pantheon-files/fix-1469122-show-user-bookmarks-when-administrator into lp:~elementary-apps/pantheon-files/trunk

[Jeremy Wootten]

* Show user bookmarks even when running as root
* Only show local folders in sidebar while root

This branch shows the normal user bookmarks even when running as root - as requested by linked bug.

In addition, the sidebar only shows local files - the Network section and any bookmark linked to a remote system are hidden.

[David Hewitt (davidmhewitt)]

Just a code review so far, I haven't tested the function. But I have two minor pieces of feedback that I've included in the diff comments.

review: Needs Fixing

[Jeremy Wootten (jeremywootten)]

David's suggestion implemented (thanks!).

Stop offering Trash bookmark and context menu option to root user because they will fail.

[David Hewitt (davidmhewitt)]

The suggested changes were made, and this seems to work as expected. Approved.

review: Approve

[RabbitBot (rabbitbot-a)]

Attempt to merge into lp:pantheon-files failed due to conflicts:

text conflict in src/View/Sidebar.vala

[Jeremy Wootten (jeremywootten)]

Trunk merged to r2563 and conflicts resolved.

[RabbitBot (rabbitbot-a)]

Attempt to merge into lp:pantheon-files failed due to conflicts:

text conflict in src/View/Window.vala

[lp:~jeremywootten/pantheon-files/fix-1469122-show-user-bookmarks-when-administrator]

1886. By Jeremy Wootten on 2017-06-04

Merge trunk to r2571 and resolve conflict; disambiguate a comment

[Adam Bieńkowski (donadigo)]

Works as expected. I did one comment on the diff, it is a minor thing though, nothing something critical.

review: Needs Fixing (testing / code)

[lp:~jeremywootten/pantheon-files/fix-1469122-show-user-bookmarks-when-administrator]

1887. By Jeremy Wootten on 2017-06-04

Assign config_dir only once in BookmarkList

[Jeremy Wootten (jeremywootten)]

Improved code style as recommended by donadigo.

[Adam Bieńkowski (donadigo)]

Looking good now.

review: Approve (code / testing)

=== modified file 'libcore/BookmarkList.vala'
--- libcore/BookmarkList.vala	2017-02-10 19:35:18 +0000
+++ libcore/BookmarkList.vala	2017-06-04 18:27:07 +0000
@@ -44,8 +44,20 @@
             list = new GLib.List<Marlin.Bookmark> ();
             pending_ops = new GLib.Queue<JobType> ();
 
+            /* Get the user config directory
+             * When running under pkexec determine real user from PKEXEC_UID
+             */
+            string? user_home = Eel.get_real_user_home ();
+            string config_dir;
+
+            if (user_home != null) {
+                config_dir = GLib.Path.build_filename (user_home, ".config");
+            } else {
+                config_dir = GLib.Environment.get_user_config_dir ();
+            }
+
             /*Check bookmarks file exists and in right place */
-            string filename = GLib.Path.build_filename (GLib.Environment.get_user_config_dir (),
+            string filename = GLib.Path.build_filename (config_dir,
                                                         "gtk-3.0",
                                                         "bookmarks",
                                                         null);
@@ -218,7 +230,8 @@
             }
             /* Do not insert bookmark for home or filesystem root (already have builtins) */
             var path = bm.gof_file.location.get_path ();
-            if ((path == Environment.get_home_dir () || path == Path.DIR_SEPARATOR_S)) {
+
+            if ((path == Eel.get_real_user_home () || path == Path.DIR_SEPARATOR_S)) {
                 return;
             }
 

=== modified file 'libcore/CMakeLists.txt'
--- libcore/CMakeLists.txt	2017-04-12 09:28:45 +0000
+++ libcore/CMakeLists.txt	2017-06-04 18:27:07 +0000
@@ -33,6 +33,7 @@
     gmodule-2.0
     libcanberra
     pantheon-files-core-C
+    posix
 OPTIONS
     --vapidir=${CMAKE_CURRENT_SOURCE_DIR}/
     --thread

=== modified file 'libcore/FileUtils.vala'
--- libcore/FileUtils.vala	2017-06-03 11:30:49 +0000
+++ libcore/FileUtils.vala	2017-06-04 18:27:07 +0000
@@ -203,7 +203,7 @@
         if (path.length > 0) {
             if (scheme == "" && path.has_prefix ("/~/")) {
                 sb.erase (0, 2);
-                sb.prepend (Environment.get_home_dir ());
+                sb.prepend (Eel.get_real_user_home ());
             }
         }
 
@@ -238,8 +238,8 @@
     public void split_protocol_from_path (string path, out string protocol, out string new_path) {
         protocol = "";
         new_path = path.dup ();
-
         string[] explode_protocol = new_path.split ("://");
+
         if (explode_protocol.length > 2) {
             new_path = "";
             return;

=== modified file 'libcore/eel-fcts.c'
--- libcore/eel-fcts.c	2017-02-10 19:35:18 +0000
+++ libcore/eel-fcts.c	2017-06-04 18:27:07 +0000
@@ -235,6 +235,34 @@
     return TRUE;
 }
 
+static const char*
+get_user_home_from_user_uid (uid_t uid)
+{
+    struct passwd *password_info;
+
+    password_info = getpwuid (uid);
+
+    if (password_info == NULL || password_info->pw_dir == NULL)
+        return NULL;
+
+    return password_info->pw_dir;
+}
+
+char*
+eel_get_real_user_home ()
+{
+    const char *real_uid_s;
+    int uid;
+
+    real_uid_s = g_environ_getenv (g_get_environ (), "PKEXEC_UID");
+    /* If running as administrator return the real user home, not root home */
+    if (real_uid_s != NULL && eel_get_id_from_digit_string (real_uid_s, &uid)) {
+        return g_strdup (get_user_home_from_user_uid ((uid_t)uid));
+    } else {
+        return g_strdup (g_get_home_dir ());
+    }
+}
+
 gboolean
 eel_get_id_from_digit_string (const char *digit_string, uid_t *id)
 {
@@ -249,7 +277,9 @@
     if (sscanf (digit_string, "%ld%c", &scanned_id, &c) != 1) {
         return FALSE;
     }
+
     *id = scanned_id;
+
     return TRUE;
 }
 

=== modified file 'libcore/eel-fcts.h'
--- libcore/eel-fcts.h	2017-02-10 19:35:18 +0000
+++ libcore/eel-fcts.h	2017-06-04 18:27:07 +0000
@@ -28,6 +28,7 @@
 GList       *eel_get_all_group_names (void);
 gboolean    eel_get_group_id_from_group_name (const char *group_name, uid_t *gid);
 gboolean    eel_get_user_id_from_user_name (const char *user_name, uid_t *uid);
+gchar*      eel_get_real_user_home ();
 gboolean    eel_get_id_from_digit_string (const char *digit_string, uid_t *id);
 
 gchar       *eel_format_size (guint64 size);

=== modified file 'libcore/gof-directory-async.vala'
--- libcore/gof-directory-async.vala	2017-06-03 11:30:49 +0000
+++ libcore/gof-directory-async.vala	2017-06-04 18:27:07 +0000
@@ -438,6 +438,9 @@
 
                 set_confirm_trash ();
 
+                /* Do not use root trash_dirs (Move to the Rubbish Bin option will not be shown) */
+                has_trash_dirs = has_trash_dirs && (Posix.getuid () != 0);
+
                 if (is_trash) {
                     connect_volume_monitor_signals ();
                 }
@@ -479,14 +482,16 @@
     }
 
     private static void toggle_ref_notify (void* data, Object object, bool is_last) {
+
         return_if_fail (object != null && object is Object);
+
         if (is_last) {
             Async dir = (Async) object;
             debug ("Async toggle_ref_notify %s", dir.file.uri);
 
-            if (!dir.removed_from_cache)
+            if (!dir.removed_from_cache) {
                 dir.remove_dir_from_cache ();
-
+            }
             dir.remove_toggle_ref ((ToggleNotify) toggle_ref_notify);
         }
     }

=== modified file 'libcore/pantheon-files-core-C.vapi'
--- libcore/pantheon-files-core-C.vapi	2017-02-04 12:33:41 +0000
+++ libcore/pantheon-files-core-C.vapi	2017-06-04 18:27:07 +0000
@@ -121,6 +121,8 @@
     [CCode (cheader_filename = "eel-fcts.h")]
     public bool get_user_id_from_user_name (string *user_name, out int uid);
     [CCode (cheader_filename = "eel-fcts.h")]
+    public string? get_real_user_home ();
+    [CCode (cheader_filename = "eel-fcts.h")]
     public bool get_group_id_from_group_name (string *group_name, out int gid);
     [CCode (cheader_filename = "eel-fcts.h")]
     public bool get_id_from_digit_string (string digit_str, out int id);

=== modified file 'libwidgets/Chrome/BreadcrumbIconList.vala'
--- libwidgets/Chrome/BreadcrumbIconList.vala	2017-02-10 19:35:18 +0000
+++ libwidgets/Chrome/BreadcrumbIconList.vala	2017-06-04 18:27:07 +0000
@@ -102,7 +102,7 @@
             }
 
             /* home */
-            dir = Environment.get_home_dir ();
+            dir = Eel.get_real_user_home ();
             if (dir.contains (Path.DIR_SEPARATOR_S)) {
                 BreadcrumbIconInfo icon = {dir, Marlin.ICON_GO_HOME_SYMBOLIC, false, null, null, dir.split (Path.DIR_SEPARATOR_S), true, null};
                 icon.exploded[0] = Path.DIR_SEPARATOR_S;

=== modified file 'src/Application.vala'
--- src/Application.vala	2017-06-03 12:06:11 +0000
+++ src/Application.vala	2017-06-04 18:27:07 +0000
@@ -361,10 +361,13 @@
             }
         }
         if (files == null) {
-            /* Restore session if settings allow */
-            if (!Preferences.settings.get_boolean ("restore-tabs") || window.restore_tabs () < 1) {
+            /* Restore session if not root and settings allow */
+            if (Posix.getuid () == 0 ||
+                !Preferences.settings.get_boolean ("restore-tabs") ||
+                window.restore_tabs () < 1) {
+
                 /* Open a tab pointing at the default location if no tabs restored*/
-                var location = File.new_for_path (Environment.get_home_dir ());
+                var location = File.new_for_path (Eel.get_real_user_home ());
                 window.add_tab (location, Marlin.ViewMode.PREFERRED);
             }
         } else {

=== modified file 'src/View/AbstractDirectoryView.vala'
--- src/View/AbstractDirectoryView.vala	2017-05-16 19:50:22 +0000
+++ src/View/AbstractDirectoryView.vala	2017-06-04 18:27:07 +0000
@@ -3407,11 +3407,17 @@
             int sort_column_id = 0;
             Gtk.SortType sort_order = 0;
 
+            /* Setting file attributes fails when root */
+            if (Posix.getuid () == 0) {
+                return;
+            }
+
             /* Ignore changes in model sort order while tree frozen (i.e. while still loading) to avoid resetting the
              * the directory file metadata incorrectly (bug 1511307).
              */
-            if (tree_frozen || !model.get_sort_column_id (out sort_column_id, out sort_order))
+            if (tree_frozen || !model.get_sort_column_id (out sort_column_id, out sort_order)) {
                 return;
+            }
 
             var info = new GLib.FileInfo ();
             var dir = slot.directory;

=== modified file 'src/View/Sidebar.vala'
--- src/View/Sidebar.vala	2017-06-03 11:30:49 +0000
+++ src/View/Sidebar.vala	2017-06-04 18:27:07 +0000
@@ -68,6 +68,7 @@
         bool internal_drag_started;
         bool dragged_out_of_window;
         bool renaming = false;
+        bool local_only;
 
         /* Identifiers for target types */
         public enum TargetType {
@@ -129,11 +130,13 @@
             }
         }
 
-        public Sidebar (Marlin.View.Window window) {
+        public Sidebar (Marlin.View.Window window, bool local_only = false) {
             init ();  /* creates the Gtk.TreeModel store. */
             this.last_selected_uri = null;
             this.set_policy (Gtk.PolicyType.NEVER, Gtk.PolicyType.AUTOMATIC);
             this.window = window;
+            this.local_only = local_only;
+
             window.loading_uri.connect (loading_uri_callback);
             window.free_space_change.connect (reload);
 
@@ -492,7 +495,7 @@
 
             /* Add Home BUILTIN */
             try {
-                mount_uri = GLib.Filename.to_uri (GLib.Environment.get_home_dir (), null);
+                mount_uri = GLib.Filename.to_uri (Eel.get_real_user_home (), null);
             }
             catch (ConvertError e) {
                 mount_uri = "";
@@ -533,24 +536,30 @@
             uint index;
             for (index = 0; index < bookmark_count; index++) {
                 bm = bookmarks.item_at (index);
-                if (bm == null
-                 || (bm.uri_known_not_to_exist () && !display_all_bookmarks))
+                if (bm == null ||
+                    (bm.uri_known_not_to_exist () && !display_all_bookmarks) ||
+                    (local_only && GLib.Uri.parse_scheme (bm.get_uri ()) != "file")) {
+
                     continue;
+                }
 
                 add_bookmark (iter, bm, index);
             }
 
-            /* Add trash */
-            add_place (Marlin.PlaceType.BUILT_IN,
-                       iter,
-                       _("Trash"),
-                       Marlin.TrashMonitor.get_icon (),
-                       Marlin.TRASH_URI,
-                       null,
-                       null,
-                       null,
-                       index + n_builtins_before,
-                       _("Open the Trash"));
+            /* Do not show Trash if running as root (cannot be loaded) */
+            if (Posix.getuid () != 0) {
+                /* Add trash */
+                add_place (Marlin.PlaceType.BUILT_IN,
+                           iter,
+                           _("Trash"),
+                           Marlin.TrashMonitor.get_icon (),
+                           Marlin.TRASH_URI,
+                           null,
+                           null,
+                           null,
+                           index + n_builtins_before,
+                           _("Open the Trash"));
+            }
 
             /* ADD STORAGE CATEGORY*/
             iter = add_category (Marlin.PlaceType.STORAGE_CATEGORY,
@@ -676,53 +685,54 @@
                 add_device_tooltip.begin (last_iter, root, update_cancellable);
             }
 
-            /* ADD NETWORK CATEGORY */
-
-            iter = add_category (Marlin.PlaceType.NETWORK_CATEGORY,
-                                 _("Network"),
-                                 _("Your network places"));
-
-            network_category_reference = new Gtk.TreeRowReference (store, store.get_path (iter));
-
-            /* Add network mounts */
-            network_mounts.reverse ();
-            foreach (Mount mount in network_mounts) {
-                root = mount.get_root ();
-                /* get_smb_share_from_uri will return the uri unaltered if does not have
-                 * the smb scheme so we need not test.  This is required because the mount
-                 * does not return the true root location of the share but the location used
-                 * when creating the mount.
-                 */
-                string uri = PF.FileUtils.get_smb_share_from_uri (root.get_uri ());
-
-                last_iter = add_place (Marlin.PlaceType.BUILT_IN,
-                                       iter,
-                                       mount.get_name (),
-                                       mount.get_icon (),
-                                       uri,
-                                       null,
-                                       null,
-                                       mount,
-                                       0,
-                                       null);
-
-                add_device_tooltip.begin (last_iter, root, update_cancellable);
+            if (!local_only) { /* Network operations fail when root */
+                /* ADD NETWORK CATEGORY */
+                iter = add_category (Marlin.PlaceType.NETWORK_CATEGORY,
+                                     _("Network"),
+                                     _("Your network places"));
+
+                network_category_reference = new Gtk.TreeRowReference (store, store.get_path (iter));
+
+                /* Add network mounts */
+                network_mounts.reverse ();
+                foreach (Mount mount in network_mounts) {
+                    root = mount.get_default_location ();
+                    /* get_smb_share_from_uri will return the uri unaltered if does not have
+                     * the smb scheme so we need not test.  This is required because the mount
+                     * does not return the true root location of the share but the location used
+                     * when creating the mount.
+                     */
+                    string uri = PF.FileUtils.get_smb_share_from_uri (root.get_uri ());
+
+                    last_iter = add_place (Marlin.PlaceType.BUILT_IN,
+                                           iter,
+                                           mount.get_name (),
+                                           mount.get_icon (),
+                                           uri,
+                                           null,
+                                           null,
+                                           mount,
+                                           0,
+                                           null);
+
+                    add_device_tooltip.begin (last_iter, root, update_cancellable);
+                }
+
+                /* Add Entire Network BUILTIN */
+                add_place (Marlin.PlaceType.BUILT_IN,
+                           iter,
+                           _("Entire Network"),
+                           new GLib.ThemedIcon (Marlin.ICON_NETWORK),
+                           "network:///",
+                           null,
+                           null,
+                           null,
+                           0,
+                           _("Browse the contents of the network"));
+
+                plugins.update_sidebar ((Gtk.Widget)this); /* Add "Connect Server plugin */
             }
 
-            /* Add Entire Network BUILTIN */
-            add_place (Marlin.PlaceType.BUILT_IN,
-                       iter,
-                       _("Entire Network"),
-                       new GLib.ThemedIcon (Marlin.ICON_NETWORK),
-                       "network:///",
-                       null,
-                       null,
-                       null,
-                       0,
-                       _("Browse the contents of the network"));
-
-            plugins.update_sidebar ((Gtk.Widget)this);
-
             expander_init_pref_state (tree_view);
 
             /* select any previously selected place or any place matching slot location */

=== modified file 'src/View/ViewContainer.vala'
--- src/View/ViewContainer.vala	2017-05-16 20:04:17 +0000
+++ src/View/ViewContainer.vala	2017-06-04 18:27:07 +0000
@@ -250,6 +250,7 @@
             view = null; /* Pre-requisite for add view */
             loading (false);
         }
+
         private void after_mode_change () {
             /* Slot is created inactive so we activate now since we must be the current tab
              * to have received a change mode instruction */

=== modified file 'src/View/Window.vala'
--- src/View/Window.vala	2017-06-03 11:30:49 +0000
+++ src/View/Window.vala	2017-06-04 18:27:07 +0000
@@ -153,7 +153,8 @@
         }
 
         private void construct_sidebar () {
-            sidebar = new Marlin.Places.Sidebar (this);
+            /* Show only local places in sidebar when running as root */
+            sidebar = new Marlin.Places.Sidebar (this, Posix.getuid () == 0);
         }
 
         public void show_sidebar (bool show = true) {
@@ -563,9 +564,10 @@
             tab.close ();
         }
 
-        public void add_window (File location = File.new_for_path (Environment.get_home_dir ()),
+        public void add_window (File location = File.new_for_path (Eel.get_real_user_home ()),
                                  Marlin.ViewMode mode = Marlin.ViewMode.PREFERRED,
                                  int x = -1, int y = -1) {
+
             ((Marlin.Application) application).create_window (location, real_mode (mode), x, y);
         }
 
@@ -666,7 +668,7 @@
                     break;
 
                 case "HOME":
-                    uri_path_change_request ("file://" + Environment.get_home_dir());
+                    uri_path_change_request ("file://" + Eel.get_real_user_home ());
                     break;
 
                 case "TRASH":
@@ -954,7 +956,7 @@
                 /* ViewContainer is responsible for returning valid uris */
                 vb.add ("(uss)",
                         view_container.view_mode,
-                        view_container.get_root_uri () ?? Environment.get_home_dir (),
+                        view_container.get_root_uri () ?? Eel.get_real_user_home (),
                         view_container.get_tip_uri () ?? ""
                        );
             }
@@ -984,8 +986,12 @@
             restoring_tabs = true;
 
             while (iter.next ("(uss)", out mode, out root_uri, out tip_uri)) {
-                if (mode < 0 || mode >= Marlin.ViewMode.INVALID || root_uri == null || root_uri == "" || tip_uri == null)
+
+                if (mode < 0 || mode >= Marlin.ViewMode.INVALID ||
+                    root_uri == null || root_uri == "" || tip_uri == null) {
+
                     continue;
+                }
 
                 /* We do not check valid location here because it may cause the interface to hang
                  * before the window appears (e.g. if trying to connect to a server that has become unavailable)
@@ -1105,7 +1111,7 @@
 
                 if (location == null || location.has_prefix (root) || location.equal (root)) {
                     if (view_container == current_tab)
-                        view_container.focus_location (File.new_for_path (Environment.get_home_dir ()));
+                        view_container.focus_location (File.new_for_path (Eel.get_real_user_home ()));
                     else
                         remove_tab (view_container);
                 }