Bug 17004 - REST API: add route to authenticate patron (Single Sign On - SSO)
Summary: REST API: add route to authenticate patron (Single Sign On - SSO)
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P3 enhancement (vote)
Assignee: Bugs List
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks: 16652
  Show dependency treegraph
 
Reported: 2016-07-31 11:03 UTC by Jiri Kozlovsky
Modified: 2023-06-25 11:51 UTC (History)
16 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Municipal Libray Ceska Trebova
Version(s) released in:


Attachments
Bug 17004: Add API route to authenticate patron (CGISESSID) (10.47 KB, patch)
2016-08-05 10:58 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17004: Add API route to authenticate patron (10.62 KB, patch)
2016-08-06 12:53 UTC, Jiri Kozlovsky
Details | Diff | Splinter Review
Bug 17004: Add API route for logging out user (5.38 KB, patch)
2016-08-22 12:09 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17004: Add API route for logging out user (5.38 KB, patch)
2016-08-22 12:10 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17004: Add API route for logging out user (5.87 KB, patch)
2016-08-22 12:25 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17004: Add API route for logging out user (5.94 KB, patch)
2016-08-22 15:13 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17004: Add API route to authenticate patron (CGISESSID) including logout (88.90 KB, patch)
2016-08-28 19:02 UTC, Jiri Kozlovsky
Details | Diff | Splinter Review
Bug 17004: Add API route to authenticate patron (CGISESSID) including logout (88.97 KB, patch)
2016-08-28 19:32 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 17004: Add API route to authenticate patron (CGISESSID) including logout (16.06 KB, patch)
2016-09-02 21:57 UTC, Benjamin Rokseth
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jiri Kozlovsky 2016-07-31 11:03:45 UTC
Add API route to authenticate patron. 

