diff --git a/.gitattributes b/.gitattributes index 475f5f2..89eb187 100644 --- a/.gitattributes +++ b/.gitattributes @@ -4,3 +4,4 @@ /.gitignore export-ignore /.travis.yml export-ignore /.scrutinizer.yml export-ignore +/codecov.yml export-ignore diff --git a/.travis.yml b/.travis.yml index 2355d89..3ffecec 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,28 +1,34 @@ -# See https://github.com/silverstripe/silverstripe-travis-support for setup details - language: php -dist: precise - -sudo: false - -php: - - 5.6 - - 7.0 - - 7.1 - env: - - DB=MYSQL CORE_RELEASE=4 + global: + - COMPOSER_ROOT_VERSION="4.0.x-dev" + +matrix: + include: + - php: 5.6 + env: DB=MYSQL PHPCS_TEST=1 PHPUNIT_TEST=1 + - php: 7.0 + env: DB=PGSQL PHPUNIT_TEST=1 + - php: 7.1 + env: DB=MYSQL PHPUNIT_COVERAGE_TEST=1 before_install: - echo "extension=ldap.so" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini before_script: - - composer self-update || true - phpenv rehash - - git clone git://github.com/silverstripe/silverstripe-travis-support.git ~/travis-support - - php ~/travis-support/travis_setup.php --source `pwd` --target ~/builds/ss - - cd ~/builds/ss + - phpenv config-rm xdebug.ini + + - composer install --prefer-dist + - composer require --prefer-dist --no-update silverstripe/recipe-cms:1.0.x-dev + - if [[ $DB == PGSQL ]]; then composer require --prefer-dist --no-update silverstripe/postgresql:2.0.x-dev; fi + - composer update script: - - vendor/bin/phpunit activedirectory/tests/ + - if [[ $PHPUNIT_TEST ]]; then vendor/bin/phpunit tests/; fi + - if [[ $PHPUNIT_COVERAGE_TEST ]]; then phpdbg -qrr vendor/bin/phpunit --coverage-clover=coverage.xml; fi + - if [[ $PHPCS_TEST ]]; then vendor/bin/phpcs --standard=framework/phpcs.xml.dist src/ tests/ ; fi + +after_success: + - if [[ $PHPUNIT_COVERAGE_TEST ]]; then bash <(curl -s https://codecov.io/bash) -f coverage.xml; fi diff --git a/.upgrade.yml b/.upgrade.yml index 3172c14..3524e56 100644 --- a/.upgrade.yml +++ b/.upgrade.yml @@ -1,11 +1,10 @@ mappings: LDAPAuthenticator: SilverStripe\ActiveDirectory\Authenticators\LDAPAuthenticator - LDAPLoginForm: SilverStripe\ActiveDirectory\Authenticators\LDAPLoginForm + LDAPLoginForm: SilverStripe\ActiveDirectory\Forms\LDAPLoginForm SAMLAuthenticator: SilverStripe\ActiveDirectory\Authenticators\SAMLAuthenticator SAMLLoginForm: SilverStripe\ActiveDirectory\Authenticators\SAMLLoginForm SAMLSecurityExtension: SilverStripe\ActiveDirectory\Authenticators\SAMLSecurityExtension LDAPDebugController: SilverStripe\ActiveDirectory\Control\LDAPDebugController - LDAPSecurityController: SilverStripe\ActiveDirectory\Control\LDAPSecurityController SAMLController: SilverStripe\ActiveDirectory\Control\SAMLController LDAPGroupExtension: SilverStripe\ActiveDirectory\Extensions\LDAPGroupExtension LDAPMemberExtension: SilverStripe\ActiveDirectory\Extensions\LDAPMemberExtension diff --git a/_config.php b/_config.php index a4abe2d..b3d9bbc 100644 --- a/_config.php +++ b/_config.php @@ -1,2 +1 @@ =5.6", - "silverstripe/framework": "^4", - "silverstripe/cms": "^4", - "silverstripe/queuedjobs": "^4.0@dev", + "silverstripe/framework": "^4@dev", + "silverstripe/cms": "^4@dev", + "symbiote/silverstripe-queuedjobs": "^4@dev", "zendframework/zend-ldap": "^2.5.1", "zendframework/zend-authentication": "^2.5.1", "zendframework/zend-session": "^2.5.1", "onelogin/php-saml": "^2.10.7" }, "require-dev": { - "phpunit/phpunit": "~5.7" + "phpunit/phpunit": "^5.7", + "squizlabs/php_codesniffer": "^3.0" }, "extra": { "branch-alias": { @@ -34,7 +35,8 @@ }, "autoload": { "psr-4": { - "SilverStripe\\ActiveDirectory\\": "src/" + "SilverStripe\\ActiveDirectory\\": "src/", + "SilverStripe\\ActiveDirectory\\Tests\\": "tests/" } }, "minimum-stability": "dev", diff --git a/docs/en/developer.md b/docs/en/developer.md index 82d6f94..ba47405 100644 --- a/docs/en/developer.md +++ b/docs/en/developer.md @@ -135,33 +135,32 @@ ADFS should now be configured to extract the SP certificate from SilverStripe's ## Configure SilverStripe Authenticators -To be able to use the SAML or the LDAP authenticator you will need to set them up in the `mysite/_config.php`. +To be able to use the SAML or the LDAP authenticator you will need to set them up in the `mysite/_config/active-directory.yml`. You can choose which authenticators you would like to display on the login form. - // Show the SAML Login button on login form - \SilverStripe\Security\Authenticator::register_authenticator( - 'SilverStripe\\ActiveDirectory\\Authenticators\\SAMLAuthenticator' - ); - // Show the LDAP Login form - \SilverStripe\Security\Authenticator::register_authenticator( - 'SilverStripe\\ActiveDirectory\\Authenticators\\LDAPAuthenticator' - ); - -You can unregister the default authenticator by adding this line: - - \SilverStripe\Security\Authenticator::unregister('SilverStripe\\Security\\MemberAuthenticator'); +### Show the SAML Login button on login form +```yaml +SilverStripe\Core\Injector\Injector: + SilverStripe\Security\Security: + properties: + Authenticators: + default: %$SilverStripe\ActiveDirectory\Authenticators\SAMLAuthenticator +``` +### Show the LDAP Login button on login form +```yaml +SilverStripe\Core\Injector\Injector: + SilverStripe\Security\Security: + properties: + Authenticators: + default: %$SilverStripe\ActiveDirectory\Authenticators\LDAPAuthenticator +``` To prevent locking yourself out, before you remove the "MemberAuthenticator" make sure you map at least one LDAP group to the SilverStripe `Administrator` Security Group. Consult [CMS usage docs](usage.md) for how to do it. ### Bypass auto login If you register the SAMLAuthenticator as the default authenticator, it will automatically send users to the ADFS login server when they are required to login. - - \SilverStripe\Security\Authenticator::set_default_authenticator( - 'SilverStripe\\ActiveDirectory\\Authenticators\\SAMLAuthenticator' - ); - Should you need to access the login form with all the configured Authenticators, go to: /Security/login?showloginform=1 @@ -472,7 +471,7 @@ The fallback authenticator will be used in the following conditions: If the LDAP bind user that is configured under 'Connect with LDAP' section has permission to write attributes to the AD, it's possible to allow users to update their password via the internet site. -Word of caution, you will potentially open a security hole by exposing an AD user that can write passwords. Normally you would only bind to LDAP via a read-only user. Windows AD stores passwords in a hashed format that is very hard to brute-force. A user with write access can take over an accounts, create objects, delete and have access to all systems that authenticate with AD. +Word of caution, you will potentially open a security hole by exposing an AD user that can write passwords. Normally you would only bind to LDAP via a read-only user. Windows AD stores passwords in a hashed format that is very hard to brute-force. A user with write access can take over an account, create objects, delete and have access to all systems that authenticate with AD. If you still need this feature, we recommend that you use a combination of encryption, scheduled password rotation and limit permission for the bind user to minimum required permissions. diff --git a/phpunit.xml.dist b/phpunit.xml.dist new file mode 100644 index 0000000..5dd6854 --- /dev/null +++ b/phpunit.xml.dist @@ -0,0 +1,16 @@ + + + + tests + + + + + src/. + + tests/ + + + + + diff --git a/scripts/rotate_ldap_password.php b/scripts/rotate_ldap_password.php index 218b0d7..3b1d6c1 100644 --- a/scripts/rotate_ldap_password.php +++ b/scripts/rotate_ldap_password.php @@ -33,11 +33,11 @@ * rotate_ldap_password /sites/_ss_environment.php ldaps://otherldap.org PRIVILEGED_DN PRIVILEGED_PASSWORD TARGET_DN TARGET_PASSWORD */ -if(count($_SERVER['argv'])!=5 && count($_SERVER['argv'])!=7) { - echo "Not enough arguments.\n\n"; - echo "Usage (`c_' suffix denotes constants, not actual value):\n"; - echo " {$_SERVER['argv'][0]} [ ]\n"; - exit(1); +if (count($_SERVER['argv'])!=5 && count($_SERVER['argv'])!=7) { + echo "Not enough arguments.\n\n"; + echo "Usage (`c_' suffix denotes constants, not actual value):\n"; + echo " {$_SERVER['argv'][0]} [ ]\n"; + exit(1); } $envFilePath = $_SERVER['argv'][1]; @@ -46,22 +46,22 @@ $privilegedPasswordConst = $_SERVER['argv'][4]; if (empty($_SERVER['argv'][5])) { - // No target user specified, assume the privileged user's password is being changed. - $DNConst = $privilegedDNConst; - $passwordConst = $privilegedPasswordConst; + // No target user specified, assume the privileged user's password is being changed. + $DNConst = $privilegedDNConst; + $passwordConst = $privilegedPasswordConst; } else { - // Target specified for change. - $DNConst = $_SERVER['argv'][5]; - $passwordConst = $_SERVER['argv'][6]; + // Target specified for change. + $DNConst = $_SERVER['argv'][5]; + $passwordConst = $_SERVER['argv'][6]; } -if(!file_exists($envFilePath)) { - echo sprintf("%s does not exist\n", $envFilePath); - exit(1); +if (!file_exists($envFilePath)) { + echo sprintf("%s does not exist\n", $envFilePath); + exit(1); } -if(!is_writable($envFilePath)) { - echo sprintf("%s is not writable\n", $envFilePath); - exit(1); +if (!is_writable($envFilePath)) { + echo sprintf("%s is not writable\n", $envFilePath); + exit(1); } require_once($envFilePath); @@ -72,36 +72,36 @@ ldap_set_option($conn, LDAP_OPT_REFERRALS, 0); $bind = @ldap_bind($conn, constant($privilegedDNConst), constant($privilegedPasswordConst)); -if(!$bind) { - echo sprintf( - "Could not bind to LDAP: %s (error code: %s)\n", - ldap_error($conn), - ldap_errno($conn) - ); - exit(1); +if (!$bind) { + echo sprintf( + "Could not bind to LDAP: %s (error code: %s)\n", + ldap_error($conn), + ldap_errno($conn) + ); + exit(1); } // ldap_read searches the directory but uses LDAP_SCOPE_BASE, so we only get a single entry. // We're just doing this to validate that the entry exists in the directory. $query = ldap_read($conn, constant($DNConst), '(objectClass=user)', ['dn']); -if(!$query) { - echo sprintf( - "Could not read the LDAP entry %s: %s (error code: %s)\n", - constant($DNConst), - ldap_error($conn), - ldap_errno($conn) - ); - exit(1); +if (!$query) { + echo sprintf( + "Could not read the LDAP entry %s: %s (error code: %s)\n", + constant($DNConst), + ldap_error($conn), + ldap_errno($conn) + ); + exit(1); } $results = ldap_get_entries($conn, $query); -if(empty($results[0]['dn'])) { - echo sprintf( - "There was a problem retrieving the details for %s\n", - constant($DNConst) - ); - exit(1); +if (empty($results[0]['dn'])) { + echo sprintf( + "There was a problem retrieving the details for %s\n", + constant($DNConst) + ); + exit(1); } // Generate a sufficiently complex password and set that as the password in LDAP. @@ -109,29 +109,28 @@ // // With 24-character long password, this hardly ever loops. for (;;) { - $password = generatePassword(24); - if ( - preg_match('/[A-Z]/', $password) && - preg_match('/[a-z]/', $password) && - preg_match('/[0-9]/', $password) && - preg_match('/[\!\@\#\$\%\&\*\?]/', $password) - ) { - break; - } + $password = generatePassword(24); + if (preg_match('/[A-Z]/', $password) && + preg_match('/[a-z]/', $password) && + preg_match('/[0-9]/', $password) && + preg_match('/[\!\@\#\$\%\&\*\?]/', $password) + ) { + break; + } } $success = ldap_modify($conn, $results[0]['dn'], [ - 'unicodePwd' => iconv('UTF-8', 'UTF-16LE', sprintf('"%s"', $password)) + 'unicodePwd' => iconv('UTF-8', 'UTF-16LE', sprintf('"%s"', $password)) ]); -if(!$success) { - echo sprintf( - "Could not modify the LDAP entry %s: %s (error code: %s)\n", - constant($DNConst), - ldap_error($conn), - ldap_errno($conn) - ); - exit(1); +if (!$success) { + echo sprintf( + "Could not modify the LDAP entry %s: %s (error code: %s)\n", + constant($DNConst), + ldap_error($conn), + ldap_errno($conn) + ); + exit(1); } ldap_close($conn); @@ -142,40 +141,45 @@ file_put_contents($envFilePath, $contents); echo sprintf( - "LDAP password for %s has been reset. Old password has been replaced in %s\n", - $results[0]['dn'], - $envFilePath + "LDAP password for %s has been reset. Old password has been replaced in %s\n", + $results[0]['dn'], + $envFilePath ); exit(0); -function generatePassword($length = 24) { - $passwordData = null; +function generatePassword($length = 24) +{ + $passwordData = null; - if(function_exists('mcrypt_create_iv')) { - $e = mcrypt_create_iv(64, MCRYPT_DEV_URANDOM); - if($e !== false) $passwordData = $e; - } + if (function_exists('mcrypt_create_iv')) { + $e = mcrypt_create_iv(64, MCRYPT_DEV_URANDOM); + if ($e !== false) { + $passwordData = $e; + } + } - if(empty($passwordData) && function_exists('openssl_random_pseudo_bytes')) { - $e = openssl_random_pseudo_bytes(64, $strong); - // Only return if strong algorithm was used - if($strong) $passwordData = $e; - } + if (empty($passwordData) && function_exists('openssl_random_pseudo_bytes')) { + $e = openssl_random_pseudo_bytes(64, $strong); + // Only return if strong algorithm was used + if ($strong) { + $passwordData = $e; + } + } - if(empty($passwordData)) { - echo "Cannot generate passwords - PRNG functions missing? Try installing mcrypt."; - exit(1); - } + if (empty($passwordData)) { + echo "Cannot generate passwords - PRNG functions missing? Try installing mcrypt."; + exit(1); + } - // Convert the binary data into password-usable characters. - $usable = str_replace('=', '', base64_encode($passwordData)); + // Convert the binary data into password-usable characters. + $usable = str_replace('=', '', base64_encode($passwordData)); - // Re-map the base64 alphabet to include special password characters so we can satisfy all character classes. - $withSpecial = strtr($usable, '/+abcABC', '!@#$%&*?'); + // Re-map the base64 alphabet to include special password characters so we can satisfy all character classes. + $withSpecial = strtr($usable, '/+abcABC', '!@#$%&*?'); - // Trim to requested size. - $password = substr($withSpecial, 0, $length); + // Trim to requested size. + $password = substr($withSpecial, 0, $length); - return $password; + return $password; } diff --git a/src/Authenticators/LDAPAuthenticator.php b/src/Authenticators/LDAPAuthenticator.php index ec016b2..d6ff298 100644 --- a/src/Authenticators/LDAPAuthenticator.php +++ b/src/Authenticators/LDAPAuthenticator.php @@ -2,16 +2,18 @@ namespace SilverStripe\ActiveDirectory\Authenticators; +use SilverStripe\ActiveDirectory\Forms\LDAPLoginForm; use SilverStripe\ActiveDirectory\Services\LDAPService; use SilverStripe\Control\Controller; use SilverStripe\Control\Email\Email; -use SilverStripe\Control\Session; +use SilverStripe\Control\HTTPRequest; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Injector\Injector; -use SilverStripe\Forms\Form; +use SilverStripe\ORM\ValidationResult; use SilverStripe\Security\Authenticator; use SilverStripe\Security\Member; -use SilverStripe\Security\MemberAuthenticator; +use SilverStripe\Security\MemberAuthenticator\MemberAuthenticator; +use Zend\Authentication\Result; /** * Class LDAPAuthenticator @@ -22,7 +24,7 @@ * * @package activedirectory */ -class LDAPAuthenticator extends Authenticator +class LDAPAuthenticator extends MemberAuthenticator { /** * @var string @@ -71,76 +73,83 @@ public static function get_name() */ public static function get_login_form(Controller $controller) { - return new LDAPLoginForm($controller, 'LoginForm'); + return LDAPLoginForm::create($controller, LDAPAuthenticator::class, 'LoginForm'); } /** * Performs the login, but will also create and sync the Member record on-the-fly, if not found. * * @param array $data - * @param Form $form - * @return bool|Member|void - * @throws HTTPResponse_Exception + * @param HTTPRequest $request + * @param ValidationResult|null $result + * @return null|Member */ - public static function authenticate($data, Form $form = null) + public function authenticate(array $data, HTTPRequest $request, ValidationResult &$result = null) { + $result = $result ?: ValidationResult::create(); + /** @var LDAPService $service */ $service = Injector::inst()->get(LDAPService::class); $login = trim($data['Login']); if (Email::is_valid_address($login)) { if (Config::inst()->get(self::class, 'allow_email_login') != 'yes') { - $form->sessionMessage( + $result->addError( _t( - 'LDAPAuthenticator.PLEASEUSEUSERNAME', + __CLASS__ . '.PLEASEUSEUSERNAME', 'Please enter your username instead of your email to log in.' - ), - 'bad' + ) ); - return; + return null; } - $username = $service->getUsernameByEmail($login); // No user found with this email. if (!$username) { if (Config::inst()->get(self::class, 'fallback_authenticator') === 'yes') { - $fallbackMember = self::fallback_authenticate($data, $form); - if ($fallbackMember) { - return $fallbackMember; + if ($fallbackMember = $this->fallbackAuthenticate($data, $request)) { + { + return $fallbackMember; + } } } - $form->sessionMessage(_t('LDAPAuthenticator.INVALIDCREDENTIALS', 'Invalid credentials'), 'bad'); - return; + $result->addError(_t(__CLASS__ . '.INVALIDCREDENTIALS', 'Invalid credentials')); + return null; } } else { $username = $login; } - $result = $service->authenticate($username, $data['Password']); - $success = $result['success'] === true; + $serviceAuthenticationResult = $service->authenticate($username, $data['Password']); + $success = $serviceAuthenticationResult['success'] === true; + if (!$success) { + /* + * Try the fallback method if admin or it failed for anything other than invalid credentials + * This is to avoid having an unhandled exception error thrown by PasswordEncryptor::create_for_algorithm() + */ if (Config::inst()->get(self::class, 'fallback_authenticator') === 'yes') { - $fallbackMember = self::fallback_authenticate($data, $form); - if ($fallbackMember) { - return $fallbackMember; + if (!in_array($serviceAuthenticationResult['code'], [Result::FAILURE_CREDENTIAL_INVALID]) + || $username === 'admin' + ) { + if ($fallbackMember = $this->fallbackAuthenticate($data, $request)) { + return $fallbackMember; + } } } - if ($form) { - $form->sessionMessage($result['message'], 'bad'); - } - return; - } + $result->addError($serviceAuthenticationResult['message']); - $data = $service->getUserByUsername($result['identity']); + return null; + } + $data = $service->getUserByUsername($serviceAuthenticationResult['identity']); if (!$data) { - if ($form) { - $form->sessionMessage( - _t('LDAPAuthenticator.PROBLEMFINDINGDATA', 'There was a problem retrieving your user data'), - 'bad' - ); - } - return; + $result->addError( + _t( + __CLASS__ . '.PROBLEMFINDINGDATA', + 'There was a problem retrieving your user data' + ) + ); + return null; } // LDAPMemberExtension::memberLoggedIn() will update any other AD attributes mapped to Member fields @@ -152,9 +161,9 @@ public static function authenticate($data, Form $form = null) // Update the users from LDAP so we are sure that the email is correct. // This will also write the Member record. - $service->updateMemberFromLDAP($member); + $service->updateMemberFromLDAP($member, $data); - Session::clear('BackURL'); + $request->getSession()->clear('BackURL'); return $member; } @@ -163,18 +172,55 @@ public static function authenticate($data, Form $form = null) * Try to authenticate using the fallback authenticator. * * @param array $data - * @param null|Form $form + * @param HTTPRequest $request * @return null|Member */ - protected static function fallback_authenticate($data, Form $form = null) + protected function fallbackAuthenticate($data, HTTPRequest $request) + { + // Set Email from Login + if (array_key_exists('Login', $data) && !array_key_exists('Email', $data)) { + $data['Email'] = $data['Login']; + } + $authenticatorClass = Config::inst()->get(self::class, 'fallback_authenticator_class'); + if ($authenticator = Injector::inst()->get($authenticatorClass)) { + $result = call_user_func( + [ + $authenticator, + 'authenticate' + ], + $data, + $request + ); + return $result; + } + } + + public function getLoginHandler($link) + { + return LDAPLoginHandler::create($link, $this); + } + + public function supportedServices() + { + $result = Authenticator::LOGIN | Authenticator::LOGOUT | Authenticator::RESET_PASSWORD; + + if ((bool)LDAPService::config()->get('allow_password_change')) { + $result |= Authenticator::CHANGE_PASSWORD; + } + return $result; + } + + public function getLostPasswordHandler($link) + { + return LDAPLostPasswordHandler::create($link, $this); + } + + /** + * @param string $link + * @return LDAPChangePasswordHandler + */ + public function getChangePasswordHandler($link) { - return call_user_func( - [ - Config::inst()->get(self::class, 'fallback_authenticator_class'), - 'authenticate' - ], - array_merge($data, ['Email' => $data['Login']]), - $form - ); + return LDAPChangePasswordHandler::create($link, $this); } } diff --git a/src/Authenticators/LDAPChangePasswordHandler.php b/src/Authenticators/LDAPChangePasswordHandler.php new file mode 100644 index 0000000..96ad5fc --- /dev/null +++ b/src/Authenticators/LDAPChangePasswordHandler.php @@ -0,0 +1,164 @@ +get(LDAPService::class); + $member = Security::getCurrentUser(); + if ($member) { + try { + $userData = $service->getUserByGUID($member->GUID); + } catch (Exception $e) { + Injector::inst()->get(LoggerInterface::class)->error($e->getMessage()); + + $form->clearMessage(); + $form->sessionMessage( + _t( + __CLASS__ . '.NOUSER', + 'Your account hasn\'t been setup properly, please contact an administrator.' + ), + 'bad' + ); + return $form->getController()->redirect($form->getController()->Link('changepassword')); + } + $loginResult = $service->authenticate($userData['samaccountname'], $data['OldPassword']); + if (!$loginResult['success']) { + $form->clearMessage(); + $form->sessionMessage( + _t( + 'SilverStripe\\Security\\Member.ERRORPASSWORDNOTMATCH', + 'Your current password does not match, please try again' + ), + 'bad' + ); + // redirect back to the form, instead of using redirectBack() which could send the user elsewhere. + return $form->getController()->redirect($form->getController()->Link('changepassword')); + } + } + + if (!$member) { + if ($this->getRequest()->getSession()->get('AutoLoginHash')) { + $member = Member::member_from_autologinhash($this->getRequest()->getSession()->get('AutoLoginHash')); + } + + // The user is not logged in and no valid auto login hash is available + if (!$member) { + $this->getRequest()->getSession()->clear('AutoLoginHash'); + return $form->getController()->redirect($form->getController()->Link('login')); + } + } + + // Check the new password + if (empty($data['NewPassword1'])) { + $form->clearMessage(); + $form->sessionMessage( + _t( + 'SilverStripe\\Security\\Member.EMPTYNEWPASSWORD', + "The new password can't be empty, please try again" + ), + 'bad' + ); + + // redirect back to the form, instead of using redirectBack() which could send the user elsewhere. + return $form->getController()->redirect($form->getController()->Link('changepassword')); + } elseif ($data['NewPassword1'] == $data['NewPassword2']) { + // Providing OldPassword to perform password _change_ operation. This will respect the + // password history policy. Unfortunately we cannot support password history policy on password _reset_ + // at the moment, which means it will not be enforced on SilverStripe-driven email password reset. + $oldPassword = !empty($data['OldPassword']) ? $data['OldPassword']: null; + + /** @var ValidationResult $validationResult */ + $validationResult = $service->setPassword($member, $data['NewPassword1'], $oldPassword); + + // try to catch connection and other errors that the ldap service can through + if ($validationResult->isValid()) { + Security::setCurrentUser($member); + + $this->getRequest()->getSession()->clear('AutoLoginHash'); + + // Clear locked out status + $member->LockedOutUntil = null; + $member->FailedLoginCount = null; + $member->write(); + + if (!empty($this->getRequest()->requestVar('BackURL')) + // absolute redirection URLs may cause spoofing + && Director::is_site_url($this->getRequest()->requestVar('BackURL')) + ) { + $url = Director::absoluteURL($this->getRequest()->requestVar('BackURL')); + return $form->getController()->redirect($url); + } else { + // Redirect to default location - the login form saying "You are logged in as..." + $redirectURL = HTTP::setGetVar( + 'BackURL', + Director::absoluteBaseURL(), + $form->getController()->Link('login') + ); + return $form->getController()->redirect($redirectURL); + } + } else { + $form->clearMessage(); + $messages = implode('. ', array_column($validationResult->getMessages(), 'message')); + $form->sessionMessage($messages, 'bad'); + // redirect back to the form, instead of using redirectBack() which could send the user elsewhere. + return $form->getController()->redirect($form->getController()->Link('changepassword')); + } + } else { + $form->clearMessage(); + $form->sessionMessage( + _t( + 'SilverStripe\\Security\\Member.ERRORNEWPASSWORD', + 'You have entered your new password differently, try again' + ), + 'bad' + ); + + // redirect back to the form, instead of using redirectBack() which could send the user elsewhere. + return $form->getController()->redirect($form->getController()->Link('changepassword')); + } + } +} diff --git a/src/Authenticators/LDAPLoginHandler.php b/src/Authenticators/LDAPLoginHandler.php new file mode 100644 index 0000000..3642b71 --- /dev/null +++ b/src/Authenticators/LDAPLoginHandler.php @@ -0,0 +1,26 @@ +authenticator), + 'LoginForm' + ); + } +} diff --git a/src/Authenticators/LDAPLostPasswordHandler.php b/src/Authenticators/LDAPLostPasswordHandler.php new file mode 100644 index 0000000..abc2b97 --- /dev/null +++ b/src/Authenticators/LDAPLostPasswordHandler.php @@ -0,0 +1,223 @@ +link = $link; + $this->authenticatorClass = get_class($authenticator); + parent::__construct($link); + } + + /** + * Forgot password form handler method. + * + * Called when the user clicks on "I've lost my password". + * + * Extensions can use the 'forgotPassword' method to veto executing + * the logic, by returning FALSE. In this case, the user will be redirected back + * to the form without further action. It is recommended to set a message + * in the form detailing why the action was denied. + * + * @param array $data Submitted data + * @param LostPasswordForm $form + * @return HTTPResponse + */ + public function forgotPassword($data, $form) + { + /** @var Controller $controller */ + $controller = $form->getController(); + + // No need to protect against injections, LDAPService will ensure that this is safe + $login = trim($data['Login']); + + $service = Injector::inst()->get(LDAPService::class); + if (Email::is_valid_address($login)) { + if (Config::inst()->get(LDAPAuthenticator::class, 'allow_email_login') != 'yes') { + $form->sessionMessage( + _t( + 'SilverStripe\\ActiveDirectory\\Forms\\LDAPLoginForm.USERNAMEINSTEADOFEMAIL', + 'Please enter your username instead of your email to get a password reset link.' + ), + 'bad' + ); + return $controller->redirect($controller->Link('lostpassword')); + } + $userData = $service->getUserByEmail($login); + } else { + $userData = $service->getUserByUsername($login); + } + // Avoid information disclosure by displaying the same status, + // regardless whether the email address actually exists + if (!isset($userData['objectguid'])) { + return $controller->redirect($controller->Link('passwordsent/') + . urlencode($data['Login'])); + } + + $member = Member::get()->filter('GUID', $userData['objectguid'])->limit(1)->first(); + // User haven't been imported yet so do that now + if (!($member && $member->exists())) { + $member = new Member(); + $member->GUID = $userData['objectguid']; + } + + // Update the users from LDAP so we are sure that the email is correct. + // This will also write the Member record. + $service->updateMemberFromLDAP($member, $userData, false); + + // Allow vetoing forgot password requests + $results = $this->extend('forgotPassword', $member); + if ($results && is_array($results) && in_array(false, $results, true)) { + return $controller->redirect('lostpassword'); + } + + if ($member) { + /** @see MemberLoginForm::forgotPassword */ + $token = $member->generateAutologinTokenAndStoreHash(); + $e = Email::create() + ->setSubject( + _t( + 'Silverstripe\\Security\\Member.SUBJECTPASSWORDRESET', + 'Your password reset link', + 'Email subject' + ) + ) + ->setHTMLTemplate('SilverStripe\\Control\\Email\\ForgotPasswordEmail') + ->setData($member) + ->setData(['PasswordResetLink' => Security::getPasswordResetLink($member, $token)]); + $e->setTo($member->Email); + $e->send(); + return $controller->redirect($controller->Link('passwordsent/') . urlencode($data['Login'])); + } elseif ($data['Login']) { + // Avoid information disclosure by displaying the same status, + // regardless whether the email address actually exists + return $controller->redirect($controller->Link('passwordsent/') . urlencode($data['Login'])); + } else { + if (Config::inst()->get(LDAPAuthenticator::class, 'allow_email_login') === 'yes') { + $form->sessionMessage( + _t( + 'SilverStripe\\ActiveDirectory\\Forms\\LDAPLoginForm.ENTERUSERNAMEOREMAIL', + 'Please enter your username or your email address to get a password reset link.' + ), + 'bad' + ); + } else { + $form->sessionMessage( + _t( + 'SilverStripe\\ActiveDirectory\\Forms\\LDAPLoginForm.ENTERUSERNAME', + 'Please enter your username to get a password reset link.' + ), + 'bad' + ); + } + return $controller->redirect($controller->Link('lostpassword')); + } + } + + /** + * Factory method for the lost password form + * + * @return Form Returns the lost password form + */ + public function lostPasswordForm() + { + $loginFieldLabel = (Config::inst()->get(LDAPAuthenticator::class, 'allow_email_login') === 'yes') ? + _t('SilverStripe\\ActiveDirectory\\Forms\\LDAPLoginForm.USERNAMEOREMAIL', 'Username or email') : + _t('SilverStripe\\ActiveDirectory\\Forms\\LDAPLoginForm.USERNAME', 'Username'); + $loginField = TextField::create('Login', $loginFieldLabel); + + $action = FormAction::create( + 'forgotPassword', + _t('SilverStripe\\Security\\Security.BUTTONSEND', 'Send me the password reset link') + ); + return LostPasswordForm::create( + $this, + $this->authenticatorClass, + 'LostPasswordForm', + FieldList::create([$loginField]), + FieldList::create([$action]), + false + ); + } + + public function lostpassword() + { + if (Config::inst()->get(LDAPAuthenticator::class, 'allow_email_login') === 'yes') { + $message = _t( + __CLASS__ . '.NOTERESETPASSWORDUSERNAMEOREMAIL', + 'Enter your username or your email address and we will send you a link with which ' + . 'you can reset your password' + ); + } else { + $message = _t( + __CLASS__ . '.NOTERESETPASSWORDUSERNAME', + 'Enter your username and we will send you a link with which you can reset your password' + ); + } + + return [ + 'Content' => DBField::create_field('HTMLFragment', "

