E-mail validation error #108

Closed
ndarville opened this Issue Feb 23, 2013 · 7 comments

Comments

Projects
None yet
1 participant
@ndarville
Owner

ndarville commented Feb 23, 2013

Identical e-mail does not throw ValidationError

+filter in e-mail does not throw ValidationError

form_tests.py is supposed to catch this with test_invalid_with_email_filter(), but it is still possible to locally register with

This has not been caught, because Travis wasn’t working.


Only one user is actually registered, though, if you look in the user list in the admin panel

It catches the error in the shell, though:

>>> user = {'username': 'test_user', 'email': 'test+filter@gmail.com', 'password1': 'password', 'password2': 'password'}
>>> Form(user).errors
{'email': [u'Sorry, we currently do not support +filters in e-mail addresses.']}
@ndarville

This comment has been minimized.

Show comment
Hide comment
@ndarville

ndarville Feb 23, 2013

Owner

The Problem

The problem is the absence of the **kwargs argument passed to render() removed in this commit in order to fix another problem.

Undoing the commit alone is not enough to fix the problem.

A view test for e-mail registration will also have to be written in order to catch errors of this sort in the future.

The code in question:

# views.py
def custom_register(request, **kwargs):
    """Registers users and redirects those who are already authenticated."""
    site = Site.objects.get_current()

    if request.user.is_authenticated():
        return HttpResponseRedirect(reverse('forum.views.home', args=()))
    else:
        site_config_error, email_config_error = False, False
        if site.domain == "example.com" or site.name == "example.com":
            site_config_error = True
        if project_settings.EMAIL_HOST_USER == "myusername@gmail.com":
            email_config_error = True

        return registration_views.register(request,
            backend='registration.backends.default.DefaultBackend',
            template_name='registration/registration_form.html',
            extra_context={
                'site_config_error':  site_config_error,
                'email_config_error': email_config_error,
                'max_username_length':
                    User._meta.get_field("username").max_length},
            **kwargs)

# urls.py
url(r'^' + getattr(project_settings, 'REGISTRATION_URL',
                                     '/accounts/register/')[1:] + '$',
                                     'custom_register',
                                    {'form_class':
                                      CustomRegistrationForm},
                                      name='register'),
Owner

ndarville commented Feb 23, 2013

The Problem

The problem is the absence of the **kwargs argument passed to render() removed in this commit in order to fix another problem.

Undoing the commit alone is not enough to fix the problem.

A view test for e-mail registration will also have to be written in order to catch errors of this sort in the future.

The code in question:

# views.py
def custom_register(request, **kwargs):
    """Registers users and redirects those who are already authenticated."""
    site = Site.objects.get_current()

    if request.user.is_authenticated():
        return HttpResponseRedirect(reverse('forum.views.home', args=()))
    else:
        site_config_error, email_config_error = False, False
        if site.domain == "example.com" or site.name == "example.com":
            site_config_error = True
        if project_settings.EMAIL_HOST_USER == "myusername@gmail.com":
            email_config_error = True

        return registration_views.register(request,
            backend='registration.backends.default.DefaultBackend',
            template_name='registration/registration_form.html',
            extra_context={
                'site_config_error':  site_config_error,
                'email_config_error': email_config_error,
                'max_username_length':
                    User._meta.get_field("username").max_length},
            **kwargs)

# urls.py
url(r'^' + getattr(project_settings, 'REGISTRATION_URL',
                                     '/accounts/register/')[1:] + '$',
                                     'custom_register',
                                    {'form_class':
                                      CustomRegistrationForm},
                                      name='register'),
@ndarville

This comment has been minimized.

Show comment
Hide comment
@ndarville

ndarville Feb 23, 2013

Owner

Try deploying a test instance to dotCloud, after their back-end gets fixed on Wednesday.

Owner

ndarville commented Feb 23, 2013

Try deploying a test instance to dotCloud, after their back-end gets fixed on Wednesday.

@ndarville

This comment has been minimized.

Show comment
Hide comment
@ndarville

ndarville Feb 23, 2013

Owner

View test:

    @override_settings(
        EMAIL_BACKEND='django.core.mail.backends.dummy.EmailBackend')
    def test_register_user_with_email_filter(self):
        """Tests registering a user with an e-mail filter."""
        self.client.post(
            reverse('forum.views.custom_register'), {
                'username':  'Foo',
                'email':     'contact+filter@example.com',
                'password1': 'password',
                'password2': 'password'})

    @override_settings(
        EMAIL_BACKEND='django.core.mail.backends.dummy.EmailBackend')
    def test_register_user_with_taken_username(self):
        """Tests registering a user with an e-mail filter."""
        self.client.post(
            reverse('forum.views.custom_register'), {
                'username':  'admin',
                'email':     'contact@example.com',
                'password1': 'password',
                'password2': 'password'})

Find out which return value to fetch from custom_register() to determine the test outcome.

Owner

ndarville commented Feb 23, 2013

View test:

    @override_settings(
        EMAIL_BACKEND='django.core.mail.backends.dummy.EmailBackend')
    def test_register_user_with_email_filter(self):
        """Tests registering a user with an e-mail filter."""
        self.client.post(
            reverse('forum.views.custom_register'), {
                'username':  'Foo',
                'email':     'contact+filter@example.com',
                'password1': 'password',
                'password2': 'password'})

    @override_settings(
        EMAIL_BACKEND='django.core.mail.backends.dummy.EmailBackend')
    def test_register_user_with_taken_username(self):
        """Tests registering a user with an e-mail filter."""
        self.client.post(
            reverse('forum.views.custom_register'), {
                'username':  'admin',
                'email':     'contact@example.com',
                'password1': 'password',
                'password2': 'password'})

