Description
Paul Poulain
2011-11-06 04:27:27 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? (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 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. (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... 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 Created attachment 40672 [details] [review] Bug 7174 - Authentication rewriting - I have come here to chew bubblegum and kick ass... and I'm all out of bubblegum. WIP WIP, sharing just for people to keep up. Deprecating lots of stuff. Encapsulating authentication behaviour into nice reusable modules. Created attachment 40678 [details] [review] Bug 7174 - Authentication rewriting - LDAP Component - WIP Created attachment 40680 [details] [review] Bug 7174 - Authentication rewriting - Looking into generating a superuser and encapsulating it nicely. 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? :) (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? Created attachment 40723 [details] [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 Created attachment 40739 [details] [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 authentication route. See Koha::Auth::Route for "interface" definition. Created attachment 41117 [details]
AuthenticationRewrite schematic
Created attachment 41124 [details] [review] Bug 7174 - Authentication Rewrite 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. Deprecates a lot of subroutines in C4::Auth. See the attached schematic in Bugzilla for a more architectural overview. 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. ----------------------------------- -::- TEST PLAN: -::- ----------------------------------- Use PageObject tests from Buugg 14540 and Buugg 14536 to test against regression. ::MANUAL TESTS:: OPAC: 1. Go to opac-main.pl 1.1. Try to access a restricted page like /cgi-bin/koha/opac-account.pl 2. Login with password. 3. Navigate to any OPAC page requiring login. 4. Observe that the login is still valid and correct username is displayed on top right of the browser window. 5. Logout 5.1. Try to access a restricted page. 6. Login again. 7. Browse a bit. 8. Logout. Anonymous search history (OPAC): 1. Make a search 2. Make another search 3. Login 4. Make yet another search 5. Go to my search history. 6. Observe that the last three searches are shown in your search history. 7. Logout STAFF CLIENT (Intra): 0. Try to access a restricted page, like /cgi-bin/koha/mainpage.pl 1. Login 2. Navigate a bit to confirm that the login remains active. 3. Logout. 4. Login again. 5. Navigate to a restricted page. 6. Logout. 6.1. Try to access a restricted page. 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. (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. Created attachment 41159 [details] [review] Bug 7174 - Authentication Rewrite Created attachment 41256 [details] [review] Bug 7174 - Authentication Rewrite Created attachment 41311 [details] [review] Bug 7174 - Authentication Rewrite - Core Created attachment 41312 [details] [review] Bug 7174 - Authentication Rewrite - Wrapper for C4::Auth Created attachment 41413 [details] [review] Bug 7174 - Authentication Rewrite - Core Fixed Cookie authentication to work behind a reverse proxy Created attachment 41536 [details] [review] Bug 7174 - Authentication Rewrite - Core Created attachment 41537 [details] [review] Bug 7174 - Authentication Rewrite - Wrapper for C4::Auth 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. It looks like you put a lot of effort in there, Lari! I'm looking to do something much less ambitious. My current plan is to refactor C4::Auth, so that the functionality remains the same, but we're using testable re-usable objects. My goal is to start using Mojolicious controllers (Bug 28325) to make different AuthN/AuthZ calls, and then eventually replace all the existing CGI scripts (easier done for the OPAC than the staff interface), but maybe we'd need to change get_template_and_user to act as a shim too. Anyway, interesting to see that work. Is it used anywhere yet? |