From 6ce8521b640c7a69f97a2fd7c96de94eb9a83125 Mon Sep 17 00:00:00 2001
From: Alexander Larsson <alexl@redhat.com>
Date: Fri, 19 May 2017 17:01:46 +0200
Subject: [PATCH] document-portal: Add AddFull() operation

This allows you to add multiple paths at the same time, plus
grant an app access to it, plus it returns the fuse mount path.

This allows you to avoid a lot of roundtrip in common cases.
---
 data/org.freedesktop.portal.Documents.xml |  26 +++
 document-portal/Makefile.am.inc           |   1 +
 document-portal/xdp-enums.h               |   7 +
 document-portal/xdp-main.c                | 216 ++++++++++++++++++----
 tests/test-doc-portal.c                   |  84 +++++++++
 5 files changed, 301 insertions(+), 33 deletions(-)

diff --git a/data/org.freedesktop.portal.Documents.xml b/data/org.freedesktop.portal.Documents.xml
index 148e33d7..8d1f1f75 100644
--- a/data/org.freedesktop.portal.Documents.xml
+++ b/data/org.freedesktop.portal.Documents.xml
@@ -96,6 +96,32 @@
       <arg type='s' name='doc_id' direction='out'/>
     </method>
 
+    <!--
+        AddFull:
+        @o_path_fds: open file descriptors for the files to export
+        @flags: flags, 1 == reuse_existing, 2 == persistent
+        @app_id: an application ID, or empty string
+        @permissions: the permissions to grant, possible values are 'read', 'write', 'grant-permissions' and 'delete'
+        @doc_ids: the IDs of the files in the document store
+        @extra_info: Extra info returned
+        Adds multiple files to the document store. The file is passed in the
+        form of an open file descriptor to prove that the caller has
+        access to the file.
+        Additionally, if app_id is specified, it will be given the permissions
+        listed in GrantPermission.
+        The method also returns some extra info that can be used to avoid
+        multiple roundtrips. For now it only contains as "mountpoint", the
+        fuse mountpoint of the document portal.
+    -->
+    <method name="AddFull">
+      <arg type='ah' name='o_path_fds' direction='in'/>
+      <arg type='u' name='flags' direction='in'/>
+      <arg type='s' name='app_id' direction='in'/>
+      <arg type='as' name='permissions' direction='in'/>
+      <arg type='as' name='doc_ids' direction='out'/>
+      <arg type='a{sv}' name='extra_out' direction='out'/>
+    </method>
+
     <!--
         GrantPermissions:
         @doc_id: the ID of the file in the document store
diff --git a/document-portal/Makefile.am.inc b/document-portal/Makefile.am.inc
index c51a1acb..1a99b1ed 100644
--- a/document-portal/Makefile.am.inc
+++ b/document-portal/Makefile.am.inc
@@ -12,6 +12,7 @@ $(xdp_dbus_built_sources) : data/org.freedesktop.portal.Documents.xml
 		--c-namespace XdpDbus				\
 		--generate-c-code $(builddir)/document-portal/xdp-dbus	\
                 --annotate "org.freedesktop.portal.Documents.Add()" org.gtk.GDBus.C.UnixFD "yes" \
+                --annotate "org.freedesktop.portal.Documents.AddFull()" org.gtk.GDBus.C.UnixFD "yes" \
 		$(srcdir)/data/org.freedesktop.portal.Documents.xml	\
 		$(NULL)
 
diff --git a/document-portal/xdp-enums.h b/document-portal/xdp-enums.h
index 7a3d7433..b3f157e6 100644
--- a/document-portal/xdp-enums.h
+++ b/document-portal/xdp-enums.h
@@ -12,6 +12,13 @@ typedef enum {
   XDP_PERMISSION_FLAGS_ALL               = ((1 << 4) - 1)
 } XdpPermissionFlags;
 
