Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Fix SessionRefresh flow for new architecture #112

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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