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

RESOLVED FIXED

Status

()

RESOLVED FIXED
2 years ago
9 months ago

People

(Reporter: dkl, Assigned: dkl)

Tracking

Production

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

2 years ago
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:

ludovic@mozilla.com
pradcliffe+bugzilla@mozilla.com
dwilliams@mozilla.com
rchilds@mozilla.com
achavez@mozilla.com
jlaz@mozilla.com
vhua@mozilla.com
kferrando@mozilla.com
sespinoza@mozilla.com

dkl
note: allow editing the real name field too.
(Assignee)

Comment 2

2 years ago
(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
Flags: needinfo?(dylan)
(Assignee)

Updated

2 years ago
Flags: needinfo?(dylan)
(Assignee)

Comment 3

2 years ago
Created attachment 8824221 [details] [diff] [review]
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
Attachment #8824221 - Flags: review?(dylan)
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...
Attachment #8824221 - Flags: review?(dylan) → review-
(Assignee)

Comment 5

2 years ago
Created attachment 8828884 [details] [diff] [review]
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
Attachment #8824221 - Attachment is obsolete: true
Attachment #8828884 - Flags: review?(dylan)
(Assignee)

Comment 6

2 years ago
Created attachment 8831339 [details] [diff] [review]
1328900_3.patch

Fixed some minor bit-rot after CSP landed.

dkl
Attachment #8828884 - Attachment is obsolete: true
Attachment #8828884 - Flags: review?(dylan)
Attachment #8831339 - Flags: review?(dylan)
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months ago
Resolution: --- → FIXED
Comment on attachment 8831339 [details] [diff] [review]
1328900_3.patch

clearing r? on fixed bug
Attachment #8831339 - Flags: review?(dylan)
You need to log in before you can comment on or make changes to this bug.