kaleidos / grails-security-stateless

Grails plugin to implement stateless authentication using Spring Security
Apache License 2.0
17 stars 8 forks source link

salt is unnecessary, when default algorithm is bcrypt #18

Open dongwq opened 9 years ago

dongwq commented 9 years ago

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
}
mgdelacroix commented 9 years ago

Hi @dongwq,

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

dongwq commented 9 years ago

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 commented 9 years ago

+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.