Fix: dont allow allauth redirects to any host (#5783)

---------

Co-authored-by: Trenton H <797416+stumpylog@users.noreply.github.com>
This commit is contained in:
shamoon 2024-02-15 16:37:34 -08:00 committed by GitHub
parent 8d664fad56
commit f1049cf889
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 47 additions and 1 deletions

View File

@ -1,4 +1,5 @@
from allauth.account.adapter import DefaultAccountAdapter
from allauth.core import context
from allauth.socialaccount.adapter import DefaultSocialAccountAdapter
from django.conf import settings
from django.urls import reverse
@ -10,6 +11,21 @@ class CustomAccountAdapter(DefaultAccountAdapter):
# Override with setting, otherwise default to super.
return getattr(settings, "ACCOUNT_ALLOW_SIGNUPS", allow_signups)
def is_safe_url(self, url):
# see https://github.com/paperless-ngx/paperless-ngx/issues/5780
from django.utils.http import url_has_allowed_host_and_scheme
# get_host already validates the given host, so no need to check it again
allowed_hosts = {context.request.get_host()} | set(settings.ALLOWED_HOSTS)
if "*" in allowed_hosts:
# dont allow wildcard to allow urls from any host
allowed_hosts.remove("*")
allowed_hosts.add(context.request.get_host())
return url_has_allowed_host_and_scheme(url, allowed_hosts=allowed_hosts)
return url_has_allowed_host_and_scheme(url, allowed_hosts=allowed_hosts)
class CustomSocialAccountAdapter(DefaultSocialAccountAdapter):
def is_open_for_signup(self, request, sociallogin):

View File

@ -1,7 +1,12 @@
from unittest import mock
from allauth.account.adapter import get_adapter
from allauth.core import context
from allauth.socialaccount.adapter import get_adapter as get_social_adapter
from django.conf import settings
from django.http import HttpRequest
from django.test import TestCase
from django.test import override_settings
from django.urls import reverse
@ -17,6 +22,31 @@ class TestCustomAccountAdapter(TestCase):
settings.ACCOUNT_ALLOW_SIGNUPS = False
self.assertFalse(adapter.is_open_for_signup(None))
def test_is_safe_url(self):
request = HttpRequest()
request.get_host = mock.Mock(return_value="example.com")
with context.request_context(request):
adapter = get_adapter()
with override_settings(ALLOWED_HOSTS=["*"]):
# True because request host is same
url = "https://example.com"
self.assertTrue(adapter.is_safe_url(url))
url = "https://evil.com"
# False despite wildcard because request host is different
self.assertFalse(adapter.is_safe_url(url))
settings.ALLOWED_HOSTS = ["example.com"]
url = "https://example.com"
# True because request host is same
self.assertTrue(adapter.is_safe_url(url))
settings.ALLOWED_HOSTS = ["*", "example.com"]
url = "//evil.com"
# False because request host is not in allowed hosts
self.assertFalse(adapter.is_safe_url(url))
class TestCustomSocialAccountAdapter(TestCase):
def test_is_open_for_signup(self):

View File

@ -193,6 +193,7 @@ urlpatterns = [
RedirectView.as_view(
url=settings.STATIC_URL + "frontend/en-US/assets/%(path)s",
),
# TODO: with localization, this is even worse! :/
),
# App logo
re_path(
@ -200,7 +201,6 @@ urlpatterns = [
serve,
kwargs={"document_root": os.path.join(settings.MEDIA_ROOT, "logo")},
),
# TODO: with localization, this is even worse! :/
# login, logout
path("accounts/", include("allauth.urls")),
# Root of the Frontend