diff --git a/config/packages/security.yaml b/config/packages/security.yaml index 7e0ded3c..f03e0feb 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -33,7 +33,7 @@ security: saml: use_referer: true user_factory: saml_user_factory - persist_user: true + persist_user: false # False as we have our own persisting mechanism in SAMLUserFactory check_path: saml_acs login_path: saml_login failure_path: login diff --git a/config/services_test.yaml b/config/services_test.yaml deleted file mode 100644 index e29da6fc..00000000 --- a/config/services_test.yaml +++ /dev/null @@ -1,9 +0,0 @@ -# Service overrides for the test environment - -services: - saml_user_factory: - class: App\Security\SamlUserFactory - public: true - - App\Security\SamlUserFactory: - public: true \ No newline at end of file diff --git a/psalm.xml b/psalm.xml index 872169ac..71c720ab 100644 --- a/psalm.xml +++ b/psalm.xml @@ -1,11 +1,13 @@ @@ -52,4 +54,4 @@ - + diff --git a/src/Entity/UserSystem/User.php b/src/Entity/UserSystem/User.php index eddf9179..4289aa0d 100644 --- a/src/Entity/UserSystem/User.php +++ b/src/Entity/UserSystem/User.php @@ -62,7 +62,7 @@ use Jbtronics\TFAWebauthn\Model\TwoFactorInterface as WebauthnTwoFactorInterface * @UniqueEntity("name", message="validator.user.username_already_used") */ class User extends AttachmentContainingDBElement implements UserInterface, HasPermissionsInterface, TwoFactorInterface, - BackupCodeInterface, TrustedDeviceInterface, WebauthnTwoFactorInterface, PreferredProviderInterface, PasswordAuthenticatedUserInterface, SamlUserInterface + BackupCodeInterface, TrustedDeviceInterface, WebauthnTwoFactorInterface, PreferredProviderInterface, PasswordAuthenticatedUserInterface { //use MasterAttachmentTrait; @@ -892,36 +892,4 @@ class User extends AttachmentContainingDBElement implements UserInterface, HasPe $this->saml_user = $saml_user; return $this; } - - - - public function setSamlAttributes(array $attributes) - { - //When mail attribute exists, set it - if (isset($attributes['email'])) { - $this->setEmail($attributes['email'][0]); - } - //When first name attribute exists, set it - if (isset($attributes['firstName'])) { - $this->setFirstName($attributes['firstName'][0]); - } - //When last name attribute exists, set it - if (isset($attributes['lastName'])) { - $this->setLastName($attributes['lastName'][0]); - } - if (isset($attributes['department'])) { - $this->setDepartment($attributes['department'][0]); - } - - //Use X500 attributes as userinfo - if (isset($attributes['urn:oid:2.5.4.42'])) { - $this->setFirstName($attributes['urn:oid:2.5.4.42'][0]); - } - if (isset($attributes['urn:oid:2.5.4.4'])) { - $this->setLastName($attributes['urn:oid:2.5.4.4'][0]); - } - if (isset($attributes['urn:oid:1.2.840.113549.1.9.1'])) { - $this->setEmail($attributes['urn:oid:1.2.840.113549.1.9.1'][0]); - } - } } diff --git a/src/Security/SamlUserFactory.php b/src/Security/SamlUserFactory.php index fd181133..168c4269 100644 --- a/src/Security/SamlUserFactory.php +++ b/src/Security/SamlUserFactory.php @@ -21,13 +21,29 @@ namespace App\Security; use App\Entity\UserSystem\User; +use Doctrine\ORM\EntityManagerInterface; +use Hslavich\OneloginSamlBundle\Event\AbstractUserEvent; +use Hslavich\OneloginSamlBundle\Event\UserCreatedEvent; +use Hslavich\OneloginSamlBundle\Event\UserModifiedEvent; +use Hslavich\OneloginSamlBundle\Security\Http\Authenticator\Token\SamlToken; use Hslavich\OneloginSamlBundle\Security\User\SamlUserFactoryInterface; +use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Symfony\Component\Security\Core\Security; use Symfony\Component\Security\Core\User\UserInterface; -class SamlUserFactory implements SamlUserFactoryInterface +class SamlUserFactory implements SamlUserFactoryInterface, EventSubscriberInterface { public const SAML_PASSWORD_PLACEHOLDER = '!!SAML!!'; + private Security $security; + private EntityManagerInterface $entityManager; + + public function __construct(Security $security, EntityManagerInterface $entityManager) + { + $this->entityManager = $entityManager; + $this->security = $security; + } + public function createUser($username, array $attributes = []): UserInterface { $user = new User(); @@ -37,8 +53,76 @@ class SamlUserFactory implements SamlUserFactoryInterface //This is a SAML user now! $user->setSamlUser(true); - $user->setSamlAttributes($attributes); + $this->updateUserInfoFromSAMLAttributes($user, $attributes); return $user; } + + public function updateAndPersistUser(AbstractUserEvent $event): void + { + $user = $event->getUser(); + $token = $this->security->getToken(); + + if (!$user instanceof User) { + throw new \RuntimeException('User must be an instance of '.User::class); + } + if (!$token instanceof SamlToken) { + throw new \RuntimeException('Token must be an instance of '.SamlToken::class); + } + + $attributes = $token->getAttributes(); + + //Update the user info based on the SAML attributes + $this->updateUserInfoFromSAMLAttributes($user, $attributes); + + //Persist the user + $this->entityManager->persist($user); + + //Flush the entity manager + $this->entityManager->flush(); + } + + public static function getSubscribedEvents() + { + return [ + UserCreatedEvent::class => 'updateAndPersistUser', + UserModifiedEvent::class => 'updateAndPersistUser', + ]; + } + + /** + * Sets the SAML attributes to the user. + * @param User $user + * @param array $attributes + * @return void + */ + public function updateUserInfoFromSAMLAttributes(User $user, array $attributes): void + { + //When mail attribute exists, set it + if (isset($attributes['email'])) { + $user->setEmail($attributes['email'][0]); + } + //When first name attribute exists, set it + if (isset($attributes['firstName'])) { + $user->setFirstName($attributes['firstName'][0]); + } + //When last name attribute exists, set it + if (isset($attributes['lastName'])) { + $user->setLastName($attributes['lastName'][0]); + } + if (isset($attributes['department'])) { + $user->setDepartment($attributes['department'][0]); + } + + //Use X500 attributes as userinfo + if (isset($attributes['urn:oid:2.5.4.42'])) { + $user->setFirstName($attributes['urn:oid:2.5.4.42'][0]); + } + if (isset($attributes['urn:oid:2.5.4.4'])) { + $user->setLastName($attributes['urn:oid:2.5.4.4'][0]); + } + if (isset($attributes['urn:oid:1.2.840.113549.1.9.1'])) { + $user->setEmail($attributes['urn:oid:1.2.840.113549.1.9.1'][0]); + } + } } \ No newline at end of file diff --git a/tests/Entity/UserSystem/UserTest.php b/tests/Entity/UserSystem/UserTest.php index 456457a4..89aab673 100644 --- a/tests/Entity/UserSystem/UserTest.php +++ b/tests/Entity/UserSystem/UserTest.php @@ -148,40 +148,4 @@ class UserTest extends TestCase } $this->assertFalse($user->isWebAuthnAuthenticatorEnabled()); } - - public function testSetSAMLAttributes(): void - { - $data = [ - 'firstName' => ['John'], - 'lastName' => ['Doe'], - 'email' => ['j.doe@invalid.invalid'], - 'department' => ['Test Department'], - ]; - - $user = new User(); - $user->setSAMLAttributes($data); - - //Test if the data was set correctly - $this->assertSame('John', $user->getFirstName()); - $this->assertSame('Doe', $user->getLastName()); - $this->assertSame('j.doe@invalid.invalid', $user->getEmail()); - $this->assertSame('Test Department', $user->getDepartment()); - - //Test that it works for X500 attributes - $data = [ - 'urn:oid:2.5.4.42' => ['Jane'], - 'urn:oid:2.5.4.4' => ['Dane'], - 'urn:oid:1.2.840.113549.1.9.1' => ['mail@invalid.invalid'], - ]; - - $user->setSAMLAttributes($data); - - //Data must be changed - $this->assertSame('Jane', $user->getFirstName()); - $this->assertSame('Dane', $user->getLastName()); - $this->assertSame('mail@invalid.invalid', $user->getEmail()); - - //Department must not be changed - $this->assertSame('Test Department', $user->getDepartment()); - } } diff --git a/tests/Security/SamlUserFactoryTest.php b/tests/Security/SamlUserFactoryTest.php index 6127237a..77fd6ab9 100644 --- a/tests/Security/SamlUserFactoryTest.php +++ b/tests/Security/SamlUserFactoryTest.php @@ -62,4 +62,40 @@ class SamlUserFactoryTest extends WebTestCase $this->assertEquals('IT', $user->getDepartment()); $this->assertEquals('j.doe@invalid.invalid', $user->getEmail()); } + + public function testUpdateUserInfoFromSAMLAttributes(): void + { + $data = [ + 'firstName' => ['John'], + 'lastName' => ['Doe'], + 'email' => ['j.doe@invalid.invalid'], + 'department' => ['Test Department'], + ]; + + $user = new User(); + $this->service->updateUserInfoFromSAMLAttributes($user, $data); + + //Test if the data was set correctly + $this->assertSame('John', $user->getFirstName()); + $this->assertSame('Doe', $user->getLastName()); + $this->assertSame('j.doe@invalid.invalid', $user->getEmail()); + $this->assertSame('Test Department', $user->getDepartment()); + + //Test that it works for X500 attributes + $data = [ + 'urn:oid:2.5.4.42' => ['Jane'], + 'urn:oid:2.5.4.4' => ['Dane'], + 'urn:oid:1.2.840.113549.1.9.1' => ['mail@invalid.invalid'], + ]; + + $this->service->updateUserInfoFromSAMLAttributes($user, $data); + + //Data must be changed + $this->assertSame('Jane', $user->getFirstName()); + $this->assertSame('Dane', $user->getLastName()); + $this->assertSame('mail@invalid.invalid', $user->getEmail()); + + //Department must not be changed + $this->assertSame('Test Department', $user->getDepartment()); + } } diff --git a/translations/messages.en.xlf b/translations/messages.en.xlf index 34be1ce5..ae66d98c 100644 --- a/translations/messages.en.xlf +++ b/translations/messages.en.xlf @@ -10990,7 +10990,7 @@ Element 3 login.local_login_hint - The form below is only for log in for a local user. If you want to log in via single sign-on, press the button above. + The form below is only for log in with a local user. If you want to log in via single sign-on, press the button above.