PedroBern / django-graphql-auth

Django registration and authentication with GraphQL.
https://django-graphql-auth.readthedocs.io/en/latest/
MIT License
333 stars 105 forks source link

Make django-graphql-auth use own salt #41

Open cutamar opened 4 years ago

cutamar commented 4 years ago

Currently the email tokens (e.g. for password reset) are created using django.signing and always with the same salt (django.core.signing). It would be wise to use specific salt for django-graphql-auth.

PedroBern commented 4 years ago

The django.core.signing already uses the secret key, so I think it is secure. However, you are right. Currently, we have this "action" that determines which token we are generating (password reset , email verification ...), maybe we can use this action as the salt to make a better code.

Something like this:

# graphql_auth/utils.py

--- def get_token(user, action, **kwargs):
+++ def get_token(user, salt, **kwargs):
    username = user.get_username()
    if hasattr(username, "pk"):
        username = username.pk
---    payload = {user.USERNAME_FIELD: username, "action": action}
+++    payload = {user.USERNAME_FIELD: username}
    if kwargs:
        payload.update(**kwargs)
    token = signing.dumps(payload, salt = salt)
    return token

--- def get_token_paylod(token, action, exp=None):
---    payload = signing.loads(token, max_age=exp)
---    _action = payload.pop("action")
---    if _action != action:
---        raise TokenScopeError
+++ def get_token_paylod(token, salt=salt, exp=None):
+++    payload = signing.loads(token, max_age=exp)
    return payload

Then where we handle TokenScopeError we need to handle the BadSignature instead. What you think?

cutamar commented 4 years ago

@PedroBern sounds like a good approach. I'd say it would also be good to prefix the actions with something, so it's namespaced to django-graphql-auth package.

PedroBern commented 4 years ago

@cutamar would you like to open a PR? If you can't it's ok, I change this later.

cutamar commented 4 years ago

If I get the time, but can't promise for sure