1328900 - Create new group called 'disableusers' that can only edit the bugmail and disabledtext fields of a user

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: Administration, task)

Description

[David Lawrence [:dkl]]

This was we can use the new group instead of giving out 'editusers' which gives a lot of unnecessary powers for just disabling a user.

Once complete, switch over the following people to the new group and remove from editusers:

[e-mail]
[e-mail]
[e-mail]
[e-mail]
[e-mail]
[e-mail]
[e-mail]
[e-mail]
[e-mail]

dkl

Comment 1

[Dylan Hardison [:dylan] (he/him)]

note: allow editing the real name field too.

Comment 2

[David Lawrence [:dkl]]

(In reply to Dylan Hardison [:dylan] from comment #1)
> note: allow editing the real name field too.

Hmm, are we now getting out of the realm of just disabling a user and should name the group something else? A group for editing a user's metadata. We already have a different group for disabling 2fa, etc.

dkl

Comment 3

[David Lawrence [:dkl]]

Attachment: 1328900_1.patch

Creates new disableusers group and allows users in the group to edit the disabledtext, bugmail, and real name attributes of users.

dkl

Comment 4

[Dylan Hardison [:dylan] (he/him)]

Comment on attachment 8824221 [details] [diff] [review]
1328900_1.patch

Review of attachment 8824221 [details] [diff] [review]:
-----------------------------------------------------------------

r-

::: extensions/BugModal/template/en/default/bug_modal/user.html.tmpl
@@ +45,4 @@
>            [% IF user.id %]
>              href="mailto:[% u.email FILTER html %]"
>              onclick="return show_usermenu([% u.id FILTER none %], '[% u.email FILTER js %]',
> +              [% user.in_group('editusers') || user.in_group('disableusers') || user.bless_groups.size > 0 ? "true" : "false" %])"

this would bitrot CSP
(or more properly, CSP has bitrotted this)

you should rebase this patch on top of the CSP patch.

::: template/en/default/global/common-links.html.tmpl
@@ -63,5 @@
>    [% IF user.settings.skin.value != 'Mozilla' && user.settings.skin.value != 'Mozilla-OpenSans' %]
>    [% IF user.login %]
>      <li><span class="separator">| </span><a href="userprefs.cgi">Preferences</a></li>
> -    [% IF user.in_group('tweakparams') || user.in_group('editusers') || user.can_bless
> -          || (Param('useclassification') && user.in_group('editclassifications'))

nothing wrong here, but I will need to remember to optimize user.in_group() more, I just realized how many thousands of times it must be called on the webheads every minute...

Comment 5

[David Lawrence [:dkl]]

Attachment: 1328900_2.patch

(In reply to Dylan Hardison [:dylan] from comment #4)
> Comment on attachment 8824221 [details] [diff] [review]
> 1328900_1.patch
> 
> Review of attachment 8824221 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r-
> 
> ::: extensions/BugModal/template/en/default/bug_modal/user.html.tmpl
> @@ +45,4 @@
> >            [% IF user.id %]
> >              href="mailto:[% u.email FILTER html %]"
> >              onclick="return show_usermenu([% u.id FILTER none %], '[% u.email FILTER js %]',
> > +              [% user.in_group('editusers') || user.in_group('disableusers') || user.bless_groups.size > 0 ? "true" : "false" %])"
> 
> this would bitrot CSP
> (or more properly, CSP has bitrotted this)

Updated to the new style of JS separation provided by the CSP patch.

> you should rebase this patch on top of the CSP patch.
> 
> ::: template/en/default/global/common-links.html.tmpl
> @@ -63,5 @@
> >    [% IF user.settings.skin.value != 'Mozilla' && user.settings.skin.value != 'Mozilla-OpenSans' %]
> >    [% IF user.login %]
> >      <li><span class="separator">| </span><a href="userprefs.cgi">Preferences</a></li>
> > -    [% IF user.in_group('tweakparams') || user.in_group('editusers') || user.can_bless
> > -          || (Param('useclassification') && user.in_group('editclassifications'))
> 
> nothing wrong here, but I will need to remember to optimize user.in_group()
> more, I just realized how many thousands of times it must be called on the
> webheads every minute...

in_group() should only hit the DB a few times as it calls groups() which reads of all of the user's groups from the DB once. We could definitely ensure it is using memcache as well if not already.

dkl

Comment 6

[David Lawrence [:dkl]]

Attachment: 1328900_3.patch

Fixed some minor bit-rot after CSP landed.

dkl

Comment 7

[:glob ✱]

Comment on attachment 8831339 [details] [diff] [review]
1328900_3.patch

clearing r? on fixed bug