Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why send activation email on user update? #546

Open
akhayyat opened this issue Oct 8, 2020 · 12 comments
Open

Why send activation email on user update? #546

akhayyat opened this issue Oct 8, 2020 · 12 comments

Comments

@akhayyat
Copy link

akhayyat commented Oct 8, 2020

If sending activation emails is enabled (SEND_ACTIVATION_EMAIL == True), updating a user currently sends an activation email!

https://github.com/sunscrapers/djoser/blob/master/djoser/views.py#L148:

class UserViewSet(viewsets.ModelViewSet):
    ...
    def perform_update(self, serializer):
        super().perform_update(serializer)
        user = serializer.instance
        # should we send activation email after update?
        if settings.SEND_ACTIVATION_EMAIL:
            context = {"user": user}
            to = [get_user_email(user)]
            settings.EMAIL.activation(self.request, context).send(to)

What's the purpose of sending an activation email with an activation link in this scenario (updating a user)? The account is already activated, and will remain activated after the update. A confirmation email would be understandable, but an activation email is puzzling.

@dekoza
Copy link
Contributor

dekoza commented Oct 13, 2020

How do you see the purpose of activation email? Usually it is used to check if given email is functional. Having users with unusable emails can lead to many problems. Following this logic, if user changes their email, you might want to confirm that it is working properly - hence the activation link. Hope that answers your concern.

@akhayyat
Copy link
Author

Right. That makes sense when the email is changed. But a user may be updated without changing their email.
In fact, in my case, email is the username. So, changing it requires using the "set username" endpoint. The email field is a read-only field in my user serializer.

But to accommodate the general case, wouldn't it make sense to send an activation email only if the email field is changed?

@tartieret
Copy link

tartieret commented Oct 13, 2020

@dekoza the issue raised by @akhayyat did not exist in previous versions of Djoser, but was modified in this commit:
ab3130e#diff-ef29656ddb7155dfc495f119a3afdb756b1181b7dee748939b9ec766fdced530
Previously the activation email was only sent IF the user was not active, which kind of makes sense.

The case when a user changes his/her email is addressed in the UserSerializer here:

def update(self, instance, validated_data):

I don't really see why we would send a confirmation email after each update, and got this issue flagged after it was reported by some of our users following an update of djoser version

So in any case, it could make sense to use a different settings like "SEND_ACTIVATION_ON_UPDATE" to maintain backward compatibility and only enable this behaviour for those who want it

@haxoza
Copy link
Member

haxoza commented Oct 21, 2020

Thanks @akhayyat, thanks @tartieret! I agree in general with your comments. I think we should improve this behaviour.

@haxoza
Copy link
Member

haxoza commented Oct 21, 2020

Following the commit mentioned above it seems that #430 is related to this issue.

@akhayyat
Copy link
Author

I would suggest one of two things:

  1. Remove the sending-email behavior altogether from the user update action, since the activation email wording does not really describe the change anyway, and is meant for another scenario.

  2. Instead of using the activation setting and email template, introduce a new email template and setting, say, confirm_user_update or confirm_email_update.

Since I don't think the current behavior is useful to anyone (sending account activation email on updating any user attribute), I'd vote for option 1 until a pull request for option 2 is available.

Here is the default activation email for reference:

Subject: Account activation on {{ site_name }}

You're receiving this email because you need to finish activation process on {{ site_name }}.

Please go to the following page to activate account:
{{ protocol }}://{{ domain }}/{{ url|safe }}
                                                                                                    
Thanks for using our site!

The {{ site_name }} team

@bodgerbarnett
Copy link
Contributor

I am facing this issue as well.

I am looking at the way allauth does this and it adds the new, updated email as an unverified address on the account until the user clicks on the link in the email they get when they change it. Until that happens, the primary email associated with the account (the original email address) is the one the account uses. Then, when they confirm the new address by clicking on the link in the email they get, the new email is verified and becomes the primary.

I think that flow makes the most sense to me. Any other updates to the user's profile don't cause any emails to be sent.

There is another use case that I am struggling with in this area though and that is when the user has signed up but not clicked the activation link in the email yet. If they just try and log in, it fails - but not with "inactive_account" as you'd expect - it fails with "invalid_credentials", here:-

self.fail("invalid_credentials")

So that's one thing - but I tend to think that really, if they haven't activated their account yet, trying to log in should really cause the activation email to be resent. Again, this is how allauth does it and it does seem to make sense. Otherwise, if the user has lost (or didn't get) their activation email, they're now stuck and can't access their account, no matter what they do.

I appreciate that this last thing is a separate issue though, so feel free to ask me to log this as such and we can keep this ticket on track.

@hkimani
Copy link

hkimani commented Jan 26, 2021

I also had an issue with activation emails being sent every time a user updates their details.

To solve this I rewrote the 'perform_update' function with a new template, that sends a 'User details updated message' rather than an activation email.

Screenshot 2021-01-26 182902

@cogat
Copy link

cogat commented Feb 3, 2021

The initially-described behaviour doesn't happen on master but the fix in #430 hasn't been released yet. Installing the dev branch (e.g. pip install git+https://github.com/sunscrapers/djoser@b648b07dc2da7dab4c00c1c39af3d6ec53f58eb6 or whatever revision you prefer) should work.

@shredding
Copy link

I did overwrite https://djoser.readthedocs.io/en/latest/settings.html#email and within it just the send method:

    def send(self, to, *args, **kwargs):
        if not self.context['user'].is_active:
            super().send(to, *args, **kwargs)

@Saksow
Copy link

Saksow commented Apr 17, 2022

Any plans to release this?

@mglowinski93
Copy link

@dekoza @haxoza
Are you planning to release a new version of Djoser soon?

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

No branches or pull requests

10 participants