Issues with packaging browserpass for Debian

[meskes]

I'm running a selfbuild extension on Linux system with Firefox and Chromium with latest app. Whenever I click on the extension symbol it shows me which URL it searches for, but never returns anything. As long as the popup is active it only shows that turning circle symbol but never any password entry. And also no error message, just nothing.

I've double checked the app version, it is up-to-date.

Any idea how to debug? Or even better, any idea what might be going on?

[erayd]

Check the developer console for both the popup window, and the background page. Do you see an error message in the console?

[meskes]

Not sure if I used it correctly, but I do see this:

TypeError: Cannot read property 'autoSubmit' of undefined

Whether I have auto submit configured or not does not make a difference.

[maximbaz]

Do you see in which file this happens, maybe even a line number?

I suspect it's this line:

https://github.com/dannyvankooten/browserpass/blob/22e8af03308ca489beeb51f6bf66714b14240740/chrome/script.browserify.js#L41

[erayd]

@maximbaz I haven't been able to reproduce this issue, but the line you've flagged is an issue - it's trying to access an object which may sometimes be uninitialised.

I've just opened PR #246 to ensure that the settings are always present - I don't know whether that's enough to fix this bug, as I've not been able to reproduce it here.

[maximbaz]

I was unable to reproduce it too, I like the idea in #246 - merged.

@meskes could you try to reproduce the issue with the latest master?

[meskes]

Current master works a little bit longer. If I access e.g. github it now shows me my github entry, but as soon as I click on it I get the neverending circle again. New error message is:

content.html:1 Error in event handler for (unknown): TypeError: Cannot read property 'error' of undefined
at chrome-extension://cbfnbkdnigbahbkcikfppmaonggdbcpa/script.js:314:20

The line in questions is the "if (response.error)" in

function getLoginData() {
searching = true;
logins = resultLogins = [];
m.redraw();

chrome.runtime.sendMessage(
{ action: "login", entry: this, urlDuringSearch: urlDuringSearch },
function(response) {
if (response.error) {
return resetWithError(response.error);
}
window.close();
}
);
}

[maximbaz]

Hmm, something is very wrong with your extension. Just curious, does it reproduce with extension in the webstore? Could you try to clone the repo from scratch, and then build the extension with $ make command?

[meskes]

No, it does not. I'm working on the Debian package for it and am trying to figure out why it worked correctly with 2.0.11 and 2.0.13 but not with 2.0.18. The build process is different because Debian does not have browserify, but only browserify-lite. I cannot see any functional differences in the js files, though.

Also, I did try copying the 2.0.18 files from firefox.zip to the location the package resides in, to no avail, some problem. However, this obviously was before the patch.

[erayd]

@meskes

The build process is different because Debian does not have browserify, but only browserify-lite.

Are you able to explain how you have changed the build process? If the 'official' builds are working, but yours is not, that might give us a clue as to where the problem might be.

If a reference would be helpful, I can confirm that builds via docker (see CONTRIBUTING.md) are working correctly against the current master.

[erayd]

@meskes One other thing that may be relevant if you're building the browserify bits differently: as of 2.0.18, options.js is now also generated via browserify (from options.browserify.js). This required a makefile change, so if you're using a custom makefile, perhaps that change was missed?

[meskes]

The sources are in https://salsa.debian.org/webext-team/browserpass

The makefile is debian/rules.

Chances are I missed something, but then why didn't copying the files from the zip file help?

[erayd]

The makefile seems OK. I'll try building from your sources and see what happens, hang on.

[erayd]

I can confirm that building from the master branch of the debian repo you have linked above works normally, and results in a usable extension. However, there is a unit test failure when building the host app, and that failure is also present for the master branch of this repo - I've opened #248 for this.

If you run the 2.0.18 browser extension that you have built, but use the host app binary from the github release, does it work correctly?

steve@neith /tmp/browserpass $ docker run --rm -v "$(pwd)":/browserpass browserpass-dev
yarn
yarn install v0.23.3
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...
Done in 5.01s.
dep ensure
node_modules/.bin/prettier --write chrome/styles.css chrome/script.browserify.js chrome/otp.js chrome/otp.css chrome/options.browserify.js chrome/inject_otp.js chrome/inject.js chrome/background.browserify.js
chrome/styles.css 322ms
chrome/script.browserify.js 565ms
chrome/otp.js 73ms
chrome/otp.css 55ms
chrome/options.browserify.js 97ms
chrome/inject_otp.js 18ms
chrome/inject.js 106ms
chrome/background.browserify.js 103ms
node_modules/.bin/browserify chrome/background.browserify.js -o chrome/background.js
node_modules/.bin/browserify chrome/script.browserify.js -o chrome/script.js
node_modules/.bin/browserify chrome/options.browserify.js -o chrome/options.js
cp chrome/host.json chrome-host.json
cp firefox/host.json firefox-host.json
cp chrome/policy.json chrome-policy.json
cp chrome/{*.html,*.css,*.js,*.png,*.svg} firefox/
go build -o browserpass ./cmd/browserpass
go test
PASS
ok      github.com/dannyvankooten/browserpass   0.003s
go test ./pass
--- FAIL: TestDefaultStorePath (0.00s)
        disk_test.go:16: user: Current not implemented on linux/amd64
        disk_test.go:27: 1: '/root/.password-store' does not match ''
