diff --git a/config/services_test.yaml b/config/services_test.yaml new file mode 100644 index 00000000..e29da6fc --- /dev/null +++ b/config/services_test.yaml @@ -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 \ No newline at end of file diff --git a/src/Entity/UserSystem/User.php b/src/Entity/UserSystem/User.php index 47b2f0a4..eddf9179 100644 --- a/src/Entity/UserSystem/User.php +++ b/src/Entity/UserSystem/User.php @@ -912,5 +912,16 @@ class User extends AttachmentContainingDBElement implements UserInterface, HasPe 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/EnsureSAMLUserForSAMLLoginChecker.php b/src/Security/EnsureSAMLUserForSAMLLoginChecker.php index a65e6146..1d4c3cfe 100644 --- a/src/Security/EnsureSAMLUserForSAMLLoginChecker.php +++ b/src/Security/EnsureSAMLUserForSAMLLoginChecker.php @@ -44,7 +44,7 @@ class EnsureSAMLUserForSAMLLoginChecker implements EventSubscriberInterface ]; } - public function onAuthenticationSuccess(AuthenticationSuccessEvent $event) + public function onAuthenticationSuccess(AuthenticationSuccessEvent $event): void { $token = $event->getAuthenticationToken(); $user = $token->getUser(); diff --git a/src/Security/SamlUserFactory.php b/src/Security/SamlUserFactory.php index 06e31264..d212f78b 100644 --- a/src/Security/SamlUserFactory.php +++ b/src/Security/SamlUserFactory.php @@ -31,13 +31,12 @@ class SamlUserFactory implements SamlUserFactoryInterface $user = new User(); $user->setName($username); $user->setNeedPwChange(false); - $user->setPassword('$$SAML$$'); + $user->setPassword('!!SAML!!'); //This is a SAML user now! $user->setSamlUser(true); $user->setSamlAttributes($attributes); - return $user; } } \ No newline at end of file diff --git a/tests/Entity/UserSystem/UserTest.php b/tests/Entity/UserSystem/UserTest.php index 89aab673..456457a4 100644 --- a/tests/Entity/UserSystem/UserTest.php +++ b/tests/Entity/UserSystem/UserTest.php @@ -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()); + } } diff --git a/tests/Security/EnsureSAMLUserForSAMLLoginCheckerTest.php b/tests/Security/EnsureSAMLUserForSAMLLoginCheckerTest.php new file mode 100644 index 00000000..193011cd --- /dev/null +++ b/tests/Security/EnsureSAMLUserForSAMLLoginCheckerTest.php @@ -0,0 +1,70 @@ +. + */ + +namespace App\Tests\Security; + +use App\Entity\UserSystem\User; +use App\Security\EnsureSAMLUserForSAMLLoginChecker; +use Hslavich\OneloginSamlBundle\Security\Http\Authenticator\Token\SamlToken; +use PHPUnit\Framework\TestCase; +use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; +use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; +use Symfony\Component\Security\Core\Event\AuthenticationSuccessEvent; +use Symfony\Component\Security\Core\Exception\CustomUserMessageAccountStatusException; + +class EnsureSAMLUserForSAMLLoginCheckerTest extends WebTestCase +{ + /** @var EnsureSAMLUserForSAMLLoginChecker */ + protected $service; + + protected function setUp(): void + { + self::bootKernel(); + $this->service = self::getContainer()->get('saml_user_factory'); + } + + public function testOnAuthenticationSuccessFailsOnSSOLoginWithLocalUser(): void + { + $local_user = new User(); + + $saml_token = $this->createMock(SamlToken::class); + $saml_token->method('getUser')->willReturn($local_user); + + $event = new AuthenticationSuccessEvent($saml_token); + + $this->expectException(CustomUserMessageAccountStatusException::class); + + $this->service->onAuthenticationSuccess($event); + } + + public function testOnAuthenticationSuccessFailsOnLocalLoginWithSAMLUser(): void + { + $saml_user = (new User())->setSamlUser(true); + + $saml_token = $this->createMock(UsernamePasswordToken::class); + $saml_token->method('getUser')->willReturn($saml_user); + + $event = new AuthenticationSuccessEvent($saml_token); + + $this->expectException(CustomUserMessageAccountStatusException::class); + + $this->service->onAuthenticationSuccess($event); + } +} diff --git a/tests/Security/SamlUserFactoryTest.php b/tests/Security/SamlUserFactoryTest.php new file mode 100644 index 00000000..6127237a --- /dev/null +++ b/tests/Security/SamlUserFactoryTest.php @@ -0,0 +1,65 @@ +. + */ + +namespace App\Tests\Security; + +use App\Entity\UserSystem\User; +use App\Security\SamlUserFactory; +use PHPUnit\Framework\TestCase; +use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; + +class SamlUserFactoryTest extends WebTestCase +{ + + /** @var SamlUserFactory */ + protected $service; + + protected function setUp(): void + { + self::bootKernel(); + $this->service = self::getContainer()->get(SamlUserFactory::class); + } + + public function testCreateUser() + { + $user = $this->service->createUser('sso_user', [ + 'email' => ['j.doe@invalid.invalid'], + 'urn:oid:2.5.4.42' => ['John'], + 'urn:oid:2.5.4.4' => ['Doe'], + 'department' => ['IT'] + ]); + + $this->assertInstanceOf(User::class, $user); + + $this->assertEquals('sso_user', $user->getUsername()); + //User must not change his password + $this->assertFalse($user->isNeedPwChange()); + //And must not be disabled + $this->assertFalse($user->isDisabled()); + //Password should not be set + $this->assertSame('!!SAML!!', $user->getPassword()); + + //Info should be set + $this->assertEquals('John', $user->getFirstName()); + $this->assertEquals('Doe', $user->getLastName()); + $this->assertEquals('IT', $user->getDepartment()); + $this->assertEquals('j.doe@invalid.invalid', $user->getEmail()); + } +}