cakephp / authentication

Authentication plugin for CakePHP. Can also be used in PSR7 based applications.
MIT License
117 stars 100 forks source link

RFC: Config Option to Hash Token Field #557

Closed amayer5125 closed 2 years ago

amayer5125 commented 2 years ago

We can improve the security of the TokenIdentifier by storing a hashed version of the token in the database instead of storing it in plaintext. This doesn't increase the security of the app directly, but provides a layer of protection if the database would get compromised, similar to the reason we hash passwords.

We would need to use a hashing algorithm that does not require a salt. This would allow us to hash the incoming token and look for a row containing the hashed version. Something like SHA-256 would probably be fine, but this option could also be configurable.

markstory commented 2 years ago

We would need to use a hashing algorithm that does not require a hash.

Do you mean a salt? I would prefer we use similar hashing strategies as we do with passwords.

amayer5125 commented 2 years ago

I did mean salt, I'll update that above.

My concern with using a similar strategy is that each password gets a unique salt, so the only way to tell if token "abc123" is associated with a user would be to get every token and hash "abc123" over and over for each unique salt to try and find a match. If you only have 5 users that might be fine, but this does not scale. bcrypt is intentionally slow so hashing "abc123" ten thousand or one hundred thousand times would lead to the user seeing huge performance hits.

This is why I recommended a hash without a salt. If we did hash('sha256', 'abc123') we always get the same value. Then we can do $usersTable->find()->where(['token' => hash('sha256', 'abc123')]). Doing one lookup is WAY faster than iterating every user and running another hash. Especially if the column is indexed in the database.

viraj-khatavkar commented 2 years ago

@markstory I think this update can be a good optional addition for the applications that don't want to store tokens in plain text. Right now I have to use a callback identifier to achieve this behavior. An in-built config for hashing should help to get rid of this extra code:

$service->loadIdentifier('Authentication.Callback', [
    'callback' => fn($data) => $this->fetchTable('Users')
        ->find()
        ->where(['api_key' => Security::hash($data['token'], 'sha256')])
        ->first(),
]);
markstory commented 2 years ago

Implemented in #559