X-Git-Url: http://git.freeside.biz/gitweb/?a=blobdiff_plain;f=rt%2Flib%2FRT%2FInterface%2FWeb.pm;h=3d1cd992db9c260905d84b6b9289e5fc2a4b3dbb;hb=a018b1a8d89157705bcb929425c03665d6842597;hp=aafca1a75f0861525882106ad29c3ea65690ba0d;hpb=cbb4c260c40779ba84c794dd68147c54f3de2f52;p=freeside.git diff --git a/rt/lib/RT/Interface/Web.pm b/rt/lib/RT/Interface/Web.pm index aafca1a75..3d1cd992d 100644 --- a/rt/lib/RT/Interface/Web.pm +++ b/rt/lib/RT/Interface/Web.pm @@ -2,7 +2,7 @@ # # COPYRIGHT: # -# This software is Copyright (c) 1996-2011 Best Practical Solutions, LLC +# This software is Copyright (c) 1996-2013 Best Practical Solutions, LLC # # # (Except where explicitly superseded by other copyright notices) @@ -244,12 +244,12 @@ 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')); + my $next = SetNextPage($ARGS); $m->comp('/NoAuth/Login.html', next => $next, actions => [$msg]); $m->abort; } else { - TangentForLogin(results => ($msg ? LoginError($msg) : undef)); + TangentForLogin($ARGS, results => ($msg ? LoginError($msg) : undef)); } } } @@ -298,7 +298,7 @@ sub LoginError { return $key; } -=head2 SetNextPage [PATH] +=head2 SetNextPage ARGSRef [PATH] Intuits and stashes the next page in the sesssion hash. If PATH is specified, uses that instead of the value of L. Returns @@ -307,24 +307,68 @@ the hash value. =cut sub SetNextPage { - my $next = shift || IntuitNextPage(); + my $ARGS = shift; + my $next = $_[0] ? $_[0] : IntuitNextPage(); my $hash = Digest::MD5::md5_hex($next . $$ . rand(1024)); + my $page = { url => $next }; + + # If an explicit URL was passed and we didn't IntuitNextPage, then + # IsPossibleCSRF below is almost certainly unrelated to the actual + # destination. Currently explicit next pages aren't used in RT, but the + # API is available. + if (not $_[0] and RT->Config->Get("RestrictReferrer")) { + # This isn't really CSRF, but the CSRF heuristics are useful for catching + # requests which may have unintended side-effects. + my ($is_csrf, $msg, @loc) = IsPossibleCSRF($ARGS); + if ($is_csrf) { + RT->Logger->notice( + "Marking original destination as having side-effects before redirecting for login.\n" + ."Request: $next\n" + ."Reason: " . HTML::Mason::Commands::loc($msg, @loc) + ); + $page->{'HasSideEffects'} = [$msg, @loc]; + } + } - $HTML::Mason::Commands::session{'NextPage'}->{$hash} = $next; + $HTML::Mason::Commands::session{'NextPage'}->{$hash} = $page; $HTML::Mason::Commands::session{'i'}++; return $hash; } +=head2 FetchNextPage HASHKEY + +Returns the stashed next page hashref for the given hash. + +=cut + +sub FetchNextPage { + my $hash = shift || ""; + return $HTML::Mason::Commands::session{'NextPage'}->{$hash}; +} + +=head2 RemoveNextPage HASHKEY + +Removes the stashed next page for the given hash and returns it. -=head2 TangentForLogin [HASH] +=cut + +sub RemoveNextPage { + my $hash = shift || ""; + return delete $HTML::Mason::Commands::session{'NextPage'}->{$hash}; +} + +=head2 TangentForLogin ARGSRef [HASH] Redirects to C, setting the value of L as -the next page. Optionally takes a hash which is dumped into query params. +the next page. Takes a hashref of request %ARGS as the first parameter. +Optionally takes all other parameters as a hash which is dumped into query +params. =cut sub TangentForLogin { - my $hash = SetNextPage(); + my $ARGS = shift; + my $hash = SetNextPage($ARGS); my %query = (@_, next => $hash); my $login = RT->Config->Get('WebURL') . 'NoAuth/Login.html?'; $login .= $HTML::Mason::Commands::m->comp('/Elements/QueryString', %query); @@ -339,8 +383,9 @@ calls L with the appropriate results key. =cut sub TangentForLoginWithError { - my $key = LoginError(HTML::Mason::Commands::loc(@_)); - TangentForLogin( results => $key ); + my $ARGS = shift; + my $key = LoginError(HTML::Mason::Commands::loc(@_)); + TangentForLogin( $ARGS, results => $key ); } =head2 IntuitNextPage @@ -452,6 +497,7 @@ sub MaybeRejectPrivateComponentRequest { / # leading slash ( Elements | _elements | # mobile UI + Callbacks | Widgets | autohandler | # requesting this directly is suspicious l (_unsafe)? ) # loc component @@ -529,6 +575,8 @@ sub AttemptExternalAuth { $user =~ s/^\Q$NodeName\E\\//i; } + my $next = RemoveNextPage($ARGS->{'next'}); + $next = $next->{'url'} if ref $next; InstantiateNewSession() unless _UserLoggedIn; $HTML::Mason::Commands::session{'CurrentUser'} = RT::CurrentUser->new(); $HTML::Mason::Commands::session{'CurrentUser'}->$load_method($user); @@ -567,7 +615,7 @@ sub AttemptExternalAuth { delete $HTML::Mason::Commands::session{'CurrentUser'}; if (RT->Config->Get('WebFallbackToInternalAuth')) { - TangentForLoginWithError('Cannot create user: [_1]', $msg); + TangentForLoginWithError($ARGS, 'Cannot create user: [_1]', $msg); } else { $m->abort(); } @@ -576,18 +624,27 @@ sub AttemptExternalAuth { if ( _UserLoggedIn() ) { $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 + # REMOTE_USER set, instead of forcing a "permission + # denied" message. Honor the $next. + Redirect($next) if $next; + # Unlike AttemptPasswordAuthentication below, we do not + # force a redirect to / if $next is not set -- otherwise, + # straight-up external auth would always redirect to / + # when you first hit it. } else { delete $HTML::Mason::Commands::session{'CurrentUser'}; $user = $orig_user; - if ( RT->Config->Get('WebExternalOnly') ) { - TangentForLoginWithError('You are not an authorized user'); + unless ( RT->Config->Get('WebFallbackToInternalAuth') ) { + TangentForLoginWithError($ARGS, '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) - TangentForLoginWithError('You are not an authorized user'); + TangentForLoginWithError($ARGS, 'You are not an authorized user'); } } else { @@ -618,7 +675,8 @@ sub AttemptPasswordAuthentication { # It's important to nab the next page from the session before we blow # the session away - my $next = delete $HTML::Mason::Commands::session{'NextPage'}->{$ARGS->{'next'} || ''}; + my $next = RemoveNextPage($ARGS->{'next'}); + $next = $next->{'url'} if ref $next; InstantiateNewSession(); $HTML::Mason::Commands::session{'CurrentUser'} = $user_obj; @@ -778,15 +836,15 @@ sub StaticFileHeaders { Takes C and returns a boolean indicating that the user-specified partial component path is safe. -Currently "safe" means that the path does not start with a dot (C<.>) and does -not contain a slash-dot C. +Currently "safe" means that the path does not start with a dot (C<.>), does +not contain a slash-dot C, and does not contain any nulls. =cut sub ComponentPathIsSafe { my $self = shift; my $path = shift; - return $path !~ m{(?:^|/)\.}; + return $path !~ m{(?:^|/)\.} and $path !~ m{\0}; } =head2 PathIsSafe @@ -1035,6 +1093,24 @@ our %is_whitelisted_component = ( # information for the search. Because it's a straight-up read, in # addition to embedding its own auth, it's fine. '/NoAuth/rss/dhandler' => 1, + + # IE doesn't send referer in window.open() + # besides, as a harmless calendar select page, it's fine + '/Helpers/CalPopup.html' => 1, + + # While both of these can be used for denial-of-service against RT + # (construct a very inefficient query and trick lots of users into + # running them against RT) it's incredibly useful to be able to link + # to a search result or bookmark a result page. + '/Search/Results.html' => 1, + '/Search/Simple.html' => 1, +); + +# Components which are blacklisted from automatic, argument-based whitelisting. +# These pages are not idempotent when called with just an id. +our %is_blacklisted_component = ( + # Takes only id and toggles bookmark state + '/Helpers/Toggle/TicketBookmark' => 1, ); sub IsCompCSRFWhitelisted { @@ -1059,6 +1135,10 @@ sub IsCompCSRFWhitelisted { delete $args{pass}; } + # Some pages aren't idempotent even with safe args like id; blacklist + # them from the automatic whitelisting below. + return 0 if $is_blacklisted_component{$comp}; + # Eliminate arguments that do not indicate an effectful request. # For example, "id" is acceptable because that is how RT retrieves a # record. @@ -1238,6 +1318,29 @@ sub MaybeShowInterstitialCSRFPage { # Calls abort, never gets here } +our @POTENTIAL_PAGE_ACTIONS = ( + qr'/Ticket/Create.html' => "create a ticket", # loc + qr'/Ticket/' => "update a ticket", # loc + qr'/Admin/' => "modify RT's configuration", # loc + qr'/Approval/' => "update an approval", # loc + qr'/Dashboards/' => "modify a dashboard", # loc + qr'/m/ticket/' => "update a ticket", # loc + qr'Prefs' => "modify your preferences", # loc + qr'/Search/' => "modify or access a search", # loc + qr'/SelfService/Create' => "create a ticket", # loc + qr'/SelfService/' => "update a ticket", # loc +); + +sub PotentialPageAction { + my $page = shift; + my @potentials = @POTENTIAL_PAGE_ACTIONS; + while (my ($pattern, $result) = splice @potentials, 0, 2) { + return HTML::Mason::Commands::loc($result) + if $page =~ $pattern; + } + return ""; +} + package HTML::Mason::Commands; use vars qw/$r $m %session/; @@ -1386,10 +1489,8 @@ sub CreateTicket { } } - foreach my $argument (qw(Encrypt Sign)) { - $MIMEObj->head->add( - "X-RT-$argument" => Encode::encode_utf8( $ARGS{$argument} ) - ) if defined $ARGS{$argument}; + for my $argument (qw(Encrypt Sign)) { + $MIMEObj->head->replace( "X-RT-$argument" => $ARGS{$argument} ? 1 : 0 ); } my %create_args = ( @@ -1578,14 +1679,33 @@ sub ProcessUpdateMessage { CurrentUser => $args{'TicketObj'}->CurrentUser, ); - # If, after stripping the signature, we have no message, move the - # UpdateTimeWorked into adjusted TimeWorked, so that a later - # ProcessBasics can deal -- then bail out. + my %txn_customfields; + + foreach my $key ( keys %{ $args{ARGSRef} } ) { + if ( $key =~ /^(?:Object-RT::Transaction--)?CustomField-(\d+)/ ) { + next if $key =~ /(TimeUnits|Magic)$/; + $txn_customfields{$key} = $args{ARGSRef}->{$key}; + } + } + + # If, after stripping the signature, we have no message, create a + # Touch transaction if necessary if ( not $args{ARGSRef}->{'UpdateAttachments'} and not length $args{ARGSRef}->{'UpdateContent'} ) { - if ( $args{ARGSRef}->{'UpdateTimeWorked'} ) { - $args{ARGSRef}->{TimeWorked} = $args{TicketObj}->TimeWorked + delete $args{ARGSRef}->{'UpdateTimeWorked'}; + #if ( $args{ARGSRef}->{'UpdateTimeWorked'} ) { + # $args{ARGSRef}->{TimeWorked} = $args{TicketObj}->TimeWorked + + # delete $args{ARGSRef}->{'UpdateTimeWorked'}; + # } + + my $timetaken = $args{ARGSRef}->{'UpdateTimeWorked'}; + if ( $timetaken or grep {length $_} values %txn_customfields ) { + my ( $Transaction, $Description, $Object ) = + $args{TicketObj}->Touch( + CustomFields => \%txn_customfields, + TimeTaken => $timetaken + ); + return $Description; } return; } @@ -1633,14 +1753,6 @@ sub ProcessUpdateMessage { my $bcc = $args{ARGSRef}->{'UpdateBcc'}; my $cc = $args{ARGSRef}->{'UpdateCc'}; - my %txn_customfields; - - foreach my $key ( keys %{ $args{ARGSRef} } ) { - if ( $key =~ /^(?:Object-RT::Transaction--)?CustomField-(\d+)/ ) { - $txn_customfields{$key} = $args{ARGSRef}->{$key}; - } - } - my %message_args = ( CcMessageTo => $cc, BccMessageTo => $bcc,