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

If password keychain helper is enabled, always sync password stored in keychain when password is changed #55227

Merged
merged 5 commits into from
Feb 7, 2025

Conversation

nyalldawson
Copy link
Collaborator

This used to be a separate menu action that the user had to remember to run. Let's err on the side of being helpful and assume that if the user is storing the password in their keychain, then they want that stored password always to be the correct one.

Copy link
Contributor

@benoitdm-oslandia benoitdm-oslandia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@benoitdm-oslandia benoitdm-oslandia added Authentication Related to the QGIS Authentication subsystem or user/password handling GUI/UX Related to QGIS application GUI or User Experience labels Nov 9, 2023
@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Nov 24, 2023
@github-actions github-actions bot closed this Dec 1, 2023
@dokterbob
Copy link

LGTM

Why's this not merged?

@github-actions github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Nov 30, 2024
@nyalldawson nyalldawson reopened this Feb 4, 2025
@nyalldawson nyalldawson force-pushed the always_sync branch 3 times, most recently from 843be53 to c5479cd Compare February 4, 2025 21:47
@nyalldawson
Copy link
Collaborator Author

@elpaso Ok this has been rebased and updated based on our discussion. Here's what the reset password dialog looks like now (if system wallet integration is enabled):

image

Copy link

github-actions bot commented Feb 4, 2025

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 70ea5e5)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 70ea5e5)

@benoitdm-oslandia
Copy link
Contributor

LGTM!

Copy link
Contributor

@elpaso elpaso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just minor remarks on wording, now that the auth db is not necessarily always a sqlite DB we should try to keep it generic (unless the code is specific to sqlite local DB).

src/gui/auth/qgsauthmasterpassresetdialog.cpp Outdated Show resolved Hide resolved
stored in keychain when password is changed

This used to be a separate menu action that the user had to
remember to run. Let's err on the side of being helpful and
assume that if the user is storing the password in their
keychain, then they want that stored password always to be
the correct one.
When resetting the master password and wallet storage is enabled,
tweak the reset password dialog label to explicitly state that
the new password will be stored in the wallet.

This keeps users fully advised of what's about to occur.
@nyalldawson nyalldawson merged commit 55288fe into qgis:master Feb 7, 2025
31 checks passed
@nyalldawson nyalldawson deleted the always_sync branch February 7, 2025 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Authentication Related to the QGIS Authentication subsystem or user/password handling GUI/UX Related to QGIS application GUI or User Experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants