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

Configurable salt length #97

Closed
res13 opened this issue Jan 23, 2023 · 8 comments
Closed

Configurable salt length #97

res13 opened this issue Jan 23, 2023 · 8 comments

Comments

@res13
Copy link

res13 commented Jan 23, 2023

Is your feature request related to a problem? Please describe.
In a situation where there are different remote java repositories for the same company, I would like to standardize all the password hashing algorithms among them. My goal is that all of them use password4j. I manage the property file (psw4j.properties) and they manage their code. Currently, the salt length can only be changed via code (not via properties), which can lead to inconsistencies between repositories.

Describe the solution you'd like
I would like to be able to have the salt length a configurable property like global.salt.length=16 which is used in the class SaltGenerator.

Describe alternatives you've considered

  • I could use the default (hardcoded) length of 64 but that is a bit too long for my taste.
  • I would need to make sure that each remote dev team uses the same length by clear communication and in-depth code review (which is a lot more work for me).

Additional context
great library by the way, huge fan!

@firaja
Copy link
Member

firaja commented Jan 23, 2023

Hello @res13,

I put this request in the roadmap for 1.6.4.

If you want you can create a pull request with that feature. I will be happy to review it.
If you need help you can ask here, but you can follow what has been done for PepperGenerator#get().

@ishu-thakur
Copy link

Hi @firaja , I would like to work on this issue .

@firaja
Copy link
Member

firaja commented Jan 25, 2023

@ishu-thakur assigned.

Remember to check code coverage produced by jacoco.

@ishu-thakur
Copy link

Hi @firaja i have raised the PR .Please go through it and if any changes required in it , i will be happy to do it :)

@ishu-thakur
Copy link

Hi @firaja I have debugged the code and i found that while running mvn -X clean test there are two testcase failures in the com.password4j.PasswordTest

  1. testBanner2 = java.lang.AssertionError.
  2. testGenericUpdate1 = java.lang.UnsupportedOperationException: PBKDF2WithHmacSHA256 is not a valid algorithm.
    But the weird thing is when I'm running passwordTest test class separately then all testcases passed .

@firaja
Copy link
Member

firaja commented Feb 4, 2023

@ishu-thakur Can you please open a new issue with the full stack trace of both failing tests?
Also details about your environment (OS, Java version, etc.) will be very appreciated, thank you.

We will discuss the problem in a different thread so this can be closed once you push the changes requested in PR #98

@firaja firaja modified the milestones: 1.6.4, 1.7.0 Feb 6, 2023
firaja added a commit that referenced this issue Feb 9, 2023
Fixed issue #97 configured salt length in properties.
firaja added a commit that referenced this issue Feb 9, 2023
@firaja
Copy link
Member

firaja commented Feb 9, 2023

Hi @res13
The feature will be deployed within the next release (1.7.0).
If you want to use it before the release you can use the version in master branch.

@firaja
Copy link
Member

firaja commented Feb 18, 2023

@res13 the feature is public under version 1.7.0.

You can use global.salt.length to define the length of salts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

3 participants