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

Custom account email verification through adapter #1946 #3648

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
Expand Up @@ -128,6 +128,7 @@ Matt Nishi-Broach
Mauro Stettler
Mikhail Mitiaev
Morgante Pell
Muneeb Shahid
Nariman Gharib
Nathan Strobbe
Nicolas Acosta
Expand Down
5 changes: 5 additions & 0 deletions ChangeLog.rst
@@ -1,3 +1,8 @@
Unreleased
----------

- Added support for custom account email verification based on the user's email.

0.61.1 (2024-02-09)
*******************

Expand Down
3 changes: 3 additions & 0 deletions allauth/account/adapter.py
Expand Up @@ -752,6 +752,9 @@ def get_reauthentication_methods(self, user):
)
return ret

def get_email_verification_method(self, email=None):
Copy link
Owner

@pennersr pennersr Feb 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def get_email_verification_method(self, email=None):
def get_email_verification_method(self, login):

Now that we have a Login class representing the current login, I think it is more useful to base this method on an instance of that. See:

https://github.com/pennersr/django-allauth/blob/main/allauth/account/utils.py#L158

So, for that to happen, I think we need to:

  • For every case where the default value (email_verification=app_settings.EMAIL_VERIFICATION) is passed, pass None. Then, inside the __init__ of Login, when the email verification is not explicitly set to a value, call this adapter method.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for reviewing I'll work on it.

Copy link
Author

@muneeb706 muneeb706 May 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In assess_unique_email method https://github.com/pennersr/django-allauth/blob/main/allauth/account/utils.py#L496

We only have access to email not the Login instance as it is called before we complete signup.
Is it a good idea to keep both login and email ? or login can be None in this scenario ?

return app_settings.EMAIL_VERIFICATION

def send_notification_mail(self, template_prefix, user, context=None, email=None):
from allauth.account.models import EmailAddress

Expand Down
6 changes: 4 additions & 2 deletions allauth/account/forms.py
Expand Up @@ -207,12 +207,14 @@ def login(self, request, redirect_url=None):
if field in credentials
}
record_authentication(request, method="password", **extra_data)
adapter = get_adapter()
email = credentials.get("email")
ret = perform_login(
request,
self.user,
email_verification=app_settings.EMAIL_VERIFICATION,
email_verification=adapter.get_email_verification_method(email),
redirect_url=redirect_url,
email=credentials.get("email"),
email=email,
)
remember = app_settings.SESSION_REMEMBER
if remember is None:
Expand Down
40 changes: 40 additions & 0 deletions allauth/account/tests/test_adapter.py
Expand Up @@ -2,6 +2,9 @@
from django.urls import reverse

from allauth.account.adapter import DefaultAccountAdapter
from allauth.account.app_settings import (
EmailVerificationMethod,
)
from allauth.core.exceptions import ImmediateHttpResponse


Expand All @@ -10,6 +13,13 @@ def pre_login(self, *args, **kwargs):
raise ImmediateHttpResponse(HttpResponseRedirect("/foo"))


class CustomAccountEmailVerificationAdapter(DefaultAccountAdapter):
def get_email_verification_method(self, email):
if email == "mandatory@example.com":
return EmailVerificationMethod.MANDATORY
return EmailVerificationMethod.OPTIONAL


def test_adapter_pre_login(settings, user, user_password, client):
settings.ACCOUNT_ADAPTER = (
"allauth.account.tests.test_adapter.PreLoginRedirectAccountAdapter"
Expand All @@ -20,3 +30,33 @@ def test_adapter_pre_login(settings, user, user_password, client):
)
assert resp.status_code == 302
assert resp["location"] == "/foo"


def test_custom_account_email_verification(settings, user_factory, client):
settings.ACCOUNT_ADAPTER = (
"allauth.account.tests.test_adapter.CustomAccountEmailVerificationAdapter"
)

resp = client.post(
reverse("account_signup"),
{
"username": "mandatory",
"email": "mandatory@example.com",
"password1": "mandatory123",
"password2": "mandatory123",
},
)
assert resp.status_code == 302
assert resp["location"] == reverse("account_email_verification_sent")

