Skip to content

Commit d986464

Browse files
committed
Merge branch 'ik-state-handling'
2 parents 44aa4ac + 189871d commit d986464

File tree

16 files changed

+308
-80
lines changed

16 files changed

+308
-80
lines changed

src/satosa/backends/apple.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,6 @@ def response_endpoint(self, context, *args):
245245
msg = "UserInfo: {}".format(all_user_claims)
246246
logline = lu.LOG_FMT.format(id=lu.get_session_id(context.state), message=msg)
247247
logger.debug(logline)
248-
del context.state[self.name]
249248
internal_resp = self._translate_response(
250249
all_user_claims, self.client.authorization_endpoint
251250
)

src/satosa/backends/github.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@ def _authn_response(self, context):
9999
internal_response.attributes = self.converter.to_internal(
100100
self.external_type, user_info)
101101
internal_response.subject_id = str(user_info[self.user_id_attr])
102-
del context.state[self.name]
103102
return self.auth_callback_func(context, internal_response)
104103

105104
def user_information(self, access_token):

src/satosa/backends/linkedin.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,6 @@ def _authn_response(self, context):
110110
self.external_type, user_info)
111111

112112
internal_response.subject_id = user_info[self.user_id_attr]
113-
del context.state[self.name]
114113
return self.auth_callback_func(context, internal_response)
115114

116115
def user_information(self, access_token, api):

src/satosa/backends/oauth.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,6 @@ def _authn_response(self, context):
145145
internal_response = InternalData(auth_info=self.auth_info(context.request))
146146
internal_response.attributes = self.converter.to_internal(self.external_type, user_info)
147147
internal_response.subject_id = user_info[self.user_id_attr]
148-
del context.state[self.name]
149148
return self.auth_callback_func(context, internal_response)
150149

151150
def auth_info(self, request):

src/satosa/backends/openid_connect.py

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
from satosa.internal import InternalData
2020
from .base import BackendModule
2121
from .oauth import get_metadata_desc_for_oauth_backend
22-
from ..exception import SATOSAAuthenticationError, SATOSAError
22+
from ..exception import SATOSAAuthenticationError
23+
from ..exception import SATOSAError
24+
from ..exception import SATOSAMissingStateError
2325
from ..response import Redirect
2426

2527

