GNOME.org
 

[network-manager-applet] wireless-security: make broken configuration entries red

commit 950c14dd71e9e9d6b9644cdbd7a148a34c2bea79
Author: Bastien Nocera <hadess hadess net>
Date:   Fri Jun 3 16:50:05 2016 +0200

    wireless-security: make broken configuration entries red
    
    When a configuration setting is wrong, set the entry or file chooser
    that contains the incorrect information to be surrounded by red.
    
    This makes it easier for users to find where the error was made that
    disallows them to click the "Apply" button.
    
    See https://bugzilla.gnome.org/show_bug.cgi?id=734446
    See https://bugzilla.gnome.org/show_bug.cgi?id=734472
    
    https://bugzilla.gnome.org/show_bug.cgi?id=767210

 src/connection-editor/page-8021x-security.c |   16 +++++++++
 src/utils/utils.c                           |   15 ++++++++
 src/utils/utils.h                           |    3 ++
 src/wireless-security/eap-method-fast.c     |   11 ++++--
 src/wireless-security/eap-method-leap.c     |   21 ++++++++----
 src/wireless-security/eap-method-simple.c   |   26 +++++++++-----
 src/wireless-security/eap-method-tls.c      |   48 +++++++++++++++++++--------
 src/wireless-security/eap-method.c          |   23 +++++++++----
 src/wireless-security/ws-leap.c             |   21 ++++++++----
 src/wireless-security/ws-wep-key.c          |    7 ++++
 src/wireless-security/ws-wpa-psk.c          |    3 ++
 11 files changed, 146 insertions(+), 48 deletions(-)
---
diff --git a/src/connection-editor/page-8021x-security.c b/src/connection-editor/page-8021x-security.c
index 5af6111..b17df6e 100644
--- a/src/connection-editor/page-8021x-security.c
+++ b/src/connection-editor/page-8021x-security.c
@@ -132,6 +132,19 @@ ce_page_8021x_security_new (NMConnectionEditor *editor,
        return CE_PAGE (self);
 }
 
+static void
+clear_widget_errors (GtkWidget *widget,
+                     gpointer   user_data)
+{
+       if (GTK_IS_CONTAINER (widget)) {
+               gtk_container_forall (GTK_CONTAINER (widget),
+                                     clear_widget_errors,
+                                     NULL);
+       } else {
+               widget_unset_error (widget);
+       }
+}
+
 static gboolean
 ce_page_validate_v (CEPage *page, NMConnection *connection, GError **error)
 {
@@ -164,6 +177,9 @@ ce_page_validate_v (CEPage *page, NMConnection *connection, GError **error)
                        g_object_unref (tmp_connection);
                }
        } else {
+               gtk_container_forall (GTK_CONTAINER (priv->security_widget),
+                                     clear_widget_errors,
+                                     NULL);
                nm_connection_remove_setting (connection, NM_TYPE_SETTING_802_1X);
                valid = TRUE;
        }
diff --git a/src/utils/utils.c b/src/utils/utils.c
index fd573cc..000ccd4 100644
--- a/src/utils/utils.c
+++ b/src/utils/utils.c
@@ -370,3 +370,18 @@ utils_fake_return_key (GdkEventKey *event)
        g_free (keys);
 }
 
