Bug 24329: Add a test

[srvgit] / C4 / Auth_with_shibboleth.pm

diff --git a/C4/Auth_with_shibboleth.pm b/C4/Auth_with_shibboleth.pm
index dc18a1f..1e89020 100644 (file)
--- a/C4/Auth_with_shibboleth.pm
+++ b/C4/Auth_with_shibboleth.pm
@@ -27,7 +27,7 @@ use Koha::Patrons;
 use C4::Members::Messaging;
 use Carp;
 use CGI;
-use List::Util qw(any);
+use List::MoreUtils qw(any);
 
 use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS $debug);
 
@@ -54,18 +54,17 @@ sub shib_ok {
 sub logout_shib {
     my ($query) = @_;
     my $uri = _get_uri();
-    print $query->redirect( $uri . "/Shibboleth.sso/Logout?return=$uri" );
+    my $return = _get_return($query);
+    print $query->redirect( $uri . "/Shibboleth.sso/Logout?return=$return" );
 }
 
 # Returns Shibboleth login URL with callback to the requesting URL
 sub login_shib_url {
     my ($query) = @_;
 
-    my $param = _get_uri() . get_script_name();
-    if ( $query->query_string() ) {
-        $param = $param . '%3F' . $query->query_string();
-    }
-    my $uri = _get_uri() . "/Shibboleth.sso/Login?target=$param";
+    my $target = _get_return($query);
+    my $uri = _get_uri() . "/Shibboleth.sso/Login?target=" . $target;
+
     return $uri;
 }
 
@@ -84,7 +83,7 @@ sub get_login_shib {
 
     my $matchAttribute = $config->{mapping}->{ $config->{matchpoint} }->{is};
 
-    if ( any { /(^psgi|^plack)/i } keys %ENV ) {
+    if ( any { /(^psgi\.|^plack\.)/i } keys %ENV ) {
       $debug and warn $matchAttribute . " value: " . $ENV{"HTTP_".uc($matchAttribute)};
       return $ENV{"HTTP_".uc($matchAttribute)} || '';
     } else {
@@ -103,10 +102,18 @@ sub checkpw_shib {
     $debug and warn "User Shibboleth-authenticated as: $match";
 
     # Does the given shibboleth attribute value ($match) match a valid koha user ?
-    my $borrower =
-      Koha::Database->new()->schema()->resultset('Borrower')
-      ->find( { $config->{matchpoint} => $match } );
+    my $borrowers = Koha::Patrons->search( { $config->{matchpoint} => $match } );
+    if ( $borrowers->count > 1 ){
+        # If we have more than 1 borrower the matchpoint is not unique
+        # we cannot know which patron is the correct one, so we should fail
+         $debug and warn "There are several users with $config->{matchpoint} of $match, matchpoints must be unique";
+        return 0;
+    }
+    my $borrower = $borrowers->next;
     if ( defined($borrower) ) {
+        if ($config->{'sync'}) {
+            _sync($borrower->borrowernumber, $config, $match);
+        }
         return ( 1, $borrower->get_column('cardnumber'), $borrower->get_column('userid') );
     }
 
@@ -125,7 +132,11 @@ sub _autocreate {
     my %borrower = ( $config->{matchpoint} => $match );
 
     while ( my ( $key, $entry ) = each %{$config->{'mapping'}} ) {
-        $borrower{$key} = ( $entry->{'is'} && $ENV{ $entry->{'is'} } ) || $entry->{'content'} || '';
+        if ( any { /(^psgi|^plack)/i } keys %ENV ) {
+            $borrower{$key} = ( $entry->{'is'} && $ENV{"HTTP_" . uc($entry->{'is'}) } ) || $entry->{'content'} || '';
+        } else {
+            $borrower{$key} = ( $entry->{'is'} && $ENV{ $entry->{'is'} } ) || $entry->{'content'} || '';
+        }
     }
 
     my $patron = Koha::Patron->new( \%borrower )->store;
@@ -134,6 +145,21 @@ sub _autocreate {
     return ( 1, $patron->cardnumber, $patron->userid );
 }
 
+sub _sync {
+    my ($borrowernumber, $config, $match ) = @_;
+    my %borrower;
+    $borrower{'borrowernumber'} = $borrowernumber;
+    while ( my ( $key, $entry ) = each %{$config->{'mapping'}} ) {
+        if ( any { /(^psgi|^plack)/i } keys %ENV ) {
+            $borrower{$key} = ( $entry->{'is'} && $ENV{"HTTP_" . uc($entry->{'is'}) } ) || $entry->{'content'} || '';
+        } else {
+            $borrower{$key} = ( $entry->{'is'} && $ENV{ $entry->{'is'} } ) || $entry->{'content'} || '';
+        }
+    }
+    my $patron = Koha::Patrons->find( $borrowernumber );
+    $patron->set(\%borrower)->store;
+}
+
 sub _get_uri {
 
     my $protocol = "https://";
@@ -167,11 +193,32 @@ sub _get_uri {
     return $return;
 }
 
+sub _get_return {
+    my ($query) = @_;
+
+    my $uri_base_part = _get_uri() . get_script_name();
+
+    my $uri_params_part = '';
+    foreach my $param ( sort $query->url_param() ) {
+        # url_param() always returns parameters that were deleted by delete()
+        # This additional check ensure that parameter was not deleted.
+        my $uriPiece = $query->param($param);
+        if ($uriPiece) {
+            $uri_params_part .= '&' if $uri_params_part;
+            $uri_params_part .= $param . '=';
+            $uri_params_part .= $uriPiece;
+        }
+    }
+    $uri_base_part .= '%3F' if $uri_params_part;
+
+    return $uri_base_part . URI::Escape::uri_escape_utf8($uri_params_part);
+}
+
 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;
     }