Attachment 23138 Details for Bug 11219 – Bug 11219 - CAS authentication fails with URL parameters

[patch] Bug 11219 - CAS authentication fails with URL parameters

Bug-11219---CAS-authentication-fails-with-URL-para.patch (text/plain), 5.66 KB, created by Biblibre Sandboxes on 2013-11-25 14:58:29 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Biblibre Sandboxes

Created: 2013-11-25 14:58:29 UTC

Size: 5.66 KB

patch

obsolete

>From 0db50caaa96c1667b8e1909d1717efbfd0f0cdd0 Mon Sep 17 00:00:00 2001
>From: Fridolyn SOMERS <fridolyn.somers@biblibre.com>
>Date: Fri, 8 Nov 2013 11:12:57 +0100
>Subject: [PATCH] Bug 11219 - CAS authentication fails 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 :
>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>
>---
> C4/Auth.pm          |    1 +
> C4/Auth_with_cas.pm |   53 +++++++++++++++++++++-----------------------------
> 2 files changed, 23 insertions(+), 31 deletions(-)
>
>diff --git a/C4/Auth.pm b/C4/Auth.pm
>index 8b0bebe..eef263c 100644
>--- a/C4/Auth.pm
>+++ b/C4/Auth.pm
>@@ -1505,6 +1505,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
>--- 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__
>-- 
>1.7.2.5

Actions: View | Diff | Splinter Review

Attachments on bug 11219: 22807 | 23138 | 23744 | 26967 | 26971 | 27054 | 27055 | 27095 | 27096