pennersr / django-allauth

Integrated set of Django applications addressing authentication, registration, account management as well as 3rd party (social) account authentication.
https://allauth.org
MIT License
9.54k stars 3.03k forks source link

Custom user attributes not validated by UserAttributeSimilarityValidator #1989

Closed timdiels closed 1 year ago

timdiels commented 6 years ago

On account_signup and account_password_change, password similarity is only checked against email and username. Most likely due to the code near this line only setting email and username. How would you suggest I add these password validation checks in all the views (signup, password change), in my code?

Edit: we're not using social accounts, only local ones.

Also, the dummy_user here is a class, not a User instance. This ends up calling UserAttributeSimilarityValidator.validate with a user class instead of an instance, not sure if that's correct.

settings.py:

AUTH_USER_MODEL = 'core.User'

AUTH_PASSWORD_VALIDATORS = [
    {
        # Not a variation of a user attribute
        'NAME': 'django.contrib.auth.password_validation.UserAttributeSimilarityValidator',
        'OPTIONS': {
            'user_attributes': (
                'username', 'email', 'first_name', 'last_name', 'lab'
            ),
        }
    },
    {
        # Minimum password length
        'NAME': 'django.contrib.auth.password_validation.MinimumLengthValidator',
        'OPTIONS': {
            'min_length': 8,
        }
    },
    {
        # Not one of a list of common passwords
        'NAME': 'django.contrib.auth.password_validation.CommonPasswordValidator',
    },
    {
        # Not entirely numeric
        'NAME': 'django.contrib.auth.password_validation.NumericPasswordValidator',
    },
]

# allauth config
ACCOUNT_AUTHENTICATION_METHOD = 'username'  # Log in by username
ACCOUNT_EMAIL_REQUIRED = True  # Have to provide email when signing up
ACCOUNT_EMAIL_VERIFICATION = 'mandatory'  # Cannot log in until email is verified
ACCOUNT_SIGNUP_FORM_CLASS = 'orcae.core.forms.SignupForm'

core.User:

from django.contrib.auth.models import AbstractUser
from django.db import models

class User(AbstractUser):
    lab = models.CharField(
        max_length=250, blank=True, null=True,
        help_text='Name of the lab you are part of, if any'
    )

    def __str__(self):
        return 'User(username={s.username!r})'.format(s=self)

custom SignupForm:

from django import forms
from .models import User

class SignupForm(forms.ModelForm):

    '''
    Form with fields to add to the default signup form
    '''

    class Meta:
        model = User
        fields = ['first_name', 'last_name', 'lab']

    def signup(self, request, user):
        user.first_name = self.instance.first_name
        user.last_name = self.instance.last_name
        user.lab = self.instance.lab
        user.save()

Package versions:

Django==2.0.2
django-allauth==0.35.0
timdiels commented 6 years ago

Seems overriding SignupForm ~(and soon change_password and set_password)~ is the way to go in my case.

ACCOUNT_FORMS = {
    'signup': 'orcae.core.forms.allauth.SignupForm'
}
from allauth.account.forms import SignupForm as AllauthSignupForm
from allauth.account.adapter import get_adapter
from django import forms

from orcae.core.models import User

class SignupForm(AllauthSignupForm):

    '''
    Custom allauth signup form

    Modified to take into account our extra user fields, tightly coupled to
    SignupExtraForm.
    '''

    def clean(self):
        # Upstream cleaning/validation
        super().clean()

        # Validate by similarity to first/last name and lab
        password = self.cleaned_data.get('password1')
        user = User(
            first_name=self.cleaned_data.get('first_name'),
            last_name=self.cleaned_data.get('last_name'),
            lab=self.cleaned_data.get('lab'),
        )
        if password:
            try:
                get_adapter().clean_password(password, user=user)
            except forms.ValidationError as e:
                self.add_error('password1', e)

        #
        return self.cleaned_data

Edit: the default password change/set/reset_from_key forms worked just fine.

timdiels commented 6 years ago

So in summary, I found a workaround (above) and see the following issues:

merwok commented 3 years ago

dummy_user being a class means that validation is not performed correctly :/

derek-adair commented 1 year ago

Close as fixed(?).

Looks like dummy user is a User instance now.... didn't read that closely here, there may be more to it than the dummy_user issue.

merwok commented 1 year ago

Indeed! https://github.com/pennersr/django-allauth/commit/16a8b7c743ea2cfedd14be4de74862ac3e9a6ba3