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

Adding database writer #8

Closed
ppmathis opened this issue Dec 5, 2013 · 47 comments
Closed

Adding database writer #8

ppmathis opened this issue Dec 5, 2013 · 47 comments
Assignees

Comments

@ppmathis
Copy link
Member

ppmathis commented Dec 5, 2013

Maybe add support for modifying and saving KeePass databases. This would finally offer the possibility to e.g. load a database, change it's master password and resave it.

@ghost ghost assigned ppmathis Dec 5, 2013
@chadly
Copy link
Contributor

chadly commented Dec 10, 2013

👍

@chadly
Copy link
Contributor

chadly commented Dec 10, 2013

Do you also have plans to implement editing of groups/entries?

@ppmathis
Copy link
Member Author

Ofcourse I do - the downside is that there will be some breaking API changes, but it should not be a lot of work. You can watch the project from time to time, I've started with a recode and just pushed one giant commit to 'develop'. Will now proceed with making some smaller commits (like you should when using git) - it is advancing well.

If there any feature requests or suggestions, feel free to open an issue.

@rodolfocaldeira
Copy link

+1 This would be an amazing feature to add.

@ppmathis
Copy link
Member Author

ppmathis commented Mar 2, 2014

Just a short update: I had some really busy weeks and therefore didn't make a lot of progress in regards to keepass.io, but I will release a version with write support in the next 2 weeks.

@ppmathis ppmathis added this to the v1.0.0 milestone Mar 4, 2014
@gesellix
Copy link

do you have any updates on write support?

@ppmathis
Copy link
Member Author

Thanks for your message. I started working on it last week but am right now on vacation. Will continue coding when I come back, so a release in the end of April 2014 should be possible. If there are any suggestions or questions, feel free to say so.

@gesellix
Copy link

I'm interested in adding write support to https://github.com/gesellix/keepass-node. Since I just saw your other project https://github.com/NeoXiD/keeweb.io, I would be quite interested to use your projects with my Angular frontend. So, without precise suggestion I would only ask you to keep potential clients of your projects in mind.
I would also like to help coding or testing in case you need some help. You mentioned the develop branch, so I does it make sense to look at it and try your code or is it too early?

@ppmathis
Copy link
Member Author

As you may have seen, I stopped working on keeweb.io and currently focus on the library itself, so feel free to use keepass.io within your project.

You should definitely take a look at the "develop" branch, it is a complete recode and has a proper API to access the database. Also this branch will get write support, so you should stick to it.

If you should need help at any time, feel free to drop me an email to dev (at) snapserv (dot) net.

@ppmathis
Copy link
Member Author

ppmathis commented Jul 7, 2014

Hello again, @gesellix @chadly

As you might have seen (or not), I wasn't able to release write support until April 2014. Had a lot of other stuff to do and kinda lost interest in this project. Now I'm back though, and a few minutes ago, I've pushed a whole new branch with database loading and saving support.

Don't use the code yet, some features are still missing, but I'm implementing them right now. There's one thing missing though: The API itself, to access the entries and groups. What do you think, is just access to a plain JSON object as with v1 sufficient? Or would you prefer a complete API? Would be glad to hear some opinions about that.

@ppmathis ppmathis removed this from the v1.0.0 milestone Jul 7, 2014
@gesellix
Copy link

gesellix commented Jul 7, 2014

First things first: 👍 for coming back :)

Regarding your question: would it be ok to access plain JSON first and add an API as needed? That way you/we could learn which use cases are relevant or most important so that the API would fit to the clients' needs. If you would prefer to add an API as early as possible, I would like to try the updated version first before giving you a qualified answer here.

@ppmathis
Copy link
Member Author

ppmathis commented Jul 8, 2014

Thanks for your answer, @gesellix. I've made a few more commits today and now database loading & saving is fully integrated. Also, there is now a basic example to illustrate the usage of KeePass.IO: https://github.com/NeoXiD/keepass.io/blob/development/examples/001_basic_usage.js

There's not a real API yet, but you should already be able to do most of the things. If you have any questions or suggestions, feel free to say so.

@ppmathis
Copy link
Member Author

ppmathis commented Jul 9, 2014

@gesellix I've just released v1.0.0 on GitHub and NPM. Most parts of the code are now covered by tests and there is also a basic example to get you started with keepass.io. Further documentation will follow tomorrow with v1.0.1.

@gesellix
Copy link

gesellix commented Jul 9, 2014

great, I'll try and give you feedback!

@gesellix
Copy link

@NeoXiD hey, I'm currently trying to use the updated release. This is no qualified feedback yet, but I already have questions/thoughts :)

My previous implementation relied heavily on the JSON structure, especially the groups and entries. Using the new version with its raw database, the structure has changed. That's no big issue for me, but now I'm rethinking on how I would prefer working with the database. Obviously, I don't like my implementation to break when you change some internal/raw representation of the database, so an API would be best (without removing the fallback to access the raw database).

I would now like to describe my tool and use cases and we can discuss about how the API could look like:

My tool is a web frontend, which gets the complete keepass database from a backend and knows about the structure of a database file. That way it doesn't need to make too many requests, but it has to re-implement logic on how to traverse the database structure.
After loading a database file, it is shown in a similar way like the KeePass2 tool does, with a tree browser on the left and entries for a selected group on the right.

So, the user needs to have an overview on the group tree, and they expect a group's entries to appear when the group is selected. Entries are initially shown by title, without details. On selecting an entry its details are shown, with the password being hidden. The user may now either copy the password (without making it visible) or show it.

With the described use case, I would need the API to provide something like the following interface/methods:

  • getGroupTree(db): returns more or less the same as the current JSON, but without the 'Entry' element.
  • getEntries(group): finds the group with the given ID and returns an overview of the entries of that group. I'm not sure if it should already return the entries with all details (e.g. the password).
  • getDetails(entry): if getEntries() doesn't return the entry's details, this method will become necassary

... well, this wasn't meant to be such a long comment. So, what do you think? Am I completely off the right track or would you say the described methods could be part of an API?

@ppmathis
Copy link
Member Author

@gesellix Hey there, thanks for such a detailed response! Indeed the JSON structure has changed, because it was not the raw database. The old stable parsed some parts of the raw database and created its own structure. There were some fields missing though, the code was quite messy and you didn't have access to every field.

The current stable gives you the raw database exactly how KeePass stores its data. This raw structure can be a bit frustrating to parse, I agree on that. Especially because Booleans for example are still present as a String. Also thanks for explaining how you're going to use keepass.io, this might simplify the API design a bit.

Basically, I had two ideas, one of them was already mentioned by you. The first idea doesn't involve much abstraction, and would work in a similar way as you told above. Methods like setGroupTree(), setEntries() or setDetails() would be also necessary. The user would have to deal a lot with JSON structures, but the API stays simple.

Second possibility would involve a bit more abstraction and fluent interfaces, in a way like jQuery does it. I've written some prototype / example code, which can be found on the following Gist: https://gist.github.com/NeoXiD/3fbf3903c62aec569395. What do you think about that?

Thanks in advance for your reply and have a nice day. PS: I am on vacation for two weeks soon, so don't worry about the project being dead again :)

@gesellix
Copy link

@NeoXiD hey, the gist looks very good to me. I guess most web developers should be familiar with the jquery style, but I'm not so sure about node developers. I personally would go with the second possibility, especially when it comes to creation of nodes.

Reading nodes or a complete group tree, I would wish to have a single request to retrieve a complete group tree (a simplified version of the raw database), but this special case would probably not be part of your api, since it could be implemented on my side as kind of a convenience function.

In your place, I wouldn't try to hide the JSON structure too much, because I like low level apis as long as they aren't too hard to understand :) Perhaps it makes sense to provide three levels of API:

  • raw
  • basic (getter/setter style)
  • fluent

@ppmathis
Copy link
Member Author

@gesellix Thanks for your feedback. I think that the idea of three different API levels is a great idea - raw is already implemented, basic will be quite easy to do and only fluent will require more bits of code. I'll think about your request of reading whole group trees, I do not see a straight way yet to implement that method as part of the API.

I don't know how much time I'll have free before my vacations, but I will definitely start coding these things when I'm back.

@ppmathis
Copy link
Member Author

Hi @gesellix! As promised, I've started working on the basic API and published a feature branch for that. It isn't finished yet, but you could already give it a try. So far, I've implemented setEntries(), getEntries(), setGroupTree(), getGroupTree(). Here's my development example file:

// The loadFile() function has some breaking changes too.. You no longer need
// the "api" parameter, as you can request the API directly from the db object.
// Examples (see later on): db.getBasicApi(), db.getRawApi()
// So it will be now: db.loadFile(databasePath, function(err) { ... }
//
var basicApi = db.getBasicApi();

