[Catalyst] Plugin::Authentication overrides $c->req->user

[Daniel Westermann-Clark]

On 2009-02-11 21:53:48 +0000, Tomas Doran wrote:
>> Why not just add a remote_user() method on $c->req instead? It's a
>> little more typing, but is more explicit about where the value comes
>> from and doesn't potentially break any existing apps.
>
> Patches on 5.80 welcome :)

Attached is a set of patches to add support for $c->req->remote_user,
including a basic test.

There is also a deprecation warning for non-Catalyst packages calling
$c->req->user.

I'm not sure about the engine patches, but it seemed like a
forward-compatible way to add support now for the new method.

-- 
Daniel Westermann-Clark
-------------- next part --------------
=== Changes
==================================================================
--- Changes	(revision 25438)
+++ Changes	(local)
@@ -28,6 +28,7 @@
         - namespace::clean related cleanups (rafl)
         - Import related cleanups and consistency fixes (rafl)
         - Fix test suite TestApp /dump/env action (t0m)
+        - Add $c->req->remote_user to disambiguate from $c->req->user (dwc)
 
 5.8000_06 2009-02-04 21:00
         - Disallow writing to config after setup (rafl)
=== lib/Catalyst/Engine/CGI.pm
==================================================================
--- lib/Catalyst/Engine/CGI.pm	(revision 25438)
+++ lib/Catalyst/Engine/CGI.pm	(local)
@@ -72,7 +72,8 @@
 
     $request->hostname( $ENV{REMOTE_HOST} ) if exists $ENV{REMOTE_HOST};
     $request->protocol( $ENV{SERVER_PROTOCOL} );
