[Bps-public-commit] rt-extension-rest2 branch, master, updated. 48dd7bbc67286e5a8e248b3e26d3a741dfeb961f

[Shawn Moore]

The branch, master has been updated
       via  48dd7bbc67286e5a8e248b3e26d3a741dfeb961f (commit)
       via  cfdca663451d00b411f90cd019d4d0acfd438cc0 (commit)
      from  fce0cb17639663e2fa7e4b806b8abaf6b8ec9fa1 (commit)

Summary of changes:
 Makefile.PL                               | 2 --
 lib/RT/Extension/REST2.pm                 | 6 ------
 lib/RT/Extension/REST2/Middleware/Auth.pm | 9 +++++++--
 t/lib/RT/Extension/REST2/Test.pm.in       | 2 +-
 4 files changed, 8 insertions(+), 11 deletions(-)

- Log -----------------------------------------------------------------
commit cfdca663451d00b411f90cd019d4d0acfd438cc0
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed Jul 19 16:35:52 2017 +0000

    Make cookie auth work under 4.2
    
    4.2 doesn't use the RequestENV abstraction, and so we need to override
    all of %ENV to provide the correct HTTP_COOKIE, SERVER_PORT, etc

diff --git a/lib/RT/Extension/REST2/Middleware/Auth.pm b/lib/RT/Extension/REST2/Middleware/Auth.pm
index af51ac4..875ebcf 100644
--- a/lib/RT/Extension/REST2/Middleware/Auth.pm
+++ b/lib/RT/Extension/REST2/Middleware/Auth.pm
@@ -32,12 +32,17 @@ sub login_from_cookie {
     # allow reusing authentication from the ordinary web UI so that
     # among other things our JS can use REST2
     if ($env->{HTTP_COOKIE}) {
+        no warnings 'redefine';
 
         # this is foul but LoadSessionFromCookie doesn't have a hook for
         # saying "look up cookie in my $env". this beats duplicating
         # LoadSessionFromCookie
-        no warnings 'redefine';
-        local *RT::Interface::Web::RequestENV = sub { return $env->{$_[0]} };
+        local *RT::Interface::Web::RequestENV = sub { return $env->{$_[0]} }
+            if RT::Handle::cmp_version($RT::VERSION, '4.4.0') >= 0;
+
+        # similar but for 4.2
+        local %ENV = %$env
+            if RT::Handle::cmp_version($RT::VERSION, '4.4.0') < 0;
 
         local *HTML::Mason::Commands::session;
 

commit 48dd7bbc67286e5a8e248b3e26d3a741dfeb961f
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed Jul 19 15:59:16 2017 +0000

    Remove unnecessary proxy middleware
    
    The ReverseProxyPath middleware was added in d86ef4b9 purportedly to
    accommodate Web::Simple's dispatcher, to avoid having to prefix every
    rule with /REST/2.0. However, PATH_INFO is correctly set to e.g.
    /ticket/1 even when the original request path looked like
    /rt/REST/2.0/ticket/1. This is because Plack::App::URLMap (used by
    Plack::Builder's mount) strips the /REST/2.0 prefix from PATH_INFO (and
    the /rt prefix was already long-since removed as part of the PSGI
    integration with the webserver).
    
    Reverting back to d86ef4b9 and interacting with the REST 2 API via curl
    successfully proves that the middleware isn't required for real RT
    deployment.
    
    However, the tests used a different codepath (->to_app) than real RT
    would (->PSGIWrap), leading their requests to include /REST/2.0 in
    PATH_INFO. I believe working around that problem is the real
    reason why the middleware were added. So instead, this commit fixes the
    test library to use the same codepath as real usage does.
    
    Furthermore, the RequestHeaders/ReverseProxyPath config we had broke on
    non-root hosted RTs (e.g. WebPath=/rt) with the following message:
    
        HTTP_X_TRAVERSAL_PATH: /REST/2.0
        is not a prefix of
        SCRIPT_NAME: /rt/REST/2.0
    
    due to its unnecessary misuse.

diff --git a/Makefile.PL b/Makefile.PL
index 10cad18..833bc4f 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -18,8 +18,6 @@ requires 'Plack::Builder';
 requires 'Scalar::Util';
 requires 'Sub::Exporter';
 requires 'Web::Machine' => '0.12';
-requires 'Plack::Middleware::RequestHeaders';
-requires 'Plack::Middleware::ReverseProxyPath';
 requires 'Module::Path';
 requires 'Pod::POM';
 requires 'Path::Dispatcher';
diff --git a/lib/RT/Extension/REST2.pm b/lib/RT/Extension/REST2.pm
index 48e5b91..2dcd277 100644
--- a/lib/RT/Extension/REST2.pm
+++ b/lib/RT/Extension/REST2.pm
@@ -564,12 +564,6 @@ sub to_app {
         enable '+RT::Extension::REST2::Middleware::ErrorAsJSON';
         enable '+RT::Extension::REST2::Middleware::Log';
         enable '+RT::Extension::REST2::Middleware::Auth';
-        enable 'RequestHeaders',
-            set => [
-                'X-Forwarded-Script-Name' => '/',
-                'X-Traversal-Path' => $REST_PATH,
-            ];
-        enable 'ReverseProxyPath';
         RT::Extension::REST2::Dispatcher->to_psgi_app;
     };
 }
diff --git a/t/lib/RT/Extension/REST2/Test.pm.in b/t/lib/RT/Extension/REST2/Test.pm.in
index 8ba0598..d4c9b5f 100644
--- a/t/lib/RT/Extension/REST2/Test.pm.in
+++ b/t/lib/RT/Extension/REST2/Test.pm.in
@@ -48,7 +48,7 @@ sub mech { RT::Extension::REST2::Test::Mechanize->new }
     sub new {
         my $class = shift;
         my %args = (
-            app => RT::Extension::REST2->to_app,
+            app => RT::Extension::REST2->PSGIWrap(sub { die "Requested non-REST path" }),
             @_,
         );
         return $class->SUPER::new(%args);

-----------------------------------------------------------------------