Bug 7174 - Authentication rewriting
Summary: Authentication rewriting
Status: BLOCKED
Alias: None
Product: Koha
Classification: Unclassified
Component: Authentication (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Olli-Antti Kivilahti
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 14540
Blocks: 14843 7119 13920
  Show dependency treegraph
 
Reported: 2011-11-06 04:27 UTC by Paul Poulain
Modified: 2017-05-08 17:10 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Large patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 7174 - Authentication rewriting - I have come here to chew bubblegum and kick ass... (10.79 KB, patch)
2015-06-26 14:21 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 7174 - Authentication rewriting - LDAP Component - WIP (8.48 KB, patch)
2015-06-26 14:59 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 7174 - Authentication rewriting - Looking into generating a superuser and encapsulating it nicely. (4.96 KB, patch)
2015-06-26 15:43 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 7174 - Authentication rewriting - Things are getting more serious. And problematic. Deprecating lots of stuff. Encapsulating authentication behaviour into nice reusable modules. Wondering about all the tiny flags and conditionals. Creating the first a (41.55 KB, patch)
2015-06-29 16:10 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 7174 - Authentication rewriting - Things are getting more serious. And problematic. (49.57 KB, patch)
2015-06-30 14:00 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
AuthenticationRewrite schematic (171.24 KB, image/jpeg)
2015-07-22 10:53 UTC, Olli-Antti Kivilahti
Details
Bug 7174 - Authentication Rewrite (80.37 KB, patch)
2015-07-22 11:25 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 7174 - Authentication Rewrite (82.27 KB, patch)
2015-07-24 14:04 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 7174 - Authentication Rewrite (96.07 KB, patch)
2015-07-31 10:35 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 7174 - Authentication Rewrite - Core (65.69 KB, patch)
2015-08-03 12:12 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 7174 - Authentication Rewrite - Wrapper for C4::Auth (30.61 KB, patch)
2015-08-03 12:12 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 7174 - Authentication Rewrite - Core (66.06 KB, patch)
2015-08-06 15:40 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 7174 - Authentication Rewrite - Core (66.11 KB, patch)
2015-08-17 12:01 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 7174 - Authentication Rewrite - Wrapper for C4::Auth (30.61 KB, patch)
2015-08-17 12:01 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 7174 - Authentication Rewrite - Core (58.39 KB, patch)
2017-05-08 17:10 UTC, Lari Taskula
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Poulain 2011-11-06 04:27:27 UTC
Auth.pm does too many things, it must be splitted in smaller parts.
In 3.6, it does :
    - authentifying
    - checking & setting permissions
    - setting a lot of syspref available on each template (many of them
being unused in most templates)
    - retrieving template

How to do it is still to be discussed. This entry is low priority, once we will have plack, for example, it may be obvious to use Plack authentication tools.
Comment 1 Dobrica Pavlinusic 2012-04-03 14:23:13 UTC
If we get rid of C4::Auth for plack auth layer, we would loose ability to execute Koha with CGI. Is this a goal?

I would love to get some work done on existing C4::Auth. Bug 7810 mentions possible improvement in our cookie usage for example. Would it make sense to open bug for it and attach dependency to this one?
Comment 2 Paul Poulain 2012-04-03 14:58:57 UTC
(In reply to comment #1)
> If we get rid of C4::Auth for plack auth layer, we would loose ability to
> execute Koha with CGI. Is this a goal?

I think that could be a goal, but not a short term one.
My feeling, but it's just a feeling is that:
* 3.8 will have plack support as experimental
* 3.10 will have plack support by default
* 3.12 will have plack support only.

I can't see a valid reason to keep CGI mode for Koha. This is much different from zebra/solr for example, where there can be some valid arguments to keep both search engines.

PS: this bug had been created in Mumbai, during hackfest11
Comment 3 Ian Walls 2012-04-03 15:10:14 UTC
I don't believe that we should wed Koha to Plack as a Fast CGI tool anymore than we should be wedded to MySQL as our database.  If we aim to implement PSGI as our standard, we'd have Koha working on Plack, FastCGI or mod_perl, which would let the institution doing the hosting choose how to set up their machines.

As for keeping CGI... I'm not sure whether we need to or not, if we've got a good selection of PSGI options that can run on a wide range of hardware.  So long as the move away from CGI doesn't disenfranchise anyone running Koha on an older or low-spec machine, I think it could be okay.
Comment 4 Paul Poulain 2012-04-05 15:38:58 UTC
(In reply to comment #3)
> I don't believe that we should wed Koha to Plack as a Fast CGI tool anymore
> than we should be wedded to MySQL as our database.  If we aim to implement
> PSGI as our standard, we'd have Koha working on Plack, FastCGI or mod_perl,
> which would let the institution doing the hosting choose how to set up their
> machines.
That's a valid point. OTOH I think we must *highly* improve our authentification workflow (read: rewrite it completely ;-) )
So if we can find a solution for that ...

> As for keeping CGI... I'm not sure whether we need to or not, if we've got a
> good selection of PSGI options that can run on a wide range of hardware.  So
> long as the move away from CGI doesn't disenfranchise anyone running Koha on
> an older or low-spec machine, I think it could be okay.

well= I have absolutely no doubts that Koha+Plack will work *better* on low-spec machines than in CGI !
1 thread, keep alive for 30 requests means, according to dobrika, 60MB
On a low-spec machine, the number of Plack threads can be limited to 3 or 2 instead of 6, to reach a low memory consumption.
(I don't count the plack itself here. Dobrika, if you can give us numbers...
Comment 5 Olli-Antti Kivilahti 2015-06-25 15:26:05 UTC
I have been looking into this issue because of Bug 13799 and 13920.
I have been analyzing the C4::Auth and draw a specification to streamline it a LOT.
https://docs.google.com/drawings/d/15w-P9rF_2lJIXX_vda-tPDzpB2ZMt02coMJofKjJcfo/edit
Comment 6 Olli-Antti Kivilahti 2015-06-26 14:21:26 UTC Comment hidden (obsolete)
Comment 7 Olli-Antti Kivilahti 2015-06-26 14:59:23 UTC Comment hidden (obsolete)
Comment 8 Olli-Antti Kivilahti 2015-06-26 15:43:40 UTC Comment hidden (obsolete)
Comment 9 M. Tompsett 2015-06-26 15:55:10 UTC
Comment on attachment 40678 [details] [review]
Bug 7174 - Authentication rewriting - LDAP Component - WIP

Review of attachment 40678 [details] [review]:
-----------------------------------------------------------------

::: C4/Auth_with_ldap.pm
@@ +115,2 @@
>  sub checkpw_ldap {
> +    my ($userid, $password) = @_;

If we are rewriting, could we please hashref everything? :)
Comment 10 Olli-Antti Kivilahti 2015-06-29 12:45:45 UTC
(In reply to M. Tompsett from comment #9)
> Comment on attachment 40678 [details] [review] [review]
> Bug 7174 - Authentication rewriting - LDAP Component - WIP
> 
> Review of attachment 40678 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: C4/Auth_with_ldap.pm
> @@ +115,2 @@
> >  sub checkpw_ldap {
> > +    my ($userid, $password) = @_;
> 
> If we are rewriting, could we please hashref everything? :)

I would like to hear more about the benefits of hashreffing?
Comment 11 Olli-Antti Kivilahti 2015-06-29 16:10:31 UTC Comment hidden (obsolete)
Comment 12 Olli-Antti Kivilahti 2015-06-30 14:00:07 UTC Comment hidden (obsolete)
Comment 13 Olli-Antti Kivilahti 2015-07-22 10:53:53 UTC
Created attachment 41117 [details]
AuthenticationRewrite schematic
Comment 14 Olli-Antti Kivilahti 2015-07-22 11:25:29 UTC Comment hidden (obsolete)
Comment 15 M. Tompsett 2015-07-22 13:10:15 UTC
Comment on attachment 41124 [details] [review]
Bug 7174 - Authentication Rewrite

Review of attachment 41124 [details] [review]:
-----------------------------------------------------------------

What about CAS? About PKI authentication? Until someone write instructions on how to set those up for testing, I fear something is likely broken.
Comment 16 Olli-Antti Kivilahti 2015-07-22 14:00:24 UTC
(In reply to M. Tompsett from comment #15)
> Comment on attachment 41124 [details] [review] [review]
> Bug 7174 - Authentication Rewrite
> 
> Review of attachment 41124 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> What about CAS? About PKI authentication? Until someone write instructions
> on how to set those up for testing, I fear something is likely broken.

Very true. The C4::Auth subroutine interfaces haven't changed at all so there "should" be no issues.
It would be great if somebody who knows how to set them up could help with this.
Comment 17 Olli-Antti Kivilahti 2015-07-24 14:04:05 UTC Comment hidden (obsolete)
Comment 18 Olli-Antti Kivilahti 2015-07-31 10:35:31 UTC Comment hidden (obsolete)
Comment 19 Olli-Antti Kivilahti 2015-08-03 12:12:26 UTC Comment hidden (obsolete)
Comment 20 Olli-Antti Kivilahti 2015-08-03 12:12:49 UTC Comment hidden (obsolete)
Comment 21 Olli-Antti Kivilahti 2015-08-06 15:40:27 UTC Comment hidden (obsolete)
Comment 22 Olli-Antti Kivilahti 2015-08-17 12:01:26 UTC
Created attachment 41536 [details] [review]
Bug 7174 - Authentication Rewrite - Core
Comment 23 Olli-Antti Kivilahti 2015-08-17 12:01:43 UTC
Created attachment 41537 [details] [review]
Bug 7174 - Authentication Rewrite - Wrapper for C4::Auth
Comment 24 Lari Taskula 2017-05-08 17:10:12 UTC
Created attachment 63255 [details] [review]
Bug 7174 - Authentication Rewrite - Core

Depends heavily on Buugg 14540, which introduces many tests to spot regression
caused by this feature.

Introduces a new modular authentication system based on Exception signaling
and reusable authentication components/challenges.
This system is cross-framework -compatible, currently supporting Mojolicious
and CGI, but adding support for any other framework is straightforward.

Central idea is to provide a single authentication function to deal with all
types of authentication scenarios, REST API, Cookie, Password, LDAP...
Also it is important to make a system that is easy to extend and can deal with
many future authentication scenarios.

Currently only CGI password, LDAP and cookie login is tested. Legacy behaviour
is used to deal with other types of authentication.

See the attached schematic in Bugzilla for a more architectural overview.

Also fixes few bugs related to anonymous_search_history

The basic principle is:
1. We get an authentication request from any Web/GUI-framework, for ex CGI.
2. Request is normalized by extracting all necessary authentication data
   elements to a separate data structure.
3. Based on the found authentication data elements, system decides which
   authentication route to take.
4. Route implements all challenges needed to authenticate the request.
   Route returns the authenticated Koha::Borrower or an Exception if login
   failed.
5. The user environment/session is set/deleted based on the Route result.
6. a Koha::Borrower and the CGISESSID-cookie is returned to the calling
   framework in the format the framework needs.
7. Framework needs to catch possible exceptions and deal with them.
   Eg. login failed, no permission, under maintenance.
8. Authentication succeeds and session is set, or failure is reported to user.