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: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: master
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: 2020-05-09 04:40 UTC (History)
11 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Who signed the patch off:
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.