Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions config/vufind/Folio.ini
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ password_field = false
; %%username_field%% = The username_field config setting (above)
; %%password_field%% = The password_field config setting (above)
;cql = '%%username_field%% == "%%username%%" and %%password_field%% == "%%password%%"'
; If this CQL statement is uncommented, it will be used when a user is logged in
; via Shibboleth.
;shib_cql = 'externalSystemId == "%%username%%"'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we made this cql_by_auth_method[Shibboleth] instead of hard-coding things to Shibboleth? That would make the feature more flexible and avoid hard-coded assumptions about one specific Auth method.

Copy link
Copy Markdown
Member

@demiankatz demiankatz Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And just to be clear, this would let you refactor the config logic down to something like:

        $cql = $this->config['User']['cql_by_auth_method'][$this->authManager->getSelectedAuthMethod()]
            ?? $this->config['User']['cql']
            ?? '%%username_field%% == "%%username%%"' . ($passwordField ? ' and %%password_field%% == "%%password%%"' : '');


; Should we try to log the user into the Okapi API (true) or just look them
; up in the database using [API] credentials above (false). If set to true,
Expand Down
55 changes: 47 additions & 8 deletions module/VuFind/src/VuFind/ILS/Driver/Folio.php
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,13 @@ class Folio extends AbstractAPI implements
*/
protected $dateConverter;

/**
* Auth Manager
*
* @var \VuFind\Auth\Manager
*/
protected $authManager = null;

/**
* Default availability messages, in case they are not defined in Folio.ini
*
Expand Down Expand Up @@ -145,13 +152,16 @@ class Folio extends AbstractAPI implements
* @param \VuFind\Date\Converter $dateConverter Date converter object
* @param callable $sessionFactory Factory function returning
* SessionContainer object
* @param \VuFind\Auth\Manager $authManager Auth Manager object
*/
public function __construct(
\VuFind\Date\Converter $dateConverter,
$sessionFactory
$sessionFactory,
\VuFind\Auth\Manager $authManager
) {
$this->dateConverter = $dateConverter;
$this->sessionFactory = $sessionFactory;
$this->authManager = $authManager;
}

/**
Expand Down Expand Up @@ -1270,6 +1280,41 @@ protected function patronLoginWithOkapi($username, $password)
return $query;
}

/**
* Get the CQL query template for retrieving the patron's information.
*
* It checks if Shibboleth authentication is being used and returns the
* corresponding CQL query template. Otherwise, it uses the configured CQL query
* to retrieve the patron's information.
*
* @param string $usernameField The field to use for the username in the CQL query
* @param string $passwordField The field to use for password in the CQL query
*
* @return string The patron lookup CQL query with placeholders.
*/
protected function getLoginCql($usernameField, $passwordField)
{
if (
isset($this->config['User']['shib_cql'])
&& $this->authManager->getSelectedAuthMethod() === 'Shibboleth'
) {
$cql = $this->config['User']['shib_cql'];
} else {
$cql = $this->config['User']['cql']
?? '%%username_field%% == "%%username%%"'
. ($passwordField ? ' and %%password_field%% == "%%password%%"' : '');
}
$placeholders = [
'%%username_field%%',
'%%password_field%%',
];
$values = [
$usernameField,
$passwordField,
];
return str_replace($placeholders, $values, $cql);
}

/**
* Support method for patronLogin(): authenticate the patron with a CQL looup.
* Returns the CQL query for retrieving more information about the user.
Expand All @@ -1284,18 +1329,12 @@ protected function getUserWithCql($username, $password)
// Construct user query using barcode, username, etc.
$usernameField = $this->config['User']['username_field'] ?? 'username';
$passwordField = $this->config['User']['password_field'] ?? false;
$cql = $this->config['User']['cql']
?? '%%username_field%% == "%%username%%"'
. ($passwordField ? ' and %%password_field%% == "%%password%%"' : '');
$cql = $this->getLoginCql($usernameField, $passwordField);
$placeholders = [
'%%username_field%%',
'%%password_field%%',
'%%username%%',
'%%password%%',
];
$values = [
$usernameField,
$passwordField,
$this->escapeCql($username),
$this->escapeCql($password),
];
Expand Down
3 changes: 2 additions & 1 deletion module/VuFind/src/VuFind/ILS/Driver/FolioFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ public function __invoke(
$manager = $container->get(\Laminas\Session\SessionManager::class);
return new \Laminas\Session\Container("Folio_$namespace", $manager);
};
return parent::__invoke($container, $requestedName, [$sessionFactory]);
$authManager = $container->get(\VuFind\Auth\Manager::class);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the only thing we're using the $authManager for in the driver is to get the current auth method, is it worth considering passing only that value, rather than the whole object? That would make the dependency coupling a little looser.

return parent::__invoke($container, $requestedName, [$sessionFactory, $authManager]);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,13 @@ protected function createConnector(string $test, ?array $config = null): void
$manager = new \Laminas\Session\SessionManager();
return new \Laminas\Session\Container("Folio_$namespace", $manager);
};
// Create a mock Auth Manager
$authManager = $this->getMockBuilder(\VuFind\Auth\Manager::class)
->disableOriginalConstructor()
->getMock();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you take my above suggestion, you won't need this at all, but if you decide to keep it you can simplify this to:

Suggested change
$authManager = $this->getMockBuilder(\VuFind\Auth\Manager::class)
->disableOriginalConstructor()
->getMock();
$authManager = $this->createMock(\VuFind\Auth\Manager::class);

// Create a stub for the SomeClass class
$this->driver = $this->getMockBuilder(Folio::class)
->setConstructorArgs([new \VuFind\Date\Converter(), $factory])
->setConstructorArgs([new \VuFind\Date\Converter(), $factory, $authManager])
->onlyMethods(['makeRequest'])
->getMock();
// Configure the stub
Expand Down