Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for browsers integration by mimicking KeePassHttp plugin (un-blobbed) #111

Open
wants to merge 93 commits into
base: master
from

Conversation

@eugenesan
Copy link

@eugenesan eugenesan commented Jul 9, 2015

Un-blobbed version of implementation based on the following trees (in chronological order):
* typz
* keithbennett
* jdachtera
* Ivan0xFF
* denk-mal

Note: There are few reverted commits that are unrelated to the PR but might be considered for merge also.

Typz and others added 30 commits Apr 11, 2013
Allows using passIfox (firefox) and Chromeipass (chrome).
This gives the option to reload the database.

TODO:
 - Settings for reloadBehavior (ask, reloadUnchanged, ignore)
 - Improve notification, by using a header instead of dialog: nicer, less
intrusive, gives more options to user, and works better when multiple databases
are open.
 - Keep tab order on reload.
Search options are presented in a context menu on the search field, as well as
links in search header.
@tuxayo
Copy link

@tuxayo tuxayo commented Apr 23, 2016

Since this project is under GPL, doesn't this automatically mean all derivative contributions are?

Since one have to fork the project to contribute, the fork have the same license. On each of the contributors page, we can we the project + their additional contributions published under the GPLv2+.

So there is no problem regarding licensing here.

@bradst
Copy link

@bradst bradst commented May 19, 2016

While running a build of the droidmonkey/keepassx_http branch, I noticed that it appears to be listening on 0.0.0.0 while using the default configuration (Host=localhost).

$ sudo netstat -lpn
Active Internet connections (only servers)
Proto Recv-Q Send-Q Local Address           Foreign Address         State       PID/Program name
tcp        0      0 0.0.0.0:19455           0.0.0.0:*               LISTEN      23563/keepassx

Doesn't this mean it's essentially public-facing? My /etc/hosts is standard, so nothing happening there. Changing the configuration to listen on 127.0.0.1 fixes the issue.

@eugenesan
Copy link
Author

@eugenesan eugenesan commented May 19, 2016

@bradst You should use https://github.com/eugenesan/keepassx/tree/2.0-http-totp instead.
It's a more updated branch which is synced with original implantation with 0.0.0.0 listening issue addressed.

P.S.
1st: Droidmonkey's branch is updated with the fix.
2nd:
Seems like @bradst is correct, for some reason the fix is not working.
I could swear it was working previously.
My guess the issue is enabled IPv6.
There was no IPv6 on system where the fix was developed.
I'll take a look at it.

P.S.S
As a temporary workaround use "127.0.0.1" instead of "localhost".

P.S.S.S
Fixed.

@eugenesan
Copy link
Author

@eugenesan eugenesan commented May 19, 2016

@EmbraceUnity
Just to be clear on the license subject.
The only part of the contributions in PR that weren't properly licensed in first version of the PR, were licensed in following versions of the PR (with permission of the contributor).
As of today all files in PR either forked from "similar" files in main project (and therefor inherit the license) or licensed with license identical to the project's one.

@voidus
Copy link

@voidus voidus commented May 24, 2016

I just tried to merge master into this, which seems to be not trivial. Master switched to qt5 in the meantime, too. But as far as I can tell, that merge needs to happen before this can go any further.

@voidus
Copy link

@voidus voidus commented May 24, 2016

Okay, looks like I'm totally confused right now. What branch is the current candidate for merging into master? If it's not this one, can this PR be closed?

@eugenesan
Copy link
Author

@eugenesan eugenesan commented May 24, 2016

@voidus, @droidmonkey already merged it with QT5 support. Please take a look at: https://github.com/droidmonkey/keepassx_http
This PR is intended for use with stable version.

@voidus
Copy link

@voidus voidus commented May 24, 2016

Ah, great. :) So, what do you think about closing this one and nudging @droidmonkey to open a new pull request? ;)

@eugenesan
Copy link
Author

@eugenesan eugenesan commented May 24, 2016

@voidus

  1. This PR is for stable branch and since majority of people use stable versions this PR probably should stay.
  2. I guess @droidmonkey could create a PR for master
  3. I am very skeptical regarding merge of this or potential @droidmonkey's PR since all code in this PR should be revised and cleaned-up to comply with project's code styling (see #111 (comment)) but nobody is ready to commit to that task. I rather see this PR as a reference, mainly for people willing to build their own build with this functionality.
@voidus
Copy link

@voidus voidus commented May 24, 2016

Ah, I assumed that this pull request was referring master. Sorry about that :)

@droidmonkey
Copy link

@droidmonkey droidmonkey commented May 25, 2016

At this time I would be wary about pulling this plugin into the core of keepassx. As far as I know there has been no security audit on the integrated capability, and as noted above there may be a security hole with 0.0.0.0 being the default server.

Recommend people in this thread submit bug tickets to my fork to address security and implementation and then we can look to merge into the main keepassx source. I am willing to put some leg work into making the plugin stand up as a core component of keepassx, I use it daily.

