From f46df4d0e1a6374192ee84953e46ef8cb4b15d46 Mon Sep 17 00:00:00 2001
From: Maxime Besson <maxime.besson@worteks.com>
Date: Wed, 29 Mar 2023 16:23:43 +0200
Subject: [PATCH 1/2] Factor all PSGI loops through a psgiAdapter method

---
 .../lib/Lemonldap/NG/Common/PSGI.pm           | 46 +++++++++++++------
 .../lib/Lemonldap/NG/Handler/Lib/PSGI.pm      | 46 +++++++++++--------
 .../lib/Lemonldap/NG/Handler/PSGI/Try.pm      | 46 ++++++++++---------
 .../lib/Lemonldap/NG/Handler/Server.pm        | 16 ++++---
 .../lib/Lemonldap/NG/Handler/Server/Nginx.pm  | 18 ++++----
 .../Lemonldap/NG/Handler/Server/Traefik.pm    | 12 ++++-
 6 files changed, 112 insertions(+), 72 deletions(-)

diff --git a/lemonldap-ng-common/lib/Lemonldap/NG/Common/PSGI.pm b/lemonldap-ng-common/lib/Lemonldap/NG/Common/PSGI.pm
index a14839272d..ecb97b94bf 100644
--- a/lemonldap-ng-common/lib/Lemonldap/NG/Common/PSGI.pm
+++ b/lemonldap-ng-common/lib/Lemonldap/NG/Common/PSGI.pm
@@ -238,9 +238,11 @@ sub sendRawHtml {
 sub abort {
     my ( $self, $err ) = @_;
     eval { $self->logger->error($err) };
-    return sub {
-        $self->sendError( $self->newRequest( $_[0] ), $err, 500 );
-    };
+    return $self->psgiAdapter(
+        sub {
+            $self->sendError( $_[0], $err, 500 );
+        }
+    );
 }
 
 sub _mustBeDefined {
@@ -335,16 +337,6 @@ sub sendHtml {
 # Main method #
 ###############
 
-sub newRequest {
-    my ( $self, $env ) = @_;
-    my $req = Lemonldap::NG::Common::PSGI::Request->new($env);
-    $self->logger->info( "New request "
-          . ref($self) . " "
-          . $req->method . " "
-          . $req->request_uri );
-    return $req;
-}
-
 sub run {
     my ( $self, $args ) = @_;
     $args //= {};
@@ -357,9 +349,33 @@ sub run {
 
 sub _run {
     my $self = shift;
+    return $self->psgiAdapter(
+        sub {
+            $self->_logAndHandle( $_[0] );
+        }
+    );
+}
+
+# This method turns a sub that takes a Lemonldap::NG::Common::PSGI::Request
+# obect and returns a PSGI response into a proper PSGI method.
+sub psgiAdapter {
+    my ( $self, $sub ) = @_;
     return sub {
-        $self->_logAndHandle( $self->newRequest( $_[0] ) );
-    };
+        my $env = shift;
+        my $req = Lemonldap::NG::Common::PSGI::Request->new($env);
+        return $self->logAndRun( $req, $sub );
+    }
+}
+
+# This method sets up LemonLDAP::NG logging for the current request
+sub logAndRun {
+    my ( $self, $req, $sub ) = @_;
+
+    $self->logger->info( "New request "
+          . ref($self) . " "
+          . $req->method . " "
+          . $req->request_uri );
+    return $sub->($req);
 }
 
 sub _logAndHandle {
diff --git a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Lib/PSGI.pm b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Lib/PSGI.pm
index f1f7c474a3..521520e44a 100644
--- a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Lib/PSGI.pm
+++ b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Lib/PSGI.pm
@@ -49,9 +49,11 @@ sub _run {
 
         # Handle requests
         # Developers, be careful: Only this part is executed at each request
-        return sub {
-            return $self->_logAuthTrace( $self->newRequest( $_[0] ) );
-        };
+        return $self->psgiAdapter(
+            sub {
+                return $self->_logAuthTrace( $_[0] );
+            }
+        );
     }
 
     else {
@@ -65,12 +67,14 @@ sub _run {
         }
 
         # Handle unprotected requests
-        return sub {
-            my $req = $self->newRequest( $_[0] );
-            my $res = $self->_logAndHandle($req);
-            push @{ $res->[1] }, $req->spliceHdrs;
-            return $res;
-        };
+        return $self->psgiAdapter(
+            sub {
+                my $req = $_[0];
+                my $res = $self->_logAndHandle($req);
+                push @{ $res->[1] }, $req->spliceHdrs;
+                return $res;
+            }
+        );
     }
 }
 
@@ -85,11 +89,13 @@ sub status {
         eval { $self->api->checkConf() };
         $self->logger->error($@) if ($@);
     }
-    return sub {
-        my $req = $self->newRequest( $_[0] );
-        $self->api->status($req);
-        return [ 200, [ $req->spliceHdrs ], [ $req->{respBody} ] ];
-    };
+    return $self->psgiAdapter(
+        sub {
+            my $req = $_[0];
+            $self->api->status($req);
+            return [ 200, [ $req->spliceHdrs ], [ $req->{respBody} ] ];
+        }
+    );
 }
 
 sub reload {
@@ -103,11 +109,13 @@ sub reload {
         eval { $self->api->checkConf() };
         $self->logger->error($@) if ($@);
     }
-    return sub {
-        my $req = $self->newRequest( $_[0] );
-        $self->api->reload($req);
-        return [ 200, [ $req->spliceHdrs ], [ $req->{respBody} ] ];
-    };
+    return $self->psgiAdapter(
+        sub {
+            my $req = $_[0];
+            $self->api->reload($req);
+            return [ 200, [ $req->spliceHdrs ], [ $req->{respBody} ] ];
+        }
+    );
 }
 
 sub _logAuthTrace {
diff --git a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/PSGI/Try.pm b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/PSGI/Try.pm
index 393fa55f62..6ab3baf661 100644
--- a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/PSGI/Try.pm
+++ b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/PSGI/Try.pm
@@ -83,30 +83,32 @@ sub defaultUnauthRoute {
 sub _run {
     my $self = shift;
 
-    return sub {
-        my $req = $self->newRequest( $_[0] );
-        my $res = $self->_logAuthTrace( $req, 1 );
-        if ( $res->[0] < 300 ) {
-            $self->routes( $self->authRoutes );
-            $req->userData( $self->api->data );
-            $req->respHeaders( $res->[1] );
-        }
-        elsif ( $res->[0] != 403 and not $req->data->{noTry} ) {
-
-            # Unset headers (handler adds a Location header)
-            $self->logger->debug(
-                "User not authenticated, Try in use, cancel redirection");
-            $req->userData( {} );
-            $req->respHeaders( [] );
-            $self->routes( $self->unAuthRoutes );
-        }
-        else {
+    return $self->psgiAdapter(
+        sub {
+            my $req = $_[0];
+            my $res = $self->_logAuthTrace( $req, 1 );
+            if ( $res->[0] < 300 ) {
+                $self->routes( $self->authRoutes );
+                $req->userData( $self->api->data );
+                $req->respHeaders( $res->[1] );
+            }
+            elsif ( $res->[0] != 403 and not $req->data->{noTry} ) {
+
+                # Unset headers (handler adds a Location header)
+                $self->logger->debug(
+                    "User not authenticated, Try in use, cancel redirection");
+                $req->userData( {} );
+                $req->respHeaders( [] );
+                $self->routes( $self->unAuthRoutes );
+            }
+            else {
+                return $res;
+            }
+            $res = $self->_logAndHandle($req);
+            push @{ $res->[1] }, $req->spliceHdrs;
             return $res;
         }
-        $res = $self->_logAndHandle($req);
-        push @{ $res->[1] }, $req->spliceHdrs;
-        return $res;
-    };
+    );
 
 }
 
diff --git a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server.pm b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server.pm
index dfc4392468..68604cc547 100644
--- a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server.pm
+++ b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server.pm
@@ -23,13 +23,15 @@ sub init {
 #
 sub _run {
     my ($self) = @_;
-    return sub {
-        my $req = $self->newRequest( $_[0] );
-        my $res = $self->_logAuthTrace($req);
-        push @{ $res->[1] }, $req->spliceHdrs,
-          Cookie => ( $req->{Cookie} // '' );
-        return $res;
-    };
+    return $self->psgiAdapter(
+        sub {
+            my $req = $_[0];
+            my $res = $self->_logAuthTrace($req);
+            push @{ $res->[1] }, $req->spliceHdrs,
+              Cookie => ( $req->{Cookie} // '' );
+            return $res;
+        }
+    );
 }
 
 ## @method PSGI-Response handler($req)
diff --git a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server/Nginx.pm b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server/Nginx.pm
index e059f3a677..c652722e3d 100644
--- a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server/Nginx.pm
+++ b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server/Nginx.pm
@@ -28,16 +28,18 @@ sub init {
 #@return subroutine that will be called to manage FastCGI queries
 sub _run {
     my $self = shift;
-    return sub {
-        my $env = $_[0];
-        my $res = $self->_logAuthTrace( $self->newRequest($env) );
+    return $self->psgiAdapter(
+        sub {
+            my $req = $_[0];
+            my $res = $self->_logAuthTrace($req);
 
-        # Transform 302 responses in 401 since Nginx refuse it
-        if ( $res->[0] == 302 or $res->[0] == 303 ) {
-            $res->[0] = 401;
+            # Transform 302 responses in 401 since Nginx refuse it
+            if ( $res->[0] == 302 or $res->[0] == 303 ) {
+                $res->[0] = 401;
+            }
+            return $res;
         }
-        return $res;
-    };
+    );
 }
 
 ## @method PSGI-Response handler()
diff --git a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server/Traefik.pm b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server/Traefik.pm
index 415ba5a0c9..100460fb7c 100644
--- a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server/Traefik.pm
+++ b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server/Traefik.pm
@@ -16,11 +16,21 @@ sub init {
 
 sub _run {
     my $self = shift;
+
+    # Create regular logAuthTrace PSGI app
+    my $app = $self->psgiAdapter(
+        sub {
+            my $req = $_[0];
+            return $self->_logAuthTrace($req);
+        }
+    );
+
+    # Middleware to set correct values for Traefik
     return sub {
         my $env = $_[0];
         $env->{HTTP_HOST}   = $env->{HTTP_X_FORWARDED_HOST};
         $env->{REQUEST_URI} = $env->{HTTP_X_FORWARDED_URI};
-        return $self->_logAuthTrace( $self->newRequest($env) );
+        return $app->($env);
     }
 }
 
-- 
GitLab


From 312e56bf9c44b9d81ee1f773b1c71c3e66f0850a Mon Sep 17 00:00:00 2001
From: Maxime Besson <maxime.besson@worteks.com>
Date: Wed, 29 Mar 2023 16:33:03 +0200
Subject: [PATCH 2/2] Refactor _logAndHandle and _logAuthTrace into
 Common::PSGI

---
 .../lib/Lemonldap/NG/Common/PSGI.pm           | 20 ++++-------
 .../lib/Lemonldap/NG/Handler/Lib/PSGI.pm      | 36 +++----------------
 .../lib/Lemonldap/NG/Handler/PSGI/Try.pm      |  4 +--
 .../lib/Lemonldap/NG/Handler/Server.pm        |  2 +-
 .../lib/Lemonldap/NG/Handler/Server/Nginx.pm  |  4 +--
 .../Lemonldap/NG/Handler/Server/Traefik.pm    |  4 +--
 6 files changed, 18 insertions(+), 52 deletions(-)

diff --git a/lemonldap-ng-common/lib/Lemonldap/NG/Common/PSGI.pm b/lemonldap-ng-common/lib/Lemonldap/NG/Common/PSGI.pm
index ecb97b94bf..25846e3e8f 100644
--- a/lemonldap-ng-common/lib/Lemonldap/NG/Common/PSGI.pm
+++ b/lemonldap-ng-common/lib/Lemonldap/NG/Common/PSGI.pm
@@ -351,7 +351,7 @@ sub _run {
     my $self = shift;
     return $self->psgiAdapter(
         sub {
-            $self->_logAndHandle( $_[0] );
+            $self->handler( $_[0] );
         }
     );
 }
@@ -371,16 +371,6 @@ sub psgiAdapter {
 sub logAndRun {
     my ( $self, $req, $sub ) = @_;
 
-    $self->logger->info( "New request "
-          . ref($self) . " "
-          . $req->method . " "
-          . $req->request_uri );
-    return $sub->($req);
-}
-
-sub _logAndHandle {
-    my ( $self, $req ) = @_;
-
     # register the request object to the logging system
     if ( ref( $self->logger ) and $self->logger->can('setRequestObj') ) {
         $self->logger->setRequestObj($req);
@@ -390,8 +380,12 @@ sub _logAndHandle {
         $self->userLogger->setRequestObj($req);
     }
 
-    # Call the handler
-    my $res = $self->handler($req);
+    $self->logger->info( "New request "
+          . ref($self) . " "
+          . $req->method . " "
+          . $req->request_uri );
+
+    my $res = $sub->($req);
 
     # Clear the logging system before the next request
     if ( ref( $self->logger ) and $self->logger->can('clearRequestObj') ) {
diff --git a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Lib/PSGI.pm b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Lib/PSGI.pm
index 521520e44a..cec3b7f4aa 100644
--- a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Lib/PSGI.pm
+++ b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Lib/PSGI.pm
@@ -38,7 +38,7 @@ sub init {
 
 ## @methodi void _run()
 # Check if protecton is activated then return a code ref that will launch
-# _logAuthTrace() if protection in on or handler() else
+# _authAndTrace() if protection in on or handler() else
 #@return code-ref
 sub _run {
     my $self = shift;
@@ -51,7 +51,7 @@ sub _run {
         # Developers, be careful: Only this part is executed at each request
         return $self->psgiAdapter(
             sub {
-                return $self->_logAuthTrace( $_[0] );
+                return $self->_authAndTrace( $_[0] );
             }
         );
     }
@@ -70,7 +70,7 @@ sub _run {
         return $self->psgiAdapter(
             sub {
                 my $req = $_[0];
-                my $res = $self->_logAndHandle($req);
+                my $res = $self->handler($req);
                 push @{ $res->[1] }, $req->spliceHdrs;
                 return $res;
             }
@@ -118,34 +118,6 @@ sub reload {
     );
 }
 
-sub _logAuthTrace {
-    my ( $self, $req, $noCall ) = @_;
-
-    # register the request object to the logging system
-    if ( ref( $self->logger ) and $self->logger->can('setRequestObj') ) {
-        $self->logger->setRequestObj($req);
-    }
-    if ( ref( $self->userLogger ) and $self->userLogger->can('setRequestObj') )
-    {
-        $self->userLogger->setRequestObj($req);
-    }
-
-    # Call the handler
-    my $res = $self->_authAndTrace( $req, $noCall );
-
-    # Clear the logging system before the next request
-    if ( ref( $self->logger ) and $self->logger->can('clearRequestObj') ) {
-        $self->logger->clearRequestObj($req);
-    }
-    if ( ref( $self->userLogger )
-        and $self->userLogger->can('clearRequestObj') )
-    {
-        $self->userLogger->clearRequestObj($req);
-    }
-
-    return $res;
-}
-
 ## @method private PSGI-Response _authAndTrace($req)
 # Launch $self->api::run() and then handler() if
 # response is 200.
@@ -173,7 +145,7 @@ sub _authAndTrace {
         }
         else {
             $self->logger->debug('User authenticated, calling handler()');
-            $res = $self->_logAndHandle($req);
+            $res = $self->handler($req);
             push @{ $res->[1] }, $req->spliceHdrs;
             return $res;
         }
diff --git a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/PSGI/Try.pm b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/PSGI/Try.pm
index 6ab3baf661..dc75f83318 100644
--- a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/PSGI/Try.pm
+++ b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/PSGI/Try.pm
@@ -86,7 +86,7 @@ sub _run {
     return $self->psgiAdapter(
         sub {
             my $req = $_[0];
-            my $res = $self->_logAuthTrace( $req, 1 );
+            my $res = $self->_authAndTrace( $req, 1 );
             if ( $res->[0] < 300 ) {
                 $self->routes( $self->authRoutes );
                 $req->userData( $self->api->data );
@@ -104,7 +104,7 @@ sub _run {
             else {
                 return $res;
             }
-            $res = $self->_logAndHandle($req);
+            $res = $self->handler($req);
             push @{ $res->[1] }, $req->spliceHdrs;
             return $res;
         }
diff --git a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server.pm b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server.pm
index 68604cc547..4a8fcc8f41 100644
--- a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server.pm
+++ b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server.pm
@@ -26,7 +26,7 @@ sub _run {
     return $self->psgiAdapter(
         sub {
             my $req = $_[0];
-            my $res = $self->_logAuthTrace($req);
+            my $res = $self->_authAndTrace($req);
             push @{ $res->[1] }, $req->spliceHdrs,
               Cookie => ( $req->{Cookie} // '' );
             return $res;
diff --git a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server/Nginx.pm b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server/Nginx.pm
index c652722e3d..bb5571fa81 100644
--- a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server/Nginx.pm
+++ b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server/Nginx.pm
@@ -17,7 +17,7 @@ sub init {
 }
 
 ## @method void _run()
-# Return a subroutine that call _logAuthTrace() and tranform redirection
+# Return a subroutine that call _authAndTrace() and tranform redirection
 # response code from 302 to 401 (not authenticated) ones. This is required
 # because Nginx "auth_request" parameter does not accept it. The Nginx
 # configuration file should transform them back to 302 using:
@@ -31,7 +31,7 @@ sub _run {
     return $self->psgiAdapter(
         sub {
             my $req = $_[0];
-            my $res = $self->_logAuthTrace($req);
+            my $res = $self->_authAndTrace($req);
 
             # Transform 302 responses in 401 since Nginx refuse it
             if ( $res->[0] == 302 or $res->[0] == 303 ) {
diff --git a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server/Traefik.pm b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server/Traefik.pm
index 100460fb7c..4edbafa652 100644
--- a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server/Traefik.pm
+++ b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Server/Traefik.pm
@@ -17,11 +17,11 @@ sub init {
 sub _run {
     my $self = shift;
 
-    # Create regular logAuthTrace PSGI app
+    # Create regular _authAndTrace PSGI app
     my $app = $self->psgiAdapter(
         sub {
             my $req = $_[0];
-            return $self->_logAuthTrace($req);
+            return $self->_authAndTrace($req);
         }
     );
 
-- 
GitLab