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

User should be warned before deleting entry with a UID that is being REF'd by another object #852

Closed
r4j4h opened this issue Aug 7, 2017 · 1 comment

Comments

@r4j4h
Copy link

r4j4h commented Aug 7, 2017

When the user clones a key and maintains a reference to the original, the key that uses a reference is displayed grayed out with "Ref: " prefixing the field. The original key appears normal, as it did before cloning. If the original key is deleted and moved to the "Recycle Bin" there is no issue and things continue to function. If the original key is deleted from the Recycle Bin the cloned field remains gray but the Ref is broken and the underlying syntax is shown. Any keys that REF'd that key are now useless. I propose that a warning would be useful here to ensure the user is okay with this potentially dangerous choice.

It is understandable to expect user caution to be enough protect from this breakage accidentally, but with a large number of objects, especially when nested in folders, it's not hard to imagine this happening without due diligence on the user's part to prevent it or notice the broken REF before saving the database.

At the very least, perhaps some documentation nudging users towards organizing ref'd objects in a thoughtful manner and preferring to delete ref clones over originals would be useful.

At the same time, it seems an easy thing to scan through the open database for.

Expected Behavior

I expect that when deleting a file that other files currently REF a field from that a warning be displayed mentioning this fact and providing an opportunity to cancel the deletion in response.

Current Behavior

The file gets deleted and any REFs silently break.

Possible Solution

When the above situation occurs, it would be nice to give a chance for automatically redistributing the REF (fill-in the value so they are no longer ref'd, or choose one of them as the new value holder and re-REF any others to that one). This would need to happen for each REF'd field, as there could be multiple.

Steps to Reproduce (for bugs)

  1. Add a new entry (Ctrl+N) named test with user/pass "123"
  2. Clone entry (Ctrl+K) and check "Replace username and password with references" and hit OK
  3. Select the original "test" entry and hit Delete entry (Ctrl+D) and hit Yes
  4. Notice that "test - Clone"'s Username is still linked by the username showing as "Ref: 123"
  5. Click "Recycle Bin" to go there
  6. Select the original, now deleted "test" entry and hit Delete entry (Ctrl+D) and hit Yes
  7. Click back to original location where steps were started from
  8. Notice that "test - Clone"'s Username is broken by the username showing as "Ref: {REF:U@....}"

Context

I was organizing some keys by Group and noticed that one key fit best in two groups, but duplicating the objects may result in them falling out of sync upon next password rotation. Therefore I felt the Refs feature would be perfect for this use case, but find it hard to determine which group should be the "master" group that the others REF.

While I can see solutions I can implement by care and convention, I want to trust my REF objects to always be up-to-date and in-sync with their REFs without having to concern myself with the very organizational details the REFs seemed meant to save me from.

Debug Info

KeePassXC - 2.2.0
Revision: caa49a8

Libraries:

  • Qt 5.9.0
  • libgcrypt 1.7.7

Operating system: Windows 8.1 (6.3)
CPU architecture: x86_64
Kernel: winnt 6.3.9600

Enabled extensions:

  • KeePassHTTP
  • Auto-Type
  • YubiKey
@TheZ3ro TheZ3ro added this to the v2.3.0 milestone Aug 12, 2017
@TheZ3ro
Copy link
Contributor

TheZ3ro commented Aug 12, 2017

I think this should be a very nice and useful feature

Unfortunately this can't be done very trivially.
It will need a scan between all the entries in the database to check if some of them has reference to the one being deleted.

@droidmonkey droidmonkey changed the title User should be warned before deleting file with a UID that is being REF'd by another object User should be warned before deleting entry with a UID that is being REF'd by another object Aug 18, 2017
@phoerious phoerious modified the milestones: v2.3.0, v2.4.0 Jan 28, 2018
wgml added a commit to wgml/keepassxc that referenced this issue Apr 7, 2018
…eboot#852.

On warning, references can be replaced with original values or ignored.
Removal process can be also skipped for each conflicting entry.
wgml added a commit to wgml/keepassxc that referenced this issue Nov 14, 2018
…eboot#852.

On warning, references can be replaced with original values or ignored.
Removal process can be also skipped for each conflicting entry.
droidmonkey pushed a commit to wgml/keepassxc that referenced this issue Dec 23, 2018
…eboot#852.

On warning, references can be replaced with original values or ignored.
Removal process can be also skipped for each conflicting entry.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants