-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
allow automatic expiration date extension on pwd modification (Addresses issue #2010) #6456
base: develop
Are you sure you want to change the base?
Conversation
3a31a80
to
34a7024
Compare
Oh that's gorgeous |
I coded myself into a hole a couple of times and had to pluck a lot of code afterwards, but I updated the PR with a commit that provides better granularity for the extension by switching to two pickers, one for the quantity and another one for the period of time (updating the screenshots right now). I need to store the extension information for the entry between sessions. I added it first into the TimeInfo class, but @phoerious pointed out to me that there is a kdbx spec to follow and I can't just add more fields and expect them to be stored. I was suggested to use Custom data, which until now I (think) was only been used for the browser extension. The commit introduces several states more into the application, but I think I tested all of them manually. |
After some more thought on my #2010 comment, I think the "Auto assign new ..." checkbox can just say "Update when password is changed.", and should be checked by default. The expiration settings can have a label "Interval:", and can default to the what you get when you subtract the last password update date from the current expiry date, capped to some some sensible minimum like 24 hours. Or maybe reformat the whole Expiration settings block like so:
And move this right under the password field, so it's more clearly associated with it. edit: If we want #6451, then after the "Expires after:" line, we can add a similar "Randomize by:" line. And when expiry date passes, change "Next expiration:" to a bold red "Expired on:". @droidmonkey , what do you think? |
I like the rewording of the checkbox but I'm not sure about checking it by default, sounds like a user choice and would require assuming defaults. In the reformatting you miss being able to set a specific date for the first expiration, which is useful when you have a password that is halfway through its expiration time but you are introducing it in KeePassXC for the first time. |
See my comment on #2010 for the reasoning. The default can be calculated as: <expiry date> minus <last password update date>. The reformatted version doesn't need a default, since the expiry interval becomes the primary setting.
If you have the "Last updated" label, then one can fairly easily figure out the interval for whatever date they want. The "Next expiration" label gives immediate feedback, and if it comes out wrong, the user can adjust the interval. I agree this is less comfortable for this particular use case, but OTOH, how often do users actually want a specific expiry date, vs an interval? (Note: the interval is always from the last update date, not from when the interval is set.) |
Added randomization, updated the screenshot |
a7a4c78
to
70f2862
Compare
Addressed issue #2010 Creates a new Expiration Settings area in the Edit Entry widget, composed of the expiration date settings and a new optional feature that allows for automatic extension of the expiration date on password modification.
Switches presets for quantity and unit of time pickers. Stores auto extension on modification data as custom data for the entry to respect the spec
Proposed in Issue #6451
70f2862
to
cc92ec1
Compare
I had an alternate idea for how to handle this, see here, and I think the UI in my suggestion is more streamlined in a usability sense. Basically, instead of "extending" the date, you instead set the expiry to be today + some relative date upon updating the password field of the entry (and no other field, since we don't want things like updating the browser settings to extend password expiry). It would also be good to be able to have a preset expiry interval. That is, the expiry date will always be set to, say, the X of the current month, or if it's in the past, the X of next month. This would be useful for, say, employers with rules like "passwords must be changed on the first of every month" |
I'm still in favor of the reorganization I suggested in my comment above. If wanted, the "Next expiration" can be made editable too, so the user can pick which way to update: if they change the "Expires after" fields, it automatically recalculates the next expiration; if they edit the next expiration field, then the "Expires after" are recalculated. Just need to be careful to avoid recursion (don't recalculate the active field). |
See: #7029, A spinner between the QDate and the Dropdown might be better then using Maybe even adding a thin #7029 can be added to this PR by @xvallspl and #7029 would be deleted/closed |
Concur with combining the two |
The last update to this PR was 5 months ago, and it's still marked as draft, so looks like it may be stuck.. |
@michaelk83 No, I still intend to merge it in by the end of the year. |
@xvallspl what's the status on this PR? |
It's stale and looking for someone to take over. I couldn't get it in before the end of the year and I shouldn't lie to myself about finding time for it anymore. |
greate feature, when will be merged? I wanna this feature too. |
Addresses issue #2010
Creates a new Expiration Settings area in the Edit Entry widget, composed
of the expiration date settings and a new optional feature that allows for
automatic extension of the expiration date on password modification.
Allows for randomization of the expiration date, within an interval (set by the user) of days from the deadline
Tasks left:
Screenshots
Testing strategy
Manual testing (?)
Type of change