Skip to content

Conversation

@coldiron
Copy link

Satisfies #792

Adds a new configuration option auth:fingerprint:max_retries with a default of 3, replacing the existing hard-coded value of 3 fingerprint retires.

@coldiron coldiron force-pushed the fingerprint-retries branch from ca0afe3 to b81dbd3 Compare July 28, 2025 16:38
@vaxerski vaxerski requested a review from PointerDilemma July 28, 2025 20:02
Copy link
Collaborator

@PointerDilemma PointerDilemma left a comment

Choose a reason for hiding this comment

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

I think you misunderstood what the retry value means in this context.

It's how many time the dbus connection is allowed to fail.
We should properly fix the issue that causes you needing more than 3 retries instead of making this number adjustable.

Nevermind I misunderstood.

@moggiesir what do you think?

Copy link
Collaborator

@PointerDilemma PointerDilemma left a comment

Choose a reason for hiding this comment

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

Thanks I think this is good after looking at the code Fingerprint code again.

Wiki MR needed though :)

@alba4k
Copy link
Contributor

alba4k commented Aug 5, 2025

Works great, thanks for this change :)))

Also, is there any reason against making something like -1 act as "infinite retries"? I don't think it would be a security hazard, as I don't think brute-forcing fingerprints is a thing anyway

I just noticed that with auth:fingerprint:max_retries = 3 (or any n) I am able to make the authentication fail 4 times (or any n+1). Is this intended? I get why this is (it is trying again 3 times, after the 1st fail) but isn't this unintuitive? I believe it would be better and clearer to rename the option to max_attempts and make it act as a "how many tries the user has" thing.

@PointerDilemma
Copy link
Collaborator

Also, is there any reason against making something like -1 act as "infinite retries"? I don't think it would be a security hazard, as I don't think brute-forcing fingerprints is a thing anyway

With this PR 0 should work and -1 too i think?

I just noticed that with auth:fingerprint:max_retries = 3 (or any n) I am able to make the authentication fail 4 times (or any n+1). Is this intended? I get why this is (it is trying again 3 times, after the 1st fail) but isn't this unintuitive? I believe it would be better and clearer to rename the option to max_attempts and make it act as a "how many tries the user has" thing.

@coldiron I think @alba4k is correct on that. Wanna move to max_attempts?

Also let me know if I should finish this up with the changes and wiki MR, or if you want to do it yourself.

@alba4k
Copy link
Contributor

alba4k commented Jan 9, 2026

@PointerDilemma I guess this PR makes no sense if that kind of stuff is moving to hyprauth, but maybe add something like infinite fingerprint retries there? :)

would make sense, as biometrics aren't as easily brute-forced as passwords

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.

3 participants