[Rt-commit] rt branch, 4.2/web-external-auth, created. rt-4.0.6-455-gc8c8a3f

Thomas Sibley trs at bestpractical.com
Tue Aug 7 21:43:23 EDT 2012


The branch, 4.2/web-external-auth has been created
        at  c8c8a3f16b237fed11be5d91b1cb41dbbd682901 (commit)

- Log -----------------------------------------------------------------
commit d305cf6f6107cc763fb80b675ad11c175bc6c266
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Wed Jul 25 16:31:19 2012 -0700

    Rename and comment test file for a more accurate description

diff --git a/t/web/remote_user.t b/t/web/basic_auth.t
similarity index 89%
rename from t/web/remote_user.t
rename to t/web/basic_auth.t
index edad6ef..c6f2fb3 100644
--- a/t/web/remote_user.t
+++ b/t/web/basic_auth.t
@@ -13,9 +13,12 @@ sub auth {
 }
 
 my ( $url, $m ) = RT::Test->started_ok( basic_auth => 1 );
+
+# This tests the plack middleware, not RT
 $m->get($url);
 is($m->status, 401, "Initial request with no creds gets 401");
 
+# This tests the plack middleware, not RT
 $m->get($url, auth( root => "wrong" ));
 is($m->status, 401, "Request with wrong creds gets 401");
 
@@ -28,6 +31,7 @@ $m->content_like(
 );
 $m->content_unlike(qr/Logout/i, "Has no logout button, no WebFallbackToInternalAuth");
 
+# Again, testing the plack middleware
 $m->get($url);
 is($m->status, 401, "Subsequent requests without credentials aren't still logged in");
 

commit 7e0cd1409b8ee96309cc49c3b575fd4a8fe8fc12
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Thu Jul 26 10:03:09 2012 -0700

    Anonymous basic auth support in tests
    
    To use, supply basic_auth => 'anon' to RT::Test->started_ok.
    
    This provides a way to pass arbitrary REMOTE_USER values into RT from
    tests, which enables testing of RT's responses in mixed auth
    environments.  Without this, all tests for auth declined responses
    (401/403 statuses) come from the auth layer rather than RT.

diff --git a/lib/RT/Test.pm b/lib/RT/Test.pm
index d9c43ab..385ddf7 100644
--- a/lib/RT/Test.pm
+++ b/lib/RT/Test.pm
@@ -1342,7 +1342,7 @@ sub test_app {
         require Plack::Middleware::Auth::Basic;
         $app = Plack::Middleware::Auth::Basic->wrap(
             $app,
-            authenticator => sub {
+            authenticator => $server_opt{basic_auth} eq 'anon' ? sub { 1 } : sub {
                 my ($username, $password) = @_;
                 return $username eq 'root' && $password eq 'password';
             }
diff --git a/lib/RT/Test/Apache.pm b/lib/RT/Test/Apache.pm
index b2733ea..28f2615 100644
--- a/lib/RT/Test/Apache.pm
+++ b/lib/RT/Test/Apache.pm
@@ -83,6 +83,23 @@ sub basic_auth {
 EOT
 }
 
+sub basic_auth_anon {
+    my $self = shift;
+
+    return <<"EOT";
+    AuthType Basic
+    AuthName "restricted area"
+    AuthBasicProvider anon
+
+    Anonymous *
+    Anonymous_NoUserID On
+    Anonymous_MustGiveEmail Off
+    Anonymous_VerifyEmail Off
+
+    Require valid-user
+EOT
+}
+
 sub start_server {
     my ($self, %config) = @_;
     my %tmp = %{$config{tmp}};
@@ -108,8 +125,14 @@ sub start_server {
         rt_sbin_path   => $RT::SbinPath,
         rt_site_config => $ENV{'RT_SITE_CONFIG'},
         load_modules   => $info{load_modules},
-        basic_auth     => $config{basic_auth} ? $self->basic_auth : "",
     );
+    if ($config{basic_auth}) {
+        if ($config{basic_auth} eq 'anon') {
+            $opt{basic_auth} = $self->basic_auth_anon;
+        } else {
+            $opt{basic_auth} = $self->basic_auth;
+        }
+    }
     foreach (qw(log pid lock)) {
         $opt{$_ .'_file'} = File::Spec->catfile(
             "$tmp{'directory'}", "apache.$_"
@@ -193,7 +216,10 @@ sub apache_server_info {
     ) unless exists $MODULES{$res{version}}{$res{variant}};
 
     my @mlist = @{$MODULES{$res{version}}{$res{variant}}};
-    push @mlist, "authn_file", "auth_basic", "authz_user" if $res{basic_auth};
+    if ($res{basic_auth}) {
+        push @mlist, "auth_basic", "authz_user";
+        push @mlist, $res{basic_auth} eq 'anon' ? "authn_anon" : "authn_file";
+    }
 
     $res{'load_modules'} = '';
     foreach my $mod ( @mlist ) {

commit c7945e593c75b099675cbaf3cd0d9de6c77134a8
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Thu Jul 26 13:38:29 2012 -0700

    Failing tests for deauthenticating when REMOTE_USER disappears

diff --git a/t/web/remote_user.t b/t/web/remote_user.t
new file mode 100644
index 0000000..314bdba
--- /dev/null
+++ b/t/web/remote_user.t
@@ -0,0 +1,109 @@
+use strict;
+use warnings;
+use RT;
+use RT::Test plan => 'no_plan';
+use MIME::Base64 qw//;
+
+sub auth {
+    return Authorization => "Basic " .
+        MIME::Base64::encode( join(":", @_) );
+}
+
+sub logged_in_as {
+    my $mech = shift;
+    my $user = shift || '';
+
+    unless ($mech->status == 200) {
+        diag "Error: status is ". $mech->status;
+        return 0;
+    }
+
+    RT::Interface::Web::EscapeUTF8(\$user);
+    unless ($mech->content =~ m{<span class="current-user">\Q$user\E</span>}i) {
+        diag "Error: page has no user name";
+        return 0;
+    }
+    return 1;
+}
+
+sub stop_server {
+    my $mech = shift;
+
+    # Ensure we're logged in for the final warnings check
+    $$mech->default_header( auth("root") );
+
+    # Force the warnings check before we stop the server
+    undef $$mech;
+
+    RT::Test->stop_server;
+}
+
+diag "Continuous + Fallback";
+{
+    RT->Config->Set( DevelMode => 0 );
+    RT->Config->Set( WebExternalAuth => 1 );
+    RT->Config->Set( WebExternalAuthContinuous => 1 );
+    RT->Config->Set( WebFallbackToInternalAuth => 1 );
+    RT->Config->Set( WebExternalAuto => 0 );
+
+    my ( $url, $m ) = RT::Test->started_ok( basic_auth => 'anon' );
+
+    diag "Internal auth";
+    {
+        # Empty REMOTE_USER
+        $m->default_header( auth("") );
+
+        # First request gets the login form
+        $m->get_ok($url, "No basic auth is OK");
+        $m->content_like(qr/Login/, "Login form");
+
+        # Log in using RT's form
+        $m->submit_form_ok({
+            with_fields => {
+                user => 'root',
+                pass => 'password',
+            },
+        }, "Submitted login form");
+        ok logged_in_as($m, "root"), "Logged in as root";
+
+        # Still logged in on another request without REMOTE_USER
+        $m->follow_link_ok({ text => 'My Tickets' });
+        ok logged_in_as($m, "root"), "Logged in as root";
+
+        ok $m->logout, "Logged out";
+
+        # We're definitely logged out?
+        $m->get_ok($url);
+        $m->content_like(qr/Login/, "Login form");
+    }
+
+    diag "External auth";
+    {
+        # REMOTE_USER of root
+        $m->default_header( auth("root") );
+
+        # Automatically logged in as root without Login page
+        $m->get_ok($url);
+        ok logged_in_as($m, "root"), "Logged in as root";
+
+        # Still logged in on another request
+        $m->follow_link_ok({ text => 'My Tickets' });
+        ok logged_in_as($m, "root"), "Still logged in as root";
+
+        # Drop credentials and...
+        $m->default_header( auth("") );
+
+        # ...see if RT notices
+        $m->get($url);
+        is $m->status, 403, "403 Forbidden from RT";
+        is $m->content, '', "No content returned";
+        # XXX should this be a 403?
+
+        # Next request gets us the login form
+        $m->get_ok($url);
+        $m->content_like(qr/Login/, "Login form");
+    }
+
+    stop_server(\$m);
+}
+

commit 9354f0ae8f6330572574deed2fb59be905492b38
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Wed Jul 25 16:11:53 2012 -0700

    Cleanup WebExternalAuth handling
    
    This standardizes on a 403 Forbidden response and avoids ever sending a
    login page with no form.

diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 8a01217..dc8b4bc 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -623,14 +623,11 @@ sub AttemptExternalAuth {
                 }
                 $HTML::Mason::Commands::session{'CurrentUser'}->Load($user);
             } else {
-
-                # we failed to successfully create the user. abort abort abort.
-                delete $HTML::Mason::Commands::session{'CurrentUser'};
+                # we failed to successfully create the user!
+                _ForceLogout();
 
                 if (RT->Config->Get('WebFallbackToInternalAuth')) {
                     TangentForLoginWithError('Cannot create user: [_1]', $msg);
-                } else {
-                    $m->abort();
                 }
             }
         }
@@ -647,22 +644,24 @@ sub AttemptExternalAuth {
             # straight-up external auth would always redirect to /
             # when you first hit it.
         } else {
-            delete $HTML::Mason::Commands::session{'CurrentUser'};
+            # Couldn't auth with the REMOTE_USER provided, either because an RT
+            # user doesn't exist or we can't create one.  Bail unless we
+            # fallback to internal auth.
             $user = $orig_user;
+            AbortExternalAuth() unless RT->Config->Get('WebFallbackToInternalAuth');
         }
-    } elsif ( RT->Config->Get('WebFallbackToInternalAuth') ) {
-        unless ( defined $HTML::Mason::Commands::session{'CurrentUser'} ) {
-            # XXX unreachable due to prior defaulting in HandleRequest (check c34d108)
-            TangentForLoginWithError('You are not an authorized user');
-        }
-    } else {
-
-        # WebExternalAuth is set, but we don't have a REMOTE_USER. abort
-        # XXX: we must return AUTH_REQUIRED status or we fallback to
-        # internal auth here too.
-        delete $HTML::Mason::Commands::session{'CurrentUser'}
-            if defined $HTML::Mason::Commands::session{'CurrentUser'};
     }
+    elsif (not RT->Config->Get('WebFallbackToInternalAuth')) {
+        # No REMOTE_USER and we don't want to fallback internally.
+        AbortExternalAuth();
+    }
+}
+
+sub AbortExternalAuth {
+    _ForceLogout();
+
+    # Return a 403 Forbidden or we may fallback to a login page with no form
+    $HTML::Mason::Commands::m->abort(403);
 }
 
 sub AttemptPasswordAuthentication {

commit 154143c13462f95556f162aa5d1ecb0a2726d4a9
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Wed Jul 25 16:16:38 2012 -0700

    Deauthentication under continuous external auth with internal fallback
    
    Under WebExternalAuth configurations with both WebExternalAuthContinuous
    and WebFallbackToInternalAuth enabled, RT now properly logs out
    externally authed users who are deauthenticated outside of RT during their
    RT session.
    
    Previously this configuration checked for REMOTE_USER on every request,
    but still allowed users to remain logged in even after they stopped
    being authenticated by the external source.  WebFallbackToInternalAuth
    prevented WebExternalAuthContinuous from working properly, presumably in
    order to let internally authed users stay logged in despite their lack
    of REMOTE_USER.
    
    By setting a flag on externally authed users' sessions, we can
    differentiate between those users and users who logged into RT directly.
    This difference lets us enforce WebExternalAuthContinuous for only the
    external users, as intended.

diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index dc8b4bc..f8229fb 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -633,6 +633,7 @@ sub AttemptExternalAuth {
         }
 
         if ( _UserLoggedIn() ) {
+            $HTML::Mason::Commands::session{'WebExternallyAuthed'} = 1;
             $m->callback( %$ARGS, CallbackName => 'ExternalAuthSuccessfulLogin', CallbackPage => '/autohandler' );
             # It is possible that we did a redirect to the login page,
             # if the external auth allows lack of auth through with no
@@ -651,8 +652,12 @@ sub AttemptExternalAuth {
             AbortExternalAuth() unless RT->Config->Get('WebFallbackToInternalAuth');
         }
     }
-    elsif (not RT->Config->Get('WebFallbackToInternalAuth')) {
-        # No REMOTE_USER and we don't want to fallback internally.
+    elsif (not RT->Config->Get('WebFallbackToInternalAuth')
+            or (_UserLoggedIn() and $HTML::Mason::Commands::session{'WebExternallyAuthed'})) {
+        # No REMOTE_USER and...
+        #
+        # a) We don't want to fallback internally, or
+        # b) The logged in external user was deauthed and we should kick them out
         AbortExternalAuth();
     }
 }

commit 735255a455eff51a195b1046e90fae56e5b9a207
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Mon Jul 30 17:04:55 2012 -0700

    Standard error pages for all reasons we abort external auth
    
    Instead of a blank page, show brief, friendlier error messages with
    information about why auth failed, contact information for the local
    admin, and an internal login link when fallback is on.  Rendering each
    error as an individual component allows easier site-specific overrides
    for customizing messages to the external authentication system in use.
    
    Behaviour under WebFallbackToInternalAuth is changed from showing login
    pages with no message on failure to showing an error message with a way
    to login.  Below is a summary of the new behaviour for each reason we
    abort and force logout during WebExternalAuth checking.  All error pages
    are 403s and provide a login link if Fallback is ON.
    
    User doesn't exist and auto-create is OFF
        Fallback  ON: Error page, since if they were supposed to login it would
                      likely be linked to remote user
    
        Fallback OFF: Error page
    
    Fail to auto-create a user (iff WebExternalAuto is ON)
        Fallback  ON: Error page with "couldn't create user message", since they're
                      unlikely to be able to do anything about it.
    
        Fallback OFF: Error page with "couldn't create user" message
    
    User logged in but deauthed by external system (iff WebExternalAuthContinuous is ON)
        Fallback  ON: Error page, suggesting refresh. Next page hit gets them
                      logged in again if it was a blip or the login form if it wasn't.
    
        Fallback OFF: Same error page as above, except they'll get the generic
                      error page below if it wasn't a blip when they refresh.
    
    No remote user
        Fallback  ON: Login page
        Fallback OFF: Error page, generic

diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index f8229fb..7185897 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -579,8 +579,6 @@ sub AttemptExternalAuth {
 
     # do we actually have a REMOTE_USER equivlent?
     if ( RT::Interface::Web::WebCanonicalizeInfo() ) {
-        my $orig_user = $user;
-
         $user = RT::Interface::Web::WebCanonicalizeInfo();
         my $load_method = RT->Config->Get('WebExternalGecos') ? 'LoadByGecos' : 'Load';
 
@@ -623,12 +621,8 @@ sub AttemptExternalAuth {
                 }
                 $HTML::Mason::Commands::session{'CurrentUser'}->Load($user);
             } else {
-                # we failed to successfully create the user!
-                _ForceLogout();
-
-                if (RT->Config->Get('WebFallbackToInternalAuth')) {
-                    TangentForLoginWithError('Cannot create user: [_1]', $msg);
-                }
+                RT->Logger->error("Couldn't auto-create user '$user' when attempting WebExternalAuth: $msg");
+                AbortExternalAuth( Error => "AutoCreate" );
             }
         }
 
@@ -645,28 +639,48 @@ sub AttemptExternalAuth {
             # straight-up external auth would always redirect to /
             # when you first hit it.
         } else {
-            # Couldn't auth with the REMOTE_USER provided, either because an RT
-            # user doesn't exist or we can't create one.  Bail unless we
-            # fallback to internal auth.
-            $user = $orig_user;
-            AbortExternalAuth() unless RT->Config->Get('WebFallbackToInternalAuth');
+            # Couldn't auth with the REMOTE_USER provided because an RT
+            # user doesn't exist and we're configured not to create one.
+            RT->Logger->error("Couldn't find internal user for '$user' when attempting WebExternalAuth and RT is not configured for auto-creation.");
+            AbortExternalAuth(
+                Error => "NoInternalUser",
+                User  => $user,
+            );
         }
     }
-    elsif (not RT->Config->Get('WebFallbackToInternalAuth')
-            or (_UserLoggedIn() and $HTML::Mason::Commands::session{'WebExternallyAuthed'})) {
-        # No REMOTE_USER and...
-        #
-        # a) We don't want to fallback internally, or
-        # b) The logged in external user was deauthed and we should kick them out
-        AbortExternalAuth();
+    elsif (_UserLoggedIn() and $HTML::Mason::Commands::session{'WebExternallyAuthed'}) {
+        # The logged in external user was deauthed by the auth system and we
+        # should kick them out.
+        AbortExternalAuth( Error => "Deauthorized" );
+    }
+    else {
+        # Abort if we don't want to fallback internally
+        MaybeAbortExternalAuth( Error => "NoRemoteUser" );
     }
 }
 
+sub MaybeAbortExternalAuth {
+    return if RT->Config->Get('WebFallbackToInternalAuth');
+    AbortExternalAuth(@_);
+}
+
 sub AbortExternalAuth {
+    my %args  = @_;
+    my $error = $args{Error} ? "/Errors/WebExternalAuth/$args{Error}" : undef;
+    my $m     = $HTML::Mason::Commands::m;
+    my $r     = $HTML::Mason::Commands::r;
+
     _ForceLogout();
 
+    # Clear the decks, not that we should have partial content.
+    $m->clear_buffer;
+
+    $r->status(403);
+    $m->comp($error, %args)
+        if $error and $m->comp_exists($error);
+
     # Return a 403 Forbidden or we may fallback to a login page with no form
-    $HTML::Mason::Commands::m->abort(403);
+    $m->abort(403);
 }
 
 sub AttemptPasswordAuthentication {
diff --git a/share/html/Errors/WebExternalAuth/AutoCreate b/share/html/Errors/WebExternalAuth/AutoCreate
new file mode 100644
index 0000000..554ba54
--- /dev/null
+++ b/share/html/Errors/WebExternalAuth/AutoCreate
@@ -0,0 +1,3 @@
+<&| Wrapper, %ARGS, Title => loc("Automatic account setup failed") &>
+<p><&|/l&>Unfortunately, RT couldn't automatically setup an account for you. Your RT administator will find more information in the logs.</&></p>
+</&>
diff --git a/share/html/Errors/WebExternalAuth/Deauthorized b/share/html/Errors/WebExternalAuth/Deauthorized
new file mode 100644
index 0000000..e31068f
--- /dev/null
+++ b/share/html/Errors/WebExternalAuth/Deauthorized
@@ -0,0 +1,3 @@
+<&| Wrapper, %ARGS, Title => loc("No longer authorized") &>
+<p><&|/l&>You were logged out of RT by your authentication system.  This may be a temporary hiccup, in which case refreshing this page may help.</&></p>
+</&>
diff --git a/share/html/Errors/WebExternalAuth/NoInternalUser b/share/html/Errors/WebExternalAuth/NoInternalUser
new file mode 100644
index 0000000..8ffa295
--- /dev/null
+++ b/share/html/Errors/WebExternalAuth/NoInternalUser
@@ -0,0 +1,3 @@
+<&| Wrapper, %ARGS, Title => loc("Unauthorized") &>
+<p><&|/l, $ARGS{User} &>You ([_1]) are not authorized to use RT.</&></p>
+</&>
diff --git a/share/html/Errors/WebExternalAuth/NoRemoteUser b/share/html/Errors/WebExternalAuth/NoRemoteUser
new file mode 100644
index 0000000..81d8ae0
--- /dev/null
+++ b/share/html/Errors/WebExternalAuth/NoRemoteUser
@@ -0,0 +1,3 @@
+<&| Wrapper, %ARGS, Title => loc("Unauthorized") &>
+<p><&|/l&>You are not authorized to use RT.</&></p>
+</&>
diff --git a/share/html/Errors/WebExternalAuth/Wrapper b/share/html/Errors/WebExternalAuth/Wrapper
new file mode 100644
index 0000000..a5318d8
--- /dev/null
+++ b/share/html/Errors/WebExternalAuth/Wrapper
@@ -0,0 +1,34 @@
+<%args>
+$Title => loc("An error occurred")
+$Error => ''
+</%args>
+<%init>
+my $next = RT::Interface::Web::SetNextPage();
+my $login_url = RT->Config->Get('WebPath') . "/NoAuth/Login.html?next=$next";
+</%init>
+<html>
+  <head>
+    <title><% $Title %></title>
+  </head>
+  <body>
+    <h1><% $Title %></h1>
+    <!-- WebExternalAuth error: <% $Error %> -->
+    <% $m->content |n%>
+
+    <p id="contact-admin">
+% if (my $owner = RT->Config->Get('OwnerEmail')) {
+%     $owner = $m->interp->apply_escapes($owner, 'h');
+      <&|/l_unsafe, qq[<a href="mailto:$owner">], $owner, '</a>' &>Contact your RT administrator via [_1]email to [_2][_3].</&>
+% } else {
+      <&|/l&>Contact your RT administrator.</&>
+% }
+    </p>
+
+% if (RT->Config->Get('WebExternalAuth') and RT->Config->Get('WebFallbackToInternalAuth')) {
+    <p id="internal-auth">
+      <&|/l_unsafe, qq[<a href="$login_url">], '</a>' &>If you have an internal RT login, you may [_1]try it instead[_2].</&>
+    </p>
+% }
+    </p>
+  </body>
+</html>
diff --git a/t/web/remote_user.t b/t/web/remote_user.t
index 314bdba..d5837fd 100644
--- a/t/web/remote_user.t
+++ b/t/web/remote_user.t
@@ -96,14 +96,64 @@ diag "Continuous + Fallback";
         # ...see if RT notices
         $m->get($url);
         is $m->status, 403, "403 Forbidden from RT";
-        is $m->content, '', "No content returned";
-        # XXX should this be a 403?
 
         # Next request gets us the login form
         $m->get_ok($url);
         $m->content_like(qr/Login/, "Login form");
     }
 
+    diag "External auth with invalid user, login internally";
+    {
+        # REMOTE_USER of invalid
+        $m->default_header( auth("invalid") );
+
+        # Login internally via the login link
+        $m->get("$url/Search/Build.html");
+        is $m->status, 403, "403 Forbidden";
+        $m->follow_link_ok({ url_regex => qr'NoAuth/Login\.html' }, "follow logout link");
+        $m->content_like(qr/Login/, "Login form");
+
+        # Log in using RT's form
+        $m->submit_form_ok({
+            with_fields => {
+                user => 'root',
+                pass => 'password',
+            },
+        }, "Submitted login form");
+        ok logged_in_as($m, "root"), "Logged in as root";
+        like $m->uri, qr'Search/Build\.html', "at our originally requested page";
+
+        # Still logged in on another request
+        $m->follow_link_ok({ text => 'Tools' });
+        ok logged_in_as($m, "root"), "Logged in as root";
+
+        ok $m->logout, "Logged out";
+
+        $m->next_warning_like(qr/Couldn't find internal user for 'invalid'/, "found warning for first request");
+        $m->next_warning_like(qr/Couldn't find internal user for 'invalid'/, "found warning for second request");
+    }
+
+    stop_server(\$m);
+}
+
+diag "Fallback OFF";
+{
+    RT->Config->Set( DevelMode => 0 );
+    RT->Config->Set( WebExternalAuth => 1 );
+    RT->Config->Set( WebExternalAuthContinuous => 0 );
+    RT->Config->Set( WebFallbackToInternalAuth => 0 );
+    RT->Config->Set( WebExternalAuto => 0 );
+
+    my ( $url, $m ) = RT::Test->started_ok( basic_auth => 'anon' );
+
+    diag "No remote user";
+    {
+        $m->default_header( auth("") );
+        $m->get($url);
+        is $m->status, 403, "Forbidden";
+    }
+
     stop_server(\$m);
 }
 
+# XXX TODO: test WebExternalAuto and AutoCreate

commit e5a6d21051d1107a480ea9619068420ff8dcb08d
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Mon Aug 6 18:35:57 2012 -0700

    Ignore REMOTE_USER if we're logged in with internal auth
    
    This prevents logged in internally-authed users from being logged out if
    they pick up a REMOTE_USER.  Viewed another way, it lets users with an
    invalid REMOTE_USER still login with internal credentials.
    
    All tests in t/web/remote_user.t now pass.

diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 7185897..e606995 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -575,10 +575,17 @@ sub AttemptExternalAuth {
     my $user = $ARGS->{user};
     my $m    = $HTML::Mason::Commands::m;
 
+    my $logged_in_external_user = _UserLoggedIn() && $HTML::Mason::Commands::session{'WebExternallyAuthed'};
+
     # If RT is configured for external auth, let's go through and get REMOTE_USER
 
-    # do we actually have a REMOTE_USER equivlent?
-    if ( RT::Interface::Web::WebCanonicalizeInfo() ) {
+    # Do we actually have a REMOTE_USER or equivalent?  We only check auth if
+    # 1) we have no logged in user, or 2) we have a user who is externally
+    # authed.  If we have a logged in user who is internally authed, don't
+    # check remote user otherwise we may log them out.
+    if (RT::Interface::Web::WebCanonicalizeInfo()
+        and (not _UserLoggedIn() or $logged_in_external_user) )
+    {
         $user = RT::Interface::Web::WebCanonicalizeInfo();
         my $load_method = RT->Config->Get('WebExternalGecos') ? 'LoadByGecos' : 'Load';
 
@@ -648,7 +655,7 @@ sub AttemptExternalAuth {
             );
         }
     }
-    elsif (_UserLoggedIn() and $HTML::Mason::Commands::session{'WebExternallyAuthed'}) {
+    elsif ($logged_in_external_user) {
         # The logged in external user was deauthed by the auth system and we
         # should kick them out.
         AbortExternalAuth( Error => "Deauthorized" );

commit b37a982995afb64b0101e738476b6de6eb66599f
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Tue Aug 7 17:08:56 2012 -0700

    Refactor the generation of login tangent links
    
    This avoids the error wrapper needing to know the details of how login
    tangents work.

diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index e606995..c6d5834 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -377,11 +377,23 @@ the next page.  Optionally takes a hash which is dumped into query params.
 =cut
 
 sub TangentForLogin {
+    my $login = TangentForLoginURL(@_);
+    Redirect( RT->Config->Get('WebBaseURL') . $login );
+}
+
+=head2 TangentForLoginURL [HASH]
+
+Returns a URL suitable for tangenting for login.  Optionally takes a hash which
+is dumped into query params.
+
+=cut
+
+sub TangentForLoginURL {
     my $hash  = SetNextPage();
     my %query = (@_, next => $hash);
-    my $login = RT->Config->Get('WebURL') . 'NoAuth/Login.html?';
+    my $login = RT->Config->Get('WebPath') . '/NoAuth/Login.html?';
     $login .= $HTML::Mason::Commands::m->comp('/Elements/QueryString', %query);
-    Redirect($login);
+    return $login;
 }
 
 =head2 TangentForLoginWithError ERROR
diff --git a/share/html/Errors/WebExternalAuth/Wrapper b/share/html/Errors/WebExternalAuth/Wrapper
index a5318d8..548d842 100644
--- a/share/html/Errors/WebExternalAuth/Wrapper
+++ b/share/html/Errors/WebExternalAuth/Wrapper
@@ -3,8 +3,7 @@ $Title => loc("An error occurred")
 $Error => ''
 </%args>
 <%init>
-my $next = RT::Interface::Web::SetNextPage();
-my $login_url = RT->Config->Get('WebPath') . "/NoAuth/Login.html?next=$next";
+my $login_url = $m->interp->apply_escapes(RT::Interface::Web::TangentForLoginURL(), 'h');
 </%init>
 <html>
   <head>

commit 231779e0ecf274b1e696e9b5a67b141e0a88b405
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Tue Aug 7 18:26:03 2012 -0700

    Tests for the WebExternalAuto and AutoCreate options
    
    One test fails (creating privileged users by default) and is marked TODO
    because of a longstanding bug.

diff --git a/t/web/remote_user.t b/t/web/remote_user.t
index d5837fd..81b9aa7 100644
--- a/t/web/remote_user.t
+++ b/t/web/remote_user.t
@@ -156,4 +156,67 @@ diag "Fallback OFF";
     stop_server(\$m);
 }
 
-# XXX TODO: test WebExternalAuto and AutoCreate
+diag "AutoCreate";
+{
+    RT->Config->Set( DevelMode => 0 );
+    RT->Config->Set( WebExternalAuth => 1 );
+    RT->Config->Set( WebExternalAuthContinuous => 1 );
+    RT->Config->Set( WebFallbackToInternalAuth => 0 );
+    RT->Config->Set( WebExternalAuto => 1 );
+    RT->Config->Set( AutoCreate => { Organization => "BPS" } );
+
+    my ( $url, $m ) = RT::Test->started_ok( basic_auth => 'anon' );
+
+    diag "New user";
+    {
+        $m->default_header( auth("anewuser") );
+        $m->get_ok($url);
+        ok logged_in_as($m, "anewuser"), "Logged in as anewuser";
+
+        my $user = RT::User->new( RT->SystemUser );
+        $user->Load("anewuser");
+        ok $user->id, "Found newly created user";
+        is $user->Organization, "BPS", "Found Organization from AutoCreate hash";
+        {
+            local $TODO = 'A bug from 2009 prevents Privileged from being set by WebExternalAutoInfo';
+            ok $user->Privileged, "Privileged by default";
+        }
+    }
+
+    stop_server(\$m);
+    RT->Config->Set(
+        AutoCreate => {
+            Privileged   => 0,
+            EmailAddress => 'foo at example.com',
+        },
+    );
+    ( $url, $m ) = RT::Test->started_ok( basic_auth => 'anon' );
+
+    diag "Create unprivileged users";
+    {
+        $m->default_header( auth("unpriv") );
+        $m->get_ok($url);
+        ok logged_in_as($m, "unpriv"), "Logged in as an unpriv user";
+        like $m->uri->path, RT->Config->Get('SelfServiceRegex'), "SelfService URL";
+
+        my $user = RT::User->new( RT->SystemUser );
+        $user->Load("unpriv");
+        ok $user->id, "Found newly created user";
+        ok !$user->Privileged, "Unprivileged per config";
+        is $user->EmailAddress, 'foo at example.com', "Email address per config";
+    }
+
+    diag "User creation failure";
+    {
+        $m->default_header( auth("conflicting") );
+        $m->get($url);
+        is $m->status, 403, "Forbidden";
+
+        my $user = RT::User->new( RT->SystemUser );
+        $user->Load("conflicting");
+        ok !$user->id, "Couldn't find conflicting user";
+    }
+
+    stop_server(\$m);
+}
+

commit c8c8a3f16b237fed11be5d91b1cb41dbbd682901
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Tue Aug 7 18:30:49 2012 -0700

    Allow Privileged and Disabled to be set from WebExternalAutoInfo()
    
    Fixes a regression introduced by e2ec2c4c in 2009.  The change to
    WritableAttributes was the correct approach (and let us pick up new
    columns automatically), but it missed that Privileged and Disabled
    aren't true User columns handled by DBIx::SearchBuilder.
    
    Privileged is particularly important since the default implementation of
    WebExternalAutoInfo() expects to be able to return it in order to make
    externally authed auto-created users Privileged by default.  The bug
    meant the default behaviour didn't work and you had to include
    Privileged => 1 in $AutoCreate if you wanted privileged users.

diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index c6d5834..df8b71e 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -627,7 +627,7 @@ sub AttemptExternalAuth {
                 my $new_user_info = RT::Interface::Web::WebExternalAutoInfo($user);
 
                 # set the attributes that have been defined.
-                foreach my $attribute ( $UserObj->WritableAttributes ) {
+                foreach my $attribute ( $UserObj->WritableAttributes, qw(Privileged Disabled) ) {
                     $m->callback(
                         Attribute    => $attribute,
                         User         => $user,
diff --git a/t/web/remote_user.t b/t/web/remote_user.t
index 81b9aa7..4ad1702 100644
--- a/t/web/remote_user.t
+++ b/t/web/remote_user.t
@@ -177,10 +177,7 @@ diag "AutoCreate";
         $user->Load("anewuser");
         ok $user->id, "Found newly created user";
         is $user->Organization, "BPS", "Found Organization from AutoCreate hash";
-        {
-            local $TODO = 'A bug from 2009 prevents Privileged from being set by WebExternalAutoInfo';
-            ok $user->Privileged, "Privileged by default";
-        }
+        ok $user->Privileged, "Privileged by default";
     }
 
     stop_server(\$m);

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


More information about the Rt-commit mailing list