FAIL
FAIL    github.com/dannyvankooten/browserpass/pass      0.016s
make: *** [Makefile:62: test] Error 1

[meskes]

Nope, same effect. Hmm, it seems like you build from my source but not with my makefile, right?

[erayd]

I built using the default makefile (i.e. the makefile from the master branch root). Does yours live elsewhere?

[maximbaz]

Unit test fails because it cannot retrieve the "current user" in the docker environment, I don't think that is relevant to this issue.

@meskes in order not to deal with browserify or other dependencies, just grab the browserpass-src.tar.gz from the releases, that archive is built specifically for package maintainers. However firefox.zip also has pre-compiled javascript sources, so it should work just as well...

[erayd]

@meskes I've diffed your master against 2.0.18 of this repo - the only difference is the presence of a debian/ subdirectory, and I can't find a custom makefile in there. Everything else is identical.

Could you please advise how you're building this?

[maximbaz]

@erayd you missed above:

The makefile is debian/rules

[meskes]

@erayd the makefile is called debian/rules
@maximbaz the problem with testing from your tarball now is that it does not contain the latest patches, so chances are I run into #246 again

[erayd]

Oops - I did indeed miss that. Thanks :-).

[maximbaz]

Here you go, based on the current master: release.zip

[meskes]

this seems to work, let's see where the difference is ...

[meskes]

It seems to be background.js

[maximbaz]

It became to be "browserified" only recently 😉 So... problem solved?

P.S. Will you please submit a PR to the README.md when it will be possible for Debian users to install browserpass via official package management tools?

[erayd]

@maximbaz Might be worth rolling 2.0.19, if the changes in master have solved a packaging problem. That way the debian package version will be properly in sync with the repo release tags. Otherwise if someone reports a problem, and the debian version of 2.0.18 contains more than what's actually in the 2.0.18 release...

[maximbaz]

Will do once @meskes confirms that the issue is solved and this ticket can be closed 👍

[meskes]

@maximbaz problem is not solved, I just noticed that it works with your background.js but not with mine, why is a different matter

As for the package, it is already available since 2.0.11, but I can surely do a PR for that

[maximbaz]

background.js is an generated script via browserify, if you don't want to use the pre-generated one from browserpass-src.tar.gz or firefox.zip, double check how you are generating the file.

[meskes]

browserify-lite $$(pwd)/chrome/background.browserify.js --outfile $$(pwd)/chrome/background.js

If it's just browserify-lite that's not working correctly, it's not an issue here, but if it's coming from someplace else ...

[maximbaz]

Just curious, why not use pre-generated sources and avoid having troubles with browserify and other dependencies alltogether? It is also a burden for you to maintain, for example a week ago background.js did not require compilation, now it does, but browserpass-src.tar.gz contained and will contain sources that work "out of the box" with no extra effort required.

[meskes]

The idea behind this is to compile everything from sources and by doing so getting the latest version of the dependencies as well.

[maximbaz]

That's not very good, we have yarn.lock and Gopkg.lock to make sure everyone is using the same versions of everything to avoid issues that cannot be reproduced by everyone.

If a dependency needs to be updated, I'd much prefer seeing a PR that updates the lock files, and then everyone is using the code that is built using the same version of tools.

[meskes]

Yeah, that's the difference between the software development and the distribution approach. There is actually a long thread on debian-devel about finding ways to make both work at the same time. Historically the problem was solved by introducing shared libraries. Anyway, I'm just trying to cope

[maximbaz]

6b5b236 updated all dependencies, so you can feel better at using the pre-compiled sources 😉

The decision is up to you of course, but I'm afraid I won't be able to support multiple build configurations, if a bug report would say "it works fine with the official release or when built from master, but doesn't work via the debian package", I'm afraid I would just have to close those bug reports with "use the official builds".

[meskes]

Fair enough, I'd prefer using your build system, too, but unfortunately it's not (yet) possible in Debian. Anyway, thanks for your help. If I find something not related to the Debian build itself, I'll open a new ticket.

[erayd]

@meskes Is using the docker build environment an option for you? That will still compile completely from source.

[meskes]

@erayd Actually no, the basic idea is that everything is buildable within the distro, so all dependencies and all toold should be packaged. There is some discussion that this has to be changed, but no action yet.

[maximbaz]

You could argue that using browserpass-src.tar.gz is okay because all it takes to make that archive is to download dependencies and to run browserify, and browserify is just a concatenation of scripts in one big script 😉 There's no binary code in there, everything is sources.

[maximbaz]

I'll close because it seems there's nothing else we can do here on our side.