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

KeePassHttp support #247

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
@jameshurst
Copy link
Contributor

jameshurst commented Oct 6, 2014

I've been using this and no one else has reported any problems, so I guess this is ready for a pull request.

@pavelgurvich

This comment has been minimized.

Copy link

pavelgurvich commented Oct 7, 2014

How can I enable it? It's always grayed out in the interface.

@mstarke

This comment has been minimized.

Copy link
Member

mstarke commented Oct 7, 2014

Use @jamesrhurst fork and use the keepass http branch. The code is not present in the main repo.

@pavelgurvich

This comment has been minimized.

Copy link

pavelgurvich commented Oct 7, 2014

@mstarke thanks for the tip. Will this be included in the main repo eventually? I can wait a bit.

@jayphizzle

This comment has been minimized.

Copy link

jayphizzle commented Oct 7, 2014

If someone could compile me a version with KeePassHttp support i would test it.
(Moved from Win/Keepass to Mac/Macpass and the KeePassHttp feature is the only one missing)

btw: is there any plan to release beta nightlies ?

@mstarke

This comment has been minimized.

Copy link
Member

mstarke commented Oct 7, 2014

@pavelgurvich of course it will get included ;).

@kumters

This comment has been minimized.

Copy link

kumters commented Oct 7, 2014

I can send you a version build on my yosemite, send me your email please...

On Tue, Oct 7, 2014 at 11:37 AM, jayphizzle notifications@github.com
wrote:

If someone could compile me a version with KeePassHttp support i would
test it.
(Moved from Win/Keepass to Mac/Macpass and the KeePassHttp feature is the
only one missing)

btw: is there any plan to release beta nightlies ?


Reply to this email directly or view it on GitHub
#247 (comment).

Peter Jankovsky

+420-724-952661@phone
347696680@icq
tifay78@skype
social: about.me/kumters

@mstarke

This comment has been minimized.

Copy link
Member

mstarke commented Oct 15, 2014

I'm just getting through this. I've cloned your KeepassHTTP repository as I'm thinking everything custom to MacPass should run under one hood. I will offer a pull request as I'm in the process of cleaning up the code to run with 10.8 as well (it's only the base64 encoding stuff that needs to be changed)

Another problem is the different code style. We should agree to one single style on all the MacPass files.

@jameshurst

This comment has been minimized.

Copy link
Contributor

jameshurst commented Oct 15, 2014

What do you mean by parts custom to MacPass? As far as I know all of the code in KeePassHttp-ObjC can function independently of MacPass.

As for code style, I agree that things in MacPass should conform the the current coding style. Although I'd say that it is fine for KeePassHttp-ObjC to have a different coding style as it is an independent library.

@mstarke

This comment has been minimized.

Copy link
Member

mstarke commented Oct 16, 2014

Mh. Right about the independence, but the project does not provide the dependencies (JSON; GCDWebserver).

I'm a bit off as to how to handle the dependcies. Maybee it's time to use cocoapods?

@mstarke

This comment has been minimized.

Copy link
Member

mstarke commented Oct 16, 2014

Argh. I just realised that KissXML is a dependency of KeePassKit. Damn. I should seriously clean up the dependencies before pointing finger ;)

@jameshurst

This comment has been minimized.

Copy link
Contributor

jameshurst commented Oct 17, 2014

I think that using cocoapods would be a change for the better.

@jameshurst

This comment has been minimized.

Copy link
Contributor

jameshurst commented Nov 5, 2014

Just rebased with your master branch since I noticed there were merge conflicts.

@mattmb

This comment has been minimized.

Copy link

mattmb commented Nov 15, 2014

@jamesrhurst really big thanks for working on this! Noticed two things. Browser plugins only seem to pick up entries in top level groups. Also new entries get created at the root, would be awesome to specify a default group in the config.

@jameshurst

This comment has been minimized.

Copy link
Contributor

jameshurst commented Nov 15, 2014

@mattmb I'll take a look into it only finding entries in top level groups. I guess I didn't notice that since my database only consists of top level groups. As for having new entries get created in root instead of presenting some sort of dialog, I've left that as a TODO in the code which hopefully I'll get to doing sometime. For now though you can just drag the entry to another group.

@jameshurst

This comment has been minimized.

Copy link
Contributor

jameshurst commented Nov 15, 2014

@mattmb It should be able to find entries in all groups now.

@mattmb

This comment has been minimized.

Copy link

mattmb commented Nov 16, 2014

@jamesrhurst thanks! That works a treat :-)

@jaimeagudo

This comment has been minimized.