https://github.com/droidmonkey/keepassx_http

@alexanderadam
Copy link

@alexanderadam alexanderadam commented Jul 3, 2016

This PR will have it's birthday next week.

Is there any plan or consensus how and when keepasshttp can or will be integrated in KeepassX?

@heX16
Copy link

@heX16 heX16 commented Jul 18, 2016

How about to make global fork (KeePassX3) and merge all unaccepted PR?

@Nairwolf
Copy link

@Nairwolf Nairwolf commented Jul 18, 2016

@heX16 : I think someone should send a mail to the main maintainer of the project and ask him if he's still on the project.

@tuxayo
Copy link

@tuxayo tuxayo commented Jul 24, 2016

@Nairwolf

Or open an issue in the bug tracker to ask if they can still handle the maintenance work or if they need help.

https://dev.keepassx.org/projects/keepassx/issues

For some reason I can't log in with GitHub (no verified email address provided although I have one in my GitHub account) does anyone can?

@multiwebinc
Copy link

@multiwebinc multiwebinc commented Jul 31, 2016

@debfx @BlueIce I remember a couple of years ago I was looking for a good password manager and I stumbled upon keepassx and the first thing I noticed is that there was no direct browser integration and I would have to copy and paste every time I wanted to log in. That was a complete show stopper for me and I instead have been using KeePass2 with mono. Fast forward a couple years to today and there's this great PR sitting here (and 57 other PRs that go back 3.5 years!), but both project maintainers appear to have gone AWOL for the past few months. Guys, if you can't keep up with the PRs, delegate the work to others. Add more people to the team. It's been forked over 400 times. Surely someone in there might be willing and able to help you guys out to make this more usable.

@shaform shaform mentioned this pull request Aug 7, 2016
9 of 9 tasks complete
@tuxayo
Copy link

@tuxayo tuxayo commented Aug 11, 2016

@multiwebinc

Surely someone in there might be willing and able to help you guys out to make this more usable.

Maintaining requires a much higher involvement(more intense + higher duration) and codebase knowledge than doing PRs.

I there anyone motivated to try that? (on a fork with a different name if no response from the maintainers)

@Polsaker
Copy link

@Polsaker Polsaker commented Sep 30, 2016

Any news on this? Or is there anyone willing to just fork the project and start maintaining it with all these pending PRs?

@droidmonkey
Copy link

@droidmonkey droidmonkey commented Sep 30, 2016

@tuxayo @Polsaker

I am willing to call my fork of keepassx with the http plugin built into it the official repository. It is already up to date with the latest commit on this keepassx. Porting over the pull requests won't be too difficult.

Anyone else interested in this move? I want to keep this project alive!

https://github.com/keepassxreboot/keepassx

@symbiogenesis
Copy link

@symbiogenesis symbiogenesis commented Sep 30, 2016

Fantastic! Thanks droidmonkey. There's 66 open pull requests that need closing. I have already been using your repos anyways. Need to get your repos into AUR and PPAs.

@jgkamat
Copy link

@jgkamat jgkamat commented Sep 30, 2016

@droidmonkey probably you should create an org and add a few more members you trust (so this dosen't happen again)? I'm hoping eventually the version in the repo's could point to your version instead of this one, unless this one is restored. 😄

EDIT: This would probably be easier if you gave it a new name? Maybe thats an option?

@droidmonkey
Copy link

@droidmonkey droidmonkey commented Sep 30, 2016

OK, created an org with an appropriate name and migrated the repo over to it. If you are interested in joining this revival please send me a PM with your credentials (experience) and preferably your LinkedIn account profile or similar.

https://github.com/keepassxreboot

@fcore117
Copy link

@fcore117 fcore117 commented Sep 30, 2016

droidmonkey: plans for binary releases?

@droidmonkey
Copy link

@droidmonkey droidmonkey commented Sep 30, 2016

@fcore117 once the dust settles and some critical PR's are pulled in we can plan a push to the upstream package repos. I have never done that before, so expertise in that area would be helpful. I'll open an issue on the reboot to track that concern.

@jonafato jonafato mentioned this pull request Oct 18, 2016
3 of 7 tasks complete
jonafato added a commit to jonafato/nixpkgs that referenced this pull request Oct 19, 2016
The `keepassx2-http` fork has been moved to a new organization and
renamed to `keepassx-reboot`. For more details on the change, see the
discussions in GitHub issues [1][2].

Included changes:
- Rename the `keepassx2-http` package to `keepassx-reboot`
- Fetch source from correct (moved) GitHub repository
- Update the version to the latest release
- Change the `homepage`, as these projects are likely to diverge over
  time
- Add `keepassx2-http` to `aliases.nix

[1] keepassx/keepassx#111 (comment)
[2] keepassxreboot/keepassxc#40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet