# Contributor(s): Terry Weissman <terry@mozilla.org>
# Myk Melez <myk@mozilla.org>
# Marc Schumann <wurblzap@gmail.com>
+# Frédéric Buclin <LpSolit@gmail.com>
use strict;
=item C<get_attachments_by_bug($bug_id)>
-Description: retrieves and returns the attachments for the given bug.
+Description: retrieves and returns the attachments the currently logged in
+ user can view for the given bug.
Params: C<$bug_id> - integer - the ID of the bug for which
to retrieve and return attachments.
sub get_attachments_by_bug {
my ($class, $bug_id) = @_;
- my $attach_ids = Bugzilla->dbh->selectcol_arrayref("SELECT attach_id
- FROM attachments
- WHERE bug_id = ?",
- undef, $bug_id);
+ my $user = Bugzilla->user;
+ my $dbh = Bugzilla->dbh;
+
+ # By default, private attachments are not accessible, unless the user
+ # is in the insider group or submitted the attachment.
+ my $and_restriction = '';
+ my @values = ($bug_id);
+
+ unless ($user->is_insider) {
+ $and_restriction = 'AND (isprivate = 0 OR submitter_id = ?)';
+ push(@values, $user->id);
+ }
+
+ my $attach_ids = $dbh->selectcol_arrayref("SELECT attach_id FROM attachments
+ WHERE bug_id = ? $and_restriction",
+ undef, @values);
my $attachments = Bugzilla::Attachment->get_list($attach_ids);
return $attachments;
}
=item C<validate_can_edit()>
-Description: validates if the user is allowed to edit the attachment.
+Description: validates if the user is allowed to view and edit the attachment.
Only the submitter or someone with editbugs privs can edit it.
+ Only the submitter and users in the insider group can view
+ private attachments.
Returns: 1 on success. Else an error is thrown.
=cut
sub validate_can_edit {
- my ($class, $attach_id) = @_;
+ my $attachment = shift;
my $dbh = Bugzilla->dbh;
my $user = Bugzilla->user;
- # People in editbugs can edit all attachments
- return if $user->in_group('editbugs');
+ # Bug 97729 - the submitter can edit their attachments.
+ return if ($attachment->attacher->id == $user->id);
- # Bug 97729 - the submitter can edit their attachments
- my ($ref) = $dbh->selectrow_array('SELECT attach_id FROM attachments
- WHERE attach_id = ? AND submitter_id = ?',
- undef, ($attach_id, $user->id));
+ # Only users in the insider group can view private attachments.
+ if ($attachment->isprivate && !$user->is_insider) {
+ ThrowUserError('illegal_attachment_edit', {attach_id => $attachment->id});
+ }
- $ref || ThrowUserError('illegal_attachment_edit', { attach_id => $attach_id });
+ # Users with editbugs privs can edit all attachments.
+ return if $user->in_group('editbugs');
+
+ # If we come here, then this attachment cannot be seen by the user.
+ ThrowUserError('illegal_attachment_edit', { attach_id => $attachment->id });
}
=item C<validate_obsolete($bug_id)>
Description: validates if attachments the user wants to mark as obsolete
really belong to the given bug and are not already obsolete.
+ Moreover, a user cannot mark an attachment as obsolete if
+ he cannot view it (due to restrictions on it).
Params: $bug_id - The bug ID obsolete attachments should belong to.
my $cgi = Bugzilla->cgi;
# Make sure the attachment id is valid and the user has permissions to view
- # the bug to which it is attached.
+ # the bug to which it is attached. Make sure also that the user can view
+ # the attachment itself.
my @obsolete_attachments;
foreach my $attachid ($cgi->param('obsolete')) {
my $vars = {};
# Make sure the attachment exists in the database.
ThrowUserError('invalid_attach_id', $vars) unless $attachment;
+ # Check that the user can view and edit this attachment.
+ $attachment->validate_can_edit;
+
$vars->{'description'} = $attachment->description;
if ($attachment->bug_id != $bug_id) {
ThrowCodeError('attachment_already_obsolete', $vars);
}
- # Check that the user can modify this attachment.
- $class->validate_can_edit($attachid);
push(@obsolete_attachments, $attachment);
}
return @obsolete_attachments;
$class->validate_is_patch($throw_error) || return 0;
$class->validate_description($throw_error) || return 0;
- if (($attachurl =~ /^(http|https|ftp):\/\/\S+/)
- && !(defined $cgi->upload('data'))) {
+ if (Bugzilla->params->{'allow_attach_url'}
+ && ($attachurl =~ /^(http|https|ftp):\/\/\S+/)
+ && !defined $cgi->upload('data'))
+ {
$filename = '';
$data = $attachurl;
$isurl = 1;
$isurl = 0;
}
+ # Check attachments the user tries to mark as obsolete.
+ my @obsolete_attachments;
+ if ($cgi->param('obsolete')) {
+ @obsolete_attachments = $class->validate_obsolete($bug->bug_id);
+ }
+
# The order of these function calls is important, as Flag::validate
# assumes User::match_field has ensured that the
# values in the requestee fields are legitimate user email addresses.
close $fh;
}
- # Now handle flags.
- my @obsolete_attachments;
- if ($cgi->param('obsolete')) {
- @obsolete_attachments = $class->validate_obsolete($bug->bug_id);
- }
-
# Make existing attachments obsolete.
my $fieldid = get_field_id('attachments.isobsolete');
package Bugzilla::Attachment::PatchReader;
use Bugzilla::Error;
+use Bugzilla::Attachment;
sub process_diff {
$reader->iterate_string('Attachment ' . $attachment->id, $attachment->data);
}
else {
- $vars->{'other_patches'} = [];
+ my @other_patches = ();
if ($lc->{interdiffbin} && $lc->{diffpath}) {
- # Get list of attachments on this bug.
+ # Get the list of attachments that the user can view in this bug.
+ my @attachments =
+ @{Bugzilla::Attachment->get_attachments_by_bug($attachment->bug_id)};
+ # Extract patches only.
+ @attachments = grep {$_->ispatch == 1} @attachments;
+ # We want them sorted from newer to older.
+ @attachments = sort { $b->id <=> $a->id } @attachments;
+
# Ignore the current patch, but select the one right before it
# chronologically.
- my $attachment_list =
- $dbh->selectall_arrayref('SELECT attach_id, description
- FROM attachments
- WHERE bug_id = ?
- AND ispatch = 1
- ORDER BY creation_ts DESC',
- undef, $attachment->bug_id);
-
my $select_next_patch = 0;
- foreach (@$attachment_list) {
- my ($other_id, $other_desc) = @$_;
- if ($other_id == $attachment->id) {
+ foreach my $attach (@attachments) {
+ if ($attach->id == $attachment->id) {
$select_next_patch = 1;
}
else {
- push(@{$vars->{'other_patches'}}, {'id' => $other_id,
- 'desc' => $other_desc,
- 'selected' => $select_next_patch});
- if ($select_next_patch) {
- $select_next_patch = 0;
- }
+ push(@other_patches, { 'id' => $attach->id,
+ 'desc' => $attach->description,
+ 'selected' => $select_next_patch });
+ $select_next_patch = 0;
}
}
}
$vars->{'bugid'} = $attachment->bug_id;
$vars->{'attachid'} = $attachment->id;
$vars->{'description'} = $attachment->description;
+ $vars->{'other_patches'} = \@other_patches;
setup_template_patch_reader($last_reader, $format, $context, $vars);
# Actually print out the patch.
return $self->{'is_mover'};
}
+sub is_insider {
+ my $self = shift;
+
+ if (!defined $self->{'is_insider'}) {
+ my $insider_group = Bugzilla->params->{'insidergroup'};
+ $self->{'is_insider'} =
+ ($insider_group && $self->in_group($insider_group)) ? 1 : 0;
+ }
+ return $self->{'is_insider'};
+}
+
sub get_userlist {
my $self = shift;
to another database. Note that this method doesn't check whether bug
moving is enabled.
+=item C<is_insider>
+
+Returns true if the user can access private comments and attachments,
+i.e. if the 'insidergroup' parameter is set and the user belongs to this group.
+
=back
=head1 CLASS FUNCTIONS
{
my $param = @_ ? $_[0] : 'id';
my $dbh = Bugzilla->dbh;
-
+ my $user = Bugzilla->user;
+
# If we're not doing interdiffs, check if id wasn't specified and
# prompt them with a page that allows them to choose an attachment.
# Happens when calling plain attachment.cgi from the urlbar directly
|| ThrowUserError("invalid_attach_id", { attach_id => $cgi->param($param) });
# Make sure the attachment exists in the database.
- my ($bugid, $isprivate) = $dbh->selectrow_array(
- "SELECT bug_id, isprivate
+ my ($bugid, $isprivate, $submitter_id) = $dbh->selectrow_array(
+ "SELECT bug_id, isprivate, submitter_id
FROM attachments
WHERE attach_id = ?",
undef, $attach_id);
unless $bugid;
# Make sure the user is authorized to access this attachment's bug.
-
ValidateBugID($bugid);
- if ($isprivate && Bugzilla->params->{"insidergroup"}) {
- Bugzilla->user->in_group(Bugzilla->params->{"insidergroup"})
- || ThrowUserError("auth_failure", {action => "access",
- object => "attachment"});
+ if ($isprivate && $user->id != $submitter_id && !$user->is_insider) {
+ ThrowUserError('auth_failure', {action => 'access',
+ object => 'attachment'});
}
- return ($attach_id,$bugid);
+ return ($attach_id, $bugid);
}
# Validates format of a diff/interdiff. Takes a list as an parameter, which
# Display all attachments for a given bug in a series of IFRAMEs within one
# HTML page.
-sub viewall
-{
+sub viewall {
# Retrieve and validate parameters
my $bugid = $cgi->param('bugid');
ValidateBugID($bugid);
+ my $bug = new Bugzilla::Bug($bugid);
- # Retrieve the attachments from the database and write them into an array
- # of hashes where each hash represents one attachment.
- my $privacy = "";
- my $dbh = Bugzilla->dbh;
+ my $attachments = Bugzilla::Attachment->get_attachments_by_bug($bugid);
- if ( Bugzilla->params->{"insidergroup"}
- && !Bugzilla->user->in_group(Bugzilla->params->{"insidergroup"}) )
- {
- $privacy = "AND isprivate < 1 ";
+ foreach my $a (@$attachments) {
+ $a->{'isviewable'} = isViewable($a->contenttype);
}
- my $attachments = $dbh->selectall_arrayref(
- "SELECT attach_id AS attachid, " .
- $dbh->sql_date_format('creation_ts', '%Y.%m.%d %H:%i') . " AS date,
- mimetype AS contenttype, description, ispatch, isobsolete, isprivate,
- LENGTH(thedata) AS datasize
- FROM attachments
- INNER JOIN attach_data
- ON attach_id = id
- WHERE bug_id = ? $privacy
- ORDER BY attach_id", {'Slice'=>{}}, $bugid);
-
- foreach my $a (@{$attachments})
- {
-
- $a->{'isviewable'} = isViewable($a->{'contenttype'});
- $a->{'flags'} = Bugzilla::Flag::match({ 'attach_id' => $a->{'attachid'} });
- }
- # Retrieve the bug summary (for displaying on screen) and assignee.
- my ($bugsummary, $assignee_id) = $dbh->selectrow_array(
- "SELECT short_desc, assigned_to FROM bugs " .
- "WHERE bug_id = ?", undef, $bugid);
+ # Define the variables and functions that will be passed to the UI template.
+ $vars->{'bug'} = $bug;
+ $vars->{'attachments'} = $attachments;
- # Define the variables and functions that will be passed to the UI template.
- $vars->{'bugid'} = $bugid;
- $vars->{'attachments'} = $attachments;
- $vars->{'bugassignee_id'} = $assignee_id;
- $vars->{'bugsummary'} = $bugsummary;
-
- print $cgi->header();
+ print $cgi->header();
- # Generate and return the UI (HTML page) from the appropriate template.
- $template->process("attachment/show-multiple.html.tmpl", $vars)
- || ThrowTemplateError($template->error());
+ # Generate and return the UI (HTML page) from the appropriate template.
+ $template->process("attachment/show-multiple.html.tmpl", $vars)
+ || ThrowTemplateError($template->error());
}
# Display a form for entering a new attachment.
# Retrieve a list of attachments for this bug as well as a summary of the bug
# to use in a navigation bar across the top of the screen.
my $bugattachments =
- $dbh->selectcol_arrayref('SELECT attach_id FROM attachments
- WHERE bug_id = ? ORDER BY attach_id',
- undef, $attachment->bug_id);
+ Bugzilla::Attachment->get_attachments_by_bug($attachment->bug_id);
+ # We only want attachment IDs.
+ @$bugattachments = map { $_->id } @$bugattachments;
my ($bugsummary, $product_id, $component_id) =
$dbh->selectrow_array('SELECT short_desc, product_id, component_id
# Users cannot edit the content of the attachment itself.
sub update
{
- my $userid = Bugzilla->user->id;
+ my $user = Bugzilla->user;
+ my $userid = $user->id;
+ my $dbh = Bugzilla->dbh;
# Retrieve and validate parameters
ValidateComment(scalar $cgi->param('comment'));
my ($attach_id, $bugid) = validateID();
- Bugzilla::Attachment->validate_can_edit($attach_id);
+ my $attachment = Bugzilla::Attachment->get($attach_id);
+ $attachment->validate_can_edit;
validateCanChangeAttachment($attach_id);
Bugzilla::Attachment->validate_description(THROW_ERROR);
Bugzilla::Attachment->validate_is_patch(THROW_ERROR);
Bugzilla::Attachment->validate_content_type(THROW_ERROR) unless $cgi->param('ispatch');
validateIsObsolete();
validatePrivate();
- my $dbh = Bugzilla->dbh;
+
+ # If the submitter of the attachment is not in the insidergroup,
+ # be sure that he cannot overwrite the private bit.
+ # This check must be done before calling Bugzilla::Flag*::validate(),
+ # because they will look at the private bit when checking permissions.
+ # XXX - This is a ugly hack. Ideally, we shouldn't have to look at the
+ # old private bit twice (first here, and then below again), but this is
+ # the less risky change.
+ unless ($user->is_insider) {
+ my $oldisprivate = $dbh->selectrow_array('SELECT isprivate FROM attachments
+ WHERE attach_id = ?', undef, $attach_id);
+ $cgi->param('isprivate', $oldisprivate);
+ }
# The order of these function calls is important, as Flag::validate
# assumes User::match_field has ensured that the values in the
# to attachments so that we can delete pending requests if the user
# is obsoleting this attachment without deleting any requests
# the user submits at the same time.
- my $attachment = Bugzilla::Attachment->get($attach_id);
Bugzilla::Flag::process($bug, $attachment, $timestamp, $cgi);
# Update the attachment record in the database.
# Make sure the administrator is allowed to edit this attachment.
my ($attach_id, $bug_id) = validateID();
- Bugzilla::Attachment->validate_can_edit($attach_id);
+ my $attachment = Bugzilla::Attachment->get($attach_id);
+ $attachment->validate_can_edit;
validateCanChangeAttachment($attach_id);
- my $attachment = Bugzilla::Attachment->get($attach_id);
$attachment->datasize || ThrowUserError('attachment_removed');
# We don't want to let a malicious URL accidentally delete an attachment.
my $status = validateStatus($cgi->param('status'));
my $form_group = validateGroup($cgi->param('group'));
- my $attach_join_clause = "flags.attach_id = attachments.attach_id";
- if (Bugzilla->params->{"insidergroup"}
- && !Bugzilla->user->in_group(Bugzilla->params->{"insidergroup"}))
- {
- $attach_join_clause .= " AND attachments.isprivate < 1";
- }
-
my $query =
# Select columns describing each flag, the bug/attachment on which
# it has been set, who set it, and of whom they are requesting it.
"
FROM flags
LEFT JOIN attachments
- ON ($attach_join_clause)
+ ON flags.attach_id = attachments.attach_id
INNER JOIN flagtypes
ON flags.type_id = flagtypes.id
INNER JOIN profiles AS requesters
(bugs.assigned_to = $userid) " .
(Bugzilla->params->{'useqacontact'} ? "OR
(bugs.qa_contact = $userid))" : ")");
-
+
+ unless ($user->is_insider) {
+ $query .= " AND (attachments.attach_id IS NULL
+ OR attachments.isprivate = 0
+ OR attachments.submitter_id = $userid)";
+ }
+
# Limit query to pending requests.
$query .= " AND flags.status = '?' " unless $status;
[% END %]
<th bgcolor="#cccccc" align="left">Actions</th>
</tr>
- [% canseeprivate = !Param("insidergroup") || user.in_group(Param("insidergroup")) %]
[% count = 0 %]
[% FOREACH attachment = attachments %]
[% count = count + 1 %]
- [% IF !attachment.isprivate || canseeprivate %]
+ [% IF !attachment.isprivate || user.is_insider || attachment.attacher.id == user.id %]
<tr [% "class=\"bz_private\"" IF attachment.isprivate %]>
<td valign="top">
<a name="a[% count %]" href="attachment.cgi?id=[% attachment.id %]">[% attachment.description FILTER html FILTER obsolete(attachment.isobsolete) %]</a>
<table class="attachment_info" cellspacing="0" cellpadding="4" border="1" width="75%">
<tr>
<td valign="top" bgcolor="#cccccc" colspan="6">
- <big><b>Attachment #[% a.attachid %]</b></big>
+ <big><b>Attachment #[% a.id %]</b></big>
</td>
</tr>
<tr>
[% END %]
</td>
- <td valign="top">[% a.date FILTER time %]</td>
+ <td valign="top">[% a.attached FILTER time %]</td>
<td valign="top">[% a.datasize FILTER unitconvert %]</td>
<td valign="top">
</td>
<td valign="top">
- <a href="attachment.cgi?id=[% a.attachid %]&action=edit">Details</a>
+ <a href="attachment.cgi?id=[% a.id %]&action=edit">Details</a>
</td>
</tr>
</table>
[% IF a.isviewable %]
- <iframe src="attachment.cgi?id=[% a.attachid %]" width="75%" height="350">
+ <iframe src="attachment.cgi?id=[% a.id %]" width="75%" height="350">
<b>You cannot view the attachment on this page because your browser does not support IFRAMEs.
- <a href="attachment.cgi?id=[% a.attachid %]">View the attachment on a separate page</a>.</b>
+ <a href="attachment.cgi?id=[% a.id %]">View the attachment on a separate page</a>.</b>
</iframe>
[% ELSE %]
<p><b>
Attachment cannot be viewed because its MIME type is not text/*, image/*, or application/vnd.mozilla.*.
- <a href="attachment.cgi?id=[% a.attachid %]">Download the attachment instead</a>.
+ <a href="attachment.cgi?id=[% a.id %]">Download the attachment instead</a>.
</b></p>
[% END %]
</div>
],
'attachment/show-multiple.html.tmpl' => [
- 'a.attachid',
+ 'a.id',
'flag.status'
],
projects / thirdparty / bugzilla.git / commitdiff
