minbrowser / min

A fast, minimal browser that protects your privacy
https://minbrowser.org/
Apache License 2.0
7.96k stars 703 forks source link

1password helper installation frozen #1028

Open genseric opened 4 years ago

genseric commented 4 years ago

while installing the 1password installer app on mac 10.15.4 the installer freezes like below. tried the op_darwin 0.10 and the new 1.0 packages. none of them worked. image min browser version : Version 1.14.1 (1.14.1)

PalmerAL commented 4 years ago

Can you open the browser console (developer > inspect browser) and see if there are any errors listed there?

genseric commented 4 years ago

image

this is the browser inspect console output after i drag,install and close the 1password installer after a successful installation

PalmerAL commented 4 years ago

So it worked correctly when run a second time? It's hard to say what happened the first time, but please let me know if you ever run into issues with it again.

genseric commented 4 years ago

no, what i meant was the helper installation was successful. this is the ss from the console when it froze with the installing... state.

PalmerAL commented 4 years ago

I tried to reproduce this once, and it froze the first time I tried it (with no errors), and it's worked every time since then. So perhaps it might work if you run it again?

If this is something that's happening repeatedly, I'll make a build with more detailed error logging that you can try.

genseric commented 4 years ago

tried a bunch of times. still stuck at installing phase after the helper is installed. waiting for your input. thanks a lot.

PalmerAL commented 4 years ago

I think I figured it out: it works when the app is run from the command line (with electron), but not when it's packaged. Reason is that we look for 1password with which, which searches based on whatever PATH is set to. When running from the command line, you get a path that includes /usr/local/bin where 1password is, but when packaged, the command runs without a shell, so the path ends up just being /usr/bin:/bin:/usr/sbin:/sbin, so it won't find the command.

I guess we should try which first, and if that fails, fall back to manually checking in /usr/local/bin.

genseric commented 4 years ago

great, and maybe a timeout or a catch for this execution so it will notify the user about the fail, just a thought )

PalmerAL commented 4 years ago

I think I've fixed it in 3ef2f3110e08b0a439c400bab800cbcf5255e02f; here's a build to try:

https://send.firefox.com/download/7891b1bcc12888be/#58ttq4COSxxDLFpFWpKlJg

I'm not sure how feasible it is to detect this failure; there isn't a way to directly communicate with the installer, so we check for the binary's existence every few seconds and continue to the next step once it's found. In this case, we're never finding the binary, so the only thing to do would be to just fail after a given amount of time, but then you're guessing how long the installation will take, and if the user has slow internet or a slow computer, it could potentially end up taking a long time.

genseric commented 4 years ago

thanks a lot. while using this build when i select 1password as the auto-fill helper no pop-up appeared, but if i select bit-warden the expected popup appeared. i thought maybe it was the old options file so went into trouble and completely removed the app and launched again fresh. it was the same. and here is the browser console ss. image

genseric commented 4 years ago

and just another thought, i think the drop-in installer is a classy idea but if 1password gets picked, you might check for the op binary within the path and reject selection reminding the user to install the binary if the executable is not found. i have not reviewed any of the code but i m just speculating if min relies only on the binary itself. thanks again.

PalmerAL commented 4 years ago

you might check for the op binary within the path and reject selection reminding the user to install the binary if the executable is not found

This is what happens already, if I'm understanding what you're saying correctly.

When you ran the installer the first time, the binary did get installed, which is why you aren't seeing the setup screen again. Can you try deleting /usr/local/bin/op, restarting Min, and enabling 1password integration again?

genseric commented 4 years ago

ok, now the 1password installer is fine but, the error i receive from the browser debug console is : bundle.js:4306 Error accessing 1Password CLI. STDOUT: undefined. STDERR: undefined SyntaxError: Unexpected end of JSON input at JSON.parse (<anonymous>) at OnePassword.loadSuggestions (bundle.js:4275)

PalmerAL commented 4 years ago

Can you try running these commands manually in the console, and see what the output is? (you might need to redact parts of the output):

  1. Run op signin --raw; you'll get a session key
  2. op list items --session=, followed by the session key from the first step.
PalmerAL commented 4 years ago

@readtedium can you also try the steps listed above to run the 1password commands manually, and see what the output is?

readtedium commented 4 years ago

Not comfortable with sharing it as it’s a list of my logins with hash codes, but it doesn’t seem to give me any errors.

PalmerAL commented 4 years ago

I still can't reproduce this, so what I'm going to do is:

  1. Merge the fix above.
  2. Add additional logging to the password manager in the next release, which will hopefully make it easier to figure out what the issue is.
genseric commented 4 years ago

image hey again. still a problem here. i am adding ss of the browser console. btw, it asks for the master password and does not ask again

PalmerAL commented 4 years ago

Here's another build to try with even more logging: https://send.firefox.com/download/c781e23a1324b9da/#V6NV17xPbXS2CTgl7_wH6g

You could also try deleting the 1password CLI configuration file; it's located at ~/.op/config.

genseric commented 4 years ago

thanks, here's the output with the new version image

PalmerAL commented 4 years ago

Thanks, I see it now. This is an interesting problem actually. The 1password CLI doesn't have any obvious way to get all items matching a certain domain (that I could find), so the integration works by getting the domain and UUIDs for all items, filtering them to just the items that match the desired domain, and then getting the passwords for those items. Since you have a lot of items in your vault, the list of all items exceeds 1MB, which is the maximum output length that Node allows from a subprocess by default (source).

I think we could fix this fairly easily by increasing the max output to something much larger, like 25mb - my test account's list is 3100 bytes for 7 items, so if that represents the average, that would increase the maximum allowable size from around 2400 items to around 60,000. I do wonder if the performance of loading that many items is actually going to be good, but I'll make a build to try out.

Also, how many items do you have in your vault?

PalmerAL commented 4 years ago

new build: https://send.firefox.com/download/d2bad14def389f43/#7gD3uFbbOG76xIsAd9tBVA

genseric commented 4 years ago

Thanks, I see it now. This is an interesting problem actually. The 1password CLI doesn't have any obvious way to get all items matching a certain domain (that I could find), so the integration works by getting the domain and UUIDs for all items, filtering them to just the items that match the desired domain, and then getting the passwords for those items. Since you have a lot of items in your vault, the list of all items exceeds 1MB, which is the maximum output length that Node allows from a subprocess by default (source).

I think we could fix this fairly easily by increasing the max output to something much larger, like 25mb - my test account's list is 3100 bytes for 7 items, so if that represents the average, that would increase the maximum allowable size from around 2400 items to around 60,000. I do wonder if the performance of loading that many items is actually going to be good, but I'll make a build to try out.

Also, how many items do you have in your vault?

more than 5k.

PalmerAL commented 4 years ago

Were you able to try that build and see if it works?

genseric commented 4 years ago

yes, here's the output. image

PalmerAL commented 4 years ago

That looks like you have a credential saved for the website you're opening that doesn't include a username or password; is that the case?

Here's another build that ignores credentials that are missing a username or password: https://send.firefox.com/download/fea0a9b4b0980246/#OL8PaBxWwfOMNyW7ER06Sw