Skip to content

Commit

Permalink
♻️ [#4246] Refactor digig-eherkenning-oidc-generics auth request flow
Browse files Browse the repository at this point in the history
Simplified the entire authentication request flow where the user
gets redirect to the relevant identity provider.

There is now a single 'view' implementation that takes a config
class/model to use which can be used to directly obtain the
redirect target instead of having to go through multiple
redirects on our own URLs.

The view takes care of input sanitation and managing the
authentication state.

This substantially cleans up the inheritance/mixin chains for
the OIDC flows and makes the code easier to follow.
  • Loading branch information
sergei-maertens committed May 6, 2024
1 parent b8901c4 commit 60f7404
Show file tree
Hide file tree
Showing 3 changed files with 159 additions and 8 deletions.
2 changes: 2 additions & 0 deletions src/digid_eherkenning_oidc_generics/exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
class OIDCProviderOutage(Exception):
pass
22 changes: 22 additions & 0 deletions src/digid_eherkenning_oidc_generics/utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
from typing import Any

from mozilla_django_oidc.utils import import_from_settings

from .models import OpenIDConnectBaseConfig


def get_setting_from_config(config: OpenIDConnectBaseConfig, attr: str, *args) -> Any:
"""
Look up a setting from the config record or fall back to Django settings.
TODO: port this into mozilla_django_oidc_db.
"""
attr_lowercase = attr.lower()
if hasattr(config, attr_lowercase):
# Workaround for OIDC_RP_IDP_SIGN_KEY being an empty string by default.
# mozilla-django-oidc explicitly checks if `OIDC_RP_IDP_SIGN_KEY` is not `None`
# https://github.com/mozilla/mozilla-django-oidc/blob/master/mozilla_django_oidc/auth.py#L189
if (value_from_config := getattr(config, attr_lowercase)) == "":
return None
return value_from_config
return import_from_settings(attr, *args)
143 changes: 135 additions & 8 deletions src/digid_eherkenning_oidc_generics/views.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,151 @@
import logging
import time
from typing import Generic, TypeVar, cast

from django.contrib import auth
from django.core.exceptions import SuspiciousOperation
from django.http import HttpResponseRedirect
from django.core.exceptions import DisallowedRedirect, SuspiciousOperation
from django.http import HttpRequest, HttpResponseRedirect
from django.utils.http import url_has_allowed_host_and_scheme

import requests
from mozilla_django_oidc.views import (
OIDCAuthenticationCallbackView as _OIDCAuthenticationCallbackView,
OIDCAuthenticationRequestView as _OIDCAuthenticationRequestView,
)

from .exceptions import OIDCProviderOutage
from .models import (
OpenIDConnectBaseConfig,
OpenIDConnectDigiDMachtigenConfig,
OpenIDConnectEHerkenningBewindvoeringConfig,
OpenIDConnectEHerkenningConfig,
OpenIDConnectPublicConfig,
)
from .utils import get_setting_from_config

logger = logging.getLogger(__name__)

T = TypeVar("T", bound=OpenIDConnectBaseConfig)


class OIDCInit(Generic[T], _OIDCAuthenticationRequestView):
"""
A 'view' to start an OIDC authentication flow.
This view class is parametrized with the config model/class to retrieve the
specific configuration, such as the identity provider endpoint to redirect the
user to.
This view is not necessarily meant to be exposed directly via a URL pattern, but
rather specific views are to be created from it, e.g.:
.. code-block:: python
>>> digid_init = OIDCInit.as_view(config_class=OpenIDConnectPublicConfig)
>>> redirect_response = digid_init(request)
# Redirect to some keycloak instance, for example.
These concrete views are intended to be wrapped by your own views so that you can
supply the ``return_url`` parameter:
.. code-block:: python
def my_digid_login(request):
return digid_init(request, return_url=request.GET["next"])
class OIDCAuthenticationRequestView(_OIDCAuthenticationRequestView):
def get_extra_params(self, request):
kc_idp_hint = self.get_settings("OIDC_KEYCLOAK_IDP_HINT", "")
if kc_idp_hint:
return {"kc_idp_hint": kc_idp_hint}
return {}
Compared to :class:`mozilla_django_oidc.views.OIDCAuthenticationRequestView`, some
extra actions are performed:
* Any Keycloak IdP hint is added, if configured
* The ``return_url`` is validated against unsafe redirects
* The availability of the identity provider endpoint is checked, if it's not
available, the :class:`digid_eherkenning_oidc_generics.exceptions.OIDCProviderOutage`
exception is raised. Note that your own code needs to handle this appropriately!
"""

config_class: type[OpenIDConnectBaseConfig] = OpenIDConnectBaseConfig
_config: T
"""
The config model/class to get the endpoints/credentials from.
Specify this as a kwarg in the ``as_view(config_class=...)`` class method.
"""

def get_settings(self, attr, *args): # type: ignore
if (config := getattr(self, "_config", None)) is None:
# django-solo and type checking is challenging, but a new release is on the
# way and should fix that :fingers_crossed:
config = cast(T, self.config_class.get_solo())
self._config = config
return get_setting_from_config(config, attr, *args)

def get(
self, request: HttpRequest, return_url: str = "", *args, **kwargs
) -> HttpResponseRedirect:
if not return_url:
raise ValueError("You must pass a return URL")

url_is_safe = url_has_allowed_host_and_scheme(
url=return_url,
allowed_hosts=request.get_host(),
require_https=request.is_secure(),
)
if not url_is_safe or not return_url:
raise DisallowedRedirect(f"Can't redirect to '{return_url}'")

self._check_idp_availability()

# 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 by the
# upstream library, so in the callback view we store this URL so that we know
# where to redirect with the error information.
self.request.session["of_redirect_next"] = return_url

response = super().get(request, *args, **kwargs)

# mozilla-django-oidc grabs this from request.GET and since that is not mutable,
# it's easiest to just override the session key with the correct value.
request.session["oidc_login_next"] = return_url

return response

def _check_idp_availability(self) -> None:
endpoint = self.OIDC_OP_AUTH_ENDPOINT
try:
# Verify that the identity provider endpoint can be reached. This is where
# the user ultimately gets redirected to.
response = requests.get(endpoint)
if response.status_code > 400:
response.raise_for_status()
except requests.RequestException as exc:
logger.info(
"OIDC provider endpoint '%s' could not be retrieved",
endpoint,
exc_info=exc,
)
raise OIDCProviderOutage() from exc

def get_extra_params(self, request: HttpRequest) -> dict[str, str]:
"""
Add a keycloak identity provider hint if configured.
"""
extra = super().get_extra_params(request)
options = self.config_class._meta
# Store which config class to use in the params so that the callback view can
# extract this again.
# TODO: verify this cannot be tampered with!
extra["config"] = f"{options.app_label}.{options.object_name}"
if kc_idp_hint := self.get_settings("OIDC_KEYCLOAK_IDP_HINT", ""):
extra["kc_idp_hint"] = kc_idp_hint
return extra


digid_init = OIDCInit.as_view(config_class=OpenIDConnectPublicConfig)
digid_machtigen_init = OIDCInit.as_view(config_class=OpenIDConnectDigiDMachtigenConfig)
eherkenning_init = OIDCInit.as_view(config_class=OpenIDConnectEHerkenningConfig)
eherkenning_bewindvoering_init = OIDCInit.as_view(
config_class=OpenIDConnectEHerkenningBewindvoeringConfig
)


class OIDCAuthenticationCallbackView(_OIDCAuthenticationCallbackView):
Expand Down

0 comments on commit 60f7404

Please sign in to comment.