Allow users without passwords, create user via social pipeline

misago/users/models/user.py

@@ -35,8 +35,6 @@ class UserManager(BaseUserManager):
 
         if not email:
             raise ValueError(_("User must have an email address."))
-        if not password:
-            raise ValueError(_("User must have a password."))
 
         if not 'joined_from_ip' in extra_fields:
             extra_fields['joined_from_ip'] = '127.0.0.1'
@@ -66,7 +64,10 @@ class UserManager(BaseUserManager):
 
         validate_username(username)
         validate_email(email)
-        validate_password(password, user=user)
+        
+        if password:
+            # password is conditional: users created with social-auth don't have one
+            validate_password(password, user=user)
 
         if not 'rank' in extra_fields:
             user.rank = Rank.objects.get_default()

misago/users/social/pipeline.py

@@ -1,12 +1,14 @@
 from django.contrib.auth import get_user_model
 from django.utils.translation import ugettext as _
 
+from misago.conf import settings
 from misago.core.exceptions import SocialAuthFailed, SocialAuthBanned
 
 from misago.users.models import Ban
 from misago.users.bans import get_request_ip_ban, get_user_ban
+from misago.users.validators import ValidationError, validate_username, validate_email
 
-from .utils import get_social_auth_backend_name
+from .utils import get_social_auth_backend_name, perpare_username
 
 
 UserModel = get_user_model()
@@ -14,7 +16,7 @@ UserModel = get_user_model()
 
 def validate_ip_not_banned(strategy, details, backend, user=None, *args, **kwargs):
     """Pipeline step that interrupts pipeline if found user is non-staff and IP banned"""
-    if user and user.is_staff:
+    if not user or user.is_staff:
         return None
     
     ban = get_request_ip_ban(strategy.request)
@@ -29,7 +31,7 @@ def validate_ip_not_banned(strategy, details, backend, user=None, *args, **kwarg
 
 def validate_user_not_banned(strategy, details, backend, user=None, *args, **kwargs):
     """Pipeline step that interrupts pipeline if found user is non-staff and banned"""
-    if user and user.is_staff:
+    if not user or user.is_staff:
         return None
 
     user_ban = get_user_ban(user)
@@ -76,10 +78,64 @@ def associate_by_email(strategy, details, backend, user=None, *args, **kwargs):
     return {'user': user, 'is_new': False}
 
 
+def get_username(strategy, details, backend, user=None, *args, **kwargs):
+    """Resolve valid username for use in new account"""
+    if user:
+        return None
+
+    username = perpare_username(details.get('username', ''))
+    full_name = perpare_username(details.get('full_name', ''))
+    first_name = perpare_username(details.get('first_name', ''))
+    last_name = perpare_username(details.get('last_name', ''))
+
+    names_to_try = [
+        username,
+        first_name,
+    ]
+
+    if username:
+        names_to_try.append(username)
+
+    if first_name:
+        names_to_try.append(first_name)
+        if last_name:
+            # if first name is taken, try first name + first char of last name
+            names_to_try.append(first_name + last_name[0])
+
+    if full_name:
+        names_to_try.append(full_name)
+
+    username_length_max = settings.username_length_max
+    for name in names_to_try:
+        if len(name) > username_length_max:
+            names_to_try.append(name[:username_length_max])
+
+    for name in filter(bool, names_to_try):
+        try:
+            validate_username(name)
+            return {'clean_username': name}
+        except ValidationError:
+            pass
+
+
 def create_user(strategy, details, backend, user=None, *args, **kwargs):
     """Aggressively attempt to register and sign in new user"""
     if user:
         return None
+    
+    email = details.get('email')
+    username = kwargs.get('clean_username')
+    
+    if not email or not username:
+        return None
+
+    try:
+        validate_email(email)
+    except ValidationError:
+        return None
+
+    user = UserModel.objects.create_user(username, email, set_default_avatar=True)
+    return {'user': user, 'is_new': True}
 
 
 def create_user_with_form(strategy, details, backend, user=None, *args, **kwargs):

misago/users/social/utils.py

@@ -1,5 +1,6 @@
 from django.urls import reverse
 from social_core.backends.utils import load_backends
+from unidecode import unidecode
 
 from misago.conf import settings
 
@@ -25,4 +26,8 @@ def get_social_auth_backend_name(backend_id):
         return settings.MISAGO_SOCIAL_AUTH_BACKENDS_NAMES[backend_id]
     if backend_id in BACKENDS_NAMES:
         return BACKENDS_NAMES[backend_id]
-    return backend_id.title()
+    return backend_id.title()
+
+
+def perpare_username(username):
+    return ''.join(filter(str.isalnum, unidecode(username)))

misago/users/tests/test_auth_api.py

@@ -93,6 +93,23 @@ class GatewayTests(TestCase):
         )
         self.assertContains(response, "Invalid data.", status_code=400)
 
+    def test_login_not_usable_password(self):
+        """login api fails to sign user with not-usable password in"""
+        UserModel.objects.create_user('Bob', 'bob@test.com')
+
+        response = self.client.post(
+            '/api/auth/',
+            data={
+                'username': 'Bob',
+                'password': 'Pass.123',
+            },
+        )
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.json(), {
+            'code': 'invalid_login',
+            'detail': 'Login or password is incorrect.',
+        })
+
     def test_login_banned(self):
         """login api fails to sign banned user in"""
         UserModel.objects.create_user('Bob', 'bob@test.com', 'Pass.123')

misago/users/tests/test_social_pipeline.py

@@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
 from django.contrib.auth import get_user_model
 from social_core.backends.github import GithubOAuth2
 