This would be very useful for 3rd party applications, such as NCIP's LookupUser service or simpleSAMLphp IdP for eduGAIN.
Comment 1 Lari Taskula 2016-08-04 10:22:49 UTC
We have implemented userid/cardnumer & password CGISESSID-authentication. I will rebase it on master and upload the patch here.
Comment 2 Jiri Kozlovsky 2016-08-05 10:56:11 UTC
(In reply to Lari Taskula from comment #1)
> We have implemented userid/cardnumer & password CGISESSID-authentication. I
> will rebase it on master and upload the patch here.

That's nice! So don't you mind me to put your solution here on bugzilla rebased on master with current REST API conventions?

I've found your implementation here:
https://github.com/KohaSuomi/kohasuomi/blob/kohasuomi3.16/api/v1/swagger/paths/auth.json (the POST method)

I think it would go quiet quick if I omit your dependencies of the PageObject-tests.
Comment 3 Lari Taskula 2016-08-05 10:58:23 UTC Comment hidden (obsolete)
Comment 4 Lari Taskula 2016-08-05 11:03:55 UTC
(In reply to Jiri Kozlovsky from comment #2)
> (In reply to Lari Taskula from comment #1)
> > We have implemented userid/cardnumer & password CGISESSID-authentication. I
> > will rebase it on master and upload the patch here.
> 
> That's nice! So don't you mind me to put your solution here on bugzilla
> rebased on master with current REST API conventions?
> 
> I've found your implementation here:
> https://github.com/KohaSuomi/kohasuomi/blob/kohasuomi3.16/api/v1/swagger/
> paths/auth.json (the POST method)
> 
> I think it would go quiet quick if I omit your dependencies of the
> PageObject-tests.
Since our authentication is based on Authentication rewrite (Bug 13920 and its dependencies), it's a bit too much work for me to rebase here now, so instead I decided to write a simple patch from scratch using session, that you can use for testing if you wish. You can mark that patch obsolete if you wish to work on this Bug more!
Comment 5 Jiri Kozlovsky 2016-08-06 12:53:46 UTC Comment hidden (obsolete)
Comment 6 Jiri Kozlovsky 2016-08-06 12:57:46 UTC
Comment on attachment 54028 [details] [review]
Bug 17004: Add API route to authenticate patron (CGISESSID)

My Sign-Off failed to mark this attachment Obsolete, so I'm now doing it manually
Comment 7 Lari Taskula 2016-08-22 12:09:12 UTC Comment hidden (obsolete)
Comment 8 Lari Taskula 2016-08-22 12:10:46 UTC Comment hidden (obsolete)
Comment 9 Lari Taskula 2016-08-22 12:11:54 UTC
(In reply to Lari Taskula from comment #7)
> Current status: Signed Off
Uncommented wrong line in git bz attach. Current status is Needs Signoff.
Comment 10 Lari Taskula 2016-08-22 12:25:21 UTC Comment hidden (obsolete)
Comment 11 Lari Taskula 2016-08-22 15:13:37 UTC Comment hidden (obsolete)
Comment 12 Benjamin Rokseth 2016-08-22 21:59:26 UTC
wow, amazing work! very much like the code we use in production, but better! AND with test coverage ;)

Would it be possible to also supply patron permissions in the response? I found the easiest way was to just iterate against haspermissions thus:

+   my $permissions = haspermission($userid); # defaults to all permissions
+   # delete all empty permissions
+   while ( my ($key, $val) = each %{$permissions} ) {
+       delete $permissions->{$key} unless $val;
+   }

then you could just return $permission as a hash object
Comment 13 Lari Taskula 2016-08-23 11:38:04 UTC
(In reply to Benjamin Rokseth from comment #12)
> wow, amazing work! very much like the code we use in production, but better!
> AND with test coverage ;)
> 
> Would it be possible to also supply patron permissions in the response? I
> found the easiest way was to just iterate against haspermissions thus:
> 
> +   my $permissions = haspermission($userid); # defaults to all permissions
> +   # delete all empty permissions
> +   while ( my ($key, $val) = each %{$permissions} ) {
> +       delete $permissions->{$key} unless $val;
> +   }
> 
> then you could just return $permission as a hash object
Thank you for the kind support, Benjamin! That made my day!

I think permissions could be useful information to provide in the response. Feel free to provide enhancement patches, I would be happy to sign-off them!
Comment 14 Jiri Kozlovsky 2016-08-28 19:02:24 UTC Comment hidden (obsolete)
Comment 15 Benjamin Rokseth 2016-08-28 19:32:26 UTC Comment hidden (obsolete)
Comment 16 Benjamin Rokseth 2016-08-28 19:37:08 UTC
Shit man, you beat me with five minutes! Was just rebasing the same patch ;)

Great work, Lari and Jiri! Everything works as expected.

Just a sidenote: perhaps we should .gitignore swagger.min.json, or we will face a whole lot of merge issues? I propose this in a new bug. Part of the testing should be to minify anyways, as it will spot issues in references.
Comment 17 Jiri Kozlovsky 2016-08-28 19:42:31 UTC
(In reply to Benjamin Rokseth from comment #16)
> Shit man, you beat me with five minutes! Was just rebasing the same patch ;)
> 
> Great work, Lari and Jiri! Everything works as expected.
> 
> Just a sidenote: perhaps we should .gitignore swagger.min.json, or we will
> face a whole lot of merge issues? I propose this in a new bug. Part of the
> testing should be to minify anyways, as it will spot issues in references.

Well then, sorry about that :D I didn't know you're working on it .. maybe an IRC channel would solve such interference?

I agree with ignoring all possible minified files in Koha in general. Are there currently any other minified files? I can't think of any right now.
Comment 18 Benjamin Rokseth 2016-09-02 21:57:56 UTC
Created attachment 55146 [details] [review]
Bug 17004: Add API route to authenticate patron (CGISESSID) including logout

POST /auth/session (login)
DELETE /auth/session (logout)

Required POST data:
- "password"
- either "userid" or "cardnumber".

To test:
1. Make sure you are logged out from Koha.
2. Make a POST request to http://yourlibrary/api/v1/auth/session with form data
   "userid" => <your_user_id> and "password" => <your_password>.
3. If your userid and password is correct, you should be returned with most
   basic patron data and your CGISESSID.
4. Also attempt with invalid login to get an error.
5. Send a DELETE request to /auth/session
6. Observe that you are no longer logged-in in Koha.
7. Run tests at b/t/db_dependent/api/v1/auth.t

You may find this cURL useful:
curl -X DELETE http://lib/api/v1/auth/session --cookie 'CGISESSID=88e735aaf7c6cf194a775425cbd00570'
(replace CGISESSID=... with your CGISESSID)

----

I've rebased both attachments to master (restructuralized swagger
definitions).

And added proposed permissions as proposed by Benjamin Rokseth.

Btw, very nice work Lari!

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 19 Benjamin Rokseth 2016-09-02 21:59:12 UTC
Rebased against master.
Basically just removed swagger.min.json from the patch.

And added Bug 17243
Comment 20 Benjamin Rokseth 2016-10-13 22:13:10 UTC
Need more to sign-off/QA this one! It's essential for the REST API development, and it's basically nothing new other than a better replacement of the svc authentication with a more sensible authentication approach.
Comment 21 Martin Renvoize 2016-10-28 09:37:58 UTC
I don't like this much.. We're in-explicitly mixing Authentication (Are you who you say you are) and Authorization (What can this person/application on behalf of person do)

At the very least I believe these functions should be made distinct to prevent leaking security context.

The second reason I don't like this is that we're inventing out own wheel. There are lots of solid standards out there to do this sort of thing. We should really be leaning on the shoulders of giants and using an off the shelf standard. OAuth and OpenID connect would be my preferred option personally.
Comment 22 Benjamin Rokseth 2016-11-08 17:44:30 UTC
(In reply to Martin Renvoize from comment #21)
> I don't like this much.. We're in-explicitly mixing Authentication (Are you
> who you say you are) and Authorization (What can this person/application on
> behalf of person do)
>

I believe the main point here is authenticating a service outside koha against its userbase and services, although of course Koha intra and opac should profit from this.

> At the very least I believe these functions should be made distinct to
> prevent leaking security context.
> 
No problem with that, as long as authorization is handled one place only.

> The second reason I don't like this is that we're inventing out own wheel.
> There are lots of solid standards out there to do this sort of thing. We
> should really be leaning on the shoulders of giants and using an off the
> shelf standard. OAuth and OpenID connect would be my preferred option
> personally.

Now this could be debated, openID and oAuth are easy to integrate against and are well formed standards, and would of course lower the barrier to use koha services outside the library without handling login in koha.

But to be sure, local login needs to be handled anyways, and to use our library as an example, having a local patron base is a strength rather than a weekness. Being a user of the library implies a presence and represents a place outside the web, and I'm not sure SSO with oauth gives the same sensation.

Just my two cent thoughts :) Now to rebase this against master...
Comment 23 Martin Renvoize 2016-11-11 07:49:35 UTC
I was actually suggesting Koha act as an OAuth/OpenID Connect provider rather than just a consumer.

There's  pretty solid Mojolicious Plugin we could use that gives us a chunk of this out of the box: https://metacpan.org/pod/Mojolicious::Plugin::OAuth2::Server

It's using something like this that I'm advocating ;)  So, when logging in the user would get directed to a Koha login page and then redirected back to whatever SSO consumer they came from.  It's clearer to the user what login they should be using and who the original party is that's storing their authentication credentials.