$message

"), + 'Form' => $this->lostPasswordForm(), + ]; + } + + public function passwordsent() + { + $username = Convert::raw2xml( + rawurldecode($this->getRequest()->param('OtherID')) + ); + $username .= ($extension = $this->request->getExtension()) ? '.' . $extension : ''; + + return [ + 'Title' => _t( + __CLASS__ . '.PASSWORDSENTHEADER', + "Password reset link sent to '{username}'", + ['username' => $username] + ), + 'Content' => + _t( + __CLASS__ . '.PASSWORDSENTTEXT', + "Thank you! A reset link has been sent to '{username}', provided an account exists.", + ['username' => $username] + ), + 'Username' => $username + ]; + } +} diff --git a/src/Authenticators/LDAPMemberLoginHandler.php b/src/Authenticators/LDAPMemberLoginHandler.php index a3eab3f..3fc3a5a 100644 --- a/src/Authenticators/LDAPMemberLoginHandler.php +++ b/src/Authenticators/LDAPMemberLoginHandler.php @@ -6,117 +6,12 @@ use SilverStripe\Control\Email\Email; use SilverStripe\Core\Injector\Injector; use SilverStripe\Security\Member; -use SilverStripe\Security\MemberLoginHandler; +use SilverStripe\Security\MemberAuthenticator\LoginHandler; -class LDAPMemberLoginHandler extends MemberLoginHandler +class LDAPMemberLoginHandler extends LoginHandler { /** * @var string */ protected $authenticator_class = LDAPAuthenticator::class; - - /** - * Forgot password form handler method. - * - * Called when the user clicks on "I've lost my password". - * - * Extensions can use the 'forgotPassword' method to veto executing - * the logic, by returning FALSE. In this case, the user will be redirected back - * to the form without further action. It is recommended to set a message - * in the form detailing why the action was denied. - * - * Overridden because we need to generate a link to the LDAPSecurityController - * instead of the SecurityController - * - * @param array $data Submitted data - * @return SS_HTTPResponse - */ - public function forgotPassword($data) - { - /** @var Controller $controller */ - $controller = $this->form->getController(); - - // No need to protect against injections, LDAPService will ensure that this is safe - $login = trim($data['Login']); - - $service = Injector::inst()->get(LDAPService::class); - if (Email::is_valid_address($login)) { - if (Config::inst()->get(LDAPAuthenticator::class, 'allow_email_login') != 'yes') { - $this->sessionMessage( - _t( - 'LDAPLoginForm.USERNAMEINSTEADOFEMAIL', - 'Please enter your username instead of your email to get a password reset link.' - ), - 'bad' - ); - $controller->redirect($controller->Link('lostpassword')); - return; - } - $userData = $service->getUserByEmail($login); - } else { - $userData = $service->getUserByUsername($login); - } - - // Avoid information disclosure by displaying the same status, - // regardless whether the email address actually exists - if (!isset($userData['objectguid'])) { - return $controller->redirect($controller->Link('passwordsent/') - . urlencode($data['Login'])); - } - - $member = Member::get()->filter('GUID', $userData['objectguid'])->limit(1)->first(); - // User haven't been imported yet so do that now - if (!($member && $member->exists())) { - $member = new Member(); - $member->GUID = $userData['objectguid']; - } - - // Update the users from LDAP so we are sure that the email is correct. - // This will also write the Member record. - $service->updateMemberFromLDAP($member); - - // Allow vetoing forgot password requests - $results = $this->extend('forgotPassword', $member); - if ($results && is_array($results) && in_array(false, $results, true)) { - return $controller->redirect($this->ldapSecController->Link('lostpassword')); - } - - if ($member) { - /** @see MemberLoginForm::forgotPassword */ - $token = $member->generateAutologinTokenAndStoreHash(); - $e = Email::create(); - $e->setSubject(_t('Member.SUBJECTPASSWORDRESET', 'Your password reset link', 'Email subject')); - $e->setTemplate('ForgotPasswordEmail'); - $e->populateTemplate($member); - $e->populateTemplate([ - 'PasswordResetLink' => LDAPSecurityController::getPasswordResetLink($member, $token) - ]); - $e->setTo($member->Email); - $e->send(); - $controller->redirect($controller->Link('passwordsent/') . urlencode($data['Login'])); - } elseif ($data['Login']) { - // Avoid information disclosure by displaying the same status, - // regardless whether the email address actually exists - $controller->redirect($controller->Link('passwordsent/') . urlencode($data['Login'])); - } else { - if (Config::inst()->get(LDAPAuthenticator::class, 'allow_email_login') === 'yes') { - $this->sessionMessage( - _t( - 'LDAPLoginForm.ENTERUSERNAMEOREMAIL', - 'Please enter your username or your email address to get a password reset link.' - ), - 'bad' - ); - } else { - $this->sessionMessage( - _t( - 'LDAPLoginForm.ENTERUSERNAME', - 'Please enter your username to get a password reset link.' - ), - 'bad' - ); - } - $controller->redirect($controller->Link('lostpassword')); - } - } } diff --git a/src/Authenticators/SAMLAuthenticator.php b/src/Authenticators/SAMLAuthenticator.php index e6be851..117af0f 100644 --- a/src/Authenticators/SAMLAuthenticator.php +++ b/src/Authenticators/SAMLAuthenticator.php @@ -5,11 +5,15 @@ use SilverStripe\ActiveDirectory\Helpers\SAMLHelper; use SilverStripe\Control\Controller; use Silverstripe\Control\Director; +use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\Session; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Injector\Injector; use SilverStripe\Forms\Form; +use SilverStripe\ORM\ValidationResult; use SilverStripe\Security\Authenticator; +use SilverStripe\Security\Member; +use SilverStripe\Security\MemberAuthenticator\MemberAuthenticator; /** * Class SAMLAuthenticator @@ -27,7 +31,7 @@ * * @package activedirectory */ -class SAMLAuthenticator extends Authenticator +class SAMLAuthenticator extends MemberAuthenticator { /** * @var string @@ -57,16 +61,32 @@ public static function get_login_form(Controller $controller) * will be delivered to the SAMLController::acs. * * @param array $data - * @param Form $form + * @param HTTPRequest $request + * @param ValidationResult|null $result * @return bool|Member|void - * @throws SS_HTTPResponse_Exception */ - public static function authenticate($data, Form $form = null) + public function authenticate(array $data, HTTPRequest $request, ValidationResult &$result = null) { // $data is not used - the form is just one button, with no fields. $auth = Injector::inst()->get(SAMLHelper::class)->getSAMLAuth(); - Session::set('BackURL', isset($data['BackURL']) ? $data['BackURL'] : null); - Session::save(); + $request->getSession()->set('BackURL', isset($data['BackURL']) ? $data['BackURL'] : null); + $request->getSession()->save($request); $auth->login(Director::absoluteBaseURL().'saml/'); } + + /** + * @inheritdoc + */ + public function getLoginHandler($link) + { + return SAMLLoginHandler::create($link, $this); + } + + /** + * @inheritdoc + */ + public function supportedServices() + { + return Authenticator::LOGIN | Authenticator::LOGOUT; + } } diff --git a/src/Authenticators/SAMLLoginForm.php b/src/Authenticators/SAMLLoginForm.php index 4981c77..2382dce 100644 --- a/src/Authenticators/SAMLLoginForm.php +++ b/src/Authenticators/SAMLLoginForm.php @@ -2,12 +2,11 @@ namespace SilverStripe\ActiveDirectory\Authenticators; -use SilverStripe\Control\Session; +use SilverStripe\Control\RequestHandler; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\HiddenField; use SilverStripe\Forms\FormAction; use SilverStripe\Security\LoginForm; -use SilverStripe\Security\Member; use SilverStripe\Security\Security; /** @@ -30,42 +29,44 @@ class SAMLLoginForm extends LoginForm */ protected $authenticator_class = SAMLAuthenticator::class; + /** + * The name of this login form, to display in the frontend + * Replaces Authenticator::get_name() + * + * @return string + */ public function getAuthenticatorName() { - return "SAML"; + return _t(__CLASS__ . '.AUTHENTICATORNAME', 'SAML'); } /** * Constructor * - * @param Controller $controller + * @param RequestHandler $controller + * @param string $authenticatorClass * @param string $name method on the $controller - * @param FieldList $fields - * @param FieldList $actions - * @param bool $checkCurrentUser - show logout button if logged in */ - public function __construct($controller, $name, $fields = null, $actions = null, $checkCurrentUser = true) + public function __construct(RequestHandler $controller, $authenticatorClass, $name) { - $backURL = Session::get('BackURL'); + $backURL = $this->getSession()->get('BackURL'); - if (isset($_REQUEST['BackURL'])) { - $backURL = $_REQUEST['BackURL']; + if (!empty($this->getRequest()->requestVar('BackURL'))) { + $backURL = $this->getRequest()->requestVar('BackURL'); } - - if ($checkCurrentUser && $this->shouldShowLogoutFields()) { + if ($this->shouldShowLogoutFields()) { $fields = FieldList::create([ HiddenField::create('AuthenticationMethod', null, $this->authenticator_class, $this) ]); $actions = FieldList::create([ - FormAction::create('logout', _t('Member.BUTTONLOGINOTHER', 'Log in as someone else')) + FormAction::create( + 'logout', + _t('SilverStripe\\Security\\Member.BUTTONLOGINOTHER', 'Log in as someone else') + ) ]); } else { - if (!$fields) { - $fields = $this->getFormFields(); - } - if (!$actions) { - $actions = $this->getFormActions(); - } + $fields = $this->getFormFields(); + $actions = $this->getFormActions(); } if ($backURL) { @@ -87,7 +88,7 @@ protected function getFormFields() protected function getFormActions() { return FieldList::create([ - FormAction::create('dologin', _t('Member.BUTTONLOGIN', 'Log in')) + FormAction::create('dologin', _t('SilverStripe\\Security\\Member.BUTTONLOGIN', 'Log in')) ]); } @@ -96,59 +97,9 @@ protected function getFormActions() */ protected function shouldShowLogoutFields() { - if (!Member::currentUser()) { - return false; - } - if (!Member::logged_in_session_exists()) { + if (!Security::getCurrentUser()) { return false; } return true; } - - /** - * Get message from session - */ - protected function getMessageFromSession() - { - // The "MemberLoginForm.force_message session" is set in Security#permissionFailure() - // and displays messages like "You don't have access to this page" - // if force isn't set, it will just display "You're logged in as {name}" - if (($member = Member::currentUser()) && !Session::get('MemberLoginForm.force_message')) { - $this->message = _t( - 'Member.LOGGEDINAS', - "You're logged in as {name}.", - ['name' => $member->{$this->loggedInAsField}] - ); - } - Session::set('MemberLoginForm.force_message', false); - parent::getMessageFromSession(); - return $this->message; - } - - /** - * Login form handler method - * - * This method is called when the user clicks on "Log in" - * - * @param array $data Submitted data - */ - public function dologin($data) - { - call_user_func_array([$this->authenticator_class, 'authenticate'], [$data, $this]); - } - - - /** - * Log out form handler method - * - * This method is called when the user clicks on "logout" on the form - * created when the parameter $checkCurrentUser of the - * {@link __construct constructor} was set to TRUE and the user was - * currently logged in. - */ - public function logout() - { - $s = new Security(); - $s->logout(false); - } } diff --git a/src/Authenticators/SAMLLoginHandler.php b/src/Authenticators/SAMLLoginHandler.php new file mode 100644 index 0000000..fc8d436 --- /dev/null +++ b/src/Authenticators/SAMLLoginHandler.php @@ -0,0 +1,17 @@ +authenticator), + 'LoginForm' + ); + } +} diff --git a/src/Authenticators/SAMLSecurityExtension.php b/src/Authenticators/SAMLSecurityExtension.php index bb86e66..d0e74c6 100644 --- a/src/Authenticators/SAMLSecurityExtension.php +++ b/src/Authenticators/SAMLSecurityExtension.php @@ -7,6 +7,7 @@ use SilverStripe\Core\Injector\Injector; use SilverStripe\Security\Authenticator; use SilverStripe\Security\Member; +use SilverStripe\Security\Security; /** * Class SAMLSecurityExtension @@ -20,36 +21,31 @@ class SAMLSecurityExtension extends Extension /** * Will redirect the user directly to the IdP login endpoint if: * - * 1) the 'SAMLAuthenticator' is the default authenticator - * 2) there isn't a GET param showloginform set to 1 - * 3) the member is not currently logged in - * 4) there are no form messages (errors or notices) + * 1) There isn't a GET param showloginform set to 1 + * 2) the member is not currently logged in + * 3) there are no form messages (errors or notices) * * @return void */ public function onBeforeSecurityLogin() { - if (Authenticator::get_default_authenticator() != SAMLAuthenticator::class) { - return; - } - // by going to the URL Security/login?showloginform=1 we bypass the auto sign on if ($this->owner->request->getVar('showloginform') == 1) { return; } // if member is already logged in, don't auto-sign-on, this is most likely because - // of unsufficient permissions. - $member = Member::currentUser(); + // of insufficient permissions. + $member = Security::getCurrentUser(); if ($member && $member->exists()) { return; } - + $session = $this->owner->getRequest()->getSession(); // if there are form messages, don't auto-sign-on, this is most likely because of // login errors / failures or other notices. - if (Session::get('FormInfo')) { + if ($session->get('FormInfo')) { // since FormInfo can be a "nulled" array, we have to check - foreach (Session::get('FormInfo') as $form => $info) { + foreach ($session->get('FormInfo') as $form => $info) { foreach ($info as $name => $value) { if ($value !== null) { return; @@ -58,12 +54,11 @@ public function onBeforeSecurityLogin() } } - $backURL = Session::get('BackURL'); + $backURL = $session->get('BackURL'); if ($this->owner->request->getVar('BackURL')) { $backURL = $this->owner->request->getVar('BackURL'); } - $authenticator = Injector::inst()->create(SAMLAuthenticator::class); - $authenticator->authenticate(['BackURL' => $backURL]); + $this->owner->getRequest()->getSession()->set('BackURL', $backURL); } } diff --git a/src/Control/LDAPDebugController.php b/src/Control/LDAPDebugController.php index bb2e820..58d2363 100644 --- a/src/Control/LDAPDebugController.php +++ b/src/Control/LDAPDebugController.php @@ -118,7 +118,8 @@ public function DefaultGroup() $group = Group::get()->filter('Code', $code)->limit(1)->first(); if (!($group && $group->exists())) { return sprintf( - 'WARNING: LDAPService.default_group configured with \'%s\' but there is no Group with that Code in the database!', + 'WARNING: LDAPService.default_group configured with \'%s\'' + .'but there is no Group with that Code in the database!', $code ); } else { diff --git a/src/Control/LDAPSecurityController.php b/src/Control/LDAPSecurityController.php deleted file mode 100644 index bf81400..0000000 --- a/src/Control/LDAPSecurityController.php +++ /dev/null @@ -1,159 +0,0 @@ -Link('changepassword') . "?m={$member->ID}&t=$autologinToken"; - } - - /** - * Factory method for the lost password form - * - * @return Form Returns the lost password form - */ - public function ChangePasswordForm() - { - return Object::create('SilverStripe\\ActiveDirectory\\Forms\\LDAPChangePasswordForm', $this, 'ChangePasswordForm'); - } - - public function lostpassword() - { - $controller = $this->getResponseController(_t('LDAPSecurityController.LOSTPASSWORDHEADER', 'Lost password')); - - // if the controller calls Director::redirect(), this will break early - if (($response = $controller->getResponse()) && $response->isFinished()) { - return $response; - } - - if (Config::inst()->get('SilverStripe\\ActiveDirectory\\Authenticators\\LDAPAuthenticator', 'allow_email_login') === 'yes') { - $customisedController = $controller->customise([ - 'Content' => - _t( - 'LDAPSecurityController.NOTERESETPASSWORDUSERNAMEOREMAIL', - 'Enter your username or your email address and we will send you a link with which ' - . 'you can reset your password' - ), - 'Form' => $this->LostPasswordForm(), - ]); - } else { - $customisedController = $controller->customise([ - 'Content' => - _t( - 'LDAPSecurityController.NOTERESETPASSWORDUSERNAME', - 'Enter your username and we will send you a link with which you can reset your password' - ), - 'Form' => $this->LostPasswordForm(), - ]); - } - - //Controller::$currentController = $controller; - return $customisedController->renderWith($this->getTemplatesFor('lostpassword')); - } - - /** - * Factory method for the lost password form - * - * @return Form Returns the lost password form - */ - public function LostPasswordForm() - { - $email = EmailField::create('Email', _t('Member.EMAIL', 'Email')); - $action = FormAction::create('forgotPassword', _t('Security.BUTTONSEND', 'Send me the password reset link')); - return LDAPLoginForm::create( - $this, - 'LostPasswordForm', - FieldList::create([$email]), - FieldList::create([$action]), - false - ); - } - - /** - * @param null $action - * @return String - */ - public function Link($action = null) - { - return Controller::join_links(Director::baseURL(), 'LDAPSecurity', $action); - } - - /** - * Show the "password sent" page, after a user has requested - * to reset their password. - * - * @param SS_HTTPRequest $request The SS_HTTPRequest for this action. - * @return string Returns the "password sent" page as HTML code. - */ - public function passwordsent($request) - { - $controller = $this->getResponseController(_t('Security.LOSTPASSWORDHEADER', 'Lost Password')); - - // if the controller calls Director::redirect(), this will break early - if (($response = $controller->getResponse()) && $response->isFinished()) { - return $response; - } - - $username = Convert::raw2xml(rawurldecode($request->param('ID'))); - - $customisedController = $controller->customise([ - 'Title' => _t( - 'LDAPSecurity.PASSWORDSENTHEADER', - "Password reset link sent to '{username}'", - ['username' => $username] - ), - 'Content' => - _t( - 'LDAPSecurity.PASSWORDSENTTEXT', - "Thank you! A reset link has been sent to '{username}', provided an account exists.", - ['username' => $username] - ), - 'Username' => $username - ]); - return $customisedController->renderWith($this->getTemplatesFor('passwordsent')); - } -} diff --git a/src/Control/SAMLController.php b/src/Control/SAMLController.php index 2ce2d25..de129a1 100644 --- a/src/Control/SAMLController.php +++ b/src/Control/SAMLController.php @@ -4,13 +4,15 @@ use Exception; use OneLogin_Saml2_Error; +use Psr\Log\LoggerInterface; +use SilverStripe\ActiveDirectory\Authenticators\SAMLLoginForm; +use SilverStripe\ActiveDirectory\Helpers\SAMLHelper; use SilverStripe\ActiveDirectory\Model\LDAPUtil; use SilverStripe\Control\Controller; use SilverStripe\Control\Director; -use SilverStripe\Control\Session; +use SilverStripe\Control\HTTPResponse; use SilverStripe\Core\Injector\Injector; -use SilverStripe\Forms\Form; -use SilverStirpe\Security\Member; +use SilverStripe\Security\Member; use SilverStripe\Security\Security; /** @@ -49,28 +51,29 @@ class SAMLController extends Controller */ public function acs() { - $auth = Injector::inst()->get('SilverStripe\\ActiveDirectory\\Helpers\\SAMLHelper')->getSAMLAuth(); + $auth = Injector::inst()->get(SAMLHelper::class)->getSAMLAuth(); $auth->processResponse(); $error = $auth->getLastErrorReason(); if (!empty($error)) { $this->getLogger()->error($error); - Form::messageForForm('SAMLLoginForm_LoginForm', "Authentication error: '{$error}'", 'bad'); - Session::save(); + + $this->getForm()->sessionMessage("Authentication error: '{$error}'", 'bad'); + $this->getRequest()->getSession()->save($this->getRequest()); return $this->getRedirect(); } if (!$auth->isAuthenticated()) { - Form::messageForForm('SAMLLoginForm_LoginForm', _t('Member.ERRORWRONGCRED'), 'bad'); - Session::save(); + $this->getForm()->sessionMessage(_t('SilverStripe\\Security\\Member.ERRORWRONGCRED'), 'bad'); + $this->getRequest()->getSession()->save($this->getRequest()); return $this->getRedirect(); } $decodedNameId = base64_decode($auth->getNameId()); // check that the NameID is a binary string (which signals that it is a guid if (ctype_print($decodedNameId)) { - Form::messageForForm('SAMLLoginForm_LoginForm', 'Name ID provided by IdP is not a binary GUID.', 'bad'); - Session::save(); + $this->getForm()->sessionMessage('Name ID provided by IdP is not a binary GUID.', 'bad'); + $this->getRequest()->getSession()->save($this->getRequest()); return $this->getRedirect(); } @@ -79,8 +82,8 @@ public function acs() if (!LDAPUtil::validGuid($guid)) { $errorMessage = "Not a valid GUID '{$guid}' recieved from server."; $this->getLogger()->error($errorMessage); - Form::messageForForm('SAMLLoginForm_LoginForm', $errorMessage, 'bad'); - Session::save(); + $this->getForm()->sessionMessage($errorMessage, 'bad'); + $this->getRequest()->getSession()->save($this->getRequest()); return $this->getRedirect(); } @@ -96,9 +99,10 @@ public function acs() foreach ($member->config()->claims_field_mappings as $claim => $field) { if (!isset($attributes[$claim][0])) { - $this->getLogger()->warn( + $this->getLogger()->warning( sprintf( - 'Claim rule \'%s\' configured in LDAPMember.claims_field_mappings, but wasn\'t passed through. Please check IdP claim rules.', + 'Claim rule \'%s\' configured in LDAPMember.claims_field_mappings, ' . + 'but wasn\'t passed through. Please check IdP claim rules.', $claim ) ); @@ -116,7 +120,7 @@ public function acs() // calling this, as any onAfterWrite hooks that attempt to update LDAP won't // have the Username field available yet for new Member records, and fail. // Both SAML and LDAP identify Members by the GUID field. - $member->logIn(); + Security::setCurrentUser($member); return $this->getRedirect(); } @@ -149,17 +153,19 @@ public function metadata() } /** - * @return SS_HTTPResponse + * @return HTTPResponse */ protected function getRedirect() { // Absolute redirection URLs may cause spoofing - if (Session::get('BackURL') && Director::is_site_url(Session::get('BackURL'))) { - return $this->redirect(Session::get('BackURL')); + if ($this->getRequest()->getSession()->get('BackURL') + && Director::is_site_url($this->getRequest()->getSession()->get('BackURL'))) { + return $this->redirect($this->getRequest()->getSession()->get('BackURL')); } // Spoofing attack, redirect to homepage instead of spoofing url - if (Session::get('BackURL') && !Director::is_site_url(Session::get('BackURL'))) { + if ($this->getRequest()->getSession()->get('BackURL') + && !Director::is_site_url($this->getRequest()->getSession()->get('BackURL'))) { return $this->redirect(Director::absoluteBaseURL()); } @@ -175,10 +181,20 @@ protected function getRedirect() /** * Get a logger * - * @return Psr\Log\LoggerInterface + * @return LoggerInterface */ public function getLogger() { - return Injector::inst()->get('Logger'); + return Injector::inst()->get(LoggerInterface::class); + } + + /** + * Gets the login form + * + * @return SAMLLoginForm + */ + public function getForm() + { + return Injector::inst()->get(SAMLLoginForm::class); } } diff --git a/src/Extensions/LDAPGroupExtension.php b/src/Extensions/LDAPGroupExtension.php index 8af9f9e..932938c 100644 --- a/src/Extensions/LDAPGroupExtension.php +++ b/src/Extensions/LDAPGroupExtension.php @@ -64,7 +64,7 @@ public function updateCMSFields(FieldList $fields) 'Root.LDAP', ReadonlyField::create( 'LastSynced', - _t('LDAPGroupExtension.LASTSYNCED', 'Last synced') + _t(__CLASS__ . '.LASTSYNCED', 'Last synced') ) ); @@ -75,7 +75,7 @@ public function updateCMSFields(FieldList $fields) $fields->addFieldToTab('Root.Members', ReadonlyField::create('Code'), 'Members'); $message = _t( - 'LDAPGroupExtension.INFOIMPORTED', + __CLASS__ . '.INFOIMPORTED', 'This group is automatically imported from LDAP.' ); $fields->addFieldToTab( @@ -89,7 +89,7 @@ public function updateCMSFields(FieldList $fields) $fields->addFieldToTab('Root.LDAP', ReadonlyField::create( 'LDAPGroupMappingsRO', - _t('LDAPGroupExtension.AUTOMAPPEDGROUPS', 'Automatically mapped LDAP Groups'), + _t(__CLASS__ . '.AUTOMAPPEDGROUPS', 'Automatically mapped LDAP Groups'), implode('; ', $this->owner->LDAPGroupMappings()->column('DN')) )); } else { @@ -100,7 +100,7 @@ public function updateCMSFields(FieldList $fields) ); $config = GridFieldConfig_RecordEditor::create(); $config->getComponentByType('GridFieldAddNewButton') - ->setButtonName(_t('LDAPGroupExtension.ADDMAPPEDGROUP', 'Add LDAP group mapping')); + ->setButtonName(_t(__CLASS__ . '.ADDMAPPEDGROUP', 'Add LDAP group mapping')); $field->setConfig($config); $fields->addFieldToTab('Root.LDAP', $field); diff --git a/src/Extensions/LDAPMemberExtension.php b/src/Extensions/LDAPMemberExtension.php index f8a8e14..d4caf91 100644 --- a/src/Extensions/LDAPMemberExtension.php +++ b/src/Extensions/LDAPMemberExtension.php @@ -2,6 +2,8 @@ namespace SilverStripe\ActiveDirectory\Extensions; +use Exception; +use SilverStripe\ActiveDirectory\Services\LDAPService; use SilverStripe\Core\Injector\Injector; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\LiteralField; @@ -104,14 +106,14 @@ public function updateCMSFields(FieldList $fields) 'IsExpired', $ldapMetadata[] = ReadonlyField::create( 'IsExpired', - _t('LDAPMemberExtension.ISEXPIRED', 'Has user\'s LDAP/AD login expired?') + _t(__CLASS__ . '.ISEXPIRED', 'Has user\'s LDAP/AD login expired?') ) ); $fields->replaceField( 'LastSynced', $ldapMetadata[] = ReadonlyField::create( 'LastSynced', - _t('LDAPMemberExtension.LASTSYNCED', 'Last synced') + _t(__CLASS__ . '.LASTSYNCED', 'Last synced') ) ); $fields->addFieldsToTab('Root.LDAP', $ldapMetadata); @@ -119,7 +121,7 @@ public function updateCMSFields(FieldList $fields) $message = ''; if ($this->owner->GUID && $this->owner->config()->update_ldap_from_local) { $message = _t( - 'LDAPMemberExtension.CHANGEFIELDSUPDATELDAP', + __CLASS__ . '.CHANGEFIELDSUPDATELDAP', 'Changing fields here will update them in LDAP.' ); } elseif ($this->owner->GUID && !$this->owner->config()->update_ldap_from_local) { @@ -131,11 +133,11 @@ public function updateCMSFields(FieldList $fields) // Set to readonly, but not disabled so that the data is still sent to the // server and doesn't break Member_Validator $field->setReadonly(true); - $field->setTitle($field->Title()._t('LDAPMemberExtension.IMPORTEDFIELD', ' (imported)')); + $field->setTitle($field->Title()._t(__CLASS__ . '.IMPORTEDFIELD', ' (imported)')); } } $message = _t( - 'LDAPMemberExtension.INFOIMPORTED', + __CLASS__ . '.INFOIMPORTED', 'This user is automatically imported from LDAP. '. 'Manual changes to imported fields will be removed upon sync.' ); @@ -184,7 +186,7 @@ public function onBeforeWrite() return; } - $service = Injector::inst()->get('SilverStripe\\ActiveDirectory\\Services\\LDAPService'); + $service = Injector::inst()->get(LDAPService::class); if (!$service->enabled() || !$this->owner->config()->create_users_in_ldap || !$this->owner->Username @@ -202,9 +204,8 @@ public function onAfterWrite() return; } - $service = Injector::inst()->get('SilverStripe\\ActiveDirectory\\Services\\LDAPService'); - if ( - !$service->enabled() || + $service = Injector::inst()->get(LDAPService::class); + if (!$service->enabled() || !$this->owner->config()->update_ldap_from_local || !$this->owner->GUID ) { @@ -219,9 +220,8 @@ public function onAfterDelete() return; } - $service = Injector::inst()->get('SilverStripe\\ActiveDirectory\\Services\\LDAPService'); - if ( - !$service->enabled() || + $service = Injector::inst()->get(LDAPService::class); + if (!$service->enabled() || !$this->owner->config()->delete_users_in_ldap || !$this->owner->GUID ) { @@ -252,9 +252,8 @@ public function writeWithoutSync() */ public function sync() { - $service = Injector::inst()->get('SilverStripe\\ActiveDirectory\\Services\\LDAPService'); - if ( - !$service->enabled() || + $service = Injector::inst()->get(LDAPService::class); + if (!$service->enabled() || !$this->owner->GUID ) { return; @@ -271,7 +270,7 @@ public function memberLoggedIn() { if ($this->owner->GUID) { Injector::inst() - ->get('SilverStripe\\ActiveDirectory\\Services\\LDAPService') + ->get(LDAPService::class) ->updateMemberFromLDAP($this->owner); } } diff --git a/src/Forms/LDAPChangePasswordForm.php b/src/Forms/LDAPChangePasswordForm.php index 39efc0b..9b1ec6b 100644 --- a/src/Forms/LDAPChangePasswordForm.php +++ b/src/Forms/LDAPChangePasswordForm.php @@ -3,14 +3,19 @@ namespace SilverStripe\ActiveDirectory\Forms; use Exception; +use SilverStripe\ActiveDirectory\Authenticators\LDAPAuthenticator; +use SilverStripe\ActiveDirectory\Services\LDAPService; use SilverStripe\Control\Director; use SilverStripe\Control\HTTP; -use SilverStripe\Control\Session; +use SilverStripe\Control\HTTPResponse; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Injector\Injector; +use SilverStripe\Forms\FieldList; use SilverStripe\Forms\TextField; -use SilverStripe\Security\ChangePasswordForm; +use SilverStripe\ORM\ValidationResult; use SilverStripe\Security\Member; +use SilverStripe\Security\MemberAuthenticator\ChangePasswordForm; +use SilverStripe\Security\Security; /** * @package activedirectory @@ -19,37 +24,44 @@ class LDAPChangePasswordForm extends ChangePasswordForm { /** * The sole purpose for overriding the constructor is surfacing the username to the user. + * @param \SilverStripe\Control\RequestHandler $controller + * @param string $name + * @param FieldList $fields + * @param FieldList $actions */ public function __construct($controller, $name, $fields = null, $actions = null) { parent::__construct($controller, $name, $fields, $actions); // Obtain the Member object. If the user got this far, they must have already been synced. - $member = Member::currentUser(); + $member = Security::getCurrentUser(); if (!$member) { - if (Session::get('AutoLoginHash')) { - $member = Member::member_from_autologinhash(Session::get('AutoLoginHash')); + if ($this->getSession()->get('AutoLoginHash')) { + $member = Member::member_from_autologinhash($this->getSession()->get('AutoLoginHash')); } // The user is not logged in and no valid auto login hash is available if (!$member) { - Session::clear('AutoLoginHash'); + $this->getSession()->clear('AutoLoginHash'); return $this->controller->redirect($this->controller->Link('login')); } } $data = Injector::inst() - ->get('SilverStripe\\ActiveDirectory\\Services\\LDAPService') + ->get(LDAPService::class) ->getUserByGUID($member->GUID, ['samaccountname']); $emailField = null; $usernameField = null; - if (Config::inst()->get('SilverStripe\\ActiveDirectory\\Authenticators\\LDAPAuthenticator', 'allow_email_login') === 'yes' + if (Config::inst()->get( + LDAPAuthenticator::class, + 'allow_email_login' + ) === 'yes' && !empty($member->Email) ) { $emailField = TextField::create( 'Email', - _t('LDAPLoginForm.USERNAMEOREMAIL', 'Email'), + _t(__CLASS__ . '.USERNAMEOREMAIL', 'Email'), $member->Email, null, $this @@ -58,7 +70,7 @@ public function __construct($controller, $name, $fields = null, $actions = null) if (!empty($data['samaccountname'])) { $usernameField = TextField::create( 'Username', - _t('LDAPLoginForm.USERNAME', 'Username'), + _t(__CLASS__ . '.USERNAME', 'Username'), $data['samaccountname'], null, $this @@ -74,121 +86,4 @@ public function __construct($controller, $name, $fields = null, $actions = null) $this->Fields()->unshift($usernameFieldReadonly); } } - - /** - * Change the password - * - * @param array $data The user submitted data - * @return HTTPResponse - */ - public function doChangePassword(array $data) - { - /** - * @var LDAPService $service - */ - $service = Injector::inst()->get('SilverStripe\\ActiveDirectory\\Services\\LDAPService'); - $member = Member::currentUser(); - if ($member) { - try { - $userData = $service->getUserByGUID($member->GUID); - } catch (Exception $e) { - Injector::inst()->get('Logger')->error($e->getMessage()); - - $this->clearMessage(); - $this->sessionMessage( - _t( - 'LDAPAuthenticator.NOUSER', - 'Your account hasn\'t been setup properly, please contact an administrator.' - ), - 'bad' - ); - return $this->controller->redirect($this->controller->Link('changepassword')); - } - $loginResult = $service->authenticate($userData['samaccountname'], $data['OldPassword']); - if (!$loginResult['success']) { - $this->clearMessage(); - $this->sessionMessage( - _t('Member.ERRORPASSWORDNOTMATCH', 'Your current password does not match, please try again'), - 'bad' - ); - // redirect back to the form, instead of using redirectBack() which could send the user elsewhere. - return $this->controller->redirect($this->controller->Link('changepassword')); - } - } - - if (!$member) { - if (Session::get('AutoLoginHash')) { - $member = Member::member_from_autologinhash(Session::get('AutoLoginHash')); - } - - // The user is not logged in and no valid auto login hash is available - if (!$member) { - Session::clear('AutoLoginHash'); - return $this->controller->redirect($this->controller->Link('login')); - } - } - - // Check the new password - if (empty($data['NewPassword1'])) { - $this->clearMessage(); - $this->sessionMessage( - _t('Member.EMPTYNEWPASSWORD', "The new password can't be empty, please try again"), - 'bad' - ); - - // redirect back to the form, instead of using redirectBack() which could send the user elsewhere. - return $this->controller->redirect($this->controller->Link('changepassword')); - } elseif ($data['NewPassword1'] == $data['NewPassword2']) { - // Providing OldPassword to perform password _change_ operation. This will respect the - // password history policy. Unfortunately we cannot support password history policy on password _reset_ - // at the moment, which means it will not be enforced on SilverStripe-driven email password reset. - $oldPassword = !empty($data['OldPassword']) ? $data['OldPassword']: null; - - /** @var ValidationResult $validationResult */ - $validationResult = $service->setPassword($member, $data['NewPassword1'], $oldPassword); - - // try to catch connection and other errors that the ldap service can through - if ($validationResult->isValid()) { - $member->logIn(); - - Session::clear('AutoLoginHash'); - - // Clear locked out status - $member->LockedOutUntil = null; - $member->FailedLoginCount = null; - $member->write(); - - if (!empty($_REQUEST['BackURL']) - // absolute redirection URLs may cause spoofing - && Director::is_site_url($_REQUEST['BackURL']) - ) { - $url = Director::absoluteURL($_REQUEST['BackURL']); - return $this->controller->redirect($url); - } else { - // Redirect to default location - the login form saying "You are logged in as..." - $redirectURL = HTTP::setGetVar( - 'BackURL', - Director::absoluteBaseURL(), - $this->controller->Link('login') - ); - return $this->controller->redirect($redirectURL); - } - } else { - $this->clearMessage(); - $messages = implode('. ', array_column($validationResult->getMessages(), 'message')); - $this->sessionMessage($messages, 'bad'); - // redirect back to the form, instead of using redirectBack() which could send the user elsewhere. - return $this->controller->redirect($this->controller->Link('changepassword')); - } - } else { - $this->clearMessage(); - $this->sessionMessage( - _t('Member.ERRORNEWPASSWORD', 'You have entered your new password differently, try again'), - 'bad' - ); - - // redirect back to the form, instead of using redirectBack() which could send the user elsewhere. - return $this->controller->redirect($this->controller->Link('changepassword')); - } - } } diff --git a/src/Authenticators/LDAPLoginForm.php b/src/Forms/LDAPLoginForm.php similarity index 63% rename from src/Authenticators/LDAPLoginForm.php rename to src/Forms/LDAPLoginForm.php index 4affe0c..d2afaac 100644 --- a/src/Authenticators/LDAPLoginForm.php +++ b/src/Forms/LDAPLoginForm.php @@ -1,18 +1,14 @@ ldapSecController = Injector::inst()->create(LDAPSecurityController::class); + parent::__construct($controller, 'LDAPAuthenticator', $name); if (Config::inst()->get(LDAPAuthenticator::class, 'allow_email_login') === 'yes') { $loginField = TextField::create( 'Login', - _t('LDAPLoginForm.USERNAMEOREMAIL', 'Username or email'), + _t(__CLASS__ . '.USERNAMEOREMAIL', 'Username or email'), null, null, $this ); } else { - $loginField = TextField::create('Login', _t('LDAPLoginForm.USERNAME', 'Username'), null, null, $this); + $loginField = TextField::create('Login', _t(__CLASS__ . '.USERNAME', 'Username'), null, null, $this); } $this->Fields()->replaceField('Email', $loginField); $this->setValidator(new RequiredFields('Login', 'Password')); if (Security::config()->remember_username) { - $loginField->setValue(Session::get('SessionForms.MemberLoginForm.Email')); + $loginField->setValue($this->getSession()->get('SessionForms.MemberLoginForm.Email')); } else { // Some browsers won't respect this attribute unless it's added to the form $this->setAttribute('autocomplete', 'off'); @@ -84,11 +71,11 @@ public function __construct($controller, $name, $fields = null, $actions = null, $this->Actions()->remove($this->Actions()->fieldByName('forgotPassword')); $allowPasswordChange = Config::inst() ->get(LDAPService::class, 'allow_password_change'); - if ($allowPasswordChange && $name != 'LostPasswordForm' && !Member::currentUser()) { + if ($allowPasswordChange && $name != 'LostPasswordForm' && !Security::getCurrentUser()) { $forgotPasswordLink = sprintf( '

