653341 - Bug.create() fails to error out if an invalid group is passed

Status: RESOLVED FIXED

(Bugzilla :: WebService, defect)

Description

[Dave Miller [:justdave]]

Like the summary says, Bug.create() fails to error out if an invalid group is passed.  As far as I can tell from the source, this was intentional, so as to not disclose group names.  However, it has the unfortunate side effect of causing automated tools which submit confidential bugs to suddenly start filing them in public when a group name changes (or in this case when the API changed to require group names instead of IDs), which caused an information disclosure on mozilla.org this morning.

Comment 1

[Max Kanat-Alexander]

I suppose this is my "I told you so" moment. :-)

I don't think we need to keep this confidential, but I do agree that it's a high-priority issue that needs to be fixed.

Comment 2

[Dave Miller [:justdave]]

And yeah, I guess I can yell at the webdev guys for using an "undocumented" API (because groups weren't documented at all in 3.6, but it was there in the source), but the name change thing could still bite someone in the future.

Comment 3

[Frédéric Buclin]

We should reject the bug submission, and the error message should be similar to what we display in the web browser, i.e. that "either the group Foo doesn't exist, or you are not allowed to restrict bugs to this group in this product". This way, we don't disclose the existence of the group.

Decreasing severity as Bug.create() works as the doc says.

Comment 4

[Frédéric Buclin]

Attachment: patch, v1

Comment 5

[Frédéric Buclin]

Attachment: patch, v1.1

The comment is now obsolete.

Comment 6

[Max Kanat-Alexander]

Comment on attachment 528905 [details] [diff] [review]
patch, v1.1

Review of attachment 528905 [details] [diff] [review]:

Awesome, looks great. :-)

Comment 7

[Max Kanat-Alexander]

We should also make sure that update() behaves similarly.

Comment 8

[Frédéric Buclin]

Attachment: patch, v1.2

We also have to update the API documentation as we no longer silently ignore illegal group names.

Comment 9

[Frédéric Buclin]

Attachment: patch, v1.3

Still another place to fix, to mention that Bug.create can now return error 120.

Comment 10

[Gervase Markham [:gerv]]

(In reply to comment #1)
> I suppose this is my "I told you so" moment. :-)

Max: I thought you were the one arguing for the "don't reveal group names" behaviour? Or am I confused?

Gerv

Comment 11

[Max Kanat-Alexander]

(In reply to comment #10)
> Max: I thought you were the one arguing for the "don't reveal group names"
> behaviour? Or am I confused?

  No, I heavily believed that Bugzilla should be as explicit and loud as possible about invalid groups.

Comment 12

[Max Kanat-Alexander]

Comment on attachment 529075 [details] [diff] [review]
patch, v1.3

Review of attachment 529075 [details] [diff] [review]:

Looks great. :-)

Comment 13

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/Bug.pm
modified Bugzilla/WebService/Bug.pm
modified Bugzilla/WebService/Constants.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 7808.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.0/
modified Bugzilla/Bug.pm
modified Bugzilla/WebService/Bug.pm
modified Bugzilla/WebService/Constants.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 7593.

Comment 14

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/qa/4.2/
modified t/webservice_bug_create.t
Committed revision 212.

Committing to: bzr+ssh://[e-mail]/bugzilla/qa/4.0/
modified t/webservice_bug_create.t
Committed revision 201.