From d460ec8e674107b2245a47cfe721fa84399a9bb9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 4 May 2019 09:37:54 +0200 Subject: [PATCH] core: remove unused error argument from NMAuthChainResultFunc NMAuthChain usually requests several permissions at once. Hence, an error argument in the overall callback does not make sense, because you wouldn't know which request failed. If at all, it could only mean that the overall request failed (like an D-Bus failure communicating to D-Bus *for all permisssions*), but we don't need to handle that specially. In fact, we don't really care why permission was not granted, whether it's due to an error or legitimate reasons. The error in the callback was always set to %NULL. Remove it. --- src/nm-auth-utils.c | 2 +- src/nm-auth-utils.h | 1 - src/nm-manager.c | 76 ++++---------------------- src/settings/nm-agent-manager.c | 94 ++++++++++++--------------------- src/settings/nm-settings.c | 16 +----- 5 files changed, 48 insertions(+), 141 deletions(-) diff --git a/src/nm-auth-utils.c b/src/nm-auth-utils.c index 6f898a87e7..ef61a92050 100644 --- a/src/nm-auth-utils.c +++ b/src/nm-auth-utils.c @@ -253,7 +253,7 @@ auth_call_complete (AuthCall *call) nm_assert (!self->is_finishing); self->is_finishing = TRUE; - self->done_func (self, NULL, self->context, self->user_data); + self->done_func (self, self->context, self->user_data); nm_assert (self->is_finishing); _auth_chain_destroy (self); } diff --git a/src/nm-auth-utils.h b/src/nm-auth-utils.h index 5f9823b695..58180bc4b1 100644 --- a/src/nm-auth-utils.h +++ b/src/nm-auth-utils.h @@ -28,7 +28,6 @@ typedef struct NMAuthChain NMAuthChain; typedef void (*NMAuthChainResultFunc) (NMAuthChain *chain, - GError *error, GDBusMethodInvocation *context, gpointer user_data); diff --git a/src/nm-manager.c b/src/nm-manager.c index 96b2d40e90..b35761b2b3 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -1129,7 +1129,6 @@ _config_changed_cb (NMConfig *config, NMConfigData *config_data, NMConfigChangeF static void _reload_auth_cb (NMAuthChain *chain, - GError *error, GDBusMethodInvocation *context, gpointer user_data) { @@ -1150,13 +1149,7 @@ _reload_auth_cb (NMAuthChain *chain, subject = nm_auth_chain_get_subject (chain); result = nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_RELOAD); - if (error) { - _LOGD (LOGD_CORE, "Reload request failed: %s", error->message); - ret_error = g_error_new (NM_MANAGER_ERROR, - NM_MANAGER_ERROR_PERMISSION_DENIED, - "Reload request failed: %s", - error->message); - } else if (result != NM_AUTH_CALL_RESULT_YES) { + if (result != NM_AUTH_CALL_RESULT_YES) { ret_error = g_error_new_literal (NM_MANAGER_ERROR, NM_MANAGER_ERROR_PERMISSION_DENIED, "Not authorized to reload configuration"); @@ -2335,7 +2328,6 @@ nm_manager_rfkill_update (NMManager *self, RfKillType rtype) static void device_auth_done_cb (NMAuthChain *chain, - GError *auth_error, GDBusMethodInvocation *context, gpointer user_data) { @@ -2362,14 +2354,7 @@ device_auth_done_cb (NMAuthChain *chain, result = nm_auth_chain_get_result (chain, permission); subject = nm_auth_chain_get_subject (chain); - if (auth_error) { - /* translate the auth error into a manager permission denied error */ - _LOGD (LOGD_CORE, "%s request failed: %s", permission, auth_error->message); - error = g_error_new (NM_MANAGER_ERROR, - NM_MANAGER_ERROR_PERMISSION_DENIED, - "%s request failed: %s", - permission, auth_error->message); - } else if (result != NM_AUTH_CALL_RESULT_YES) { + if (result != NM_AUTH_CALL_RESULT_YES) { _LOGD (LOGD_CORE, "%s request failed: not authorized", permission); error = g_error_new (NM_MANAGER_ERROR, NM_MANAGER_ERROR_PERMISSION_DENIED, @@ -5619,7 +5604,6 @@ nm_manager_deactivate_connection (NMManager *manager, static void deactivate_net_auth_done_cb (NMAuthChain *chain, - GError *auth_error, GDBusMethodInvocation *context, gpointer user_data) { @@ -5638,13 +5622,7 @@ deactivate_net_auth_done_cb (NMAuthChain *chain, result = nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_NETWORK_CONTROL); active = active_connection_get_by_path (self, path); - if (auth_error) { - _LOGD (LOGD_CORE, "Disconnect request failed: %s", auth_error->message); - error = g_error_new (NM_MANAGER_ERROR, - NM_MANAGER_ERROR_PERMISSION_DENIED, - "Deactivate request failed: %s", - auth_error->message); - } else if (result != NM_AUTH_CALL_RESULT_YES) { + if (result != NM_AUTH_CALL_RESULT_YES) { error = g_error_new_literal (NM_MANAGER_ERROR, NM_MANAGER_ERROR_PERMISSION_DENIED, "Not authorized to deactivate connections"); @@ -6050,13 +6028,11 @@ _internal_enable (NMManager *self, gboolean enable) static void enable_net_done_cb (NMAuthChain *chain, - GError *error, GDBusMethodInvocation *context, gpointer user_data) { NMManager *self = NM_MANAGER (user_data); NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); - GError *ret_error = NULL; NMAuthCallResult result; gboolean enable; NMAuthSubject *subject; @@ -6068,18 +6044,12 @@ enable_net_done_cb (NMAuthChain *chain, subject = nm_auth_chain_get_subject (chain); result = nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_ENABLE_DISABLE_NETWORK); - if (error) { - _LOGD (LOGD_CORE, "Enable request failed: %s", error->message); - ret_error = g_error_new (NM_MANAGER_ERROR, - NM_MANAGER_ERROR_PERMISSION_DENIED, - "Enable request failed: %s", - error->message); - } else if (result != NM_AUTH_CALL_RESULT_YES) { + if (result != NM_AUTH_CALL_RESULT_YES) { + GError *ret_error; + ret_error = g_error_new_literal (NM_MANAGER_ERROR, NM_MANAGER_ERROR_PERMISSION_DENIED, "Not authorized to enable/disable networking"); - } - if (ret_error) { nm_audit_log_control_op (NM_AUDIT_OP_NET_CONTROL, enable ? "on" : "off", FALSE, subject, ret_error->message); g_dbus_method_invocation_take_error (context, ret_error); @@ -6154,27 +6124,16 @@ get_perm_add_result (NMManager *self, NMAuthChain *chain, GVariantBuilder *resul static void get_permissions_done_cb (NMAuthChain *chain, - GError *error, GDBusMethodInvocation *context, gpointer user_data) { NMManager *self = NM_MANAGER (user_data); NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); - GError *ret_error; GVariantBuilder results; nm_assert (G_IS_DBUS_METHOD_INVOCATION (context)); priv->auth_chains = g_slist_remove (priv->auth_chains, chain); - if (error) { - _LOGD (LOGD_CORE, "Permissions request failed: %s", error->message); - ret_error = g_error_new (NM_MANAGER_ERROR, - NM_MANAGER_ERROR_PERMISSION_DENIED, - "Permissions request failed: %s", - error->message); - g_dbus_method_invocation_take_error (context, ret_error); - return; - } g_variant_builder_init (&results, G_VARIANT_TYPE ("a{ss}")); @@ -6362,7 +6321,6 @@ device_connectivity_done (NMDevice *device, static void check_connectivity_auth_done_cb (NMAuthChain *chain, - GError *auth_error, GDBusMethodInvocation *context, gpointer user_data) { @@ -6377,13 +6335,7 @@ check_connectivity_auth_done_cb (NMAuthChain *chain, result = nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_NETWORK_CONTROL); - if (auth_error) { - _LOGD (LOGD_CORE, "CheckConnectivity request failed: %s", auth_error->message); - error = g_error_new (NM_MANAGER_ERROR, - NM_MANAGER_ERROR_PERMISSION_DENIED, - "Connectivity check request failed: %s", - auth_error->message); - } else if (result != NM_AUTH_CALL_RESULT_YES) { + if (result != NM_AUTH_CALL_RESULT_YES) { error = g_error_new_literal (NM_MANAGER_ERROR, NM_MANAGER_ERROR_PERMISSION_DENIED, "Not authorized to recheck connectivity"); @@ -6792,7 +6744,6 @@ typedef struct { static void _dbus_set_property_auth_cb (NMAuthChain *chain, - GError *error, GDBusMethodInvocation *invocation, gpointer user_data) { @@ -6815,10 +6766,9 @@ _dbus_set_property_auth_cb (NMAuthChain *chain, priv->auth_chains = g_slist_remove (priv->auth_chains, chain); result = nm_auth_chain_get_result (chain, property_info->writable.permission); - if ( error - || result != NM_AUTH_CALL_RESULT_YES) { + if (result != NM_AUTH_CALL_RESULT_YES) { error_name = NM_PERM_DENIED_ERROR; - error_message = error ? error->message : "Not authorized to perform this operation"; + error_message = "Not authorized to perform this operation"; goto out; } @@ -6923,7 +6873,6 @@ _checkpoint_mgr_get (NMManager *self, gboolean create_as_needed) static void checkpoint_auth_done_cb (NMAuthChain *chain, - GError *auth_error, GDBusMethodInvocation *context, gpointer user_data) { @@ -6947,12 +6896,7 @@ checkpoint_auth_done_cb (NMAuthChain *chain, NM_AUDIT_OP_CHECKPOINT_ADJUST_ROLLBACK_TIMEOUT)) arg = checkpoint_path = nm_auth_chain_get_data (chain, "checkpoint_path"); - if (auth_error) { - error = g_error_new (NM_MANAGER_ERROR, - NM_MANAGER_ERROR_PERMISSION_DENIED, - "checkpoint check request failed: %s", - auth_error->message); - } else if (result != NM_AUTH_CALL_RESULT_YES) { + if (result != NM_AUTH_CALL_RESULT_YES) { error = g_error_new_literal (NM_MANAGER_ERROR, NM_MANAGER_ERROR_PERMISSION_DENIED, "Not authorized to checkpoint/rollback"); diff --git a/src/settings/nm-agent-manager.c b/src/settings/nm-agent-manager.c index f5acbb1a42..73f59c0b3c 100644 --- a/src/settings/nm-agent-manager.c +++ b/src/settings/nm-agent-manager.c @@ -311,7 +311,6 @@ validate_identifier (const char *identifier, GError **error) static void agent_register_permissions_done (NMAuthChain *chain, - GError *error, GDBusMethodInvocation *context, gpointer user_data) { @@ -319,7 +318,6 @@ agent_register_permissions_done (NMAuthChain *chain, NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self); NMSecretAgent *agent; const char *sender; - GError *local = NULL; NMAuthCallResult result; CList *iter; @@ -327,37 +325,29 @@ agent_register_permissions_done (NMAuthChain *chain, priv->chains = g_slist_remove (priv->chains, chain); - if (error) { - local = g_error_new (NM_AGENT_MANAGER_ERROR, - NM_AGENT_MANAGER_ERROR_PERMISSION_DENIED, - "Failed to request agent permissions: %s", - error->message); - g_dbus_method_invocation_take_error (context, local); - } else { - agent = nm_auth_chain_steal_data (chain, "agent"); - g_assert (agent); + agent = nm_auth_chain_steal_data (chain, "agent"); + nm_assert (agent); - result = nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_WIFI_SHARE_PROTECTED); - if (result == NM_AUTH_CALL_RESULT_YES) - nm_secret_agent_add_permission (agent, NM_AUTH_PERMISSION_WIFI_SHARE_PROTECTED, TRUE); + result = nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_WIFI_SHARE_PROTECTED); + if (result == NM_AUTH_CALL_RESULT_YES) + nm_secret_agent_add_permission (agent, NM_AUTH_PERMISSION_WIFI_SHARE_PROTECTED, TRUE); - result = nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_WIFI_SHARE_OPEN); - if (result == NM_AUTH_CALL_RESULT_YES) - nm_secret_agent_add_permission (agent, NM_AUTH_PERMISSION_WIFI_SHARE_OPEN, TRUE); + result = nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_WIFI_SHARE_OPEN); + if (result == NM_AUTH_CALL_RESULT_YES) + nm_secret_agent_add_permission (agent, NM_AUTH_PERMISSION_WIFI_SHARE_OPEN, TRUE); - priv->agent_version_id += 1; - sender = nm_secret_agent_get_dbus_owner (agent); - g_hash_table_insert (priv->agents, g_strdup (sender), agent); - _LOGI (agent, "agent registered"); - g_dbus_method_invocation_return_value (context, NULL); + priv->agent_version_id += 1; + sender = nm_secret_agent_get_dbus_owner (agent); + g_hash_table_insert (priv->agents, g_strdup (sender), agent); + _LOGI (agent, "agent registered"); + g_dbus_method_invocation_return_value (context, NULL); - /* Signal an agent was registered */ - g_signal_emit (self, signals[AGENT_REGISTERED], 0, agent); + /* Signal an agent was registered */ + g_signal_emit (self, signals[AGENT_REGISTERED], 0, agent); - /* Add this agent to any in-progress secrets requests */ - c_list_for_each (iter, &priv->requests) - request_add_agent (c_list_entry (iter, Request, lst_request), agent); - } + /* Add this agent to any in-progress secrets requests */ + c_list_for_each (iter, &priv->requests) + request_add_agent (c_list_entry (iter, Request, lst_request), agent); } static NMSecretAgent * @@ -1011,7 +1001,6 @@ _con_get_request_start_proceed (Request *req, gboolean include_system_secrets) static void _con_get_request_start_validated (NMAuthChain *chain, - GError *error, GDBusMethodInvocation *context, gpointer user_data) { @@ -1025,28 +1014,20 @@ _con_get_request_start_validated (NMAuthChain *chain, req->con.chain = NULL; - if (error) { - _LOGD (req->current, "agent "LOG_REQ_FMT" MODIFY check error: %s", - LOG_REQ_ARG (req), - error->message); - /* Try the next agent */ - request_next_agent (req); - } else { - /* If the agent obtained the 'modify' permission, we send all system secrets - * to it. If it didn't, we still ask it for secrets, but we don't send - * any system secrets. - */ - perm = nm_auth_chain_get_data (chain, "perm"); - g_assert (perm); - if (nm_auth_chain_get_result (chain, perm) == NM_AUTH_CALL_RESULT_YES) - req->con.current_has_modify = TRUE; + /* If the agent obtained the 'modify' permission, we send all system secrets + * to it. If it didn't, we still ask it for secrets, but we don't send + * any system secrets. + */ + perm = nm_auth_chain_get_data (chain, "perm"); + g_assert (perm); + if (nm_auth_chain_get_result (chain, perm) == NM_AUTH_CALL_RESULT_YES) + req->con.current_has_modify = TRUE; - _LOGD (req->current, "agent "LOG_REQ_FMT" MODIFY check result %s", - LOG_REQ_ARG (req), - req->con.current_has_modify ? "YES" : "NO"); + _LOGD (req->current, "agent "LOG_REQ_FMT" MODIFY check result %s", + LOG_REQ_ARG (req), + req->con.current_has_modify ? "YES" : "NO"); - _con_get_request_start_proceed (req, req->con.current_has_modify); - } + _con_get_request_start_proceed (req, req->con.current_has_modify); } static void @@ -1470,7 +1451,6 @@ nm_agent_manager_all_agents_have_capability (NMAgentManager *manager, static void agent_permissions_changed_done (NMAuthChain *chain, - GError *error, GDBusMethodInvocation *context, gpointer user_data) { @@ -1484,16 +1464,12 @@ agent_permissions_changed_done (NMAuthChain *chain, agent = nm_auth_chain_get_data (chain, "agent"); g_assert (agent); - if (error) - _LOGD (agent, "failed to request updated agent permissions"); - else { - _LOGD (agent, "updated agent permissions"); + _LOGD (agent, "updated agent permissions"); - if (nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_WIFI_SHARE_PROTECTED) == NM_AUTH_CALL_RESULT_YES) - share_protected = TRUE; - if (nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_WIFI_SHARE_OPEN) == NM_AUTH_CALL_RESULT_YES) - share_open = TRUE; - } + if (nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_WIFI_SHARE_PROTECTED) == NM_AUTH_CALL_RESULT_YES) + share_protected = TRUE; + if (nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_WIFI_SHARE_OPEN) == NM_AUTH_CALL_RESULT_YES) + share_open = TRUE; nm_secret_agent_add_permission (agent, NM_AUTH_PERMISSION_WIFI_SHARE_PROTECTED, share_protected); nm_secret_agent_add_permission (agent, NM_AUTH_PERMISSION_WIFI_SHARE_OPEN, share_open); diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 7e92e4f9ca..23ae87984d 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -1134,7 +1134,6 @@ send_agent_owned_secrets (NMSettings *self, static void pk_add_cb (NMAuthChain *chain, - GError *chain_error, GDBusMethodInvocation *context, gpointer user_data) { @@ -1159,12 +1158,7 @@ pk_add_cb (NMAuthChain *chain, result = nm_auth_chain_get_result (chain, perm); - if (chain_error) { - error = g_error_new (NM_SETTINGS_ERROR, - NM_SETTINGS_ERROR_FAILED, - "Error checking authorization: %s", - chain_error->message); - } else if (result != NM_AUTH_CALL_RESULT_YES) { + if (result != NM_AUTH_CALL_RESULT_YES) { error = g_error_new_literal (NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_PERMISSION_DENIED, "Insufficient privileges."); @@ -1503,7 +1497,6 @@ impl_settings_reload_connections (NMDBusObject *obj, static void pk_hostname_cb (NMAuthChain *chain, - GError *chain_error, GDBusMethodInvocation *context, gpointer user_data) { @@ -1520,12 +1513,7 @@ pk_hostname_cb (NMAuthChain *chain, result = nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_SETTINGS_MODIFY_HOSTNAME); /* If our NMSettingsConnection is already gone, do nothing */ - if (chain_error) { - error = g_error_new (NM_SETTINGS_ERROR, - NM_SETTINGS_ERROR_FAILED, - "Error checking authorization: %s", - chain_error->message); - } else if (result != NM_AUTH_CALL_RESULT_YES) { + if (result != NM_AUTH_CALL_RESULT_YES) { error = g_error_new_literal (NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_PERMISSION_DENIED, "Insufficient privileges.");