177786 - make it obvious from error page that users can be added to bugs in a group

Status: NEW

(Bugzilla :: User Interface, defect)

Description

[Asa Dotzler [:asa]]

One problem I've seen on bmo is that developers and qa don't realize that they
can be added to a restricted bug without being added to the restricted group. A
developer that doesn't work on security regularly might assume that it's not
worth going through the trouble of getting added to the security group just to
fulfill one review request on one security sensitive bug. 

It's not obvious from the error "You are not authorized to access bug #168316.
To see this bug, you must first log in to an account with the appropriate
permissions." that a user can be added to a bug without being added to the full
group. 

It would be helpful to advertise the flexibility of groups. One way would be to
add some text like "If you have good reason to see this specific bug please
email <bug assignee> asking to be added to the cc: list which grants non-group
members access to the specific bug" (that's horrible wording but you get the idea). 

Maybe this isn't necessary because eventually everyone will learn that groups
aren't an all or nothing deal (I'm not so sure they will). But in the mean time
I've seen enough cases of people saying "I'm not in that group so I couldn't see
the bug" to think that we could do something in the error page to let them know
that's not the case.

Comment 1

[timeless]

i'd suggest that they be offered the qa contact instead of the assignee. it
might also be interesting for groups to have a contact field which might be listed.

Comment 2

[Max Kanat-Alexander]

Oh, this is SO easy, I could do this.

Comment 3

[Stephen Lee]

(In reply to comment #0)
> [...] getting added to the security group just to
> fulfill one review request on one security sensitive bug. 

One might question how/why non-group members can receive review requests for 
bugs they cannot see. Perhaps also worth someone filing a separate bug about 
that?

Comment 4

[Jeffrey Hulten]

I have started working on this.  The language I chose was:

    You are not authorized to access [% terms.bug %] #[% bug_id FILTER html %].
    <br />
    Email [% assignee_email %] or [% qa_email %], ask to be added to the 
    cc: list, and give the reason you need access to this bug.

Comment 5

[Jeffrey Hulten]

Attachment: Patch to add bug owner name and email to error page.

First try.  Let me know what you think.

Comment 6

[Jeffrey Hulten]

Changed the wording slightly.  This is largely because the QA contact is not required to be set, so might not be the best choice.

Comment 7

[timeless]

Comment on attachment 230857 [details] [diff] [review]
Patch to add bug owner name and email to error page.

1. Please pass user objects instead of strings.
2. there's a Param for QAContact so that should be used, but more importantly, from the template's perspective, it doesn't matter if the QAContact param is enabled, what matters is whether a bug *has* a QAContact, so if the QAContact is null, the template shouldn't suggest contacting the QAContact (param enabled or not).
3. Note that I suggested the QAContact instead of the assignee because at the time qacontacts tended to be real people who were often QA's for hundreds of bugs. Whereas assignees tended to be guessable by area, i.e. if I find out that a bug exists and that I can't view it, I can guess something about it merely by being told who the assignee is. QAContacts historically didn't tell me that much, unfortunately with today's model where the QAContact is per component being told the QAContact discloses the component, which may or may not be acceptable.
4. I still kinda stand by my view that groups should have a contact field. Preferably multiple groups should have the same contact, otherwise people can use that information to guess too much about a bug.
5. If a bug is set not to allow cc'd people to get bugmails, do we want to offer this?

Comment 8

[Frédéric Buclin]

Comment on attachment 230857 [details] [diff] [review]
Patch to add bug owner name and email to error page.

A more important reason to deny review is that this patch doesn't respect group visibility restrictions, see the 'usevisibilitygroups' and 'strict_isolation' parameters in the "Group Security" panel.

Comment 9

[Jeffrey Hulten]

Per my conversation with LpSolit:

The message will not provide names or email for the assignee or QA contact.
The message will not show up if strict_isolation is turned on.

Comment 10

[Jeffrey Hulten]

Attachment: Patch to add message to contact bug owner or QA contact to get access to a bug. No manes or emails are exposed.

Comment 11

[timeless]

wait. so now you're telling me to email an assignee or qa contact but i have no idea who to email? how does this help me?

Comment 12

[Jeffrey Hulten]

(In reply to comment #11)
> wait. so now you're telling me to email an assignee or qa contact but i have no
> idea who to email? how does this help me?
> 

This was all part of an IRC discussion Friday.  If the user does not have access to the bug, then the user does not get to know anything about the bug other than that it exists.  This wording is intended to deal with the original reported issue:

"that developers and qa don't realize that they can be added to a restricted bug without being added to the restricted group"

If you have come across the bug you have a chance of knowing who it belongs to from the context you heard about it from.  Now this does not help in the case of blocking or dependant bugs, but that is the tradeoff.

I also suggested a form to request access that puts a comment in the log and limits a user to one request per ticket, but that was rejected as a spam (either in the commercial or the excessive unwanted requests sense) trap.

If you have an alternate suggestion, let me know.

Comment 13

[Frédéric Buclin]

Comment on attachment 231139 [details] [diff] [review]
Patch to add message to contact bug owner or QA contact to get access to a bug.  No manes or emails are exposed.

>+    [% IF (!Param('strict_isolation')) %]

What I said on IRC is that based on restrictions which may apply, displaying the email addresses of the assignee or QA contact could be a bad idea. But I never said to not display the message if strict_isolation was in use. Moreover, instead of suggesting to send an email, you should simply suggest them to *contact* the assignee. Probably each installation has its own policy on how to contact developers or module owners. Nit: could we try to make this message shorter?

Comment 14

[Jeffrey Hulten]

(In reply to comment #13)
> (From update of attachment 231139 [details] [diff] [review] [edit])
> >+    [% IF (!Param('strict_isolation')) %]
> 
> What I said on IRC is that based on restrictions which may apply, displaying
> the email addresses of the assignee or QA contact could be a bad idea. But I
> never said to not display the message if strict_isolation was in use. Moreover,
> instead of suggesting to send an email, you should simply suggest them to
> *contact* the assignee. Probably each installation has its own policy on how to
> contact developers or module owners. Nit: could we try to make this message
> shorter?
> 

Changed wording to 'Contact' instead of 'Email'.

Dropped 'give reason why you need access' to shorten.

Now.  Parameters...

Here I will need some help as I am not familiar with all the parameters within BZ.  This is part of the reason I am starting to pitch in so that I understand the codebase and how people use it.

strict_isolation:

The description reads: "Don't allow users to be assigned to, be qa-contacts on, be added to CC list, or make or remove dependencies involving any bug that is in a product on which that user is forbidden to edit."

This indicates to me that the person requesting access will not be able to be added to the CC list, and therefore we should not offer them the option.  The only way to get access to the bug is to be in the group that can edit it.  Is this correct?  If not, what am I not understanding?

usevisibilitygroups:

"Do you wish to restrict visibility of users to members of specific groups?"

So for this I see, digging through the code, that there is a method on the User object called can_see_user.  If I call this with the assignee and qa contact user objects and include or replace with 'Assignee' or 'QA Contact' if they are not allowed to see that user will that meet the requirement?

Are there any other parameters I need to know about?

Comment 15

[Frédéric Buclin]

Bugzilla 3.2 is restricted to security bugs only. Moreover, this bug is either assigned to nobody or got no traction for several months now. Rather than retargetting it at each new release, I'm clearing the target milestone and the bug will be retargetted to some sensible release when someone starts fixing this bug for real (Bugzilla 3.8 more likely).