-
Notifications
You must be signed in to change notification settings - Fork 12
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
use password4j to generate and check hashes #3313
use password4j to generate and check hashes #3313
Conversation
Signed-off-by: Max Thonagel <12283268+thoniTUB@users.noreply.github.com>
Signed-off-by: Max Thonagel <12283268+thoniTUB@users.noreply.github.com>
Signed-off-by: Max Thonagel <12283268+thoniTUB@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sieht im ganzen gut aus. Finde etwas Spooky, dass wir jetzt doch Strings statt char[] benutzen. Die password4j ist sehr etabliert, oder warum der umstieg?
} | ||
HashEntry hashedEntry = passwordStore.get(new UserId(username)); | ||
if (hashedEntry == null) { | ||
throw new UnknownAccountException("Provided username or password was not valid."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Best Practice ist hier idR auch IncorrectCredentials oä zu werfen um zu verhindern, dass Leute für deinen Account fishen können.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guter Punkt, passe ich an :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich bleibe bei den Exceptions explizit im Realm, aber fange und konvertiere sie zu einer generellen NotAuthorizedException
die ein 401er aus lösen bevor sie zum User kommen.
CompressedPBKDF2Function.class, CompressedPBKDF2Function::getInstanceFromHash | ||
); | ||
|
||
HashingFunction getHashingFunction(String hash) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ich verstehe nicht was die methode macht
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich habe eine doku ergänzt und einen Test dazu geschrieben.
Du gibst der methode einen hash und sie gibt dir die HashingFunction zurück, mit der sie erstellt wurde.
backend/src/main/java/com/bakdata/conquery/models/config/auth/LocalAuthenticationConfig.java
Show resolved
Hide resolved
…or the end user Signed-off-by: Max Thonagel <12283268+thoniTUB@users.noreply.github.com>
062ff19
to
5b4b88c
Compare
Signed-off-by: Max Thonagel <12283268+thoniTUB@users.noreply.github.com>
Ich habe mich umgeschaut. Ich habe mich zu der lib entschieden, weil sie einfach zu bedienen ist und die akuellen Algorithmen unterstützt. die Alternative wäre z.B. die Spring Security Lib zu importieren, aber das ist ein overkill oder für die einzelnen Algorithmen einzelne Libs zu importieren. |
Don't use PBKDF2 anymore