@@ -5,7 +6,7 @@ from misago.core.exceptions import SocialAuthFailed, SocialAuthBanned
 
 from misago.users.models import Ban, BanCache
 from misago.users.social.pipeline import (
-    associate_by_email, validate_ip_not_banned, validate_user_not_banned
+    associate_by_email, create_user, get_username, validate_ip_not_banned, validate_user_not_banned
 )
 from misago.users.testutils import UserTestCase
 
@@ -93,10 +94,133 @@ class AssociateByEmailTests(PipelineTestCase):
         self.assertEqual(result, {'user': self.user, 'is_new': False})
     
 
+class CreateUser(PipelineTestCase):
+    def test_skip_if_user_is_set(self):
+        """pipeline step is skipped if user was passed"""
+        result = create_user(None, {}, None, user=self.user)
+        self.assertIsNone(result)
+
+    def test_skip_if_no_email_passed(self):
+        """pipeline step is skipped if no email was passed"""
+        details = {
+            'clean_username': 'TestBob',
+        }
+        result = create_user(None, details, None)
+        self.assertIsNone(result)
+
+    def test_skip_if_no_clean_username_passed(self):
+        """pipeline step is skipped if cleaned username wasnt passed"""
+        details = {
+            'email': 'hello@example.com',
+        }
+        result = create_user(None, details, None)
+        self.assertIsNone(result)
+
+    def test_skip_if_email_is_taken(self):
+        """pipeline step is skipped if email was taken"""
+        details = {
+            'email': self.user.email,
+        }
+        result = create_user(None, details, None)
+        self.assertIsNone(result)
+
+    def test_user_is_created(self):
+        """pipeline step returns user if data is correct"""
+        details = {
+            'email': 'new@example.com'
+            'clean_username': 'NewUser',
+        }
+        result = create_user(None, details, None)
+        new_user = UserModel.objects.get(email='new@example.com')
+        self.assertEqual(result, {
+            'user': new_user,
+            'is_new': True,
+        })
+        self.assertEqual(new_user.username, 'NewUser')
+        self.assertFalse(new_user.has_useable_password())
+
+
+class GetUsernameTests(PipelineTestCase):
+    def test_skip_if_user_is_set(self):
+        """pipeline step is skipped if user was passed"""
+        result = get_username(None, {}, None, user=self.user)
+        self.assertIsNone(result)
+
+    def test_skip_if_no_names(self):
+        """pipeline step is skipped if API returned no names"""
+        result = get_username(None, {}, None)
+        self.assertIsNone(result)
+
+    def test_resolve_to_username(self):
+        """pipeline step resolves username"""
+        result = get_username(None, {'username': 'BobBoberson'}, None)
+        self.assertEqual(result, {'clean_username': 'BobBoberson'})
+
+    def test_normalize_username(self):
+        """pipeline step normalizes username"""
+        result = get_username(None, {'username': u'Błop Błoperson'}, None)
+        self.assertEqual(result, {'clean_username': 'BlopBloperson'})
+
+    def test_resolve_to_first_name(self):
+        """pipeline attempts to use first name because username is taken"""
+        details = {
+            'username': self.user.username,
+            'first_name': u'Błob',
+        }
+        result = get_username(None, details, None)
+        self.assertEqual(result, {'clean_username': 'Blob'})
+
+    def test_dont_resolve_to_last_name(self):
+        """pipeline will not fallback to last name because username is taken"""
+        details = {
+            'username': self.user.username,
+            'last_name': u'Błob',
+        }
+        result = get_username(None, details, None)
+        self.assertIsNone(result)
+
+    def test_resolve_to_first_last_name_first_char(self):
+        """pipeline will construct username from first name and first char of surname"""
+        details = {
+            'first_name': self.user.username,
+            'last_name': u'Błob',
+        }
+        result = get_username(None, details, None)
+        self.assertEqual(result, {'clean_username': self.user.username + 'B'})
+
+    def test_dont_resolve_to_banned_name(self):
+        """pipeline will not resolve to banned name"""
+        Ban.objects.create(banned_value='*Admin*', check_type=Ban.USERNAME)
+        details = {
+            'username': 'Misago Admin',
+            'first_name': u'Błob',
+        }
+        result = get_username(None, details, None)
+        self.assertEqual(result, {'clean_username': 'Blob'})
+
+    def test_resolve_full_name(self):
+        """pipeline will resolve to full name"""
+        Ban.objects.create(banned_value='*Admin*', check_type=Ban.USERNAME)
+        details = {
+            'username': 'Misago Admin',
+            'full_name': u'Błob Błopo',
+        }
+        result = get_username(None, details, None)
+        self.assertEqual(result, {'clean_username': 'BlobBlopo'})
+
+    def test_resolve_to_cut_name(self):
+        """pipeline will resolve cut too long name on second pass"""
+        details = {
+            'username': u'Abrakadabrapokuskonstantynopolitańczykowianeczkatrzy',
+        }
+        result = get_username(None, details, None)
+        self.assertEqual(result, {'clean_username': 'Abrakadabrapok'})
+
+
 class ValidateIpNotBannedTests(PipelineTestCase):
     def test_skip_if_user_not_set(self):
         """pipeline step is skipped if no user was passed"""
-        result = associate_by_email(None, {}, GithubOAuth2)
+        result = validate_ip_not_banned(None, {}, GithubOAuth2)
         self.assertIsNone(result)
 
     def test_raise_if_banned(self):
@@ -124,7 +248,7 @@ class ValidateIpNotBannedTests(PipelineTestCase):
 class ValidateUserNotBannedTests(PipelineTestCase):
     def test_skip_if_user_not_set(self):
         """pipeline step is skipped if no user was passed"""
-        result = associate_by_email(None, {}, GithubOAuth2)
+        result = validate_user_not_banned(None, {}, GithubOAuth2)
         self.assertIsNone(result)
 
     def test_raise_if_banned(self):