Revert "Moved all user info updating logic into SAMLUserFactory"

This reverts commit 960ee342e4.
This commit is contained in:
Jan Böhmer 2023-02-24 00:12:44 +01:00
parent 960ee342e4
commit 99f04d71af
8 changed files with 88 additions and 133 deletions

View file

@ -33,7 +33,7 @@ security:
saml:
use_referer: true
user_factory: saml_user_factory
persist_user: false # False as we have our own persisting mechanism in SAMLUserFactory
persist_user: true
check_path: saml_acs
login_path: saml_login
failure_path: login

View file

@ -0,0 +1,9 @@
# Service overrides for the test environment
services:
saml_user_factory:
class: App\Security\SamlUserFactory
public: true
App\Security\SamlUserFactory:
public: true

View file

@ -1,13 +1,11 @@
<?xml version="1.0"?>
<psalm
errorLevel="5"
totallyTyped="false"
resolveFromConfigFile="true"
findUnusedBaselineEntry="true"
findUnusedCode="true"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://getpsalm.org/schema/config"
xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
totallyTyped="false"
resolveFromConfigFile="true"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://getpsalm.org/schema/config"
xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
>
<projectFiles>
<directory name="src"/>
@ -54,4 +52,4 @@
<InvalidStringClass errorLevel="info"/>
</issueHandlers>
<plugins><pluginClass class="Psalm\SymfonyPsalmPlugin\Plugin"/></plugins></psalm>
<plugins><pluginClass class="Psalm\SymfonyPsalmPlugin\Plugin"/></plugins></psalm>

View file

@ -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
BackupCodeInterface, TrustedDeviceInterface, WebauthnTwoFactorInterface, PreferredProviderInterface, PasswordAuthenticatedUserInterface, SamlUserInterface
{
//use MasterAttachmentTrait;
@ -892,4 +892,36 @@ 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]);
}
}
}

View file

@ -21,29 +21,13 @@
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, EventSubscriberInterface
class SamlUserFactory implements SamlUserFactoryInterface
{
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();
@ -53,76 +37,8 @@ class SamlUserFactory implements SamlUserFactoryInterface, EventSubscriberInterf
//This is a SAML user now!
$user->setSamlUser(true);
$this->updateUserInfoFromSAMLAttributes($user, $attributes);
$user->setSamlAttributes($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]);
}
}
}

View file

@ -148,4 +148,40 @@ 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());
}
}

View file

@ -62,40 +62,4 @@ 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());
}
}

View file

@ -10990,7 +10990,7 @@ Element 3</target>
<unit id="wnMLanX" name="login.local_login_hint">
<segment>
<source>login.local_login_hint</source>
<target>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.</target>
<target>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.</target>
</segment>
</unit>
</file>