Currently, the system supports match points by -email, -userid, and -cardnumber. Many external authorization systems have the authentication by phone number, which is also required for libraries.
Created attachment 160122 [details] [review] Bug 35617: Extending the matchpoint in "identity_providers" by phone number Update identity_providers to include 'phone' as a matchpoint, also as option in the dropdown menu. Further when authentication phase 'phone' will be used as the matchpoint like email, userid, or cardnumber.
The new match point for -phone numbers will function exactly as the -email match point as it is implemented now in master with known imperfection: It is known that Koha does not check for email uniqueness for now even if it's a matchpoint, so same behavior present with a phone number field. Making Koha check for uniqueness for any of both fields is much more complicated architecture-wise and should be solved later as a separate task, but this does not limit current use for libraries that already have email or phone as matchpoints anyway.
Hi Slava, I've worked with identity providers before so I'm willing to test this and sign it off. Do you think you could provide a step by step test plan? It would greatly speed up the process.
Hi Olivier! Thanks for your willingness to test. Here is a brief plan for testing the patch. This will help verify that the "Matchpoint" field includes the "phone" option and that changes with this field are saved correctly: 1. Go to the "Identity providers" settings from the administration page. 2. Select "+ New identity provider". 3. Fill in the required fields with test data. You can use the "Add default OAuth configuration" and "Add default OAuth mapping" hints for the Configuration and Mapping fields, respectively. 4. Ensure that the "Matchpoint" field now has a new option — "phone", and select it. 5. Save the "Identity provider" settings. 6. Edit the newly created provider and verify that the settings are saved correctly. Further testing requires an external provider that sends a phone number, such as a banking identity service. I hope this helps!
I usually try and use Koha Testing Docker with Keycloak for testing SSO authentication and patches that touch on SSO. Can you look at https://wiki.koha-community.org/wiki/Testing_SSO#Keycloak and let me know if you think I could test the whole process, from adding the identity provider to authenticating with a user and having them match on Koha through their phone number? Although the code change in Koha is relatively small and benign, I'm reluctant to sign off on your patch if I cannot test the entire workflow.
Created attachment 192394 [details] [review] Bug 35617: Extending the matchpoint in "identity_providers" by phone number Update identity_providers to include 'phone' as a matchpoint, also as option in the dropdown menu. Further when authentication phase 'phone' will be used as the matchpoint like email, userid, or cardnumber. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 192395 [details] [review] Bug 35617: Fix file permission and tidy - Fixed file permission - Make output use say_success - Clean-up comments and tidy Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
0) Conflict There was a minor conflict in identity_providers.tt that I resolved. 1) QA checks (minor) Some minor QA test fails (tidyness and file permission): Processing files [RUNNING] api/v1/swagger/definitions/identity_provider.yaml [RUNNING] installer/data/mysql/atomicupdate/extend_identity_providers.pl [RUNNING] installer/data/mysql/kohastructure.sql [RUNNING] koha-tmpl/intranet-tmpl/prog/en/modules/admin/identity_providers.tt [PASS] api/v1/swagger/definitions/identity_provider.yaml [FAIL] installer/data/mysql/atomicupdate/extend_identity_providers.pl [PASS] koha-tmpl/intranet-tmpl/prog/en/modules/admin/identity_providers.tt [PASS] installer/data/mysql/kohastructure.sql Result: [PASS] api/v1/swagger/definitions/identity_provider.yaml [FAIL] installer/data/mysql/atomicupdate/extend_identity_providers.pl FAIL file_permissions File must have the exec flag FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl installer/data/mysql/atomicupdate/extend_identity_providers.pl` [PASS] installer/data/mysql/kohastructure.sql [PASS] koha-tmpl/intranet-tmpl/prog/en/modules/admin/identity_providers.tt -> Fixed these in a follow-up. Also included say_success and removed some unneeded comments from database update. 2) Code review (minor) Nothing else spotted. 3) Testing - updatedatabase - dbic - yarn build - restart_all Verified that the matchpoint phone can be configured, saves correctly and can be edited. This all works as expected. I didn't verify that the matchpoint actually works, but I believe it's enough for a SO at this point.
I thought I'd commented on this before. This seems like it could be awfully error prone, and I imagine it would be common for phone numbers to be shared across accounts...
(In reply to David Cook from comment #9) > I thought I'd commented on this before. This seems like it could be awfully > error prone, and I imagine it would be common for phone numbers to be shared > across accounts... The same is true for email which is already a valid matchpoint. I'd think it would be up to the organisation to decide what works best with their data?