OAuth is of course the 'Authorization' side of the story, we'd still need to work out the OpenID Connect side of the story to bring this back down to simple 'user identification'.
Comment 24 Martin Renvoize 2016-11-11 07:51:16 UTC
To be clear, I'm trying not to be negative here.. I'm actually really keen for this work.. it's just I'd like to see it done 'right' in a commonly accepted manor so we stand the best chance possible for the api to be adopted in the wider world ;)
Comment 25 Benjamin Rokseth 2016-11-11 09:31:14 UTC
Ah, so I totally misunderstood ;)

What you suggest totally makes sense! Hope to see this sooner than later. Unfortunately I'm too deep in other bugs to be any help here at the moment, but would gladly assist in testing/signing off!
Comment 26 Martin Renvoize 2016-11-11 10:36:48 UTC
Well..  I'm bogged down at the minute.. but doing this has been on my todo list for nearly a year ;)

If I get a chance, I might try and take some time over christmas to work on it.
Comment 27 Marcel de Rooy 2017-06-16 09:33:31 UTC
(In reply to Martin Renvoize from comment #26)
> Well..  I'm bogged down at the minute.. but doing this has been on my todo
> list for nearly a year ;)
> 
> If I get a chance, I might try and take some time over christmas to work on
> it.

Moving this to In Discussion in view of the comments above. Sorry..
Martin: Could you tell us which christmas you will be working on it? :)
Comment 28 Tomás Cohen Arazi 2018-05-14 15:51:09 UTC
(In reply to Marcel de Rooy from comment #27)
> (In reply to Martin Renvoize from comment #26)
> > Well..  I'm bogged down at the minute.. but doing this has been on my todo
> > list for nearly a year ;)
> > 
> > If I get a chance, I might try and take some time over christmas to work on
> > it.
> 
> Moving this to In Discussion in view of the comments above. Sorry..
> Martin: Could you tell us which christmas you will be working on it? :)

