Closed Bug 200957 Opened 22 years ago Closed 21 years ago

CanSeeBug should check if the user will have access after changes

Categories

(Bugzilla :: Bugzilla-General, defect)

defect
Not set
minor

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: myk, Assigned: justdave)

Details

CanSeeBug should check if the user will have access to the bug after the
currently submitted changes go through.  Otherwise it'll report incorrectly that
the user will or won't be able to see the bug when that access is changing (f.e.
when the user is being added to or removed from the CC: list).

See bug 179510.
Some code I was including in my patch for bug 179510 before I decided to spawn a
separate bug:

          if (# the user can't see the bug
              !&::CanSeeBug($bug_id, $requestee_id)
              # and they won't be able to see it after these changes
              && (# either because they're being removed from the cc: list
                  ($data->{'removecc'}
                   && grep($_ eq $requestee_email, @{$::MFORM{'cc'}}))
                  # or because they aren't being added to it
                  || !($data->{'cclist_accessible'}
                       && $data->{'newcc'} =~ /\Q$requestee_email\E/)))
'CanSeeBug should check if the user will have access to the bug after the
currently submitted changes go through.'

Well, not CanSeeBug, since thats going to be a method on Bug.pm which knows
nothign about process_bug yet.

See, if we had transactions, we could make the change, test, and then rollback
if needed, but.... :)

Does this affect anything apart from this now, though?
As far as I know, this is the first case of something affected by this.
Blocks: 190911
This isn't making the 2.17.4 train unless it's resolved in the next 24 hours.
No longer blocks: 190911
Whiteboard: [wanted for 2.17.5]
Does this affect 2.16.3?
Target Milestone: --- → Bugzilla 2.18
myk/bbaetz: status?
Blocks: bz-2.17.5
Flags: approval?
Oops. Windows in a twist. Sorry. De-requesting approval.

Gerv
Flags: approval?
Can someone explain why this is a security-sensitive bug blocking 2.17.5? 

From the description, it seems that all that'll happen is that e.g. someone may
get a notification mail saying "this bug is being secured, and you can't see it
any more" rather than it just disappearing off their radar.

Gerv
pushing to 2.17.6, since we obviously don't have code yet, and this isn't quite
a show-stopper.
No longer blocks: bz-2.17.5
Whiteboard: [wanted for 2.17.5] → [wanted for 2.17.6]
so, is it, or isn't it?   Looks like we're doing 2.17.6 this weekend because of
the js buglists thing.  Can this go with it?
I seem to recall this being debated back when cclist_accessible was first
implemented, and seems like we chose to do it the way it is now for some reason...
If Gerv's comment #8 describes precisely what happens (including the gist of the
message), I'd suggest WONTFIX, on the ground that it provides decent user feedback.
Having just run a test:

If a person is removed from the CC list of a secure bug, it simply drops off
their radar with no notification. 

I believe that this was the original behaviour designed in by dmose several
years ago, because I helped him test it. See this comment on line 715 of BugMail.pm:

    # XXX - This currently means that if a bug is suddenly given
    # more restrictive permissions, people without those permissions won't
    # see the action of restricting the bug itself; the bug will just 
    # quietly disappear from their radar.

I think this is reasonable, to be honest. It's also safe - someone who doesn't
think too hard about how this might work may well add a sensitive comment at the
same time they remove the person in question from the CC list.

Gerv
ok, so this bug is a WORKSFORME then?  Or did it regress?
I believe the problem this bug is meant to address is that request mail gets
sent to cc: users who are being removed from the cc: list.  I'll do some more
testing to verify.
Whiteboard: [wanted for 2.17.6] → [wanted for 2.17.7]
Nope, not a flag problem.  Flag emails get sent after CC list changes have
already happened.
After updating the CC list (and making all other changes besides),
process_bug.cgi processes bug/process/results.html.tmpl, which processes
bug/process/bugmail.html.tmpl, which calls SendBugMail() (defined in
Bugzilla/Template.pm), which calls Bugzilla::BugMail::send(), which calls
ProcessOneBug(), which calls NewProcessOnePerson(), which calls CanSeeBug() and
doesn't send the mail if the user can't see the bug.

So this isn't a problem with non-Flag emails either.  So this was either a brain
fart or a problem with an earlier version of the code that doesn't exist now. 
Either way, WORKSFORME.
Group: webtools-security
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → WORKSFORME
Whiteboard: [wanted for 2.17.7]
clearing target on worksforme/duplicate/invalid so they'll show up as untriaged
if they get reopened
Target Milestone: Bugzilla 2.18 → ---
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.