Skip to content

Commit

Permalink
inclusion_connect: Improve security on next_url
Browse files Browse the repository at this point in the history
  • Loading branch information
tonial committed Sep 30, 2024
1 parent 3828836 commit 73cc043
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 1 deletion.
5 changes: 4 additions & 1 deletion itou/openid_connect/inclusion_connect/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@

import httpx
from allauth.account.adapter import get_adapter
from django.conf import settings
from django.contrib import messages
from django.contrib.auth import login
from django.http import HttpResponseRedirect
from django.urls import reverse
from django.utils import crypto
from django.utils.html import format_html
from django.utils.http import urlencode
from django.utils.http import url_has_allowed_host_and_scheme, urlencode

from itou.prescribers.models import PrescriberOrganization
from itou.users.enums import KIND_EMPLOYER, KIND_PRESCRIBER, IdentityProvider, UserKind
Expand Down Expand Up @@ -117,6 +118,8 @@ def inclusion_connect_authorize(request):
user_kind = request.GET.get("user_kind")
previous_url = request.GET.get("previous_url", reverse("search:employers_home"))
next_url = request.GET.get("next_url")
if next_url and not url_has_allowed_host_and_scheme(next_url, settings.ALLOWED_HOSTS, request.is_secure()):
return _redirect_to_login_page_on_error(error_msg="Forbidden external url")
register = request.GET.get("register")
if not user_kind:
return _redirect_to_login_page_on_error(error_msg="User kind missing.")
Expand Down
7 changes: 7 additions & 0 deletions tests/openid_connect/inclusion_connect/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,13 @@ def test_authorize_check_user_kind(self):
response = self.client.get(url)
self.assertRedirects(response, reverse("search:employers_home"))

def test_next_url(self):
url = f"{reverse('inclusion_connect:authorize')}?{urlencode({'next_url': 'https://external.url.com'})}"
with self.assertLogs() as cm:
response = self.client.get(url, follow=False)
self.assertRedirects(response, reverse("search:employers_home"))
assert cm.records[0].message == "Forbidden external url"


class InclusionConnectCallbackViewTest(MessagesTestMixin, InclusionConnectBaseTestCase):
@respx.mock
Expand Down

0 comments on commit 73cc043

Please sign in to comment.