From 68d9471f9e06f9d669aee38889f38d89a1193aa6 Mon Sep 17 00:00:00 2001 From: Johan Lundberg Date: Mon, 7 Oct 2024 14:13:36 +0200 Subject: [PATCH 1/2] add authn_options to final response from the IdP to always get an authenticated data set --- src/eduid/webapp/idp/helpers.py | 4 +++- src/eduid/webapp/idp/views/next.py | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/eduid/webapp/idp/helpers.py b/src/eduid/webapp/idp/helpers.py index 44af3e0ae..2da6d407b 100644 --- a/src/eduid/webapp/idp/helpers.py +++ b/src/eduid/webapp/idp/helpers.py @@ -1,4 +1,5 @@ from enum import Enum, unique +from typing import Any from saml2 import BINDING_HTTP_POST @@ -67,7 +68,7 @@ def lookup_user(username: str, managed_account_allowed: bool = False) -> IdPUser return current_app.userdb.lookup_user(username) -def create_saml_sp_response(saml_params: SAMLResponseParams) -> FluxData: +def create_saml_sp_response(saml_params: SAMLResponseParams, authn_options: dict[str, Any]) -> FluxData: """ Create a response to frontend that should be posted to the SP """ @@ -81,5 +82,6 @@ def create_saml_sp_response(saml_params: SAMLResponseParams) -> FluxData: "target": saml_params.url, "parameters": saml_params.post_params, "missing_attributes": saml_params.missing_attributes, + "authn_options": authn_options, }, ) diff --git a/src/eduid/webapp/idp/views/next.py b/src/eduid/webapp/idp/views/next.py index c05928283..e08169ac7 100644 --- a/src/eduid/webapp/idp/views/next.py +++ b/src/eduid/webapp/idp/views/next.py @@ -186,7 +186,8 @@ def next_view(ticket: LoginContext, sso_session: SSOSession | None) -> FluxData: if isinstance(ticket, LoginContextSAML): saml_params = sso.get_response_params(_next.authn_info, ticket, user) - return create_saml_sp_response(saml_params=saml_params) + authn_options = _get_authn_options(ticket, sso_session, required_user.eppn) + return create_saml_sp_response(saml_params=saml_params, authn_options=authn_options) elif isinstance(ticket, LoginContextOtherDevice): if not ticket.is_other_device_2: # We shouldn't be able to get here, but this clearly shows where this code runs From c50359b5370ec28191edab33eb75a1d853d7f139 Mon Sep 17 00:00:00 2001 From: Johan Lundberg Date: Mon, 7 Oct 2024 14:20:22 +0200 Subject: [PATCH 2/2] all saml_sp_response needs authn_options use parameter names to clarify None usage --- src/eduid/webapp/idp/views/next.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/eduid/webapp/idp/views/next.py b/src/eduid/webapp/idp/views/next.py index e08169ac7..c5f55474e 100644 --- a/src/eduid/webapp/idp/views/next.py +++ b/src/eduid/webapp/idp/views/next.py @@ -50,7 +50,8 @@ def next_view(ticket: LoginContext, sso_session: SSOSession | None) -> FluxData: if _next.message == IdPMsg.aborted: if isinstance(ticket, LoginContextSAML): saml_params = cancel_saml_request(ticket, current_app.conf) - return create_saml_sp_response(saml_params=saml_params) + authn_options = _get_authn_options(ticket=ticket, sso_session=sso_session, eppn=None) + return create_saml_sp_response(saml_params=saml_params, authn_options=authn_options) elif isinstance(ticket, LoginContextOtherDevice): state = ticket.other_device_req if state.state in [OtherDeviceState.NEW, OtherDeviceState.IN_PROGRESS, OtherDeviceState.AUTHENTICATED]: @@ -78,7 +79,8 @@ def next_view(ticket: LoginContext, sso_session: SSOSession | None) -> FluxData: if _next.message == IdPMsg.assurance_failure: if isinstance(ticket, LoginContextSAML): saml_params = authn_context_class_not_supported(ticket, current_app.conf) - return create_saml_sp_response(saml_params=saml_params) + authn_options = _get_authn_options(ticket=ticket, sso_session=sso_session, eppn=None) + return create_saml_sp_response(saml_params=saml_params, authn_options=authn_options) current_app.logger.error(f"Don't know how to send error response for request {ticket}") return error_response(message=IdPMsg.general_failure) @@ -100,7 +102,7 @@ def next_view(ticket: LoginContext, sso_session: SSOSession | None) -> FluxData: _payload = { "action": IdPAction.OTHER_DEVICE.value, "target": url_for("other_device.use_other_1", _external=True), - "authn_options": _get_authn_options(ticket, sso_session, required_user.eppn), + "authn_options": _get_authn_options(ticket=ticket, sso_session=sso_session, eppn=required_user.eppn), "service_info": _get_service_info(ticket), } @@ -113,7 +115,7 @@ def next_view(ticket: LoginContext, sso_session: SSOSession | None) -> FluxData: _payload = { "action": IdPAction.PWAUTH.value, "target": url_for("pw_auth.pw_auth", _external=True), - "authn_options": _get_authn_options(ticket, sso_session, required_user.eppn), + "authn_options": _get_authn_options(ticket=ticket, sso_session=sso_session, eppn=required_user.eppn), "service_info": _get_service_info(ticket), } @@ -128,7 +130,7 @@ def next_view(ticket: LoginContext, sso_session: SSOSession | None) -> FluxData: payload={ "action": IdPAction.MFA.value, "target": url_for("mfa_auth.mfa_auth", _external=True), - "authn_options": _get_authn_options(ticket, sso_session, required_user.eppn), + "authn_options": _get_authn_options(ticket=ticket, sso_session=sso_session, eppn=required_user.eppn), "service_info": _get_service_info(ticket), }, ) @@ -139,7 +141,7 @@ def next_view(ticket: LoginContext, sso_session: SSOSession | None) -> FluxData: payload={ "action": IdPAction.TOU.value, "target": url_for("tou.tou", _external=True), - "authn_options": _get_authn_options(ticket, sso_session, required_user.eppn), + "authn_options": _get_authn_options(ticket=ticket, sso_session=sso_session, eppn=required_user.eppn), }, ) @@ -186,7 +188,7 @@ def next_view(ticket: LoginContext, sso_session: SSOSession | None) -> FluxData: if isinstance(ticket, LoginContextSAML): saml_params = sso.get_response_params(_next.authn_info, ticket, user) - authn_options = _get_authn_options(ticket, sso_session, required_user.eppn) + authn_options = _get_authn_options(ticket=ticket, sso_session=sso_session, eppn=required_user.eppn) return create_saml_sp_response(saml_params=saml_params, authn_options=authn_options) elif isinstance(ticket, LoginContextOtherDevice): if not ticket.is_other_device_2: