Skip to content

Commit

Permalink
feat: return kc_action_status from Keycloak on authentication
Browse files Browse the repository at this point in the history
kc_action_status is used to indicate e.g. whether a requested
password change was successful or not. It will be returned
from tunnistamo along with code and state to the callback url
the same way keycloak does it. This should make a later
transtition to keycloak (by dropping tunnistamo from the middle)
not require extra work for any logic using kc_action_status.

Upon login the kc_action_status is saved to the session from
which it is then retrieved in TunnistamoAuthorizeEndpoint.

Refs: HP-2491
  • Loading branch information
charn committed Aug 7, 2024
1 parent 64c370a commit f26f601
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 1 deletion.
16 changes: 16 additions & 0 deletions tunnistamo/endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,22 @@ def is_tunnistamo_session_active(self):


class TunnistamoAuthorizeEndpoint(TunnistamoSessionEndpointMixin, AuthorizeEndpoint):

def __init__(self, request):
super().__init__(request)

if kc_value := self.request.session.get("kc_action_status"):
self.params["kc_action_status"] = kc_value

def create_response_uri(self):
"""Add the status of the Keycloak action the callback url."""
uri = super().create_response_uri()

if kc_value := self.params.get("kc_action_status"):
uri = add_params_to_url(uri, {"kc_action_status": kc_value})

return uri

def _get_tunnistamo_session(self):
tunnistamo_session_id = self.request.session.get("tunnistamo_session_id")
if not tunnistamo_session_id:
Expand Down
3 changes: 3 additions & 0 deletions tunnistamo/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,9 @@

# Add loa to Tunnistamo Session
'users.pipeline.add_loa_to_tunnistamo_session',

# Save status of the Keycloak action to session
'users.pipeline.save_kc_action_status_to_session',
)

ALLOW_DUPLICATE_EMAILS = env("ALLOW_DUPLICATE_EMAILS")
Expand Down
32 changes: 32 additions & 0 deletions tunnistamo/tests/test_tunnistamo_session_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,38 @@ def test_authorize_endpoint(user, response_type, ended):
assert 'code=' in response['Location']


@pytest.mark.django_db
def test_authorize_endpoint_redirect_url_contains_kc_action_status_parameter(user):
django_test_client = DjangoTestClient()
django_test_client.force_login(user)
oidc_client = create_oidc_clients_and_api()

authorize_url = reverse("authorize")

authorize_request_data = {
"client_id": oidc_client.client_id,
"redirect_uri": oidc_client.redirect_uris[0],
"scope": "openid profile",
"response_type": "code",
"response_mode": "form_post",
"nonce": get_random_string(12),
}

# Add the expected parameters to the session which would be added in the social
# auth pipeline.
session = django_test_client.session
session.update({"kc_action_status": "success"})
session.save()

response = django_test_client.get(
authorize_url, authorize_request_data, follow=False
)

assert response.status_code == 302, response["Location"]

assert "kc_action_status=success" in response["Location"]


@pytest.mark.django_db
@pytest.mark.parametrize('response_type', [
'code',
Expand Down
15 changes: 15 additions & 0 deletions users/pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -337,3 +337,18 @@ def associate_between_helsinki_on_prem_ad_and_azure_ad(backend, details, user=No
return {
'user': existing_social_auth.user,
}


def save_kc_action_status_to_session(backend, strategy, *args, **kwargs):
"""When a Keycloak action status is present save it to the session."""
if isinstance(backend, (HelsinkiTunnus, HelsinkiTunnistus)) and (
kc_action_status := strategy.request_data().get("kc_action_status")
):
logger.debug(
f"kc_action_status {kc_action_status} in pipeline. Backend: {backend.name}."
)
strategy.session_set("kc_action_status", kc_action_status)
else:
strategy.session_pop("kc_action_status")

return
33 changes: 32 additions & 1 deletion users/tests/test_pipeline.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
from urllib.parse import urlencode

import pytest
from django.contrib import auth
from django.contrib.sessions.middleware import SessionMiddleware
from django.urls import reverse
from django.utils.crypto import get_random_string
from social_core.exceptions import AuthFailed
from social_django.models import UserSocialAuth
from social_django.utils import load_backend, load_strategy

from tunnistamo.tests.conftest import create_rsa_key, reload_social_django_utils
from users.pipeline import association_by_keycloak_uuid, update_ad_groups
from users.pipeline import association_by_keycloak_uuid, save_kc_action_status_to_session, update_ad_groups
from users.tests.conftest import DummyFixedOidcBackend


Expand Down Expand Up @@ -103,3 +106,31 @@ def test_association_by_keycloak_uuid(login_backend, existing_backend, user, exp
assert pipeline_data == {"user": user}
else:
assert pipeline_data is None


@pytest.mark.parametrize(
"backend_name,expected",
(
("heltunnistussuomifi", "success"),
("helsinki_tunnus", "success"),
("helsinki_adfs", None),
),
ids=repr,
)
def test_save_kc_action_status_to_session(backend_name, expected, rf):
uri = "{}?{}".format(
reverse("social:complete", kwargs={"backend": backend_name}),
urlencode(
{
"kc_action_status": "success",
}
),
)
request = rf.get(uri)
SessionMiddleware(get_response=lambda response: response).process_request(request)
strategy = load_strategy(request)
backend = load_backend(strategy, backend_name, uri)

save_kc_action_status_to_session(backend, strategy)

assert strategy.session_get("kc_action_status") == expected

0 comments on commit f26f601

Please sign in to comment.