Skip to content

Commit 8610ec2

Browse files
PeshekDotDevPeshekDotDev
authored
sources/saml: update handling statusmessage (#19739)
* sources/saml: update handling statusmessage * add tests * Catch ValueError properly
1 parent 524ab27 commit 8610ec2

File tree

6 files changed

+106
-3
lines changed

6 files changed

+106
-3
lines changed

‎authentik/common/saml/constants.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
SAML_BINDING_POST = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"
2929
SAML_BINDING_REDIRECT = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect"
3030

31+
SAML_STATUS_SUCCESS = "urn:oasis:names:tc:SAML:2.0:status:Success"
32+
3133
DSA_SHA1 = "http://www.w3.org/2000/09/xmldsig#dsa-sha1"
3234
RSA_SHA1 = "http://www.w3.org/2000/09/xmldsig#rsa-sha1"
3335
# https://datatracker.ietf.org/doc/html/rfc4051#section-2.3.2

‎authentik/sources/saml/processors/response.py‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
SAML_NAME_ID_FORMAT_TRANSIENT,
2424
SAML_NAME_ID_FORMAT_WINDOWS,
2525
SAML_NAME_ID_FORMAT_X509,
26+
SAML_STATUS_SUCCESS,
2627
)
2728
from authentik.core.models import (
2829
USER_ATTRIBUTE_DELETE_ON_LOGOUT,
@@ -186,9 +187,19 @@ def _verify_status(self):
186187
status = self._root.find(f"{{{NS_SAML_PROTOCOL}}}Status")
187188
if status is None:
188189
return
190+
status_code = status.find(f"{{{NS_SAML_PROTOCOL}}}StatusCode")
189191
message = status.find(f"{{{NS_SAML_PROTOCOL}}}StatusMessage")
190-
if message is not None:
191-
raise ValueError(message.text)
192+
message_text = message.text if message is not None else None
193+
detail = status.find(f"{{{NS_SAML_PROTOCOL}}}StatusDetail")
194+
detail_text = etree.tostring(detail, encoding="unicode") if detail is not None else None
195+
if status_code.attrib.get("Value") != SAML_STATUS_SUCCESS:
196+
if detail_text and message_text:
197+
raise ValueError(f"{message_text}: {detail_text}")
198+
raise ValueError(
199+
detail_text or message_text or f"SAML Status: {status_code.attrib.get('Value')}"
200+
)
201+
if message_text or detail_text:
202+
LOGGER.debug("SAML Status message", message=message_text, detail=detail_text)
192203

193204
def _handle_name_id_transient(self) -> SourceFlowManager:
194205
"""Handle a NameID with the Format of Transient. This is a bit more complex than other

‎authentik/sources/saml/tests/fixtures/response_error_with_detail.xml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
2+
<saml2p:Response xmlns:saml2p="urn:oasis:names:tc:SAML:2.0:protocol" Destination="https://127.0.0.1:9443/source/saml/google/acs/" ID="_ee7a8865ac457e7b22cb4f16b39ceca9" IssueInstant="2022-10-14T13:52:04.479Z" Version="2.0">
3+
<saml2:Issuer xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion">https://accounts.google.com/o/saml2?idpid=</saml2:Issuer>
4+
<saml2p:Status>
5+
<saml2p:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Requester">
6+
<saml2p:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:RequestDenied"></saml2p:StatusCode>
7+
</saml2p:StatusCode>
8+
<saml2p:StatusMessage>Authentication failed</saml2p:StatusMessage>
9+
<saml2p:StatusDetail>
10+
<Cause>User account is disabled</Cause>
11+
</saml2p:StatusDetail>
12+
</saml2p:Status>
13+
</saml2p:Response>

‎authentik/sources/saml/tests/fixtures/response_success_with_message.xml‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
2+
<saml2p:Response xmlns:saml2p="urn:oasis:names:tc:SAML:2.0:protocol" Destination="https://127.0.0.1:9443/source/saml/google/acs/" ID="_1e17063957f10819a5a8e147971fec22" IssueInstant="2022-10-14T14:11:49.590Z" Version="2.0">
3+
<saml2:Issuer xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion">https://accounts.google.com/o/saml2?idpid=</saml2:Issuer>
4+
<saml2p:Status>
5+
<saml2p:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"></saml2p:StatusCode>
6+
<saml2p:StatusMessage>Login successful</saml2p:StatusMessage>
7+
<saml2p:StatusDetail>
8+
<Detail>Additional info from IdP</Detail>
9+
</saml2p:StatusDetail>
10+
</saml2p:Status>
11+
<saml2:Assertion xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion" ID="_346001c5708ffd118c40edbc0c72fc60" IssueInstant="2022-10-14T14:11:49.590Z" Version="2.0">
12+
<saml2:Issuer>https://accounts.google.com/o/saml2?idpid=</saml2:Issuer>
13+
<saml2:Subject>
14+
<saml2:NameID Format="urn:oasis:names:tc:SAML:2.0:nameid-format:persistent">jens@goauthentik.io</saml2:NameID>
15+
<saml2:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer">
16+
<saml2:SubjectConfirmationData NotOnOrAfter="2022-10-14T14:16:49.590Z" Recipient="https://127.0.0.1:9443/source/saml/google/acs/"></saml2:SubjectConfirmationData>
17+
</saml2:SubjectConfirmation>
18+
</saml2:Subject>
19+
<saml2:Conditions NotBefore="2022-10-14T14:06:49.590Z" NotOnOrAfter="2022-10-14T14:16:49.590Z">
20+
<saml2:AudienceRestriction>
21+
<saml2:Audience>https://accounts.google.com/o/saml2?idpid=</saml2:Audience>
22+
</saml2:AudienceRestriction>
23+
</saml2:Conditions>
24+
<saml2:AttributeStatement>
25+
<saml2:Attribute Name="name">
26+
<saml2:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema"
27+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:anyType">foo</saml2:AttributeValue>
28+
</saml2:Attribute>
29+
<saml2:Attribute Name="sn">
30+
<saml2:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema"
31+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:anyType">bar</saml2:AttributeValue>
32+
</saml2:Attribute>
33+
<saml2:Attribute Name="email">
34+
<saml2:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema"
35+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:anyType">foo@bar.baz</saml2:AttributeValue>
36+
</saml2:Attribute>
37+
</saml2:AttributeStatement>
38+
<saml2:AuthnStatement AuthnInstant="2022-10-14T12:16:21.000Z" SessionIndex="_346001c5708ffd118c40edbc0c72fc60">
39+
<saml2:AuthnContext>
40+
<saml2:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:unspecified</saml2:AuthnContextClassRef>
41+
</saml2:AuthnContext>
42+
</saml2:AuthnStatement>
43+
</saml2:Assertion>
44+
</saml2p:Response>

‎authentik/sources/saml/tests/test_response.py‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,39 @@ def test_success(self):
7272
},
7373
)
7474

75+
def test_success_with_status_message_and_detail(self):
76+
"""Test success with StatusMessage and StatusDetail present (should not raise error)"""
77+
request = self.factory.post(
78+
"/",
79+
data={
80+
"SAMLResponse": b64encode(
81+
load_fixture("fixtures/response_success_with_message.xml").encode()
82+
).decode()
83+
},
84+
)
85+
86+
parser = ResponseProcessor(self.source, request)
87+
parser.parse()
88+
sfm = parser.prepare_flow_manager()
89+
self.assertEqual(sfm.user_properties["username"], "jens@goauthentik.io")
90+
91+
def test_error_with_message_and_detail(self):
92+
"""Test error status with StatusMessage and StatusDetail includes both in error"""
93+
request = self.factory.post(
94+
"/",
95+
data={
96+
"SAMLResponse": b64encode(
97+
load_fixture("fixtures/response_error_with_detail.xml").encode()
98+
).decode()
99+
},
100+
)
101+
102+
with self.assertRaises(ValueError) as ctx:
103+
ResponseProcessor(self.source, request).parse()
104+
# Should contain both detail and message
105+
self.assertIn("User account is disabled", str(ctx.exception))
106+
self.assertIn("Authentication failed", str(ctx.exception))
107+
75108
def test_encrypted_correct(self):
76109
"""Test encrypted"""
77110
key = load_fixture("fixtures/encrypted-key.pem")

‎authentik/sources/saml/views.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ def post(self, request: HttpRequest, source_slug: str) -> HttpResponse:
156156
processor = ResponseProcessor(source, request)
157157
try:
158158
processor.parse()
159-
except (InvalidSignature, MissingSAMLResponse, VerificationError) as exc:
159+
except (InvalidSignature, MissingSAMLResponse, VerificationError, ValueError) as exc:
160160
return bad_request_message(request, str(exc))
161161

162162
try:

0 commit comments

Comments
 (0)