282574 - use the new "auth_failure" error message for all authentication failures

Status: RESOLVED FIXED

(Bugzilla :: Administration, task)

Description

[Frédéric Buclin]

All edit*.cgi files already use the new ThrowUserError("auth_failure",...) error
message, see bug 265898. We should extend this to all *.cgi files in order to
prevent the duplication of error messages in user-error.html.tmpl. Concerned
files are:

buglist.cgi, doeditparams.cgi, quips.cgi and sanitycheck.cgi.

Comment 1

[Frédéric Buclin]

Attachment: patch, v1

This patch also includes attachment.cgi, which I forgot to mention in my
previous comment.

Comment 2

[Marc Schumann [:Wurblzap]]

Comment on attachment 174576 [details] [diff] [review]
patch, v1

r=wurblzap by inspection, assuming you checked the remaining code for the
removed error codes :)

Comment 3

[Shane H. W. Travis]

Comment on attachment 174576 [details] [diff] [review]
patch, v1

Heh... I hit a midair with Marc.

Frederic, your change of attachment_access_denied reveals that there *is* an
'insider group', which some sites may want left unpublicized. I was going to
suggest that you remove the 'group' from the faulire message... but then on
looking at the error code I see that this would create the following message:

"Sorry, and so you aren't allowed to..." 

The r- is because you are revealing too much information. Somehow, you have to
make it so that it is okay not to send a group; that can be part of another bug
(which is then a prerequisite to this one) or you can just fix it here. The
solution is simple:

-    and so you aren't allowed to
+    [% if GROUP %]and so [% END %]
+    you aren't allowed to

should do it. I'll leave it to you how you want to address it. r? me when
you're done.

Comment 4

[Frédéric Buclin]

Attachment: patch, v2

some improvement here...

Comment 5

[Shane H. W. Travis]

Comment on attachment 176618 [details] [diff] [review]
patch, v2

This looks good, and you even caught a condition I hadn't. Nic going!

One small nit: since you're changing the line anyway, I'd like to see 'allowed'
morph into 'authorized'. It's what a lot of the error messages you're deleting
used, and IMHO it's more clear about why someone is being refused access.

r+ by inspection.

Comment 6

[Frédéric Buclin]

Attachment: patch, v2.1

replace "aren't allowed" by "are not authorized"

Comment 7

[Shane H. W. Travis]

Comment on attachment 176627 [details] [diff] [review]
patch, v2.1

Beautiful. Full, unreserved approval.

Comment 8

[Shane H. W. Travis]

Checking in attachment.cgi;
/cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v  <--  attachment.cgi
new revision: 1.74; previous revision: 1.73
done
Checking in buglist.cgi;
/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v  <--  buglist.cgi
new revision: 1.285; previous revision: 1.284
done
Checking in doeditparams.cgi;
/cvsroot/mozilla/webtools/bugzilla/doeditparams.cgi,v  <--  doeditparams.cgi
new revision: 1.32; previous revision: 1.31
done
Checking in quips.cgi;
/cvsroot/mozilla/webtools/bugzilla/quips.cgi,v  <--  quips.cgi
new revision: 1.26; previous revision: 1.25
done
Checking in sanitycheck.cgi;
/cvsroot/mozilla/webtools/bugzilla/sanitycheck.cgi,v  <--  sanitycheck.cgi
new revision: 1.86; previous revision: 1.85
done
Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-
error.html.tmpl,v  <--  user-error.html.tmpl
new revision: 1.101; previous revision: 1.100
done