Add ability to sync group structure with KeeShare

welusa commented 5 years ago

Summary

Hi there, first of all, sorry for my terrible english and sorry for wasting your time, but I am too much of a noob to deal with this myself. I stumbled across the KeeShare feature in KeePassXC. Now I am trying to get a hang of how this is working. I am able to synchronize two databases with it. Now to my problem. Synchronizing the entrys works just fine, but the groups aren't getting synchronized. Is there any way to get this to work, so that i can create a subgroup in a synchronized group and it is going to synchronize it as well? I really don't know if this is a feature request, maybe it's only me being to stupid to use this. But when I import or synchronize it on a different device i only get the entrys, all in the group i activatet the keeshare feature for.

Desired Behavior

If i create a sub group in a shared group it should be synchronized / imported to other devices as well. Not only the entrys.

Possible Solution

Context

I would really like this, because i kinda like sorting things. In the future i want everyone in the office to have his own database, in which i can synchronize only the groups he needs or he is supposed to have. Giving him the structure of the groups i chose in my database would make it way more easily to use.

droidmonkey commented 5 years ago

Make sure you have both import and export enabled in the KeeShare settings:

welusa commented 5 years ago

I have enabled those everywhere. Just tried again. If I create a new group in a keeshared group, the entries made in the new one appear in the original keeshared group, without adding the new subgroup.

welusa commented 5 years ago

grafik grafik Maybe those two pictures show my problem a bit better.

ckieschnick commented 5 years ago

In short, the feature was designed to work exactly this way. We thought about synchronizing group structures with KeeShare, but after several discussions we decided against it since there a lot of edge cases where its hard to specify and understand the behavior concerning the synchronization of groups and subgroups.

The advantage of only sharing entries is, that receiver is free to rearrange shared entries (moving them into new subgroups, moving them outside of the importing group the share since the synchronize feature will locate those entries - or even deleting them locally).

droidmonkey commented 5 years ago

Ah, I did not even know this was the behavior. Perhaps there should be an option to "preserve share group structure" which would imply the behavior of enforcing a particular entry share into a particular location.

welusa commented 5 years ago

Thanks. So I guess a way to work around this could be to share every group individually. Not nice but works for me. Other wise I totally get why you did it this way. It just isn't the case for me.

Jonathan White notifications@github.com schrieb am Mo., 22. Apr. 2019, 14:40:

Ah, I did not even know this was the behavior. Perhaps there should be an option to "preserve share group structure" which would imply the behavior of enforcing a particular entry share into a particular location.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/keepassxreboot/keepassxc/issues/3045#issuecomment-485408606, or mute the thread https://github.com/notifications/unsubscribe-auth/AL4BWQS4IIG25MK5IVP4OBDPRWW5VANCNFSM4HHJYRJQ .

mrjk commented 5 years ago

Yep, I agree with droidmonkey, using flat only files limits the interest in my case. It would be nice if keeshare could act as a symlink to another DB, not having the possibility to share the password folders is a no go for me. I hope this feature will be available at some points :) Thanks for you amazing work guys ;)

ckieschnick commented 5 years ago

@mrjk you may have a look at https://github.com/keepassxreboot/keepassxc/issues/2658. I think this would be a better fit for your description.

dsonck92 commented 5 years ago

Not entirely sure if it warrants a new bug report but, @ckieschnick, the current import/export (and sync) behavior doesn't exactly match what is described:

If I export (export or sync) a group, the group gets flattened, as already known
If I export (export or sync) a group, and I move the item outside the group, the item is removed from the container
If I import (import or sync) a group, and the keeshare file does have structure, this structure is kept
If I move an entry around inside a keeshare group, it stays. If I then directly modify the keeshare database by moving the entry to another place, it will also be moved in the parent database when it imports/syncs again

So as the status is currently:

To a keeshare database will flatten, and in fact export will remove anything moved outside
From a keeshare database will enforce the keeshare hierarchy if present, but can be moved outside and still update from the container

