Skip to content

Commit

Permalink
Merge pull request #112 from maykinmedia/issue/fix-infinite-redirect-…
Browse files Browse the repository at this point in the history
…on-sessionrefresh

🐛 Fix SessionRefresh flow for new architecture
  • Loading branch information
sergei-maertens authored Jul 2, 2024
2 parents 428e8c0 + 9cdd3c6 commit 08552a5
Show file tree
Hide file tree
Showing 17 changed files with 1,322 additions and 20 deletions.
1 change: 0 additions & 1 deletion docs/quickstart.rst
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ and the following settings from ``mozilla-django-oidc`` for OpenID Connect can b
- ``OIDC_RP_IDP_SIGN_KEY``
- ``OIDC_USE_NONCE``
- ``OIDC_STATE_SIZE``
- ``OIDC_EXEMPT_URLS``

In case no value is provided for one of these variables, the default from ``mozilla-django-oidc``
will be used (if there is one). A detailed description of all settings can be found in the `mozilla-django-oidc settings documentation`_.
Expand Down
1 change: 0 additions & 1 deletion mozilla_django_oidc_db/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ class OpenIDConnectConfigAdmin(SingletonModelAdmin):
"oidc_use_nonce",
"oidc_nonce_size",
"oidc_state_size",
"oidc_exempt_urls",
"userinfo_claims_source",
),
"classes": [
Expand Down
1 change: 1 addition & 0 deletions mozilla_django_oidc_db/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ class MozillaDjangoOidcDbConfig(AppConfig):

def ready(self) -> None:
from . import checks # noqa
from . import signals # noqa
10 changes: 9 additions & 1 deletion mozilla_django_oidc_db/backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,15 @@ def authenticate( # type: ignore
return None

# Allright, now try to actually authenticate the user.
return super().authenticate(request, nonce=nonce, code_verifier=code_verifier)
user = super().authenticate(request, nonce=nonce, code_verifier=code_verifier)

# Store the config class name on the user, so that we can store this in the user's
# session after they have been successfully authenticated (by listening to the `user_logged_in` signal)
if user:
options = self.config_class._meta
user._oidcdb_config_class = f"{options.app_label}.{options.object_name}" # type: ignore

return user

def _extract_username(
self, claims: JSONObject, *, raise_on_empty: bool = False
Expand Down
19 changes: 13 additions & 6 deletions mozilla_django_oidc_db/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from mozilla_django_oidc.utils import import_from_settings
from typing_extensions import Self, TypedDict, Unpack

from .constants import CONFIG_CLASS_SESSION_KEY
from .models import OpenIDConnectConfigBase


Expand Down Expand Up @@ -73,7 +74,7 @@ class MyBackend(BaseBackend):
default: T

def __init__(self, **kwargs: Unpack[DynamicSettingKwargs[T]]):
if default := kwargs.get("default"):
if (default := kwargs.get("default")) is not None:
self.default = default
self._default_set = True

Expand Down Expand Up @@ -109,19 +110,25 @@ def store_config(request: HttpRequest) -> None:
mozilla-django-oidc's callback view deletes the state key after it has validated it,
so our :func:`lookup_config` cannot extract it from the session anymore.
"""
# Attempt to retrieve the config_class from the session, this only works for users
# that are actually logged in as Django users
# The config_class key is added to the state in the OIDCInit.get method.
# TODO: verify that the state query param is present for error flows! Need to check
# the OAUTH2 spec for this, but according to ChatGeePeeTee if the request contains
# it, the callback must have it too.
config_class = ""
state_key = request.GET.get("state")
if not state_key or state_key not in (
states := request.session.get("oidc_states", [])
if state_key and state_key in (states := request.session.get("oidc_states", [])):
state = states[state_key]
config_class = state.get("config_class", "")

if not config_class and (
_config := request.session.get(CONFIG_CLASS_SESSION_KEY, "")
):
raise BadRequest("Could not look up the referenced config.")
config_class = _config

state = states[state_key]
try:
config = apps.get_model(state.get("config_class", ""))
config = apps.get_model(config_class)
except (LookupError, ValueError) as exc:
raise BadRequest("Could not look up the referenced config.") from exc

Expand Down
2 changes: 2 additions & 0 deletions mozilla_django_oidc_db/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,5 @@
}

OPEN_ID_CONFIG_PATH = ".well-known/openid-configuration"

CONFIG_CLASS_SESSION_KEY = "_OIDCDB_CONFIG_CLASS"
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Generated by Django 4.2.11 on 2024-07-01 15:15

from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
(
"mozilla_django_oidc_db",
"0003_openidconnectconfig_oidc_keycloak_idp_hint_and_more",
),
]

operations = [
migrations.RemoveField(
model_name="openidconnectconfig",
name="oidc_exempt_urls",
),
]
11 changes: 0 additions & 11 deletions mozilla_django_oidc_db/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,17 +160,6 @@ class OpenIDConnectConfigBase(SingletonModel):
),
default=32,
)
oidc_exempt_urls = ArrayField(
verbose_name=_("URLs exempt from session renewal"),
base_field=models.CharField(_("Exempt URL"), max_length=1000),
default=list,
blank=True,
help_text=_(
"This is a list of absolute url paths, regular expressions for url paths, "
"or Django view names. This plus the mozilla-django-oidc urls are exempted "
"from the session renewal by the SessionRefresh middleware."
),
)

# Keycloak specific config
oidc_keycloak_idp_hint = models.CharField(
Expand Down
14 changes: 14 additions & 0 deletions mozilla_django_oidc_db/signals.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
from django.contrib.auth.signals import user_logged_in
from django.dispatch import receiver

from .constants import CONFIG_CLASS_SESSION_KEY


@receiver([user_logged_in], dispatch_uid="oidcdb.set_config_class")
def set_oidcdb_config_class_on_session(sender, user, request, **kwargs):
"""
Record the OIDC config class on the session, this is needed so the callback view
can retrieve the config in case of a SessionRefresh flow
"""
if hasattr(user, "_oidcdb_config_class"):
request.session[CONFIG_CLASS_SESSION_KEY] = user._oidcdb_config_class
17 changes: 17 additions & 0 deletions testapp/migrations/0003_remove_emptyconfig_oidc_exempt_urls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Generated by Django 5.0.2 on 2024-07-02 07:50

from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
("testapp", "0002_emptyconfig_oidc_keycloak_idp_hint_and_more"),
]

operations = [
migrations.RemoveField(
model_name="emptyconfig",
name="oidc_exempt_urls",
),
]
3 changes: 3 additions & 0 deletions testapp/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@
OIDCAuthenticationRequestView,
)

from .views import custom_callback_view_init

urlpatterns = [
path("admin/login/failure/", AdminLoginFailure.as_view(), name="admin-oidc-error"),
path("admin/", admin.site.urls),
path("login", OIDCAuthenticationRequestView.as_view(), name="login"),
path("oidc/", include("mozilla_django_oidc.urls")),
path("custom-init-login/", custom_callback_view_init, name="custom-init-login"),
] + staticfiles_urlpatterns()
Loading

0 comments on commit 08552a5

Please sign in to comment.