Skip to content

Commit

Permalink
prevent csrf attack against django integration by saving state
Browse files Browse the repository at this point in the history
Previously, the integration was vulnerable against the kind of CSRF
attack described in RFC 6749 section 10.12. (See [1]). In this, a
user-agent could be forcibly logged into a new session by redirecting
it to a login-callback url that could be supplied by an attacker.

This has now been fixed by storing an authentication state in the
django session associated with a user-agent.

[1]: https://www.rfc-editor.org/rfc/rfc6749#section-10.12
  • Loading branch information
lilioid committed Dec 26, 2024
1 parent f4571cb commit d7b76db
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/simple_openid_connect/integrations/django/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class SettingsModel(BaseModel):
OPENID_USER_MAPPER: str = (
"simple_openid_connect.integrations.django.user_mapping.UserMapper"
)
"An string specifying a class that inherits from :class:`simple_openid_connect.integrations.django.user_mapping.UserMapper`."
"A string specifying a class that inherits from :class:`simple_openid_connect.integrations.django.user_mapping.UserMapper`."


class OpenidAppConfig(AppConfig):
Expand Down
23 changes: 23 additions & 0 deletions src/simple_openid_connect/integrations/django/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,18 @@
logger = logging.getLogger(__name__)


class InvalidAuthStateError(Exception):
"""
Exception that is thrown when the LoginCallbackView is served and the user-agent has no authentication procedure currently in progress
"""

def __init__(self) -> None:
super().__init__(
self,
"User-Agent has no authentication procedures in progress so the login-callback will not be processed",
)


class InitLoginView(View):
"""
The view which handles initiating a login.
Expand All @@ -42,6 +54,11 @@ def get(self, request: HttpRequest) -> HttpResponse:
logout(request)
if "next" in request.GET.keys():
request.session["login_redirect_url"] = request.GET["next"]

# save the login state into the session to prevent CSRF attacks (openid state parameter could be used instead)
# See https://www.rfc-editor.org/rfc/rfc6749#section-10.12
request.session["openid_auth_in_progress"] = True

client = OpenidAppConfig.get_instance().get_client(request)
redirect = client.authorization_code_flow.start_authentication()
return HttpResponseRedirect(redirect)
Expand All @@ -61,6 +78,12 @@ class LoginCallbackView(View):
def get(self, request: HttpRequest) -> HttpResponse:
client = OpenidAppConfig.get_instance().get_client(request)

# prevent CSRF attacks by verifying that the user agent is curently in the process of authenticating
if request.session.get("openid_auth_in_progress", False) is not True:
raise InvalidAuthStateError()
else:
del request.session["openid_auth_in_progress"]

# exchange the passed code for tokens
token_response = client.authorization_code_flow.handle_authentication_result(
current_url=request.get_full_path(),
Expand Down
20 changes: 20 additions & 0 deletions tests/django_test_project/django_test_project/tests/test_login.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from responses import matchers

from simple_openid_connect.integrations.django.apps import OpenidAppConfig
from simple_openid_connect.integrations.django.views import InvalidAuthStateError


@pytest.mark.django_db
Expand Down Expand Up @@ -159,3 +160,22 @@ def test_directly_accessing_protected_resource(
assert response.status_code == 200
assert response.wsgi_request.path == resolve_url("test-protected-view")
assert response.content == b"hello user user1"


@pytest.mark.django_db
def test_unsolicited_callback_csrf(
dyn_client, dummy_provider_config, dummy_provider_settings, response_mock, jwks
):
# arrange
settings = OpenidAppConfig.get_instance().safe_settings

# act
# throws ConnectionError when the implementation tries to redeem the code
with pytest.raises(InvalidAuthStateError):
response = dyn_client.get(
"https://app.example.com"
+ resolve_url(settings.OPENID_REDIRECT_URI)
+ "?code=code.foobar123"
)

# assert

0 comments on commit d7b76db

Please sign in to comment.