Keeshare Support
Fixes #1161
-
New Features
- KeeShare integration: per-device shared DB paths, configuration UI, manual sync, background import/export, export-on-save and import-on-load hooks, and read-only protections for imported groups/entries; sync flow coordinated with OTP aux sync.
-
Tests
- Added unit tests and platform-agnostic helpers covering KeeShare group detection and extensive RSA signature verification scenarios.
-
Chores
- Test project configuration updated, assembly test access enabled, new UI resources/preferences added, and VCS ignore rule adjusted.
cc @beveradb & @a13xis
Nice work!
Please could you add some tests for the new code and for any integration points with existing code (if viable)?
Also, could you provide some instructions for how one would manually test the new functionality, or a video demo of it in action or similar? I should be able to figure out how to get a dev APK built and running on my phone but without any documentation I'm not exactly sure how a user should use this.
I've added lil' more to the bounty for this :)
@beveradb will do!
@beveradb here is the docs for keeshare support i've made w/ github gist: https://gist.github.com/plyght/72c365d062b94f9d2c0a862ca91671f4
Hi @plyght , thank you very much for starting to work on this! ❤️
I took a first look at your implementation and left some review comments. I also have some additional questions, partially because I don't know a lot about KeeShare.
- I didn't find documentation about KeePassXC's KeeShare/Reference. Why did you choose not to use this?
- Does KeeShare have any support for having different paths on different devices? Is your plan to keep the custom KeeShare.FilePath because of this or would you suggest to switch to the KeePassXC-Format?
- For Shares which are "Import" - would it make sense to switch to read-only UI such that modifications don't get lost?
@PhilippC
- I didn't find documentation about KeePassXC's KeeShare/Reference. Why did you choose not to use this?
i chose a simple keyvalue CustomData format for the initial implementation to get the basic functionality working quickly... i'm of course happy to add KeePassXC KeeShare/Reference format parsing for better interop. the structure isn't complex, i just prioritized getting import/sync working first. it's also worth noting that KeePassXC deprecated signature verification in their implementation.
- Does KeeShare have any support for having different paths on different devices? do you plan to keep the custom
KeeShare.FilePathbecause of this or would you suggest we switch to theKeePassXCformat?
yeah, the custom KeeShare.FilePath was intentional for cross-device support... different devices may have the share file at different locations (for example, /sdcard/Download/ on one device, cloud sync folder on another). KeePassXC's embedded path wouldn't handle this well. i'm open to suggestions though; we could support both formats, or add a device-specific path override
- For Shares which are "Import" - would it make sense to switch to read-only UI such that modifications don't get lost?
fixed; import groups are now read-only in the UI. i added a KeeShare.IsReadOnlyBecauseKeeShareImport() helper that checks if a group/entry is within an Import group, then i disabled add entry/group buttons in GroupActivity and the edit button in EntryActivity for those groups.
keep in mind i'm fairly new to Android dev, so please bear with me if i make any mistakes!
Thanks for the quick update @plyght ! Looks pretty good from reading the code. (I haven't tried to run it or tested it in the app yet.)
From my perspective, the next steps should be
- make sure that KeeShare groups created in KeePassXC can be used in KP2A. In the majority of cases, I guess this requires some KP2A-specific setup for accessing the file, i.e. the user probably needs to use the file selection dialog to browse to the file. The path should then be stored in some custom data, using the device ID as part of the key such that different paths can be used on different devices. (see src\keepass2android-app\KeeAutoExec.cs for something similar). I wonder if the most consistent way of doing this is adding a similar UI as with ConfigureChildDatabasesActivity? We should probably rename "Child databases" in the UI to "Child databases (KeeAutoExec)" to differentiate the two clearly? For KeeShare, I would never use the term "child database" but always just call it KeeShare (or KeeShare group).
- Add KeeShare logic also in SaveDb such that changes to KeeShare groups are synced/written to shared files?
What do you think?
- make sure that KeeShare groups created in KeePassXC can be used in KP2A. In the majority of cases, I guess this requires some KP2A-specific setup for accessing the file, i.e. the user probably needs to use the file selection dialog to browse to the file. The path should then be stored in some custom data, using the device ID as part of the key such that different paths can be used on different devices. (see src\keepass2android-app\KeeAutoExec.cs for something similar). I wonder if the most consistent way of doing this is adding a similar UI as with ConfigureChildDatabasesActivity? We should probably rename "Child databases" in the UI to "Child databases (KeeAutoExec)" to differentiate the two clearly?
- For KeeShare, I would never use the term "child database" but always just call it KeeShare (or KeeShare group). Add KeeShare logic also in SaveDb such that changes to KeeShare groups are synced/written to shared files?
@PhilippC sounds good, will get on these
hey @PhilippC should be ready for review!
@plyght I'm still reviewing the code, but I also realized that I cannot build the current state. Can you please double check that you have pushed all recent changes?
@plyght could you please also update https://gist.github.com/plyght/72c365d062b94f9d2c0a862ca91671f4?
@plyght The current implementation doesn't support the KeeShare/Reference data. How difficult do you think is it to get this? If a user has configured a KeeShare database and now opens the DB in KP2A, there is no KP2A-only way of getting the share enabled, right? I would have to fiddle with CustomData in KeePassXC first if I see it correctly?
with all the comments above please don't get me wrong: I like your work a lot and it really looks like it's on a very good way! Thank you!