Skip to content

Add the change passphrase feature#2224

Merged
m3nu merged 21 commits intoborgbase:masterfrom
VandalByte:dev-passphrase-change
Jul 13, 2025
Merged

Add the change passphrase feature#2224
m3nu merged 21 commits intoborgbase:masterfrom
VandalByte:dev-passphrase-change

Conversation

@VandalByte
Copy link
Collaborator

@VandalByte VandalByte commented Mar 31, 2025

Description

This PR intends to add the much requested and discussed functionality to change passphrase in vorta UI.

Related Issue

Closes #303

Motivation and Context

This change adds the ability to change the passphrase directly within the Vorta backup client through a user-friendly interface. Previously, users had to rely on manual borg command-line operations to update their passphrase. This simplifies the process, improving usability by allowing users to easily update their passphrase without leaving the Vorta interface.

How Has This Been Tested?

Manual testing and unit testing.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have read the CONTRIBUTING guide.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

I provide my contribution under the terms of the license of this repository and I affirm the Developer Certificate of Origin.

@VandalByte
Copy link
Collaborator Author

VandalByte commented Mar 31, 2025

This is the design I thought to got with, I have considered a way to logically group this in a new drop down button.

  • This button serves more like a utility button for the selected repo.
  • Currently, the 'Change Passphrase' option is being added, but the dropdown can be easily expanded to include other options in the future (e.g., 'Change Repo Name' feature).
  • I may adjust the icon based on feedback.

I have read the requirements mentioned in #303 by everyone and will try to do this the best way possible. Any suggestions regarding the UI design or functionality are always welcome!

Screenshot 2025-04-01 003733

@m3nu
Copy link
Contributor

m3nu commented Apr 3, 2025

How about moving more lesser-used features (like unlink) into the submenu?

and I'd use 3 dots as the submenu icon. I feel it's more widely used. The cog icon usually means Settings.

@VandalByte
Copy link
Collaborator Author

VandalByte commented Apr 3, 2025

How about moving more lesser-used features (like unlink) into the submenu?

I can do that.

and I'd use 3 dots as the submenu icon. I feel it's more widely used. The cog icon usually means Settings.

Yeah, so I was actually stuck with these two, I feel like the horizontal one looks a bit better. But if we want to go with vertical one then there's already a 3 dot present ellipse-v.svg we can reuse that icon, and I can't find a horizontal version of that, I'll have to rotate and make an new svg in that case.

Screenshot 2025-04-03 202046

The ellipse-vertical
image

Icons used were sourced from Google Material Icons

@VandalByte
Copy link
Collaborator Author

I have moved the unlink, should I keep the icon or remove it?

image

@m3nu
Copy link
Contributor

m3nu commented Apr 3, 2025

I would either do a button for both or none.

@VandalByte
Copy link
Collaborator Author

@m3nu I'd like your opinion on a few things. The current implementation changes the passphrase for the currently selected repo (same behavior as how unlink works).

  1. At the moment, I'm not validating the current passphrase (i.e., checking whether the old passphrase entered is correct before applying the new passphrase). I'm just directly setting the new one.

    • I felt this might be fine, since from what I saw, vorta when adding an existing repo takes the passphrase from the keyring and applies it, rather than asking the user to enter and confirm it against the keyring value before adding the repo.
  2. For the confirmation message, ie, when passphrase was successfully changed, i have two methods in mind:

    • First, show a message right below the errortext validation, green if successful red if failed.
    • Second, show a dialog with an "OK" button containing the message.

I am thinking of going with the 1st option, to avoid the need for too many dialogs popping up.

This is the screenshot of the currently implemented UI:
Screenshot 2025-04-12 110937

@m3nu
Copy link
Contributor

m3nu commented Apr 12, 2025

I'm just directly setting the new one.

Can you change the passphrase without knowing the existing one? I doubt it.

Still better to run only one command in total for this and throw an error if it fails for any reason (issue with connecting to repo or wrong passphrase or anything else).

Second, show a dialog with an "OK" button containing the message.

This one is probably better because the user isn't left with an open dialog.

@VandalByte
Copy link
Collaborator Author

ssr-2025-05-03_09.33.02.mp4

@VandalByte VandalByte requested a review from m3nu May 3, 2025 04:08
@VandalByte VandalByte marked this pull request as ready for review May 3, 2025 04:24
@VandalByte VandalByte changed the title WIP: Add the change passphrase feature Add the change passphrase feature May 3, 2025
@VandalByte
Copy link
Collaborator Author

Added passphrase change only for encryption type = repokey, as mentioned in the comment.

@m3nu
Copy link
Contributor

m3nu commented Jun 28, 2025

Sorry for the delay in reviewing this. Just ran this locally and think this can be improved in the following ways:

  • It won't update the passphrase in the keychain after changing it. As a result it keeps using the old one and everything fails after changing the passphrase.
Screenshot 2025-06-28 at 07 50 32 Screenshot 2025-06-28 at 07 50 56
  • The space in the dialog could be used more efficiently. And the note at the bottom is cut off:
Screenshot 2025-06-28 at 07 48 45

@VandalByte
Copy link
Collaborator Author

The UI part I made some adjustments for the bottom label property and overall widget layout was changed from grid to vertical. The input fields are inside a form layout idk why it's showing up misaligned (labels would be left aligned) in your screenshot. I also noticed you are missing the widget title "Change Passphrase" and the close button at the top. This is how it looks for me (also resizes flawlessly) in MX Linux XFCE.

image

It won't update the passphrase in the keychain after changing it. As a result it keeps using the old one and everything fails after changing the passphrase.

Now about the error, could you tell me how to reproduce this? because I couldn't.

These are the steps I tried:

  • I changed passphrase for a local repo and changed it again for the same repo.
  • I changed the passphrase quit vorta and started vorta again and started a manual backup.

Is this maybe a remote repo specific issue? Could you test it out with a local repo and let me know if it works?

@m3nu
Copy link
Contributor

m3nu commented Jun 29, 2025

It's not due to the remote repo. I check the macOS keychain and it still had the old password after the change. After changing the password manually, it worked again.

So it might be macOS-specific. Could also be with keychain permissions.

@VandalByte
Copy link
Collaborator Author

VandalByte commented Jul 1, 2025

I might have missed the part to update the keyring since it worked properly in linux during testing (for some reason). I have updated the PR, can you verify? Thanks

@m3nu
Copy link
Contributor

m3nu commented Jul 1, 2025

Nope. Password is still unchanged in keyring.

Quite possible that our current macos code doesn't allow overwriting an existing item. The method we use is SecKeychainAddGenericPassword.

Does it work on Linux? How did it work on Linux without this code?

I guess we need to add a delete_password() method for all keyring classes to make this work. The only one that works for sure is our internal db.py class, which already uses get_or_create().

@VandalByte
Copy link
Collaborator Author

Does it work on Linux? How did it work on Linux without this code?

Yeah sorry, my mistake, I had the keyring code written in the borg/change_passphrase.py, I was looking at the wrong file.

I guess we need to add a delete_password() method for all keyring classes to make this work. The only one that works for sure is our internal db.py class, which already uses get_or_create().

I've added some changes to consider this. It works ok in Linux (I've tested it out). I've added the macOS code as well in darwin.py, it just needs testing.

@m3nu
Copy link
Contributor

m3nu commented Jul 2, 2025

OK. Will test.

One other thing I had noticed, but forgot to add: The icon for this dialog could be set to type warning or confirmation or something. It may use some default right now. Just whatever we use in other places.

Screenshot 2025-07-01 at 11 18 46

@VandalByte
Copy link
Collaborator Author

I've added the appropriate info and warning icon properties to the message boxes. The icons should now display correctly.

@m3nu
Copy link
Contributor

m3nu commented Jul 4, 2025

I can confirm that the latest commits ensure that the password is updated in the keychain.

For the UI and dialog icon it's unchanged.

Screenshot 2025-07-04 at 17 11 11 Screenshot 2025-07-04 at 17 11 21

@VandalByte
Copy link
Collaborator Author

Recent changes include:

  • Made the NOTE label expanding horizontally and added word wrap.
  • Added form property to make the LineEdits expand (FieldGrowthPolicy as ExpandingFieldsGrow).
  • Minor changes like margin and alignment.

The changes has been tested in MX Linux 22 (XFCE).

@m3nu
Copy link
Contributor

m3nu commented Jul 6, 2025

This new feature works as expected locally and could be merged right away.

I would just add 2 things (which could be in a separate PR, if this one is getting too big or blocking something):

  • At least a simple unit test to make sure this keeps working after future changes.
  • I would show the repo URL in the dialog somewhere so it's always clear which passphrase is updated. (given how important this is to the user)

@VandalByte
Copy link
Collaborator Author

VandalByte commented Jul 11, 2025

All the requested changes have been included. This should be good to go.

Screenshot 2025-07-07 035759

@m3nu
Copy link
Contributor

m3nu commented Jul 11, 2025

Great! Will test one more time and then merge.

@m3nu m3nu merged commit e4064b4 into borgbase:master Jul 13, 2025
7 checks passed
@VandalByte VandalByte deleted the dev-passphrase-change branch July 20, 2025 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Change password

2 participants