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

Conversation

muneeb706
Copy link

Submitting Pull Requests

General

  • Make sure you use semantic commit messages.
    Examples: "fix(google): Fixed foobar bug", "feat(accounts): Added foobar feature".
  • All Python code must formatted using Black, and clean from pep8 and isort issues.
  • JavaScript code should adhere to StandardJS.
  • If your changes are significant, please update ChangeLog.rst.
  • If your change is substantial, feel free to add yourself to AUTHORS.

Provider Specifics

N/A

  • Make sure unit tests are available.
  • Add an entry of your provider in test_settings.py::INSTALLED_APPS and docs/installation.rst::INSTALLED_APPS.
  • Add documentation to docs/providers/<provider name>.rst and docs/providers/index.rst Provider Specifics toctree.
  • Add an entry to the list of supported providers over at docs/overview.rst.

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.

@@ -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 ?

@muneeb706 muneeb706 marked this pull request as draft May 1, 2024 06:45
@muneeb706 muneeb706 marked this pull request as ready for review May 2, 2024 06:07
@muneeb706 muneeb706 requested a review from pennersr May 2, 2024 06:36
@muneeb706 muneeb706 changed the title feat(adapter): Custom account email verification through adapter #1946 Custom account email verification through adapter #1946 May 2, 2024
@coveralls
Copy link

Coverage Status

coverage: 95.698% (+0.02%) from 95.683%
when pulling 3d9a847 on muneeb706:feature_1946-customizable-account-email-verification
into fac8b9a on pennersr:main.

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

Successfully merging this pull request may close these issues.

None yet

3 participants