Attachment 80026 Details for Bug 17776 – Bug 17776: (QA follow-up) Remove shibboleth package variables

[patch] Bug 17776: (QA follow-up) Remove shibboleth package variables

Bug-17776-QA-follow-up-Remove-shibboleth-package-v.patch (text/plain), 4.34 KB, created by Martin Renvoize (ashimema) on 2018-10-05 10:52:05 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2018-10-05 10:52:05 UTC

Size: 4.34 KB

patch

obsolete

>From 3feddf8742b5c8f9eb6a7f2df13e940e4ef7f3dd Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Date: Fri, 5 Oct 2018 10:19:14 +0200
>Subject: [PATCH] Bug 17776: (QA follow-up) Remove shibboleth package variables
>
>This is about $shib and $shib_login.
>We move in the right direction by calling get_login_shib in
>get_template_and_user and checkauth. In the same line we can do the
>shib_ok check at that time (just checking cached values). This paves
>the way for the third subroutine using the two package vars: checkpw.
>Note that checkpw is also called outside Auth.pm. So I would be more
>comfortable if we do the same calls like in checkauth and remove both
>variables from the package level (especially under Plack of course).
>
>The former changes actually justify a 'use C4::Auth_with_shibboleth'
>instead of the current require and import.
>
>Note: When calling checkpw from checkauth, we are calling get_login_shib
>twice now. But the time involved for doing so is around zero (cache), so
>not really an argument for extra parameters and complexer code.
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>---
> C4/Auth.pm                 | 22 ++++++++++------------
> C4/Auth_with_shibboleth.pm |  2 +-
> 2 files changed, 11 insertions(+), 13 deletions(-)
>
>diff --git a/C4/Auth.pm b/C4/Auth.pm
>index 303db34019..ddcaf1a81b 100644
>--- a/C4/Auth.pm
>+++ b/C4/Auth.pm
>@@ -41,9 +41,10 @@ use Koha::Patron::Consents;
> use POSIX qw/strftime/;
> use List::MoreUtils qw/ any /;
> use Encode qw( encode is_utf8);
>+use C4::Auth_with_shibboleth;
> 
> # use utf8;
>-use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS $debug $ldap $cas $caslogout $shib $shib_login);
>+use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS $debug $ldap $cas $caslogout);
> 
> BEGIN {
>     sub psgi_env { any { /^psgi\./ } keys %ENV }
>@@ -62,7 +63,6 @@ BEGIN {
>     %EXPORT_TAGS = ( EditPermissions => [qw(get_all_subpermissions get_user_subpermissions)] );
>     $ldap      = C4::Context->config('useldapserver') || 0;
>     $cas       = C4::Context->preference('casAuthentication');
>-    $shib      = C4::Context->config('useshibboleth') || 0;
>     $caslogout = C4::Context->preference('casLogout');
>     require C4::Auth_with_cas;    # no import
> 
>@@ -70,14 +70,6 @@ BEGIN {
>         require C4::Auth_with_ldap;
>         import C4::Auth_with_ldap qw(checkpw_ldap);
>     }
>-    if ($shib) {
>-        require C4::Auth_with_shibboleth;
>-        import C4::Auth_with_shibboleth
>-          qw(shib_ok checkpw_shib logout_shib login_shib_url get_login_shib);
>-
>-        # Check for good config
>-        $shib = 0 unless shib_ok();
>-    }
>     if ($cas) {
>         import C4::Auth_with_cas qw(check_api_auth_cas checkpw_cas login_cas logout_cas login_cas_url logout_if_required);
>     }
>@@ -153,7 +145,8 @@ sub get_template_and_user {
>     my ( $user, $cookie, $sessionID, $flags );
> 
>     # Get shibboleth login attribute
>-    $shib_login = get_login_shib() if $shib;
>+    my $shib = C4::Context->config('useshibboleth') && shib_ok();
>+    my $shib_login = $shib ? get_login_shib() : undef;
> 
>     C4::Context->interface( $in->{type} );
> 
>@@ -786,7 +779,8 @@ sub checkauth {
>     $debug and warn "Checking Auth";
> 
>     # Get shibboleth login attribute
>-    $shib_login = get_login_shib() if $shib;
>+    my $shib = C4::Context->config('useshibboleth') && shib_ok();
>+    my $shib_login = $shib ? get_login_shib() : undef;
> 
>     # $authnotrequired will be set for scripts which will run without authentication
>     my $authnotrequired = shift;
>@@ -1773,6 +1767,10 @@ sub checkpw {
>     my ( $dbh, $userid, $password, $query, $type, $no_set_userenv ) = @_;
>     $type = 'opac' unless $type;
> 
>+    # Get shibboleth login attribute
>+    my $shib = C4::Context->config('useshibboleth') && shib_ok();
>+    my $shib_login = $shib ? get_login_shib() : undef;
>+
>     my @return;
>     my $patron = Koha::Patrons->find({ userid => $userid });
>     my $check_internal_as_fallback = 0;
>diff --git a/C4/Auth_with_shibboleth.pm b/C4/Auth_with_shibboleth.pm
>index 480d5d8d32..0ff0d25604 100644
>--- a/C4/Auth_with_shibboleth.pm
>+++ b/C4/Auth_with_shibboleth.pm
>@@ -171,7 +171,7 @@ sub _get_shib_config {
>     my $config = C4::Context->config('shibboleth');
> 
>     if ( !$config ) {
>-        carp 'shibboleth config not defined';
>+        carp 'shibboleth config not defined' if $debug;
>         return 0;
>     }
> 
>-- 
>2.19.0

Actions: View | Diff | Splinter Review

Attachments on bug 17776: 60800 | 60847 | 61426 | 68216 | 79572 | 79573 | 80000 | 80001 | 80002 | 80003 | 80023 | 80024 | 80025 | 80026