GNOME.org
 

[network-manager-applet/ce-highlight-invalid-bgo767210: 1/3] wireless-security: Make broken configuration entries red

commit 2997352c55df386da6c456c87a9e2b7257c0d0c5
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     |   16 +++++++--
 src/wireless-security/eap-method-simple.c   |   27 +++++++++++-----
 src/wireless-security/eap-method-tls.c      |   43 ++++++++++++++++++---------
 src/wireless-security/eap-method.c          |   24 ++++++++++----
 src/wireless-security/ws-leap.c             |   16 +++++++--
 src/wireless-security/ws-wep-key.c          |    7 ++++
 src/wireless-security/ws-wpa-psk.c          |    3 ++
 11 files changed, 140 insertions(+), 41 deletions(-)
---
diff --git a/src/connection-editor/page-8021x-security.c b/src/connection-editor/page-8021x-security.c
index 5af6111..2a41335 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 c895cbc..4fa6780 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
@@ -59,6 +60,7 @@ validate (EAPMethod *parent, GError **error)
        const char *file;
        gboolean provisioning;
        gboolean valid = FALSE;
+       gboolean ret = TRUE;
 
        widget = GTK_WIDGET (gtk_builder_get_object (parent->builder, "eap_fast_pac_provision_checkbutton"));
        g_assert (widget);
@@ -67,8 +69,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;
+               ret = FALSE;
+       } else {
+               widget_unset_error (widget);
        }
 
        widget = GTK_WIDGET (gtk_builder_get_object (parent->builder, "eap_fast_inner_auth_combo"));
@@ -77,9 +82,9 @@ 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, *error ? NULL : error);
        eap_method_unref (eap);
-       return valid;
+       return ret ? valid : ret;
 }
 
 static void
diff --git a/src/wireless-security/eap-method-leap.c b/src/wireless-security/eap-method-leap.c
index 63e2d51..5fbdd86 100644
--- a/src/wireless-security/eap-method-leap.c
+++ b/src/wireless-security/eap-method-leap.c
@@ -57,20 +57,28 @@ 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;
+               widget_set_error (GTK_WIDGET (method->password_entry));
+               if (!*error)
+                       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 TRUE;
+       return ret;
 }
 
 static void
diff --git a/src/wireless-security/eap-method-simple.c b/src/wireless-security/eap-method-simple.c
index 692695b..0f72029 100644
--- a/src/wireless-security/eap-method-simple.c
+++ b/src/wireless-security/eap-method-simple.c
@@ -66,24 +66,33 @@ 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;
+       if (always_ask_selected (method->password_entry)) {
+               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 (!*error)
+                               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 114e5be..619d6d5 100644
--- a/src/wireless-security/eap-method-tls.c
+++ b/src/wireless-security/eap-method-tls.c
@@ -58,31 +58,42 @@ 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 (!*error)
+                       g_set_error (error, NMA_ERROR, NMA_ERROR_GENERIC, _("invalid EAP-TLS CA certificate: 
%s"), local->message);
                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;
+               ret = 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 (!*error)
+                       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 (!*error)
+                       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 +102,24 @@ 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 (!*error)
+                       g_set_error (error, NMA_ERROR, NMA_ERROR_GENERIC, _("invalid EAP-TLS private-key: 
%s"), local->message);
                g_clear_error (&local);
-               return FALSE;
+               widget_set_error (GTK_WIDGET (gtk_builder_get_object (parent->builder, 
"eap_tls_private_key_button")));
+               ret = FALSE;
        }
 
        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 (!*error)
+                               g_set_error (error, NMA_ERROR, NMA_ERROR_GENERIC, _("invalid EAP-TLS 
user-certificate: %s"), local->message);
                        g_clear_error (&local);
-                       return FALSE;
+                       widget_set_error (GTK_WIDGET (gtk_builder_get_object (parent->builder, 
"eap_tls_user_cert_button")));
+                       ret = FALSE;
                }
        }
 
-       return TRUE;
+       return ret;
 }
 
 static void
diff --git a/src/wireless-security/eap-method.c b/src/wireless-security/eap-method.c
index 36e34ae..90efea1 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,33 @@ 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) {
+                       widget_set_error (widget);
                        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;
+               widget_set_error (widget);
                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;
@@ -251,6 +256,9 @@ eap_method_validate_filepicker (GtkBuilder *builder,
        } else
                g_warning ("%s: invalid item type %d.", __func__, item_type);
 
+       if (!success)
+               widget_set_error (widget);
+
        g_object_unref (setting);
 
 out:
@@ -258,6 +266,8 @@ out:
 
        if (!success && error && !*error)
                g_set_error_literal (error, NMA_ERROR, NMA_ERROR_GENERIC, _("unspecified error validating 
eap-method file"));
+       else
+               widget_unset_error (widget);
        return success;
 }
 
diff --git a/src/wireless-security/ws-leap.c b/src/wireless-security/ws-leap.c
index 0bf1707..2b8b615 100644
--- a/src/wireless-security/ws-leap.c
+++ b/src/wireless-security/ws-leap.c
@@ -53,24 +53,32 @@ 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 (!*error)
+                       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]