Skip to content

Commit

Permalink
[#3656] Refactored digid eherkenning oidc concerning error messages
Browse files Browse the repository at this point in the history
Backport-of: #3687
  • Loading branch information
vaszig authored and sergei-maertens committed Dec 21, 2023
1 parent c2318b8 commit 84af07e
Show file tree
Hide file tree
Showing 4 changed files with 160 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from unittest.mock import patch

from django.test import TestCase, override_settings
from django.test import TestCase, override_settings, tag
from django.urls import reverse

import requests_mock
Expand Down Expand Up @@ -322,3 +322,35 @@ def test_redirect_with_keycloak_identity_provider_hint(self, *m):
f"http://testserver{reverse('digid_oidc:oidc_authentication_callback')}",
)
self.assertEqual(query_params["kc_idp_hint"], "oidc-digid")

@tag("gh-3656")
# This is an example of a specific provider. It may differs when a different provider is used.
# According to https://openid.net/specs/openid-connect-core-1_0.html#AuthError and
# https://www.rfc-editor.org/rfc/rfc6749.html#section-4.1.2.1 , this is the error we expect from OIDC
def test_redirect_to_form_login_cancelled(self):
form_path = reverse("core:form-detail", kwargs={"slug": self.form.slug})
form_url = f"http://testserver{form_path}"
redirect_form_url = furl(form_url).set({"_start": "1"})
redirect_url = furl(
reverse(
"authentication:return",
kwargs={"slug": self.form.slug, "plugin_id": "digid_oidc"},
)
).set({"next": redirect_form_url})

session = self.client.session
session["oidc_login_next"] = redirect_url.url
session.save()

response = self.client.get(
reverse("digid_oidc:callback"),
{"error": "access_denied", "error_description": "The user cancelled"},
)

self.assertEqual(response.status_code, status.HTTP_302_FOUND)

parsed = furl(response.url)
query_params = parsed.query.params

self.assertEqual(query_params["_digid-message"], "login-cancelled")
self.assertIsNone(query_params.get("of-auth-problem"))
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from unittest.mock import patch

from django.test import TestCase, override_settings
from django.test import TestCase, override_settings, tag
from django.urls import reverse

import requests_mock
Expand Down Expand Up @@ -318,3 +318,35 @@ def test_redirect_with_keycloak_identity_provider_hint(self, m_get_solo):
f"http://testserver{reverse('digid_machtigen_oidc:oidc_authentication_callback')}",
)
self.assertEqual(query_params["kc_idp_hint"], "oidc-digid-machtigen")

@tag("gh-3656")
# This is an example of a specific provider. It may differs when a different provider is used.
# According to https://openid.net/specs/openid-connect-core-1_0.html#AuthError and
# https://www.rfc-editor.org/rfc/rfc6749.html#section-4.1.2.1 , this is the error we expect from OIDC
def test_redirect_to_form_login_cancelled(self, m):
form_path = reverse("core:form-detail", kwargs={"slug": self.form.slug})
form_url = f"http://testserver{form_path}"
redirect_form_url = furl(form_url).set({"_start": "1"})
redirect_url = furl(
reverse(
"authentication:return",
kwargs={"slug": self.form.slug, "plugin_id": "digid_machtigen_oidc"},
)
).set({"next": redirect_form_url})

session = self.client.session
session["oidc_login_next"] = redirect_url.url
session.save()

response = self.client.get(
reverse("digid_machtigen_oidc:callback"),
{"error": "access_denied", "error_description": "The user cancelled"},
)

self.assertEqual(response.status_code, status.HTTP_302_FOUND)

parsed = furl(response.url)
query_params = parsed.query.params

self.assertEqual(query_params["_digid-message"], "login-cancelled")
self.assertIsNone(query_params.get("of-auth-problem"))
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from unittest.mock import patch

from django.test import TestCase, override_settings
from django.test import TestCase, override_settings, tag
from django.urls import reverse

import requests_mock
Expand Down Expand Up @@ -331,3 +331,35 @@ def test_redirect_with_keycloak_identity_provider_hint(self, *m):
f"http://testserver{reverse('eherkenning_oidc:oidc_authentication_callback')}",
)
self.assertEqual(query_params["kc_idp_hint"], "oidc-eHerkenning")

@tag("gh-3656")
# This is an example of a specific provider. It may differs when a different provider is used.
# According to https://openid.net/specs/openid-connect-core-1_0.html#AuthError and
# https://www.rfc-editor.org/rfc/rfc6749.html#section-4.1.2.1 , this is the error we expect from OIDC
def test_redirect_to_form_login_cancelled(self):
form_path = reverse("core:form-detail", kwargs={"slug": self.form.slug})
form_url = f"http://testserver{form_path}"
redirect_form_url = furl(form_url).set({"_start": "1"})
redirect_url = furl(
reverse(
"authentication:return",
kwargs={"slug": self.form.slug, "plugin_id": "eherkenning_oidc"},
)
).set({"next": redirect_form_url})

