browserpass / browserpass-legacy

Legacy Browserpass repo, development is now happening at:
https://github.com/browserpass/browserpass-extension
MIT License
998 stars 87 forks source link

Improve setup script (set executable bit, be verbose about what happens) #287

Closed TKLCZ closed 5 years ago

TKLCZ commented 5 years ago

General information


Exact steps to reproduce the problem

  1. Run installation script in Linux
  2. Run installation script in Windows 7

What should happen?

Correct installation of host app

What happened instead?

  1. In Linux: a) Only the json file installed to ~/.mozilla/native.messaging-hosts folder. The binary file stay in their downloading location and this path is added to the json (this is very confusing, I want to delete all my old downloaded files immedially). b) The installation script does not set the execution bit on the binary file. Without 'chmod +x' browserpass doesn't work.

  2. In Windows 7 (the powershell... why??? ) a) at first the allowing of unsigned scripts is needed b) the json file conversion failed so only binary file is installed to AppData\Local\browserpass folder

By my opinion - because the 'pass' password manager is geeky thing - no installation scripts needed. A simple installation instructions will be much much better, something like this for example:

or

The browserpass Firefox addon is great, much better than passff. It's a shame kill it by semi-functional installation scripts.

maximbaz commented 5 years ago

Thanks for the feedback 😉

I think it's better to provide a script, eliminates differences in how people install the app and helps debugging issues. But it's a simple shell script, if you don't want to blindly run it, open it in a text editor and read the commands 😉

1. A) the path to json file is set in stone by mozilla, the path to browserpass binary can be arbitrary, so this is by design. B) Good point, should do this.

  1. Powershell because it was a user contribution, also why not? 😉 A) yes, this is by design B) feel free to send a PR, I would be happy to merge the fix 😉 I can't test, I'm on Linux.

In summary, I prefer having a script over instructions, fixing 1.B will close this issue.

TKLCZ commented 5 years ago

I think it's better to provide a script, eliminates differences in how people install the app and helps debugging issues. But it's a simple shell script, if you don't want to blindly run it, open it in a text editor and read the commands

OK, but the function of script needs to be clearly. I think that there is a big space for improvements. I can help you and try to improve the installation script.

A) the path to json file is set in stone by mozilla, the path to browserpass binary can be arbitrary, so this is by design.

No problem, but there needs to be provided a unambiguous information to the user of the script. For now is not evident where the binary file will be stored. The common procedure is: download of a archive file - extraction - running of a installation script - deleting of ALL downloaded files. In the case of browserpass the last step (deleting of downloaded files) cause the malfunction because the user dont'know that the binary file must stay in this place forever.

1. Powershell because it was a user contribution, also why not?
   B) feel free to send a PR, I would be happy to merge the fix I can't test, I'm on Linux.

I'm not familiar with the powershell too much and I'm on Windows sporadically, but I can try to get some information about the error and send it to you.

maximbaz commented 5 years ago

I would be happy to merge a patch that would print some info messages before performing an installation, to clarify to the user what will happen, and pausing to allow the user to press ctrl+c to abort and move the files. Don't want to overcomplicate the script too much though :)

Feel free to post the error messages that Powershell script throws at you, I won't be personally fixing them as I don't use pass on Windows, but perhaps one day someone from the community will chip in and submit a PR 😉

TKLCZ commented 5 years ago

I would be happy to merge a patch that would print some info messages before performing an installation, to clarify to the user what will happen, and pausing to allow the user to press ctrl+c to abort and move the files. Don't want to overcomplicate the script too much though :)

I'm done. What's new in the script:

No big changes but now the script is (by my opinion) better understandable for users. Tested with all browsers in their last versions (Firefox, Chromium, Chrome, Vivaldi) on Ubuntu 18.04.

Feel free to change anything, my english especially :-) Because I'm not using git, here is a link to a gpg-signed file, placed on my website: https://www.tkl.cz/download/browserpass_install_signed.zip

Unpack and verify: gpg --verify install.sh.gpg install.sh

If you'll find any issue, please let me know.

maximbaz commented 5 years ago

Thanks 😉 I would like to make some changes to the script before merging, I will get back to this later.

Just in case, re-attaching the zip file on Github: browserpass_install_signed.zip

TKLCZ commented 5 years ago
  1. Powershell because it was a user contribution, also why not? wink A) yes, this is by design B) feel free to send a PR, I would be happy to merge the fix wink I can't test, I'm on Linux.

In summary, I prefer having a script over instructions, fixing 1.B will close this issue.

Hello, here is the error message from powershell installation script:

The term 'ConvertTo-json' is not recognized as the name of a cmdlet, function, script file, or operable program. Check the spelling of the name, or if a path was included, verify that the path is correct and try again. At C:\Users\username\Downloads\browserpass\install.ps1:18 char:114

The term 'ConvertTo-json' is not recognized as the name of a cmdlet, function, script file, or operable program. Check the spelling of the name, or if a path was included, verify that the path is correct and try again. At C:\Users\username\Downloads\browserpass\install.ps1:21 char:114

WIndows 7 Professional SP1 64bit Czech. This was another computer than last time but the error is the same. I hope it will help.

maximbaz commented 5 years ago

Nice, thanks 🙂 Quick googling shows:

  1. ConvertTo-Json: This cmdlet was introduced in Windows PowerShell 3.0.

  2. Windows PowerShell 3.0: v2.0 can only be found on some Windows 7, and it is possible to upgrade to v3.0.

I can see now why nobody reported this issue so far 🙂

I will accept PRs that achieve the same behavior without using ConvertTo-Json, but as a workaround you can upgrade your PowerShell to v3.0.

maximbaz commented 5 years ago

In Browserpass v3 I went for using Makefile to install and configure Browserpass (if you want to test, see https://github.com/browserpass/browserpass/issues/331)

Closing as irrelevant at this point, but thanks for your help!