LOL
Comment 29 Johanna Räisä 2019-07-08 12:18:37 UTC
(In reply to Martin Renvoize from comment #23)
> I was actually suggesting Koha act as an OAuth/OpenID Connect provider
> rather than just a consumer.
> 
> There's  pretty solid Mojolicious Plugin we could use that gives us a chunk
> of this out of the box:
> https://metacpan.org/pod/Mojolicious::Plugin::OAuth2::Server
> 
> It's using something like this that I'm advocating ;)  So, when logging in
> the user would get directed to a Koha login page and then redirected back to
> whatever SSO consumer they came from.  It's clearer to the user what login
> they should be using and who the original party is that's storing their
> authentication credentials.
> 
> OAuth is of course the 'Authorization' side of the story, we'd still need to
> work out the OpenID Connect side of the story to bring this back down to
> simple 'user identification'.

This would be great for clarity. How about if some service would authenticate on background like SIP does?
Comment 30 Lari Taskula 2020-03-17 21:19:01 UTC
I'm no longer able to work on this, so I'm setting assignee to default. Feel free to continue this work.
Comment 31 Katrin Fischer 2021-01-25 15:34:13 UTC
I believe while what Martin suggests here is the "right" way to do it it won't be an option for many legacy systems to implement that. Is there a way we could provide both? A simple auth route that given the user credentials returns some basic information, valid/invalid and maybe a system identifier (borrowernumber) that can be used to get more information about privileges etc?
Comment 32 Martin Renvoize 2021-02-19 14:04:58 UTC
People do realise there's a 'AuthenticatePatron' option in ils-di right.. so this being blocked doesn't mean there isn't already an alternative that's been available in Koha for.. I dunno.. a decade?
Comment 33 Katrin Fischer 2021-02-22 22:37:36 UTC
(In reply to Martin Renvoize from comment #32)
> People do realise there's a 'AuthenticatePatron' option in ils-di right.. so
> this being blocked doesn't mean there isn't already an alternative that's
> been available in Koha for.. I dunno.. a decade?

We might end up going this route, but ILS-DI has its own security concerns as far as I understand.
Comment 34 Ulrich Kleiber 2021-03-17 13:42:48 UTC
I think leaving the proven path of pure doctrine is better than the alternative of using ILS-DI for patron authentication, where the password appears in the URL and thus in the Apache log files and the Plack log files.
We have legacy systems which are not part of a centralized single sign-on infrastructure. But they are part of our in-house Koha infrastructure. Our patrons do not have to give there password to a third party service.
Comment 35 Tomás Cohen Arazi 2021-03-17 13:48:36 UTC
Until we implement Koha-as-an-IdP with OAuth2 bells and whistles, it wouldn't hurt to have a way to generate a session using the API the same way the old .pl scripts do.
Comment 36 Arthur Suzuki 2021-10-28 22:57:10 UTC
(In reply to Ulrich Kleiber from comment #34)
> I think leaving the proven path of pure doctrine is better than the
> alternative of using ILS-DI for patron authentication, where the password
> appears in the URL and thus in the Apache log files and the Plack log files.
> We have legacy systems which are not part of a centralized single sign-on
> infrastructure. But they are part of our in-house Koha infrastructure. Our
> patrons do not have to give there password to a third party service.

About ILS-DI having the password in the URL, the easy fix is to have the third-party software querying Koha with HTTP-POST instead of HTTP-GET, that works.
The parameters are then not part of the URL anymore, hence not logged.
(still transfered in cleartext if https is not used though)
Comment 37 David Cook 2021-10-29 01:59:31 UTC
(In reply to Tomás Cohen Arazi from comment #35)
> Until we implement Koha-as-an-IdP with OAuth2 bells and whistles, it
> wouldn't hurt to have a way to generate a session using the API the same way
> the old .pl scripts do.

I've already done a lot of this work locally, but it's unpolished, and there's not enough... incentive(?) to upstream it at the moment.

But that's really the key. Refactoring checkauth() so that we can create a session with a function call would be great. Then we call leave up the actual authentication to the caller. 

One day I'll have all the time and money in the world to bring pluggable authentication to Koha hehe. It actually wouldn't take that much time and effort, but there are only 24 hours in the day...
Comment 38 Ulrich Kleiber 2021-10-29 05:55:02 UTC
(In reply to Arthur Suzuki from comment #36)
> (In reply to Ulrich Kleiber from comment #34)
> > I think leaving the proven path of pure doctrine is better than the
> > alternative of using ILS-DI for patron authentication, where the password
> > appears in the URL and thus in the Apache log files and the Plack log files.
> > We have legacy systems which are not part of a centralized single sign-on
> > infrastructure. But they are part of our in-house Koha infrastructure. Our
> > patrons do not have to give there password to a third party service.
> 
> About ILS-DI having the password in the URL, the easy fix is to have the
> third-party software querying Koha with HTTP-POST instead of HTTP-GET, that
> works.
> The parameters are then not part of the URL anymore, hence not logged.
> (still transfered in cleartext if https is not used though)

Thanks for your hint Arthur, it works :)
Comment 39 David Cook 2022-06-15 05:02:12 UTC
I'm going to add another wrinkle into all of this!

Since Koha isn't yet capable of being an OpenID Connect/SAML2 IDP itself, we're using the Keycloak IDAM system instead (which Red Hat very actively develops and supports).

The cool part is that I've written an extension using the User Storage SPI (https://www.keycloak.org/docs/latest/server_development/#_user-storage-spi) to use the Koha database as the User Federation provider. 

Right now, I'm using the existing REST API to GET patrons, but I need a REST API endpoint to validate the credentials entered by the user. I'll be creating a custom endpoint for that this afternoon. *Note that this isn't creating a Koha authentication session. It's just validating that the password entered by the user into Keycloak is the same password stored in the Koha database.*

So Keycloak will provide the SSO for Koha and other systems, but the real backend user database (and potentially business rules) will still happen in Koha. 

I thought about doing the REST API endpoint as a Koha plugin, but I'm going to just code it into our local Koha, so that it's easier to maintain and distribute. 

Once we've battle tested everything, I could look at sharing.
Comment 40 David Cook 2023-04-26 01:13:05 UTC
Now that bug 30962 has been pushed to 22.11.03+, I'll be using that in my Keycloak extension.

--

Also I think that bug 30962 might actually meet quite a few of the original goals outlined in this bug report...
Comment 41 Katrin Fischer 2023-06-25 11:51:03 UTC
(In reply to David Cook from comment #40)
> Now that bug 30962 has been pushed to 22.11.03+, I'll be using that in my
> Keycloak extension.
> 
> --
> 
> Also I think that bug 30962 might actually meet quite a few of the original
> goals outlined in this bug report...

It feels like this can be closed now. Please reopen if something is still missing.