Interestingly enough, this actually fits me a bit because I can make several local databases that pull from different shared keeshare databases. These databases are the ones I keep active on my local machines to autofill/type passwords. And I simply modify the keeshare database files so they retain hierarchy. However I understand this would not entirely be the correct use case.

I think ideally it should be configurable to keep or flatten the hierarchy. Where, if it's flattened, the destination hierarchy is kept. As I see it currently, this means including a checkbox for export, and modify how it exports based on this state

crosscodr commented 5 years ago

In my opinion, the flattening of the database, when synchronizing with keeshare is not an expected behavoir. At least, it should be decidable to sync/export with flattening or not. In my case, I first imported a password-group with many supgroups from a shared keepass-database into my private one. Then I made some changes in my local version of the imported group and found the option to synchronize instead of just import from another keepass-db. When I did this, the shared keepass-db was flattened without any warning. I think, this behaviour does not reflect, what the word 'synchronize' means. In my opinion, the synced group should be equal including all subgroups in both locations. Took me some time to repair the shared db :( I considered this a bug and just wanted to open a new bug-report, before I found this issue. I would endorse to label this issue as a bug.

welusa commented 5 years ago

I fully agree to what crosscodr is saying. I startet this issue because i am searching for a solution for a small company. While trying out the share feature I destroyed my database multiple times too. It'd be awesome if there was some way to sync the database without flattening. I am definately not trying this one more time on my main database, since i have learned my lesson now.

ckieschnick commented 5 years ago

I hope I don't repeat the documentation, but it looks like a misunderstanding for using this feature. Therefore I'll try to clarify the usage of KeeShare:

Neither the database used by the synchronize feature nor the import/export of KeeShare is intended to be used as real database. They are just a transfer format which happens to be a database readable by KeePass. The containers should be in between real databases opened by KeePassXC which use KeeShare to pull or push the changes to the transfer container. The transfer container can an be overwritten or updated anytime by any client - therefore you shouldn't open such a database in a regular scenario - it is intended to be written and read only be KeeShare.

The idea is the following: Client A <=[synchronizes with KeeShare]=> (Transfer container) <=[synchronizes with KeeShare]=> Client B

There is no explicit support for scenarios like this (unless the KeePassXC team says otherwise): Client A <=[synchronizes with KeeShare]=> (Transfer container) <=[opens directly]=> Client B in this case, I think you can use the standard synchronize feature of KeePassXC.

Nonetheless, if you want to open a transfer container directly - make sure it is read only since every change maybe overwritten by another client using KeeShare. At least from my side, there is no guarantee, that transfer format doesn't change, so this may not work in future versions.

ckieschnick commented 5 years ago

Interestingly enough, this actually fits me a bit because I can make several local databases that pull from different shared keeshare databases. These databases are the ones I keep active on my local machines to autofill/type passwords.

@dsonck92 ... this is more or less how we intended to use the feature. It is a bit cumbersome to setup, but in the end it is the most easy way to understand and handle complex scenarios without to much headache. The smaller containers are easier to compose and divide according to responsibilities (which was the main driver behind this feature).

And I simply modify the keeshare database files so they retain hierarchy. However I understand this would not entirely be the correct use case.

Do you edit them by hand? Or are those containers just the leaf groups of your group tree (which is mirrored in the client databases)? The second one was the one had in mind since this provides the most ease. Enforcing the structure of your passwords to other users didn't look that nice - but in a single user multiple computer scenario, the structure sharing would be ok.

crosscodr commented 5 years ago

Thanks @ckieschnick for clarification. It makes sense when you explain it this way. I still think this is not the most obvious behaviour for users. Maybe this could be made more clear in the documentation.

dsonck92 commented 5 years ago

I meant by that, If I edit the keeshare container (unsigned) by hand, an import forces the structure of entries that change. This was unexpected for me but does work for me personally and I think would work for others participating in this ticket.

But as you already mentioned (hence I was saying "I understand this would not entirely be the correct use case") that directly modifying the keeshare container is not supported. I was merely doing that to understand specific corner cases:

Modifying the container (importing into a keepass database):

Item is added to container
- What happens if I move this item around in a keeshare import -> container stays the same (expected)
Item is modified in container
- What happens to the moved item -> moved item is still updated (expected)
Item is moved/created in container in subgroup
- What happens now -> moved item is moved back to the shared group (unexpected) or created in a subgroup under the share (unexpected)
Item is removed
- Item disappears from database -> expected

Modifying the sharegroup (exporting to keepass database):

Item added to subdir
- How is it stored -> flattened items (expected)

So I didn't expect import to respect any structure, which comes back to this request. I'm currently trying to write some proof of concept code that adds the aforementioned "Flatten/Keep Structure" functionality to (for now) the Export/Sync function. A checkbox shows up when Sync or Export is chosen which should skip the flattening step if flattening is not desired. The import code can remain as-is as far as I can see now. Though this does not fully take into account every corner case. However, I personally think that adding a checkbox to get the original behavior or the (for some) expected behavior is useful. That gives end users the ability to choose which vision they require.

At a later stage, it could be decided whether this flattening can also happen during import as well, so the "any present structure is forced" can be avoided on demand as well.

Currently having some unexplainable crashing bugs which I expect are due to some library issues, which hold me back from testing my work. I'll give some updates and if you have any input feel free to share those.

dsonck92 commented 5 years ago

I actually got the code to work, turned out to be a silly mistake: I forgot to actually make the change to KeeShareSettings::Reference also affect sorting and equality. The commit https://github.com/dsonck92/keepassxc/commit/2d6ad06d14da5f789b9adf2037b7937bdf7be684 has the changes.

The reason I did not yet create the pull request for it is because there are some open bugs that annoy me:

The way flattening works on import will not prevent items being moved back if the container structure changes. I want to know what could cause this before I submit it because I want the "do not keep structure" to allow people to ignore structure on new structured containers. Giving the best of both worlds.
When 2 databases sync with keep structure, they always cause one of the two to be "unsaved". Saving touches the container again which makes the other import and become "unsaved". Saving the other export again, etc. etc. This is more of a small inconvenience issue but could completely break autosave and makes using yubikey a chore as it would constantly ask for the yubikey to give the secret.

dsonck92 commented 5 years ago

@crosscodr for your use case I would recommend the following thing (keeping in mind that ckieschnick might still change the format and also noting it won't support secure containers), which I will personally use on my main database until my proposed code changes get polished enough for inclusion and when the developers accept this idea.

Create one or more shared containers that only import, noting that importing will retain structure.
Also create an AutoOpen group under the root group of the keepassxc database. This is the auto-open feature that is already present for some time.
Add entries for each shared container, pointing the URL field to the container file and putting the associated password inside the password boxes

Now, whenever you open up your database, it will also open up the related shares in the background. This gives you (at least in my eyes) all the features you want:

Autotype/Browser fill in will work properly as the main database (which contains the imports) is mostly active
Structure is kept
You can still update the containers directly
It is still shared, so you can give those containers to others you trust

Again, note that, as ckieschnick pointed out, the container format is not to be modified directly, mostly because it can also be overwritten by an export or sync. And because format might change (which would essentially mean that at some point you cannot import anymore, and have to use the auto-open feature to access your separate containers, however I don't see a very big threat there)

Another way would actually be to try my fork and see how well it works for you, and possibly give some input on it if there are some things that could be better. Note that it's very alpha, I will try to use it myself carefully but I might create some stupid bug that simply erases everything from a container. Yes, I have had that happen while trying to flatten the database.

dssouza-ti commented 4 years ago

Hi @dsonck92

Any new on this feature? I have a use case where keeping the structures would be awsome, so I'm really looking forward to seeing it fully functional. In the mean time, I'll try to use your commit with a test database for a while to see if anything strange happens.

dsonck92 commented 4 years ago

I've been a bit preoccupied with other tasks, mainly as I was very annoyed by bug 2 that I mentioned (always unsaved). However, as I've updated my keepassxc install in the meantime (and thus not use my own patch), it seems like it also happens without my patch so only the first bug remains.

I would certainly appreciate you trying it out, and see if there is more interest in it. Currently the auto-open workflow does work. But I do remember ckieschnick's warning that the format might change, which would mean the patch would be better as you never have to touch the internal format ever.

If I have more time/more interest is shown, I can continue to squash that bug of "flattening will still modify existing items". It would be very helpful to have:

keep structure -> items will be forced into structure on import / items will keep structure on export
discard structure -> items will be merged with existing on import, not altering structure / items will be flattened on export

dsonck92 commented 4 years ago

At least I've updated my branch to reflect latest changes in develop https://github.com/dsonck92/keepassxc/tree/feature/sharing_keep_structure

droidmonkey commented 4 years ago

Which bugs are you talking about, please link to the issue reports.

dssouza-ti commented 4 years ago

@dsonck92 I have been testing your changes with my team at work and it works really well for our use case. Since we are only interested in the "Keep Structure" part of this, the bug you mention does not affect us.

@droidmonkey I just opened #3791, which describes the bug 2 that @dsonck92 has stated above.

stridger commented 4 years ago

So is there something one can do to help to get this merged? Seems like a no-brainer feature which would actually make KeeShare useful...

HLeithner commented 4 years ago

@dsonck92 I tested your branch and it works fine thank you.

@droidmonkey did I missed the reason why this is not merged into main release?

droidmonkey commented 4 years ago

Submit it as a PR...

HLeithner commented 4 years ago

Since I'm not the coder of this (and my c++ knowledge is about 0) I don't want to create this PR but I hope @dsonck92 will do it.

dsonck92 commented 4 years ago

@HLeithner I will look into this again within a few days

dsonck92 commented 4 years ago

There are a few idiosyncrasies that prevent me from submitting a PR:

Put both on sync with "keep structure" and they will indefinitely keep themselves "unsaved"
Put both on sync but one on "keep structure". If you move the item in the "keep structure" between folders, the "flatten" one will move back to the root. If you move the item in the "flatten" one to a folder, the "keep structure" one moves back to the root
- According to some previous comment (forgot where) the flattening should prevent items from moving, which is the main feature of making "keep structure" separate on both sides of the sync. This because to keepassxc, it should not look like the item has moved.

The overall idea of the changes are:

If a person does not want to impose a structure, they can export flat
If a person does not want to get structure imposed, they can import flat
If a person does want to manage structure, they can export/import with structure

I'm pretty sure both issues are related to the history tracking and I need to have another go at it to try and figure out what causes it and how it can be fixed.

droidmonkey commented 4 years ago

I'm really not in favor of allowing two instances using the same shared database to impose different saving methods. The flatten/structure setting should be stored in the shared database file's custom data. This way the setting can be enforced immediately on loading the database.

HLeithner commented 4 years ago

Personally I don't see an use case for flattern sync maybe for import (one way sync).

dl6nm commented 4 years ago

I agree with @HLeithner and I also don't see an use case for me/us flattening the structure on sync.

My team colleagues and I would like to share a bunch of credentials and we decided together storing credential structured. But for now the missing structure sync prevents us switching to KeePassXC. (This also valids my private environment where I'm sharing credentials with my wife and my childrens.)

welusa commented 4 years ago

It'd be really useful in a work environment, where one individual takes care of the database and the rest is only using the credentials or updating them at most. Maybe it'd be even more useful, if the original owner could lock the structure somehow, but this might go too far.

dssouza-ti commented 4 years ago

I also agree. If I want to flatten the structure, why do I need your feature anyway? It already happens by default. This feature is only good for the keep structure part, so I believe we are ready to see this PR submited. :)

HLeithner commented 4 years ago

There are a few idiosyncrasies that prevent me from submitting a PR:

Put both on sync with "keep structure" and they will indefinitely keep themselves "unsaved"

Put both on sync but one on "keep structure". If you move the item in the "keep structure" between folders, the "flatten" one will move back to the root. If you move the item in the "flatten" one to a folder, the "keep structure" one moves back to the root

According to some previous comment (forgot where) the flattening should prevent items from moving, which is the main feature of making "keep structure" separate on both sides of the sync. This because to keepassxc, it should not look like the item has moved.

The overall idea of the changes are:

If a person does not want to impose a structure, they can export flat

If a person does not want to get structure imposed, they can import flat

If a person does want to manage structure, they can export/import with structure

I'm pretty sure both issues are related to the history tracking and I need to have another go at it to try and figure out what causes it and how it can be fixed.

At least 1. should be fixed. Point 2 could be solved like @droidmonkey described to save the keep-structure property in the file.

dssouza-ti commented 4 years ago

At least 1. should be fixed.

@HLeithner and @dsonck92, see #3791 that I opened describing this issue. It's on the 2.6.0 milestone assigned to @droidmonkey.

dsonck92 commented 4 years ago

@dssouza-ti the flattening mode actually prevents this from happening in my feature branch

islamadel commented 4 years ago

If i create a sub group in a shared group it should be synchronized / imported to other devices as well. Not only the entrys.

I would appreciate such a feature too, as we have different cataegories of data entries which we would like to keep sorted.

b-fuze commented 3 years ago

Can there at least be some tool that can import/export group structure? I have a lot of groups, and while synchronizing would be great, it'd be nice to at least have some tool to copy-paste groups or such.

dsonck92 commented 3 years ago

@b-fuze I currently use the following workaround to get this (note, it only works with unsigned containers unfortunately):

Create a new database with a secure password
Open the database which has your your structure as well
Ctrl+Drag the desired groups to this new database (this will copy them over)
Open up the actual database you want to "sync" it into (I say "sync" as it's going to be one way)
Create a group to contain the structured group, setup KeeShare to only import, and specify the path of the new database you just created in step 1 with your chosen password
Create a top level group "AutoOpen" with an entry which has as URL the path to the new database, and password the same as you used in 1.

The end result of this is, when you open up the database which should receive the synced passwords, it will also automatically open up that new database from 1. You can switch to this tab to edit your passwords, and once saved, it will get replicated to your main database. I use this with great success while I'm still working and finding time to fix the sync with structure. The only downside is you have to switch to a new tab to edit the shared passwords, and must not forget to save and switch back to the global one once done.

Now, as I said, this only works with unsigned containers, because the signed variant is a zip file which carries the certificate and database together. The unsigned variant is simply a kdbx file, ready to be opened directly by keepassxc. Keep in mind that it's technically undefined behaviour by the author, but it does work great for now.

Ravinou commented 3 years ago

I have been using keepassxc for several years as an individual. For teamwork, we are constantly confronted with this technical limitation. It is impossible on some projects to work without a tree structure, there are sometimes dozens of passwords in keeshare that make keepassxc unreadable and complicated to use.

Is there open funding for this feature? I regularly make donations, but I would really like to participate in the implementation of this crucial feature which for the moment forces us to spend on solutions such as Bitwarden or Lastpass.

droidmonkey commented 3 years ago

It's on my list for keeshare refactor. No funding necessary I get paid by donations.

Ravinou commented 3 years ago

Thank you for your work and your answer @droidmonkey :heart:

ZorioLan commented 3 years ago

Is it worth waiting for synchronization via ftp, webdav, etc?

droidmonkey commented 3 years ago

You are on the wrong issue

JackDev-cc commented 2 years ago

Any further update to this? I know its open funding but this is a multi-year open issue any timeframe? as i'd also love this feature.

xmgr commented 2 years ago

What's the progress for this feature? I'd also love having the ability to preserve the group structure :)

droidmonkey commented 2 years ago

None made for a while, I've been working on other problems.

meow464 commented 1 year ago

I used a "syncthing" group to send and receive new entries, which have to be manually organized, then simply copied my database and keeshare file to the new machine to preserve already existing groups. If you are using syncthing it's important to enable versioning for the sync folder.

Merry Christmas. KeepassXC rocks!

xstable commented 1 year ago

come from linked issue to here, I do comment to be noticed if anything new here.

lindhe commented 1 year ago

@xstable No need to comment for that. You could simply have clicked the "Subscribe" button at the right hand side of the page (or at the bottom, in mobile view) to get subscribed for notifications. :)

keepassxreboot / keepassxc