+void
+widget_set_error (GtkWidget *widget)
+{
+       g_return_if_fail (GTK_IS_WIDGET (widget));
+
+       gtk_style_context_add_class (gtk_widget_get_style_context (widget), "error");
+}
+
+void
+widget_unset_error (GtkWidget *widget)
+{
+       g_return_if_fail (GTK_IS_WIDGET (widget));
+
+       gtk_style_context_remove_class (gtk_widget_get_style_context (widget), "error");
+}
diff --git a/src/utils/utils.h b/src/utils/utils.h
index 908741c..8202127 100644
--- a/src/utils/utils.h
+++ b/src/utils/utils.h
@@ -94,5 +94,8 @@ void utils_set_cell_background (GtkCellRenderer *cell,
 
 void utils_fake_return_key (GdkEventKey *event);
 
+void widget_set_error   (GtkWidget *widget);
+void widget_unset_error (GtkWidget *widget);
+
 #endif /* UTILS_H */
 
diff --git a/src/wireless-security/eap-method-fast.c b/src/wireless-security/eap-method-fast.c
index 682ff1f..644a213 100644
--- a/src/wireless-security/eap-method-fast.c
+++ b/src/wireless-security/eap-method-fast.c
@@ -28,6 +28,7 @@
 #include "eap-method.h"
 #include "wireless-security.h"
 #include "utils.h"
+#include "helpers.h"
 
 #define I_NAME_COLUMN   0
 #define I_METHOD_COLUMN 1
@@ -58,7 +59,7 @@ validate (EAPMethod *parent, GError **error)
        EAPMethod *eap = NULL;
        const char *file;
        gboolean provisioning;
-       gboolean valid = FALSE;
+       gboolean valid = TRUE;
 
        widget = GTK_WIDGET (gtk_builder_get_object (parent->builder, "eap_fast_pac_provision_checkbutton"));
        g_assert (widget);
@@ -67,9 +68,11 @@ validate (EAPMethod *parent, GError **error)
        g_assert (widget);
        file = gtk_file_chooser_get_filename (GTK_FILE_CHOOSER (widget));
        if (!provisioning && !file) {
+               widget_set_error (widget);
                g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("missing EAP-FAST PAC file"));
-               return FALSE;
-       }
+               valid = FALSE;
+       } else
+               widget_unset_error (widget);
 
        widget = GTK_WIDGET (gtk_builder_get_object (parent->builder, "eap_fast_inner_auth_combo"));
        g_assert (widget);
@@ -77,7 +80,7 @@ validate (EAPMethod *parent, GError **error)
        gtk_combo_box_get_active_iter (GTK_COMBO_BOX (widget), &iter);
        gtk_tree_model_get (model, &iter, I_METHOD_COLUMN, &eap, -1);
        g_assert (eap);
-       valid = eap_method_validate (eap, error);
+       valid = eap_method_validate (eap, valid ? error : NULL) && valid;
        eap_method_unref (eap);
        return valid;
 }
diff --git a/src/wireless-security/eap-method-leap.c b/src/wireless-security/eap-method-leap.c
index 63e2d51..2872084 100644
--- a/src/wireless-security/eap-method-leap.c
+++ b/src/wireless-security/eap-method-leap.c
@@ -57,20 +57,27 @@ validate (EAPMethod *parent, GError **error)
 {
        EAPMethodLEAP *method = (EAPMethodLEAP *)parent;
        const char *text;
+       gboolean ret = TRUE;
 
        text = gtk_entry_get_text (method->username_entry);
        if (!text || !strlen (text)) {
+               widget_set_error (GTK_WIDGET (method->username_entry));
                g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("missing EAP-LEAP username"));
-               return FALSE;
-       }
+               ret = FALSE;
+       } else
+               widget_unset_error (GTK_WIDGET (method->username_entry));
 
        text = gtk_entry_get_text (method->password_entry);
        if (!text || !strlen (text)) {
-               g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("missing EAP-LEAP password"));
-               return FALSE;
-       }
-
-       return TRUE;
+               widget_set_error (GTK_WIDGET (method->password_entry));
+               if (ret) {
+                       g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("missing EAP-LEAP 
password"));
+                       ret = FALSE;
+               }
+       } else
+               widget_unset_error (GTK_WIDGET (method->password_entry));
+
+       return ret;
 }
 
 static void
diff --git a/src/wireless-security/eap-method-simple.c b/src/wireless-security/eap-method-simple.c
index 692695b..411bba6 100644
--- a/src/wireless-security/eap-method-simple.c
+++ b/src/wireless-security/eap-method-simple.c
@@ -66,24 +66,32 @@ validate (EAPMethod *parent, GError **error)
 {
        EAPMethodSimple *method = (EAPMethodSimple *)parent;
        const char *text;
+       gboolean ret = TRUE;
 
        text = gtk_entry_get_text (method->username_entry);
        if (!text || !strlen (text)) {
+               widget_set_error (GTK_WIDGET (method->username_entry));
                g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("missing EAP username"));
-               return FALSE;
-       }
+               ret = FALSE;
+       } else
+               widget_unset_error (GTK_WIDGET (method->username_entry));
 
        /* Check if the password should always be requested */
        if (always_ask_selected (method->password_entry))
