When the "Hide password" checkbox in the SMTP authentication is checked and the settings are saved, the checkbox disappears.

This makes some sense, but there is no normal way even for the administrator to get it back. That is a break of a fundamental rule of user interface design that, to the best of my knowledge, is not broken anywhere else in Drupal. New admins may try out the checkbox, only to find that they cannot get the system back to what it was before.

I propose to leave the checkbox in place for administrators or at the very least for the first administrator, user 0.

An absolutely minimal "solution" to this problem would be a warning at the checkbox that, once checked, it will disappear forever, but I would find that only somewhat better than the current design, but still not fully acceptable.

CommentFileSizeAuthor
#6 phpmailer.unhide-password.6.patch2.65 KBsun
#5 phpmailer.unhide-password.5.patch2.4 KBsun
#1 851072-unhide-password.patch2.58 KBsmk-ka

Comments

smk-ka’s picture

smk-ka commented
StatusFileSize
new 851072-unhide-password.patch2.58 KB

Hm, that feature was thought as a security measure, to not let anybody see the mail server password. Unfortunately this means unhiding for anybody would defeat the whole purpose, so here's a patch just for the proposed idea to let the super admin restore the default setting. Don't know if we need more warnings than now, though (made permanent bold and added a note about super admin).

smk-ka’s picture

smk-ka commented
Status: Active » Needs review
sun’s picture

sun
Primary language German
Location Karlsruhe
commented 
+++ phpmailer.admin.inc	16 Aug 2010 20:21:28 -0000
@@ -101,6 +97,14 @@ function phpmailer_settings_form($form_s
+  if (!variable_get('smtp_hide_password', 0) || $user->uid == 1) {
+    $form['auth']['smtp_hide_password'] = array(
+      '#type' => 'checkbox',
+      '#title' => t('Hide password'),
+      '#default_value' => variable_get('smtp_hide_password', 0),
+      '#description' => t("Check this option to <strong>permanently</strong> hide the plaintext password from prying eyes. You may still change the password afterwards, but it won't be displayed anymore. Only the super admin of the site (user id 1) will be able to restore the default setting."),
+    );
+  }

@@ -195,8 +199,8 @@ function phpmailer_settings_form_submit(
+  if (($form_state['values']['smtp_hide_password'] || variable_get('smtp_hide_password', 0)) && $form_state['values']['smtp_password'] == '') {

The if condition could be the value of the #access property, to always have the submitted form value and not trigger a PHP notice...?

Powered by Dreditor.

smk-ka’s picture

smk-ka commented

Eh, what? Where does this trigger a notice? We're not hiding any form element, so I'm not sure where #access could help us either?

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Version: 6.x-2.2 » 6.x-3.x-dev
StatusFileSize
new phpmailer.unhide-password.5.patch2.4 KB

By only conditionally adding the 'smtp_hide_password' element to the form, there will not be a key nor value for it in $form_state['values']. Hence,

@@ -195,8 +199,8 @@ function phpmailer_settings_form_submit(
+  if (($form_state['values']['smtp_hide_password'] || ...) {

tries to access an array key that does not exist. By using #access, the value is always set.

Tweaked that and also the #description in attached patch.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
StatusFileSize
new phpmailer.unhide-password.6.patch2.65 KB

That condition in the submit handler wasn't really obvious, so clarified it in a comment.

oadaeh’s picture

oadaeh commented
Issue summary: View changes
Status: Needs review » Closed (won't fix)

This issue is being closed due to Drupal 6 being at the end of its life. If you feel this is still a valid issue for the Drupal 7 compatible branch of this module, feel free to re-open this issue and provide updated information.