|
Description
Martin Renvoize (ashimema)
2017-04-28 08:05:01 UTC
Created attachment 62810 [details] [review] BUG 18506 - Add xShibOnly preference for shibboleth This patch adds two system preferences to Koha, opacShibOnly and staffShibOnly, allowing users to restrict authentication to just one method, Shibboleth. A system preference was chosen over a configuration file update to allow for local override at the virtualhost level. In this way a hosting provider can setup a 'backdoor opac' for example to allow fallback to local logins for support operations. Assigning bug to Martin as he has provided the patch. Depends on a bug that is in 'Failed QA' - marking as BLOCKED for now. Comment on attachment 62810 [details] [review] BUG 18506 - Add xShibOnly preference for shibboleth Review of attachment 62810 [details] [review]: ----------------------------------------------------------------- From looking at the patch: Author in patch file is not correct - PTFS Admin <hosting@ptfs-europe.com> ::: installer/data/mysql/atomicupdate/shibOnly.perl @@ +1,5 @@ > +$DBversion = 'XXX'; # will be replaced by the RM > +if( CheckVersion( $DBversion ) ) { > + $dbh->do( "INSERT IGNORE INTO systempreferences (variable,value,explanation,options,type) VALUES ('opacShibOnly','0','If ON enables shibboleth only authentication for the opac','','YesNo'),('staffShibOnly','0','If ON enables shibboleth only authentication for the staff client','','YesNo')" ); > + SetVersion( $DBversion ); > + print "Upgrade to $DBversion done (Bug XXXXX - shibOnly preferences)\n"; Missing the bug number and maybe a nicer explanation? ::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref @@ +828,5 @@ > + - pref: opacShibOnly > + choices: > + yes: "Don't allow" > + no: Allow > + - patrons to login by means other than shibboleth. Should shibboleth be Shibboleth? Created attachment 79050 [details] [review] BUG 18506 - Add xShibOnly preference for shibboleth This patch adds two system preferences to Koha, opacShibOnly and staffShibOnly, allowing users to restrict authentication to just one method, Shibboleth. A system preference was chosen over a configuration file update to allow for local override at the virtualhost level. In this way a hosting provider can setup a 'backdoor opac' for example to allow fallback to local logins for support operations. Rebased and made requested corrections. Hi Martin, it looks like the patch is not complete. It works as expected for OPAC when opacPublic is disabled. But when it's enabled, users are still able to login using their local credentials. Created attachment 79522 [details] [review] BUG 18506 - Add xShibOnly preference for shibboleth This patch adds two system preferences to Koha, opacShibOnly and staffShibOnly, allowing users to restrict authentication to just one method, Shibboleth. A system preference was chosen over a configuration file update to allow for local override at the virtualhost level. In this way a hosting provider can setup a 'backdoor opac' for example to allow fallback to local logins for support operations. Created attachment 79524 [details] [review] Bug 18506: (followup) Make this play nicely with opacPublic Not my most elegant of followups but I think it resolves the issue. Thanks for testing Julian. Created attachment 79540 [details] [review] Bug 18506: (followup) Make this play nicely with opacPublic Created attachment 89423 [details] [review] Bug 18506: Add xShibOnly preference for Shibboleth authentication This patch adds two system preferences to Koha, opacShibOnly and staffShibOnly, allowing users to restrict authentication to just one method, Shibboleth. A system preference was chosen over a configuration file update to allow for local override at the virtualhost level. In this way a hosting provider can setup a 'backdoor opac' for example to allow fallback to local logins for support operations. Created attachment 89424 [details] [review] Bug 18506: (followup) Make this play nicely with opacPublic Freshly rebased. Needs changes to opac-auth.tt too (In reply to Nick Clemens from comment #14) > Needs changes to opac-auth.tt too In theory you shouldn't be able to navigate to opac-auth.tt.. but it's not a bad idea to include handling there anyway. Follow-up incoming Created attachment 91011 [details] [review] Bug 18506: Add xShibOnly preference for Shibboleth authentication This patch adds two system preferences to Koha, opacShibOnly and staffShibOnly, allowing users to restrict authentication to just one method, Shibboleth. A system preference was chosen over a configuration file update to allow for local override at the virtualhost level. In this way a hosting provider can setup a 'backdoor opac' for example to allow fallback to local logins for support operations. Created attachment 91012 [details] [review] Bug 18506: (follow-up) Make this play nicely with opacPublic Created attachment 91013 [details] [review] Bug 18506: (follow-up) Handle shibOnly in opac-auth.tt This patch adds the shibOnly conditional to the opac-auth.tt template such that if shibOnly is enabled then only the shibboleth login link ,and any shibboleth login errors if there are any, will display. It does not however preclude SCO and SCI login, those are still available if when SCO_login or SCI_login parameters are passed. Fresh rebase, tidy up of QA script failures and follow-up added to address Nicks concerns. That last followup was more complex than anticipated.. I think that template could do with a bit of a tidy up and my patch includes some whitespace changes I did along the way to try and keep track of what I was doing.. I could remove those if requested. Hi, I've done some testing, and it works consistently with the various combinations of opacShibOnly, OpacPublic and staffShibOnly. However, when opacShibOnly is enabled, the authentication process always redirect to opac-user.pl, instead of the page the user was currently on (like a search result, for example). The user should be redirected to the page he was on, like when opacShibOnly is disabled. Also I'm a little concerned that users might lock themselves out of the admin interface when enabling staffShibOnly, but that's nothing an SQL query to modify the syspref's value can't fix. (In reply to Matthias Meusburger from comment #20) > Hi, > > I've done some testing, and it works consistently with the various > combinations of opacShibOnly, OpacPublic and staffShibOnly. > > However, when opacShibOnly is enabled, the authentication process always > redirect to opac-user.pl, instead of the page the user was currently on > (like a search result, for example). > > The user should be redirected to the page he was on, like when opacShibOnly > is disabled. Noted, I'll take a look at doing this in a follow-up now. > > Also I'm a little concerned that users might lock themselves out of the > admin interface when enabling staffShibOnly, but that's nothing an SQL query > to modify the syspref's value can't fix. So I agree with this, but as it's the feature our customers have requested I think it's a valid use case. What we do at PTFS-E is use the syspref override in the vhosts and setup a second alias for local support so we can still login. Created attachment 92665 [details] [review] Bug 18506: (follow-up) Set redirect type to 303 This should ensure query parameters are passed through the redirection. The parameters are now kept after login, I'm signing this off. Created attachment 92680 [details] [review] Bug 18506: Add xShibOnly preference for Shibboleth authentication This patch adds two system preferences to Koha, opacShibOnly and staffShibOnly, allowing users to restrict authentication to just one method, Shibboleth. A system preference was chosen over a configuration file update to allow for local override at the virtualhost level. In this way a hosting provider can setup a 'backdoor opac' for example to allow fallback to local logins for support operations. Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com> Created attachment 92681 [details] [review] Bug 18506: (follow-up) Make this play nicely with opacPublic Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com> Created attachment 92682 [details] [review] Bug 18506: (follow-up) Handle shibOnly in opac-auth.tt This patch adds the shibOnly conditional to the opac-auth.tt template such that if shibOnly is enabled then only the shibboleth login link ,and any shibboleth login errors if there are any, will display. It does not however preclude SCO and SCI login, those are still available if when SCO_login or SCI_login parameters are passed. Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com> Created attachment 92683 [details] [review] Bug 18506: (follow-up) Set redirect type to 303 This should ensure query parameters are passed through the redirection. Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com> As we have a strong sign-off here and also some QA opinions, we should try and move this forward. Can we get a rebase please? Also a request: Could we make it OpacShibOnly or better OPACShibOnly? That seems to be more the standard looking at the other existing prefs. Pretty interested by this. We have our own separate Shibboleth integration, and I recently did a SAML-only SSO auth for it. It would be interesting to compare them... Created attachment 116250 [details] [review] Bug 18506: Add xShibOnly preference for Shibboleth authentication This patch adds two system preferences to Koha, opacShibOnly and staffShibOnly, allowing users to restrict authentication to just one method, Shibboleth. We do however, allow for local fallback for the SCO/SCI logins. A system preference was chosen over a configuration file update to allow for local override at the virtualhost level. In this way a hosting provider can setup a 'backdoor opac' for example to allow fallback to local logins for support operations. Created attachment 116251 [details] [review] Bug 18506: Add xShibOnly preference for Shibboleth authentication This patch adds two system preferences to Koha, opacShibOnly and staffShibOnly, allowing users to restrict authentication to just one method, Shibboleth. We do however, allow for local fallback for the SCO/SCI logins. A system preference was chosen over a configuration file update to allow for local override at the virtualhost level. In this way a hosting provider can setup a 'backdoor opac' for example to allow fallback to local logins for support operations. Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com> Created attachment 116253 [details] [review] Bug 18506: (QA follow-up) opacShibOnly -> OPACShibOnly As requested I've been porting this work from version to version on customer sites for a matter of years now.. would be wonderful to get it passed the finish line. David, can you put your QA stamp on this one? Created attachment 116280 [details] [review] Bug 18506: Add xShibOnly preference for Shibboleth authentication This patch adds two system preferences to Koha, opacShibOnly and staffShibOnly, allowing users to restrict authentication to just one method, Shibboleth. We do however, allow for local fallback for the SCO/SCI logins. A system preference was chosen over a configuration file update to allow for local override at the virtualhost level. In this way a hosting provider can setup a 'backdoor opac' for example to allow fallback to local logins for support operations. Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 116281 [details] [review] Bug 18506: (QA follow-up) opacShibOnly -> OPACShibOnly As requested Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 116282 [details] [review] Bug 18506: (QA follow-up) Indentation and more conditionals add Indentation fixes for readability Cleaned up a few places where the ability to login otherwise was leakign through Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 116290 [details] [review] Bug 18506: (QA follow-up) Indentation and more conditionals Indentation fixes for readability Cleaned up a few places where the ability to login otherwise was leakign through Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Thanks for the followup.. seems more had changed since the last rebase than I thought :(. Looks great and I've double-checked with some extra testing here :) Please rebase on top of bug 27476. Created attachment 116536 [details] [review] Bug 18506: Add xShibOnly preference for Shibboleth authentication This patch adds two system preferences to Koha, opacShibOnly and staffShibOnly, allowing users to restrict authentication to just one method, Shibboleth. We do however, allow for local fallback for the SCO/SCI logins. A system preference was chosen over a configuration file update to allow for local override at the virtualhost level. In this way a hosting provider can setup a 'backdoor opac' for example to allow fallback to local logins for support operations. Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 116537 [details] [review] Bug 18506: (QA follow-up) opacShibOnly -> OPACShibOnly As requested Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 116538 [details] [review] Bug 18506: (QA follow-up) Indentation and more conditionals Indentation fixes for readability Cleaned up a few places where the ability to login otherwise was leakign through Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Rebased Created attachment 116540 [details] [review] Bug 18506: (QA follow-up) Indentation and more conditionals Indentation fixes for readability Cleaned up a few places where the ability to login otherwise was leakign through Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> JD amended patch: Remove trailing spaces Pushed to master for 21.05, thanks to everybody involved! Do you think we can backport to 20.11.x ? It applies reasonably easily, I've backported it to our local 20.11 branch.. but it is an enhancement strictly, so your call as rmaint.. we trust you :) Pushed to 20.11.x for 20.11.04 Not backporting to 20.05. Please ask if needed. One of these days I'll rewrite checkauth() / replace it. One of these days... I'm glad to see this though. We use Shibboleth in a very different way locally, but I implemented a Shibboleth only option a year or two as I found a lot of people were asking for it. *** Bug 22842 has been marked as a duplicate of this bug. *** |