+typedef enum {
+  XDP_ADD_FLAGS_REUSE_EXISTING             = (1 << 0),
+  XDP_ADD_FLAGS_PERSISTENT                 = (1 << 1),
+
+  XDP_ADD_FLAGS_FLAGS_ALL                  = ((1 << 2) - 1)
+} XdpAddFullFlags;
+
 G_END_DECLS
 
 #endif /* XDP_ENUMS_H */
diff --git a/document-portal/xdp-main.c b/document-portal/xdp-main.c
index a5976d7d..958216f7 100644
--- a/document-portal/xdp-main.c
+++ b/document-portal/xdp-main.c
@@ -452,6 +452,36 @@ validate_fd (int fd,
   return TRUE;
 }
 
+static char *
+verify_existing_document (struct stat *st_buf, gboolean reuse_existing)
+{
+  g_autoptr(FlatpakDbEntry) old_entry = NULL;
+  g_autofree char *id = NULL;
+
+  g_assert (st_buf->st_dev == fuse_dev);
+
+  /* The passed in fd is on the fuse filesystem itself */
+  id = xdp_fuse_lookup_id_for_inode (st_buf->st_ino);
+  g_debug ("path on fuse, id %s", id);
+  if (id == NULL)
+    return NULL;
+
+  /* Don't lock the db before doing the fuse call above, because it takes takes a lock
+     that can block something calling back, causing a deadlock on the db lock */
+  AUTOLOCK (db);
+
+  /* If the entry doesn't exist anymore, fail.  Also fail if not
+   * reuse_existing, because otherwise the user could use this to
+   * get a copy with permissions and thus escape later permission
+   * revocations
+   */
+  old_entry = flatpak_db_lookup (db, id);
+  if (old_entry == NULL || !reuse_existing)
+    return NULL;
+
+  return g_steal_pointer (&id);
+}
+
 static void
 portal_add (GDBusMethodInvocation *invocation,
             GVariant              *parameters,
@@ -490,10 +520,7 @@ portal_add (GDBusMethodInvocation *invocation,
   if (st_buf.st_dev == fuse_dev)
     {
       /* The passed in fd is on the fuse filesystem itself */
-      g_autoptr(FlatpakDbEntry) old_entry = NULL;
-
-      id = xdp_fuse_lookup_id_for_inode (st_buf.st_ino);
-      g_debug ("path on fuse, id %s", id);
+      id = verify_existing_document (&st_buf, reuse_existing);
       if (id == NULL)
         {
           g_dbus_method_invocation_return_error (invocation,
@@ -501,26 +528,6 @@ portal_add (GDBusMethodInvocation *invocation,
                                                  "Invalid fd passed");
           return;
         }
-
-      /* Don't lock the db before doing the fuse call above, because it takes takes a lock
-         that can block something calling back, causing a deadlock on the db lock */
-
-      AUTOLOCK (db);
-
-      /* If the entry doesn't exist anymore, fail.  Also fail if not
-       * reuse_existing, because otherwise the user could use this to
-       * get a copy with permissions and thus escape later permission
-       * revocations
-       */
-      old_entry = flatpak_db_lookup (db, id);
-      if (old_entry == NULL ||
-          !reuse_existing)
-        {
-          g_dbus_method_invocation_return_error (invocation,
-                                                 FLATPAK_PORTAL_ERROR, FLATPAK_PORTAL_ERROR_INVALID_ARGUMENT,
-                                                 "Invalid fd passed");
-          return;
-        }
     }
   else
     {
@@ -531,21 +538,18 @@ portal_add (GDBusMethodInvocation *invocation,
 
         if (app_id[0] != '\0')
           {
-            g_autoptr(FlatpakDbEntry) entry = NULL;
+            g_autoptr(FlatpakDbEntry) entry = flatpak_db_lookup (db, id);
             XdpPermissionFlags perms =
               XDP_PERMISSION_FLAGS_GRANT_PERMISSIONS |
               XDP_PERMISSION_FLAGS_READ |
               XDP_PERMISSION_FLAGS_WRITE;
-            {
-              entry = flatpak_db_lookup (db, id);
 
-              /* If its a unique one its safe for the creator to
-                 delete it at will */
-              if (!reuse_existing)
-                perms |= XDP_PERMISSION_FLAGS_DELETE;
+            /* If its a unique one its safe for the creator to
+               delete it at will */
+            if (!reuse_existing)
+              perms |= XDP_PERMISSION_FLAGS_DELETE;
 
-              do_set_permissions (entry, id, app_id, perms);
-            }
+            do_set_permissions (entry, id, app_id, perms);
           }
       }
 
@@ -559,6 +563,151 @@ portal_add (GDBusMethodInvocation *invocation,
                                          g_variant_new ("(s)", id));
 }
 
+static void
+portal_add_full (GDBusMethodInvocation *invocation,
+                 GVariant              *parameters,
+                 const char            *app_id)
+{
+  GDBusMessage *message;
+  GUnixFDList *fd_list;
+  char *id;
+  int fd_id, fd, fds_len;
+  char path_buffer[PATH_MAX + 1];
+  const int *fds = NULL;
+  struct stat st_buf;
+  gboolean reuse_existing, persistent;
+  GError *error = NULL;
+  guint32 flags = 0;
+  GKeyFile *app_info = g_object_get_data (G_OBJECT (invocation), "app-info");
+  g_autoptr(GVariant) array = NULL;
+  const char *target_app_id;
+  g_autofree const char **permissions = NULL;
+  g_autofree guint32 *arg_fds = NULL;
+  g_autoptr(GPtrArray) ids = g_ptr_array_new_with_free_func (g_free);
+  g_autoptr(GPtrArray) paths = g_ptr_array_new_with_free_func (g_free);
+  g_autofree struct stat *real_parent_st_bufs = NULL;
+  int i;
+  gsize n_args;
+  XdpPermissionFlags target_perms;
+  GVariantBuilder builder;
+
+  g_variant_get (parameters, "(@ahus^a&s)",
+                 &array, &flags, &target_app_id, &permissions);
+
+  if ((flags & ~XDP_ADD_FLAGS_FLAGS_ALL) != 0)
+    {
+      g_dbus_method_invocation_return_error (invocation,
+                                             FLATPAK_PORTAL_ERROR, FLATPAK_PORTAL_ERROR_INVALID_ARGUMENT,
+                                             "Invalid flags");
+      return;
+    }
+
+  reuse_existing = (flags & XDP_ADD_FLAGS_REUSE_EXISTING) != 0;
+  persistent = (flags & XDP_ADD_FLAGS_PERSISTENT) != 0;
+
+  target_perms = xdp_parse_permissions (permissions);
+
+  n_args = g_variant_n_children (array);
+  g_ptr_array_set_size (ids, n_args + 1);
+  g_ptr_array_set_size (paths, n_args + 1);
+  real_parent_st_bufs = g_new0 (struct stat, n_args);
+
+  message = g_dbus_method_invocation_get_message (invocation);
+  fd_list = g_dbus_message_get_unix_fd_list (message);
+  if (fd_list != NULL)
+    fds = g_unix_fd_list_peek_fds (fd_list, &fds_len);
+
+  for (i = 0; i < n_args; i++)
+    {
+      g_variant_get_child (array, i, "h", &fd_id);
+
+      fd = -1;
+      if (fds != NULL & fd_id < fds_len)
+        fd = fds[fd_id];
+
+      if (!validate_fd (fd, app_info, &st_buf, &real_parent_st_bufs[i], path_buffer, &error))
+        {
+          g_dbus_method_invocation_take_error (invocation, error);
+          return;
+        }
+
+      if (st_buf.st_dev == fuse_dev)
+        {
+          /* The passed in fd is on the fuse filesystem itself */
+          id = verify_existing_document (&st_buf, reuse_existing);
+          if (id == NULL)
+            {
+              g_dbus_method_invocation_return_error (invocation,
+                                                     FLATPAK_PORTAL_ERROR, FLATPAK_PORTAL_ERROR_INVALID_ARGUMENT,
+                                                     "Invalid fd passed");
+              return;
+            }
+          g_ptr_array_index(ids,i) = id;
+        }
+      else
+        {
+          g_ptr_array_index(paths,i) = g_strdup (path_buffer);
+        }
+    }
+
+  for (i = 0; i < n_args; i++)
+    {
+      AUTOLOCK (db);
+
+      if (g_ptr_array_index(ids,i) == NULL)
+        {
+          const char *path = g_ptr_array_index(paths,i);
+          g_assert (path != NULL);
+
+          id = do_create_doc (&real_parent_st_bufs[i], path, reuse_existing, persistent);
+          g_ptr_array_index(ids,i) = id;
+          if (app_id[0] != '\0' && strcmp (app_id, target_app_id) != 0)
+            {
+              g_autoptr(FlatpakDbEntry) entry = flatpak_db_lookup (db, id);;
+              XdpPermissionFlags perms =
+                XDP_PERMISSION_FLAGS_GRANT_PERMISSIONS |
+                XDP_PERMISSION_FLAGS_READ |
+                XDP_PERMISSION_FLAGS_WRITE;
+
+              /* If its a unique one its safe for the creator to
+                 delete it at will */
+              if (!reuse_existing)
+                perms |= XDP_PERMISSION_FLAGS_DELETE;
+
+              do_set_permissions (entry, id, app_id, perms);
+            }
+
+          if (target_app_id[0] != '\0' && target_perms != 0)
+            {
+              g_autoptr(FlatpakDbEntry) entry = flatpak_db_lookup (db, id);
+              do_set_permissions (entry, id, target_app_id, target_perms);
+            }
+        }
+    }
+
+  /* Invalidate with lock dropped to avoid deadlock */
+  for (i = 0; i < n_args; i++)
+    {
+      id = g_ptr_array_index (ids,i);
+      g_assert (id != NULL);
+
+      xdp_fuse_invalidate_doc_app (id, NULL);
+      if (app_id[0] != '\0')
+        xdp_fuse_invalidate_doc_app (id, app_id);
+      if (target_app_id[0] != '\0' && target_perms != 0)
+        xdp_fuse_invalidate_doc_app (id, target_app_id);
+    }
+
+  g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}"));
+  g_variant_builder_add (&builder, "{s@v}", "mountpoint",
+                         g_variant_new_variant (g_variant_new_string (xdp_fuse_get_mountpoint ())));
+
+  g_dbus_method_invocation_return_value (invocation,
+                                         g_variant_new ("(^as@a{sv})",
+                                                        (char **)ids->pdata,
+                                                        g_variant_builder_end (&builder)));
+}
+
 static void
 portal_add_named (GDBusMethodInvocation *invocation,
                   GVariant              *parameters,
@@ -878,6 +1027,7 @@ on_bus_acquired (GDBusConnection *connection,
   g_signal_connect_swapped (dbus_api, "handle-get-mount-point", G_CALLBACK (handle_get_mount_point), NULL);
   g_signal_connect_swapped (dbus_api, "handle-add", G_CALLBACK (handle_method), portal_add);
   g_signal_connect_swapped (dbus_api, "handle-add-named", G_CALLBACK (handle_method), portal_add_named);
+  g_signal_connect_swapped (dbus_api, "handle-add-full", G_CALLBACK (handle_method), portal_add_full);
   g_signal_connect_swapped (dbus_api, "handle-grant-permissions", G_CALLBACK (handle_method), portal_grant_permissions);
   g_signal_connect_swapped (dbus_api, "handle-revoke-permissions", G_CALLBACK (handle_method), portal_revoke_permissions);
   g_signal_connect_swapped (dbus_api, "handle-delete", G_CALLBACK (handle_method), portal_delete);
diff --git a/tests/test-doc-portal.c b/tests/test-doc-portal.c
index 5daa3b28..a6a220db 100644
--- a/tests/test-doc-portal.c
+++ b/tests/test-doc-portal.c
@@ -328,6 +328,89 @@ test_recursive_doc (void)
   g_assert_cmpstr (id, ==, id3);
 }
 
+static void
+test_create_docs (void)
+{
+  g_autofree char *doc_path = NULL;
+  g_autofree char *doc_app_path = NULL;
+  g_autofree char *host_path = NULL;
+  GError *error = NULL;
+  g_autofree char *path1 = NULL;
+  g_autofree char *path2 = NULL;
+  int fd1, fd2;
+  guint32 fd_ids[2];
+  GUnixFDList *fd_list = NULL;
+  gboolean res;
+  char **out_doc_ids;
+  g_autoptr(GVariant) out_extra = NULL;
+  const char *permissions[] = { "read", NULL };
+  const char *basenames[] = { "doc1", "doc2" };
+  int i;
+
+  if (!have_fuse)
+    {
+      g_test_skip ("this test requires FUSE");
+      return;
+    }
+
+  path1 = g_build_filename (outdir, basenames[0], NULL);
+  g_file_set_contents (path1, basenames[0], -1, &error);
+  g_assert_no_error (error);
+
+  fd1 = open (path1, O_PATH | O_CLOEXEC);
+  g_assert (fd1 >= 0);
+
+  path2 = g_build_filename (outdir, basenames[1], NULL);
+  g_file_set_contents (path2, basenames[1], -1, &error);
+  g_assert_no_error (error);
+
+  fd2 = open (path2, O_PATH | O_CLOEXEC);
+  g_assert (fd2 >= 0);
+
+  fd_list = g_unix_fd_list_new ();
+  fd_ids[0] = g_unix_fd_list_append (fd_list, fd1, &error);
+  g_assert_no_error (error);
+  close (fd1);
+  fd_ids[1] = g_unix_fd_list_append (fd_list, fd2, &error);
+  g_assert_no_error (error);
+  close (fd2);
+
+  res = xdp_dbus_documents_call_add_full_sync (documents,
+                                               g_variant_new_fixed_array (G_VARIANT_TYPE_HANDLE,
+                                                                          fd_ids, 2, sizeof (guint32)),
+                                               0,
+                                               "org.other.App",
+                                               permissions,
+                                               fd_list,
+                                               &out_doc_ids,
+                                               &out_extra,
+                                               NULL,
+                                               NULL, &error);
+  g_assert_no_error (error);
+  g_assert (res);
+
+  g_assert (g_strv_length (out_doc_ids) == 2);
+  for (i = 0; i < 2; i++)
+    {
+      const char *id = out_doc_ids[i];
+
+      /* Ensure its there and not viewable by apps */
+      assert_doc_has_contents (id, basenames[i], NULL, basenames[i]);
+      assert_host_has_contents (basenames[i], basenames[i]);
+      assert_doc_not_exist (id, basenames[i], "com.test.App1");
+      assert_doc_not_exist (id, basenames[i], "com.test.App2");
+      assert_doc_not_exist (id, "another-file", NULL);
+      assert_doc_not_exist ("anotherid", basenames[i], NULL);
+
+      assert_doc_has_contents (id, basenames[i], "org.other.App", basenames[i]);
+      update_doc (id, basenames[i], "org.other.App", "tmpdata2", &error);
+      g_assert_error (error, G_FILE_ERROR, G_FILE_ERROR_ACCES);
+      g_clear_error (&error);
+    }
+  g_assert (g_variant_lookup_value (out_extra, "mountpoint", G_VARIANT_TYPE_VARIANT) == 0);
+}
+
+
 static void
 global_setup (void)
 {
@@ -413,6 +496,7 @@ main (int argc, char **argv)
 
   g_test_add_func ("/db/create_doc", test_create_doc);
   g_test_add_func ("/db/recursive_doc", test_recursive_doc);
+  g_test_add_func ("/db/create_docs", test_create_docs);
 
   global_setup ();