diff options
author | Ivan Kohler <ivan@freeside.biz> | 2012-06-07 16:55:45 -0700 |
---|---|---|
committer | Ivan Kohler <ivan@freeside.biz> | 2012-06-07 16:55:45 -0700 |
commit | c24d6e2242ae0e026684b8f95decf156aba6e75e (patch) | |
tree | 8597d00e2e6bf2cf400437b9344f43b1500da412 /rt/lib | |
parent | 6686c29104e555ea23446fe1db330664fa110bc0 (diff) |
rt 4.0.6
Diffstat (limited to 'rt/lib')
44 files changed, 875 insertions, 200 deletions
diff --git a/rt/lib/RT/ACL.pm b/rt/lib/RT/ACL.pm index d7c9ef2a8..49a7f1d64 100755 --- a/rt/lib/RT/ACL.pm +++ b/rt/lib/RT/ACL.pm @@ -182,6 +182,9 @@ sub LimitToPrincipal { ALIAS2 => $cgm, FIELD2 => 'GroupId' ); + $self->Limit( ALIAS => $cgm, + FIELD => 'Disabled', + VALUE => 0 ); $self->Limit( ALIAS => $cgm, FIELD => 'MemberId', OPERATOR => '=', diff --git a/rt/lib/RT/Action/CreateTickets.pm b/rt/lib/RT/Action/CreateTickets.pm index c26e2eb1d..31489c8ff 100644 --- a/rt/lib/RT/Action/CreateTickets.pm +++ b/rt/lib/RT/Action/CreateTickets.pm @@ -325,9 +325,19 @@ sub Prepare { } + my $active = 0; + if ( $self->TemplateObj->Type eq 'Perl' ) { + $active = 1; + } else { + RT->Logger->info(sprintf( + "Template #%d is type %s. You most likely want to use a Perl template instead.", + $self->TemplateObj->id, $self->TemplateObj->Type + )); + } + $self->Parse( Content => $self->TemplateObj->Content, - _ActiveContent => 1 + _ActiveContent => $active, ); return 1; @@ -1171,6 +1181,7 @@ sub UpdateCustomFields { my $cf = $1; my $CustomFieldObj = RT::CustomField->new($self->CurrentUser); + $CustomFieldObj->SetContextObject( $ticket ); $CustomFieldObj->LoadById($cf); my @values; diff --git a/rt/lib/RT/Action/SendEmail.pm b/rt/lib/RT/Action/SendEmail.pm index e2aa00bb6..4ae1a8b66 100755 --- a/rt/lib/RT/Action/SendEmail.pm +++ b/rt/lib/RT/Action/SendEmail.pm @@ -348,7 +348,7 @@ sub AddAttachments { $MIMEObj->head->delete('RT-Attach-Message'); - my $attachments = RT::Attachments->new(RT->SystemUser); + my $attachments = RT::Attachments->new( RT->SystemUser ); $attachments->Limit( FIELD => 'TransactionId', VALUE => $self->TransactionObj->Id @@ -408,6 +408,10 @@ sub AddAttachment { my $attach = shift; my $MIMEObj = shift || $self->TemplateObj->MIMEObj; + # $attach->TransactionObj may not always be $self->TransactionObj + return unless $attach->Id + and $attach->TransactionObj->CurrentUserCanSee; + # ->attach expects just the disposition type; extract it if we have the header my $disp = ($attach->GetHeader('Content-Disposition') || '') =~ /^\s*(inline|attachment)/i ? $1 : undef; @@ -471,8 +475,7 @@ sub AddTicket { my $self = shift; my $tid = shift; - # XXX: we need a current user here, but who is current user? - my $attachs = RT::Attachments->new(RT->SystemUser); + my $attachs = RT::Attachments->new( $self->TransactionObj->CreatorObj ); my $txn_alias = $attachs->TransactionAlias; $attachs->Limit( ALIAS => $txn_alias, FIELD => 'Type', VALUE => 'Create' ); $attachs->Limit( diff --git a/rt/lib/RT/Article.pm b/rt/lib/RT/Article.pm index 7310241ee..24b952ad4 100644 --- a/rt/lib/RT/Article.pm +++ b/rt/lib/RT/Article.pm @@ -543,6 +543,17 @@ sub CurrentUserHasRight { } +=head2 CurrentUserCanSee + +Returns true if the current user can see the article, using ShowArticle + +=cut + +sub CurrentUserCanSee { + my $self = shift; + return $self->CurrentUserHasRight('ShowArticle'); +} + # }}} # {{{ _Set diff --git a/rt/lib/RT/Attachments.pm b/rt/lib/RT/Attachments.pm index c640b206e..2bdbc244c 100755 --- a/rt/lib/RT/Attachments.pm +++ b/rt/lib/RT/Attachments.pm @@ -227,15 +227,12 @@ sub Next { my $Attachment = $self->SUPER::Next; return $Attachment unless $Attachment; - my $txn = $Attachment->TransactionObj; - if ( $txn->__Value('Type') eq 'Comment' ) { - return $Attachment if $txn->CurrentUserHasRight('ShowTicketComments'); - } elsif ( $txn->CurrentUserHasRight('ShowTicket') ) { + if ( $Attachment->TransactionObj->CurrentUserCanSee ) { return $Attachment; + } else { + # If the user doesn't have the right to show this ticket + return $self->Next; } - - # If the user doesn't have the right to show this ticket - return $self->Next; } diff --git a/rt/lib/RT/Class.pm b/rt/lib/RT/Class.pm index bb694ce9c..3906b9fed 100644 --- a/rt/lib/RT/Class.pm +++ b/rt/lib/RT/Class.pm @@ -275,6 +275,7 @@ sub ArticleCustomFields { my $cfs = RT::CustomFields->new( $self->CurrentUser ); if ( $self->CurrentUserHasRight('SeeClass') ) { + $cfs->SetContextObject( $self ); $cfs->LimitToGlobalOrObjectId( $self->Id ); $cfs->LimitToLookupType( RT::Article->CustomFieldLookupType ); $cfs->ApplySortOrder; diff --git a/rt/lib/RT/Config.pm b/rt/lib/RT/Config.pm index cc47df35a..c56d4c602 100644 --- a/rt/lib/RT/Config.pm +++ b/rt/lib/RT/Config.pm @@ -620,6 +620,7 @@ our %META = ( } } }, + ReferrerWhitelist => { Type => 'ARRAY' }, ResolveDefaultUpdateType => { PostLoadCheck => sub { my $self = shift; diff --git a/rt/lib/RT/CustomField.pm b/rt/lib/RT/CustomField.pm index 095caa52f..263bde877 100644 --- a/rt/lib/RT/CustomField.pm +++ b/rt/lib/RT/CustomField.pm @@ -474,10 +474,12 @@ sub LoadByName { } # if we're looking for a queue by name, make it a number - if ( defined $args{'Queue'} && $args{'Queue'} =~ /\D/ ) { + if ( defined $args{'Queue'} && ($args{'Queue'} =~ /\D/ || !$self->ContextObject) ) { my $QueueObj = RT::Queue->new( $self->CurrentUser ); $QueueObj->Load( $args{'Queue'} ); $args{'Queue'} = $QueueObj->Id; + $self->SetContextObject( $QueueObj ) + unless $self->ContextObject; } # XXX - really naive implementation. Slow. - not really. still just one query @@ -535,6 +537,8 @@ sub Values { # if the user has no rights, return an empty object if ( $self->id && $self->CurrentUserHasRight( 'SeeCustomField') ) { $cf_values->LimitToCustomField( $self->Id ); + } else { + $cf_values->Limit( FIELD => 'id', VALUE => 0, SUBCLAUSE => 'acl' ); } return ($cf_values); } @@ -890,7 +894,77 @@ sub ContextObject { my $self = shift; return $self->{'context_object'}; } - + +sub ValidContextType { + my $self = shift; + my $class = shift; + + my %valid; + $valid{$_}++ for split '-', $self->LookupType; + delete $valid{'RT::Transaction'}; + + return $valid{$class}; +} + +=head2 LoadContextObject + +Takes an Id for a Context Object and loads the right kind of RT::Object +for this particular Custom Field (based on the LookupType) and returns it. +This is a good way to ensure you don't try to use a Queue as a Context +Object on a User Custom Field. + +=cut + +sub LoadContextObject { + my $self = shift; + my $type = shift; + my $contextid = shift; + + unless ( $self->ValidContextType($type) ) { + RT->Logger->debug("Invalid ContextType $type for Custom Field ".$self->Id); + return; + } + + my $context_object = $type->new( $self->CurrentUser ); + my ($id, $msg) = $context_object->LoadById( $contextid ); + unless ( $id ) { + RT->Logger->debug("Invalid ContextObject id: $msg"); + return; + } + return $context_object; +} + +=head2 ValidateContextObject + +Ensure that a given ContextObject applies to this Custom Field. +For custom fields that are assigned to Queues or to Classes, this checks that the Custom +Field is actually applied to that objects. For Global Custom Fields, it returns true +as long as the Object is of the right type, because you may be using +your permissions on a given Queue of Class to see a Global CF. +For CFs that are only applied Globally, you don't need a ContextObject. + +=cut + +sub ValidateContextObject { + my $self = shift; + my $object = shift; + + return 1 if $self->IsApplied(0); + + # global only custom fields don't have objects + # that should be used as context objects. + return if $self->ApplyGlobally; + + # Otherwise, make sure we weren't passed a user object that we're + # supposed to treat as a queue. + return unless $self->ValidContextType(ref $object); + + # Check that it is applied correctly + my ($applied_to) = grep {ref($_) eq $self->RecordClassFromLookupType} ($object, $object->ACLEquivalenceObjects); + return unless $applied_to; + return $self->IsApplied($applied_to->id); +} + sub _Set { my $self = shift; @@ -1702,6 +1776,7 @@ sub SetBasedOn { unless defined $value and length $value; my $cf = RT::CustomField->new( $self->CurrentUser ); + $cf->SetContextObject( $self->ContextObject ); $cf->Load( ref $value ? $value->id : $value ); return (0, "Permission denied") @@ -1719,6 +1794,7 @@ sub BasedOnObj { my $self = shift; my $obj = RT::CustomField->new( $self->CurrentUser ); + $obj->SetContextObject( $self->ContextObject ); if ( $self->BasedOn ) { $obj->Load( $self->BasedOn ); } diff --git a/rt/lib/RT/Dashboard/Mailer.pm b/rt/lib/RT/Dashboard/Mailer.pm index 85589787e..40b53b111 100644 --- a/rt/lib/RT/Dashboard/Mailer.pm +++ b/rt/lib/RT/Dashboard/Mailer.pm @@ -252,7 +252,7 @@ SUMMARY $content = HTML::RewriteAttributes::Links->rewrite( $content, - RT->Config->Get('WebURL') . '/Dashboards/Render.html', + RT->Config->Get('WebURL') . 'Dashboards/Render.html', ); $self->EmailDashboard( @@ -447,6 +447,9 @@ sub BuildEmail { autohandler_name => '', # disable forced login and more data_dir => $data_dir, ); + $mason->set_escape( h => \&RT::Interface::Web::EscapeUTF8 ); + $mason->set_escape( u => \&RT::Interface::Web::EscapeURI ); + $mason->set_escape( j => \&RT::Interface::Web::EscapeJS ); } return $mason; } diff --git a/rt/lib/RT/Date.pm b/rt/lib/RT/Date.pm index 7a97fbe0e..442c7701d 100644 --- a/rt/lib/RT/Date.pm +++ b/rt/lib/RT/Date.pm @@ -604,6 +604,10 @@ sub Get my $self = shift; my %args = (Format => 'ISO', @_); my $formatter = $args{'Format'}; + unless ( $self->ValidFormatter($formatter) ) { + RT->Logger->warning("Invalid date formatter '$formatter', falling back to ISO"); + $formatter = 'ISO'; + } $formatter = 'ISO' unless $self->can($formatter); return $self->$formatter( %args ); } @@ -642,6 +646,20 @@ sub Formatters return @FORMATTERS; } +=head3 ValidFormatter FORMAT + +Returns a true value if C<FORMAT> is a known formatter. Otherwise returns +false. + +=cut + +sub ValidFormatter { + my $self = shift; + my $format = shift; + return (grep { $_ eq $format } $self->Formatters and $self->can($format)) + ? 1 : 0; +} + =head3 DefaultFormat =cut @@ -720,15 +738,19 @@ sub LocalizedDateTime my %args = ( Date => 1, Time => 1, Timezone => '', - DateFormat => 'date_format_full', - TimeFormat => 'time_format_medium', + DateFormat => '', + TimeFormat => '', AbbrDay => 1, AbbrMonth => 1, @_, ); - my $date_format = $args{'DateFormat'}; - my $time_format = $args{'TimeFormat'}; + # Require valid names for the format methods + my $date_format = $args{DateFormat} =~ /^\w+$/ + ? $args{DateFormat} : 'date_format_full'; + + my $time_format = $args{TimeFormat} =~ /^\w+$/ + ? $args{TimeFormat} : 'time_format_medium'; my $formatter = $self->LocaleObj; $date_format = $formatter->$date_format; diff --git a/rt/lib/RT/Generated.pm b/rt/lib/RT/Generated.pm index b02a413d2..2abcf3b6e 100644 --- a/rt/lib/RT/Generated.pm +++ b/rt/lib/RT/Generated.pm @@ -50,7 +50,7 @@ package RT; use warnings; use strict; -our $VERSION = '4.0.5'; +our $VERSION = '4.0.6'; diff --git a/rt/lib/RT/Graph/Tickets.pm b/rt/lib/RT/Graph/Tickets.pm index 112934ea3..b839824f9 100644 --- a/rt/lib/RT/Graph/Tickets.pm +++ b/rt/lib/RT/Graph/Tickets.pm @@ -100,7 +100,7 @@ EOT sub gv_escape($) { my $value = shift; - $value =~ s{(?=")}{\\}g; + $value =~ s{(?=["\\])}{\\}g; return $value; } @@ -278,6 +278,14 @@ sub TicketLinks { ShowLinkDescriptions => 0, @_ ); + + my %valid_links = map { $_ => 1 } + qw(Members MemberOf RefersTo ReferredToBy DependsOn DependedOnBy); + + # Validate our link types + $args{ShowLinks} = [ grep { $valid_links{$_} } @{$args{ShowLinks}} ]; + $args{LeadingLink} = 'Members' unless $valid_links{ $args{LeadingLink} }; + unless ( $args{'Graph'} ) { $args{'Graph'} = GraphViz->new( name => 'ticket_links_'. $args{'Ticket'}->id, diff --git a/rt/lib/RT/Group.pm b/rt/lib/RT/Group.pm index 779c02648..b367b2f96 100755 --- a/rt/lib/RT/Group.pm +++ b/rt/lib/RT/Group.pm @@ -1171,8 +1171,18 @@ sub CurrentUserHasRight { } +=head2 CurrentUserCanSee +Always returns 1; unfortunately, for historical reasons, users have +always been able to examine groups they have indirect access to, even if +they do not have SeeGroup explicitly. +=cut + +sub CurrentUserCanSee { + my $self = shift; + return 1; +} =head2 PrincipalObj diff --git a/rt/lib/RT/Groups.pm b/rt/lib/RT/Groups.pm index 46f1c232b..578109c4f 100755 --- a/rt/lib/RT/Groups.pm +++ b/rt/lib/RT/Groups.pm @@ -234,6 +234,8 @@ sub WithMember { ALIAS2 => $members, FIELD2 => 'GroupId'); $self->Limit(ALIAS => $members, FIELD => 'MemberId', OPERATOR => '=', VALUE => $args{'PrincipalId'}); + $self->Limit(ALIAS => $members, FIELD => 'Disabled', VALUE => 0) + if $args{'Recursively'}; return $members; } @@ -261,6 +263,12 @@ sub WithoutMember { VALUE => $args{'PrincipalId'}, ); $self->Limit( + LEFTJOIN => $members_alias, + ALIAS => $members_alias, + FIELD => 'Disabled', + VALUE => 0 + ) if $args{'Recursively'}; + $self->Limit( ALIAS => $members_alias, FIELD => 'MemberId', OPERATOR => 'IS', diff --git a/rt/lib/RT/Handle.pm b/rt/lib/RT/Handle.pm index bb19aa957..99d10e367 100644 --- a/rt/lib/RT/Handle.pm +++ b/rt/lib/RT/Handle.pm @@ -226,14 +226,12 @@ sub CheckIntegrity { my $self = shift; $self = new $self unless ref $self; - do { + unless ($RT::Handle and $RT::Handle->dbh) { local $@; unless ( eval { RT::ConnectToDatabase(); 1 } ) { return (0, 'no connection', "$@"); } - }; - - RT::InitLogging(); + } require RT::CurrentUser; my $test_user = RT::CurrentUser->new; @@ -748,6 +746,10 @@ sub InsertData { my $self = shift; my $datafile = shift; my $root_password = shift; + my %args = ( + disconnect_after => 1, + @_ + ); # Slurp in stuff to insert from the datafile. Possible things to go in here:- our (@Groups, @Users, @ACL, @Queues, @ScripActions, @ScripConditions, @@ -1071,8 +1073,14 @@ sub InsertData { $RT::Logger->debug("done."); } - my $db_type = RT->Config->Get('DatabaseType'); - $RT::Handle->Disconnect() unless $db_type eq 'SQLite'; + # XXX: This disconnect doesn't really belong here; it's a relict from when + # this method was extracted from rt-setup-database. However, too much + # depends on it to change without significant testing. At the very least, + # we can provide a way to skip the side-effect. + if ( $args{disconnect_after} ) { + my $db_type = RT->Config->Get('DatabaseType'); + $RT::Handle->Disconnect() unless $db_type eq 'SQLite'; + } $RT::Logger->debug("Done setting up database content."); diff --git a/rt/lib/RT/I18N.pm b/rt/lib/RT/I18N.pm index 971eaa1bd..cadf7cc7c 100644 --- a/rt/lib/RT/I18N.pm +++ b/rt/lib/RT/I18N.pm @@ -219,13 +219,6 @@ sub SetMIMEEntityToEncoding { my $head = $entity->head; - # convert at least MIME word encoded attachment filename - foreach my $attr (qw(content-type.name content-disposition.filename)) { - if ( my $name = $head->mime_attr($attr) and !$preserve_words ) { - $head->mime_attr( $attr => DecodeMIMEWordsToUTF8($name) ); - } - } - # If this is a textual entity, we'd need to preserve its original encoding $head->replace( "X-RT-Original-Encoding" => $charset ) if $head->mime_attr('content-type.charset') or IsTextualContentType($head->mime_type); @@ -292,7 +285,28 @@ sub DecodeMIMEWordsToEncoding { my $to_charset = _CanonicalizeCharset(shift); my $field = shift || ''; - my @list = $str =~ m/(.*?)=\?([^?]+)\?([QqBb])\?([^?]+)\?=([^=]*)/gcs; + # handle filename*=ISO-8859-1''%74%E9%73%74%2E%74%78%74, parameter value + # continuations, and similar syntax from RFC 2231 + if ($field =~ /^Content-(Type|Disposition)/i) { + # This concatenates continued parameters and normalizes encoded params + # to QB encoded-words which we handle below + $str = MIME::Field::ParamVal->parse($str)->stringify; + } + + # XXX TODO: use decode('MIME-Header', ...) and Encode::Alias to replace our + # custom MIME word decoding and charset canonicalization. We can't do this + # until we parse before decode, instead of the other way around. + my @list = $str =~ m/(.*?) # prefix + =\? # =? + ([^?]+?) # charset + (?:\*[^?]+)? # optional '*language' + \? # ? + ([QqBb]) # encoding + \? # ? + ([^?]+) # encoded string + \?= # ?= + ([^=]*) # trailing + /xgcs; if ( @list ) { # add everything that hasn't matched to the end of the latest @@ -350,27 +364,6 @@ sub DecodeMIMEWordsToEncoding { } } -# handle filename*=ISO-8859-1''%74%E9%73%74%2E%74%78%74, see also rfc 2231 - @list = $str =~ m/(.*?\*=)([^']*?)'([^']*?)'(\S+)(.*?)(?=(?:\*=|$))/gcs; - if (@list) { - $str = ''; - while (@list) { - my ( $prefix, $charset, $language, $enc_str, $trailing ) = - splice @list, 0, 5; - $prefix =~ s/\*=$/=/; # remove the * - $charset = _CanonicalizeCharset($charset); - $enc_str =~ s/%(\w{2})/chr hex $1/eg; - unless ( $charset eq $to_charset ) { - Encode::from_to( $enc_str, $charset, $to_charset ); - } - $enc_str = qq{"$enc_str"} - if $enc_str =~ /[,;]/ - and $enc_str !~ /^".*"$/ - and (!$field || $field =~ /^(?:To$|From$|B?Cc$|Content-)/i); - $str .= $prefix . $enc_str . $trailing; - } - } - # We might have \n without trailing whitespace, which will result in # invalid headers. $str =~ s/\n//g; diff --git a/rt/lib/RT/Installer.pm b/rt/lib/RT/Installer.pm index 3976adec6..d12abb678 100644 --- a/rt/lib/RT/Installer.pm +++ b/rt/lib/RT/Installer.pm @@ -252,7 +252,7 @@ sub CurrentValues { sub ConfigFile { require File::Spec; - return File::Spec->catfile( $RT::EtcPath, 'RT_SiteConfig.pm' ); + return $ENV{RT_SITE_CONFIG} || File::Spec->catfile( $RT::EtcPath, 'RT_SiteConfig.pm' ); } sub SaveConfig { diff --git a/rt/lib/RT/Interface/Email.pm b/rt/lib/RT/Interface/Email.pm index b9145d63a..02a1ec0c0 100755 --- a/rt/lib/RT/Interface/Email.pm +++ b/rt/lib/RT/Interface/Email.pm @@ -57,6 +57,7 @@ use RT::EmailParser; use File::Temp; use UNIVERSAL::require; use Mail::Mailer (); +use Text::ParseWords qw/shellwords/; BEGIN { use base 'Exporter'; @@ -404,11 +405,11 @@ sub SendEmail { if ( $mail_command eq 'sendmailpipe' ) { my $path = RT->Config->Get('SendmailPath'); - my $args = RT->Config->Get('SendmailArguments'); + my @args = shellwords(RT->Config->Get('SendmailArguments')); # SetOutgoingMailFrom and bounces conflict, since they both want -f if ( $args{'Bounce'} ) { - $args .= ' '. RT->Config->Get('SendmailBounceArguments'); + push @args, shellwords(RT->Config->Get('SendmailBounceArguments')); } elsif ( RT->Config->Get('SetOutgoingMailFrom') ) { my $OutgoingMailAddress; @@ -425,7 +426,7 @@ sub SendEmail { $OutgoingMailAddress ||= RT->Config->Get('OverrideOutgoingMailFrom')->{'Default'}; - $args .= " -f $OutgoingMailAddress" + push @args, "-f", $OutgoingMailAddress if $OutgoingMailAddress; } @@ -437,32 +438,36 @@ sub SendEmail { my $from = $TransactionObj->CreatorObj->EmailAddress; $from =~ s/@/=/g; $from =~ s/\s//g; - $args .= " -f $prefix$from\@$domain"; + push @args, "-f", "$prefix$from\@$domain"; } eval { # don't ignore CHLD signal to get proper exit code local $SIG{'CHLD'} = 'DEFAULT'; - open( my $mail, '|-', "$path $args >/dev/null" ) - or die "couldn't execute program: $!"; - # if something wrong with $mail->print we will get PIPE signal, handle it local $SIG{'PIPE'} = sub { die "program unexpectedly closed pipe" }; + + require IPC::Open2; + my ($mail, $stdout); + my $pid = IPC::Open2::open2( $stdout, $mail, $path, @args ) + or die "couldn't execute program: $!"; + $args{'Entity'}->print($mail); + close $mail or die "close pipe failed: $!"; - unless ( close $mail ) { - die "close pipe failed: $!" if $!; # system error + waitpid($pid, 0); + if ($?) { # sendmail exit statuses mostly errors with data not software # TODO: status parsing: core dump, exit on signal or EX_* - my $msg = "$msgid: `$path $args` exitted with code ". ($?>>8); + my $msg = "$msgid: `$path @args` exited with code ". ($?>>8); $msg = ", interrupted by signal ". ($?&127) if $?&127; $RT::Logger->error( $msg ); die $msg; } }; if ( $@ ) { - $RT::Logger->crit( "$msgid: Could not send mail with command `$path $args`: " . $@ ); + $RT::Logger->crit( "$msgid: Could not send mail with command `$path @args`: " . $@ ); if ( $TicketObj ) { _RecordSendEmailFailure( $TicketObj ); } @@ -743,16 +748,19 @@ sub SendForward { $mail->add_part( $entity ); my $from; - my $subject = ''; - $subject = $txn->Subject if $txn; - $subject ||= $ticket->Subject if $ticket; + unless (defined $mail->head->get('Subject')) { + my $subject = ''; + $subject = $txn->Subject if $txn; + $subject ||= $ticket->Subject if $ticket; + + unless ( RT->Config->Get('ForwardFromUser') ) { + # XXX: what if want to forward txn of other object than ticket? + $subject = AddSubjectTag( $subject, $ticket ); + } - unless ( RT->Config->Get('ForwardFromUser') ) { - # XXX: what if want to forward txn of other object than ticket? - $subject = AddSubjectTag( $subject, $ticket ); + $mail->head->set( Subject => EncodeToMIME( String => "Fwd: $subject" ) ); } - $mail->head->set( Subject => EncodeToMIME( String => "Fwd: $subject" ) ); $mail->head->set( From => EncodeToMIME( String => GetForwardFrom( Transaction => $txn, Ticket => $ticket ) diff --git a/rt/lib/RT/Interface/Web.pm b/rt/lib/RT/Interface/Web.pm index 7c9d57821..814a8293f 100644 --- a/rt/lib/RT/Interface/Web.pm +++ b/rt/lib/RT/Interface/Web.pm @@ -158,6 +158,25 @@ sub EncodeJSON { JSON::to_json(shift, { utf8 => 1, allow_nonref => 1 }); } +sub _encode_surrogates { + my $uni = $_[0] - 0x10000; + return ($uni / 0x400 + 0xD800, $uni % 0x400 + 0xDC00); +} + +sub EscapeJS { + my $ref = shift; + return unless defined $$ref; + + $$ref = "'" . join('', + map { + chr($_) =~ /[a-zA-Z0-9]/ ? chr($_) : + $_ <= 255 ? sprintf("\\x%02X", $_) : + $_ <= 65535 ? sprintf("\\u%04X", $_) : + sprintf("\\u%X\\u%X", _encode_surrogates($_)) + } unpack('U*', $$ref)) + . "'"; +} + =head2 WebCanonicalizeInfo(); Different web servers set different environmental varibles. This @@ -234,8 +253,10 @@ sub HandleRequest { ValidateWebConfig(); DecodeARGS($ARGS); + local $HTML::Mason::Commands::DECODED_ARGS = $ARGS; PreprocessTimeUpdates($ARGS); + InitializeMenu(); MaybeShowInstallModePage(); $HTML::Mason::Commands::m->comp( '/Elements/SetupSessionCookie', %$ARGS ); @@ -285,6 +306,8 @@ sub HandleRequest { } } + MaybeShowInterstitialCSRFPage($ARGS); + # now it applies not only to home page, but any dashboard that can be used as a workspace $HTML::Mason::Commands::session{'home_refresh_interval'} = $ARGS->{'HomeRefreshInterval'} if ( $ARGS->{'HomeRefreshInterval'} ); @@ -347,8 +370,6 @@ sub SetNextPage { $HTML::Mason::Commands::session{'NextPage'}->{$hash} = $next; $HTML::Mason::Commands::session{'i'}++; - - SendSessionCookie(); return $hash; } @@ -465,7 +486,6 @@ sub MaybeShowNoAuthPage { 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 ); $m->abort; } @@ -492,7 +512,7 @@ sub MaybeRejectPrivateComponentRequest { _elements | # mobile UI Widgets | autohandler | # requesting this directly is suspicious - l ) # loc component + l (_unsafe)? ) # loc component ( $ | / ) # trailing slash or end of path }xi && $path !~ m{ /RTx/Statistics/\w+/Elements/Chart }xi @@ -526,13 +546,13 @@ sub ShowRequestedPage { my $m = $HTML::Mason::Commands::m; + # Ensure that the cookie that we send is up-to-date, in case the + # session-id has been modified in any way + SendSessionCookie(); + # precache all system level rights for the current user $HTML::Mason::Commands::session{CurrentUser}->PrincipalObj->HasRights( Object => RT->System ); - InitializeMenu(); - - SendSessionCookie(); - # If the user isn't privileged, they can only see SelfService unless ( $HTML::Mason::Commands::session{'CurrentUser'}->Privileged ) { @@ -681,7 +701,6 @@ sub AttemptPasswordAuthentication { InstantiateNewSession(); $HTML::Mason::Commands::session{'CurrentUser'} = $user_obj; - SendSessionCookie(); $m->callback( %$ARGS, CallbackName => 'SuccessfulLogin', CallbackPage => '/autohandler' ); @@ -736,6 +755,7 @@ sub LoadSessionFromCookie { sub InstantiateNewSession { tied(%HTML::Mason::Commands::session)->delete if tied(%HTML::Mason::Commands::session); tie %HTML::Mason::Commands::session, 'RT::Interface::Web::Session', undef; + SendSessionCookie(); } sub SendSessionCookie { @@ -817,6 +837,10 @@ sub StaticFileHeaders { # make cache public $HTML::Mason::Commands::r->headers_out->{'Cache-Control'} = 'max-age=259200, public'; + # remove any cookie headers -- if it is cached publicly, it + # shouldn't include anyone's cookie! + delete $HTML::Mason::Commands::r->err_headers_out->{'Set-Cookie'}; + # Expire things in a month. $date->Set( Value => time + 30 * 24 * 60 * 60 ); $HTML::Mason::Commands::r->headers_out->{'Expires'} = $date->RFC2616; @@ -828,6 +852,22 @@ sub StaticFileHeaders { # $HTML::Mason::Commands::r->headers_out->{'Last-Modified'} = $date->RFC2616; } +=head2 ComponentPathIsSafe PATH + +Takes C<PATH> 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</.>. + +=cut + +sub ComponentPathIsSafe { + my $self = shift; + my $path = shift; + return $path !~ m{(?:^|/)\.}; +} + =head2 PathIsSafe Takes a C<< Path => path >> and returns a boolean indicating that @@ -1138,6 +1178,218 @@ sub ComponentRoots { return @roots; } +our %is_whitelisted_component = ( + # The RSS feed embeds an auth token in the path, but query + # 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, +); + +sub IsCompCSRFWhitelisted { + my $comp = shift; + my $ARGS = shift; + + return 1 if $is_whitelisted_component{$comp}; + + my %args = %{ $ARGS }; + + # If the user specifies a *correct* user and pass then they are + # golden. This acts on the presumption that external forms may + # hardcode a username and password -- if a malicious attacker knew + # both already, CSRF is the least of your problems. + my $AllowLoginCSRF = not RT->Config->Get('RestrictReferrerLogin'); + if ($AllowLoginCSRF and defined($args{user}) and defined($args{pass})) { + my $user_obj = RT::CurrentUser->new(); + $user_obj->Load($args{user}); + return 1 if $user_obj->id && $user_obj->IsPassword($args{pass}); + + delete $args{user}; + delete $args{pass}; + } + + # Eliminate arguments that do not indicate an effectful request. + # For example, "id" is acceptable because that is how RT retrieves a + # record. + delete $args{id}; + + # If they have a valid results= from MaybeRedirectForResults, that's + # also fine. + delete $args{results} if $args{results} + and $HTML::Mason::Commands::session{"Actions"}->{$args{results}}; + + # The homepage refresh, which uses the Refresh header, doesn't send + # a referer in most browsers; whitelist the one parameter it reloads + # with, HomeRefreshInterval, which is safe + delete $args{HomeRefreshInterval}; + + # If there are no arguments, then it's likely to be an idempotent + # request, which are not susceptible to CSRF + return 1 if !%args; + + return 0; +} + +sub IsRefererCSRFWhitelisted { + my $referer = _NormalizeHost(shift); + my $base_url = _NormalizeHost(RT->Config->Get('WebBaseURL')); + $base_url = $base_url->host_port; + + my $configs; + for my $config ( $base_url, RT->Config->Get('ReferrerWhitelist') ) { + push @$configs,$config; + return 1 if $referer->host_port eq $config; + } + + return (0,$referer,$configs); +} + +=head3 _NormalizeHost + +Takes a URI and creates a URI object that's been normalized +to handle common problems such as localhost vs 127.0.0.1 + +=cut + +sub _NormalizeHost { + + my $uri= URI->new(shift); + $uri->host('127.0.0.1') if $uri->host eq 'localhost'; + + return $uri; + +} + +sub IsPossibleCSRF { + my $ARGS = shift; + + # If first request on this session is to a REST endpoint, then + # whitelist the REST endpoints -- and explicitly deny non-REST + # endpoints. We do this because using a REST cookie in a browser + # would open the user to CSRF attacks to the REST endpoints. + my $path = $HTML::Mason::Commands::r->path_info; + $HTML::Mason::Commands::session{'REST'} = $path =~ m{^/+REST/\d+\.\d+(/|$)} + unless defined $HTML::Mason::Commands::session{'REST'}; + + if ($HTML::Mason::Commands::session{'REST'}) { + return 0 if $path =~ m{^/+REST/\d+\.\d+(/|$)}; + my $why = <<EOT; +This login session belongs to a REST client, and cannot be used to +access non-REST interfaces of RT for security reasons. +EOT + my $details = <<EOT; +Please log out and back in to obtain a session for normal browsing. If +you understand the security implications, disabling RT's CSRF protection +will remove this restriction. +EOT + chomp $details; + HTML::Mason::Commands::Abort( $why, Details => $details ); + } + + return 0 if IsCompCSRFWhitelisted( + $HTML::Mason::Commands::m->request_comp->path, + $ARGS + ); + + # if there is no Referer header then assume the worst + return (1, + "your browser did not supply a Referrer header", # loc + ) if !$ENV{HTTP_REFERER}; + + my ($whitelisted, $browser, $configs) = IsRefererCSRFWhitelisted($ENV{HTTP_REFERER}); + return 0 if $whitelisted; + + if ( @$configs > 1 ) { + return (1, + "the Referrer header supplied by your browser ([_1]) is not allowed by RT's configured hostname ([_2]) or whitelisted hosts ([_3])", # loc + $browser->host_port, + shift @$configs, + join(', ', @$configs) ); + } + + return (1, + "the Referrer header supplied by your browser ([_1]) is not allowed by RT's configured hostname ([_2])", # loc + $browser->host_port, + $configs->[0]); +} + +sub ExpandCSRFToken { + my $ARGS = shift; + + my $token = delete $ARGS->{CSRF_Token}; + return unless $token; + + my $data = $HTML::Mason::Commands::session{'CSRF'}{$token}; + return unless $data; + return unless $data->{path} eq $HTML::Mason::Commands::r->path_info; + + my $user = $HTML::Mason::Commands::session{'CurrentUser'}->UserObj; + return unless $user->ValidateAuthString( $data->{auth}, $token ); + + %{$ARGS} = %{$data->{args}}; + $HTML::Mason::Commands::DECODED_ARGS = $ARGS; + + # We explicitly stored file attachments with the request, but not in + # the session yet, as that would itself be an attack. Put them into + # the session now, so they'll be visible. + if ($data->{attach}) { + my $filename = $data->{attach}{filename}; + my $mime = $data->{attach}{mime}; + $HTML::Mason::Commands::session{'Attachments'}{$filename} + = $mime; + } + + return 1; +} + +sub StoreRequestToken { + my $ARGS = shift; + + my $token = Digest::MD5::md5_hex(time . {} . $$ . rand(1024)); + my $user = $HTML::Mason::Commands::session{'CurrentUser'}->UserObj; + my $data = { + auth => $user->GenerateAuthString( $token ), + path => $HTML::Mason::Commands::r->path_info, + args => $ARGS, + }; + if ($ARGS->{Attach}) { + my $attachment = HTML::Mason::Commands::MakeMIMEEntity( AttachmentFieldName => 'Attach' ); + my $file_path = delete $ARGS->{'Attach'}; + $data->{attach} = { + filename => Encode::decode_utf8("$file_path"), + mime => $attachment, + }; + } + + $HTML::Mason::Commands::session{'CSRF'}->{$token} = $data; + $HTML::Mason::Commands::session{'i'}++; + return $token; +} + +sub MaybeShowInterstitialCSRFPage { + my $ARGS = shift; + + return unless RT->Config->Get('RestrictReferrer'); + + # Deal with the form token provided by the interstitial, which lets + # browsers which never set referer headers still use RT, if + # painfully. This blows values into ARGS + return if ExpandCSRFToken($ARGS); + + my ($is_csrf, $msg, @loc) = IsPossibleCSRF($ARGS); + return if !$is_csrf; + + $RT::Logger->notice("Possible CSRF: ".RT::CurrentUser->new->loc($msg, @loc)); + + my $token = StoreRequestToken($ARGS); + $HTML::Mason::Commands::m->comp( + '/Elements/CSRF', + OriginalURL => RT->Config->Get('WebPath') . $HTML::Mason::Commands::r->path_info, + Reason => HTML::Mason::Commands::loc( $msg, @loc ), + Token => $token, + ); + # Calls abort, never gets here +} + package HTML::Mason::Commands; use vars qw/$r $m %session/; @@ -1418,6 +1670,7 @@ sub CreateTicket { my $cfid = $1; my $cf = RT::CustomField->new( $session{'CurrentUser'} ); + $cf->SetContextObject( $Queue ); $cf->Load($cfid); unless ( $cf->id ) { $RT::Logger->error( "Couldn't load custom field #" . $cfid ); @@ -2144,10 +2397,11 @@ sub ProcessTicketReminders { if ( $args->{'update-reminders'} ) { while ( my $reminder = $reminder_collection->Next ) { - if ( $reminder->Status ne 'resolved' && $args->{ 'Complete-Reminder-' . $reminder->id } ) { + my $resolve_status = $reminder->QueueObj->Lifecycle->ReminderStatusOnResolve; + if ( $reminder->Status ne $resolve_status && $args->{ 'Complete-Reminder-' . $reminder->id } ) { $Ticket->Reminders->Resolve($reminder); } - elsif ( $reminder->Status eq 'resolved' && !$args->{ 'Complete-Reminder-' . $reminder->id } ) { + elsif ( $reminder->Status eq $resolve_status && !$args->{ 'Complete-Reminder-' . $reminder->id } ) { $Ticket->Reminders->Open($reminder); } @@ -2239,6 +2493,7 @@ sub ProcessObjectCustomFieldUpdates { foreach my $cf ( keys %{ $custom_fields_to_mod{$class}{$id} } ) { my $CustomFieldObj = RT::CustomField->new( $session{'CurrentUser'} ); + $CustomFieldObj->SetContextObject($Object); $CustomFieldObj->LoadById($cf); unless ( $CustomFieldObj->id ) { $RT::Logger->warning("Couldn't load custom field #$cf"); @@ -2851,50 +3106,71 @@ sub ScrubHTML { =head2 _NewScrubber -Returns a new L<HTML::Scrubber> object. Override this if you insist on -letting more HTML through. +Returns a new L<HTML::Scrubber> object. + +If you need to be more lax about what HTML tags and attributes are allowed, +create C</opt/rt4/local/lib/RT/Interface/Web_Local.pm> with something like the +following: + + package HTML::Mason::Commands; + # Let tables through + push @SCRUBBER_ALLOWED_TAGS, qw(TABLE THEAD TBODY TFOOT TR TD TH); + 1; =cut +our @SCRUBBER_ALLOWED_TAGS = qw( + A B U P BR I HR BR SMALL EM FONT SPAN STRONG SUB SUP STRIKE H1 H2 H3 H4 H5 + H6 DIV UL OL LI DL DT DD PRE BLOCKQUOTE BDO +); + +our %SCRUBBER_ALLOWED_ATTRIBUTES = ( + # Match http, ftp and relative urls + # XXX: we also scrub format strings with this module then allow simple config options + href => qr{^(?:http:|ftp:|https:|/|__Web(?:Path|BaseURL|URL)__)}i, + face => 1, + size => 1, + target => 1, + style => qr{ + ^(?:\s* + (?:(?:background-)?color: \s* + (?:rgb\(\s* \d+, \s* \d+, \s* \d+ \s*\) | # rgb(d,d,d) + \#[a-f0-9]{3,6} | # #fff or #ffffff + [\w\-]+ # green, light-blue, etc. + ) | + text-align: \s* \w+ | + font-size: \s* [\w.\-]+ | + font-family: \s* [\w\s"',.\-]+ | + font-weight: \s* [\w\-]+ | + + # MS Office styles, which are probably fine. If we don't, then any + # associated styles in the same attribute get stripped. + mso-[\w\-]+?: \s* [\w\s"',.\-]+ + )\s* ;? \s*) + +$ # one or more of these allowed properties from here 'till sunset + }ix, + dir => qr/^(rtl|ltr)$/i, + lang => qr/^\w+(-\w+)?$/, +); + +our %SCRUBBER_RULES = (); + sub _NewScrubber { require HTML::Scrubber; my $scrubber = HTML::Scrubber->new(); $scrubber->default( 0, { - '*' => 0, - id => 1, - class => 1, - # Match http, ftp and relative urls - # XXX: we also scrub format strings with this module then allow simple config options - href => qr{^(?:http:|ftp:|https:|/|__Web(?:Path|BaseURL|URL)__)}i, - face => 1, - size => 1, - target => 1, - style => qr{ - ^(?:\s* - (?:(?:background-)?color: \s* - (?:rgb\(\s* \d+, \s* \d+, \s* \d+ \s*\) | # rgb(d,d,d) - \#[a-f0-9]{3,6} | # #fff or #ffffff - [\w\-]+ # green, light-blue, etc. - ) | - text-align: \s* \w+ | - font-size: \s* [\w.\-]+ | - font-family: \s* [\w\s"',.\-]+ | - font-weight: \s* [\w\-]+ | - - # MS Office styles, which are probably fine. If we don't, then any - # associated styles in the same attribute get stripped. - mso-[\w\-]+?: \s* [\w\s"',.\-]+ - )\s* ;? \s*) - +$ # one or more of these allowed properties from here 'till sunset - }ix, - } + %SCRUBBER_ALLOWED_ATTRIBUTES, + '*' => 0, # require attributes be explicitly allowed + }, ); $scrubber->deny(qw[*]); - $scrubber->allow( - qw[A B U P BR I HR BR SMALL EM FONT SPAN STRONG SUB SUP STRIKE H1 H2 H3 H4 H5 H6 DIV UL OL LI DL DT DD PRE BLOCKQUOTE] - ); + $scrubber->allow(@SCRUBBER_ALLOWED_TAGS); + $scrubber->rules(%SCRUBBER_RULES); + + # Scrubbing comments is vital since IE conditional comments can contain + # arbitrary HTML and we'd pass it right on through. $scrubber->comment(0); return $scrubber; diff --git a/rt/lib/RT/Interface/Web/Handler.pm b/rt/lib/RT/Interface/Web/Handler.pm index 69eee60f6..a740167c6 100644 --- a/rt/lib/RT/Interface/Web/Handler.pm +++ b/rt/lib/RT/Interface/Web/Handler.pm @@ -69,12 +69,12 @@ sub DefaultHandlerArgs { ( ], default_escape_flags => 'h', data_dir => "$RT::MasonDataDir", - allow_globals => [qw(%session)], + allow_globals => [qw(%session $DECODED_ARGS)], # Turn off static source if we're in developer mode. static_source => (RT->Config->Get('DevelMode') ? '0' : '1'), use_object_files => (RT->Config->Get('DevelMode') ? '0' : '1'), autoflush => 0, - error_format => (RT->Config->Get('DevelMode') ? 'html': 'brief'), + error_format => (RT->Config->Get('DevelMode') ? 'html': 'rt_error'), request_class => 'RT::Interface::Web::Request', named_component_subs => $INC{'Devel/Cover.pm'} ? 1 : 0, ) }; @@ -116,6 +116,7 @@ sub NewHandler { $handler->interp->set_escape( h => \&RT::Interface::Web::EscapeUTF8 ); $handler->interp->set_escape( u => \&RT::Interface::Web::EscapeURI ); + $handler->interp->set_escape( j => \&RT::Interface::Web::EscapeJS ); return($handler); } @@ -202,6 +203,13 @@ sub CleanupRequest { } +sub HTML::Mason::Exception::as_rt_error { + my ($self) = @_; + $RT::Logger->error( $self->full_message ); + return "An internal RT error has occurred. Your administrator can find more details in RT's log files."; +} + + # PSGI App use RT::Interface::Web::Handler; diff --git a/rt/lib/RT/Interface/Web/QueryBuilder/Tree.pm b/rt/lib/RT/Interface/Web/QueryBuilder/Tree.pm index e2ec1e58d..2cfc88998 100755 --- a/rt/lib/RT/Interface/Web/QueryBuilder/Tree.pm +++ b/rt/lib/RT/Interface/Web/QueryBuilder/Tree.pm @@ -92,8 +92,8 @@ sub TraversePrePost { =head2 GetReferencedQueues -Returns a hash reference with keys each queue name referenced in a clause in -the key (even if it's "Queue != 'Foo'"), and values all 1. +Returns a hash reference; each queue referenced with an '=' operation +will appear as a key whose value is 1. =cut @@ -110,10 +110,12 @@ sub GetReferencedQueues { return unless $node->isLeaf; my $clause = $node->getNodeValue(); + return unless $clause->{Key} eq 'Queue'; + return unless $clause->{Op} eq '='; - if ( $clause->{Key} eq 'Queue' ) { - $queues->{ $clause->{Value} } = 1; - }; + my $value = $clause->{Value}; + $value =~ s/\\(.)/$1/g if $value =~ s/^'(.*)'$/$1/; + $queues->{ $value } = 1; } ); @@ -275,7 +277,7 @@ sub ParseSQL { $value = "'$value'"; } - if ($key =~ s/(['\\])/\\$1/g or $key =~ /\s/) { + if ($key =~ s/(['\\])/\\$1/g or $key =~ /[^{}\w\.]/) { $key = "'$key'"; } diff --git a/rt/lib/RT/Lifecycle.pm b/rt/lib/RT/Lifecycle.pm index edb179569..056599edb 100644 --- a/rt/lib/RT/Lifecycle.pm +++ b/rt/lib/RT/Lifecycle.pm @@ -367,6 +367,28 @@ sub DefaultOnMerge { return $self->DefaultStatus('on_merge'); } +=head3 ReminderStatusOnOpen + +Returns the status that should be used when reminders are opened. + +=cut + +sub ReminderStatusOnOpen { + my $self = shift; + return $self->DefaultStatus('reminder_on_open') || 'open'; +} + +=head3 ReminderStatusOnResolve + +Returns the status that should be used when reminders are resolved. + +=cut + +sub ReminderStatusOnResolve { + my $self = shift; + return $self->DefaultStatus('reminder_on_resolve') || 'resolved'; +} + =head2 Transitions, rights, labels and actions. =head3 Transitions diff --git a/rt/lib/RT/Links.pm b/rt/lib/RT/Links.pm index 0d8ed2f11..ccc72d749 100644 --- a/rt/lib/RT/Links.pm +++ b/rt/lib/RT/Links.pm @@ -91,15 +91,17 @@ sub Limit { if ( ($args{'FIELD'} eq 'Target') or ($args{'FIELD'} eq 'LocalTarget') ) { - $self->OrderBy (ALIAS => 'main', - FIELD => 'Base', - ORDER => 'ASC'); + $self->OrderByCols( + { ALIAS => 'main', FIELD => 'LocalBase', ORDER => 'ASC' }, + { ALIAS => 'main', FIELD => 'Base', ORDER => 'ASC' }, + ); } elsif ( ($args{'FIELD'} eq 'Base') or ($args{'FIELD'} eq 'LocalBase') ) { - $self->OrderBy (ALIAS => 'main', - FIELD => 'Target', - ORDER => 'ASC'); + $self->OrderByCols( + { ALIAS => 'main', FIELD => 'LocalTarget', ORDER => 'ASC' }, + { ALIAS => 'main', FIELD => 'Target', ORDER => 'ASC' }, + ); } diff --git a/rt/lib/RT/ObjectCustomField.pm b/rt/lib/RT/ObjectCustomField.pm index 0b815aef3..61bc35532 100644 --- a/rt/lib/RT/ObjectCustomField.pm +++ b/rt/lib/RT/ObjectCustomField.pm @@ -137,7 +137,19 @@ Returns the CustomField Object which has the id returned by CustomField sub CustomFieldObj { my $self = shift; my $id = shift || $self->CustomField; + + # To find out the proper context object to load the CF with, we need + # data from the CF -- namely, the record class. Go find that as the + # system user first. + my $system_CF = RT::CustomField->new( RT->SystemUser ); + $system_CF->Load( $id ); + my $class = $system_CF->RecordClassFromLookupType; + + my $obj = $class->new( $self->CurrentUser ); + $obj->Load( $self->ObjectId ); + my $CF = RT::CustomField->new( $self->CurrentUser ); + $CF->SetContextObject( $obj ); $CF->Load( $id ); return $CF; } diff --git a/rt/lib/RT/ObjectCustomFieldValue.pm b/rt/lib/RT/ObjectCustomFieldValue.pm index 0fd9d735c..98714a048 100644 --- a/rt/lib/RT/ObjectCustomFieldValue.pm +++ b/rt/lib/RT/ObjectCustomFieldValue.pm @@ -251,6 +251,8 @@ my $re_ip_serialized = qr/$re_ip_sunit(?:\.$re_ip_sunit){3}/; sub Content { my $self = shift; + return undef unless $self->CustomFieldObj->CurrentUserHasRight('SeeCustomField'); + my $content = $self->_Value('Content'); if ( $self->CustomFieldObj->Type eq 'IPAddress' || $self->CustomFieldObj->Type eq 'IPAddressRange' ) @@ -364,11 +366,11 @@ sub _FillInTemplateURL { # special case, whole value should be an URL if ( $url =~ /^__CustomField__/ ) { my $value = $self->Content; - # protect from javascript: URLs - if ( $value =~ /^\s*javascript:/i ) { + # protect from potentially malicious URLs + if ( $value =~ /^\s*(?:javascript|data):/i ) { my $object = $self->Object; $RT::Logger->error( - "Dangerouse value with JavaScript in custom field '". $self->CustomFieldObj->Name ."'" + "Potentially dangerous URL type in custom field '". $self->CustomFieldObj->Name ."'" ." on ". ref($object) ." #". $object->id ); return undef; diff --git a/rt/lib/RT/Queue.pm b/rt/lib/RT/Queue.pm index 3cb87c4be..406df9214 100755 --- a/rt/lib/RT/Queue.pm +++ b/rt/lib/RT/Queue.pm @@ -692,6 +692,7 @@ sub TicketTransactionCustomFields { my $cfs = RT::CustomFields->new( $self->CurrentUser ); if ( $self->CurrentUserHasRight('SeeQueue') ) { + $cfs->SetContextObject( $self ); $cfs->LimitToGlobalOrObjectId( $self->Id ); $cfs->LimitToLookupType( 'RT::Queue-RT::Ticket-RT::Transaction' ); $cfs->ApplySortOrder; @@ -1249,6 +1250,17 @@ sub CurrentUserHasRight { } +=head2 CurrentUserCanSee + +Returns true if the current user can see the queue, using SeeQueue + +=cut + +sub CurrentUserCanSee { + my $self = shift; + + return $self->CurrentUserHasRight('SeeQueue'); +} =head2 HasRight diff --git a/rt/lib/RT/Reminders.pm b/rt/lib/RT/Reminders.pm index e7c28a8e1..2b663256a 100644 --- a/rt/lib/RT/Reminders.pm +++ b/rt/lib/RT/Reminders.pm @@ -137,7 +137,8 @@ sub Open { my $self = shift; my $reminder = shift; - my ( $status, $msg ) = $reminder->SetStatus('open'); + my ( $status, $msg ) = + $reminder->SetStatus( $reminder->QueueObj->Lifecycle->ReminderStatusOnOpen ); $self->TicketObj->_NewTransaction( Type => 'OpenReminder', Field => 'RT::Ticket', @@ -149,7 +150,8 @@ sub Open { sub Resolve { my $self = shift; my $reminder = shift; - my ( $status, $msg ) = $reminder->SetStatus('resolved'); + my ( $status, $msg ) = + $reminder->SetStatus( $reminder->QueueObj->Lifecycle->ReminderStatusOnResolve ); $self->TicketObj->_NewTransaction( Type => 'ResolveReminder', Field => 'RT::Ticket', diff --git a/rt/lib/RT/Report/Tickets.pm b/rt/lib/RT/Report/Tickets.pm index af6e4ed15..de40dbdd4 100644 --- a/rt/lib/RT/Report/Tickets.pm +++ b/rt/lib/RT/Report/Tickets.pm @@ -89,13 +89,7 @@ sub Groupings { foreach my $id (keys %$queues) { my $queue = RT::Queue->new( $self->CurrentUser ); $queue->Load($id); - unless ($queue->id) { - # XXX TODO: This ancient code dates from a former developer - # we have no idea what it means or why cfqueues are so encoded. - $id =~ s/^.'*(.*).'*$/$1/; - $queue->Load($id); - } - $CustomFields->LimitToQueue($queue->Id); + $CustomFields->LimitToQueue($queue->Id) if $queue->Id; } $CustomFields->LimitToGlobal; while ( my $CustomField = $CustomFields->Next ) { diff --git a/rt/lib/RT/Report/Tickets/Entry.pm b/rt/lib/RT/Report/Tickets/Entry.pm index f3eeb4d69..87754c47d 100644 --- a/rt/lib/RT/Report/Tickets/Entry.pm +++ b/rt/lib/RT/Report/Tickets/Entry.pm @@ -83,6 +83,10 @@ sub LabelValue { return $value; } +sub ObjectType { + return 'RT::Ticket'; +} + RT::Base->_ImportOverlays(); 1; diff --git a/rt/lib/RT/Scrip.pm b/rt/lib/RT/Scrip.pm index d513e0aa0..950661624 100755 --- a/rt/lib/RT/Scrip.pm +++ b/rt/lib/RT/Scrip.pm @@ -514,13 +514,35 @@ sub _Set { } - if (length($args{Value})) { + if (exists $args{Value}) { if ($args{Field} eq 'CustomIsApplicableCode' || $args{Field} eq 'CustomPrepareCode' || $args{Field} eq 'CustomCommitCode') { unless ( $self->CurrentUser->HasRight( Object => $RT::System, Right => 'ExecuteCode' ) ) { return ( 0, $self->loc('Permission Denied') ); } } + elsif ($args{Field} eq 'Queue') { + if ($args{Value}) { + # moving to another queue + my $queue = RT::Queue->new( $self->CurrentUser ); + $queue->Load($args{Value}); + unless ($queue->Id and $queue->CurrentUserHasRight('ModifyScrips')) { + return ( 0, $self->loc('Permission Denied') ); + } + } else { + # moving to global + unless ($self->CurrentUser->HasRight( Object => RT->System, Right => 'ModifyScrips' )) { + return ( 0, $self->loc('Permission Denied') ); + } + } + } + elsif ($args{Field} eq 'Template') { + my $template = RT::Template->new( $self->CurrentUser ); + $template->Load($args{Value}); + unless ($template->Id and $template->CurrentUserCanRead) { + return ( 0, $self->loc('Permission Denied') ); + } + } } return $self->__Set(@_); diff --git a/rt/lib/RT/SearchBuilder.pm b/rt/lib/RT/SearchBuilder.pm index 02d4c5058..3e9855110 100644 --- a/rt/lib/RT/SearchBuilder.pm +++ b/rt/lib/RT/SearchBuilder.pm @@ -105,10 +105,14 @@ sub JoinTransactions { TABLE2 => 'Transactions', FIELD2 => 'ObjectId', ); + + my $item = $self->NewItem; + my $object_type = $item->can('ObjectType') ? $item->ObjectType : ref $item; + $self->RT::SearchBuilder::Limit( LEFTJOIN => $alias, FIELD => 'ObjectType', - VALUE => ref $self->NewItem, + VALUE => $object_type, ); $self->{'_sql_aliases'}{'transactions'} = $alias unless $args{'New'}; @@ -127,6 +131,19 @@ sub OrderByCols { return $self->SUPER::OrderByCols( @sort ); } +# If we're setting RowsPerPage or FirstRow, ensure we get a natural number or undef. +sub RowsPerPage { + my $self = shift; + return if @_ and defined $_[0] and $_[0] =~ /\D/; + return $self->SUPER::RowsPerPage(@_); +} + +sub FirstRow { + my $self = shift; + return if @_ and defined $_[0] and $_[0] =~ /\D/; + return $self->SUPER::FirstRow(@_); +} + =head2 LimitToEnabled Only find items that haven't been disabled diff --git a/rt/lib/RT/Shredder.pm b/rt/lib/RT/Shredder.pm index 10d353677..40c73b36d 100644 --- a/rt/lib/RT/Shredder.pm +++ b/rt/lib/RT/Shredder.pm @@ -351,6 +351,8 @@ sub CastObjectsToRecords } elsif ( UNIVERSAL::isa( $targets, 'SCALAR' ) || !ref $targets ) { $targets = $$targets if ref $targets; my ($class, $id) = split /-/, $targets; + RT::Shredder::Exception->throw( "Unsupported class $class" ) + unless $class =~ /^\w+(::\w+)*$/; $class = 'RT::'. $class unless $class =~ /^RTx?::/i; eval "require $class"; die "Couldn't load '$class' module" if $@; diff --git a/rt/lib/RT/Shredder/Plugin.pm b/rt/lib/RT/Shredder/Plugin.pm index e70d207ac..ad9af6ac6 100644 --- a/rt/lib/RT/Shredder/Plugin.pm +++ b/rt/lib/RT/Shredder/Plugin.pm @@ -167,6 +167,7 @@ sub LoadByName { my $self = shift; my $name = shift or return (0, "Name not specified"); + $name =~ /^\w+(::\w+)*$/ or return (0, "Invalid plugin name"); local $@; my $plugin = "RT::Shredder::Plugin::$name"; diff --git a/rt/lib/RT/Shredder/Queue.pm b/rt/lib/RT/Shredder/Queue.pm index c2ec44538..2c0d068fb 100644 --- a/rt/lib/RT/Shredder/Queue.pm +++ b/rt/lib/RT/Shredder/Queue.pm @@ -91,6 +91,7 @@ sub __DependsOn # Custom Fields $objs = RT::CustomFields->new( $self->CurrentUser ); + $objs->SetContextObject( $self ); $objs->LimitToQueue( $self->id ); push( @$list, $objs ); diff --git a/rt/lib/RT/Template.pm b/rt/lib/RT/Template.pm index 158547a0e..117cc3f1c 100755 --- a/rt/lib/RT/Template.pm +++ b/rt/lib/RT/Template.pm @@ -96,10 +96,34 @@ sub _Accessible { sub _Set { my $self = shift; + my %args = ( + Field => undef, + Value => undef, + @_, + ); unless ( $self->CurrentUserHasQueueRight('ModifyTemplate') ) { return ( 0, $self->loc('Permission Denied') ); } + + if (exists $args{Value}) { + if ($args{Field} eq 'Queue') { + if ($args{Value}) { + # moving to another queue + my $queue = RT::Queue->new( $self->CurrentUser ); + $queue->Load($args{Value}); + unless ($queue->Id and $queue->CurrentUserHasRight('ModifyTemplate')) { + return ( 0, $self->loc('Permission Denied') ); + } + } else { + # moving to global + unless ($self->CurrentUser->HasRight( Object => RT->System, Right => 'ModifyTemplate' )) { + return ( 0, $self->loc('Permission Denied') ); + } + } + } + } + return $self->SUPER::_Set( @_ ); } diff --git a/rt/lib/RT/Test.pm b/rt/lib/RT/Test.pm index 0d6da1b9e..7d69dd60d 100644 --- a/rt/lib/RT/Test.pm +++ b/rt/lib/RT/Test.pm @@ -409,7 +409,11 @@ sub bootstrap_db { $args{$forceopt}=1; } - return if $args{nodb}; + # Short-circuit the rest of ourselves if we don't want a db + if ($args{nodb}) { + __drop_database(); + return; + } my $db_type = RT->Config->Get('DatabaseType'); __create_database(); @@ -556,6 +560,13 @@ sub __drop_database { RT::Handle->SystemDSN, $ENV{RT_DBA_USER}, $ENV{RT_DBA_PASSWORD} ); + + # We ignore errors intentionally by not checking the return value of + # DropDatabase below, so let's also suppress DBI's printing of errors when + # we overzealously drop. + local $dbh->{PrintError} = 0; + local $dbh->{PrintWarn} = 0; + RT::Handle->DropDatabase( $dbh ); $dbh->disconnect if $my_dbh; } @@ -1276,8 +1287,10 @@ sub started_ok { require RT::Test::Web; - if ($rttest_opt{nodb}) { - die "you are trying to use a test web server without db, try use noinitialdata => 1 instead"; + if ($rttest_opt{nodb} and not $rttest_opt{server_ok}) { + die "You are trying to use a test web server without a database. " + ."You may want noinitialdata => 1 instead. " + ."Pass server_ok => 1 if you know what you're doing."; } @@ -1298,11 +1311,31 @@ sub test_app { my $self = shift; my %server_opt = @_; - require RT::Interface::Web::Handler; - my $app = RT::Interface::Web::Handler->PSGIApp; + my $app; + + my $warnings = ""; + open( my $warn_fh, ">", \$warnings ); + local *STDERR = $warn_fh; + + if ($server_opt{variant} and $server_opt{variant} eq 'rt-server') { + $app = do { + my $file = "$RT::SbinPath/rt-server"; + my $psgi = do $file; + unless ($psgi) { + die "Couldn't parse $file: $@" if $@; + die "Couldn't do $file: $!" unless defined $psgi; + die "Couldn't run $file" unless $psgi; + } + $psgi; + }; + } else { + require RT::Interface::Web::Handler; + $app = RT::Interface::Web::Handler->PSGIApp; + } require Plack::Middleware::Test::StashWarnings; - $app = Plack::Middleware::Test::StashWarnings->wrap($app); + my $stashwarnings = Plack::Middleware::Test::StashWarnings->new; + $app = $stashwarnings->wrap($app); if ($server_opt{basic_auth}) { require Plack::Middleware::Auth::Basic; @@ -1314,6 +1347,10 @@ sub test_app { } ); } + + close $warn_fh; + $stashwarnings->add_warning( $warnings ) if $warnings; + return $app; } @@ -1346,7 +1383,8 @@ sub start_plack_server { my $Tester = Test::Builder->new; $Tester->ok(1, "started plack server ok"); - __reconnect_rt(); + __reconnect_rt() + unless $rttest_opt{nodb}; return ("http://localhost:$port", RT::Test::Web->new); } diff --git a/rt/lib/RT/Test/Web.pm b/rt/lib/RT/Test/Web.pm index 28ca3b844..c2d9ac314 100644 --- a/rt/lib/RT/Test/Web.pm +++ b/rt/lib/RT/Test/Web.pm @@ -52,15 +52,19 @@ use strict; use warnings; use base qw(Test::WWW::Mechanize); +use Scalar::Util qw(weaken); -require RT::Test; +BEGIN { require RT::Test; } require Test::More; +my $instance; + sub new { my ($class, @args) = @_; push @args, app => $RT::Test::TEST_APP if $RT::Test::TEST_APP; - my $self = $class->SUPER::new(@args); + my $self = $instance = $class->SUPER::new(@args); + weaken $instance; $self->cookie_jar(HTTP::Cookies->new); return $self; @@ -100,6 +104,7 @@ sub login { Test::More::diag("error: page has no Logout"); return 0; } + RT::Interface::Web::EscapeUTF8(\$user); unless ( $self->content =~ m{<span class="current-user">\Q$user\E</span>}i ) { Test::More::diag("Page has no user name"); return 0; @@ -370,4 +375,10 @@ sub DESTROY { } } +END { + return unless $instance; + return if RT::Test->builder->{Original_Pid} != $$; + $instance->no_warnings_ok if !$RT::Test::Web::DESTROY++; +} + 1; diff --git a/rt/lib/RT/Ticket.pm b/rt/lib/RT/Ticket.pm index 27bdc48ee..de8a396da 100755 --- a/rt/lib/RT/Ticket.pm +++ b/rt/lib/RT/Ticket.pm @@ -2438,7 +2438,7 @@ sub _Links { my $links = $self->{ $cache_key } = RT::Links->new( $self->CurrentUser ); unless ( $self->CurrentUserHasRight('ShowTicket') ) { - $links->Limit( FIELD => 'id', VALUE => 0 ); + $links->Limit( FIELD => 'id', VALUE => 0, SUBCLAUSE => 'acl' ); return $links; } @@ -3594,6 +3594,16 @@ sub CurrentUserHasRight { } +=head2 CurrentUserCanSee + +Returns true if the current user can see the ticket, using ShowTicket + +=cut + +sub CurrentUserCanSee { + my $self = shift; + return $self->CurrentUserHasRight('ShowTicket'); +} =head2 HasRight @@ -3706,7 +3716,9 @@ sub Transactions { sub TransactionCustomFields { my $self = shift; - return $self->QueueObj->TicketTransactionCustomFields; + my $cfs = $self->QueueObj->TicketTransactionCustomFields; + $cfs->SetContextObject( $self ); + return $cfs; } diff --git a/rt/lib/RT/Tickets.pm b/rt/lib/RT/Tickets.pm index db54b525b..4e2415b37 100755 --- a/rt/lib/RT/Tickets.pm +++ b/rt/lib/RT/Tickets.pm @@ -1132,6 +1132,12 @@ sub _GroupMembersJoin { FIELD2 => 'GroupId', ENTRYAGGREGATOR => 'AND', ); + $self->SUPER::Limit( + $args{'Left'} ? (LEFTJOIN => $alias) : (), + ALIAS => $alias, + FIELD => 'Disabled', + VALUE => 0, + ); $self->{'_sql_group_members_aliases'}{ $args{'GroupsAlias'} } = $alias unless $args{'New'}; @@ -1296,6 +1302,12 @@ sub _WatcherMembershipLimit { FIELD2 => 'id' ); + $self->Limit( + ALIAS => $groupmembers, + FIELD => 'Disabled', + VALUE => 0, + ); + $self->Join( ALIAS1 => $memberships, FIELD1 => 'MemberId', @@ -1303,6 +1315,13 @@ sub _WatcherMembershipLimit { FIELD2 => 'id' ); + $self->Limit( + ALIAS => $memberships, + FIELD => 'Disabled', + VALUE => 0, + ); + + $self->_CloseParen; } @@ -1639,11 +1658,8 @@ sub _CustomFieldLimit { $self->_CloseParen; } else { - my $cf = RT::CustomField->new( $self->CurrentUser ); - $cf->Load($field); - # need special treatment for Date - if ( $cf->Type eq 'DateTime' && $op eq '=' ) { + if ( $cf and $cf->Type eq 'DateTime' and $op eq '=' ) { if ( $value =~ /:/ ) { # there is time speccified. @@ -3647,7 +3663,11 @@ sub _RestrictionsToClauses { # here is where we store extra data, say if it's a keyword or # something. (I.e. "TYPE SPECIFIC STUFF") - push @{ $clause{$realfield} }, $data; + if (lc $ea eq 'none') { + $clause{$realfield} = [ $data ]; + } else { + push @{ $clause{$realfield} }, $data; + } } return \%clause; } diff --git a/rt/lib/RT/Transaction.pm b/rt/lib/RT/Transaction.pm index d0e64568b..1f1bab15a 100755 --- a/rt/lib/RT/Transaction.pm +++ b/rt/lib/RT/Transaction.pm @@ -512,7 +512,7 @@ sub Attachments { $self->{'attachments'} = RT::Attachments->new( $self->CurrentUser ); unless ( $self->CurrentUserCanSee ) { - $self->{'attachments'}->Limit(FIELD => 'id', VALUE => '0'); + $self->{'attachments'}->Limit(FIELD => 'id', VALUE => '0', SUBCLAUSE => 'acl'); return $self->{'attachments'}; } @@ -714,6 +714,7 @@ sub BriefDescription { if ( $self->Field ) { my $cf = RT::CustomField->new( $self->CurrentUser ); + $cf->SetContextObject( $self->Object ); $cf->Load( $self->Field ); $field = $cf->Name(); $field = $self->loc('a custom field') if !defined($field); @@ -1072,14 +1073,8 @@ sub CurrentUserCanSee { $cf->Load( $cf_id ); return 0 unless $cf->CurrentUserHasRight('SeeCustomField'); } - #if they ain't got rights to see, don't let em - elsif ( $self->__Value('ObjectType') eq "RT::Ticket" ) { - unless ( $self->CurrentUserHasRight('ShowTicket') ) { - return 0; - } - } - - return 1; + # Defer to the object in question + return $self->Object->CurrentUserCanSee("Transaction"); } @@ -1103,7 +1098,7 @@ sub OldValue { return $Object->Content; } else { - return $self->__Value('OldValue'); + return $self->_Value('OldValue'); } } @@ -1117,7 +1112,7 @@ sub NewValue { return $Object->Content; } else { - return $self->__Value('NewValue'); + return $self->_Value('NewValue'); } } @@ -1207,6 +1202,7 @@ sub CustomFieldValues { # do we want to cover this situation somehow here? unless ( defined $field && $field =~ /^\d+$/o ) { my $CFs = RT::CustomFields->new( $self->CurrentUser ); + $CFs->SetContextObject( $self->Object ); $CFs->Limit( FIELD => 'Name', VALUE => $field ); $CFs->LimitToLookupType($self->CustomFieldLookupType); $CFs->LimitToGlobalOrObjectId($self->Object->QueueObj->id); diff --git a/rt/lib/RT/URI.pm b/rt/lib/RT/URI.pm index 4af1cb0da..fce04598a 100644 --- a/rt/lib/RT/URI.pm +++ b/rt/lib/RT/URI.pm @@ -130,7 +130,7 @@ sub FromURI { # Special case: integers passed in as URIs must be ticket ids if ($uri =~ /^(\d+)$/) { $scheme = "fsck.com-rt"; - } elsif ($uri =~ /^((?:\w|\.|-)+?):/) { + } elsif ($uri =~ /^((?!javascript|data)(?:\w|\.|-)+?):/i) { $scheme = $1; } else { diff --git a/rt/lib/RT/URI/fsck_com_article.pm b/rt/lib/RT/URI/fsck_com_article.pm index 503434a1c..0c09b7c3c 100644 --- a/rt/lib/RT/URI/fsck_com_article.pm +++ b/rt/lib/RT/URI/fsck_com_article.pm @@ -188,7 +188,7 @@ Otherwise, return its URI sub HREF { my $self = shift; if ($self->IsLocal && $self->Object) { - return ( RT->Config->Get('WebURL') . "/Articles/Article/Display.html?id=".$self->Object->Id); + return ( RT->Config->Get('WebURL') . "Articles/Article/Display.html?id=".$self->Object->Id); } else { return ($self->URI); diff --git a/rt/lib/RT/User.pm b/rt/lib/RT/User.pm index 2a14cd154..9b4a82683 100755 --- a/rt/lib/RT/User.pm +++ b/rt/lib/RT/User.pm @@ -1206,6 +1206,37 @@ sub HasRight { return $self->PrincipalObj->HasRight(@_); } +=head2 CurrentUserCanSee [FIELD] + +Returns true if the current user can see the user, based on if it is +public, ourself, or we have AdminUsers + +=cut + +sub CurrentUserCanSee { + my $self = shift; + my ($what) = @_; + + # If it's public, fine. Note that $what may be "transaction", which + # doesn't have an Accessible value, and thus falls through below. + if ( $self->_Accessible( $what, 'public' ) ) { + return 1; + } + + # Users can see their own properties + elsif ( defined($self->Id) && $self->CurrentUser->Id == $self->Id ) { + return 1; + } + + # If the user has the admin users right, that's also enough + elsif ( $self->CurrentUser->HasRight( Right => 'AdminUsers', Object => $RT::System) ) { + return 1; + } + else { + return 0; + } +} + =head2 CurrentUserCanModify RIGHT If the user has rights for this object, either because @@ -1334,12 +1365,13 @@ sub Stylesheet { my $style = RT->Config->Get('WebDefaultStylesheet', $self->CurrentUser); + if (RT::Interface::Web->ComponentPathIsSafe($style)) { + my @css_paths = map { $_ . '/NoAuth/css' } RT::Interface::Web->ComponentRoots; - my @css_paths = map { $_ . '/NoAuth/css' } RT::Interface::Web->ComponentRoots; - - for my $css_path (@css_paths) { - if (-d "$css_path/$style") { - return $style + for my $css_path (@css_paths) { + if (-d "$css_path/$style") { + return $style + } } } @@ -1409,6 +1441,12 @@ sub WatchedQueues { FIELD => 'MemberId', VALUE => $self->PrincipalId, ); + $watched_queues->Limit( + ALIAS => $queues_alias, + FIELD => 'Disabled', + VALUE => 0, + ); + $RT::Logger->debug("WatchedQueues got " . $watched_queues->Count . " queues"); @@ -1447,7 +1485,9 @@ sub _Set { if ( $ret == 0 ) { return ( 0, $msg ); } if ( $args{'RecordTransaction'} == 1 ) { - + if ($args{'Field'} eq "Password") { + $args{'Value'} = $Old = '********'; + } my ( $Trans, $Msg, $TransObj ) = $self->_NewTransaction( Type => $args{'TransactionType'}, Field => $args{'Field'}, @@ -1473,25 +1513,9 @@ sub _Value { my $self = shift; my $field = shift; - #if the field is public, return it. - if ( $self->_Accessible( $field, 'public' ) ) { - return ( $self->SUPER::_Value($field) ); - - } - - #If the user wants to see their own values, let them - # TODO figure ouyt a better way to deal with this - elsif ( defined($self->Id) && $self->CurrentUser->Id == $self->Id ) { - return ( $self->SUPER::_Value($field) ); - } - - #If the user has the admin users right, return the field - elsif ( $self->CurrentUser->HasRight(Right =>'AdminUsers', Object => $RT::System) ) { - return ( $self->SUPER::_Value($field) ); - } else { - return (undef); - } - + # Defer to the abstraction above to know if the field can be read + return $self->SUPER::_Value($field) if $self->CurrentUserCanSee($field); + return undef; } =head2 FriendlyName diff --git a/rt/lib/RT/Users.pm b/rt/lib/RT/Users.pm index f3b1b5cef..2784fc757 100755 --- a/rt/lib/RT/Users.pm +++ b/rt/lib/RT/Users.pm @@ -188,6 +188,9 @@ sub MemberOfGroup { FIELD1 => 'id', ALIAS2 => $groupalias, FIELD2 => 'MemberId' ); + $self->Limit( ALIAS => $groupalias, + FIELD => 'Disabled', + VALUE => 0 ); $self->Limit( ALIAS => "$groupalias", FIELD => 'GroupId', @@ -266,6 +269,11 @@ sub _JoinGroupMembers ALIAS2 => $principals, FIELD2 => 'id' ); + $self->Limit( + ALIAS => $group_members, + FIELD => 'Disabled', + VALUE => 0, + ) if $args{'IncludeSubgroupMembers'}; return $group_members; } |