From a82e15f23793900119104addd67f8ebf703e5b15 Mon Sep 17 00:00:00 2001 From: Gareth Coles Date: Sat, 19 Oct 2019 16:21:38 +0100 Subject: Allauth: Re-add GitHub provider, prevent GH signups --- pydis_site/utils/account.py | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 pydis_site/utils/account.py (limited to 'pydis_site/utils') diff --git a/pydis_site/utils/account.py b/pydis_site/utils/account.py new file mode 100644 index 00000000..adafcea9 --- /dev/null +++ b/pydis_site/utils/account.py @@ -0,0 +1,44 @@ +from allauth.account.adapter import DefaultAccountAdapter +from allauth.exceptions import ImmediateHttpResponse +from allauth.socialaccount.adapter import DefaultSocialAccountAdapter +from allauth.socialaccount.models import SocialLogin +from django.contrib.messages import ERROR, add_message +from django.http import HttpRequest +from django.shortcuts import redirect +from django.urls import reverse + + +class AccountAdapter(DefaultAccountAdapter): + """An Allauth account adapter that prevents signups via form submission.""" + + def is_open_for_signup(self, request: HttpRequest) -> bool: + """ + Checks whether or not the site is open for signups. + + We override this to always return False so that users may never sign up using + Allauth's signup form endpoints, to be on the safe side - since we only want users + to sign up using their Discord account. + """ + return False + + +class SocialAccountAdapter(DefaultSocialAccountAdapter): + """An Allauth SocialAccount adapter that prevents signups via non-Discord connections.""" + + def is_open_for_signup(self, request: HttpRequest, social_login: SocialLogin) -> bool: + """ + Checks whether or not the site is open for signups. + + We override this method in order to prevent users from creating a new account using + a non-Discord connection, as we require this connection for our users. + """ + if social_login.account.provider != "discord": + add_message( + request, ERROR, + "You must login with Discord before connecting another account. Your account " + "details have not been saved." + ) + + raise ImmediateHttpResponse(redirect(reverse("home"))) + + return True -- cgit v1.2.3 From 3caa7675f9c5c33b6a8e91c633945d5152383a76 Mon Sep 17 00:00:00 2001 From: Gareth Coles Date: Sun, 20 Oct 2019 18:05:33 +0100 Subject: Update Django users with Discord username+discrim. This sets both the Django User's `username` and `first_name` params. --- pydis_site/__init__.py | 5 +++++ pydis_site/apps/home/signals.py | 11 +++++++++-- pydis_site/settings.py | 7 ++++++- pydis_site/utils/account.py | 20 ++++++++++++++++++++ 4 files changed, 40 insertions(+), 3 deletions(-) (limited to 'pydis_site/utils') diff --git a/pydis_site/__init__.py b/pydis_site/__init__.py index c6146450..df67cf71 100644 --- a/pydis_site/__init__.py +++ b/pydis_site/__init__.py @@ -2,3 +2,8 @@ from wiki.plugins.macros.mdx import toc # Remove the toc header prefix. There's no option for this, so we gotta monkey patch it. toc.HEADER_ID_PREFIX = '' + +# Empty list of validators for Allauth to ponder over. This is referred to in settings.py +# by a string because Allauth won't let us just give it a list _there_, we have to point +# at a list _somewhere else_ instead. +VALIDATORS = [] diff --git a/pydis_site/apps/home/signals.py b/pydis_site/apps/home/signals.py index 43e861d2..4cb4564b 100644 --- a/pydis_site/apps/home/signals.py +++ b/pydis_site/apps/home/signals.py @@ -262,6 +262,13 @@ class AllauthSignalListener: except SocialAccount.user.RelatedObjectDoesNotExist: return # There's no user account yet, this will be handled by another receiver + # Ensure that the username on this account is correct + new_username = f"{user.name}#{user.discriminator}" + + if account.user.username != new_username: + account.user.username = new_username + account.user.first_name = new_username + if not user.in_guild: deletion = True @@ -278,7 +285,6 @@ class AllauthSignalListener: if account.user.is_staff: # They're marked as a staff user and they shouldn't be, so let's fix that account.user.is_staff = False - account.user.save(update_fields=("is_staff", )) else: new_groups = [] is_staff = False @@ -304,4 +310,5 @@ class AllauthSignalListener: if account.user.is_staff != is_staff: account.user.is_staff = is_staff - account.user.save(update_fields=("is_staff", )) + + account.user.save() diff --git a/pydis_site/settings.py b/pydis_site/settings.py index d6ca4860..0d893b2c 100644 --- a/pydis_site/settings.py +++ b/pydis_site/settings.py @@ -409,7 +409,12 @@ AUTHENTICATION_BACKENDS = ( ) ACCOUNT_ADAPTER = "pydis_site.utils.account.AccountAdapter" -ACCOUNT_EMAIL_REQUIRED = False # Undocumented allauth setting; don't require emails +ACCOUNT_EMAIL_REQUIRED = False # Undocumented allauth setting; don't require emails ACCOUNT_EMAIL_VERIFICATION = "none" # No verification required; we don't use emails for anything + +# We use this validator because Allauth won't let us actually supply a list with no validators +# in it, and we can't just give it a lambda - that'd be too easy, I suppose. +ACCOUNT_USERNAME_VALIDATORS = "pydis_site.VALIDATORS" + LOGIN_REDIRECT_URL = "home" SOCIALACCOUNT_ADAPTER = "pydis_site.utils.account.SocialAccountAdapter" diff --git a/pydis_site/utils/account.py b/pydis_site/utils/account.py index adafcea9..9faad986 100644 --- a/pydis_site/utils/account.py +++ b/pydis_site/utils/account.py @@ -1,7 +1,10 @@ +from typing import Any, Dict + from allauth.account.adapter import DefaultAccountAdapter from allauth.exceptions import ImmediateHttpResponse from allauth.socialaccount.adapter import DefaultSocialAccountAdapter from allauth.socialaccount.models import SocialLogin +from django.contrib.auth.models import User from django.contrib.messages import ERROR, add_message from django.http import HttpRequest from django.shortcuts import redirect @@ -42,3 +45,20 @@ class SocialAccountAdapter(DefaultSocialAccountAdapter): raise ImmediateHttpResponse(redirect(reverse("home"))) return True + + def populate_user(self, request: HttpRequest, + social_login: SocialLogin, + data: Dict[str, Any]) -> User: + """ + Method used to populate a Django User with data. + + We override this so that the Django user is created with the username#discriminator, + instead of just the username, as Django users must have unique usernames. For display + purposes, we also set the `name` key, which is used for `first_name` in the database. + """ + if social_login.account.provider == "discord": + discriminator = social_login.account.extra_data["discriminator"] + data["username"] = f"{data['username']}#{discriminator}" + data["name"] = data["username"] + + return super().populate_user(request, social_login, data) -- cgit v1.2.3 From 604e3c09f9d07268bc4424fd06b0a1dcfb1b8976 Mon Sep 17 00:00:00 2001 From: Gareth Coles Date: Tue, 22 Oct 2019 17:05:01 +0100 Subject: Prevent user signup when they've never joined+verified --- pydis_site/tests/test_utils_account.py | 94 +++++++++++++++++++++++++++++++--- pydis_site/utils/account.py | 29 ++++++++--- 2 files changed, 110 insertions(+), 13 deletions(-) (limited to 'pydis_site/utils') diff --git a/pydis_site/tests/test_utils_account.py b/pydis_site/tests/test_utils_account.py index 91c2808c..d2946368 100644 --- a/pydis_site/tests/test_utils_account.py +++ b/pydis_site/tests/test_utils_account.py @@ -7,6 +7,7 @@ from django.contrib.messages.storage.base import BaseStorage from django.http import HttpRequest from django.test import TestCase +from pydis_site.apps.api.models import Role, User as DiscordUser from pydis_site.utils.account import AccountAdapter, SocialAccountAdapter @@ -18,21 +19,74 @@ class AccountUtilsTests(TestCase): user=self.django_user, provider="discord", uid=0 ) + self.discord_account_role = SocialAccount.objects.create( + user=self.django_user, provider="discord", uid=1 + ) + + self.discord_account_two_roles = SocialAccount.objects.create( + user=self.django_user, provider="discord", uid=2 + ) + + self.discord_account_not_present = SocialAccount.objects.create( + user=self.django_user, provider="discord", uid=3 + ) + self.github_account = SocialAccount.objects.create( user=self.django_user, provider="github", uid=0 ) + self.discord_user = DiscordUser.objects.create( + id=0, + name="user", + discriminator=0 + ) + + self.discord_user_role = DiscordUser.objects.create( + id=1, + name="user present", + discriminator=0 + ) + + self.discord_user_two_roles = DiscordUser.objects.create( + id=2, + name="user with both roles", + discriminator=0 + ) + + everyone_role = Role.objects.create( + id=0, + name="@everyone", + colour=0, + permissions=0, + position=0 + ) + + self.discord_user_role.roles.add(everyone_role) + self.discord_user_two_roles.roles.add(everyone_role) + + self.discord_user_two_roles.roles.add(Role.objects.create( + id=1, + name="Developers", + colour=0, + permissions=0, + position=1 + )) + def test_account_adapter(self): """Test that our Allauth account adapter functions correctly.""" adapter = AccountAdapter() self.assertFalse(adapter.is_open_for_signup(HttpRequest())) - def test_social_account_adapter(self): - """Test that our Allauth social account adapter functions correctly.""" + def test_social_account_adapter_signup(self): + """Test that our Allauth social account adapter correctly handles signups.""" adapter = SocialAccountAdapter() discord_login = SocialLogin(account=self.discord_account) + discord_login_role = SocialLogin(account=self.discord_account_role) + discord_login_two_roles = SocialLogin(account=self.discord_account_two_roles) + discord_login_not_present = SocialLogin(account=self.discord_account_not_present) + github_login = SocialLogin(account=self.github_account) messages_request = HttpRequest() @@ -43,8 +97,36 @@ class AccountUtilsTests(TestCase): with self.assertRaises(ImmediateHttpResponse): adapter.is_open_for_signup(messages_request, github_login) - self.assertEqual(len(messages_request._messages._queued_messages), 1) - self.assertEqual(mock_redirect.call_count, 1) - self.assertEqual(mock_reverse.call_count, 1) + with self.assertRaises(ImmediateHttpResponse): + adapter.is_open_for_signup(messages_request, discord_login) + + with self.assertRaises(ImmediateHttpResponse): + adapter.is_open_for_signup(messages_request, discord_login_role) + + with self.assertRaises(ImmediateHttpResponse): + adapter.is_open_for_signup(messages_request, discord_login_not_present) + + self.assertEqual(len(messages_request._messages._queued_messages), 4) + self.assertEqual(mock_redirect.call_count, 4) + self.assertEqual(mock_reverse.call_count, 4) + + self.assertTrue(adapter.is_open_for_signup(HttpRequest(), discord_login_two_roles)) + + def test_social_account_adapter_populate(self): + """Test that our Allauth social account adapter correctly handles data population.""" + adapter = SocialAccountAdapter() + + discord_login = SocialLogin( + account=self.discord_account, + user=self.django_user + ) + + discord_login.account.extra_data["discriminator"] = "0000" + + user = adapter.populate_user( + HttpRequest(), discord_login, + {"username": "user"} + ) - self.assertTrue(adapter.is_open_for_signup(HttpRequest(), discord_login)) + self.assertEqual(user.username, "user#0000") + self.assertEqual(user.first_name, "user#0000") diff --git a/pydis_site/utils/account.py b/pydis_site/utils/account.py index 9faad986..e64919de 100644 --- a/pydis_site/utils/account.py +++ b/pydis_site/utils/account.py @@ -4,12 +4,19 @@ from allauth.account.adapter import DefaultAccountAdapter from allauth.exceptions import ImmediateHttpResponse from allauth.socialaccount.adapter import DefaultSocialAccountAdapter from allauth.socialaccount.models import SocialLogin -from django.contrib.auth.models import User +from django.contrib.auth.models import User as DjangoUser from django.contrib.messages import ERROR, add_message from django.http import HttpRequest from django.shortcuts import redirect from django.urls import reverse +from pydis_site.apps.api.models import User as DiscordUser + +ERROR_CONNECT_DISCORD = ("You must login with Discord before connecting another account. " + "Your account details have not been saved.") +ERROR_JOIN_DISCORD = ("Please join the Discord server and verify that you accept the rules and " + "privacy policy.") + class AccountAdapter(DefaultAccountAdapter): """An Allauth account adapter that prevents signups via form submission.""" @@ -36,11 +43,19 @@ class SocialAccountAdapter(DefaultSocialAccountAdapter): a non-Discord connection, as we require this connection for our users. """ if social_login.account.provider != "discord": - add_message( - request, ERROR, - "You must login with Discord before connecting another account. Your account " - "details have not been saved." - ) + add_message(request, ERROR, ERROR_CONNECT_DISCORD) + + raise ImmediateHttpResponse(redirect(reverse("home"))) + + try: + user = DiscordUser.objects.get(id=int(social_login.account.uid)) + except DiscordUser.DoesNotExist: + add_message(request, ERROR, ERROR_JOIN_DISCORD) + + raise ImmediateHttpResponse(redirect(reverse("home"))) + + if user.roles.count() <= 1: + add_message(request, ERROR, ERROR_JOIN_DISCORD) raise ImmediateHttpResponse(redirect(reverse("home"))) @@ -48,7 +63,7 @@ class SocialAccountAdapter(DefaultSocialAccountAdapter): def populate_user(self, request: HttpRequest, social_login: SocialLogin, - data: Dict[str, Any]) -> User: + data: Dict[str, Any]) -> DjangoUser: """ Method used to populate a Django User with data. -- cgit v1.2.3 From c8207ab9af12e9b068ca3362788c0c164c48e64a Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Tue, 17 Dec 2019 00:05:59 +0100 Subject: Pad the discriminator with zeroes in profile. previously, lemon#0001 would display as just lemon#1. This commit fixes this. All credit goes to @gdude2002 --- pydis_site/utils/account.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pydis_site/utils') diff --git a/pydis_site/utils/account.py b/pydis_site/utils/account.py index e64919de..2d699c88 100644 --- a/pydis_site/utils/account.py +++ b/pydis_site/utils/account.py @@ -73,7 +73,7 @@ class SocialAccountAdapter(DefaultSocialAccountAdapter): """ if social_login.account.provider == "discord": discriminator = social_login.account.extra_data["discriminator"] - data["username"] = f"{data['username']}#{discriminator}" + data["username"] = f"{data['username']}#{discriminator:0>4}" data["name"] = data["username"] return super().populate_user(request, social_login, data) -- cgit v1.2.3 From a32c9f1a7c5e2a84627c87b049a43c281fa26b5b Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Tue, 2 Jun 2020 21:14:11 +0100 Subject: Alter account utilities to check list length instead of queryset length --- pydis_site/utils/account.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pydis_site/utils') diff --git a/pydis_site/utils/account.py b/pydis_site/utils/account.py index 2d699c88..b4e41198 100644 --- a/pydis_site/utils/account.py +++ b/pydis_site/utils/account.py @@ -54,7 +54,7 @@ class SocialAccountAdapter(DefaultSocialAccountAdapter): raise ImmediateHttpResponse(redirect(reverse("home"))) - if user.roles.count() <= 1: + if len(user.roles) <= 1: add_message(request, ERROR, ERROR_JOIN_DISCORD) raise ImmediateHttpResponse(redirect(reverse("home"))) -- cgit v1.2.3