Copy link

jaimeagudo commented Nov 21, 2014

Thank guys, I don't have a clue about Objective-C but let me know if I can help as tester or whatever, can't wait for this :) 👍

@jameshurst

This comment has been minimized.

Copy link
Contributor

jameshurst commented Nov 21, 2014

@jaimeagudo Testing would be great! Just use it as your normally would and if you find any issues I'd be glad to fix them.

@mstarke

This comment has been minimized.

Copy link
Member

mstarke commented Dec 10, 2014

@jamesrhurst I'm trying to get 0.5.2 out. Turns out, Autotype is still a bit wonky (who would've guessed? ;) ) and there might be some more 0.5.x releases to address some issues. I'm trying to add history support (and a changed edit interface) to MacPass after that for a 0.6 release. When history support is done (or sort of done) I'd like to merge your feature. I'll ping you again to ask if you can pull in the changes to make the merge possible.

@rfilmyer

This comment has been minimized.

Copy link

rfilmyer commented Feb 24, 2015

@jamesrhurst James, would you remind rebasing again so I can try this with 0.5.1?

@jameshurst

This comment has been minimized.

Copy link
Contributor

jameshurst commented Feb 24, 2015

I actually just rebased against master yesterday so it should have everything in 0.5.1.

@rfilmyer

This comment has been minimized.

Copy link

rfilmyer commented Feb 24, 2015

That explains it, I haven't checked since yesterday :p

@rfilmyer

This comment has been minimized.

Copy link

rfilmyer commented Feb 24, 2015

@jamesrhurst I'm having trouble building your fork on 10.10. I loaded the 10.8 SDK, but it fails building at this line in the HNHUi submodule.

@jameshurst

This comment has been minimized.

Copy link
Contributor

jameshurst commented Feb 24, 2015

I'm building using the 10.10 SDK, maybe that's the issue? Also are your submodules up to date?

@rfilmyer

This comment has been minimized.

Copy link

rfilmyer commented Feb 24, 2015

Never mind, I checked out your fork's master instead of the KeyPassHTTPbranch.
Now everything builds fine.

@Mirocow

This comment has been minimized.

Copy link

Mirocow commented Mar 17, 2015

why you not merge this pullrequest?

@mstarke

This comment has been minimized.

Copy link
Member

mstarke commented Mar 18, 2015

The reason is given in #247 (comment)

@sheavner

This comment has been minimized.

Copy link

sheavner commented Apr 25, 2015

@jamesrhurst I've been using the http integration for a week or so with chrome latest on Yosemite 10.10.3 and no problems so far. I do have a feature request tho - to be able to ignore some folders. I've migrated over from KeePassX v0.43 or something that was pretty old and am still using KP1 files because they work everywhere I need them to work. KPX and maybe some of my other tools save off every modification to a Backups group and there are multiple copies of each entry. I'd like to be able to configure it to ignore the Backups folder.

@mstarke It would also be nice if it didn't prompt me everytime I save to KP1 that I'm losing information. Am I really known to be losing information at this point or does it just say that with every save?

@shaded2

This comment has been minimized.

Copy link

shaded2 commented Jun 21, 2015

Thanks for the zip @rfilmyer. seems to be working well so far. I can finally unpin keepassx from my dock!

@tristan-k

This comment has been minimized.

Copy link

tristan-k commented Jun 22, 2015

Thanks @rfilmyer. In the end I succeeded in building it.

@Floppy

This comment has been minimized.

Copy link

Floppy commented Jun 23, 2015

Using @rfilmyer's build with PassIFox, and it seems to work perfectly. Good job :D

@jaimeagudo

This comment has been minimized.

Copy link

jaimeagudo commented Jun 23, 2015

Thanks @rfilmyer it works just fine for me with chromeIpass on Chrome 45.0.2435.0 canary (64-bit) Mac OS 10.10.3 💥 👍

@max-b

This comment has been minimized.

Copy link

max-b commented Jun 29, 2015

Built the @jameshurst keepasshttp fork with xcode 4 for yosemite and working fine for me with chromeipass.

Does require a bit of tinkering with xcode build settings. I think the default in the repo is to build for 10.8. @shaded2 if you're still interested in walking through the build process we could set up some out of band chat.

@nazriel

This comment has been minimized.

Copy link

nazriel commented Jul 5, 2015

Works very well with
https://github.com/mmichaa/passafari.safariextension/releases/tag/v1.0.beta2

@jameshurst great job :)

@mstarke maybe there's a way to introduce better workflow so pull requests like this one, which works fine won't rot?

Now I just hope that Safari extension will mature enough, because at the moment it is rather limited compared to the ones for Chromium/Firefox :)

@mstarke

This comment has been minimized.

Copy link
Member

mstarke commented Jul 6, 2015

Better workflow: Yup.
The main problem right now is lack of time on my side. This results in a very strange state for the project. There are still a lot of places that lack a lot of polish and functionality and those need to be addressed if any of the "bigger" stuff gets done (namely your suggestion for a plugin system)

@mstarke

This comment has been minimized.

Copy link
Member

mstarke commented Jul 6, 2015

Oh. Before I forget - the main reason for this lingering state is the 10.9 target. MacPass in all it's current releases works on 10.8 so including this branch would break that and I didn't want to spent time back-porting to 10.8 and just finish some other stuff and then up to 10.9 for the whole project as a minimum requirement.

@nazriel

This comment has been minimized.

Copy link

nazriel commented Jul 6, 2015

@mstarke I perfectly understand you, lack of the time is also a big issue for me.

Although I had no idea about 10.8 breakage with this PR so I'm sorry for my ignorance in this regard.
I must admit that I'm fairly new to OSX/iOS world as a user and programmer.

Anyways, cheers up and keep up good work, MacPass is already great at this stage 👍

@mstarke

This comment has been minimized.

Copy link
Member

mstarke commented Jul 7, 2015

No worries :). I'm the one not getting shit done at the moment so I'm the last person to point fingers ;-)

@mstarke

This comment has been minimized.

Copy link
Member

mstarke commented Jul 7, 2015

@jameshurst should we try to use this featuer to define plugin API so we can move the implementation into a plugin-bundle? I think this is a nice way to determine some core functionality a plugin system has to offer.

@TomK

This comment has been minimized.

Copy link

TomK commented Jul 20, 2015

Hey, @mstarke thanks for the good work on MacPass!
I currently use the keepasshttp branch by @jameshurst and thought I'd mention here (since github isses are not enabled on the fork) i've just noticed the about box still references CocoaHTTPServer which i believe you swapped out for something else in your fork.
Thanks again!

@Floppy

This comment has been minimized.

Copy link

Floppy commented Jul 21, 2015

Does this PR add this icon in the status bar? screen shot 2015-07-21 at 17 23 47

As far as I can tell, the icon appears only when KeePassHttp is running. However, there are no options - is necessary to display it (assuming this is where it comes from)?

@mstarke

This comment has been minimized.

Copy link
Member

mstarke commented Jul 21, 2015

Take a look here: #331

@Floppy

This comment has been minimized.

Copy link

Floppy commented Jul 21, 2015

Oh, thankyou, I hadn't spotted that. Appreciate the pointer :)

@sir-brickalot

This comment has been minimized.

Copy link

sir-brickalot commented Oct 12, 2015

Having trouble using the @jameshurst fork:
I am using it in combination with PasslFox and I never get a match. Using the context menu "Fill User & Pass" opens a window to choose from a couple of entries. All those entries appear to be in the 'Recycle Bin' in MacPass. (Sidenote: Emptying the trash needs a 'Save file...' too, to really delete the files.)
I used the binary from @rfilmyer because I didn't manage to compile from source.

System: 10.10.3 (OSX86)

Any idea?

@Floppy

This comment has been minimized.

Copy link

Floppy commented Oct 13, 2015

@sir-brickalot normally when it does that for me, it's because the URL isn't set in the keepass database for that key. Do you have those set correctly for the keys you're trying to use?

@sir-brickalot

This comment has been minimized.

Copy link

sir-brickalot commented Oct 13, 2015

@Floppy the behaviour is the same no matter if URL is present or not. Strange is that as long as there are entries in the trash, EVERY entry will be shown as a n option. They don't have to have any correlation to the actual website.
Just tested again with just one key in the trash and fields get actually filled even if it has no similarity to the site. Not the same behaviour on every site. Sometimes I get the "No login found."-warning.

Reproduceable:

  • create a fake key with random content and place it in the trash (only one item in trash!)
  • open https://my.asos.com/signin and choose "Fill User & Pass"

(sorry for the stupid domain ;) )

@robbie-c

This comment has been minimized.

Copy link

robbie-c commented Oct 14, 2015

Can I help with this at all? Tried to compile https://github.com/jameshurst/MacPass/tree/keepasshttp and got a build error. Here is exactly what I did:
git clone https://github.com/jameshurst/MacPass.git
cd MacPass/
git checkout keepasshttp
git submodule init
git submodule update
open MacPass.xcodeproj/

Then when I build it fails with the following error:
/Users/MyName/Projects/MacPass/KeePassKit/Core/KPKEntry.m:618:21: Property 'key' not found on object of type 'KPKBinary *'

I'm on OS X Yosemite 10.10.5, Xcode 7.0.1, compiling against 10.11.

Happy to help out e.g. if you can't reproduce this yourselves. Also in general. This is a really awesome project and I'm grateful for all the work that must have gone into it!

@sebastianrothe

This comment has been minimized.

Copy link

sebastianrothe commented Oct 17, 2015

KPKBinary has the following properties only, data and name. I think the property should be name, not key.

@jameshurst

This comment has been minimized.

Copy link
Contributor

jameshurst commented Oct 18, 2015

I took a look at this issue and it appears to be an issue with the version of KeePassKit that we have checked out. The issue in KeePassKit got fixed in MacPass/KeePassKit@ab34319, but because of the API changes made in MacPass/KeePassKit@25b5fd8 I can't update the KeePassKit dependency to the latest verison. The problem with MacPass/KeePassKit@25b5fd8 was that it removed public support for adding/moving an entry to a group at a specified index which is required to support reordering entries. @mstarke is there another way to reorder the entries or should that API be made public again?

@mstarke

This comment has been minimized.

Copy link
Member

mstarke commented Oct 19, 2015

@jameshurst To make KeePassKit a framework I started pulling private API into separate headers and in that I also tried to reduce the public API. My initial goal was to use a better parent/child structure but that didn't turn out so nicely. Anyway. The index on an entry was useless from my point of view since entries get sorted all the time, but I guess matching goes through them in the order they are stored inside the array and so order does count there. It's no problem to make the API public again, but I found only one usage to move the configuration entry in MPDocumentQueryService.m#L85 which should also work with a simple - (void) moveToGroup:(KPKGroup*)group;

@jameshurst

This comment has been minimized.

Copy link
Contributor

jameshurst commented Oct 19, 2015

@mstarke When building MacPass with the master branch of KeePassKit the issues that I'm seeing are MPOutlineDataSource.m#L163 and MPOutlineDataSource.m#L168 which both require the atIndex parameter.

Checking right now, it looks like the index parameter of NSOutlineViewDataSource returns -1 for any drop that I do, so perhaps - (void)moveToGroup:(KPKGroup *)group would suffice there.

@mstarke

This comment has been minimized.

Copy link
Member

mstarke commented Oct 19, 2015

@jameshurst Ah, didn't take a look at MacPass since I'm ahead locally with my trail (and error) run to get editing to work in a better fashion. That's the reason I did not push anything using the current KeePassKit code for MacPass yet. Sorry for any inconveniences this is causing!

@flinz

This comment has been minimized.

Copy link
Contributor

flinz commented Nov 11, 2015

@jameshurst I finally got around to trying the keepasshttp branch, but unfortunately i can not get it to work so far. I am on OSX 10.11.1 and built with Xcode 7.1.

The KeepassHttp server seems to be started (i can see the icon), however clicking the icon does not open any dropdown - don't know whether this is intended behavior.

To build it i had to circumvent the KeePassKit issue by changing binary.key --> binary.name in MacPass/KeePassKit/Core/KPKEntry.m line 618, which let me build (got this from the issue). Might it be that this breaks something?

@rodneyrod

This comment has been minimized.

Copy link

rodneyrod commented Nov 19, 2015

I'm building it for 10.10 with XCode 7.1 too, I had an error with the KPKEntry.m file too, but editing as per above enabled it to build.

However, the option to enable the KeepassHttp server is greyed out and there seems to be no way to enable it.

Also, is integration with KeeFox being worked on?

@mstarke

This comment has been minimized.

Copy link
Member

mstarke commented Nov 19, 2015

KeeFox uses KeePassRPC so this requires a port of KeePassRPC. I'm a bit bewildered why there isn't a common approach and everyone that creates a browser plugin tries to built a new solution. Anyway. Not my time to argue ;).

Also a side note. I've created a very raw but sort of working plugin system for MacPass and did a dirty port of KeePassHTTP over to it and it seems to be working.

@mstarke

This comment has been minimized.

Copy link
Member

mstarke commented Nov 26, 2015

Im closing this for now since I did port the fork to a plugin MacPassHTTP . There seems to be a bug in the plugin so it's not working correctly. But when fixed it should run.

Beware that the current master is a mess. I need to clean it up but I wanted to push all the changes out.

@mstarke mstarke closed this Nov 26, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment