Skip to content

Commit

Permalink
sources/saml: fix pickle error, add saml auth tests (cherry-pick #10348
Browse files Browse the repository at this point in the history
…) (#10352)

sources/saml: fix pickle error, add saml auth tests (#10348)

* test with persistent nameid



* fix pickle



* user_write: dont attempt to write to read only property



* add test for enroll + auth



* unwrap lazy user



---------

Signed-off-by: Jens Langhammer <jens@goauthentik.io>
Co-authored-by: Jens L <jens@goauthentik.io>
  • Loading branch information
gcp-cherry-pick-bot[bot] and BeryJu authored Jul 3, 2024
1 parent 6cdae09 commit 5997b93
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 1 deletion.
3 changes: 2 additions & 1 deletion authentik/sources/saml/processors/response.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from django.core.exceptions import SuspiciousOperation
from django.http import HttpRequest
from django.utils.timezone import now
from lxml import etree # nosec
from structlog.stdlib import get_logger

from authentik.core.models import (
Expand Down Expand Up @@ -240,7 +241,7 @@ def prepare_flow_manager(self) -> SourceFlowManager:
name_id.text,
delete_none_values(self.get_attributes()),
)
flow_manager.policy_context["saml_response"] = self._root
flow_manager.policy_context["saml_response"] = etree.tostring(self._root)
return flow_manager


Expand Down
9 changes: 9 additions & 0 deletions authentik/stages/user_write/stage.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from django.db import transaction
from django.db.utils import IntegrityError, InternalError
from django.http import HttpRequest, HttpResponse
from django.utils.functional import SimpleLazyObject
from django.utils.translation import gettext as _
from rest_framework.exceptions import ValidationError

Expand Down Expand Up @@ -118,6 +119,14 @@ def update_user(self, user: User):
UserWriteStageView.write_attribute(user, key, value)
# User has this key already
elif hasattr(user, key):
if isinstance(user, SimpleLazyObject):
user._setup()
user = user._wrapped
attr = getattr(type(user), key)
if isinstance(attr, property):
if not attr.fset:
self.logger.info("discarding key", key=key)
continue
setattr(user, key, value)
# If none of the cases above matched, we have an attribute that the user doesn't have,
# has no setter for, is not a nested attributes value and as such is invalid
Expand Down
23 changes: 23 additions & 0 deletions tests/e2e/test-saml-idp/saml20-sp-remote.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php
/**
* SAML 2.0 remote SP metadata for SimpleSAMLphp.
*
* See: https://simplesamlphp.org/docs/stable/simplesamlphp-reference-sp-remote
*/

$metadata[getenv('SIMPLESAMLPHP_SP_ENTITY_ID')] = array(
'AssertionConsumerService' => getenv('SIMPLESAMLPHP_SP_ASSERTION_CONSUMER_SERVICE'),
'SingleLogoutService' => getenv('SIMPLESAMLPHP_SP_SINGLE_LOGOUT_SERVICE'),
);

if (null != getenv('SIMPLESAMLPHP_SP_NAME_ID_FORMAT')) {
$metadata[getenv('SIMPLESAMLPHP_SP_ENTITY_ID')] = array_merge($metadata[getenv('SIMPLESAMLPHP_SP_ENTITY_ID')], array('NameIDFormat' => getenv('SIMPLESAMLPHP_SP_NAME_ID_FORMAT')));
}

if (null != getenv('SIMPLESAMLPHP_SP_NAME_ID_ATTRIBUTE')) {
$metadata[getenv('SIMPLESAMLPHP_SP_ENTITY_ID')] = array_merge($metadata[getenv('SIMPLESAMLPHP_SP_ENTITY_ID')], array('simplesaml.nameidattribute' => getenv('SIMPLESAMLPHP_SP_NAME_ID_ATTRIBUTE')));
}

if (null != getenv('SIMPLESAMLPHP_SP_SIGN_ASSERTION')) {
$metadata[getenv('SIMPLESAMLPHP_SP_ENTITY_ID')] = array_merge($metadata[getenv('SIMPLESAMLPHP_SP_ENTITY_ID')], array('saml20.sign.assertion' => ('true' == getenv('SIMPLESAMLPHP_SP_SIGN_ASSERTION'))));
}
119 changes: 119 additions & 0 deletions tests/e2e/test_source_saml.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""test SAML Source"""

from pathlib import Path
from time import sleep
from typing import Any

Expand Down Expand Up @@ -88,8 +89,20 @@ def get_container_specs(self) -> dict[str, Any] | None:
interval=5 * 1_000 * 1_000_000,
start_period=1 * 1_000 * 1_000_000,
),
"volumes": {
str(
(Path(__file__).parent / Path("test-saml-idp/saml20-sp-remote.php")).absolute()
): {
"bind": "/var/www/simplesamlphp/metadata/saml20-sp-remote.php",
"mode": "ro",
}
},
"environment": {
"SIMPLESAMLPHP_SP_ENTITY_ID": "entity-id",
"SIMPLESAMLPHP_SP_NAME_ID_FORMAT": (
"urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress"
),
"SIMPLESAMLPHP_SP_NAME_ID_ATTRIBUTE": "email",
"SIMPLESAMLPHP_SP_ASSERTION_CONSUMER_SERVICE": (
self.url("authentik_sources_saml:acs", source_slug=self.slug)
),
Expand Down Expand Up @@ -318,3 +331,109 @@ def test_idp_post_auto(self):
.exclude(pk=self.user.pk)
.first()
)

@retry()
@apply_blueprint(
"default/flow-default-authentication-flow.yaml",
"default/flow-default-invalidation-flow.yaml",
)
@apply_blueprint(
"default/flow-default-source-authentication.yaml",
"default/flow-default-source-enrollment.yaml",
"default/flow-default-source-pre-authentication.yaml",
)
def test_idp_post_auto_enroll_auth(self):
"""test SAML Source With post binding (auto redirect)"""
# Bootstrap all needed objects
authentication_flow = Flow.objects.get(slug="default-source-authentication")
enrollment_flow = Flow.objects.get(slug="default-source-enrollment")
pre_authentication_flow = Flow.objects.get(slug="default-source-pre-authentication")
keypair = CertificateKeyPair.objects.create(
name=generate_id(),
certificate_data=IDP_CERT,
key_data=IDP_KEY,
)

source = SAMLSource.objects.create(
name=generate_id(),
slug=self.slug,
authentication_flow=authentication_flow,
enrollment_flow=enrollment_flow,
pre_authentication_flow=pre_authentication_flow,
issuer="entity-id",
sso_url=f"http://{self.host}:8080/simplesaml/saml2/idp/SSOService.php",
binding_type=SAMLBindingTypes.POST_AUTO,
signing_kp=keypair,
)
ident_stage = IdentificationStage.objects.first()
ident_stage.sources.set([source])
ident_stage.save()

self.driver.get(self.live_server_url)

flow_executor = self.get_shadow_root("ak-flow-executor")
identification_stage = self.get_shadow_root("ak-stage-identification", flow_executor)
wait = WebDriverWait(identification_stage, self.wait_timeout)

wait.until(
ec.presence_of_element_located(
(By.CSS_SELECTOR, ".pf-c-login__main-footer-links-item > button")
)
)
identification_stage.find_element(
By.CSS_SELECTOR, ".pf-c-login__main-footer-links-item > button"
).click()

# Now we should be at the IDP, wait for the username field
self.wait.until(ec.presence_of_element_located((By.ID, "username")))
self.driver.find_element(By.ID, "username").send_keys("user1")
self.driver.find_element(By.ID, "password").send_keys("user1pass")
self.driver.find_element(By.ID, "password").send_keys(Keys.ENTER)

# Wait until we're logged in
self.wait_for_url(self.if_user_url("/library"))
self.driver.get(self.if_user_url("/settings"))

self.assert_user(
User.objects.exclude(username="akadmin")
.exclude(username__startswith="ak-outpost")
.exclude_anonymous()
.exclude(pk=self.user.pk)
.first()
)

# Clear all cookies and log in again
self.driver.delete_all_cookies()
self.driver.get(self.live_server_url)

flow_executor = self.get_shadow_root("ak-flow-executor")
identification_stage = self.get_shadow_root("ak-stage-identification", flow_executor)
wait = WebDriverWait(identification_stage, self.wait_timeout)

wait.until(
ec.presence_of_element_located(
(By.CSS_SELECTOR, ".pf-c-login__main-footer-links-item > button")
)
)
identification_stage.find_element(
By.CSS_SELECTOR, ".pf-c-login__main-footer-links-item > button"
).click()

# Now we should be at the IDP, wait for the username field
self.wait.until(ec.presence_of_element_located((By.ID, "username")))
self.driver.find_element(By.ID, "username").send_keys("user1")
self.driver.find_element(By.ID, "password").send_keys("user1pass")
self.driver.find_element(By.ID, "password").send_keys(Keys.ENTER)

# Wait until we're logged in
self.wait_for_url(self.if_user_url("/library"))
self.driver.get(self.if_user_url("/settings"))

# sleep(999999)
self.assert_user(
User.objects.exclude(username="akadmin")
.exclude(username__startswith="ak-outpost")
.exclude_anonymous()
.exclude(pk=self.user.pk)
.first()
)

0 comments on commit 5997b93

Please sign in to comment.