Added support for fetching VuFind account in ILS login primarily by t…

[EreMaijala]

…he patron ID that the ILS driver returns.

If 'id' field is returned from patronLogin, it is used first to look up an existing user account in VuFind. If not found, the username field is used as before. If a user is found using the username, the id will be added to the database for use in upcoming logins.

This allows the correct account to be found even if the user is issued a new library card, as long as the patron id stays the same. I suppose the ILS_username_field config setting could have been used for similar effect, but it doesn't allow the fallback to cat_username for existing users.

Most if not all drivers return 'id' field from patronLogin(), although some use the barcode instead of a persistent ID. This should not cause any issues other than this mechanism being useless until the drivers are fixed.

This also works fine with the multiple library card support too. The old card will still be listed in user's cards (user can delete it as necessary), and the new one will be automatically added.

TODO

• Test with PostgreSQL (and make sure migration is syntactically correct)
• Update database changelog
• Update ILS driver spec

[demiankatz]

This makes sense to me. A few questions/comments, though:

1.) I've added a couple of documentation TODOs above so we don't forget about those when we're ready to merge. No action necessary yet.

2.) You've created a PostgreSQL migration but have not updated the core pgsql.sql script. If you don't have time to deal with that, feel free to just make it another TODO above and I can do it later.

3.) This only affects the ILS login method. Are there any other places where it might be appropriate to use the new cat_id value? For example, should the $user->saveCredentials() method accept the ID as a third optional parameter, and changes be made to the ILSAuthenticator class so that we ensure this value is kept up to date in all contexts? (I realize that right now it only actually matters when using ILS authentication, but I'm wondering if it would be advantageous to broaden it, or if it's actually safer to keep it limited in scope for now).

4.) Is there any danger that any of the existing drivers are returning junk values in 'id' that would cause a violation of the unique key in the cat_id field and thus break the system when this change is introduced? (Perhaps not likely, but possible if there's a dummy value being used anywhere).

5.) Should we make a list of supported and unsupported drivers? I'm happy to help with this -- just not sure how much time you've already put into studying the existing drivers.

Thanks!

[EreMaijala]

2.) Oops, now added. Not tested, and help with testing Postgresql would be appreciated.

3.) Since the stored credentials are used to perform a patronLogin() request (that returns the ID), I don't think storing cat_id with them provides anything useful, unless we significantly change the way user authentication against the ILS works. And I don't think we should, apart from maybe caching patronLogin results in the user's session for a short while to avoid repeating patronLogin excessively.
I think cat_id should be considered an accessory to username. All of this wouldn't be needed if VuFind had originally used patron id as username in VuFind's database, but that's a choice made a long time ago, and there's not much that can be done now.

4.) As far as I can see all the drivers in master return something sensible in the 'id', if anything. Only suspect is Symphony, where the field to be used is configurable, and while the comments in Symphony.ini give only two unique-sounding options, someone could, in theory, have selected something else.

5.) That's a good idea. I composed the following lists:

Drivers that return a proper unique ID:

• Aleph (unless password is null???)
• Evergreen
• Koha
• KohaILSDI
• NewGenLib
• PAIA (?)
• Polaris
• Symphony (depends on config)
• Virtua
• Voyager/VoyagerRestful
• XCNCIP2

Drivers that return barcode:

• Amicus
• Clavius
• Demo (and it's probably a good idea to keep it that way!)
• Horizon
• Innovative
• LBS4
• Symphony (depends on config)
• Unicorn

[demiankatz]

Thanks for sharing the driver list and other details. I've added a TODO above to test this with PostgreSQL and will work on that when time permits.

[EreMaijala]

Tested with PostgreSQL. #930 is required for library card handling to work properly on initial login (not related to changes here).

[EreMaijala]

I updated the database change log and added some documentation to the patronLogin method at https://vufind.org/wiki/development:plugins:ils_drivers?&#patronlogin.

[demiankatz]

Looks good. I adjusted the pgsql.sql file to include a unique key on the new field to match mysql.sql; I tested that it works correctly in PostgreSQL. Thanks again!