-               return TRUE;
-
-       text = gtk_entry_get_text (method->password_entry);
-       if (!text || !strlen (text)) {
-               g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("missing EAP password"));
-               return FALSE;
+               widget_unset_error (GTK_WIDGET (method->password_entry));
+       else {
+               text = gtk_entry_get_text (method->password_entry);
+               if (!text || !strlen (text)) {
+                       widget_set_error (GTK_WIDGET (method->password_entry));
+                       if (ret) {
+                               g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("missing EAP 
password"));
+                               ret = FALSE;
+                       }
+               } else
+                       widget_unset_error (GTK_WIDGET (method->password_entry));
        }
 
-       return TRUE;
+       return ret;
 }
 
 static void
diff --git a/src/wireless-security/eap-method-tls.c b/src/wireless-security/eap-method-tls.c
index da954e4..9855b2e 100644
--- a/src/wireless-security/eap-method-tls.c
+++ b/src/wireless-security/eap-method-tls.c
@@ -58,31 +58,45 @@ validate (EAPMethod *parent, GError **error)
        GtkWidget *widget;
        const char *password, *identity;
        GError *local = NULL;
+       gboolean ret = TRUE;
 
        widget = GTK_WIDGET (gtk_builder_get_object (parent->builder, "eap_tls_identity_entry"));
        g_assert (widget);
        identity = gtk_entry_get_text (GTK_ENTRY (widget));
        if (!identity || !strlen (identity)) {
+               widget_set_error (widget);
                g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("missing EAP-TLS identity"));