Find out which return value to fetch from custom_register() to determine the test outcome.

@ndarville

This comment has been minimized.

Show comment
Hide comment
@ndarville

ndarville Feb 26, 2013

Owner
>>> send_mail('subject', 'message', 'foo@gmail.com', ['foo@gmail.com'])
1
>>> send_mail('subject', 'message', 'Do Not Reply <foo@gmail.com>', ['foo@gmail.com'])
1

Using EmailMessage()

>>> EmailMessage('subject', 'message', 'foo@gmail.com', ['foo@gmail.com']).send()
1
>>> EmailMessage('subject', 'message', 'Foo <foo@gmail.com>', ['foo@gmail.com']).send()
1
Owner

ndarville commented Feb 26, 2013

>>> send_mail('subject', 'message', 'foo@gmail.com', ['foo@gmail.com'])
1
>>> send_mail('subject', 'message', 'Do Not Reply <foo@gmail.com>', ['foo@gmail.com'])
1

Using EmailMessage()

>>> EmailMessage('subject', 'message', 'foo@gmail.com', ['foo@gmail.com']).send()
1
>>> EmailMessage('subject', 'message', 'Foo <foo@gmail.com>', ['foo@gmail.com']).send()
1
@ndarville

This comment has been minimized.

Show comment
Hide comment
@ndarville

ndarville Mar 1, 2013

Owner

Using the settings from the ticket Django app paid off on dotCloud. The files replaced were definesite.py and postinstall.

Owner

ndarville commented Mar 1, 2013

Using the settings from the ticket Django app paid off on dotCloud. The files replaced were definesite.py and postinstall.

@ndarville

This comment has been minimized.

Show comment
Hide comment
@ndarville

ndarville Mar 23, 2013

Owner

Removing the exception-catching lets the form catch used e-mail addresses and +filters:

# try:
return registration_views.register(request,
    backend='registration.backends.default.DefaultBackend',
    template_name='registration/registration_form.html',
    extra_context={
        'site_config_error':  site_config_error,
        'email_config_error': email_config_error,
        'max_username_length':
            User._meta.get_field("username").max_length},
    form_class=CustomRegistrationForm)
# except SMTPRecipientsRefused:
#     messages.error(request,
#         "The e-mail server refused to send an e-mail to your address.\
#         Contact the site administrator, if this problem persists.")
#     return HttpResponseRedirect(reverse('forum.views.custom_register', args=()))

... because the correct form_class is used.

Instead, I get this:

SMTPRecipientsRefused at /accounts/register/

{'': (555, '5.5.2 Syntax error. t17sm2722868lam.9 - gsmtp')}

The latter breaks into

{
    '': (
        555,
        '5.5.2 Syntax error. t17sm2722868lam.9 - gsmtp'
    )
}
Owner

ndarville commented Mar 23, 2013

Removing the exception-catching lets the form catch used e-mail addresses and +filters:

# try:
return registration_views.register(request,
    backend='registration.backends.default.DefaultBackend',
    template_name='registration/registration_form.html',
    extra_context={
        'site_config_error':  site_config_error,
        'email_config_error': email_config_error,
        'max_username_length':
            User._meta.get_field("username").max_length},
    form_class=CustomRegistrationForm)
# except SMTPRecipientsRefused:
#     messages.error(request,
#         "The e-mail server refused to send an e-mail to your address.\
#         Contact the site administrator, if this problem persists.")
#     return HttpResponseRedirect(reverse('forum.views.custom_register', args=()))

... because the correct form_class is used.

Instead, I get this:

SMTPRecipientsRefused at /accounts/register/

{'': (555, '5.5.2 Syntax error. t17sm2722868lam.9 - gsmtp')}

The latter breaks into

{
    '': (
        555,
        '5.5.2 Syntax error. t17sm2722868lam.9 - gsmtp'
    )
}
@ndarville

This comment has been minimized.

Show comment
Hide comment
@ndarville

ndarville Mar 23, 2013

Owner

Tried

  • Used the format 'foo@example.com <foo@example.com>' as FROM address.
    • Done in settings.py and local_settings.py
  • Used the format 'foo@example.com <foo@example.com>' as TO address.
    • Done in forms.py

This fixes the problem—together with the aforementioned views.py changes.

Detailed in http://blog.yimingliu.com/2008/11/26/email-servers-and-mail-from-syntax.

Remember to check both settings.py and local_settings.py.

Owner

ndarville commented Mar 23, 2013

Tried

  • Used the format 'foo@example.com <foo@example.com>' as FROM address.
    • Done in settings.py and local_settings.py
  • Used the format 'foo@example.com <foo@example.com>' as TO address.
    • Done in forms.py

This fixes the problem—together with the aforementioned views.py changes.

Detailed in http://blog.yimingliu.com/2008/11/26/email-servers-and-mail-from-syntax.

Remember to check both settings.py and local_settings.py.

@ndarville ndarville closed this in ce5035a Mar 23, 2013

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