diff --git a/extensions/Needinfo/Extension.pm b/extensions/Needinfo/Extension.pm index 79331e13eb..4b6051d390 100644 --- a/extensions/Needinfo/Extension.pm +++ b/extensions/Needinfo/Extension.pm @@ -12,14 +12,18 @@ use warnings; use base qw(Bugzilla::Extension); +use Bugzilla::Constants; use Bugzilla::Error; use Bugzilla::Flag; use Bugzilla::FlagType; +use Bugzilla::Logging; use Bugzilla::User; use Bugzilla::User::Setting; our $VERSION = '0.01'; +use constant MAX_MENTIONS => 10; + BEGIN { *Bugzilla::User::needinfo_blocked = \&_user_needinfo_blocked; } @@ -60,7 +64,7 @@ sub install_update_db { is_active => 1, is_requestable => 1, is_requesteeble => 1, - is_multiplicable => 0, + is_multiplicable => 1, request_group => '', grant_group => '', inclusions => ['0:0'], @@ -97,10 +101,19 @@ sub bug_end_of_create { $self->bug_start_of_update($args); } -# Clear the needinfo? flag if comment is being given by -# requestee or someone used the override flag. sub bug_start_of_update { my ($self, $args) = @_; + _process_needinfo_params($args); + + # Runs after the explicit needinfo handling so that the duplicate check + # sees any flags that were just requested through the form. + _process_mentions($args->{bug}) if $args->{old_bug}; +} + +# Clear the needinfo? flag if comment is being given by +# requestee or someone used the override flag. +sub _process_needinfo_params { + my ($args) = @_; my $bug = $args->{bug}; my $old_bug = $args->{old_bug}; @@ -248,6 +261,117 @@ sub bug_start_of_update { } } +# Returns the unique nicknames @mentioned in a comment, in order of first +# appearance. Mentions inside quoted lines (> ...) and code are ignored, as +# are email addresses (the @ must not follow a word character). +sub _extract_mentions { + my ($text) = @_; + return () unless defined $text; + + $text =~ s/^[ \t]*(`{3,}|~{3,}).*?(?:^[ \t]*\1[^\n]*$|\z)//msg; + # A code span closes on a backtick run of the same length, within a paragraph. + $text = join "\n\n", + map { s/(?.*$//mg; + + my (@nicks, %seen); + # Same character set as extract_nicks in Bugzilla/Util.pm. + while ($text =~ /(?dbh; + my $rows = $dbh->selectall_arrayref( + 'SELECT userid, nickname FROM profiles WHERE is_enabled = 1 AND ' + . $dbh->sql_in('nickname', [map { $dbh->quote($_) } @nicks])); + + my %ids_by_nick; + push @{$ids_by_nick{lc $_->[1]}}, $_->[0] foreach @$rows; + my @ids = map { $_->[0] } grep { @$_ == 1 } values %ids_by_nick; + return @{Bugzilla::User->new_from_list(\@ids)}; +} + +# GitHub-style mentions: each @nickname in a new comment is CC'd and gets a +# needinfo request. Mentions never block the comment from being saved; anyone +# who can't be needinfo'd (blocked, already asked, no permission, or the +# needinfo type isn't multiplicable and a flag exists) is only CC'd. +# +# Safeguards against misuse: +# - only editbugs users can trigger mentions; for everyone else they are text +# - only the first MAX_MENTIONS distinct nicknames per update are processed +# - users who can't already see the bug are skipped, so a mention (or a typo, +# or a squatted nickname) can never grant access to a restricted bug +sub _process_mentions { + my ($bug) = @_; + # Lowercased nick => true if mentioned in at least one public comment. + my %public; + + # The first MAX_MENTIONS distinct lowercased nicks, in order of appearance. + my @nicks; + + foreach my $comment (@{$bug->{added_comments} || []}) { + foreach my $nick (map {lc} _extract_mentions($comment->{thetext})) { + push @nicks, $nick if !exists $public{$nick} && @nicks < MAX_MENTIONS; + $public{$nick} ||= !$comment->{isprivate}; + } + } + return unless @nicks; + + # Checked after parsing so updates without mentions skip the group lookup. + my $user = Bugzilla->user; + return unless $user->in_group('editbugs', $bug->product_id); + + my @mentioned = grep { + $_->id != $user->id + && ($public{lc $_->nick} || $_->is_insider) + && $_->can_see_bug($bug->id) + } _users_for_mentions(@nicks); + return unless @mentioned; + + my ($type) = grep { $_->name eq 'needinfo' } @{$bug->flag_types}; + undef $type + unless $type && $bug->check_can_change_field('flagtypes.name', 0, 1)->{allowed}; + + # A failure for one user (e.g. add_cc's strict_isolation check) skips that + # user rather than rolling back the whole update. Throw*Error only dies + # (instead of printing an error page and exiting) in ERROR_MODE_DIE, so the + # eval can catch it. + local Bugzilla->request_cache->{error_mode} = ERROR_MODE_DIE; + local $@; + foreach my $mentioned (@mentioned) { + eval { _cc_and_needinfo($bug, $mentioned, $type); 1 } + or WARN('Skipped mention of ' . $mentioned->login . " on bug " + . $bug->id . ": $@"); + } +} + +sub _cc_and_needinfo { + my ($bug, $mentioned, $type) = @_; + $bug->add_cc($mentioned); + + return if !$type || $mentioned->needinfo_blocked; + + # A non-multiplicable type allows only one needinfo flag per bug. + return if !$type->is_multiplicable && @{$type->{flags}}; + return if grep { + $_->status eq '?' && ($_->requestee_id // 0) == $mentioned->id + } @{$type->{flags}}; + + # One call per user so $type->{flags} reflects each flag as it is added. + $bug->set_flags([], + [{type_id => $type->id, status => '?', requestee => $mentioned->login}]); +} + sub _check_requestee { my ($requestee) = @_; my $user diff --git a/extensions/Needinfo/t/mentions.t b/extensions/Needinfo/t/mentions.t new file mode 100644 index 0000000000..dabcd7bcad --- /dev/null +++ b/extensions/Needinfo/t/mentions.t @@ -0,0 +1,171 @@ +#!/usr/bin/env perl +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. +use strict; +use warnings; +use lib qw( . lib local/lib/perl5 ); + +BEGIN { + $ENV{LOG4PERL_CONFIG_FILE} = 'log4perl-t.conf'; + $ENV{BUGZILLA_DISABLE_HOSTAGE} = 1; +} + +use Bugzilla::Test::MockDB; +use Bugzilla::Test::MockParams; + +use Test2::V0; + +use Bugzilla; +BEGIN { Bugzilla->extensions } + +my $extract = \&Bugzilla::Extension::Needinfo::_extract_mentions; + +is([$extract->(undef)], [], 'undef text'); +is([$extract->('no mentions here')], [], 'no mentions'); +is([$extract->('@alice can you look?')], ['alice'], 'leading mention'); +is([$extract->('thanks @alice, @bob and @carol.')], + ['alice', 'bob', 'carol'], 'multiple mentions with punctuation'); +is([$extract->('@alice and @Alice again @alice')], ['alice'], + 'duplicates collapsed case-insensitively'); +is([$extract->('(@dk.l_x-y)')], ['dk.l_x-y'], 'nick symbols, parens'); +is([$extract->('mail foo@example.com or a/@b or x.@y')], [], + 'emails and embedded @ ignored'); +is([$extract->("> \@quoted said\n\@alice")], ['alice'], 'quoted lines ignored'); +is([$extract->('run `@decorator` @alice')], ['alice'], 'inline code ignored'); +is([$extract->('``@alice`` and `` a`@b ``')], [], 'double backtick code ignored'); +is([$extract->("it's a ` stray\n\n\@alice `x`")], + ['alice'], 'unclosed backtick does not span paragraphs'); +is([$extract->('@foo|bar and |@alice|')], ['foo|bar', 'alice'], + 'pipe allowed in nick, trailing pipe stripped'); +is( + [$extract->("```perl\n\@inside\n```\n\@after")], + ['after'], 'fenced code block ignored' +); +is([$extract->("~~~\n\@unclosed\n\@more")], [], 'unclosed fence runs to end'); + +# _process_mentions safeguards, using minimal stand-ins for users and bugs. +{ + + package FakeUser; + sub new { my ($class, %args) = @_; bless {%args}, $class } + sub id { $_[0]{id} } + sub nick { $_[0]{nick} } + sub login { "$_[0]{nick}\@example.com" } + sub in_group { $_[0]{editbugs} } + sub is_insider { $_[0]{insider} } + sub can_see_bug { $_[0]{sees_bug} // 1 } + sub needinfo_blocked { 0 } + + package FakeBug; + sub new { + my ($class, $text, %type) = @_; + bless { + added_comments => [{thetext => $text}], + cc => [], + needinfo => [], + type => bless({flags => [], multiplicable => 1, %type}, 'FakeType'), + }, $class; + } + sub id {1} + sub product_id {1} + sub flag_types { [$_[0]{type}] } + sub check_can_change_field { {allowed => 1} } + sub add_cc { + my ($self, $user) = @_; + die "add_cc failed\n" if $user->{cc_fails}; + push @{$self->{cc}}, $user->nick; + } + + # Like Bugzilla::Flag->set_flag, new flags are added to the type's list. + sub set_flags { + my ($self, undef, $new_flags) = @_; + foreach my $flag (@$new_flags) { + push @{$self->{needinfo}}, $flag->{requestee}; + push @{$self->{type}{flags}}, + bless({status => '?', requestee_id => 0}, 'FakeFlag'); + } + } + + package FakeType; + sub name {'needinfo'} + sub id {1} + sub is_multiplicable { $_[0]{multiplicable} } + + package FakeFlag; + sub status { $_[0]{status} } + sub requestee_id { $_[0]{requestee_id} } +} + +my %users = map { $_->nick => $_ } ( + FakeUser->new(id => 2, nick => 'alice'), + FakeUser->new(id => 3, nick => 'bob'), + FakeUser->new(id => 4, nick => 'hidden', sees_bug => 0), + FakeUser->new(id => 6, nick => 'boom', cc_fails => 1), + map { FakeUser->new(id => 10 + $_, nick => "u$_") } 1 .. 12, +); +my $commenter = FakeUser->new(id => 1, nick => 'me', editbugs => 1); +my @looked_up; + +my $mock_bugzilla = mock 'Bugzilla' => (override => [user => sub {$commenter}]); +my $mock_ext = mock 'Bugzilla::Extension::Needinfo' => ( + override => [ + _users_for_mentions => sub { + @looked_up = @_; + return grep {defined} map { $users{$_} } @_; + }, + ], +); + +my $process = \&Bugzilla::Extension::Needinfo::_process_mentions; + +my $bug = FakeBug->new('@alice and @bob please look, cc @me'); +$process->($bug); +is($bug->{cc}, ['alice', 'bob'], 'mentioned users CCd, self skipped'); +is($bug->{needinfo}, ['alice@example.com', 'bob@example.com'], + 'mentioned users needinfod'); + +$bug = FakeBug->new('@hidden @alice'); +$process->($bug); +is($bug->{cc}, ['alice'], 'user who cannot see the bug is not CCd'); +is($bug->{needinfo}, ['alice@example.com'], '... nor needinfod'); + +$bug = FakeBug->new(join ' ', map {"\@u$_"} 1 .. 12); +$process->($bug); +is(\@looked_up, [map {"u$_"} 1 .. 10], 'only the first 10 nicknames are looked up'); +is($bug->{cc}, [map {"u$_"} 1 .. 10], '... and only they are CCd'); + +$bug = FakeBug->new('@alice @bob', multiplicable => 0); +$process->($bug); +is($bug->{cc}, ['alice', 'bob'], 'non-multiplicable type: everyone CCd'); +is($bug->{needinfo}, ['alice@example.com'], '... but only one needinfo'); + +$bug = FakeBug->new('@alice', multiplicable => 0); +push @{$bug->{type}{flags}}, bless({status => '?', requestee_id => 9}, 'FakeFlag'); +$process->($bug); +is($bug->{cc}, ['alice'], 'non-multiplicable with existing flag: CCd'); +is($bug->{needinfo}, [], '... and no needinfo, instead of an error'); + +$bug = FakeBug->new('@alice'); +push @{$bug->{type}{flags}}, bless({status => '?', requestee_id => 2}, 'FakeFlag'); +$process->($bug); +is($bug->{needinfo}, [], 'no duplicate needinfo for a pending request'); + +$bug = FakeBug->new('@boom @alice'); +ok(lives { $process->($bug) }, + 'a failing CC (e.g. strict_isolation) does not throw'); +is($bug->{cc}, ['alice'], '... that user is skipped'); +is($bug->{needinfo}, ['alice@example.com'], '... others still processed'); + +$commenter->{editbugs} = 0; +@looked_up = (); +$bug = FakeBug->new('@alice'); +$process->($bug); +is(\@looked_up, [], 'no lookup for commenters without editbugs'); +is($bug->{cc}, [], 'no CC for commenters without editbugs'); +is($bug->{needinfo}, [], 'no needinfo for commenters without editbugs'); + +done_testing;