diff --git a/lib/SP/Core/Application.php b/lib/SP/Core/Application.php index 5f0ece4c..dfe1d805 100644 --- a/lib/SP/Core/Application.php +++ b/lib/SP/Core/Application.php @@ -4,7 +4,7 @@ * * @author nuxsmin * @link https://syspass.org - * @copyright 2012-2022, Rubén Domínguez nuxsmin@$syspass.org + * @copyright 2012-2023, Rubén Domínguez nuxsmin@$syspass.org * * This file is part of sysPass. * @@ -36,15 +36,16 @@ final class Application /** * Module constructor. * - * @param ConfigInterface $config - * @param EventDispatcherInterface $eventDispatcher - * @param ContextInterface $context + * @param ConfigInterface $config + * @param EventDispatcherInterface $eventDispatcher + * @param ContextInterface $context */ public function __construct( - private ConfigInterface $config, - private EventDispatcherInterface $eventDispatcher, - private ContextInterface $context - ) {} + private readonly ConfigInterface $config, + private readonly EventDispatcherInterface $eventDispatcher, + private readonly ContextInterface $context + ) { + } public function getConfig(): ConfigInterface { diff --git a/lib/SP/Core/Definitions/CoreDefinitions.php b/lib/SP/Core/Definitions/CoreDefinitions.php index 80b12d6d..b10284a0 100644 --- a/lib/SP/Core/Definitions/CoreDefinitions.php +++ b/lib/SP/Core/Definitions/CoreDefinitions.php @@ -78,6 +78,7 @@ use SP\Mvc\View\TemplateInterface; use SP\Providers\Acl\AclHandler; use SP\Providers\Auth\AuthProvider; use SP\Providers\Auth\AuthProviderInterface; +use SP\Providers\Auth\AuthTypeEnum; use SP\Providers\Auth\Browser\BrowserAuth; use SP\Providers\Auth\Browser\BrowserAuthInterface; use SP\Providers\Auth\Database\DatabaseAuth; @@ -155,20 +156,27 @@ final class CoreDefinitions 'ldap', factory([LdapBase::class, 'factory']) ), - AuthProviderInterface::class => - static function (ContainerInterface $c, ConfigDataInterface $configData) { - $provider = $c->get(AuthProvider::class); - + AuthProviderInterface::class => factory( + static function ( + AuthProvider $authProvider, + ConfigDataInterface $configData, + LdapAuthInterface $ldapAuth, + BrowserAuthInterface $browserAuth, + DatabaseAuthInterface $databaseAuth, + ) { if ($configData->isLdapEnabled()) { - $provider->withLdapAuth($c->get(LdapAuthInterface::class)); + $authProvider->registerAuth($ldapAuth, AuthTypeEnum::Ldap); } if ($configData->isAuthBasicEnabled()) { - $provider->withBrowserAuth($c->get(BrowserAuthInterface::class)); + $authProvider->registerAuth($browserAuth, AuthTypeEnum::Browser); } - return $provider; - }, + $authProvider->registerAuth($databaseAuth, AuthTypeEnum::Database); + + return $authProvider; + } + )->parameter('authProvider', autowire(AuthProvider::class)), Logger::class => create(Logger::class) ->constructor('syspass'), \GuzzleHttp\Client::class => create(\GuzzleHttp\Client::class) diff --git a/lib/SP/Providers/Acl/AclHandler.php b/lib/SP/Providers/Acl/AclHandler.php index f21862ef..83bbba6b 100644 --- a/lib/SP/Providers/Acl/AclHandler.php +++ b/lib/SP/Providers/Acl/AclHandler.php @@ -83,8 +83,6 @@ final class AclHandler extends Provider implements EventReceiver * * @param string $eventType event's type * @param Event $event event's source object - * - * @throws SPException */ public function update(string $eventType, Event $event): void { diff --git a/lib/SP/Providers/Auth/AuthInterface.php b/lib/SP/Providers/Auth/AuthInterface.php index a559beca..fec7b72a 100644 --- a/lib/SP/Providers/Auth/AuthInterface.php +++ b/lib/SP/Providers/Auth/AuthInterface.php @@ -29,19 +29,18 @@ use SP\DataModel\UserLoginData; /** * Interface AuthInterface * - * @template T of AuthDataBase + * @template T * @package Auth */ interface AuthInterface { /** - * Autentificar al usuario + * Authenticate using user's data * * @param UserLoginData $userLoginData - * * @return T */ - public function authenticate(UserLoginData $userLoginData): object; + public function authenticate(UserLoginData $userLoginData): AuthDataBase; /** * Indica si es requerida para acceder a la aplicación diff --git a/lib/SP/Providers/Auth/AuthProvider.php b/lib/SP/Providers/Auth/AuthProvider.php index b67cce2e..ca11f881 100644 --- a/lib/SP/Providers/Auth/AuthProvider.php +++ b/lib/SP/Providers/Auth/AuthProvider.php @@ -27,11 +27,9 @@ namespace SP\Providers\Auth; use SP\Core\Application; use SP\Core\Exceptions\SPException; use SP\DataModel\UserLoginData; -use SP\Domain\Auth\Ports\LdapAuthInterface; use SP\Domain\Auth\Services\AuthException; -use SP\Providers\Auth\Browser\BrowserAuthInterface; -use SP\Providers\Auth\Database\DatabaseAuthInterface; use SP\Providers\Provider; +use SplObjectStorage; use function SP\__u; @@ -46,18 +44,41 @@ defined('APP_ROOT') || die(); */ class AuthProvider extends Provider implements AuthProviderInterface { - /** - * @var callable[] - */ - protected array $auths = []; - protected ?BrowserAuthInterface $browserAuth = null; - protected ?LdapAuthInterface $ldapAuth = null; + protected readonly SplObjectStorage $auths; - public function __construct( - Application $application, - protected readonly DatabaseAuthInterface $databaseAuth - ) { + public function __construct(Application $application) + { parent::__construct($application); + + $this->auths = new SplObjectStorage(); + } + + /** + * Auth initializer + * + */ + public function initialize(): void + { + } + + /** + * Register authentication methods + * + * @param AuthInterface $auth + * @param AuthTypeEnum $authTypeEnum + * @throws AuthException + */ + public function registerAuth(AuthInterface $auth, AuthTypeEnum $authTypeEnum): void + { + if ($this->auths->contains($auth)) { + throw new AuthException( + __u('Authentication already initialized'), + SPException::ERROR, + $auth::class + ); + } + + $this->auths->attach($auth, $authTypeEnum->value); } /** @@ -71,91 +92,18 @@ class AuthProvider extends Provider implements AuthProviderInterface { $authsResult = []; - foreach ($this->auths as $authName => $auth) { - $data = $auth($userLoginData); + $this->auths->rewind(); - if ($data instanceof AuthDataBase) { - $authsResult[] = new AuthResult($authName, $data); - } + while ($this->auths->valid()) { + /** @var AuthInterface $auth */ + $auth = $this->auths->current(); + $authName = $this->auths->getInfo(); + + $authsResult[] = new AuthResult($authName, $auth->authenticate($userLoginData)); + + $this->auths->next(); } return count($authsResult) > 0 ? $authsResult : false; } - - /** - * Auth initializer - * - * @throws AuthException - */ - public function initialize(): void - { - $configData = $this->config->getConfigData(); - - if ($this->browserAuth && $configData->isAuthBasicEnabled()) { - $this->registerAuth( - function (UserLoginData $userLoginData) { - return $this->browserAuth->authenticate($userLoginData); - }, - 'authBrowser' - ); - } - - if ($this->ldapAuth && $configData->isLdapEnabled()) { - $this->registerAuth( - function (UserLoginData $userLoginData) { - $ldapAuthData = $this->ldapAuth->authenticate($userLoginData); - - if ($ldapAuthData->getAuthenticated()) { - // Comprobamos si la cuenta está bloqueada o expirada - if ($ldapAuthData->getExpire() > 0) { - $ldapAuthData->setStatusCode(LdapAuthInterface::ACCOUNT_EXPIRED); - } elseif (!$ldapAuthData->isInGroup()) { - $ldapAuthData->setStatusCode(LdapAuthInterface::ACCOUNT_NO_GROUPS); - } - } - - return $ldapAuthData; - }, - 'authLdap' - ); - } - - $this->registerAuth( - function (UserLoginData $userLoginData) { - return $this->databaseAuth->authenticate($userLoginData); - }, - 'authDatabase' - ); - } - - /** - * Registrar un método de autentificación primarios - * - * @param callable $auth Función de autentificación - * @param string $name - * - * @throws AuthException - */ - private function registerAuth(callable $auth, string $name): void - { - if (array_key_exists($name, $this->auths)) { - throw new AuthException( - __u('Authentication already initialized'), - SPException::ERROR, - __FUNCTION__ - ); - } - - $this->auths[$name] = $auth; - } - - public function withLdapAuth(LdapAuthInterface $ldapAuth): void - { - $this->ldapAuth = $ldapAuth; - } - - public function withBrowserAuth(BrowserAuthInterface $browserAuth): void - { - $this->browserAuth = $browserAuth; - } } diff --git a/lib/SP/Providers/Auth/AuthProviderInterface.php b/lib/SP/Providers/Auth/AuthProviderInterface.php index 91133786..bcd0de42 100644 --- a/lib/SP/Providers/Auth/AuthProviderInterface.php +++ b/lib/SP/Providers/Auth/AuthProviderInterface.php @@ -24,11 +24,8 @@ namespace SP\Providers\Auth; - use SP\DataModel\UserLoginData; -use SP\Domain\Auth\Ports\LdapAuthInterface; use SP\Domain\Auth\Services\AuthException; -use SP\Providers\Auth\Browser\BrowserAuthInterface; /** * Class Auth @@ -42,20 +39,9 @@ interface AuthProviderInterface /** * Probar los métodos de autentificación * - * @param UserLoginData $userLoginData + * @param UserLoginData $userLoginData * * @return false|AuthResult[] */ - public function doAuth(UserLoginData $userLoginData); - - /** - * Auth initializer - * - * @throws AuthException - */ - public function initialize(): void; - - public function withLdapAuth(LdapAuthInterface $ldapAuth): void; - - public function withBrowserAuth(BrowserAuthInterface $browserAuth): void; + public function doAuth(UserLoginData $userLoginData): array|bool; } diff --git a/lib/SP/Providers/Auth/AuthResult.php b/lib/SP/Providers/Auth/AuthResult.php index 695aa018..764e1360 100644 --- a/lib/SP/Providers/Auth/AuthResult.php +++ b/lib/SP/Providers/Auth/AuthResult.php @@ -4,7 +4,7 @@ * * @author nuxsmin * @link https://syspass.org - * @copyright 2012-2021, Rubén Domínguez nuxsmin@$syspass.org + * @copyright 2012-2023, Rubén Domínguez nuxsmin@$syspass.org * * This file is part of sysPass. * @@ -31,25 +31,15 @@ namespace SP\Providers\Auth; */ final class AuthResult { - /** - * @var string - */ - public string $authName; - /** - * @var AuthDataBase - */ - public AuthDataBase $data; /** * AuthResult constructor. * - * @param string $authName + * @param string $authName * @param AuthDataBase $data */ - public function __construct(string $authName, AuthDataBase $data) + public function __construct(private readonly string $authName, private readonly AuthDataBase $data) { - $this->authName = $authName; - $this->data = $data; } /** @@ -67,4 +57,4 @@ final class AuthResult { return $this->data; } -} \ No newline at end of file +} diff --git a/lib/SP/Providers/Auth/AuthTypeEnum.php b/lib/SP/Providers/Auth/AuthTypeEnum.php new file mode 100644 index 00000000..73f47841 --- /dev/null +++ b/lib/SP/Providers/Auth/AuthTypeEnum.php @@ -0,0 +1,36 @@ +. + */ + +namespace SP\Providers\Auth; + +/** + * Class AuthTypeEnum + */ +enum AuthTypeEnum: string +{ + case Ldap = 'authLdap'; + case Browser = 'authDatabase'; + case Database = 'authBrowser'; + +} diff --git a/lib/SP/Providers/Auth/Browser/BrowserAuth.php b/lib/SP/Providers/Auth/Browser/BrowserAuth.php index 348d1db7..8cb0fe91 100644 --- a/lib/SP/Providers/Auth/Browser/BrowserAuth.php +++ b/lib/SP/Providers/Auth/Browser/BrowserAuth.php @@ -48,11 +48,12 @@ final class BrowserAuth implements BrowserAuthInterface } /** - * Autentificar al usuario + * Authenticate using user's data * - * @param UserLoginData $userLoginData Datos del usuario + * @param UserLoginData $userLoginData + * @return BrowserAuthData */ - public function authenticate(UserLoginData $userLoginData): object + public function authenticate(UserLoginData $userLoginData): BrowserAuthData { $browserAuthData = new BrowserAuthData($this->isAuthGranted()); diff --git a/lib/SP/Providers/Auth/Database/DatabaseAuth.php b/lib/SP/Providers/Auth/Database/DatabaseAuth.php index b0428ba4..29933c0f 100644 --- a/lib/SP/Providers/Auth/Database/DatabaseAuth.php +++ b/lib/SP/Providers/Auth/Database/DatabaseAuth.php @@ -49,12 +49,10 @@ final class DatabaseAuth implements DatabaseAuthInterface ) { } - /** - * Autentificar al usuario - * - * @param UserLoginData $userLoginData Datos del usuario + * Authenticate using user's data * + * @param UserLoginData $userLoginData * @return DatabaseAuthData */ public function authenticate(UserLoginData $userLoginData): DatabaseAuthData diff --git a/lib/SP/Providers/Auth/Ldap/LdapAuth.php b/lib/SP/Providers/Auth/Ldap/LdapAuth.php index 75a4af15..72ffc91d 100644 --- a/lib/SP/Providers/Auth/Ldap/LdapAuth.php +++ b/lib/SP/Providers/Auth/Ldap/LdapAuth.php @@ -58,11 +58,12 @@ final class LdapAuth implements LdapAuthInterface } /** - * Autentificar al usuario + * Authenticate using user's data * - * @param UserLoginData $userLoginData Datos del usuario + * @param UserLoginData $userLoginData + * @return LdapAuthData */ - public function authenticate(UserLoginData $userLoginData): object + public function authenticate(UserLoginData $userLoginData): LdapAuthData { $ldapAuthData = new LdapAuthData($this->isAuthGranted()); @@ -73,6 +74,17 @@ final class LdapAuth implements LdapAuthInterface $this->getAttributes($userLoginData->getLoginUser(), $ldapAuthData); + // Comprobamos si la cuenta está bloqueada o expirada + if ($ldapAuthData->getExpire() > 0) { + $ldapAuthData->setStatusCode(LdapAuthInterface::ACCOUNT_EXPIRED); + + return $ldapAuthData->fail(); + } elseif (!$ldapAuthData->isInGroup()) { + $ldapAuthData->setStatusCode(LdapAuthInterface::ACCOUNT_NO_GROUPS); + + return $ldapAuthData->fail(); + } + $this->ldap->connect($ldapAuthData->getDn(), $userLoginData->getLoginPass()); return $ldapAuthData->success(); diff --git a/lib/SP/Providers/Provider.php b/lib/SP/Providers/Provider.php index 779dbca7..920e16ad 100644 --- a/lib/SP/Providers/Provider.php +++ b/lib/SP/Providers/Provider.php @@ -36,10 +36,10 @@ use SP\Domain\Config\Ports\ConfigInterface; */ abstract class Provider implements ProviderInterface { - protected ConfigInterface $config; - protected ContextInterface $context; - protected EventDispatcherInterface $eventDispatcher; - protected bool $initialized = false; + protected readonly ConfigInterface $config; + protected readonly ContextInterface $context; + protected readonly EventDispatcherInterface $eventDispatcher; + protected bool $initialized = false; /** * Provider constructor. diff --git a/tests/SP/Providers/Auth/AuthProviderTest.php b/tests/SP/Providers/Auth/AuthProviderTest.php new file mode 100644 index 00000000..df49b823 --- /dev/null +++ b/tests/SP/Providers/Auth/AuthProviderTest.php @@ -0,0 +1,101 @@ +. + */ + +namespace SP\Tests\Providers\Auth; + +use PHPUnit\Framework\MockObject\Exception; +use SP\DataModel\UserLoginData; +use SP\Domain\Auth\Services\AuthException; +use SP\Providers\Auth\AuthInterface; +use SP\Providers\Auth\AuthProvider; +use SP\Providers\Auth\AuthTypeEnum; +use SP\Providers\Auth\Browser\BrowserAuthData; +use SP\Tests\UnitaryTestCase; + +/** + * Class AuthProviderTest + * + * @group unitary + */ +class AuthProviderTest extends UnitaryTestCase +{ + + private AuthProvider $authProvider; + + /** + * @throws AuthException + * @throws Exception + */ + public function testRegisterAuthFail() + { + $auth1 = $this->createMock(AuthInterface::class); + + $this->authProvider->registerAuth($auth1, AuthTypeEnum::Ldap); + + $this->expectException(AuthException::class); + $this->expectExceptionMessage('Authentication already initialized'); + + $this->authProvider->registerAuth($auth1, AuthTypeEnum::Ldap); + } + + /** + * @throws AuthException + * @throws Exception + */ + public function testDoAuth() + { + $userLoginData = new UserLoginData(); + $userLoginData->setLoginUser(self::$faker->userName); + $userLoginData->setLoginPass(self::$faker->password); + + $browserAuthData = new BrowserAuthData(false); + $browserAuthData->setName(self::$faker->name); + $browserAuthData->setEmail(self::$faker->email); + $browserAuthData->setStatusCode(0); + $browserAuthData->success(); + + $auth = $this->createMock(AuthInterface::class); + $auth->expects(self::once()) + ->method('authenticate') + ->with($userLoginData) + ->willReturn($browserAuthData); + + $this->authProvider->registerAuth($auth, AuthTypeEnum::Ldap); + + $out = $this->authProvider->doAuth($userLoginData); + + self::assertCount(1, $out); + self::assertEquals(AuthTypeEnum::Ldap->value, $out[0]->getAuthName()); + self::assertEquals($browserAuthData, $out[0]->getData()); + } + + protected function setUp(): void + { + parent::setUp(); + + $this->authProvider = new AuthProvider($this->application); + } + + +} diff --git a/tests/SP/Providers/Auth/Ldap/LdapAuthTest.php b/tests/SP/Providers/Auth/Ldap/LdapAuthTest.php index 83d16482..223861e8 100644 --- a/tests/SP/Providers/Auth/Ldap/LdapAuthTest.php +++ b/tests/SP/Providers/Auth/Ldap/LdapAuthTest.php @@ -94,11 +94,20 @@ class LdapAuthTest extends UnitaryTestCase ->method('actions') ->willReturn($ldapActions); + $attributes = $this->buildAttributes(); + $attributes->set('expire', 0); + $ldapActions ->expects(self::once()) ->method('getAttributes') ->with($filter) - ->willReturn($this->buildAttributes()); + ->willReturn($attributes); + + $this->ldap + ->expects(self::once()) + ->method('isUserInGroup') + ->with($attributes->get('dn'), $userLoginData->getLoginUser(), $attributes->get('group')) + ->willReturn(true); $out = $this->ldapAuth->authenticate($userLoginData); @@ -125,6 +134,102 @@ class LdapAuthTest extends UnitaryTestCase ]); } + /** + * @throws Exception + */ + public function testAuthenticateWithExpireFail() + { + $userLoginData = new UserLoginData(); + $userLoginData->setLoginUser(self::$faker->userName); + $userLoginData->setLoginPass(self::$faker->password); + + $ldapActions = $this->createMock(LdapActionsInterface::class); + + $this->ldap + ->expects(self::once()) + ->method('connect') + ->with(null, null); + + $filter = 'test'; + + $this->ldap + ->expects(self::once()) + ->method('getUserDnFilter') + ->with($userLoginData->getLoginUser()) + ->willReturn($filter); + + $this->ldap + ->expects(self::once()) + ->method('actions') + ->willReturn($ldapActions); + + $attributes = $this->buildAttributes(); + + $ldapActions + ->expects(self::once()) + ->method('getAttributes') + ->with($filter) + ->willReturn($attributes); + + $this->ldap + ->expects(self::once()) + ->method('isUserInGroup') + ->with($attributes->get('dn'), $userLoginData->getLoginUser(), $attributes->get('group')) + ->willReturn(true); + + $out = $this->ldapAuth->authenticate($userLoginData); + + self::assertFalse($out->isOk()); + } + + /** + * @throws Exception + */ + public function testAuthenticateWithGroupFail() + { + $userLoginData = new UserLoginData(); + $userLoginData->setLoginUser(self::$faker->userName); + $userLoginData->setLoginPass(self::$faker->password); + + $ldapActions = $this->createMock(LdapActionsInterface::class); + + $this->ldap + ->expects(self::once()) + ->method('connect') + ->with(null, null); + + $filter = 'test'; + + $this->ldap + ->expects(self::once()) + ->method('getUserDnFilter') + ->with($userLoginData->getLoginUser()) + ->willReturn($filter); + + $this->ldap + ->expects(self::once()) + ->method('actions') + ->willReturn($ldapActions); + + $attributes = $this->buildAttributes(); + + $ldapActions + ->expects(self::once()) + ->method('getAttributes') + ->with($filter) + ->willReturn($attributes); + + $this->ldap + ->expects(self::once()) + ->method('isUserInGroup') + ->with($attributes->get('dn'), $userLoginData->getLoginUser(), $attributes->get('group')) + ->willReturn(false); + + $out = $this->ldapAuth->authenticate($userLoginData); + + self::assertFalse($out->isOk()); + } + /** * @throws Exception */