Skip to content
This repository has been archived by the owner on Dec 5, 2021. It is now read-only.

User creation for ldap user unnecessarily copies password #34

Open
ghost opened this issue Aug 25, 2014 · 2 comments
Open

User creation for ldap user unnecessarily copies password #34

ghost opened this issue Aug 25, 2014 · 2 comments

Comments

@ghost
Copy link

ghost commented Aug 25, 2014

When using ldap authentication, the by_ldap_credentials methods checks if a corresponding local User object exists in the database, and creates one if it does not:

if user_ldap is None:
log.debug('create user %s'%login)
user_ldap = User()
user_ldap.login = login
user_ldap.password = password
..snip..

Currently, the ldap password is getting copied into the model instance, even though the model's password will never be used, since the ldap server will be queried every time an ldap user attempts to log in. Copying the ldap password into the User instance seems to be an unnecessary exposure of an ldap user's password - bcrypt potentially reduces the risk somewhat, but it is still possible someone could grab the pyshop db and use it as a vector for getting ldap passwords.

I propose that in the case of an ldap login, any User instance creation should refrain from copying the ldap password, rather some "dummy" password could be used - an empty string, a random digit, anything at all really, given that the dummy password will never be used.

Thoughts?

@EvaSDK
Copy link
Contributor

EvaSDK commented Sep 4, 2014

Sounds like a good idea.

Link to line in question: https://github.com/mardiros/pyshop/blob/master/pyshop/models.py#L301

@ghost
Copy link
Author

ghost commented Sep 4, 2014

Ok, I will submit a pull request later today then.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant