After Firefox Beta crash OTP setting disappears

apiraino commented 4 years ago

Hi,

I have a strange problem that occurs randomly, I'm not sure I can provide anything actionable here.

I'm using Firefox 82.0b9 and browserpass-extension 3.7.1 and Browserpass host app version: 3.0.6.

Being an unstable version sometimes Firefox does its job of crashing -__-

I noticed that after recovering from a crash the option set to "enable support for OTP tokens" is lost.

When I close and reopen Firefox regularly this doesn't happen.

Any idea?

I might keep keep an eye and report if this behaviour disappears by ✨ magic ✨ in one of the next betas ...

General information

Operating system + version: Ubuntu 19.10
Browser + version: Firefox 82.0b9
Information about the host app:
- How did you install it?
- official release: Browserpass host app version: 3.0.6
Information about the browser extension:
- How did you install it? AMO
- Browserpass extension version as reported by your browser: browserpass-extension 3.7.1

Exact steps to reproduce the problem

Do my things on the browser, Firefox crashes because it has a bad day
Restart Firefox, recover the sessions
Browserpass "enable support for OTP tokens" setting lost the check

What should happen?

Browserpass settings should not be lost for any reason (once they're written in the localstorage, right?

What happened instead?

Browserpass "enable support for OTP tokens" setting lost the check

maximbaz commented 4 years ago

Yeah all we do is we store the settings in localStorage and read them every time from there. Is OTP the only setting that you use, that is not default? I'm just wondering if the entire localStorage is not accessible / corrupted or only this specific setting...

apiraino commented 4 years ago

Is OTP the only setting that you use, that is not default?

ah, good question. I have also the "dark mode" enabled and that setting is not lost

maximbaz commented 4 years ago

It's dark by default though, so you wouldn't be able to tell the difference... Could you enable light mode and use it until you reproduce the issue next time? My bet is that when OTP setting disappears, browserpass will also revert to black theme, and then we can conclude that Firefox somehow corrupts localStorage on some occasion

apiraino commented 4 years ago

Hey @maximbaz I have a way to reproduce this!

Open Firefox, ensure to have browserpass-extension set with light mode and OTP enabled
(optional) Close Firefox, open the file ~/.mozilla/firefox/<profile-id>/webappsstore.sqlite and verify that the settings are there
Open again Firefox, the settings are correctly set
Turn off the computer either by provoking a shutdown (long press laptop power button) or rip the power cables of a desktop :smile:
(optional) On the next reboot I verify that the .sqlite file still has the right settings (light mode and OTP enabled)
BUT after opening firefox they are erased and I am back to dark mode and no OTP
The above mentioned sqlite file is now empty

I suspect this has to do with some Firefox recovery procedure? But it's weird that only browserpass-extension loses the settings. I have a bunch of other extensions (included one that I mantain) and none seems to lose their settings :thinking:

Anyway, I realize it's rather a weird corner case, so feel free to either investigate or close this as wonfix :)

maximbaz commented 4 years ago

Thanks for the follow-up! 🙂

Interesting 🤔 While the real issue is probably in Firefox, it is nevertheless curious why only Browserpass is affected... I thought maybe the file gets empty because we unsuccessfully read it and then write back empty settings, but we do that only when settings actually change, such as if you click on something in options screen 🤔

I don't have any ideas to be honest, while as you say it's a weird case that might not be worth solving, it's an interesting puzzle, we could leave it open for a while, but even if we close it eventually... patches welcome 😄 I would love to learn what was the cause here...

erayd commented 4 years ago

Just a guess, but I wonder if Firefox has some sort of checkpointing process, and it rolls back in the event of an unclean shutdown. Given how important browsers are to most people, and the lack of widespread in-depth technical knowledge to self-troubleshoot, recovery needs to be absolutely bulletproof and automatic, and discarding a bit of local storage to reduce the risk of a recurring problem seems like a reasonable approach.

apiraino commented 4 years ago

I'll leave here a thought after investigating a bit the code.

You guys are using the Window.localStorage API which - according to MDN - seems to be discouraged because it can be deleted in "various scenarios". They suggest using the storage API because guaranteed to persist across sessions. The storage API requires the storage permission which I see you don't request.

Compared this to two other random extensions (for example mine and VIM vixen), they both use the storage API and request the storage permission in the manifest.

I'm not sure if these two facts are correlated, what do you think?

I tried hacking your code for a quick POC but couldn't make it work as quick as my time allows.

browserpass / browserpass-extension