session = self.client.session
session["oidc_login_next"] = redirect_url.url
session.save()

response = self.client.get(
reverse("eherkenning_oidc:callback"),
{"error": "access_denied", "error_description": "The user cancelled"},
)

self.assertEqual(response.status_code, status.HTTP_302_FOUND)

parsed = furl(response.url)
query_params = parsed.query.params

self.assertEqual(query_params["_eherkenning-message"], "login-cancelled")
self.assertIsNone(query_params.get("of-auth-problem"))
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@
OIDCAuthenticationCallbackView as _OIDCAuthenticationCallbackView,
OIDCAuthenticationRequestView as _OIDCAuthenticationRequestView,
)
from openforms.authentication.contrib.digid.views import (
DIGID_MESSAGE_PARAMETER,
LOGIN_CANCELLED,
)
from openforms.authentication.contrib.eherkenning.views import MESSAGE_PARAMETER

from ...views import BACKEND_OUTAGE_RESPONSE_PARAMETER
from .backends import (
Expand Down Expand Up @@ -61,14 +66,31 @@ def get(self, request):


class OIDCAuthenticationCallbackView(_OIDCAuthenticationCallbackView):
def get_error_message_parameters(
self, parameter: str, problem_code: str
) -> tuple[str, str]:
"""
Return a tuple of the parameter type and the problem code.
"""
return (
parameter or BACKEND_OUTAGE_RESPONSE_PARAMETER,
problem_code or self.plugin_identifier,
)

@property
def failure_url(self):
"""
On failure, redirect to the form with an appropriate error message
"""
f = furl(self.success_url)
f = furl(f.args["next"])
f.args[BACKEND_OUTAGE_RESPONSE_PARAMETER] = self.plugin_identifier

parameter, problem_code = self.get_error_message_parameters(
self.request.GET.get("error", ""),
self.request.GET.get("error_description", ""),
)
f.args[parameter] = problem_code

return f.url


Expand All @@ -84,6 +106,14 @@ class DigiDOIDCAuthenticationCallbackView(
plugin_identifier = "digid_oidc"
auth_backend_class = OIDCAuthenticationDigiDBackend

def get_error_message_parameters(
self, error: str, error_description: str
) -> tuple[str, str]:
if error == "access_denied" and error_description == "The user cancelled":
return (DIGID_MESSAGE_PARAMETER, LOGIN_CANCELLED)

return (BACKEND_OUTAGE_RESPONSE_PARAMETER, self.plugin_identifier)


class eHerkenningOIDCAuthenticationRequestView(
SoloConfigEHerkenningMixin, OIDCAuthenticationRequestView
Expand All @@ -97,6 +127,17 @@ class eHerkenningOIDCAuthenticationCallbackView(
plugin_identifier = "eherkenning_oidc"
auth_backend_class = OIDCAuthenticationEHerkenningBackend

def get_error_message_parameters(
self, error: str, error_description: str
) -> tuple[str, str]:
if error == "access_denied" and error_description == "The user cancelled":
return (
MESSAGE_PARAMETER % {"plugin_id": self.plugin_identifier.split("_")[0]},
LOGIN_CANCELLED,
)

return (BACKEND_OUTAGE_RESPONSE_PARAMETER, self.plugin_identifier)


class DigiDMachtigenOIDCAuthenticationRequestView(
SoloConfigDigiDMachtigenMixin, OIDCAuthenticationRequestView
Expand All @@ -110,6 +151,14 @@ class DigiDMachtigenOIDCAuthenticationCallbackView(
plugin_identifier = "digid_machtigen_oidc"
auth_backend_class = OIDCAuthenticationDigiDMachtigenBackend

def get_error_message_parameters(
self, error: str, error_description: str
) -> tuple[str, str]:
if error == "access_denied" and error_description == "The user cancelled":
return (DIGID_MESSAGE_PARAMETER, LOGIN_CANCELLED)

return (BACKEND_OUTAGE_RESPONSE_PARAMETER, self.plugin_identifier)


class EHerkenningBewindvoeringOIDCAuthenticationRequestView(
SoloConfigEHerkenningBewindvoeringMixin, OIDCAuthenticationRequestView
Expand All @@ -122,3 +171,14 @@ class EHerkenningBewindvoeringOIDCAuthenticationCallbackView(
):
plugin_identifier = "eherkenning_bewindvoering_oidc"
auth_backend_class = OIDCAuthenticationEHerkenningBewindvoeringBackend

def get_error_message_parameters(
self, error: str, error_description: str
) -> tuple[str, str]:
if error == "access_denied" and error_description == "The user cancelled":
return (
MESSAGE_PARAMETER % {"plugin_id": self.plugin_identifier.split("_")[0]},
LOGIN_CANCELLED,
)

return (BACKEND_OUTAGE_RESPONSE_PARAMETER, self.plugin_identifier)

0 comments on commit 84af07e

Please sign in to comment.