Bug 11219: make CAS authentication work with URL parameters

Bug 10029 tries to fix the use of URL parameters in CAS authentication.
But is does not work.
The full URL must be used in all methods of C4::Auth_with_cas.
Also, in checkpw_cas(), the 'ticket' parameter must be removed to find
the original URL.

This patch removes the 'ticket' parameter from query before calling
checkpw_cas() since the ticket is passed as method arguemnt.
In C4::Auth_with_cas, many methods use the same code to get the CAS
handler and the service URI. This patch adds a private method
_get_cas_and_service() to do the job.

Test plan:
- Enable CAS
- Go to opac without been logged-in
- Try to place hold on a record
=> You get to /cgi-bin/koha/opac-reserve.pl?biblionumber=XXX showing
   authentication page
=> Check that CAS link contains query param "biblionumber"
- Click on CAS link and log in
=> Check you return well logged-in to reserve page with biblionumber
   param
- Check CAS loggout
- Check Proxy CAS auth

Signed-off-by: Koha team AMU <koha.aixmarseille@gmail.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests in t, xt, and t/db_dependent/Auth.t.
Also passes QA script.

As I have no working CAS server, I focused on regression testing:
Activated Persona and casAuthentication.
- Verified normal login against database still works.
- Verified Persona login works.
  Note: With Persona you are always forwarded to the patron
  account - so you have to search for the record again before
  you can place a hold.
- Verified that the CAS URL contains the biblionumber when
  logging in while placing a hold.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Retested 2014-04-12

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>

diff --git a/C4/Auth.pm b/C4/Auth.pm
index f6d3b55..fe4a9b1 100644 (file)
--- a/C4/Auth.pm
+++ b/C4/Auth.pm
@@ -1544,6 +1544,7 @@ sub checkpw {
         $debug and print STDERR "## checkpw - checking CAS\n";
     # In case of a CAS authentication, we use the ticket instead of the password
         my $ticket = $query->param('ticket');
+        $query->delete('ticket'); # remove ticket to come back to original URL
         my ($retval,$retcard,$retuserid) = checkpw_cas($dbh, $ticket, $query);    # EXTERNAL AUTH
         ($retval) and return ($retval,$retcard,$retuserid);
         return 0;

diff --git a/C4/Auth_with_cas.pm b/C4/Auth_with_cas.pm
index 46cb1ec..e71898e 100644 (file)
--- a/C4/Auth_with_cas.pm
+++ b/C4/Auth_with_cas.pm
@@ -65,36 +65,21 @@ sub getMultipleAuth {
 # Logout from CAS
 sub logout_cas {
     my ($query) = @_;
-    my $uri = C4::Context->preference('OPACBaseURL') . $query->script_name();
-    my $casparam = $query->param('cas');
-    # FIXME: This should be more generic and handle whatever parameters there might be
-    $uri .= "?cas=" . $casparam if (defined $casparam);
-    $casparam = $defaultcasserver if (not defined $casparam);
-    my $cas = Authen::CAS::Client->new($casservers->{$casparam});
+    my ( $cas, $uri ) = _get_cas_and_service($query);
     print $query->redirect( $cas->logout_url($uri));
 }
 
 # Login to CAS
 sub login_cas {
     my ($query) = @_;
-    my $uri = C4::Context->preference('OPACBaseURL') . $query->script_name();
-    my $casparam = $query->param('cas');
-    # FIXME: This should be more generic and handle whatever parameters there might be
-    $uri .= "?cas=" . $casparam if (defined $casparam);
-    $casparam = $defaultcasserver if (not defined $casparam);
-    my $cas = Authen::CAS::Client->new($casservers->{$casparam});
+    my ( $cas, $uri ) = _get_cas_and_service($query);
     print $query->redirect( $cas->login_url($uri));
 }
 
 # Returns CAS login URL with callback to the requesting URL
 sub login_cas_url {
-
-    my ($query, $key) = @_;
-    my $uri = C4::Context->preference('OPACBaseURL') . $query->url( -absolute => 1, -query => 1 );
-    my $casparam = $query->param('cas');
-    $casparam = $defaultcasserver if (not defined $casparam);
-    $casparam = $key if (defined $key);
-    my $cas = Authen::CAS::Client->new($casservers->{$casparam});
+    my ( $query, $key ) = @_;
+    my ( $cas, $uri ) = _get_cas_and_service( $query, $key );
     return $cas->login_url($uri);
 }
 
@@ -104,12 +89,7 @@ sub checkpw_cas {
     $debug and warn "checkpw_cas";
     my ($dbh, $ticket, $query) = @_;
     my $retnumber;
-    my $uri = C4::Context->preference('OPACBaseURL') . $query->script_name();
-    my $casparam = $query->param('cas');
-    # FIXME: This should be more generic and handle whatever parameters there might be
-    $uri .= "?cas=" . $casparam if (defined $casparam);
-    $casparam = $defaultcasserver if (not defined $casparam);
-    my $cas = Authen::CAS::Client->new($casservers->{$casparam});
+    my ( $cas, $uri ) = _get_cas_and_service($query);
 
     # If we got a ticket
     if ($ticket) {
@@ -157,15 +137,11 @@ sub check_api_auth_cas {
     $debug and warn "check_api_auth_cas";
     my ($dbh, $PT, $query) = @_;
     my $retnumber;
-    my $url = C4::Context->preference('OPACBaseURL') . $query->script_name();
-
-    my $casparam = $query->param('cas');
-    $casparam = $defaultcasserver if (not defined $casparam);
-    my $cas = Authen::CAS::Client->new($casservers->{$casparam});
+    my ( $cas, $uri ) = _get_cas_and_service($query);
 
     # If we have a Proxy Ticket
     if ($PT) {
-        my $r = $cas->proxy_validate( $url, $PT );
+        my $r = $cas->proxy_validate( $uri, $PT );
 
         # If the PT is valid
         if ( $r->is_success ) {
@@ -203,6 +179,21 @@ sub check_api_auth_cas {
     return 0;
 }
 
+# Get CAS handler and service URI
+sub _get_cas_and_service {
+    my $query = shift;
+    my $key   = shift;    # optional
+
+    my $uri = C4::Context->preference('OPACBaseURL'); # server address
+    $uri .= $query->url( -absolute => 1, -query => 1 ); # page with params
+
+    my $casparam = $defaultcasserver;
+    $casparam = $query->param('cas') if defined $query->param('cas');
+    $casparam = $key if defined $key;
+    my $cas = Authen::CAS::Client->new( $casservers->{$casparam} );
+
+    return ( $cas, $uri );
+}
 
 1;
 __END__