flask-praetorian icon indicating copy to clipboard operation
flask-praetorian copied to clipboard

send_reset_email is referencing a non existent User attribute

Open sathyanmurugan opened this issue 4 years ago • 5 comments

https://github.com/dusktreader/flask-praetorian/blob/4740713858965e4c933b12e7d33d8972cd1d5618/flask_praetorian/base.py#L896-L899

send_reset_email attempts to pass the attribute user.email to the function send_token_email . The User object may not necessarily have an email attribute (I for instance have set it up as username in my model), and is causing the execution of send_reset_email to fail.

Can we update the function to return email instead of user.email? Similar to how its currently done for send_registration_email https://github.com/dusktreader/flask-praetorian/blob/4740713858965e4c933b12e7d33d8972cd1d5618/flask_praetorian/base.py#L827-L831

sathyanmurugan avatar Dec 30 '20 09:12 sathyanmurugan

I am also having this issue. When calling as such:

data = request.get_json()
user = User.lookup(data['username'])  # works
guard.send_reset_email(user.username, subject="Password Reset Request")

I am receiving the following error:

'User' object has no attribute 'email'

I don't have an email attribute in my User class/model, so it fails. I'm using username as that's how the rest of the system seems to be working.

However, unlike the request by @sathyanmurugan I want it to return username. What would be the best way forward here? Do I need to have duplicate data in my database?

therealrobster avatar Jan 18 '21 06:01 therealrobster

As a temporary solution, you can add a class property as email that returns username. Something like this :

class User(object):
    def __init__(self, user_id, password, username, roles=""):
        self.user_id = user_id
        self.username = username
        self.password = password
        self.roles = roles

    def __repr__(self):
        return str(self.user_id)

    @property
    def email(self):
        return self.username

mukeshsharma1201 avatar Jan 21 '21 08:01 mukeshsharma1201

@mukeshsharma1201 that did it and it's now working.

May I ask why you consider it to be temporary? Is there a change in how the code will work in the future?

Thanks so much, really appreciate your help there.

therealrobster avatar Jan 21 '21 23:01 therealrobster

Sorry I'm so slow getting to this one. Yeah, that's a legitimate issue! I'll be thinking about how best to address this. It will be one of two ways, I think:

  1. Just document that email features require the user class to have an email attribute
  2. Update the code so that email is supplied in a different way Thanks for opening this issue!

dusktreader avatar May 17 '21 15:05 dusktreader

Addressed with Merge Request #251

Will get this merged into master and then do some regression and integration testing to make sure it's working as expected. There are also going to be some updats such as dropping support for python 3.5 and adding some other automation for linting, etc. Hopefully a new release will be coming soon.

dusktreader avatar Jun 08 '21 06:06 dusktreader