-    $request->user( $ENV{REMOTE_USER} );
+    $request->user( $ENV{REMOTE_USER} );  # XXX: Deprecated. See Catalyst::Request for removal information
+    $request->remote_user( $ENV{REMOTE_USER} );
     $request->method( $ENV{REQUEST_METHOD} );
 
     if ( $ENV{HTTPS} && uc( $ENV{HTTPS} ) eq 'ON' ) {
=== lib/Catalyst/Request.pm
==================================================================
--- lib/Catalyst/Request.pm	(revision 25438)
+++ lib/Catalyst/Request.pm	(local)
@@ -26,7 +26,7 @@
 has secure => (is => 'rw', default => 0);
 has captures => (is => 'rw', default => sub { [] });
 has uri => (is => 'rw', predicate => 'has_uri');
-has user => (is => 'rw');
+has remote_user => (is => 'rw');
 has headers => (
   is      => 'rw',
   isa     => 'HTTP::Headers',
@@ -121,6 +121,20 @@
 
 has _path => ( is => 'rw', predicate => '_has_path', clearer => '_clear_path' );
 
+# XXX: Deprecated in 5.8000 due to confusion between Engines and Plugin::Authentication. Remove in 5.x000?
+has user => (is => 'rw');
+
+before user => sub {
+  my ($self, $user) = @_;
+  # Allow Engines and Plugin::Authentication to set without warning
+  my $caller = (caller(2))[0];
+  if ( $caller !~ /^Catalyst::(Engine|Plugin::Authentication)/ ) {
+    $self->_context->log->warn(
+      'Attempt to use $c->req->user; this is deprecated. ' .
+      'You probably meant to call $c->user or $c->req->remote_user' );
+  }
+};
+
 sub args            { shift->arguments(@_) }
 sub body_params     { shift->body_parameters(@_) }
 sub input           { shift->body(@_) }
@@ -577,6 +591,11 @@
 Returns the currently logged in user. Deprecated. The method recommended for
 newer plugins is $c->user.
 
+=head2 $req->remote_user
+
+Returns the value of the C<REMOTE_USER> environment variable. Previously
+available via $req->user.
+
 =head2 $req->user_agent
 
 Shortcut to $req->headers->user_agent. Returns the user agent (browser)
=== lib/Catalyst.pm
==================================================================
--- lib/Catalyst.pm	(revision 25438)
+++ lib/Catalyst.pm	(local)
@@ -2595,6 +2595,8 @@
 
 Drew Taylor
 
+dwc: Daniel Westermann-Clark <danieltwc at cpan.org>
+
 esskar: Sascha Kiefer
 
 fireartist: Carl Franks <cfranks at cpan.org>
=== t/aggregate/live_engine_request_remote_user.t
==================================================================
--- t/aggregate/live_engine_request_remote_user.t	(revision 25438)
+++ t/aggregate/live_engine_request_remote_user.t	(local)
@@ -0,0 +1,42 @@
+#!perl
+
+# This tests to make sure the REMOTE_USER environment variable is properly passed through by the engine.
+
+use strict;
+use warnings;
+
+use FindBin;
+use lib "$FindBin::Bin/../lib";
+
+use Test::More tests => 7;
+use Catalyst::Test 'TestApp';
+
+use Catalyst::Request;
+use HTTP::Request::Common;
+
+{
+    my $creq;
+
+    local $ENV{REMOTE_USER} = 'dwc';
+    my $request = GET(
+        'http://localhost/dump/request',
+    );
+
+    ok( my $response = request($request), 'Request' );
+    ok( $response->is_success, 'Response Successful 2xx' );
+    is( $response->content_type, 'text/plain', 'Response Content-Type' );
+    like( $response->content, qr/'Catalyst::Request'/,
+        'Content is a serialized Catalyst::Request' );
+
+    {
+        no strict 'refs';
+        ok(
+            eval '$creq = ' . $response->content,
+            'Unserialize Catalyst::Request'
+        );
+    }
+
+    isa_ok( $creq, 'Catalyst::Request' );
+    
+    is( $creq->remote_user, 'dwc', '$c->req->remote_user ok' );
+}
-------------- next part --------------
=== Changes
==================================================================
--- Changes	(revision 25262)
+++ Changes	(local)
@@ -1,5 +1,7 @@
 This file documents the revision history for Catalyst::Engine::Apache.
 
+        - Add support for req->remote_user, added in Catalyst 5.80
+
 1.13
         - Workaround change in LWP that broke a cookie test (RT #40037)
         - Update streaming test to latest 5.70
=== lib/Catalyst/Engine/Apache.pm
==================================================================
--- lib/Catalyst/Engine/Apache.pm	(revision 25262)
+++ lib/Catalyst/Engine/Apache.pm	(local)
@@ -47,6 +47,9 @@
     $c->request->hostname( $self->apache->connection->remote_host );
     $c->request->protocol( $self->apache->protocol );
     $c->request->user( $self->apache->user );
+    if ( $c->request->can('remote_user') ) {
+        $c->request->remote_user( $self->apache->user );
+    }
 
     # when config options are set, check them here first
     if ($INC{'Apache2/ModSSL.pm'}) {
-------------- next part --------------
=== Changes
==================================================================
--- Changes	(revision 25262)
+++ Changes	(local)
@@ -1,4 +1,6 @@
 This file documents the revision history for Catalyst::Engine::Zeus.
 
+        - Add support for req->remote_user, added in Catalyst 5.80
+
 0.01  2005-05-24 18:46:00
         - Initial release
=== lib/Catalyst/Engine/Zeus/Base.pm
==================================================================
--- lib/Catalyst/Engine/Zeus/Base.pm	(revision 25262)
+++ lib/Catalyst/Engine/Zeus/Base.pm	(local)
@@ -118,6 +118,9 @@
     $c->request->hostname( $c->zeus->connection->remote_host );
     $c->request->protocol( $c->zeus->protocol );
     $c->request->user( $c->zeus->user );
+    if ( $c->request->can('remote_user') ) {
+        $c->request->remote_user( $c->zeus->user );
+    }
     
     if ( $ENV{HTTPS} || $c->zeus->get_server_port == 443 ) {
         $c->request->secure(1);