From a2f57f64fda112938d86de032cd1f1e8d29ca376 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Tue, 4 Dec 2018 10:48:46 -0500 Subject: [PATCH] Add flags that allow to 'upgrade' permissions This is to avoid multiple polkit dialogs, regardless of transaction ordering. FlatpakTransaction calculates the 'strongest' op it has, and passes the hints accordingly. FlatpakInstallation doesn't pass hints, since it does individual operations. The system helper uses the hints to determine which PolicyKit permission to request. Since the policy typically has 'keep' set, this mean that the following operations in the same transaction will be able to reuse the permission obtained for the first one. Closes: #2384 Approved by: alexlarsson --- common/flatpak-dir-private.h | 9 ++++++++- common/flatpak-dir.c | 14 ++++++++++++++ common/flatpak-installation.c | 4 ++-- common/flatpak-transaction.c | 24 ++++++++++++++++++++++++ system-helper/flatpak-system-helper.c | 16 ++++++++++++++++ 5 files changed, 64 insertions(+), 3 deletions(-) diff --git a/common/flatpak-dir-private.h b/common/flatpak-dir-private.h index bd8a80b3..58ff3962 100644 --- a/common/flatpak-dir-private.h +++ b/common/flatpak-dir-private.h @@ -151,13 +151,17 @@ typedef enum { FLATPAK_HELPER_DEPLOY_FLAGS_LOCAL_PULL = 1 << 2, FLATPAK_HELPER_DEPLOY_FLAGS_REINSTALL = 1 << 3, FLATPAK_HELPER_DEPLOY_FLAGS_NO_INTERACTION = 1 << 4, + FLATPAK_HELPER_DEPLOY_FLAGS_APP_HINT = 1 << 5, + FLATPAK_HELPER_DEPLOY_FLAGS_INSTALL_HINT = 1 << 6, } FlatpakHelperDeployFlags; #define FLATPAK_HELPER_DEPLOY_FLAGS_ALL (FLATPAK_HELPER_DEPLOY_FLAGS_UPDATE |\ FLATPAK_HELPER_DEPLOY_FLAGS_NO_DEPLOY |\ FLATPAK_HELPER_DEPLOY_FLAGS_LOCAL_PULL |\ FLATPAK_HELPER_DEPLOY_FLAGS_REINSTALL |\ - FLATPAK_HELPER_DEPLOY_FLAGS_NO_INTERACTION) + FLATPAK_HELPER_DEPLOY_FLAGS_NO_INTERACTION |\ + FLATPAK_HELPER_DEPLOY_FLAGS_APP_HINT |\ + FLATPAK_HELPER_DEPLOY_FLAGS_INSTALL_HINT) typedef enum { FLATPAK_HELPER_UNINSTALL_FLAGS_NONE = 0, @@ -592,6 +596,7 @@ gboolean flatpak_dir_install (FlatpakDir *self, gboolean no_deploy, gboolean no_static_deltas, gboolean reinstall, + gboolean app_hint, FlatpakRemoteState *state, const char *ref, const char *opt_commit, @@ -633,6 +638,8 @@ gboolean flatpak_dir_update (FlatpakDir *self, gboolean no_deploy, gboolean no_static_deltas, gboolean allow_downgrade, + gboolean app_hint, + gboolean install_hint, FlatpakRemoteState *state, const char *ref, const char *checksum_or_latest, diff --git a/common/flatpak-dir.c b/common/flatpak-dir.c index b94791d4..80b0a2d1 100644 --- a/common/flatpak-dir.c +++ b/common/flatpak-dir.c @@ -7652,6 +7652,7 @@ flatpak_dir_install (FlatpakDir *self, gboolean no_deploy, gboolean no_static_deltas, gboolean reinstall, + gboolean app_hint, FlatpakRemoteState *state, const char *ref, const char *opt_commit, @@ -7769,6 +7770,11 @@ flatpak_dir_install (FlatpakDir *self, if (reinstall) helper_flags |= FLATPAK_HELPER_DEPLOY_FLAGS_REINSTALL; + if (app_hint) + helper_flags |= FLATPAK_HELPER_DEPLOY_FLAGS_APP_HINT; + + helper_flags |= FLATPAK_HELPER_DEPLOY_FLAGS_INSTALL_HINT; + if (!flatpak_dir_system_helper_call_deploy (self, child_repo_path ? child_repo_path : "", helper_flags, ref, state->remote_name, @@ -8220,6 +8226,8 @@ flatpak_dir_update (FlatpakDir *self, gboolean no_deploy, gboolean no_static_deltas, gboolean allow_downgrade, + gboolean app_hint, + gboolean install_hint, FlatpakRemoteState *state, const char *ref, const char *commit, @@ -8360,6 +8368,12 @@ flatpak_dir_update (FlatpakDir *self, if (no_deploy) helper_flags |= FLATPAK_HELPER_DEPLOY_FLAGS_NO_DEPLOY; + if (app_hint) + helper_flags |= FLATPAK_HELPER_DEPLOY_FLAGS_APP_HINT; + + if (install_hint) + helper_flags |= FLATPAK_HELPER_DEPLOY_FLAGS_INSTALL_HINT; + if (!flatpak_dir_system_helper_call_deploy (self, child_repo_path ? child_repo_path : "", helper_flags, ref, state->remote_name, diff --git a/common/flatpak-installation.c b/common/flatpak-installation.c index 4afc426c..64cd0ae7 100644 --- a/common/flatpak-installation.c +++ b/common/flatpak-installation.c @@ -1890,7 +1890,7 @@ flatpak_installation_install_full (FlatpakInstallation *self, (flags & FLATPAK_INSTALL_FLAGS_NO_PULL) != 0, (flags & FLATPAK_INSTALL_FLAGS_NO_DEPLOY) != 0, (flags & FLATPAK_INSTALL_FLAGS_NO_STATIC_DELTAS) != 0, - FALSE, state, + FALSE, FALSE, state, ref, NULL, (const char **) subpaths, ostree_progress, cancellable, error)) goto out; @@ -2061,7 +2061,7 @@ flatpak_installation_update_full (FlatpakInstallation *self, (flags & FLATPAK_UPDATE_FLAGS_NO_PULL) != 0, (flags & FLATPAK_UPDATE_FLAGS_NO_DEPLOY) != 0, (flags & FLATPAK_UPDATE_FLAGS_NO_STATIC_DELTAS) != 0, - FALSE, state, + FALSE, FALSE, FALSE, state, ref, target_commit, (const OstreeRepoFinderResult * const *) check_results, (const char **) subpaths, diff --git a/common/flatpak-transaction.c b/common/flatpak-transaction.c index 9db816db..13dc2361 100644 --- a/common/flatpak-transaction.c +++ b/common/flatpak-transaction.c @@ -68,6 +68,13 @@ /* This is an internal-only element of FlatpakTransactionOperationType */ #define FLATPAK_TRANSACTION_OPERATION_INSTALL_OR_UPDATE FLATPAK_TRANSACTION_OPERATION_LAST_TYPE + 1 +enum { + RUNTIME_UPDATE, + RUNTIME_INSTALL, + APP_UPDATE, + APP_INSTALL +}; + struct _FlatpakTransactionOperation { GObject parent; @@ -133,6 +140,7 @@ struct _FlatpakTransactionPrivate gboolean force_uninstall; gboolean can_run; char *default_arch; + guint max_op; }; enum { @@ -2001,6 +2009,19 @@ resolve_ops (FlatpakTransaction *self, /* op->kind is INSTALL or UPDATE */ + if (g_str_has_prefix (op->ref, "app/")) + { + if (op->kind == FLATPAK_TRANSACTION_OPERATION_INSTALL) + priv->max_op = APP_INSTALL; + else + priv->max_op = MAX (priv->max_op, APP_UPDATE); + } + else if (g_str_has_prefix (op->ref, "runtime/")) + { + if (op->kind == FLATPAK_TRANSACTION_OPERATION_INSTALL) + priv->max_op = MAX (priv->max_op, RUNTIME_INSTALL); + } + state = flatpak_transaction_ensure_remote_state (self, op->kind, op->remote, error); if (state == NULL) return FALSE; @@ -2728,6 +2749,7 @@ flatpak_transaction_run (FlatpakTransaction *self, priv->no_deploy, priv->disable_static_deltas, priv->reinstall, + priv->max_op >= APP_UPDATE, state, op->ref, op->resolved_commit, (const char **) op->subpaths, progress->ostree_progress, @@ -2767,6 +2789,8 @@ flatpak_transaction_run (FlatpakTransaction *self, priv->no_deploy, priv->disable_static_deltas, op->commit != NULL, /* Allow downgrade if we specify commit */ + priv->max_op >= APP_UPDATE, + priv->max_op == APP_INSTALL || priv->max_op == RUNTIME_INSTALL, state, op->ref, op->resolved_commit, NULL, (const char **) op->subpaths, diff --git a/system-helper/flatpak-system-helper.c b/system-helper/flatpak-system-helper.c index ca7a7a5a..29c1542a 100644 --- a/system-helper/flatpak-system-helper.c +++ b/system-helper/flatpak-system-helper.c @@ -1334,6 +1334,22 @@ flatpak_authorize_method_handler (GDBusInterfaceSkeleton *interface, is_update = (flags & FLATPAK_HELPER_DEPLOY_FLAGS_UPDATE) != 0; is_app = g_str_has_prefix (ref, "app/"); + /* These flags allow clients to "upgrade" the permission, + * avoiding the need for multiple polkit dialogs when we first + * update a runtime, then install the app that needs it. + * + * Note that our policy has implications: + * app-install > app-update > runtime-install > runtime-update + * which means that these hints only ever select a stronger + * permission, and are safe in that sense. + */ + + if ((flags & FLATPAK_HELPER_DEPLOY_FLAGS_APP_HINT) != 0) + is_app = TRUE; + + if ((flags & FLATPAK_HELPER_DEPLOY_FLAGS_INSTALL_HINT) != 0) + is_update = FALSE; + if (is_update) { if (is_app)