[Rt-commit] rt branch, 3.8-trunk, updated. rt-3.8.8-188-g0575522
Thomas Sibley
trs at bestpractical.com
Wed Oct 27 10:51:29 EDT 2010
The branch, 3.8-trunk has been updated
via 057552287159e801535e59b8fbd5bd98d1322069 (commit)
via d37f3e93f799824e84aa146dba44e5669525cb01 (commit)
via a87820750251000aba859cc45f510db27b0c1049 (commit)
via a813943bb23cb4692dabb634c53a2213f8a2300d (commit)
via e71c90f88c0f12936f3cae192ee4dc10bf11292c (commit)
via 6fa0a4e500a8be35a30fa9a7d9f7415ebb2d7405 (commit)
via 23a912a292ad0c3d4bca4d3f9aae39c0100bc042 (commit)
via 87408ef895b625d44512392de68c76a5c35fdd26 (commit)
via fbf7e7066d7d17df218c19cb4ef7e52871aef762 (commit)
via 917c211820590950f7eb0521f7f43b31aeed44c4 (commit)
from 432df1da150eff66d7b17b2b0812114e0db9896a (commit)
Summary of changes:
lib/RT/Interface/Web.pm | 193 ++++++++++++++--
share/html/Elements/ListActions | 3 +-
share/html/Elements/Login | 75 +-----
.../html/{Elements/TitleBox => NoAuth/Login.html} | 5 +-
share/html/NoAuth/css/web2/login.css | 4 +
t/web/redirect-after-login.t | 243 ++++++++++++++++++++
6 files changed, 435 insertions(+), 88 deletions(-)
copy share/html/{Elements/TitleBox => NoAuth/Login.html} (92%)
mode change 100644 => 100755
create mode 100644 t/web/redirect-after-login.t
- Log -----------------------------------------------------------------
commit 917c211820590950f7eb0521f7f43b31aeed44c4
Author: Thomas Sibley <trs at bestpractical.com>
Date: Fri Oct 15 17:11:14 2010 -0400
Redirect to the desired page after logging in a user
This prevents back button attacks in the form of resubmitting form data
after the user has logged out of the browser (but not closed it). See
also rt3 #15804.
For non-homepage hits, the browser now also gets redirected to
/NoAuth/Login.html to get a login form.
We use the session to store the next page URL (referenced by a hash),
similar to how action results work.
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 29deaa4..255f763 100755
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -207,13 +207,30 @@ sub HandleRequest {
unless ( _UserLoggedIn() ) {
_ForceLogout();
- # If the user is logging in, let's authenticate
- if ( defined $ARGS->{user} && defined $ARGS->{pass} ) {
- AttemptPasswordAuthentication($ARGS);
- } else {
- # if no credentials then show him login page
- $HTML::Mason::Commands::m->comp( '/Elements/Login', %$ARGS );
- $HTML::Mason::Commands::m->abort;
+ # Authenticate if the user is trying to login via user/pass query args
+ my ($authed, $msg) = AttemptPasswordAuthentication($ARGS);
+
+ unless ($authed) {
+ my $m = $HTML::Mason::Commands::m;
+ my $results = $msg ? LoginError($msg) : undef;
+
+ # REST urls get a special 401 response
+ if ($m->request_comp->path =~ '^/REST/\d+\.\d+/') {
+ $HTML::Mason::Commands::r->content_type("text/plain");
+ $m->error_format("text");
+ $m->out("RT/$RT::VERSION 401 Credentials required\n");
+ $m->out("\n$msg\n") if $msg;
+ $m->abort;
+ }
+ # Specially handle /index.html so that we get a nicer URL
+ elsif ( $m->request_comp->path eq '/index.html' ) {
+ my $next = SetNextPage(RT->Config->Get('WebURL'));
+ $m->comp('/NoAuth/Login.html', next => $next, results => $results);
+ $m->abort;
+ }
+ else {
+ TangentForLogin(results => $results);
+ }
}
}
@@ -245,6 +262,90 @@ sub _UserLoggedIn {
}
+=head2 LoginError ERROR
+
+Pushes a login error into the Actions session store and returns the hash key.
+
+=cut
+
+sub LoginError {
+ my $new = shift;
+ my $key = Digest::MD5::md5_hex( rand(1024) );
+ push @{ $HTML::Mason::Commands::session{"Actions"}->{$key} ||= [] }, $new;
+ $HTML::Mason::Commands::session{'i'}++;
+ return $key;
+}
+
+=head2 SetNextPage [PATH]
+
+Intuits and stashes the next page in the sesssion hash. If PATH is
+specified, uses that instead of the value of L<IntuitNextPage()>. Returns
+the hash value.
+
+=cut
+
+sub SetNextPage {
+ my $next = shift || IntuitNextPage();
+ my $hash = Digest::MD5::md5_hex( $next . $$ );
+
+ $HTML::Mason::Commands::session{'NextPage'}->{$hash} = $next;
+ $HTML::Mason::Commands::session{'i'}++; # Cargo culted, do we need this?
+
+ SendSessionCookie();
+ return $hash;
+}
+
+
+=head2 TangentForLogin [HASH]
+
+Redirects to C</NoAuth/Login.html>, setting the value of L<IntuitNextPage> as
+the next page. Optionally takes a hash which is dumped into query params.
+
+=cut
+
+sub TangentForLogin {
+ my $hash = SetNextPage();
+ my %query = (@_, next => $hash);
+ my $login = RT->Config->Get('WebURL') . 'NoAuth/Login.html?';
+ $login .= $HTML::Mason::Commands::m->comp('/Elements/QueryString', %query);
+ Redirect($login);
+}
+
+=head2 IntuitNextPage
+
+Attempt to figure out the path to which we should return the user after a
+tangent. The current request URL is used, or failing that, the C<WebURL>
+configuration variable.
+
+=cut
+
+sub IntuitNextPage {
+ my $req_uri;
+
+ # This includes any query parameters. Redirect will take care of making
+ # it an absolute URL.
+ $req_uri = $ENV{'REQUEST_URI'} if $ENV{'REQUEST_URI'};
+
+ my $next = defined $req_uri ? $req_uri : RT->Config->Get('WebURL');
+
+ # sanitize $next
+ my $uri = URI->new($next);
+
+ # You get undef scheme with a relative uri like "/Search/Build.html"
+ unless (!defined($uri->scheme) || $uri->scheme eq 'http' || $uri->scheme eq 'https') {
+ $next = RT->Config->Get('WebURL');
+ }
+
+ # Make sure we're logging in to the same domain
+ # You can get an undef authority with a relative uri like "index.html"
+ my $uri_base_url = URI->new(RT->Config->Get('WebBaseURL'));
+ unless (!defined($uri->authority) || $uri->authority eq $uri_base_url->authority) {
+ $next = RT->Config->Get('WebURL');
+ }
+
+ return $next;
+}
+
=head2 MaybeShowInstallModePage
This function, called exclusively by RT's autohandler, dispatches
@@ -284,6 +385,10 @@ sub MaybeShowNoAuthPage {
return unless $m->base_comp->path =~ RT->Config->Get('WebNoAuthRegex');
+ # Don't show the login page to logged in users
+ Redirect(RT->Config->Get('WebURL'))
+ if $m->base_comp->path eq '/NoAuth/Login.html' and _UserLoggedIn();
+
# If it's a noauth file, don't ask for auth.
SendSessionCookie();
$m->comp( { base_comp => $m->request_comp }, $m->fetch_next, %$ARGS );
@@ -386,9 +491,15 @@ sub AttemptExternalAuth {
# we failed to successfully create the user. abort abort abort.
delete $HTML::Mason::Commands::session{'CurrentUser'};
- $m->comp( '/Elements/Login', %$ARGS, Error => HTML::Mason::Commands::loc( 'Cannot create user: [_1]', $msg ) )
- if RT->Config->Get('WebFallbackToInternalAuth');;
- $m->abort();
+
+ if (RT->Config->Get('WebFallbackToInternalAuth')) {
+ my $key = LoginError(
+ HTML::Mason::Commands::loc('Cannot create user: [_1]', $msg)
+ );
+ TangentForLogin( results => $key );
+ } else {
+ $m->abort();
+ }
}
}
@@ -399,15 +510,19 @@ sub AttemptExternalAuth {
$user = $orig_user;
if ( RT->Config->Get('WebExternalOnly') ) {
- $m->comp( '/Elements/Login', %$ARGS, Error => HTML::Mason::Commands::loc('You are not an authorized user') );
- $m->abort();
+ my $key = LoginError(
+ HTML::Mason::Commands::loc('You are not an authorized user')
+ );
+ TangentForLogin( results => $key );
}
}
} elsif ( RT->Config->Get('WebFallbackToInternalAuth') ) {
unless ( defined $HTML::Mason::Commands::session{'CurrentUser'} ) {
# XXX unreachable due to prior defaulting in HandleRequest (check c34d108)
- $m->comp( '/Elements/Login', %$ARGS, Error => HTML::Mason::Commands::loc('You are not an authorized user') );
- $m->abort();
+ my $key = LoginError(
+ HTML::Mason::Commands::loc('You are not an authorized user')
+ );
+ TangentForLogin( results => $key );
}
} else {
@@ -420,7 +535,9 @@ sub AttemptExternalAuth {
}
sub AttemptPasswordAuthentication {
- my $ARGS = shift;
+ my $ARGS = shift;
+ return unless defined $ARGS->{user} && defined $ARGS->{pass};
+
my $user_obj = RT::CurrentUser->new();
$user_obj->Load( $ARGS->{user} );
@@ -428,15 +545,34 @@ sub AttemptPasswordAuthentication {
unless ( $user_obj->id && $user_obj->IsPassword( $ARGS->{pass} ) ) {
$RT::Logger->error("FAILED LOGIN for @{[$ARGS->{user}]} from $ENV{'REMOTE_ADDR'}");
- $m->comp( '/Elements/Login', %$ARGS, Error => HTML::Mason::Commands::loc('Your username or password is incorrect'), );
$m->callback( %$ARGS, CallbackName => 'FailedLogin', CallbackPage => '/autohandler' );
- $m->abort;
+ return (0, HTML::Mason::Commands::loc('Your username or password is incorrect'));
}
+ else {
+ $RT::Logger->info("Successful login for @{[$ARGS->{user}]} from $ENV{'REMOTE_ADDR'}");
+
+ # It's important to nab the next page from the session before we blow
+ # the session away
+ my $next = $HTML::Mason::Commands::session{'NextPage'}->{$ARGS->{'next'} || ''};
- $RT::Logger->info("Successful login for @{[$ARGS->{user}]} from $ENV{'REMOTE_ADDR'}");
- InstantiateNewSession();
- $HTML::Mason::Commands::session{'CurrentUser'} = $user_obj;
- $m->callback( %$ARGS, CallbackName => 'SuccessfulLogin', CallbackPage => '/autohandler' );
+ InstantiateNewSession();
+ $HTML::Mason::Commands::session{'CurrentUser'} = $user_obj;
+ SendSessionCookie();
+
+ $m->callback( %$ARGS, CallbackName => 'SuccessfulLogin', CallbackPage => '/autohandler' );
+
+ # Really the only time we don't want to redirect here is if we were
+ # passed user and pass as query params in the URL.
+ if ($next) {
+ Redirect($next);
+ }
+ elsif ($ARGS->{'next'}) {
+ # Invalid hash, but still wants to go somewhere, take them to /
+ Redirect(RT->Config->Get('WebURL'));
+ }
+
+ return (1, HTML::Mason::Commands::loc('Logged in'));
+ }
}
=head2 LoadSessionFromCookie
@@ -503,6 +639,10 @@ sub Redirect {
untie $HTML::Mason::Commands::session;
my $uri = URI->new($redir_to);
my $server_uri = URI->new( RT->Config->Get('WebURL') );
+
+ # Make relative URIs absolute from the server host and scheme
+ $uri->scheme($server_uri->scheme) if not defined $uri->scheme;
+ $uri->host($server_uri->host) if not defined $uri->host;
# If the user is coming in via a non-canonical
# hostname, don't redirect them to the canonical host,
diff --git a/share/html/Elements/ListActions b/share/html/Elements/ListActions
index de7584b..1e76bfe 100755
--- a/share/html/Elements/ListActions
+++ b/share/html/Elements/ListActions
@@ -46,7 +46,7 @@
%#
%# END BPS TAGGED BLOCK }}}
<div class="results">
-<&| /Widgets/TitleBox, title => loc('Results') &>
+<&| /Widgets/TitleBox, title => loc('Results'), %{$titlebox || {}} &>
<ul class="action-results">
% foreach my $action (@actions) {
<li><%$action%></li>
@@ -90,5 +90,6 @@ return unless @actions;
</%init>
<%ARGS>
+$titlebox => {}
@actions => undef
</%ARGS>
diff --git a/share/html/Elements/Login b/share/html/Elements/Login
index 00a8ab8..a7820c3 100755
--- a/share/html/Elements/Login
+++ b/share/html/Elements/Login
@@ -45,42 +45,6 @@
%# those contributions and any derivatives thereof.
%#
%# END BPS TAGGED BLOCK }}}
-<%INIT>
-if ($m->request_comp->path =~ '^/REST/\d+\.\d+/') {
- $r->content_type("text/plain");
- $m->error_format("text");
- $m->out("RT/$RT::VERSION 401 Credentials required\n");
- $m->out("\n$Error\n") if $Error;
- $m->abort;
-}
-
-my $req_uri;
-
-if (UNIVERSAL::can($r, 'uri') and $r->uri =~ m{.*/(.*)}) {
- $req_uri = $1;
-}
-
-my $form_action = defined $goto ? $goto
- : defined $req_uri ? $req_uri
- : RT->Config->Get('WebPath')
- ;
-
-# sanitize $form_action
-my $uri = URI->new($form_action);
-
-# You get undef scheme with a relative uri like "/Search/Build.html"
-unless (!defined($uri->scheme) || $uri->scheme eq 'http' || $uri->scheme eq 'https') {
- $form_action = RT->Config->Get('WebPath');
-}
-
-# Make sure we're logging in to the same domain
-# You can get an undef authority with a relative uri like "index.html"
-my $uri_base_url = URI->new(RT->Config->Get('WebBaseURL'));
-unless (!defined($uri->authority) || $uri->authority eq $uri_base_url->authority) {
- $form_action = RT->Config->Get('WebPath');
-}
-</%INIT>
-
% $m->callback( %ARGS, CallbackName => 'Header' );
<& /Elements/Header, Title => loc('Login'), Focus => 'user' &>
@@ -89,11 +53,12 @@ unless (!defined($uri->authority) || $uri->authority eq $uri_base_url->authority
</div>
<div id="body" class="login-body">
-% if ($Error) {
-<&| "/Widgets/TitleBox", title => loc('Error'), hideable => 0, class => 'error' &>
-<% $Error %>
-</&>
-% }
+
+<& /Elements/ListActions,
+ title => loc('Error'),
+ titlebox => { class => 'error', hideable => 0 },
+ actions => $actions
+&>
% $m->callback( %ARGS, CallbackName => 'BeforeForm' );
@@ -101,7 +66,7 @@ unless (!defined($uri->authority) || $uri->authority eq $uri_base_url->authority
<&| /Widgets/TitleBox, title => loc('Login'), titleright => $RT::VERSION, hideable => 0 &>
% unless (RT->Config->Get('WebExternalAuth') and !RT->Config->Get('WebFallbackToInternalAuth')) {
-<form id="login" name="login" method="post" action="<% $form_action %>">
+<form id="login" name="login" method="post" action="<% RT->Config->Get('WebPath') %>/NoAuth/Login.html">
<div class="input-row">
<span class="label"><&|/l&>Username</&>:</span>
@@ -113,6 +78,8 @@ unless (!defined($uri->authority) || $uri->authority eq $uri_base_url->authority
<span class="input"><input type="password" name="pass" autocomplete="off" /></span>
</div>
+<input type="hidden" name="next" value="<% $next %>" />
+
<div class="button-row">
<span class="input"><input type="submit" class="button" value="<&|/l&>Login</&>" /></span>
</div>
@@ -120,25 +87,6 @@ unless (!defined($uri->authority) || $uri->authority eq $uri_base_url->authority
%# Give callbacks a chance to add more control elements
% $m->callback( %ARGS );
-% # From mason 1.0.1 forward, this doesn't work. in fact, it breaks things.
-% # But on Mason 1.15 it's fixed again, so we still use it.
-% # The code below iterates through everything in the passed in arguments
-% # Preserving all the old parameters
-% # This would be easier, except mason is 'smart' and calls multiple values
-% # arrays rather than multiple hash keys
-% my $key; my $val;
-% foreach $key (keys %ARGS) {
-% if (($key ne 'user') and ($key ne 'pass')) {
-% if (ref($ARGS{$key}) =~ /ARRAY/) {
-% foreach $val (@{$ARGS{$key}}) {
-<input type="hidden" class="hidden" name="<%$key %>" value="<% $val %>" />
-% }
-% }
-% else {
-<input type="hidden" class="hidden" name="<% $key %>" value="<% $ARGS{$key} %>" />
-% }
-% }
-% }
</form>
% }
</&>
@@ -147,8 +95,7 @@ unless (!defined($uri->authority) || $uri->authority eq $uri_base_url->authority
</div><!-- #login-body -->
<& /Elements/Footer, Menu => 0 &>
<%ARGS>
+$next => ''
$user => ""
-$pass => undef
-$goto => undef
-$Error => undef
+$actions => undef
</%ARGS>
diff --git a/share/html/Elements/ListActions b/share/html/NoAuth/Login.html
similarity index 66%
copy from share/html/Elements/ListActions
copy to share/html/NoAuth/Login.html
index de7584b..6a3084a 100755
--- a/share/html/Elements/ListActions
+++ b/share/html/NoAuth/Login.html
@@ -45,50 +45,8 @@
%# those contributions and any derivatives thereof.
%#
%# END BPS TAGGED BLOCK }}}
-<div class="results">
-<&| /Widgets/TitleBox, title => loc('Results') &>
- <ul class="action-results">
-% foreach my $action (@actions) {
- <li><%$action%></li>
-% }
- </ul>
-</&>
-</div>
<%init>
-
-# backward compatibility, don't use array in new code, but use keyed hash
-if ( ref( $session{'Actions'} ) eq 'ARRAY' ) {
- unshift @actions, @{ delete $session{'Actions'} };
-}
-
-if ( ref( $session{'Actions'}{''} ) eq 'ARRAY' ) {
- unshift @actions, @{ delete $session{'Actions'}{''} };
-}
-
-my $actions_pointer = $m->request_args->{'results'};
-
-if ($actions_pointer && ref( $session{'Actions'}->{$actions_pointer} ) eq 'ARRAY' ) {
- unshift @actions, @{ delete $session{'Actions'}->{$actions_pointer} };
-}
-
-# XXX: run callbacks per row really crazy idea
- at actions =
- grep $_,
- grep {
- my $skip;
- $m->callback(
- %ARGS,
- row => \$_,
- skip => \$skip,
- CallbackName => 'ModifyRow',
- );
- !$skip;
- }
- grep $_, @actions;
-
-return unless @actions;
-
+my ($good, $msg) = RT::Interface::Web::AttemptPasswordAuthentication(\%ARGS);
+$ARGS{'actions'} = [$msg] if not $good and $msg;
</%init>
-<%ARGS>
- at actions => undef
-</%ARGS>
+<& /Elements/Login, %ARGS &>
commit fbf7e7066d7d17df218c19cb4ef7e52871aef762
Author: Thomas Sibley <trs at bestpractical.com>
Date: Fri Oct 22 17:56:37 2010 -0400
Add more entropy to the next page hash and yes, we need to ++ the session
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 255f763..42270d1 100755
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -286,10 +286,10 @@ the hash value.
sub SetNextPage {
my $next = shift || IntuitNextPage();
- my $hash = Digest::MD5::md5_hex( $next . $$ );
+ my $hash = Digest::MD5::md5_hex($next . $$ . rand(1024));
$HTML::Mason::Commands::session{'NextPage'}->{$hash} = $next;
- $HTML::Mason::Commands::session{'i'}++; # Cargo culted, do we need this?
+ $HTML::Mason::Commands::session{'i'}++;
SendSessionCookie();
return $hash;
commit 87408ef895b625d44512392de68c76a5c35fdd26
Author: Thomas Sibley <trs at bestpractical.com>
Date: Fri Oct 22 17:57:04 2010 -0400
Refactor the "error and tangent" pattern to a method
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 42270d1..46d6133 100755
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -311,6 +311,18 @@ sub TangentForLogin {
Redirect($login);
}
+=head2 TangentForLoginWithError ERROR
+
+Localizes the passed error message, stashes it with L<LoginError> and then
+calls L<TangentForLogin> with the appropriate results key.
+
+=cut
+
+sub TangentForLoginWithError {
+ my $key = LoginError(HTML::Mason::Commands::loc(@_));
+ TangentForLogin( results => $key );
+}
+
=head2 IntuitNextPage
Attempt to figure out the path to which we should return the user after a
@@ -493,10 +505,7 @@ sub AttemptExternalAuth {
delete $HTML::Mason::Commands::session{'CurrentUser'};
if (RT->Config->Get('WebFallbackToInternalAuth')) {
- my $key = LoginError(
- HTML::Mason::Commands::loc('Cannot create user: [_1]', $msg)
- );
- TangentForLogin( results => $key );
+ TangentForLoginWithError('Cannot create user: [_1]', $msg);
} else {
$m->abort();
}
@@ -510,19 +519,13 @@ sub AttemptExternalAuth {
$user = $orig_user;
if ( RT->Config->Get('WebExternalOnly') ) {
- my $key = LoginError(
- HTML::Mason::Commands::loc('You are not an authorized user')
- );
- TangentForLogin( results => $key );
+ TangentForLoginWithError('You are not an authorized user');
}
}
} elsif ( RT->Config->Get('WebFallbackToInternalAuth') ) {
unless ( defined $HTML::Mason::Commands::session{'CurrentUser'} ) {
# XXX unreachable due to prior defaulting in HandleRequest (check c34d108)
- my $key = LoginError(
- HTML::Mason::Commands::loc('You are not an authorized user')
- );
- TangentForLogin( results => $key );
+ TangentForLoginWithError('You are not an authorized user');
}
} else {
commit 23a912a292ad0c3d4bca4d3f9aae39c0100bc042
Author: Thomas Sibley <trs at bestpractical.com>
Date: Mon Oct 25 12:55:40 2010 -0400
Don't show login errors as a bulleted list
diff --git a/share/html/NoAuth/css/web2/login.css b/share/html/NoAuth/css/web2/login.css
index 7aaf6b1..5cee016 100644
--- a/share/html/NoAuth/css/web2/login.css
+++ b/share/html/NoAuth/css/web2/login.css
@@ -45,6 +45,10 @@
%# those contributions and any derivatives thereof.
%#
%# END BPS TAGGED BLOCK }}}
+.login-body .action-results {
+ list-style: none;
+}
+
#login-box hr {
display: none;
}
commit 6fa0a4e500a8be35a30fa9a7d9f7415ebb2d7405
Author: Thomas Sibley <trs at bestpractical.com>
Date: Mon Oct 25 15:31:37 2010 -0400
Fix passing login errors when on /
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 46d6133..3459e73 100755
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -212,7 +212,6 @@ sub HandleRequest {
unless ($authed) {
my $m = $HTML::Mason::Commands::m;
- my $results = $msg ? LoginError($msg) : undef;
# REST urls get a special 401 response
if ($m->request_comp->path =~ '^/REST/\d+\.\d+/') {
@@ -225,11 +224,11 @@ sub HandleRequest {
# Specially handle /index.html so that we get a nicer URL
elsif ( $m->request_comp->path eq '/index.html' ) {
my $next = SetNextPage(RT->Config->Get('WebURL'));
- $m->comp('/NoAuth/Login.html', next => $next, results => $results);
+ $m->comp('/NoAuth/Login.html', next => $next, actions => [$msg]);
$m->abort;
}
else {
- TangentForLogin(results => $results);
+ TangentForLogin(results => ($msg ? LoginError($msg) : undef));
}
}
}
commit e71c90f88c0f12936f3cae192ee4dc10bf11292c
Author: Thomas Sibley <trs at bestpractical.com>
Date: Mon Oct 25 15:32:39 2010 -0400
Do the right thing with //Ticket/Display.html when it's the REQUEST_URI
Collapse leading slashes so that we interpret it as a relative URL and
not a schema-less hostname + path. REQUEST_URI should never have a
schema-less URI.
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 3459e73..53dc1b4 100755
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -335,7 +335,13 @@ sub IntuitNextPage {
# This includes any query parameters. Redirect will take care of making
# it an absolute URL.
- $req_uri = $ENV{'REQUEST_URI'} if $ENV{'REQUEST_URI'};
+ if ($ENV{'REQUEST_URI'}) {
+ $req_uri = $ENV{'REQUEST_URI'};
+
+ # collapse multiple leading slashes so the first part doesn't look like
+ # a hostname of a schema-less URI
+ $req_uri =~ s{^/+}{/};
+ }
my $next = defined $req_uri ? $req_uri : RT->Config->Get('WebURL');
commit a813943bb23cb4692dabb634c53a2213f8a2300d
Author: Thomas Sibley <trs at bestpractical.com>
Date: Mon Oct 25 15:35:07 2010 -0400
If we're adding the hostname to a relative URL, also add the port
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 53dc1b4..c6c006d 100755
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -650,7 +650,10 @@ sub Redirect {
# Make relative URIs absolute from the server host and scheme
$uri->scheme($server_uri->scheme) if not defined $uri->scheme;
- $uri->host($server_uri->host) if not defined $uri->host;
+ if (not defined $uri->host) {
+ $uri->host($server_uri->host);
+ $uri->port($server_uri->port);
+ }
# If the user is coming in via a non-canonical
# hostname, don't redirect them to the canonical host,
commit a87820750251000aba859cc45f510db27b0c1049
Author: Thomas Sibley <trs at bestpractical.com>
Date: Mon Oct 25 15:35:38 2010 -0400
Tests for the new login flow
diff --git a/t/web/redirect-after-login.t b/t/web/redirect-after-login.t
new file mode 100644
index 0000000..d39bb58
--- /dev/null
+++ b/t/web/redirect-after-login.t
@@ -0,0 +1,243 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use RT::Test tests => 120;
+
+my ($baseurl, $agent) = RT::Test->started_ok;
+
+my $url = $agent->rt_base_url;
+diag $url if $ENV{TEST_VERBOSE};
+
+# test a login from the main page
+{
+ $agent->get_ok($url);
+ is($agent->{'status'}, 200, "Loaded a page");
+ is($agent->uri, $url, "didn't redirect to /NoAuth/Login.html for base URL");
+ ok($agent->current_form->find_input('user'));
+ ok($agent->current_form->find_input('pass'));
+ like($agent->current_form->action, qr{/NoAuth/Login\.html$}, "login form action is correct");
+
+ ok($agent->content =~ /username:/i);
+ $agent->field( 'user' => 'root' );
+ $agent->field( 'pass' => 'password' );
+
+ # the field isn't named, so we have to click link 0
+ $agent->click(0);
+ is( $agent->status, 200, "Fetched the page ok");
+ ok( $agent->content =~ /Logout/i, "Found a logout link");
+ is( $agent->uri, $url, "right URL" );
+ like( $agent->{redirected_uri}, qr{/NoAuth/Login\.html$}, "We redirected from login");
+ $agent->logout();
+}
+
+# test a bogus login from the main page
+{
+ $agent->get_ok($url);
+ is($agent->{'status'}, 200, "Loaded a page");
+ is($agent->uri, $url, "didn't redirect to /NoAuth/Login.html for base URL");
+ ok($agent->current_form->find_input('user'));
+ ok($agent->current_form->find_input('pass'));
+ like($agent->current_form->action, qr{/NoAuth/Login\.html$}, "login form action is correct");
+
+ ok($agent->content =~ /username:/i);
+ $agent->field( 'user' => 'root' );
+ $agent->field( 'pass' => 'wrongpass' );
+
+ # the field isn't named, so we have to click link 0
+ $agent->click(0);
+ is( $agent->status, 200, "Fetched the page ok");
+
+ ok( $agent->content =~ /Your username or password is incorrect/i, "Found the error message");
+ like( $agent->uri, qr{/NoAuth/Login\.html$}, "now on /NoAuth/Login.html" );
+ $agent->logout();
+
+ # Handle the warning after we're done with the page, since this leaves us
+ # with a completely different $mech
+ $agent->warning_like(qr/FAILED LOGIN for root/, "got failed login warning");
+}
+
+# test a login from a non-front page, both with a double leading slash and without
+for my $path (qw(Prefs/Other.html /Prefs/Other.html)) {
+ my $requested = $url.$path;
+ $agent->get_ok($requested);
+ is($agent->status, 200, "Loaded a page");
+ like($agent->uri, qr'/NoAuth/Login\.html\?next=[a-z0-9]{32}', "on login page, with next page hash");
+ is($agent->{redirected_uri}, $requested, "redirected from our requested page");
+
+ ok($agent->current_form->find_input('user'));
+ ok($agent->current_form->find_input('pass'));
+ ok($agent->current_form->find_input('next'));
+ like($agent->value('next'), qr/^[a-z0-9]{32}$/i, "next page argument is a hash");
+ like($agent->current_form->action, qr{/NoAuth/Login\.html$}, "login form action is correct");
+
+ ok($agent->content =~ /username:/i);
+ $agent->field( 'user' => 'root' );
+ $agent->field( 'pass' => 'password' );
+
+ # the field isn't named, so we have to click link 0
+ $agent->click(0);
+ is( $agent->status, 200, "Fetched the page ok");
+ ok( $agent->content =~ /Logout/i, "Found a logout link");
+
+ if ($path =~ m{/}) {
+ (my $collapsed = $path) =~ s{^/}{};
+ is( $agent->uri, $url.$collapsed, "right URL, with leading slashes in path collapsed" );
+ } else {
+ is( $agent->uri, $requested, "right URL" );
+ }
+
+ like( $agent->{redirected_uri}, qr{/NoAuth/Login\.html}, "We redirected from login");
+ $agent->logout();
+}
+
+# test a bogus login from a non-front page
+{
+ my $requested = $url.'Prefs/Other.html';
+ $agent->get_ok($requested);
+ is($agent->status, 200, "Loaded a page");
+ like($agent->uri, qr'/NoAuth/Login\.html\?next=[a-z0-9]{32}', "on login page, with next page hash");
+ is($agent->{redirected_uri}, $requested, "redirected from our requested page");
+
+ ok($agent->current_form->find_input('user'));
+ ok($agent->current_form->find_input('pass'));
+ ok($agent->current_form->find_input('next'));
+ like($agent->value('next'), qr/^[a-z0-9]{32}$/i, "next page argument is a hash");
+ like($agent->current_form->action, qr{/NoAuth/Login\.html$}, "login form action is correct");
+
+ ok($agent->content =~ /username:/i);
+ $agent->field( 'user' => 'root' );
+ $agent->field( 'pass' => 'wrongpass' );
+
+ # the field isn't named, so we have to click link 0
+ $agent->click(0);
+ is( $agent->status, 200, "Fetched the page ok");
+
+ ok( $agent->content =~ /Your username or password is incorrect/i, "Found the error message");
+ like( $agent->uri, qr{/NoAuth/Login\.html$}, "still on /NoAuth/Login.html" );
+
+ # try to login again
+ ok($agent->current_form->find_input('user'));
+ ok($agent->current_form->find_input('pass'));
+ ok($agent->current_form->find_input('next'));
+ like($agent->value('next'), qr/^[a-z0-9]{32}$/i, "next page argument is a hash");
+ like($agent->current_form->action, qr{/NoAuth/Login\.html$}, "login form action is correct");
+
+ ok($agent->content =~ /username:/i);
+ $agent->field( 'user' => 'root' );
+ $agent->field( 'pass' => 'password' );
+
+ # the field isn't named, so we have to click link 0
+ $agent->click(0);
+ is( $agent->status, 200, "Fetched the page ok");
+
+ # check out where we got to
+ is( $agent->uri, $requested, "right URL" );
+ like( $agent->{redirected_uri}, qr{/NoAuth/Login\.html}, "We redirected from login");
+ $agent->logout();
+
+ # Handle the warning after we're done with the page, since this leaves us
+ # with a completely different $mech
+ $agent->warning_like(qr/FAILED LOGIN for root/, "got failed login warning");
+}
+
+# test a login from the main page with query params
+{
+ my $requested = $url."?user=root;pass=password";
+ $agent->get_ok($requested);
+ is($agent->{'status'}, 200, "Loaded a page");
+ is($agent->uri, $requested, "didn't redirect to /NoAuth/Login.html for base URL");
+ ok($agent->content =~ /Logout/i, "Found a logout link - we're logged in");
+ $agent->logout();
+}
+
+# test a bogus login from the main page with query params
+{
+ my $requested = $url."?user=root;pass=wrongpass";
+ $agent->get_ok($requested);
+ is($agent->{'status'}, 200, "Loaded a page");
+ is($agent->uri, $requested, "didn't redirect to /NoAuth/Login.html for base URL");
+
+ ok($agent->content =~ /Your username or password is incorrect/i, "Found the error message");
+ ok($agent->current_form->find_input('user'));
+ ok($agent->current_form->find_input('pass'));
+ like($agent->current_form->action, qr{/NoAuth/Login\.html$}, "login form action is correct");
+
+ # Handle the warning after we're done with the page, since this leaves us
+ # with a completely different $mech
+ $agent->warning_like(qr/FAILED LOGIN for root/, "got failed login warning");
+}
+
+# test a bogus login from a non-front page with query params
+{
+ my $requested = $url."Prefs/Other.html?user=root;pass=wrongpass";
+ $agent->get_ok($requested);
+ is($agent->status, 200, "Loaded a page");
+ like($agent->uri, qr'/NoAuth/Login\.html\?next=[a-z0-9]{32}', "on login page, with next page hash");
+ is($agent->{redirected_uri}, $requested, "redirected from our requested page");
+ ok( $agent->content =~ /Your username or password is incorrect/i, "Found the error message");
+
+ ok($agent->current_form->find_input('user'));
+ ok($agent->current_form->find_input('pass'));
+ ok($agent->current_form->find_input('next'));
+ like($agent->value('next'), qr/^[a-z0-9]{32}$/i, "next page argument is a hash");
+ like($agent->current_form->action, qr{/NoAuth/Login\.html$}, "login form action is correct");
+
+ # Try to login again
+ ok($agent->content =~ /username:/i);
+ $agent->field( 'user' => 'root' );
+ $agent->field( 'pass' => 'password' );
+
+ # the field isn't named, so we have to click link 0
+ $agent->click(0);
+ is( $agent->status, 200, "Fetched the page ok");
+
+ # check out where we got to
+ is( $agent->uri, $requested, "right URL" );
+ like( $agent->{redirected_uri}, qr{/NoAuth/Login\.html}, "We redirected from login");
+ $agent->logout();
+
+ # Handle the warning after we're done with the page, since this leaves us
+ # with a completely different $mech
+ $agent->warning_like(qr/FAILED LOGIN for root/, "got failed login warning");
+}
+
+# test REST login response
+{
+ my $requested = $url."REST/1.0/?user=root;pass=password";
+ $agent->get($requested);
+ is($agent->status, 200, "Loaded a page");
+ is($agent->uri, $requested, "didn't redirect to /NoAuth/Login.html for REST");
+ $agent->get_ok($url);
+ $agent->logout();
+}
+
+# test REST login response for wrong pass
+{
+ my $requested = $url."REST/1.0/?user=root;pass=passwrong";
+ $agent->get_ok($requested);
+ is($agent->status, 200, "Loaded a page");
+ is($agent->uri, $requested, "didn't redirect to /NoAuth/Login.html for REST");
+ like($agent->content, qr/401 Credentials required/i, "got error status");
+ like($agent->content, qr/Your username or password is incorrect/, "got error message");
+
+ # Handle the warning after we're done with the page, since this leaves us
+ # with a completely different $mech
+ $agent->warning_like(qr/FAILED LOGIN for root/, "got failed login warning");
+}
+
+# test REST login response for no creds
+{
+ my $requested = $url."REST/1.0/";
+ $agent->get_ok($requested);
+ is($agent->status, 200, "Loaded a page");
+ is($agent->uri, $requested, "didn't redirect to /NoAuth/Login.html for REST");
+ like($agent->content, qr/401 Credentials required/i, "got error status");
+ unlike($agent->content, qr/Your username or password is incorrect/, "didn't get any error message");
+}
+
+# XXX TODO: we should also be testing WebExternalAuth here, but we don't have
+# the framework for dealing with that
+
+1;
commit d37f3e93f799824e84aa146dba44e5669525cb01
Author: Thomas Sibley <trs at bestpractical.com>
Date: Mon Oct 25 15:38:57 2010 -0400
Delete next page hashes from the session as we use them
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index c6c006d..4c74b6a 100755
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -561,7 +561,7 @@ sub AttemptPasswordAuthentication {
# It's important to nab the next page from the session before we blow
# the session away
- my $next = $HTML::Mason::Commands::session{'NextPage'}->{$ARGS->{'next'} || ''};
+ my $next = delete $HTML::Mason::Commands::session{'NextPage'}->{$ARGS->{'next'} || ''};
InstantiateNewSession();
$HTML::Mason::Commands::session{'CurrentUser'} = $user_obj;
commit 057552287159e801535e59b8fbd5bd98d1322069
Merge: 432df1d d37f3e9
Author: Thomas Sibley <trs at bestpractical.com>
Date: Wed Oct 27 10:42:35 2010 -0400
Merge branch 'redirect-after-login' into 3.8-trunk
-----------------------------------------------------------------------
More information about the Rt-commit
mailing list