@@ -58,11 +60,24 @@ def __init__(self, auth_callback_func, internal_attributes, config, base_url, na
5860
self.config = config
5961
cfg_verify_ssl = config["client"].get("verify_ssl", True)
6062
oidc_settings = PyoidcSettings(verify_ssl=cfg_verify_ssl)
61-
self.client = _create_client(
62-
provider_metadata=config["provider_metadata"],
63-
client_metadata=config["client"]["client_metadata"],
64-
settings=oidc_settings,
65-
)
63+
64+
try:
65+
self.client = _create_client(
66+
provider_metadata=config["provider_metadata"],
67+
client_metadata=config["client"]["client_metadata"],
68+
settings=oidc_settings,
69+
)
70+
except Exception as exc:
71+
msg = {
72+
"message": f"Failed to initialize client",
73+
"error": str(exc),
74+
"client_metadata": self.config['client']['client_metadata'],
75+
"provider_metadata": self.config['provider_metadata'],
76+
}
77+
logline = lu.LOG_FMT.format(id=lu.get_session_id(context.state), message=msg)
78+
logger.error(logline)
79+
raise SATOSAAuthenticationError(context.state, msg) from exc
80+
6681
if "scope" not in config["client"]["auth_req_params"]:
6782
config["auth_req_params"]["scope"] = "openid"
6883
if "response_type" not in config["client"]["auth_req_params"]:
@@ -185,6 +200,22 @@ def response_endpoint(self, context, *args):
185200
:param args: None
186201
:return:
187202
"""
203+
204+
if self.name not in context.state:
205+
"""
206+
If we end up here, it means that the user returns to the proxy
207+
without the SATOSA session cookie. This can happen at least in the
208+
following cases:
209+
- the user deleted the cookie from the browser
210+
- the browser of the user blocked the cookie
211+
- the user has completed an authentication flow, the cookie has
212+
been removed by SATOSA and then the user used the back button
213+
of their browser and resend the authentication response, but
214+
without the SATOSA session cookie
215+
"""
216+
error = "Received AuthN response without a SATOSA session cookie"
217+
raise SATOSAMissingStateError(error)
218+
188219
backend_state = context.state[self.name]
189220
authn_resp = self.client.parse_response(AuthorizationResponse, info=context.request, sformat="dict")
190221
if backend_state[STATE_KEY] != authn_resp["state"]:
@@ -215,7 +246,6 @@ def response_endpoint(self, context, *args):
215246
msg = "UserInfo: {}".format(all_user_claims)
216247
logline = lu.LOG_FMT.format(id=lu.get_session_id(context.state), message=msg)
217248
logger.debug(logline)
218-
del context.state[self.name]
219249
internal_resp = self._translate_response(all_user_claims, self.client.authorization_endpoint)
220250
return self.auth_callback_func(context, internal_resp)
221251

src/satosa/backends/orcid.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ def _authn_response(self, context):
7979
internal_response.attributes = self.converter.to_internal(
8080
self.external_type, user_info)
8181
internal_response.subject_id = user_info[self.user_id_attr]
82-
del context.state[self.name]
8382
return self.auth_callback_func(context, internal_response)
8483

8584
def user_information(self, access_token, orcid, name=None):

src/satosa/backends/saml2.py

Lines changed: 99 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
from satosa.internal import AuthenticationInformation
2828
from satosa.internal import InternalData
2929
from satosa.exception import SATOSAAuthenticationError
30+
from satosa.exception import SATOSAMissingStateError
31+
from satosa.exception import SATOSAAuthenticationFlowError
3032
from satosa.response import SeeOther, Response
3133
from satosa.saml_util import make_saml_response
3234
from satosa.metadata_creation.description import (
@@ -224,6 +226,14 @@ def disco_query(self, context):
224226
loc = self.sp.create_discovery_service_request(
225227
disco_url, self.sp.config.entityid, **args
226228
)
229+
230+
msg = {
231+
"message": "Sending user to the discovery service",
232+
"disco_url": loc
233+
}
234+
logline = lu.LOG_FMT.format(id=lu.get_session_id(context.state), message=msg)
235+
logger.info(logline)
236+
227237
return SeeOther(loc)
228238

229239
def construct_requested_authn_context(self, entity_id, *, target_accr=None):
@@ -268,10 +278,13 @@ def authn_request(self, context, entity_id):
268278
with open(self.idp_blacklist_file) as blacklist_file:
269279
blacklist_array = json.load(blacklist_file)['blacklist']
270280
if entity_id in blacklist_array:
271-
msg = "IdP with EntityID {} is blacklisted".format(entity_id)
281+
msg = {
282+
"message": "AuthnRequest Failed",
283+
"error": f"Selected IdP with EntityID {entity_id} is blacklisted for this backend",
284+
}
272285
logline = lu.LOG_FMT.format(id=lu.get_session_id(context.state), message=msg)
273-
logger.debug(logline, exc_info=False)
274-
raise SATOSAAuthenticationError(context.state, "Selected IdP is blacklisted for this backend")
286+
logger.info(logline)
287+
raise SATOSAAuthenticationError(context.state, msg)
275288

276289
kwargs = {}
277290
target_accr = context.state.get(Context.KEY_TARGET_AUTHN_CONTEXT_CLASS_REF)
@@ -299,16 +312,22 @@ def authn_request(self, context, entity_id):
299312
**kwargs,
300313
)
301314
except Exception as e:
302-
msg = "Failed to construct the AuthnRequest for state"
315+
msg = {
316+
"message": "AuthnRequest Failed",
317+
"error": f"Failed to construct the AuthnRequest for state: {e}",
318+
}
303319
logline = lu.LOG_FMT.format(id=lu.get_session_id(context.state), message=msg)
304-
logger.error(logline, exc_info=True)
305-
raise SATOSAAuthenticationError(context.state, "Failed to construct the AuthnRequest") from e
320+
logger.info(logline)
321+
raise SATOSAAuthenticationError(context.state, msg) from e
306322

307323
if self.sp.config.getattr('allow_unsolicited', 'sp') is False:
308324
if req_id in self.outstanding_queries:
309-
msg = "Request with duplicate id {}".format(req_id)
325+
msg = {
326+
"message": "AuthnRequest Failed",
327+
"error": f"Request with duplicate id {req_id}",
328+
}
310329
logline = lu.LOG_FMT.format(id=lu.get_session_id(context.state), message=msg)
311-
logger.debug(logline)
330+
logger.info(logline)
312331
raise SATOSAAuthenticationError(context.state, msg)
313332
self.outstanding_queries[req_id] = req_id
314333

@@ -378,43 +397,78 @@ def authn_response(self, context, binding):
378397
:param binding: The saml binding type
379398
:return: response
380399
"""
400+
401+
if self.name not in context.state:
402+
"""
403+
If we end up here, it means that the user returns to the proxy
404+
without the SATOSA session cookie. This can happen at least in the
405+
following cases:
406+
- the user deleted the cookie from the browser
407+
- the browser of the user blocked the cookie
408+
- the user has completed an authentication flow, the cookie has
409+
been removed by SATOSA and then the user used the back button
410+
of their browser and resend the authentication response, but
411+
without the SATOSA session cookie
412+
"""
413+
msg = {
414+
"message": "Authentication failed",
415+
"error": "Received AuthN response without a SATOSA session cookie",
416+
}
417+
logline = lu.LOG_FMT.format(id=lu.get_session_id(context.state), message=msg)
418+
logger.info(logline)
419+
raise SATOSAMissingStateError(msg)
420+
381421
if not context.request.get("SAMLResponse"):
382-
msg = "Missing Response for state"
422+
msg = {
423+
"message": "Authentication failed",
424+
"error": "SAML Response not found in context.request",
425+
}
383426
logline = lu.LOG_FMT.format(id=lu.get_session_id(context.state), message=msg)
384-
logger.debug(logline)
385-
raise SATOSAAuthenticationError(context.state, "Missing Response")
427+
logger.info(logline)
428+
raise SATOSAAuthenticationError(context.state, msg)
386429

387430
try:
388431
authn_response = self.sp.parse_authn_request_response(
389432
context.request["SAMLResponse"],
390-
binding, outstanding=self.outstanding_queries)
391-
except Exception as err:
392-
msg = "Failed to parse authn request for state"
433+
binding,
434+
outstanding=self.outstanding_queries,
435+
)
436+
except Exception as e:
437+
msg = {
438+
"message": "Authentication failed",
439+
"error": f"Failed to parse Authn response: {err}",
440+
}
393441
logline = lu.LOG_FMT.format(id=lu.get_session_id(context.state), message=msg)
394442
logger.debug(logline, exc_info=True)
395-
raise SATOSAAuthenticationError(context.state, "Failed to parse authn request") from err
443+
logger.info(logline)
444+
raise SATOSAAuthenticationError(context.state, msg) from e
396445

