-
Notifications
You must be signed in to change notification settings - Fork 401
Use config array instead of object for Auth plugins #5620
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from 4 commits
71379da
5c0a492
de19999
8980019
242c25a
7e21c77
1a65b99
1d8746b
d356131
28cf6b4
24f412d
af668be
9abba25
f165d99
68e998c
e81a514
f519c7e
ed4e014
8775658
e8e53a2
69c0c60
44cb3de
3d01265
02510d8
2ffc761
87338ac
6a3eba5
170e81e
d122970
0dd88d7
4d7f3e4
c33c539
d949534
41ef080
960caa4
50f8329
441f7da
d01cbb7
861a889
3b97a93
7c7546f
67f63f8
101a171
efc5f44
576ac47
ccea50b
305b4eb
25a4688
488570d
43622d2
189323d
d61fd64
6407fde
d98e35c
c5797d9
872e4c2
9d4a7d6
fd4430a
6af3049
1a62470
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -70,7 +70,7 @@ abstract class AbstractBase implements | |||||||
| /** | ||||||||
| * Configuration settings. | ||||||||
| * | ||||||||
| * @var \VuFind\Config\Config | ||||||||
| * @var array | ||||||||
| */ | ||||||||
| protected $config = null; | ||||||||
|
|
||||||||
|
|
@@ -101,7 +101,7 @@ abstract class AbstractBase implements | |||||||
| * exception if the configuration is invalid. | ||||||||
| * | ||||||||
| * @throws AuthException | ||||||||
| * @return \VuFind\Config\Config | ||||||||
| * @return array | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should also be nullable to match the property.
Suggested change
|
||||||||
| */ | ||||||||
| public function getConfig() | ||||||||
| { | ||||||||
|
|
@@ -145,7 +145,7 @@ public function clearLoginState() | |||||||
| /** | ||||||||
| * Set configuration. | ||||||||
| * | ||||||||
| * @param \VuFind\Config\Config $config Configuration to set | ||||||||
| * @param array $config Configuration to set | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably makes sense to make this consistent with the getter.
Suggested change
|
||||||||
| * | ||||||||
| * @return void | ||||||||
| */ | ||||||||
|
|
@@ -450,9 +450,8 @@ public function getPolicyConfig(string $type): array | |||||||
| { | ||||||||
| $policy = []; | ||||||||
| $config = $this->getConfig(); | ||||||||
| $authConfig = isset($config->Authentication) | ||||||||
| ? $config->Authentication->toArray() | ||||||||
| : []; | ||||||||
| $authConfig = $config['Authentication'] | ||||||||
| ?? []; | ||||||||
|
Comment on lines
+453
to
+454
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This fits on a single line now:
Suggested change
|
||||||||
| /* Map settings to the policy array, e.g.: | ||||||||
| * | ||||||||
| * password_minimum_length or username_minimum_length => minLength | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -71,7 +71,7 @@ public function clearLoginState(); | |||||
| /** | ||||||
| * Set configuration. | ||||||
| * | ||||||
| * @param \VuFind\Config\Config $config Configuration to set | ||||||
| * @param array $config Configuration to set | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to make this consistent with AbstractBase:
Suggested change
|
||||||
| * | ||||||
| * @return void | ||||||
| */ | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -107,8 +107,8 @@ public function __construct(\Laminas\Session\Container $container) | |||||||
| protected function validateConfig() | ||||||||
| { | ||||||||
| if ( | ||||||||
| !isset($this->config->ChoiceAuth->choice_order) | ||||||||
| || !strlen($this->config->ChoiceAuth->choice_order) | ||||||||
| !isset($this->config['ChoiceAuth']['choice_order']) | ||||||||
| || !strlen($this->config['ChoiceAuth']['choice_order']) | ||||||||
|
Comment on lines
+110
to
+111
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be simplified:
Suggested change
|
||||||||
| ) { | ||||||||
| throw new AuthException( | ||||||||
| 'One or more ChoiceAuth parameters are missing. ' . | ||||||||
|
|
@@ -120,7 +120,7 @@ protected function validateConfig() | |||||||
| /** | ||||||||
| * Set configuration; throw an exception if it is invalid. | ||||||||
| * | ||||||||
| * @param \VuFind\Config\Config $config Configuration to set | ||||||||
| * @param $config Configuration to set | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Type got lost here:
Suggested change
|
||||||||
| * | ||||||||
| * @throws AuthException | ||||||||
| * @return void | ||||||||
|
|
@@ -130,7 +130,7 @@ public function setConfig($config) | |||||||
| parent::setConfig($config); | ||||||||
| $this->strategies = array_map( | ||||||||
| 'trim', | ||||||||
| explode(',', $this->getConfig()->ChoiceAuth->choice_order) | ||||||||
| explode(',', $this->getConfig()['ChoiceAuth']['choice_order']) | ||||||||
| ); | ||||||||
| } | ||||||||
|
|
||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,7 +35,6 @@ | |
| use Lmc\Rbac\Identity\IdentityInterface; | ||
| use Lmc\Rbac\Mvc\Identity\IdentityProviderInterface; | ||
| use Psr\Log\LoggerAwareInterface; | ||
| use VuFind\Config\Config; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is still being used on line 206 -- that's the reason the tests are failing. |
||
| use VuFind\Cookie\CookieManager; | ||
| use VuFind\Db\Entity\UserEntityInterface; | ||
| use VuFind\Db\Service\AuditEventServiceInterface; | ||
|
|
@@ -121,7 +120,7 @@ class Manager implements IdentityProviderInterface, LoggerAwareInterface | |
| /** | ||
| * Constructor. | ||
| * | ||
| * @param Config $config VuFind configuration | ||
| * @param array $config VuFind configuration | ||
| * @param UserServiceInterface $userService User database service | ||
| * @param UserSessionPersistenceInterface $userSession User session persistence service | ||
| * @param SessionManager $sessionManager Session manager | ||
|
|
@@ -134,7 +133,7 @@ class Manager implements IdentityProviderInterface, LoggerAwareInterface | |
| * @param AuditEventServiceInterface $auditEventService Event database service | ||
| */ | ||
| public function __construct( | ||
| protected Config $config, | ||
| protected array $config, | ||
| protected UserServiceInterface $userService, | ||
| protected UserSessionPersistenceInterface $userSession, | ||
| protected SessionManager $sessionManager, | ||
|
|
@@ -148,7 +147,7 @@ public function __construct( | |
| ) { | ||
| // Initialize active authentication setting (defaulting to Database | ||
| // if no setting passed in): | ||
| $method = $this->getPreAuthenticationData()['authMethod'] ?? $config->Authentication->method ?? 'Database'; | ||
| $method = $this->getPreAuthenticationData()['authMethod'] ?? $config['Authentication']['method'] ?? 'Database'; | ||
| // Set the active authentication method and force it legal: | ||
| $this->setAuthMethod($method, true); | ||
| } | ||
|
|
@@ -231,7 +230,7 @@ public function supportsCreation(?string $authMethod = null): bool | |
| */ | ||
| public function supportsRecovery(?string $authMethod = null, ?string $target = null): bool | ||
| { | ||
| return ($this->config->Authentication->recover_password ?? false) | ||
| return ($this->config['Authentication']['recover_password'] ?? false) | ||
| && $this->getAuth($authMethod)->supportsPasswordRecovery($target); | ||
| } | ||
|
|
||
|
|
@@ -269,7 +268,7 @@ public function getPasswordRecoveryData(array $params): ?array | |
| */ | ||
| public function supportsEmailChange(?string $authMethod = null): bool | ||
| { | ||
| return $this->config->Authentication->change_email ?? false; | ||
| return $this->config['Authentication']['change_email'] ?? false; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -282,7 +281,7 @@ public function supportsEmailChange(?string $authMethod = null): bool | |
| */ | ||
| public function supportsPasswordChange(?string $authMethod = null): bool | ||
| { | ||
| return ($this->config->Authentication->change_password ?? false) | ||
| return ($this->config['Authentication']['change_password'] ?? false) | ||
| && $this->getAuth($authMethod)->supportsPasswordChange(); | ||
| } | ||
|
|
||
|
|
@@ -296,7 +295,7 @@ public function supportsPasswordChange(?string $authMethod = null): bool | |
| */ | ||
| public function supportsConnectingLibraryCard(?string $authMethod = null): bool | ||
| { | ||
| return ($this->config->Catalog->auth_based_library_cards ?? false) | ||
| return ($this->config['Catalog']['auth_based_library_cards'] ?? false) | ||
| && $this->getAuth($authMethod)->supportsConnectingLibraryCard(); | ||
| } | ||
|
|
||
|
|
@@ -309,10 +308,10 @@ public function supportsConnectingLibraryCard(?string $authMethod = null): bool | |
| */ | ||
| public function supportsPersistentLogin(?string $authMethod = null): bool | ||
| { | ||
| if (!empty($this->config->Authentication->persistent_login)) { | ||
| if (!empty($this->config['Authentication']['persistent_login'])) { | ||
| return in_array( | ||
| strtolower($authMethod ?? $this->getSelectedAuthMethod() ?? ''), | ||
| explode(',', strtolower($this->config->Authentication->persistent_login)) | ||
| explode(',', strtolower($this->config['Authentication']['persistent_login'])) | ||
| ); | ||
| } | ||
| return false; | ||
|
|
@@ -325,7 +324,7 @@ public function supportsPersistentLogin(?string $authMethod = null): bool | |
| */ | ||
| public function getPersistentLoginLifetime(): int | ||
| { | ||
| return $this->config->Authentication->persistent_login_lifetime ?? 14; | ||
| return $this->config['Authentication']['persistent_login_lifetime'] ?? 14; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -516,7 +515,7 @@ public function loginEnabled(): bool | |
| { | ||
| if (null === $this->hideLogin) { | ||
| // Assume login is enabled unless explicitly turned off: | ||
| $this->hideLogin = ($this->config->Authentication->hideLogin ?? false); | ||
| $this->hideLogin = ($this->config['Authentication']['hideLogin'] ?? false); | ||
|
|
||
| if (!$this->hideLogin) { | ||
| try { | ||
|
|
@@ -544,7 +543,7 @@ public function loginEnabled(): bool | |
| public function ajaxEnabled(): bool | ||
| { | ||
| // Assume ajax is enabled unless explicitly turned off: | ||
| return $this->config->Authentication->enableAjax ?? true; | ||
| return $this->config['Authentication']['enableAjax'] ?? true; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -555,7 +554,7 @@ public function ajaxEnabled(): bool | |
| public function dropdownEnabled(): bool | ||
| { | ||
| // Assume dropdown is disabled unless explicitly turned on: | ||
| return $this->config->Authentication->enableDropdown ?? false; | ||
| return $this->config['Authentication']['enableDropdown'] ?? false; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -740,7 +739,7 @@ public function checkForExpiredCredentials(): bool | |
| */ | ||
| public function inPrivacyMode(): bool | ||
| { | ||
| return $this->config->Authentication->privacy ?? false; | ||
| return $this->config['Authentication']['privacy'] ?? false; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -838,7 +837,7 @@ public function updateEmail(UserEntityInterface $user, string $email): void | |
| { | ||
| // Depending on verification setting, either do a direct update or else | ||
| // put the new address into a pending state. | ||
| if ($this->config->Authentication->verify_email ?? false) { | ||
| if ($this->config['Authentication']['verify_email'] ?? false) { | ||
| // If new email address is the current address, just reset any pending | ||
| // email address: | ||
| $user->setPendingEmail($email === $user->getEmail() ? '' : $email); | ||
|
|
@@ -996,7 +995,7 @@ public function login(Request $request): ?UserEntityInterface | |
| // Attempt catalog login so that any bad credentials are cleared before further processing | ||
| // (avoids e.g. multiple login attempts by account AJAX checks). | ||
| if ( | ||
| ($this->config->Catalog->checkILSCredentialsOnLogin ?? true) | ||
| ($this->config['Catalog']['checkILSCredentialsOnLogin'] ?? true) | ||
| && $this->ilsAuthenticator | ||
| && $this->allowsUserIlsLogin() | ||
| && ($catUsername = $user->getCatUsername()) | ||
|
|
@@ -1225,7 +1224,7 @@ protected function updateUser(UserEntityInterface $user, ?string $authMethod): v | |
| */ | ||
| public function allowsUserIlsLogin(): bool | ||
| { | ||
| return $this->config->Catalog->allowUserLogin ?? true; | ||
| return $this->config['Catalog']['allowUserLogin'] ?? true; | ||
| } | ||
|
|
||
| /** | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be nullable since it's initialized to null.