resp = client.post(
reverse("account_signup"),
{
"username": "optional",
"email": "optional@example.com",
"password1": "optional123",
"password2": "optional123",
},
)
assert resp.status_code == 302
assert resp["location"] == settings.LOGIN_REDIRECT_URL
3 changes: 2 additions & 1 deletion allauth/account/utils.py
Expand Up @@ -545,14 +545,15 @@ def assess_unique_email(email) -> Optional[bool]:
False -- email is already in use
None -- email is in use, but we should hide that using email verification.
"""
adapter = get_adapter()
if not filter_users_by_email(email):
# All good.
return True
elif not app_settings.PREVENT_ENUMERATION:
# Fail right away.
return False
elif (
app_settings.EMAIL_VERIFICATION
adapter.get_email_verification_method(email)
== app_settings.EmailVerificationMethod.MANDATORY
):
# In case of mandatory verification and enumeration prevention,
Expand Down
5 changes: 3 additions & 2 deletions allauth/account/views.py
Expand Up @@ -266,14 +266,15 @@ def get_success_url(self):
return ret

def form_valid(self, form):
adapter = get_adapter()
self.user, resp = form.try_save(self.request)
if resp:
return resp
try:
return complete_signup(
self.request,
self.user,
app_settings.EMAIL_VERIFICATION,
adapter.get_email_verification_method(self.user.email),
self.get_success_url(),
)
except ImmediateHttpResponse as e:
Expand Down Expand Up @@ -940,7 +941,7 @@ def form_valid(self, form):
return perform_login(
self.request,
self.reset_user,
email_verification=app_settings.EMAIL_VERIFICATION,
email_verification=adapter.get_email_verification_method(self.reset_user.email),
)

return super(PasswordResetFromKeyView, self).form_valid(form)
Expand Down
8 changes: 7 additions & 1 deletion allauth/socialaccount/adapter.py
Expand Up @@ -147,7 +147,10 @@ def validate_disconnect(self, account, accounts):
if not account.user.has_usable_password():
raise ValidationError(_("Your account has no password set up."))
# No email address, no password reset
if app_settings.EMAIL_VERIFICATION == EmailVerificationMethod.MANDATORY:
if (
get_account_adapter().get_email_verification_method(account.user.email)
== EmailVerificationMethod.MANDATORY
):
if not EmailAddress.objects.filter(
user=account.user, verified=True
).exists():
Expand Down Expand Up @@ -315,6 +318,9 @@ def get_requests_session(self):
)
return session

def get_email_verification_method(self, email=None):
return app_settings.EMAIL_VERIFICATION

def is_email_verified(self, provider, email):
"""
Returns ``True`` iff the given email encountered during a social
Expand Down
8 changes: 6 additions & 2 deletions allauth/socialaccount/helpers.py
Expand Up @@ -108,7 +108,9 @@ def _login_social_account(request, sociallogin):
return perform_login(
request,
sociallogin.user,
email_verification=app_settings.EMAIL_VERIFICATION,
email_verification=get_account_adapter().get_email_verification_method(
sociallogin.user.email
),
redirect_url=sociallogin.get_redirect_url(request),
signal_kwargs={"sociallogin": sociallogin},
)
Expand Down Expand Up @@ -244,7 +246,9 @@ def complete_social_signup(request, sociallogin):
return complete_signup(
request,
sociallogin.user,
app_settings.EMAIL_VERIFICATION,
get_account_adapter(request).get_email_verification_method(
sociallogin.user.email
),
sociallogin.get_redirect_url(request),
signal_kwargs={"sociallogin": sociallogin},
)
Expand Down
5 changes: 4 additions & 1 deletion allauth/socialaccount/providers/authentiq/provider.py
@@ -1,3 +1,4 @@
from allauth.socialaccount.adapter import get_adapter
from allauth.account.models import EmailAddress
from allauth.socialaccount import app_settings
from allauth.socialaccount.providers.base import AuthAction, ProviderAccount
Expand Down Expand Up @@ -58,14 +59,16 @@ class AuthentiqProvider(OAuth2Provider):
account_class = AuthentiqAccount

def get_scope(self, request):
adapter = get_adapter()
scope = set(super(AuthentiqProvider, self).get_scope(request))
scope.add("openid")

if Scope.EMAIL in scope:
modifiers = ""
if app_settings.EMAIL_REQUIRED:
modifiers += "r"
if app_settings.EMAIL_VERIFICATION:
#TODO: How to get user email here for get_email_verification_method ?
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can I get user's email in this method to pass it to get_email_verification_method ?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is called to get a list of scopes to ask permission for as part of the OAuth handshake. So, at this point in time you do not know what user is going to be authenticated, yet.

if adapter.get_email_verification_method():
modifiers += "s"
if modifiers:
scope.add(Scope.EMAIL + "~" + modifiers)
Expand Down