Working on logout and XSS (#595)

lemonldap-ng-common/lib/Lemonldap/NG/Common/PSGI/Request.pm

@@ -15,6 +15,7 @@ has HTTP_ACCEPT_ENCODING => ( is => 'ro', reader => 'encodings' );
 has HTTP_ACCEPT_LANGUAGE => ( is => 'ro', reader => 'languages' );
 has HTTP_COOKIE          => ( is => 'ro', reader => 'cookies' );
 has HTTP_HOST            => ( is => 'ro', reader => 'hostname' );
+has REFERER              => ( is => 'ro', reader => 'referer' );
 has REMOTE_ADDR => ( is => 'ro', isa => 'Str', reader => 'remote_ip' );
 has REMOTE_PORT    => ( is => 'ro', isa => 'Int', reader => 'port' );
 has REQUEST_METHOD => ( is => 'ro', isa => 'Str', reader => 'method' );

lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Process.pm

@@ -75,7 +75,7 @@ sub controlUrl {
         # For logout request, test if Referer comes from an authorizated site
         my $tmp = (
               $req->param('logout')
-            ? $ENV{HTTP_REFERER}
+            ? $req->referer
             : $req->{urldc}
         );
 
@@ -112,7 +112,8 @@ sub controlUrl {
 sub checkLogout {
     my ( $self, $req ) = @_;
     if ( $req->param('logout') ) {
-        $req->steps( [ @{ $self->beforeLogout }, 'deleteSession' ] );
+        $req->steps(
+            [ @{ $self->beforeLogout }, 'authLogout', 'deleteSession' ] );
     }
     PE_OK;
 }

lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Request.pm

@@ -47,6 +47,9 @@ has template => ( is => 'rw' );
 # Boolean to indicate that response must be a redirection
 has mustRedirect => ( is => 'rw' );
 
+# Store URL for redirections
+has urldc => ( is => 'rw' );
+
 # Boolean to indicate that url isn't Base64 encoded
 has urlNotBase64 => ( is => 'rw' );
 

lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Run.pm

@@ -86,8 +86,8 @@ sub authenticatedRequest {
     return $self->do(
         $req,
         [
-            'importHandlerDatas', 'checkLogout',
-            'controlUrl',         @{ $self->forAuthUser }
+            'importHandlerDatas', 'controlUrl',
+            'checkLogout',        @{ $self->forAuthUser }
         ]
     );
 }
@@ -97,8 +97,8 @@ sub postAuthenticatedRequest {
     return $self->do(
         $req,
         [
-            'importHandlerDatas', 'checkLogout',
-            'restoreArgs',        'controlUrl',
+            'importHandlerDatas', 'restoreArgs',
+            'controlUrl',         'checkLogout',
             @{ $self->forAuthUser }
         ]
     );
@@ -106,8 +106,13 @@ sub postAuthenticatedRequest {
 
 sub logout {
     my ( $self, $req ) = @_;
-    return $self->do( $req,
-        [ @{ $self->beforeLogout }, 'authLogout', 'deleteSession' ] );
+    return $self->do(
+        $req,
+        [
+            'controlUrl', @{ $self->beforeLogout },
+            'authLogout', 'deleteSession'
+        ]
+    );
 }
 
 # RUNNING METHODS
@@ -182,10 +187,8 @@ sub autoRedirect {
 
     # Redirection should be made if urldc defined
     if ( $req->{urldc} ) {
-        return [
-            302, [ Location => $req->{urldc}, @{ $req->respHeaders } ],
-            []
-        ];
+        return [ 302, [ Location => $req->{urldc}, @{ $req->respHeaders } ],
+            [] ];
     }
     else {
         my ( $tpl, $prms ) = $self->display($req);

lemonldap-ng-portal/t/03-XSS-protection.t

@@ -8,7 +8,13 @@ BEGIN {
 
 require 't/test-lib.pm';
 
-init( { useSafeJail => 1, trustedDomains => 'example3.com *.example2.com' } );
+init(
+    {
+        logLevel       => 'error',
+        useSafeJail    => 1,
+        trustedDomains => 'example3.com *.example2.com'
+    }
+);
 
 my @tests = (
 
@@ -87,12 +93,12 @@ my @tests = (
 
     # 19 url=http://www.toto.com/, good referer
     'aHR0cDovL3d3dy50b3RvLmNvbS8=',
-    'http://test.example.com/' => 1,
+    'http://test1.example.com/' => 1,
     'Logout required by good site',
 
     # 20 url=http://www?<script>, good referer
     'aHR0cDovL3d3dz88c2NyaXB0Pg==',
-    'http://test.example.com/' => 0,
+    'http://test1.example.com/' => 0,
     'script with logout',
 );
 
@@ -132,17 +138,42 @@ while ( defined( my $url = shift(@tests) ) ) {
     count(2);
 }
 
+while ( defined( my $url = shift(@tests) ) ) {
+    my $referer = shift @tests;
+    my $redir   = shift @tests;
+    my $detail  = shift @tests;
+    ok(
+        $res = &client->_get(
+            '/',
+            query  => "url=$url&logout=1",
+            cookie => "lemonldap=$id",
+
+            accept  => 'text/html',
+            referer => $referer,
+        ),
+        $detail
+    );
+    ok( ( $res->[0] == ( $redir ? 302 : 200 ) ),
+        ( $redir ? 'Get redirection' : 'Redirection dropped' ) )
+      or explain( $res->[0], ( $redir ? 302 : 200 ) );
+    ok(
+        $res = &client->_post(
+            '/', '',
+            IO::String->new('user=dwho&password=dwho'),
+            'application/x-www-form-urlencoded', 23
+        ),
+        'Auth query'
+    );
+    ok( $res->[0] == 200, 'Response is 200' ) or explain( $res->[0], 200 );
+    ok( $id = getCookies($res)->{lemonldap}, 'Get LLNG cookie' )
+      or explain( $res, 'Set-Cookie: something' );
+    count(5);
+}
+
 clean_sessions();
 
 done_testing( count() );
 __END__
-while ( defined( $url = shift(@h) ) ) {
-    last if ( $url eq 'LOGOUT' );
-    $result = shift @h;
-    my $text = shift @h;
-
-    ok( $p->controlUrlOrigin() == $result, $text );
-}
 
 # LOGOUT CASES
 $logout = 1;

lemonldap-ng-portal/t/test-lib.pm

@@ -113,11 +113,12 @@ sub _get {
               || 'application/json, text/plain, */*',
             'HTTP_ACCEPT_LANGUAGE' => 'fr,fr-FR;q=0.8,en-US;q=0.5,en;q=0.3',
             'HTTP_CACHE_CONTROL'   => 'max-age=0',
-            ( $args{cookie} ? ( 'HTTP_COOKIE' => $args{cookie} ) : () ),
+            ( $args{cookie} ? ( HTTP_COOKIE => $args{cookie} ) : () ),
             'HTTP_HOST' => 'auth.example.com',
             'HTTP_USER_AGENT' =>
               'Mozilla/5.0 (VAX-4000; rv:36.0) Gecko/20350101 Firefox',
-            'PATH_INFO'   => $path,
+            'PATH_INFO' => $path,
+            ( $args{referer} ? ( REFERER => $args{referer} ) : () ),
             'REMOTE_ADDR' => '127.0.0.1',
             (
                 $args{remote_user}
@@ -126,7 +127,7 @@ sub _get {
             ),
             'REQUEST_METHOD' => 'GET',
             'REQUEST_URI'    => $path . ( $args{query} ? "?$args{query}" : '' ),
-            ( $args{query} ? ( 'QUERY_STRING' => $args{query} ) : () ),
+            ( $args{query} ? ( QUERY_STRING => $args{query} ) : () ),
             'SCRIPT_NAME'     => '',
             'SERVER_NAME'     => 'auth.example.com',
             'SERVER_PORT'     => '8002',