Skip to content

Commit

Permalink
[#3692] Fixed next url when authenticated user cancels login
Browse files Browse the repository at this point in the history
Backport-of: #3696
Co-authored-by: @SilviaAmAm
  • Loading branch information
vaszig authored and sergei-maertens committed Dec 28, 2023
1 parent 93f3651 commit c94654a
Show file tree
Hide file tree
Showing 6 changed files with 127 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from rest_framework import status

from digid_eherkenning_oidc_generics.models import OpenIDConnectPublicConfig
from openforms.accounts.tests.factories import StaffUserFactory
from openforms.authentication.views import BACKEND_OUTAGE_RESPONSE_PARAMETER
from openforms.forms.tests.factories import FormFactory

Expand Down Expand Up @@ -149,7 +150,7 @@ def test_redirect_to_digid_oidc_callback_error(self, *m):
).set({"next": redirect_form_url})

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

with patch(
Expand Down Expand Up @@ -323,11 +324,11 @@ def test_redirect_with_keycloak_identity_provider_hint(self, *m):
)
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.
@tag("gh-3656", "gh-3692")
# This is an example of a specific provider. It may differ 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):
def test_redirect_to_form_when_login_cancelled_by_anonymous_user(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"})
Expand All @@ -339,7 +340,39 @@ def test_redirect_to_form_login_cancelled(self):
).set({"next": redirect_form_url})

session = self.client.session
session["oidc_login_next"] = redirect_url.url
session["of_redirect_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"))

@tag("gh-3656", "gh-3692")
def test_redirect_to_form_when_login_cancelled_by_authenticated_user(self):
user = StaffUserFactory.create()
self.client.force_login(user=user)

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["of_redirect_next"] = redirect_url.url
session.save()

response = self.client.get(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
OpenIDConnectDigiDMachtigenConfig,
OpenIDConnectPublicConfig,
)
from openforms.accounts.tests.factories import StaffUserFactory
from openforms.authentication.views import BACKEND_OUTAGE_RESPONSE_PARAMETER
from openforms.forms.tests.factories import FormFactory

Expand Down Expand Up @@ -147,7 +148,7 @@ def test_redirect_to_digid_machtigen_oidc_callback_error(self, m_get_solo):
).set({"next": redirect_form_url})

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

with patch(
Expand Down Expand Up @@ -319,11 +320,11 @@ def test_redirect_with_keycloak_identity_provider_hint(self, m_get_solo):
)
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.
@tag("gh-3656", "gh-3692")
# This is an example of a specific provider. It may differ 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):
def test_redirect_to_form_when_login_cancelled_by_anonymous_user(self, m_get_solo):
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"})
Expand All @@ -335,7 +336,41 @@ def test_redirect_to_form_login_cancelled(self, m):
).set({"next": redirect_form_url})

session = self.client.session
session["oidc_login_next"] = redirect_url.url
session["of_redirect_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"))

@tag("gh-3656", "gh-3692")
def test_redirect_to_form_when_login_cancelled_by_authenticated_user(
self, m_get_solo
):
user = StaffUserFactory.create()
self.client.force_login(user=user)

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["of_redirect_next"] = redirect_url.url
session.save()

response = self.client.get(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from rest_framework import status

from digid_eherkenning_oidc_generics.models import OpenIDConnectEHerkenningConfig
from openforms.accounts.tests.factories import StaffUserFactory
from openforms.authentication.views import BACKEND_OUTAGE_RESPONSE_PARAMETER
from openforms.forms.tests.factories import FormFactory

Expand Down Expand Up @@ -151,7 +152,7 @@ def test_redirect_to_eherkenning_oidc_callback_error(self, *m):
).set({"next": redirect_form_url})

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

with patch(
Expand Down Expand Up @@ -332,11 +333,11 @@ def test_redirect_with_keycloak_identity_provider_hint(self, *m):
)
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.
@tag("gh-3656", "gh-3692")
# This is an example of a specific provider. It may differ 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):
def test_redirect_to_form_when_login_cancelled_by_anonymous_user(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"})
Expand All @@ -348,7 +349,7 @@ def test_redirect_to_form_login_cancelled(self):
).set({"next": redirect_form_url})

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

response = self.client.get(
Expand All @@ -362,4 +363,36 @@ def test_redirect_to_form_login_cancelled(self):
query_params = parsed.query.params

self.assertEqual(query_params["_eherkenning-message"], "login-cancelled")
self.assertIsNone(query_params.get("_digid-message"))

@tag("gh-3656", "gh-3692")
def test_redirect_to_form_when_login_cancelled_by_authenticated_user(self):
user = StaffUserFactory.create()
self.client.force_login(user=user)

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["of_redirect_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
Expand Up @@ -170,7 +170,7 @@ def test_redirect_to_eherkenning_bewindvoering_oidc_callback_error(
).set({"next": redirect_form_url})

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

with patch(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ def get(self, request):
if not next_url:
raise DisallowedRedirect

# We add our own key to keep track of the redirect URL. In the case of authentication failure (or canceled logins),
# the session is cleared, so in OIDCAuthenticationCallbackView we store this URL so that we know where to.
self.request.session["of_redirect_next"] = next_url

try:
# Verify that the identity provider endpoint can be reached
response = requests.get(self.OIDC_OP_AUTH_ENDPOINT)
Expand All @@ -66,6 +70,10 @@ def get(self, request):


class OIDCAuthenticationCallbackView(_OIDCAuthenticationCallbackView):
def get(self, request):
self._redirect_next = request.session.get("of_redirect_next")
return super().get(request)

def get_error_message_parameters(
self, parameter: str, problem_code: str
) -> tuple[str, str]:
Expand All @@ -82,7 +90,7 @@ def failure_url(self):
"""
On failure, redirect to the form with an appropriate error message
"""
f = furl(self.success_url)
f = furl(self._redirect_next or self.get_settings("LOGIN_REDIRECT_URL", "/"))
f = furl(f.args["next"])

parameter, problem_code = self.get_error_message_parameters(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def test_redirect_to_org_oidc_callback_error(self, *m):
).set({"next": redirect_form_url})

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

with patch(
Expand Down

0 comments on commit c94654a

Please sign in to comment.