Bug 18315 - authentication and authorization refactoring
Summary: authentication and authorization refactoring
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Authentication (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Francesco Rivetti
QA Contact: Testopia
URL: https://github.com/digibib/Koha/commi...
Keywords:
Depends on:
Blocks:
 
Reported: 2017-03-22 14:31 UTC by Francesco Rivetti
Modified: 2024-02-15 02:19 UTC (History)
8 users (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Francesco Rivetti 2017-03-22 14:31:30 UTC
Currently, C4::Auth is responsible to do session management, authentication and authorization.

There is no clear separations between them, and the authentication is not modular, making it increasingly harder to maintain and/or introduce no authentication integrations.

I was experimenting on this for a while, and I managed to put together few commits in a branch.

It started as a proposal, but turned out to be working in _most_ of my tests.

here more details:

I completely removed any need for CGI and CGI::Session while maintaining a fork-exec aproach to the scripts. (This made it easier to implement Koha::Middleware::Session, Koha::Session::DB and ::KohaCache)

Koha::Middleware::Session inject a session object in the Plack request, and invoke Koha::Auth if any authentication tokens are available.

Koha::Auth has a list of modules which will be invoked in turn and could be enabled/disabled

plack.psgi is changed as well, it has enabled the new Middleware and the applications used are no longer Plack::App::CGIBin but Koha::Middleware::App.


here few of the things i want to do to simplify the interaction with the current "legacy" code:

I didn't change the scripts or any other module at all, instead I tried to hijack C4::Auth methods to rely on Koha::Session and Koha::Auth. This make it simple to switch back to the previous plack.psgi OR using apache and the normal cgi-bin.

authentication should happen upfront, before the script is even started. script will have to verify authorization.

authentication MUST be modular, possibly having different systems cooperate. (Some serious discussions must be around this point, especially about accounting and authority)


what needs to be done:

permissions should be reintroduced. right now permissions are read for the user and set in the template, but no checks are perform on requests. (should be fairly simple to do)

reintroduce the code related to shib, cas and any other.

more testing, especially for the installer (there is lots of code regarding the web installer which isn't there anymore)
Comment 1 Francesco Rivetti 2017-03-22 14:39:57 UTC
regarding the web installer, C4::Auth has few places where it returns "maintenance" which i believe happens during the web installer or immediately after. I clean up that part and I must reintroduce it back.
Comment 2 Francesco Rivetti 2017-03-22 14:58:37 UTC
so far, things that does NOT work:

- redirects are probably broken
- Koha::Auth::Simple tries to authenticate you when you send userid/passwd even when those are sent to create a new user
- logout.x fights with login
- permissions are there (and with the fixed JOIN) but only used for templates, not for authorization
- CGI is mocked, and Session is new. So I expect few modules to land to not implemented methods there. should be easy to add those
Comment 3 Francesco Rivetti 2017-03-27 13:06:54 UTC
added few more fixes:
- honor cgi->redirect()
- trap exit() and instead returns what has been sent so far
- added session->remote_addr()
- added branchname in session and LoginBranchName in templates
Comment 4 Francesco Rivetti 2017-03-29 07:11:56 UTC
this is something i forgot to mention before:

I replaced WrapCGI and CGIBin with Koha::Middleware::App 

It pretty much do the same thing as WrapCGI (fork and require the cgi script) but instead of having the script building the response, it uses a pipe and serialize the data back to the parent.

It's the first step to move the logic from the scripts to proper functions.

It handles script exit() properly, and should work properly with other forks, which make it relevant to Bug 15032
Comment 5 Olli-Antti Kivilahti 2017-04-04 13:02:11 UTC
Hi!

Check out

https://github.com/KohaSuomi/kohasuomi/blob/kohasuomi3.16/Koha/Auth/PermissionManager.pm
https://github.com/KohaSuomi/kohasuomi/blob/kohasuomi3.16/Koha/Auth.pm
https://github.com/KohaSuomi/kohasuomi/blob/kohasuomi3.16/Koha/Auth/Route/Password.pm
https://github.com/KohaSuomi/kohasuomi/blob/kohasuomi3.16/C4/Auth.pm#L1713


I rewrote the permission system and authentication system some time ago. Nobody is interested about it, even thought the horrible way of managing permissions is a blocker for writing smooth user interface tests (or REST API tests). Not a lot of people care about user interface tests either.

We have been indecisive should we drop my authentication rewrite and revert to the community system, but it is so stale and in a need of a rewrite that we couldn't get back into it.

I hope you can work on top of my solution and improve it, instead of reinventing the wheel.
It would be great to get best of both worlds.
My solution has a compatibility layer between CGI and Mojolicious, normalizing the way HTTP request headers/cookies/parameters are fetched.
It would be bestest to get rid of CGI completely and just rewrite everything as a Mojolicious application.


Also ashimema and tcohen are working with OAuth2.0 and replacing CGISESSID with JWT. Not sure how they can do it with the current C4::Auth. Maybe they will provide a third and a fourth rewrite :) (I hope not)

BTW: Where is your code? I see no attachments?
My work in Bugzilla: Bug 7174 - Authentication rewriting
Comment 6 Olli-Antti Kivilahti 2017-04-04 13:03:53 UTC
(In reply to Olli-Antti Kivilahti from comment #5)

> BTW: Where is your code? I see no attachments?

But I found the URL to github.
Comment 7 Francesco Rivetti 2017-04-04 13:07:30 UTC
Hi Olli-Antti Kivilahti,

I didn't know you were working on it, will have a look immediately.

You are right: I haven't submitted any patch here. But you can see what i did so far here: https://github.com/digibib/Koha/commits/auth_session_refactor

there are lots of changes on my side, but I might be able to combine what you did so far and integrate it. I just wish there were more hours in a day!
Comment 8 Olli-Antti Kivilahti 2017-05-04 08:52:45 UTC
I am relieved to hear that.

This auth work just nudged forward in our dev pipeline and Lari is working with it atm.

I hope you can work together to bring together the best of both worlds.
Comment 9 David Cook 2020-10-28 06:14:14 UTC
I have a feeling that a lot of people are thinking about this and maybe working on it separately.

It's too bad that we don't have more coordination across distant developers...