-               return FALSE;
+               ret = FALSE;
+       } else {
+               widget_unset_error (widget);
        }
 
        if (!eap_method_validate_filepicker (parent->builder, "eap_tls_ca_cert_button", TYPE_CA_CERT, NULL, 
NULL, &local)) {
-               g_set_error (error, NMA_ERROR, NMA_ERROR_GENERIC, _("invalid EAP-TLS CA certificate: %s"), 
local->message);
+               widget_set_error (GTK_WIDGET (gtk_builder_get_object (parent->builder, 
"eap_tls_ca_cert_button")));
+               if (ret) {
+                       g_set_error (error, NMA_ERROR, NMA_ERROR_GENERIC, _("invalid EAP-TLS CA certificate: 
%s"), local->message);
+                       ret = FALSE;
+               }
                g_clear_error (&local);
-               return FALSE;
-       }
-       if (eap_method_ca_cert_required (parent->builder, "eap_tls_ca_cert_not_required_checkbox", 
"eap_tls_ca_cert_button")) {
-               g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("invalid EAP-TLS CA certificate: 
no certificate specified"));
-               return FALSE;
+       } else if (eap_method_ca_cert_required (parent->builder, "eap_tls_ca_cert_not_required_checkbox", 
"eap_tls_ca_cert_button")) {
+               widget_set_error (GTK_WIDGET (gtk_builder_get_object (parent->builder, 
"eap_tls_ca_cert_button")));
+               if (ret) {
+                       g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("invalid EAP-TLS CA 
certificate: no certificate specified"));
+                       ret = FALSE;
+               }
        }
 
        widget = GTK_WIDGET (gtk_builder_get_object (parent->builder, "eap_tls_private_key_password_entry"));
        g_assert (widget);
        password = gtk_entry_get_text (GTK_ENTRY (widget));
        if (!password || !strlen (password)) {
-               g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("invalid EAP-TLS password: 
missing"));
-               return FALSE;
+               widget_set_error (widget);
+               if (ret) {
+                       g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("invalid EAP-TLS 
password: missing"));
+                       ret = FALSE;
+               }
+       } else {
+               widget_unset_error (widget);
        }
 
        if (!eap_method_validate_filepicker (parent->builder,
@@ -91,20 +105,26 @@ validate (EAPMethod *parent, GError **error)
                                             password,
                                             &format,
                                             &local)) {
-               g_set_error (error, NMA_ERROR, NMA_ERROR_GENERIC, _("invalid EAP-TLS private-key: %s"), 
local->message);
+               if (ret) {
+                       g_set_error (error, NMA_ERROR, NMA_ERROR_GENERIC, _("invalid EAP-TLS private-key: 
%s"), local->message);
+                       ret = FALSE;
+               }
                g_clear_error (&local);
-               return FALSE;
+               widget_set_error (GTK_WIDGET (gtk_builder_get_object (parent->builder, 
"eap_tls_private_key_button")));
        }
 
        if (format != NM_SETTING_802_1X_CK_FORMAT_PKCS12) {
                if (!eap_method_validate_filepicker (parent->builder, "eap_tls_user_cert_button", 
TYPE_CLIENT_CERT, NULL, NULL, &local)) {
-                       g_set_error (error, NMA_ERROR, NMA_ERROR_GENERIC, _("invalid EAP-TLS 
user-certificate: %s"), local->message);
+                       if (ret) {
+                               g_set_error (error, NMA_ERROR, NMA_ERROR_GENERIC, _("invalid EAP-TLS 
user-certificate: %s"), local->message);
+                               ret = FALSE;
+                       }
                        g_clear_error (&local);
-                       return FALSE;
+                       widget_set_error (GTK_WIDGET (gtk_builder_get_object (parent->builder, 
"eap_tls_user_cert_button")));
                }
        }
 
-       return TRUE;
+       return ret;
 }
 
 static void
diff --git a/src/wireless-security/eap-method.c b/src/wireless-security/eap-method.c
index 36e34ae..c7964cb 100644
--- a/src/wireless-security/eap-method.c
+++ b/src/wireless-security/eap-method.c
@@ -32,6 +32,7 @@
 #include "eap-method.h"
 #include "nm-utils.h"
 #include "utils.h"
+#include "helpers.h"
 
 G_DEFINE_BOXED_TYPE (EAPMethod, eap_method, eap_method_ref, eap_method_unref)
 
@@ -216,29 +217,32 @@ eap_method_validate_filepicker (GtkBuilder *builder,
        GtkWidget *widget;
        char *filename;
        NMSetting8021x *setting;
-       gboolean success = FALSE;
+       gboolean success = TRUE;
 
        if (item_type == TYPE_PRIVATE_KEY) {
-               g_return_val_if_fail (password != NULL, FALSE);
-               g_return_val_if_fail (strlen (password), FALSE);
+               if (!password || *password == '\0')
+                       success = FALSE;
        }
 
        widget = GTK_WIDGET (gtk_builder_get_object (builder, name));
        g_assert (widget);
        filename = gtk_file_chooser_get_filename (GTK_FILE_CHOOSER (widget));
        if (!filename) {
-               if (item_type == TYPE_CA_CERT)
-                       success = TRUE;
-               else
+               if (item_type != TYPE_CA_CERT) {
+                       success = FALSE;
                        g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("no file selected"));
+               }
                goto out;
        }
 
-       if (!g_file_test (filename, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_REGULAR))
+       if (!g_file_test (filename, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_REGULAR)) {
+               success = FALSE;
                goto out;
+       }
 
        setting = (NMSetting8021x *) nm_setting_802_1x_new ();
 
+       success = FALSE;
        if (item_type == TYPE_PRIVATE_KEY) {
                if (nm_setting_802_1x_set_private_key (setting, filename, password, 
NM_SETTING_802_1X_CK_SCHEME_PATH, out_format, error))
                        success = TRUE;
@@ -258,6 +262,11 @@ out:
 
        if (!success && error && !*error)
                g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("unspecified error validating 
eap-method file"));
+
+       if (success)
+               widget_unset_error (widget);
+       else
+               widget_set_error (widget);
        return success;
 }
 
diff --git a/src/wireless-security/ws-leap.c b/src/wireless-security/ws-leap.c
index 0bf1707..83b90bb 100644
--- a/src/wireless-security/ws-leap.c
+++ b/src/wireless-security/ws-leap.c
@@ -53,24 +53,31 @@ validate (WirelessSecurity *parent, GError **error)
 {
        GtkWidget *entry;
        const char *text;
+       gboolean ret = TRUE;
 
        entry = GTK_WIDGET (gtk_builder_get_object (parent->builder, "leap_username_entry"));
        g_assert (entry);
        text = gtk_entry_get_text (GTK_ENTRY (entry));
        if (!text || !strlen (text)) {
+               widget_set_error (entry);
                g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("missing leap-username"));
-               return FALSE;
-       }
+               ret = FALSE;
+       } else
+               widget_unset_error (entry);
 
        entry = GTK_WIDGET (gtk_builder_get_object (parent->builder, "leap_password_entry"));
        g_assert (entry);
        text = gtk_entry_get_text (GTK_ENTRY (entry));
        if (!text || !strlen (text)) {
-               g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("missing leap-password"));
-               return FALSE;
-       }
+               widget_set_error (entry);
+               if (ret) {
+                       g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("missing leap-password"));
+                       ret = FALSE;
+               }
+       } else
+               widget_unset_error (entry);
 
-       return TRUE;
+       return ret;
 }
 
 static void
diff --git a/src/wireless-security/ws-wep-key.c b/src/wireless-security/ws-wep-key.c
index 3628d67..045ac4b 100644
--- a/src/wireless-security/ws-wep-key.c
+++ b/src/wireless-security/ws-wep-key.c
@@ -26,6 +26,7 @@
 
 #include "wireless-security.h"
 #include "utils.h"
+#include "helpers.h"
 #include "nma-ui-utils.h"
 
 struct _WirelessSecurityWEPKey {
@@ -102,6 +103,7 @@ validate (WirelessSecurity *parent, GError **error)
 
        key = gtk_entry_get_text (GTK_ENTRY (entry));
        if (!key) {
+               widget_set_error (entry);
                g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("missing wep-key"));
                return FALSE;
        }
@@ -110,6 +112,7 @@ validate (WirelessSecurity *parent, GError **error)
                if ((strlen (key) == 10) || (strlen (key) == 26)) {
                        for (i = 0; i < strlen (key); i++) {
                                if (!g_ascii_isxdigit (key[i])) {
+                                       widget_set_error (entry);
                                        g_set_error (error, NMA_ERROR, NMA_ERROR_GENERIC, _("invalid wep-key: 
key with a length of %zu must contain only hex-digits"), strlen (key));
                                        return FALSE;
                                }
@@ -117,16 +120,19 @@ validate (WirelessSecurity *parent, GError **error)
                } else if ((strlen (key) == 5) || (strlen (key) == 13)) {
                        for (i = 0; i < strlen (key); i++) {
                                if (!utils_char_is_ascii_print (key[i])) {
+                                       widget_set_error (entry);
                                        g_set_error (error, NMA_ERROR, NMA_ERROR_GENERIC, _("invalid wep-key: 
key with a length of %zu must contain only ascii characters"), strlen (key));
                                        return FALSE;
                                }
                        }
                } else {
+                       widget_set_error (entry);
                        g_set_error (error, NMA_ERROR, NMA_ERROR_GENERIC, _("invalid wep-key: wrong key 
length %zu. A key must be either of length 5/13 (ascii) or 10/26 (hex)"), strlen (key));
                        return FALSE;
                }
        } else if (sec->type == NM_WEP_KEY_TYPE_PASSPHRASE) {
                if (!*key || (strlen (key) > 64)) {
+                       widget_set_error (entry);
                        if (!*key)
                                g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("invalid wep-key: 
passphrase must be non-empty"));
                        else
@@ -134,6 +140,7 @@ validate (WirelessSecurity *parent, GError **error)
                        return FALSE;
                }
        }
+       widget_unset_error (entry);
 
        return TRUE;
 }
diff --git a/src/wireless-security/ws-wpa-psk.c b/src/wireless-security/ws-wpa-psk.c
index aec5563..e56f348 100644
--- a/src/wireless-security/ws-wpa-psk.c
+++ b/src/wireless-security/ws-wpa-psk.c
@@ -66,6 +66,7 @@ validate (WirelessSecurity *parent, GError **error)
        key = gtk_entry_get_text (GTK_ENTRY (entry));
        len = key ? strlen (key) : 0;
        if ((len < 8) || (len > 64)) {
+               widget_set_error (entry);
                g_set_error (error, NMA_ERROR, NMA_ERROR_GENERIC, _("invalid wpa-psk: invalid key-length %zu. 
Must be [8,63] bytes or 64 hex digits"), len);
                return FALSE;
        }
@@ -74,11 +75,13 @@ validate (WirelessSecurity *parent, GError **error)
                /* Hex PSK */
                for (i = 0; i < len; i++) {
                        if (!isxdigit (key[i])) {
+                               widget_set_error (entry);
                                g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("invalid wpa-psk: 
cannot interpret key with 64 bytes as hex"));
                                return FALSE;
                        }
                }
        }
+       widget_unset_error (entry);
 
        /* passphrase can be between 8 and 63 characters inclusive */
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]