Skip to content

Commit

Permalink
♻️ Refactor out need for oidc error mapping
Browse files Browse the repository at this point in the history
Discussed with Steven and Alex in the office, ultimately the hardcoded
approach is favourable so we can drop the error_mapping config field
entirely.

Some testing/tweaking may be needed to see if different brokers provide
the same error_description values, but adding the possible combinations
for all brokers is not even outrageous - there's only a handful of
such brokers in NL.
  • Loading branch information
sergei-maertens committed Jul 8, 2024
1 parent 6d33551 commit f078ed1
Showing 1 changed file with 23 additions and 11 deletions.
34 changes: 23 additions & 11 deletions src/digid_eherkenning_oidc_generics/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
_OIDC_ERROR_SESSION_KEY,
OIDCAuthenticationCallbackView as BaseCallbackView,
OIDCInit,
get_exception_message,
)
from solo.models import SingletonModel

Expand Down Expand Up @@ -60,6 +59,7 @@ def get(self, request):
class OIDCCallbackView(BaseCallbackView):
failure_url = reverse_lazy("oidc-error")
generic_error_msg = ""
error_message_mapping: dict[tuple[str, str], str]

def get(self, request):
try:
Expand All @@ -70,26 +70,28 @@ def get(self, request):
"Something went wrong while attempting to authenticate via OIDC",
exc_info=exc,
)
exc_message = get_exception_message(exc)
request.session[_OIDC_ERROR_SESSION_KEY] = exc_message
request.session[_OIDC_ERROR_SESSION_KEY] = str(self.generic_error_msg)
response = self.login_failure()
else:
# Upstream library doesn't do any error handling by default.
if _OIDC_ERROR_SESSION_KEY in request.session:
del request.session[_OIDC_ERROR_SESSION_KEY]

# XXX: move this to a separate model
error_message_mapping = self.get_settings("ERROR_MESSAGE_MAPPING")

error = request.GET.get("error_description")
error_label = error_message_mapping.get(error, str(self.generic_error_msg))
if error and error_label:
request.session[_OIDC_ERROR_SESSION_KEY] = error_label
elif _OIDC_ERROR_SESSION_KEY in request.session and error_label:
if error_label := self._map_error(request):
request.session[_OIDC_ERROR_SESSION_KEY] = error_label

return response

def _map_error(self, request) -> str:
if not (error := request.GET.get("error")):
return ""

error_description = request.GET.get("error_description", "")

# Look up the combination of error code and description in the mapping.
mapped_error = self.error_message_mapping.get((error, error_description))
return mapped_error or str(self.generic_error_msg)


class BaseOIDCLogoutView(View):
config_class: type[SingletonModel]
Expand Down Expand Up @@ -117,6 +119,11 @@ def get(self, request):
# FIXME: mozilla-django-oidc-db has a proper construct for this now.
class DigiDOIDCAuthenticationCallbackView(OIDCCallbackView):
generic_error_msg = GENERIC_DIGID_ERROR_MSG
error_message_mapping = {
("access_denied", "The user cancelled"): (
"Je hebt het inloggen met DigiD geannuleerd."
)
}


class DigiDOIDCLogoutView(BaseOIDCLogoutView):
Expand All @@ -126,6 +133,11 @@ class DigiDOIDCLogoutView(BaseOIDCLogoutView):
# FIXME: mozilla-django-oidc-db has a proper construct for this now.
class eHerkenningOIDCAuthenticationCallbackView(OIDCCallbackView):
generic_error_msg = GENERIC_EHERKENNING_ERROR_MSG
error_message_mapping = {
("access_denied", "The user cancelled"): (
"Je hebt het inloggen met eHerkenning geannuleerd."
)
}


class eHerkenningOIDCLogoutView(BaseOIDCLogoutView):
Expand Down

0 comments on commit f078ed1

Please sign in to comment.