Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

admin.js: hide password for connection #539

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xypron
Copy link
Contributor

@xypron xypron commented Nov 28, 2018

In the editor for connections do not display the
characters of the password but use a input of type
password.

Closes issue #538

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

@gotthardp
Copy link
Owner

To be honest, I am not sure about this. The password element doesn't support the "display password" function, so one cannot review that the password has been written correctly. For me this is an extremely important feature. It seems to be possible with React though (https://itnext.io/building-a-toggled-mask-password-input-component-w-react-and-material-ui-f55e6bd73434).

@xypron
Copy link
Contributor Author

xypron commented Dec 2, 2018

Retrieving passwords via the GUI should be made completely impossible. I admit that this patch only hides the password. What is really needed is that passwords are not even transferred to the browser by adjusting the Mnesia query. This avoids risks like cross-site-scripting and man in the middle attacks.

Displaying the clear text password all time is a complete no-no when working in an open space office with anybody being able to look over your shoulder.

If you ever should want to verify the password you can use an Erlang shell and query the database.

@altishchenko
Copy link
Contributor

@xypron Heinrich, you should also remember, that this particular interface is also used for correct backup/restore of the server's database, making passwords unavailable through API will break this heavily.
The server admin interface is not designed for any end user story, it is strictly for troubleshooting and/or initial setup. Everything else should be done via user frontend application using the REST API (and this is how we do it here). I find ability to view the passwords directly extremely usefull, for when I am trying to figure out what is going wrong with one of 50+ connectors (in-cloud version), checking password becomes one of the routine procedures.
Also, another story is with the customer on-premises rollouts, while it is quite possible to get http/https access to the customer's server, it is not always possible to get ssh or erl access there too.
I'd rather vote for leaving this feature as it was.

In the editor for connections do not display the
characters of the password but use a input of type
password.

Closes issue gotthardp#538

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
@xypron
Copy link
Contributor Author

xypron commented Apr 27, 2019

@altishchenko The requested change is about the GUI and not about any API. Backup is not done via the GUI. So I cannot follow your concern about backups and restores.

@altishchenko
Copy link
Contributor

@xypron Ok, but what about

The server admin interface is not designed for any end user story, it is strictly for troubleshooting and/or initial setup.

I was saying that GUI is used for quick on-premises troubleshooting as well. What can be done with this? While this feature improves some security on your side, it breaks working procedures on mine for example, hence my vote against.

If you need this feature that much, why not make it configurable and let others decide if they need this or not?

Or, if it is not possible, run a vote with other community members too, otherwise it is improvement on your side against deterioration on mine .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants