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

Honor default preferences instead of using hardcoded defaults #41

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fgndev
Copy link

@fgndev fgndev commented Feb 8, 2021

Contrary to the default preference value of 2000 iterations a hardcoded value of 1000 is used. Only after opening the settings dialog the 2000 iterations will become effective. I fixed it here. The 1000 default value is hardcoded into
app/src/main/java/org/secuso/privacyfriendlypasswordgenerator/activities/SettingsActivity.java
as well, even though it does not seem to have an effect here.
Both my Java and Android knowledge are very limited so far, so this is rather a suggestion that I basically copied from Stackexchange, so someone should look into this,

I strongly vote for turning the hash algorithm / number of iterations into per-account settings, too. I do unterstand that from a technical point those are rather per-device settings, but making them per-account settings would
a) serve as a reminder that those values actually effect the generated password (I know its in the docs, I'm leaning more towards practicality here)
b) Would allow to open the 'old password / new password' dialog as right now, changing the hash value 'corrupts' the entire db
but goes otherwise unnoticed
c) Users can choose to wait longer for the generated password if the use-case for that password is worth the sacrifice

@Kamuno
Copy link
Member

Kamuno commented Feb 8, 2021

I strongly vote for turning the hash algorithm / number of iterations into per-account settings, too. I do unterstand that from a technical point those are rather per-device settings, but making them per-account settings would

We actually discussed the same thing. So this will be done in the next update.

I'll look at your changes later :) Thanks for the PR

@Kamuno Kamuno added the bug label Feb 8, 2021
@udenr
Copy link
Contributor

udenr commented Aug 30, 2023

Sorry, I didn't see your PR before working on my changes in #50
I guess this PR is no longer needed then, or what were the changes to the dialogs supposed to do? was there also an issue?

@fgndev
Copy link
Author

fgndev commented Oct 14, 2023

Sorry, I didn't see your PR before working on my changes in #50 I guess this PR is no longer needed then, or what were the changes to the dialogs supposed to do? was there also an issue?

Sorry for confusion, the changes to the dialog should have gone into a separate PR. The (perceived) issue here is that a generated password remains visible even after a lock/unlock cycle, if I remember correctly. This is arguably paranoia, but at least it should be configurable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants