keepass2android icon indicating copy to clipboard operation
keepass2android copied to clipboard

Keeshare Support

Open plyght opened this issue 1 month ago • 12 comments

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

plyght avatar Nov 30 '25 08:11 plyght

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 avatar Nov 30 '25 16:11 beveradb

@beveradb will do!

plyght avatar Nov 30 '25 23:11 plyght

@beveradb here is the docs for keeshare support i've made w/ github gist: https://gist.github.com/plyght/72c365d062b94f9d2c0a862ca91671f4

plyght avatar Dec 01 '25 00:12 plyght

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 avatar Dec 01 '25 11:12 PhilippC

@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.FilePath because of this or would you suggest we switch to the KeePassXC format?

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!

plyght avatar Dec 02 '25 03:12 plyght

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?

PhilippC avatar Dec 02 '25 17:12 PhilippC

  • 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

plyght avatar Dec 02 '25 17:12 plyght

hey @PhilippC should be ready for review!

plyght avatar Dec 04 '25 23:12 plyght

@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?

PhilippC avatar Dec 08 '25 09:12 PhilippC

@plyght could you please also update https://gist.github.com/plyght/72c365d062b94f9d2c0a862ca91671f4?

PhilippC avatar Dec 08 '25 10:12 PhilippC

@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?

PhilippC avatar Dec 08 '25 10:12 PhilippC

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!

PhilippC avatar Dec 08 '25 10:12 PhilippC