var myGroups = basicApi.getGroupTree();
myGroups[0].Groups = [];
console.log('Parent group: ' + myGroups[0].Name);
myGroups[0].Name = 'Basic API works!';
basicApi.setGroupTree(myGroups);

var myEntries = basicApi.getEntries(myGroups[0].UUID);
myEntries.splice(0, 1);
basicApi.setEntries(myGroups[0].UUID, myEntries);

var rawDatabase = basicApi.findOrphanedEntries();
console.log('========== ORPHANED ENTRIES ==========');
console.log(require('util').inspect(rawDatabase, { depth: null, colors: true }));

var rawDatabase = db.getRawApi().get().KeePassFile.Root.Group;
console.log('========== RAW DATABASE DUMP ==========');
console.log(require('util').inspect(rawDatabase, { depth: null, colors: true }));

Note: I've decided that after deleting a group within the group tree, the entries won't be deleted. They still remain as "orphaned entries", which can be found by using "findOrphanedEntries()". I'm going to implement a "deleteOrphanedEntries()" in 1-2 hours too. The idea behind that was, that after deleting a group, these entries can still be accessed and moved into another group, for example. However, as soon as you reload/save the database, these entries will be lost forever.

Edit: I've added some comments about the loadFile() method above.

@gesellix
Copy link

gesellix commented Aug 1, 2014

hi @NeoXiD, I'll be on holiday for about 3 weeks, so please have some patience, I'll check the API update later!

@ppmathis
Copy link
Member Author

ppmathis commented Sep 8, 2014

@gesellix Sorry to bother you, but did you have time to test the API out? Thanks in advance for your reply.

@gesellix
Copy link

gesellix commented Sep 9, 2014

@NeoXiD Hey, I didn't have a chance to test the new API, yet. But I'll come back to you soon, give me a week or so :)

@gesellix
Copy link

@NeoXiD I just tried your feature/basic-api branch, updating my code to reflect your api changes. Reading still works as expected, I'll have a look at the new write support.
...sorry for the delay...

@korve
Copy link

korve commented Sep 29, 2014

I've tested modifying db entries which worked fine. Are you planning on adding functionality to add/ new groups/entries?

@ppmathis
Copy link
Member Author

@gesellix @korve Thanks for trying it out! Adding new entries or groups should be already possible, but on a veeery low-level way of doing it. I'll add some additional helper methods, examples and tests next week.

@korve
Copy link

korve commented Oct 1, 2014

@gesellix I'd love to help - is there any documentation about the raw db format?

@gesellix
Copy link

gesellix commented Oct 1, 2014

@korve I'm not aware of a good documentation other than the actual raw contents of a keepass file. Would you like to help improving the keepass.io or the keepass-node write support? keepass-node only tries to adapt the keepass.io api to make it more easily available for a web browser. So, in which area would you like to help? I guess you can help in keepass.io and keepass-node as well :)

@ppmathis
Copy link
Member Author

ppmathis commented Oct 1, 2014

@korve Thanks for offering me your help! There's no documentation at all, I reverse engineered the original KeePass sourcecode. You can save doing that hassle though - I exactly know how to do all the things I promised. Just got a lot to do in real life so I didn't have much time to continue here.

As said though, I'm going to have time next week and will implement the API functions. I'm sure though that @gesellix would appreciate your help too - would be nice to have a secure, clean and fast web based interface!

If there are any API wishes / suggestions, please tell them now :)

@gesellix
Copy link

gesellix commented Oct 4, 2014

@NeoXiD I'm working on the https://github.com/gesellix/keepass-node/tree/write-support branch which integrates your basic-api feature branch. In case you're curious to know if your changes break anything on my side, just make a fresh npm install and npm test. I'll try to always use your most recent changes.

@korve The recent changes on the write-support branch of keepass-node should show you where I'd like to go regarding the backend with it's more or less restful api and the frontend with a minimal but useful apporach to access the keepass entries. What's missing now is the actual write support, with the use-cases being described at gesellix/keepass-node#2

@gesellix
Copy link

gesellix commented Oct 6, 2014

@NeoXiD would it be possible to distinguish between a wrong/missing password and a more technical problem? I'm asking because I'd like to return a more helpful http status code based on the actual problem (wrong user input 4xx or technical problem 5xx).

@ppmathis
Copy link
Member Author

ppmathis commented Oct 6, 2014

