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

salt is unnecessary, when default algorithm is bcrypt #18

Open
dongwq opened this issue Feb 23, 2015 · 3 comments
Open

salt is unnecessary, when default algorithm is bcrypt #18

dongwq opened this issue Feb 23, 2015 · 3 comments
Assignees

Comments

@dongwq
Copy link

dongwq commented Feb 23, 2015

salt is unnecessary, when default algorithm is bcrypt
the code below throw exception with a salt field

 public String getUserSalt(String username){
    String salt = null
    userClass.withTransaction { status ->
        def user = userClass.findWhere((usernameProperty): username)

        if (!user) {
            salt = null
        } else if (userClass.metaClass.hasProperty(user, saltField)) {
            salt = user."$saltField"
        } else {
            throw new RuntimeException("$userClass class needs $saltField field")
        }
    }
    return salt
}
@dongwq dongwq changed the title slat is unnecessary, when default algorithm is bcrypt salt is unnecessary, when default algorithm is bcrypt Feb 23, 2015
@mgdelacroix
Copy link
Contributor

Hi @dongwq,

how is this code related to the plugin and which part of the plugin's behaviour is wrong?

@mgdelacroix mgdelacroix self-assigned this Feb 25, 2015
@dongwq
Copy link
Author

dongwq commented Feb 25, 2015

Hi @mgdelacroix ,
this method comes from class UserSaltProvider.It throws

throw new RuntimeException("$userClass class needs $saltField field")

if the userClass doesn't have a salt Filed.

salt field is unnessary,when algorithem is bcrypt. you can see it org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder(from spring-security)

@niwinz
Copy link
Member

niwinz commented Mar 19, 2015

+1, The salt for bcrypt and pbkdf2 like key derivation functions should be completely random. And generated from CSRNG,

It not make sense have a salt stored in the user model, as usually the salts should be public and can be prepended or appended to the result of pasword derivation function. As sprint framework is doing for you as far as I known.

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

No branches or pull requests

3 participants