397446
if self.sp.config.getattr('allow_unsolicited', 'sp') is False:
398447
req_id = authn_response.in_response_to
399448
if req_id not in self.outstanding_queries:
400-
msg = "No request with id: {}".format(req_id),
449+
msg = {
450+
"message": "Authentication failed",
451+
"error": f"No corresponding request with id: {req_id}",
452+
}
401453
logline = lu.LOG_FMT.format(id=lu.get_session_id(context.state), message=msg)
402-
logger.debug(logline)
454+
logger.info(logline)
403455
raise SATOSAAuthenticationError(context.state, msg)
404456
del self.outstanding_queries[req_id]
405457

406458
# check if the relay_state matches the cookie state
407459
if context.state[self.name]["relay_state"] != context.request["RelayState"]:
408-
msg = "State did not match relay state for state"
460+
msg = {
461+
"message": "Authentication failed",
462+
"error": "Response state query param did not match relay state for request",
463+
}
409464
logline = lu.LOG_FMT.format(id=lu.get_session_id(context.state), message=msg)
410-
logger.debug(logline)
411-
raise SATOSAAuthenticationError(context.state, "State did not match relay state")
465+
logger.info(logline)
466+
raise SATOSAAuthenticationError(context.state, msg)
412467

413468
context.decorate(Context.KEY_METADATA_STORE, self.sp.metadata)
414469
if self.config.get(SAMLBackend.KEY_MEMORIZE_IDP):
415470
issuer = authn_response.response.issuer.text.strip()
416471
context.state[Context.KEY_MEMORIZED_IDP] = issuer
417-
context.state.pop(self.name, None)
418472
context.state.pop(Context.KEY_FORCE_AUTHN, None)
419473
return self.auth_callback_func(context, self._translate_response(authn_response, context.state))
420474

@@ -431,13 +485,18 @@ def disco_response(self, context):
431485
info = context.request
432486
state = context.state
433487

434-
try:
435-
entity_id = info["entityID"]
436-
except KeyError as err:
437-
msg = "No IDP chosen for state"
438-
logline = lu.LOG_FMT.format(id=lu.get_session_id(state), message=msg)
439-
logger.debug(logline, exc_info=True)
440-
raise SATOSAAuthenticationError(state, "No IDP chosen") from err
488+
if 'SATOSA_BASE' not in state:
489+
raise SATOSAAuthenticationFlowError("Discovery response without AuthN request")
490+
491+
entity_id = info.get("entityID")
492+
msg = {
493+
"message": "Received response from the discovery service",
494+
"entity_id": entity_id,
495+
}
496+
logline = lu.LOG_FMT.format(id=lu.get_session_id(context.state), message=msg)
497+
logger.info(logline)
498+
if not entity_id:
499+
raise SATOSAAuthenticationError(state, msg) from err
441500

442501
return self.authn_request(context, entity_id)
443502

@@ -488,11 +547,20 @@ def _translate_response(self, response, state):
488547
subject_id=name_id,
489548
)
490549

491-
msg = "backend received attributes:\n{}".format(
492-
json.dumps(response.ava, indent=4)
493-
)
550+
msg = "backend received attributes: {}".format(response.ava)
494551
logline = lu.LOG_FMT.format(id=lu.get_session_id(state), message=msg)
495552
logger.debug(logline)
553+
554+
msg = {
555+
"message": "Attributes received by the backend",
556+
"issuer": issuer,
557+
"attributes": " ".join(list(response.ava.keys()))
558+
}
559+
if name_id_format:
560+
msg['name_id'] = name_id_format
561+
logline = lu.LOG_FMT.format(id=lu.get_session_id(state), message=msg)
562+
logger.info(logline)
563+
496564
return internal_resp
497565

498566
def _metadata_endpoint(self, context):

0 commit comments

Comments
 (0)