search

shkdee / KeePassPHP

A port of KeePass Password Safe in PHP
MIT License
74 stars 23 forks source link

Allow read extra fields by regex #4

Closed sitnikovv closed 7 years ago

sitnikovv commented 7 years ago

Example: Read without extra fields: KeePassPHP::openDatabaseFile($path_to_database_file, $composite_key, $error)); or KeePassPHP::openDatabaseFile($path_to_database_file, $composite_key, $error, false));

Read all extra fields: KeePassPHP::openDatabaseFile($path_to_database_file, $composite_key, $error, "/.*/"));

Read extra fields, contains "eur" or "usd" in name (case insensitive): KeePassPHP::openDatabaseFile($path_to_database_file, $composite_key, $error, "/(eur|usd)/i"));

Read extra fields, beginning "newBranch" in name (case sensitivity): KeePassPHP::openDatabaseFile($path_to_database_file, $composite_key, $error, "/^newBranch.*/"));

shkdee commented 7 years ago

I see your point, but your solution is still a bit too specific IMHO. I focused on getting only password, url, title and username from an entry because that's what I needed, but it would be as easy to get all the strings in a map (key => value) and let the client code deal with it the way it wants. Would that be okay for you ?

sitnikovv commented 7 years ago

Hello. My request does not affect or alter the established format of the call in any way, that is, it has backwards compatibility and does not affect the existing functionality in any way. To use additional fields, you must specify an additional parameter (or not). Therefore, I believe that my request has the right to exist, although it's up to you, of course.

shkdee commented 7 years ago

Oh, it definitely has the right to exist, and I completely understand that you want to extend the features of KeePassPHP if it useful to you.

But I am just proposing a different implementation that would give you the same features, and that would be more generic and more useful in long term (I think).

shkdee commented 7 years ago

To be clear, I am not asking you to do that implementation - I already did a major part of it. I just wanted to check if that would be okay for you (because it would not be straight compatible with the changes of that pull request).

sitnikovv commented 7 years ago

In my projects, I use extra fields to restrict access (allow, deny, order) and to store additional information that I can not put into the main fields (for example, Oracle TNS). If you do it, I will use your implementation. It will be great if you can not get all the fields, but only selectively, but even if it is not implemented, I will change the external module that works with KeePassPHP. Thank you.

shkdee commented 7 years ago

Solved with 3ed3bc0c070da12cd76a08afefdd60030083d5f7. The relevant part regarding this pull request is that all string fields are extracted now ; you will find them all in the Entry class, and can access either through the $stringFields field or getStringField($key) method.

It's true that it gets all the fields (against your last remark); I pondered it for a long time, but I chose to do this because in any case you always need to read everything from a kdbx file, because if there are protected fields you must read them all in the order they were written (because of the random stream cipher). So I thought that instead of cluttering the code responsible for reading, I would let the client code decide what it wants to do with the full result ; filtering string fields out would not be hard.

I did add a filter to select what you can write back to the disk though (in KeePassPHP own json format), so you can at least chose to store only non-sensitive data.