@gesellix kpio reports already when the database file got an incorrect header. Further distinctions aren't really useful / possible, as an invalid result could either mean a wrong master key OR a corrupted file. That's just how AES works, it will always spit something out.

@gesellix
Copy link

gesellix commented Oct 6, 2014

@NeoXiD alright, thanks :)

@gesellix
Copy link

@NeoXiD would you like to add some more convenient functions like below?

var group = basicApi.getGroup(group.UUID);
// update/overwrite/add an entry
group.setEntry(entry.UUID, entry);

Are changes written instantly to the kdbx file?

@gesellix
Copy link

forget my question about saving - i only needed to rtfm :)

@gesellix
Copy link

@NeoXiD adding an entry with a single Strings element like this:
{ UUID: "...", Strings:[{ "Key" : "key1", "Value" : "value1" }] }

It seems like the Strings array is converted to an object. When adding a Strings array with two elements, the array is saved as unchanged.

Is the conversion from array to object intentional?

@ppmathis
Copy link
Member Author

@gesellix You might have gotten a totally different email notice about that comment, but I decided to rewrite it from scratch as I made a silly mistake. First of, I've added these 2 group-related methods to simplify working with groups:

getGroup(groupUuid)
setGroup(groupUuid, groupData)

They will help you with getting a single group, so you don't have to loop through the tree for yourself, and with overwriting an existing group. You can even change the UUID within "setGroup()", it's your job though to update all the entries or they'll be orphaned.

Another possible method could be one for bulletproof UUID generation, that's something which can be easily added. Your other request would require objects/instances though for groups. I'm not thaaat sure if that makes sense within the basic API, that looks more like the planned fluent API. (Which, I have to be honest, has barely no progress, due to me being busy with other projects)

I'll investigate your last comment now.. Stay tuned.

@ppmathis
Copy link
Member Author

@gesellix About the array -> object conversion, I can only think about one reason... Are you saving and reloading the database inbetween? If yes, that conversion is due to xml2js. I could disable that flag, although -EVERY- XML node would then be an array, even if there's only one value.

@gesellix
Copy link

@NeoXiD about the xml2js issue: yes, I'm running some tests, where I load, change, save and reload the db. There's no production code yet, but I'm quite sure that I'll need to perform at least a load, change and save roundtrip, since I'll only load (and update) the db on a request.

Changing the xml2js setting to always produce an array sounds quite invasive and I'm not sure if I would like such a change... and I'm not even sure how many other people use your module. So, I guess we'll stick with it for now.

@gesellix
Copy link

Thanks for adding the get/set Groups functions, that would have been my next request ;)
I didn't understand though, why that's not so easy for entries. Can you elaborate on that? If it's too hard or weird to implement, it's ok for me, because I already handle that on my side.

Do you have details on the UUID requirements? I currently use the node-uuid module without encoding the uuid to base64. Do you have experience if that's according to any native KeePass requirements?

@gesellix
Copy link

only as short notice: adding entries and groups has been implemented using your basic api... now I'll have to write some missing tests and polish the edges ;-)

@gesellix
Copy link

gesellix commented Nov 7, 2014

@NeoXiD do you have plans to make a release only with the basic api? Would be great :-)

@ppmathis
Copy link
Member Author

@gesellix Uuugh, sorry first for being that inactive. If you should have added these "addEntry/addGroup" functions directly within the kpio library, feel free to open up a pull request. I'll try to polish some edges too this weekend and make a new release containing only the basic API. About your UUID question, node-uuid is fine, you should just check that the ID is really unique and not already somewhere in the database. KeePass doesn't actually care that much about the UUID, it just has to be base64-encoded.

@gesellix
Copy link

@NeoXiD no need to be sorry for latency;-)
I haven't changed any of the kpio code, but only added some adapting code for my needs on my side. You can think of it like convenience functions to handle save functionality with deciding whether to "add" or to "update" an entry or group.
So, please feel free to release the current api, and if you need help on polishing or testing just ask!

@ppmathis
Copy link
Member Author

@gesellix Just started working on updating and writing tests for the basic API branch. After doing that and adding some documentation, I'll make a release.

@gesellix
Copy link

grrrreeeaaaat 👍 :)

@ppmathis
Copy link
Member Author

I've just released version 1.1.0 of kpio on GitHub and npm. Feel free to test! This issue will be closed for now, as it is over a year old and the main goal (write support) was reached. If there should be any further feature requests, please open up separate issues on GitHub.

@gesellix
Copy link

I just updated the dependency on my side, all tests are passing.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants