[Security] do not expose passwords

[jaymecd]

Once browser remains unattended even for a some minutes, passwords could be hi-jacked using show/hide or export functionality, or even via dev-tools console $('#password').value by inspecting popin.html.

~Also this PR provides:~ ~- [doc] install packages from lock file using npm ci~ ~- [doc] note on how to build using docker - using node v12, as higher version requires complex review~ ~- [deps] update gulp and its tasks to version 4 - number of compilation failures with version 3~ ~- [deps] update minor versions using npm audit fix~

UPD: doc and deps off-loaded into #84

[krtek4]

Hello,

I love what you did regarding docker and upgrading gulp.

However, I'd say the "security" issue is more of a feature for me than an issue.

Would you mind creating two different PRs ? One for the build tool upgrade and the other for the password change so we can separate the two ?

Regarding the securty concerns I think they are valid depending on the use case, but, at least for me, it's too disrupting to remove the possiblity to show the password completly. Maybe add a configuration option ?

Also, if you leave your computer unnatended, even with your changes, the storage of the extension could still be accessed and the password retrieved. I also imagine you'd have much bigger concerns than those password if you leave your computer unatended.

[jaymecd]

Good point, I had separation in mind initially, however decided to go with one shot. Now it's separated.

Regarding my security concern. password I'm talking about is corparate AD password, due to number of legacy systems does not support proper kerberos auth, it falls back to basic auth. Therefore I don't see it's a good practice to expose password via hide/show functionality.

IMO, it should be 1-way only: import. Why? JSON file with credentials should be stored encrypted in git and imported to the extension on change, aka GitOps.

[krtek4]

Hi,

Thanks for separating both PRs, I just merged the other one.

As I said, I understand your security concern. However, even with your changes, someone with access to your computer can still see the passwords by looking a the localStorage for the extension.

I get that it's a bit more complicated than just the "hide / show" button, but it's still possible to get the password.

BasicAuth isn't even a secure method of authentication in the first place, so if your infrastructure use it, having the possibility for an unnatended computer to leak the password wouldn't be my first order of concern. I agree that the current implementation isn't a "best" practice, but using BasicAuth is even worse.

Also, an unnatended computer is a risk in itself without this feature, I am quite sure that someone accessing your computer could do a lot more damage than getting a password through this extension.

And as I said, I use the "show / hide" feature to display the password :)

All in all, I can't see myself merging this PR as is. I can offer you two options:

• releasing another, "more secure", version of the extension yourself under another name, the license totally pertmits it
• find a way to make this "optional" so people, like myself, using the "show / hide" feature can still do so

Thanks for your time and sorry for net merging the PR as is.

Best,

[jaymecd]

That's pity to hear, feel free to reject/close this PR.

[krtek4]

What is a pity ? Not catering to your own usecase ? So far you're the only one that I know of that raised this concern, so why should I remove a "feature" just for your sake ?

I proposed a way for you to get the PR merged, make your change optional for people.

Also, as I said, if your concern is security, your PR does not fix that as someone accessing your computer could still see the password as they are stored in plaintext in the local storage of your browser. And could also login to any website for which you have the password for and have a look at the headers and get the password that way.

Basic auth is not a secure protocol anyhow, and your fix changes nothing to that, it only makes getting the password marginally more difficult, which is kinda a good thing, but as I said, if an attacker has access to your computer, you probably have bigger issues than those unesecure passwords leaking.

So, as I said, I'll gladly accept a better PR, one that doesn't remove a feature I am actively using :)