%s

', - $this->ldapSecController->Link('lostpassword'), - _t('Member.BUTTONLOSTPASSWORD', "I've lost my password") + Security::singleton()->Link('lostpassword'), + _t('SilverStripe\\Security\\Member.BUTTONLOSTPASSWORD', "I've lost my password") ); $forgotPassword = LiteralField::create('forgotPassword', $forgotPasswordLink); $this->Actions()->add($forgotPassword); @@ -106,10 +93,13 @@ public function __construct($controller, $name, $fields = null, $actions = null, } /** - * @return LDAPMemberLoginHandler + * The name of this login form, to display in the frontend + * Replaces Authenticator::get_name() + * + * @return string */ - protected function buildRequestHandler() + public function getAuthenticatorName() { - return LDAPMemberLoginHandler::create($this); + return _t(__CLASS__ . '.AUTHENTICATORNAME', 'LDAP'); } } diff --git a/src/Helpers/SAMLHelper.php b/src/Helpers/SAMLHelper.php index 68de576..e696859 100644 --- a/src/Helpers/SAMLHelper.php +++ b/src/Helpers/SAMLHelper.php @@ -2,6 +2,7 @@ namespace SilverStripe\ActiveDirectory\Helpers; +use SilverStripe\ActiveDirectory\Services\SAMLConfiguration; use SilverStripe\Core\Injector\Injectable; use OneLogin_Saml2_Auth; diff --git a/src/Jobs/LDAPAllSyncJob.php b/src/Jobs/LDAPAllSyncJob.php index caf5b02..7633384 100644 --- a/src/Jobs/LDAPAllSyncJob.php +++ b/src/Jobs/LDAPAllSyncJob.php @@ -3,12 +3,12 @@ namespace SilverStripe\ActiveDirectory\Jobs; use Exception; +use SilverStripe\ActiveDirectory\Tasks\LDAPGroupSyncTask; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Injector\Injector; -use SilverStripe\QueuedJobs\Services\AbstractQueuedJob; -use SilverStripe\QueuedJobs\Services\QueuedJob; -use SilverStripe\QueuedJobs\Services\QueuedJobService; - +use Symbiote\QueuedJobs\Services\AbstractQueuedJob; +use Symbiote\QueuedJobs\Services\QueuedJob; +use Symbiote\QueuedJobs\Services\QueuedJobService; /** * Class LDAPAllSyncJob @@ -48,7 +48,7 @@ public function getJobType() */ public function getTitle() { - return _t('LDAPAllSyncJob.SYNCTITLE', 'Sync all groups and users from Active Directory, and set mappings up.'); + return _t(__CLASS__ . '.SYNCTITLE', 'Sync all groups and users from Active Directory, and set mappings up.'); } /** @@ -64,7 +64,7 @@ public function getSignature() */ public function validateRegenerateTime() { - $regenerateTime = Config::inst()->get('SilverStripe\\ActiveDirectory\\Jobs\\LDAPAllSyncJob', 'regenerate_time'); + $regenerateTime = Config::inst()->get(LDAPAllSyncJob::class, 'regenerate_time'); // don't allow this job to run less than every 15 minutes, as it could take a while. if ($regenerateTime !== null && $regenerateTime < 900) { @@ -77,18 +77,18 @@ public function validateRegenerateTime() */ public function process() { - $regenerateTime = Config::inst()->get('SilverStripe\\ActiveDirectory\\Jobs\\LDAPAllSyncJob', 'regenerate_time'); + $regenerateTime = Config::inst()->get(LDAPAllSyncJob::class, 'regenerate_time'); if ($regenerateTime) { $this->validateRegenerateTime(); - $nextJob = Injector::inst()->create('SilverStripe\\ActiveDirectory\\Jobs\\LDAPAllSyncJob'); + $nextJob = Injector::inst()->create(LDAPAllSyncJob::class); singleton(QueuedJobService::class)->queueJob($nextJob, date('Y-m-d H:i:s', time() + $regenerateTime)); } - $task = Injector::inst()->create('SilverStripe\\ActiveDirectory\\Tasks\\LDAPGroupSyncTask'); + $task = Injector::inst()->create(LDAPGroupSyncTask::class); $task->run(null); - $task = Injector::inst()->create('SilverStripe\\ActiveDirectory\\Tasks\\LDAPMemberSyncTask'); + $task = Injector::inst()->create(LDAPGroupSyncTask::class); $task->run(null); $this->isComplete = true; diff --git a/src/Jobs/LDAPMemberSyncJob.php b/src/Jobs/LDAPMemberSyncJob.php index 4b1c61e..b02dcc5 100644 --- a/src/Jobs/LDAPMemberSyncJob.php +++ b/src/Jobs/LDAPMemberSyncJob.php @@ -3,12 +3,12 @@ namespace SilverStripe\ActiveDirectory\Jobs; use Exception; +use SilverStripe\ActiveDirectory\Tasks\LDAPMemberSyncTask; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Injector\Injector; -use SilverStripe\QueuedJobs\Services\AbstractQueuedJob; -use SilverStripe\QueuedJobs\Services\QueuedJob; -use SilverStripe\QueuedJobs\Services\QueuedJobService; - +use Symbiote\QueuedJobs\Services\AbstractQueuedJob; +use Symbiote\QueuedJobs\Services\QueuedJob; +use Symbiote\QueuedJobs\Services\QueuedJobService; /** * Class LDAPMemberSyncJob @@ -47,7 +47,7 @@ public function getJobType() */ public function getTitle() { - return _t('LDAPMemberSyncJob.SYNCTITLE', 'Sync all users from Active Directory'); + return _t(__CLASS__ . '.SYNCTITLE', 'Sync all users from Active Directory'); } /** @@ -64,7 +64,7 @@ public function getSignature() public function validateRegenerateTime() { $regenerateTime = Config::inst()->get( - 'SilverStripe\\ActiveDirectory\\Jobs\\LDAPMemberSyncJob', + LDAPMemberSyncJob::class, 'regenerate_time' ); @@ -80,17 +80,17 @@ public function validateRegenerateTime() public function process() { $regenerateTime = Config::inst()->get( - 'SilverStripe\\ActiveDirectory\\Jobs\\LDAPMemberSyncJob', + LDAPMemberSyncJob::class, 'regenerate_time' ); if ($regenerateTime) { $this->validateRegenerateTime(); - $nextJob = Injector::inst()->create('SilverStripe\\ActiveDirectory\\Jobs\\LDAPMemberSyncJob'); + $nextJob = Injector::inst()->create(LDAPMemberSyncJob::class); singleton(QueuedJobService::class)->queueJob($nextJob, date('Y-m-d H:i:s', time() + $regenerateTime)); } - $task = Injector::inst()->create('SilverStripe\\ActiveDirectory\\Tasks\\LDAPMemberSyncTask'); + $task = Injector::inst()->create(LDAPMemberSyncTask::class); $task->run(null); $this->isComplete = true; diff --git a/src/Model/LDAPGateway.php b/src/Model/LDAPGateway.php index 6337fe3..9ab9039 100644 --- a/src/Model/LDAPGateway.php +++ b/src/Model/LDAPGateway.php @@ -30,7 +30,7 @@ class LDAPGateway private static $options = []; /** - * @var Zend\Ldap\Ldap + * @var Ldap */ private $ldap; @@ -103,21 +103,29 @@ public function authenticate($username, $password) * Query for LDAP nodes (organizational units, containers, and domains). * * @param null|string $baseDn The DN to search from. Default is the baseDn option in the connection if not given - * @param int $scope The scope to perform the search. Zend_Ldap::SEARCH_SCOPE_ONE, Zend_LDAP::SEARCH_SCOPE_BASE. Default is Zend_Ldap::SEARCH_SCOPE_SUB + * @param int $scope The scope to perform the search. Zend_Ldap::SEARCH_SCOPE_ONE, Zend_LDAP::SEARCH_SCOPE_BASE. + * Default is Zend_Ldap::SEARCH_SCOPE_SUB * @param array $attributes Restrict to specific AD attributes. An empty array will return all attributes * @param string $sort Sort results by this attribute if given * @return array */ public function getNodes($baseDn = null, $scope = Ldap::SEARCH_SCOPE_SUB, $attributes = [], $sort = '') { - return $this->search('(|(objectClass=organizationalUnit)(objectClass=container)(objectClass=domain))', $baseDn, $scope, $attributes, $sort); + return $this->search( + '(|(objectClass=organizationalUnit)(objectClass=container)(objectClass=domain))', + $baseDn, + $scope, + $attributes, + $sort + ); } /** * Query for LDAP groups. * * @param null|string $baseDn The DN to search from. Default is the baseDn option in the connection if not given - * @param int $scope The scope to perform the search. Zend_Ldap::SEARCH_SCOPE_ONE, Zend_LDAP::SEARCH_SCOPE_BASE. Default is Zend_Ldap::SEARCH_SCOPE_SUB + * @param int $scope The scope to perform the search. Zend_Ldap::SEARCH_SCOPE_ONE, Zend_LDAP::SEARCH_SCOPE_BASE. + * Default is Zend_Ldap::SEARCH_SCOPE_SUB * @param array $attributes Restrict to specific AD attributes. An empty array will return all attributes * @param string $sort Sort results by this attribute if given * @return array @@ -132,7 +140,8 @@ public function getGroups($baseDn = null, $scope = Ldap::SEARCH_SCOPE_SUB, $attr * * @param string $dn * @param null|string $baseDn The DN to search from. Default is the baseDn option in the connection if not given - * @param int $scope The scope to perform the search. Zend_Ldap::SEARCH_SCOPE_ONE, Zend_LDAP::SEARCH_SCOPE_BASE. Default is Zend_Ldap::SEARCH_SCOPE_SUB + * @param int $scope The scope to perform the search. Zend_Ldap::SEARCH_SCOPE_ONE, Zend_LDAP::SEARCH_SCOPE_BASE. + * Default is Zend_Ldap::SEARCH_SCOPE_SUB * @param array $attributes Restrict to specific AD attributes. An empty array will return all attributes * @return array */ @@ -151,7 +160,8 @@ public function getNestedGroups($dn, $baseDn = null, $scope = Ldap::SEARCH_SCOPE * * @param string $guid * @param null|string $baseDn The DN to search from. Default is the baseDn option in the connection if not given - * @param int $scope The scope to perform the search. Zend_Ldap::SEARCH_SCOPE_ONE, Zend_LDAP::SEARCH_SCOPE_BASE. Default is Zend_Ldap::SEARCH_SCOPE_SUB + * @param int $scope The scope to perform the search. Zend_Ldap::SEARCH_SCOPE_ONE, Zend_LDAP::SEARCH_SCOPE_BASE. + * Default is Zend_Ldap::SEARCH_SCOPE_SUB * @param array $attributes Restrict to specific AD attributes. An empty array will return all attributes * @return array */ @@ -170,7 +180,8 @@ public function getGroupByGUID($guid, $baseDn = null, $scope = Ldap::SEARCH_SCOP * * @param string $dn * @param null|string $baseDn The DN to search from. Default is the baseDn option in the connection if not given - * @param int $scope The scope to perform the search. Zend_Ldap::SEARCH_SCOPE_ONE, Zend_LDAP::SEARCH_SCOPE_BASE. Default is Zend_Ldap::SEARCH_SCOPE_SUB + * @param int $scope The scope to perform the search. Zend_Ldap::SEARCH_SCOPE_ONE, Zend_LDAP::SEARCH_SCOPE_BASE. + * Default is Zend_Ldap::SEARCH_SCOPE_SUB * @param array $attributes Restrict to specific AD attributes. An empty array will return all attributes * @return array */ @@ -188,12 +199,13 @@ public function getGroupByDN($dn, $baseDn = null, $scope = Ldap::SEARCH_SCOPE_SU * Query for LDAP users, but don't include built-in user accounts. * * @param null|string $baseDn The DN to search from. Default is the baseDn option in the connection if not given - * @param int $scope The scope to perform the search. Zend_Ldap::SEARCH_SCOPE_ONE, Zend_LDAP::SEARCH_SCOPE_BASE. Default is Zend_Ldap::SEARCH_SCOPE_SUB + * @param int $scope The scope to perform the search. Zend_Ldap::SEARCH_SCOPE_ONE, Zend_LDAP::SEARCH_SCOPE_BASE. + * Default is Zend_Ldap::SEARCH_SCOPE_SUB * @param array $attributes Restrict to specific AD attributes. An empty array will return all attributes * @param string $sort Sort results by this attribute if given * @return array */ - public function getUsers($baseDn = null, $scope = Zend\Ldap\Ldap::SEARCH_SCOPE_SUB, $attributes = [], $sort = '') + public function getUsers($baseDn = null, $scope = Ldap::SEARCH_SCOPE_SUB, $attributes = [], $sort = '') { return $this->search( '(&(objectClass=user)(!(objectClass=computer))(!(samaccountname=Guest))(!(samaccountname=Administrator))(!(samaccountname=krbtgt)))', @@ -225,7 +237,8 @@ public function getUserByGUID($guid, $baseDn = null, $scope = Ldap::SEARCH_SCOPE * * @param string $dn * @param null|string $baseDn The DN to search from. Default is the baseDn option in the connection if not given - * @param int $scope The scope to perform the search. Zend_Ldap::SEARCH_SCOPE_ONE, Zend_LDAP::SEARCH_SCOPE_BASE. Default is Zend_Ldap::SEARCH_SCOPE_SUB + * @param int $scope The scope to perform the search. Zend_Ldap::SEARCH_SCOPE_ONE, Zend_LDAP::SEARCH_SCOPE_BASE. + * Default is Zend_Ldap::SEARCH_SCOPE_SUB * @param array $attributes Restrict to specific AD attributes. An empty array will return all attributes * @return array */ @@ -260,7 +273,8 @@ public function getUserByEmail($email, $baseDn = null, $scope = Ldap::SEARCH_SCO * * @param string $username * @param null|string $baseDn The DN to search from. Default is the baseDn option in the connection if not given - * @param int $scope The scope to perform the search. Zend_Ldap::SEARCH_SCOPE_ONE, Zend_LDAP::SEARCH_SCOPE_BASE. Default is Zend_Ldap::SEARCH_SCOPE_SUB + * @param int $scope The scope to perform the search. Zend_Ldap::SEARCH_SCOPE_ONE, Zend_LDAP::SEARCH_SCOPE_BASE. + * Default is Zend_Ldap::SEARCH_SCOPE_SUB * @param array $attributes Restrict to specific AD attributes. An empty array will return all attributes * @return array * @throws Exception @@ -282,14 +296,20 @@ public function getUserByUsername($username, $baseDn = null, $scope = Ldap::SEAR throw new Exception('Backslash style not supported in LDAPGateway::getUserByUsername()!'); break; case Ldap::ACCTNAME_FORM_PRINCIPAL: // principal style usernames, e.g. alice@foo.com - $filter = sprintf('(&(objectClass=user)(userprincipalname=%s))', AbstractFilter::escapeValue($username)); + $filter = sprintf( + '(&(objectClass=user)(userprincipalname=%s))', + AbstractFilter::escapeValue($username) + ); break; case Ldap::ACCTNAME_FORM_DN: // distinguished name, e.g. CN=someone,DC=example,DC=co,DC=nz // @todo Not supported yet! throw new Exception('DN style not supported in LDAPGateway::getUserByUsername()!'); break; default: // default to principal style - $filter = sprintf('(&(objectClass=user)(userprincipalname=%s))', AbstractFilter::escapeValue($username)); + $filter = sprintf( + '(&(objectClass=user)(userprincipalname=%s))', + AbstractFilter::escapeValue($username) + ); break; } @@ -307,7 +327,6 @@ public function getCanonicalUsername($data) { $options = $this->config()->options; $option = isset($options['accountCanonicalForm']) ? $options['accountCanonicalForm'] : null; - switch ($option) { case Ldap::ACCTNAME_FORM_USERNAME: // traditional style usernames, e.g. alice if (empty($data['samaccountname'])) { @@ -391,7 +410,7 @@ public function resetPassword($dn, $password) $dn, ['unicodePwd' => iconv('UTF-8', 'UTF-16LE', sprintf('"%s"', $password))] ); - } catch(LdapException $e) { + } catch (LdapException $e) { throw new Exception($this->getLastPasswordError()); } } @@ -461,7 +480,7 @@ public function add($dn, array $attributes) private function getLastPasswordError() { $defaultError = _t( - 'LDAPAuthenticator.CANTCHANGEPASSWORD', + 'SilverStripe\\ActiveDirectory\\Authenticators\\LDAPAuthenticator.CANTCHANGEPASSWORD', 'We couldn\'t change your password, please contact an administrator.' ); $error = ''; diff --git a/src/Model/LDAPGroupMapping.php b/src/Model/LDAPGroupMapping.php index d89a6da..b142fab 100644 --- a/src/Model/LDAPGroupMapping.php +++ b/src/Model/LDAPGroupMapping.php @@ -3,6 +3,7 @@ namespace SilverStripe\ActiveDirectory\Model; use SilverStripe\Forms\DropdownField; +use SilverStripe\Forms\FieldList; use SilverStripe\ORM\DataObject; /** @@ -56,8 +57,8 @@ public function getCMSFields() $fields = parent::getCMSFields(); $fields->removeByName('DN'); - $field = DropdownField::create('DN', _t('LDAPGroupMapping.LDAPGROUP', 'LDAP Group')); - $field->setEmptyString(_t('LDAPGroupMapping.SELECTONE', 'Select one')); + $field = DropdownField::create('DN', _t(__CLASS__ . '.LDAPGROUP', 'LDAP Group')); + $field->setEmptyString(_t(__CLASS__ . '.SELECTONE', 'Select one')); $groups = $this->ldapService->getGroups(true, ['dn', 'name']); if ($groups) { foreach ($groups as $dn => $record) { @@ -71,12 +72,12 @@ public function getCMSFields() $fields->removeByName('Scope'); $fields->addFieldToTab( 'Root.Main', - DropdownField::create('Scope', _t('LDAPGroupMapping.SCOPE', 'Scope'), [ + DropdownField::create('Scope', _t(__CLASS__ . '.SCOPE', 'Scope'), [ 'Subtree' => _t( - 'LDAPGroupMapping.SUBTREE_DESCRIPTION', + __CLASS__ . '.SUBTREE_DESCRIPTION', 'Users within this group and all nested groups within' ), - 'OneLevel' => _t('LDAPGroupMapping.ONELEVEL_DESCRIPTION', 'Only users within this group'), + 'OneLevel' => _t(__CLASS__ . '.ONELEVEL_DESCRIPTION', 'Only users within this group'), ]) ); diff --git a/src/Services/LDAPService.php b/src/Services/LDAPService.php index cf70f8b..f3767e5 100644 --- a/src/Services/LDAPService.php +++ b/src/Services/LDAPService.php @@ -194,14 +194,14 @@ public function authenticate($username, $password) // show better errors than the defaults for various status codes returned by LDAP if (!empty($messages[1]) && strpos($messages[1], 'NT_STATUS_ACCOUNT_LOCKED_OUT') !== false) { $message = _t( - 'LDAPService.ACCOUNTLOCKEDOUT', + __CLASS__ . '.ACCOUNTLOCKEDOUT', 'Your account has been temporarily locked because of too many failed login attempts. ' . 'Please try again later.' ); } if (!empty($messages[1]) && strpos($messages[1], 'NT_STATUS_LOGON_FAILURE') !== false) { $message = _t( - 'LDAPService.INVALIDCREDENTIALS', + __CLASS__ . '.INVALIDCREDENTIALS', 'The provided details don\'t seem to be correct. Please try again.' ); } @@ -209,7 +209,8 @@ public function authenticate($username, $password) return [ 'success' => $result->getCode() === 1, 'identity' => $result->getIdentity(), - 'message' => $message + 'message' => $message, + 'code' => $result->getCode() ]; } @@ -435,7 +436,12 @@ public function getUserByUsername($username, $attributes = []) { $searchLocations = $this->config()->users_search_locations ?: [null]; foreach ($searchLocations as $searchLocation) { - $records = $this->gateway->getUserByUsername($username, $searchLocation, Ldap::SEARCH_SCOPE_SUB, $attributes); + $records = $this->gateway->getUserByUsername( + $username, + $searchLocation, + Ldap::SEARCH_SCOPE_SUB, + $attributes + ); if ($records) { return $records[0]; } @@ -488,26 +494,29 @@ public function getLDAPGroupMembers($dn) * Constraints: * - GUID of the member must have already been set, for integrity reasons we don't allow it to change here. * - * @param Member + * @param Member $member * @param array|null $data If passed, this is pre-existing AD attribute data to update the Member with. * If not given, the data will be looked up by the user's GUID. + * @param bool $updateGroups controls whether to run the resource-intensive group update function as well. This is + * skipped during login to reduce load. * @return bool + * @internal param $Member */ - public function updateMemberFromLDAP(Member $member, $data = null) + public function updateMemberFromLDAP(Member $member, $data = null, $updateGroups = true) { if (!$this->enabled()) { return false; } if (!$member->GUID) { - $this->getLogger()->warn(sprintf('Cannot update Member ID %s, GUID not set', $member->ID)); + $this->getLogger()->warning(sprintf('Cannot update Member ID %s, GUID not set', $member->ID)); return false; } if (!$data) { $data = $this->getUserByGUID($member->GUID); if (!$data) { - $this->getLogger()->warn(sprintf('Could not retrieve data for user. GUID: %s', $member->GUID)); + $this->getLogger()->warning(sprintf('Could not retrieve data for user. GUID: %s', $member->GUID)); return false; } } @@ -519,7 +528,8 @@ public function updateMemberFromLDAP(Member $member, $data = null) if (!isset($data[$attribute])) { $this->getLogger()->notice( sprintf( - 'Attribute %s configured in Member.ldap_field_mappings, but no available attribute in AD data (GUID: %s, Member ID: %s)', + 'Attribute %s configured in Member.ldap_field_mappings, ' . + 'but no available attribute in AD data (GUID: %s, Member ID: %s)', $attribute, $data['objectguid'], $member->ID @@ -534,9 +544,10 @@ public function updateMemberFromLDAP(Member $member, $data = null) if ($imageClass !== Image::class && !is_subclass_of($imageClass, Image::class) ) { - $this->getLogger()->warn( + $this->getLogger()->warning( sprintf( - 'Member field %s configured for thumbnailphoto AD attribute, but it isn\'t a valid relation to an Image class', + 'Member field %s configured for thumbnailphoto AD attribute, but it isn\'t a ' . + 'valid relation to an Image class', $field ) ); @@ -575,7 +586,7 @@ public function updateMemberFromLDAP(Member $member, $data = null) if ($this->config()->default_group) { $group = Group::get()->filter('Code', $this->config()->default_group)->limit(1)->first(); if (!($group && $group->exists())) { - $this->getLogger()->warn( + $this->getLogger()->warning( sprintf( 'LDAPService.default_group misconfiguration! There is no such group with Code = \'%s\'', $this->config()->default_group @@ -597,13 +608,28 @@ public function updateMemberFromLDAP(Member $member, $data = null) // the Member, in effect deleting all their LDAP group associations! $member->writeWithoutSync(); - // ensure the user is in any mapped groups + if ($updateGroups) { + $this->updateMemberGroups($data, $member); + } + + // This will throw an exception if there are two distinct GUIDs with the same email address. + // We are happy with a raw 500 here at this stage. + $member->write(); + } + + /** + * Ensure the user is mapped to any applicable groups. + * @param array $data + * @param Member $member + */ + public function updateMemberGroups($data, Member $member) + { if (isset($data['memberof'])) { $ldapGroups = is_array($data['memberof']) ? $data['memberof'] : [$data['memberof']]; foreach ($ldapGroups as $groupDN) { foreach (LDAPGroupMapping::get() as $mapping) { if (!$mapping->DN) { - $this->getLogger()->warn( + $this->getLogger()->warning( sprintf( 'LDAPGroupMapping ID %s is missing DN field. Skipping', $mapping->ID @@ -656,18 +682,17 @@ public function updateMemberFromLDAP(Member $member, $data = null) ) ); - foreach ($groupRecords as $groupRecord) { - if (!in_array($groupRecord['GroupID'], $mappedGroupIDs)) { - $group = Group::get()->byId($groupRecord['GroupID']); - // Some groups may no longer exist. SilverStripe does not clean up join tables. - if ($group) { - $group->Members()->remove($member); + if (!empty($mappedGroupIDs)) { + foreach ($groupRecords as $groupRecord) { + if (!in_array($groupRecord['GroupID'], $mappedGroupIDs)) { + $group = Group::get()->byId($groupRecord['GroupID']); + // Some groups may no longer exist. SilverStripe does not clean up join tables. + if ($group) { + $group->Members()->remove($member); + } } } } - // This will throw an exception if there are two distinct GUIDs with the same email address. - // We are happy with a raw 500 here at this stage. - $member->write(); } /** @@ -1014,10 +1039,10 @@ public function setPassword(Member $member, $password, $oldPassword = null) $this->extend('onBeforeSetPassword', $member, $password, $validationResult); if (!$member->GUID) { - $this->getLogger()->warn(sprintf('Cannot update Member ID %s, GUID not set', $member->ID)); + $this->getLogger()->warning(sprintf('Cannot update Member ID %s, GUID not set', $member->ID)); $validationResult->addError( _t( - 'LDAPAuthenticator.NOUSER', + 'SilverStripe\\ActiveDirectory\\Authenticators\\LDAPAuthenticator.NOUSER', 'Your account hasn\'t been setup properly, please contact an administrator.' ) ); @@ -1028,7 +1053,7 @@ public function setPassword(Member $member, $password, $oldPassword = null) if (empty($userData['distinguishedname'])) { $validationResult->addError( _t( - 'LDAPAuthenticator.NOUSER', + 'SilverStripe\\ActiveDirectory\\Authenticators\\LDAPAuthenticator.NOUSER', 'Your account hasn\'t been setup properly, please contact an administrator.' ) ); diff --git a/src/Services/SAMLConfiguration.php b/src/Services/SAMLConfiguration.php index be10864..8c1b65d 100644 --- a/src/Services/SAMLConfiguration.php +++ b/src/Services/SAMLConfiguration.php @@ -70,7 +70,8 @@ public function asArray() 'url' => $sp['entityId'] . '/saml/acs', 'binding' => OneLogin_Saml2_Constants::BINDING_HTTP_POST ]; - $conf['sp']['NameIDFormat'] = isset($sp['nameIdFormat']) ? $sp['nameIdFormat'] : OneLogin_Saml2_Constants::NAMEID_TRANSIENT; + $conf['sp']['NameIDFormat'] = isset($sp['nameIdFormat']) ? + $sp['nameIdFormat'] : OneLogin_Saml2_Constants::NAMEID_TRANSIENT; $conf['sp']['x509cert'] = file_get_contents($spCertPath); $conf['sp']['privateKey'] = file_get_contents($spKeyPath); diff --git a/src/Tasks/LDAPGroupSyncTask.php b/src/Tasks/LDAPGroupSyncTask.php index cc5395e..7143aee 100644 --- a/src/Tasks/LDAPGroupSyncTask.php +++ b/src/Tasks/LDAPGroupSyncTask.php @@ -44,7 +44,7 @@ class LDAPGroupSyncTask extends BuildTask */ public function getTitle() { - return _t('LDAPGroupSyncJob.SYNCTITLE', 'Sync all groups from Active Directory'); + return _t(__CLASS__ . '.SYNCTITLE', 'Sync all groups from Active Directory'); } /** diff --git a/src/Tasks/LDAPMemberSyncTask.php b/src/Tasks/LDAPMemberSyncTask.php index bf90e3a..97cc8db 100644 --- a/src/Tasks/LDAPMemberSyncTask.php +++ b/src/Tasks/LDAPMemberSyncTask.php @@ -4,6 +4,7 @@ use Exception; use SilverStripe\Control\Director; +use SilverStripe\Control\HTTPRequest; use SilverStripe\Core\Config\Config; use SilverStripe\Dev\BuildTask; use SilverStripe\ORM\DB; @@ -45,7 +46,7 @@ class LDAPMemberSyncTask extends BuildTask */ public function getTitle() { - return _t('LDAPMemberSyncJob.SYNCTITLE', 'Sync all users from Active Directory'); + return _t(__CLASS__ . '.SYNCTITLE', 'Sync all users from Active Directory'); } /** diff --git a/src/Tasks/LDAPMigrateExistingMembersTask.php b/src/Tasks/LDAPMigrateExistingMembersTask.php index 276ce3f..26eb636 100644 --- a/src/Tasks/LDAPMigrateExistingMembersTask.php +++ b/src/Tasks/LDAPMigrateExistingMembersTask.php @@ -2,6 +2,7 @@ namespace SilverStripe\ActiveDirectory\Tasks; +use SilverStripe\Control\HTTPRequest; use SilverStripe\Dev\BuildTask; use SilverStripe\Core\Convert; use SilverStripe\Control\Director; diff --git a/tests/Model/LDAPFakeGateway.php b/tests/Model/LDAPFakeGateway.php deleted file mode 100644 index c69fa46..0000000 --- a/tests/Model/LDAPFakeGateway.php +++ /dev/null @@ -1,94 +0,0 @@ - [ - 'CN=Users,DC=playpen,DC=local' => [ - ['dn' => 'CN=Group1,CN=Users,DC=playpen,DC=local'], - ['dn' => 'CN=Group2,CN=Users,DC=playpen,DC=local'], - ['dn' => 'CN=Group3,CN=Users,DC=playpen,DC=local'], - ['dn' => 'CN=Group4,CN=Users,DC=playpen,DC=local'], - ['dn' => 'CN=Group5,CN=Users,DC=playpen,DC=local'] - ], - 'CN=Others,DC=playpen,DC=local' => [ - ['dn' => 'CN=Group6,CN=Others,DC=playpen,DC=local'], - ['dn' => 'CN=Group7,CN=Others,DC=playpen,DC=local'], - ['dn' => 'CN=Group8,CN=Others,DC=playpen,DC=local'] - ] - ], - 'users' => [ - '123' => [ - 'distinguishedname' => 'CN=Joe,DC=playpen,DC=local', - 'objectguid' => '123', - 'cn' => 'jbloggs', - 'useraccountcontrol' => '1', - 'givenname' => 'Joe', - 'sn' => 'Bloggs', - 'mail' => 'joe@bloggs.com' - ] - ] - ]; - - public function authenticate($username, $password) - { - } - - public function getNodes($baseDn = null, $scope = Ldap::SEARCH_SCOPE_SUB, $attributes = [], $sort = '') - { - } - - public function getGroups($baseDn = null, $scope = Ldap::SEARCH_SCOPE_SUB, $attributes = [], $sort = '') - { - if (isset($baseDn)) { - return !empty(self::$data['groups'][$baseDn]) ? self::$data['groups'][$baseDn] : null; - } - } - - public function getNestedGroups($dn, $baseDn = null, $scope = Ldap::SEARCH_SCOPE_SUB, $attributes = []) - { - } - - public function getGroupByGUID($guid, $baseDn = null, $scope = Ldap::SEARCH_SCOPE_SUB, $attributes = []) - { - } - - public function getUsers($baseDn = null, $scope = Ldap::SEARCH_SCOPE_SUB, $attributes = [], $sort = '') - { - } - - public function getUserByGUID($guid, $baseDn = null, $scope = Ldap::SEARCH_SCOPE_SUB, $attributes = []) - { - return [self::$data['users'][$guid]]; - } - - public function update($dn, array $attributes) - { - } - - public function delete($dn, $recursively = false) - { - } - - public function move($fromDn, $toDn, $recursively = false) - { - } - - public function add($dn, array $attributes) - { - } -} diff --git a/tests/php/FakeGatewayTest.php b/tests/php/FakeGatewayTest.php new file mode 100644 index 0000000..2c9294a --- /dev/null +++ b/tests/php/FakeGatewayTest.php @@ -0,0 +1,30 @@ +registerService($gateway, LDAPGateway::class); + + $service = Injector::inst()->get(LDAPService::class); + $service->setGateway($gateway); + + $this->service = $service; + } +} diff --git a/tests/php/LDAPAuthenticatorTest.php b/tests/php/LDAPAuthenticatorTest.php new file mode 100644 index 0000000..edae8af --- /dev/null +++ b/tests/php/LDAPAuthenticatorTest.php @@ -0,0 +1,119 @@ +authenticator = Injector::inst()->create(LDAPAuthenticator::class); + Config::modify()->set(LDAPAuthenticator::class, 'allow_email_login', 'yes'); + $this->request = new HTTPRequest('get', '/'); + $this->request->setSession(new Session([])); + $this->result = new ValidationResult(); + $this->data = [ + 'Login' => null, + 'Password' => null + ]; + } + + public function testDisallowedEmailLogin() + { + Config::modify()->set(LDAPAuthenticator::class, 'allow_email_login', 'no'); + $this->data['Login'] = 'joe@soap.com'; + $this->data['Password'] = 'test'; + $this->callAuthMethod(); + $this->assertFalse($this->result->isValid()); + } + + /** + * Tests whether a validator error results if User not found at gateway and no fallback member found + */ + public function testEmailNotFoundAtGateWay() + { + $invalidGatewayAndLocalEmail = 'invalid@example.com'; + $this->data = ['Login' => $invalidGatewayAndLocalEmail, 'Password' => 'test']; + $this->callAuthMethod(); + $this->assertFalse($this->result->isValid()); + } + + /** + * Tests whether fallback authenticator returns a member if enabled + */ + public function testFallbackAuthenticator() + { + Config::modify()->set(LDAPAuthenticator::class, 'fallback_authenticator', 'yes'); + $member = $this->objFromFixture(Member::class, 'dbOnlyMember'); + $this->data = ['Login' => $member->Email, 'Email' => $member->Email, 'Password' => 'password']; + $result = $this->callAuthMethod(); + $this->assertInstanceOf(Member::class, $result); + $this->assertEquals($member->Email, $result->Email); + } + + /** + * Tests for Invalid Credentials upon LDAP authentication failure + */ + public function testLDAPAuthenticationFailure() + { + $this->data = ['Login' => 'usernotfound', 'Password' => 'passwordnotfound']; + $this->callAuthMethod(); + $this->assertFalse($this->result->isValid()); + $this->assertContains('Username not found', $this->result->getMessages()[0]['message']); + } + + /** + * Tests whether a new member is created in SS if it was found in LDAP but doesn't + * exist in SS + */ + public function testAuthenticateCreatesNewMemberIfNotFound() + { + $this->data = ['Login' => 'joe@bloggs.com', 'Password' => 'mockPassword']; + $member = $this->callAuthMethod(); + $this->assertTrue($this->result->isValid()); + $this->assertInstanceOf(Member::class, $member); + $this->assertEquals(123, $member->GUID); + } + + private function callAuthMethod() + { + $result = $this->authenticator->authenticate( + $this->data, + $this->request, + $this->result + ); + + return $result; + } +} diff --git a/tests/php/LDAPAuthenticatorTest.yml b/tests/php/LDAPAuthenticatorTest.yml new file mode 100644 index 0000000..3478c41 --- /dev/null +++ b/tests/php/LDAPAuthenticatorTest.yml @@ -0,0 +1,6 @@ +SilverStripe\Security\Member: + dbOnlyMember: + FirstName: Jane + Surname: Doe + Email: jane@doe.com + Password: password diff --git a/tests/LDAPServiceTest.php b/tests/php/LDAPServiceTest.php similarity index 88% rename from tests/LDAPServiceTest.php rename to tests/php/LDAPServiceTest.php index 96d72f3..1b99af0 100644 --- a/tests/LDAPServiceTest.php +++ b/tests/php/LDAPServiceTest.php @@ -1,14 +1,13 @@ registerService($gateway, LDAPGateway::class); - - $service = Injector::inst()->create(LDAPService::class); - $service->setGateway($gateway); - $this->service = $service; - Config::modify()->set(LDAPGateway::class, 'options', ['host' => '1.2.3.4']); Config::modify()->set(LDAPService::class, 'groups_search_locations', [ 'CN=Users,DC=playpen,DC=local', diff --git a/tests/php/Model/LDAPFakeGateway.php b/tests/php/Model/LDAPFakeGateway.php new file mode 100644 index 0000000..0f3540e --- /dev/null +++ b/tests/php/Model/LDAPFakeGateway.php @@ -0,0 +1,190 @@ + [ + 'CN=Users,DC=playpen,DC=local' => [ + ['dn' => 'CN=Group1,CN=Users,DC=playpen,DC=local'], + ['dn' => 'CN=Group2,CN=Users,DC=playpen,DC=local'], + ['dn' => 'CN=Group3,CN=Users,DC=playpen,DC=local'], + ['dn' => 'CN=Group4,CN=Users,DC=playpen,DC=local'], + ['dn' => 'CN=Group5,CN=Users,DC=playpen,DC=local'] + ], + 'CN=Others,DC=playpen,DC=local' => [ + ['dn' => 'CN=Group6,CN=Others,DC=playpen,DC=local'], + ['dn' => 'CN=Group7,CN=Others,DC=playpen,DC=local'], + ['dn' => 'CN=Group8,CN=Others,DC=playpen,DC=local'] + ] + ], + 'users' => [ + '123' => [ + 'distinguishedname' => 'CN=Joe,DC=playpen,DC=local', + 'objectguid' => '123', + 'cn' => 'jbloggs', + 'useraccountcontrol' => '1', + 'givenname' => 'Joe', + 'sn' => 'Bloggs', + 'mail' => 'joe@bloggs.com', + 'password' => 'mockPassword', + 'canonicalName'=>'mockCanonicalName', + 'userprincipalname' => 'joe@bloggs.com', + 'samaccountname' => 'joe' + ] + ] + ]; + + /** + * @inheritdoc + */ + public function authenticate($username, $password) + { + $messages = []; + if (!$user = $this->getUserByEmail($username)) { + $messages[0] = 'Username not found'; + $code = AuthenticationResult::FAILURE; + return new AuthenticationResult($code, $username, $messages); + } + if ($user[0]['password'] == $password) { + $messages[0] = 'OK'; + return new AuthenticationResult(AuthenticationResult::SUCCESS, $username, $messages); + } else { + $messages[0] = 'Password doesn\'t match'; + return new AuthenticationResult(AuthenticationResult::FAILURE, $username, $messages); + } + } + + public function getNodes($baseDn = null, $scope = Ldap::SEARCH_SCOPE_SUB, $attributes = [], $sort = '') + { + } + + public function getGroups($baseDn = null, $scope = Ldap::SEARCH_SCOPE_SUB, $attributes = [], $sort = '') + { + if (isset($baseDn)) { + return !empty(self::$data['groups'][$baseDn]) ? self::$data['groups'][$baseDn] : null; + } + } + + public function getNestedGroups($dn, $baseDn = null, $scope = Ldap::SEARCH_SCOPE_SUB, $attributes = []) + { + } + + public function getGroupByGUID($guid, $baseDn = null, $scope = Ldap::SEARCH_SCOPE_SUB, $attributes = []) + { + } + + public function getUsers($baseDn = null, $scope = Ldap::SEARCH_SCOPE_SUB, $attributes = [], $sort = '') + { + } + + public function getUserByGUID($guid, $baseDn = null, $scope = Ldap::SEARCH_SCOPE_SUB, $attributes = []) + { + return [self::$data['users'][$guid]]; + } + + public function update($dn, array $attributes) + { + } + + public function delete($dn, $recursively = false) + { + } + + public function move($fromDn, $toDn, $recursively = false) + { + } + + public function add($dn, array $attributes) + { + } + + protected function search($filter, $baseDn = null, $scope = Ldap::SEARCH_SCOPE_SUB, $attributes = [], $sort = '') + { + $records = self::$data; + $results = []; + foreach ($records as $record) { + foreach ($record as $attribute => $value) { + // if the value is an array with a single value, e.g. 'samaccountname' => array(0 => 'myusername') + // then make sure it's just set in the results as 'samaccountname' => 'myusername' so that it + // can be used directly by ArrayData + if (is_array($value) && count($value) == 1) { + $value = $value[0]; + } + + // ObjectGUID and ObjectSID attributes are in binary, we need to convert those to strings + if ($attribute == 'objectguid') { + $value = LDAPUtil::bin_to_str_guid($value); + } + if ($attribute == 'objectsid') { + $value = LDAPUtil::bin_to_str_sid($value); + } + + $record[$attribute] = $value; + } + + $results[] = $record; + } + + return $results; + } + + /** + * Mock to search trough dummy $data. + * + * @param string $email + * @param null $baseDn + * @param int $scope + * @param array $attributes + * @return array + */ + public function getUserByEmail($email, $baseDn = null, $scope = Ldap::SEARCH_SCOPE_SUB, $attributes = []) + { + $result = []; + foreach (self::$data['users'] as $guid => $info) { + if ($info['mail'] == $email) { + $result[] = $info; + break; + } + } + + return $result; + } + + /** + * Mock to search trough dummy $data. + * + * @param string $username + * @param null $baseDn + * @param int $scope + * @param array $attributes + * @return array + * @internal param string $email + */ + public function getUserByUsername($username, $baseDn = null, $scope = Ldap::SEARCH_SCOPE_SUB, $attributes = []) + { + $result = []; + foreach (self::$data['users'] as $guid => $info) { + if ($info['userprincipalname'] == $username) { + $result[] = $info; + break; + } + } + + return $result; + } +}