Closed
Bug 302936
Opened 19 years ago
Closed 19 years ago
Reject the requestee if he cannot access private attachments
Categories
(Bugzilla :: Attachments & Requests, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.20
People
(Reporter: LpSolit, Assigned: LpSolit)
Details
Attachments
(1 file, 1 obsolete file)
2.24 KB,
patch
|
jouni
:
review+
|
Details | Diff | Splinter Review |
Actually, if a user cannot access a bug, a request asking him to set a flag is rejected. But if a user cannot access an attachment because the attachment is private (the user is not in the insidergroup), no error is displayed. For this user, request.cgi displays the attachment number only, but the description remains hidden.
Assignee | ||
Comment 1•19 years ago
|
||
The validation only fails on new attachments, but works correctly on existing attachments.
Severity: minor → normal
Status: NEW → ASSIGNED
Target Milestone: --- → Bugzilla 2.20
Assignee | ||
Comment 2•19 years ago
|
||
The reason is in FlagType::validate: # Throw an error if the target is a private attachment and # the requestee isn't in the group of insiders who can see it. if ($attach_id && Param("insidergroup") && $cgi->param('isprivate') && !$requestee->in_group(Param("insidergroup"))) If the attachment is new, $attach_id doesn't exist yet and we ignore this test!
Assignee | ||
Comment 3•19 years ago
|
||
As the attachment has not been inserted into the DB yet, its ID is unknown. We use '-1' to indicate that the attachment is new. I see no better solution without invasive changes in attachment.cgi; which we should avoid, especially on the branch. A solution (for the tip only) would be to first insert the attachment into the DB, and then validate flags. This would also fix the user matching problem where the confirmation page makes the attachment data to be lost. I would say that this is another bug (the important point here being to validate requestees in all cases, even with a fake attachment ID for now). The patch applies cleanly to 2.20 and tip.
Attachment #196108 -
Flags: review?(myk)
Comment 4•19 years ago
|
||
Comment on attachment 196108 [details] [diff] [review] patch, v1 Looks good from this end. Sending the -1 (new attachment) to the validate function does force a check.
Attachment #196108 -
Flags: review+
Assignee | ||
Comment 5•19 years ago
|
||
explain the meaning of $attach_id = -1
Attachment #196108 -
Attachment is obsolete: true
Attachment #199914 -
Flags: review?(jouni)
Assignee | ||
Updated•19 years ago
|
Attachment #196108 -
Flags: review?(myk)
Comment 6•19 years ago
|
||
Comment on attachment 199914 [details] [diff] [review] patch, v2 >+ # FlagType::validate() and Flag::validate() should not detect >+ # any reference to existing flags when creating a new attachment. >+ # Setting the third param to -1 will force this function to check this point. Nit: Wrap the third line to avoid exceeding 80 chars. r=jouni, by inspection and discussion only. The patch is functionally equivalent to v1 though.
Attachment #199914 -
Flags: review?(jouni) → review+
Assignee | ||
Updated•19 years ago
|
Flags: approval?
Flags: approval2.20?
Updated•19 years ago
|
Flags: approval?
Flags: approval2.20?
Flags: approval2.20+
Flags: approval+
Assignee | ||
Comment 7•19 years ago
|
||
tip: Checking in attachment.cgi; /cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v <-- attachment.cgi new revision: 1.99; previous revision: 1.98 done Checking in Bugzilla/Flag.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Flag.pm,v <-- Flag.pm new revision: 1.55; previous revision: 1.54 done Checking in Bugzilla/FlagType.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/FlagType.pm,v <-- FlagType.pm new revision: 1.23; previous revision: 1.22 done 2.20: Checking in attachment.cgi; /cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v <-- attachment.cgi new revision: 1.89.2.1; previous revision: 1.89 done Checking in Bugzilla/Flag.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Flag.pm,v <-- Flag.pm new revision: 1.45.2.2; previous revision: 1.45.2.1 done Checking in Bugzilla/FlagType.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/FlagType.pm,v <-- FlagType.pm new revision: 1.19.2.2; previous revision: 1.19.2.1 done
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•