TEXTINPUT_PASSWORD fields rendered as plain text

dnutan · July 18, 2018, 8:56pm

Some password fields assigned as TEXTINPUT_PASSWORD on the templates, at the end are rendered as input fields with type=“text” instead of type=“password”.

As an example the password fields on backup data page.

mrmarkuz · July 18, 2018, 9:07pm

Another example is the pop3 connector password.

dnutan · July 18, 2018, 9:08pm

I think all but the ones with password strength check are affected.

davidep · July 19, 2018, 9:28pm

In some cases a visible password could be useful. It’s now widely adopted the “eye” button that shows/hides the password text in the input field…

davidep · July 24, 2018, 7:43am

…however what you pointed out is surely a #bug due to a mismatched right parenthesis

github.com

NethServer/nethserver-backup-data/blob/b185f7cde427cddc327bf8af72227bf9058ccbde/root/usr/share/nethesis/NethServer/Template/BackupData.php#L32


$destination = $view->panel()
->setAttribute('title', $T('BackupData_Destination_Title'))
->insert($view->fieldset()->setAttribute('template', $T('BackupData_Destination_Title'))
->insert($view->fieldsetSwitch('VFSType', 'usb',$view::FIELDSETSWITCH_EXPANDABLE)
    ->insert($view->selector('USBLabel', $view::SELECTOR_DROPDOWN))
)
->insert($view->fieldsetSwitch('VFSType', 'cifs',$view::FIELDSETSWITCH_EXPANDABLE)
    ->insert($view->textInput('SMBHost'))
    ->insert($view->textInput('SMBShare'))
    ->insert($view->textInput('SMBLogin'))
    ->insert($view->textInput('SMBPassword'), $view::TEXTINPUT_PASSWORD)
)
->insert($view->fieldsetSwitch('VFSType', 'nfs',$view::FIELDSETSWITCH_EXPANDABLE)
    ->insert($view->textInput('NFSHost'))
    ->insert($view->textInput('NFSShare'))
)


->insert($view->fieldsetSwitch('VFSType', 'webdav',$view::FIELDSETSWITCH_EXPANDABLE)
    ->insert($view->textInput('WebDAVUrl'))
    ->insert($view->textInput('WebDAVLogin'))
    ->insert($view->textInput('WebDAVPassword'), $view::TEXTINPUT_PASSWORD)

The $view::TEXTINPUT flag should be the 2nd argument to textInput().

@dnutan @mrmarkuz would you like to submit a patch?

dnutan · July 24, 2018, 2:24pm

PR made for Backup module.
Optional PR proposed but untested. I understand sometimes it’s useful to see the password (without going to CLI), feel free to accept/reject them or ask what users prefer:

Maybe change could also apply to these, but no PR made:

davidep · July 24, 2018, 3:03pm

I’d merge all the optional PRs if we add the show/hide behavior to the TEXINPUT_PASSWORD flag.

… and when the “eye-slash” button is pressed becomes:

HTML snippet, appended to the INPUT tag

<button style="display: inline-block;border: 1px solid gray;width: 31px;height: 31px;vertical-align: top;"><i class="fa fa-eye"></i></button>

davidep · July 24, 2018, 7:39pm

@dnutan would you mind opening an issue on GitHub? The description could be just a summary of this topic (and a link to it). Decide by yourself if writing it as bug or enhancement.

All your PRs should contain an URL reference to the new issue.

I could extend the Nethgui password widget unless you prefer to dive in it - I can wait too

dnutan · July 24, 2018, 8:47pm

The reported issue is now filed as a bug.
The eye-candy it’s filed as unlabeled issue but to be considered as an enhancement.

Go for it! I’m out of snorkels, safer staying on solid ground.

davidep · July 24, 2018, 8:57pm

Thank you for your help!

Once the eye button is added they can be changed to enahanced password fields too…

giacomo · July 30, 2018, 10:26am

I agree this is a bug, but I still prefer to see the passwords as input text fields when they are saved in clear text inside the esmith db.

The best solution is to implement the “eye” icon to show and hide the password as suggested by @davidep

For now, I’d go with fixing the backup-data form which must display the password, otherwise the user doesn’t know if it has been filled correctly.
If no one other than @davidep can add the “eye” icon to the NethGUI framework, I would like to leave existing modules untouched.
But I also would like to sponsor this suggestion for the new Cockpit-based UI.

davidep · August 6, 2018, 1:53pm

@dnutan, your pull requests have been merged!

Now who wants to test those packages? /cc @quality_team

Please refer to

…and don’t forget the login form

stephdl · August 7, 2018, 4:38pm

@dnutan, testing your code, it seems you missed the password field of ldapservice user when you created a local AD (account provider menu), would you mind to add it please ?

stephdl · August 7, 2018, 5:26pm

@davidep @dnutan I have some doubts concerning nethserver-vpn, I saw that you patched the rpm but I can read in GitHub that it has been merged in nethserver-openvpn for ns7, what is the current state ?

davidep · August 7, 2018, 5:34pm

I blindly trusted the PRs: let’s ask @dnutan

stephdl · August 7, 2018, 5:36pm

OT but nethserver-vpn requires nethserver-directory, so it drives to conflict with nethserver-dc

dnutan · August 7, 2018, 6:03pm

Still at work, will check it tonight.

My fault. AFAIK nethserver-vpn is not used on ns7. It might still be used on ns6 (idk for sure).

stephdl · August 7, 2018, 6:16pm

No problem at all, QA is made to ask questions, please continue the good work

dnutan · August 7, 2018, 7:43pm

AD Bind Password field was left as is on purpose. Didn’t know whether the implementation would work well with disabled fields (in this case is not disabled but read-only). You may find the same with other read-only fields.
Tested it on a read-only field and we can add the “eye” button without problems.

davidep · August 8, 2018, 5:13pm

Packages released