establish reports and processes for continued auditing of bugzilla security group membership

RESOLVED FIXED

Status

()

bugzilla.mozilla.org
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: glob, Assigned: glob)

Tracking

Production
x86
Mac OS X

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
following on from discussions in bug 812349, we need to establish reports and processes for continued auditing of bugzilla security group membership.

i propose:

1. ensure each security group has at least one administrator
   https://bugzilla.mozilla.org/page.cgi?id=group_admins.html

2. write a report which allows these admins to easily show members of the group
   - currently only visible via editusers, which is clumsy and doesn't
     distinguish between direct and inherited membership

3. send a report of each group's members to the admins on a monthly basis
(Assignee)

Comment 1

5 years ago
the current process for this is when someone's ldap account is disabled, it's included on a nightly report.  if their bmo account is a @mozilla.com address, the account is disabled.  if their bmo account isn't, they are removed from the mozilla-corporation-confidential group.  if i notice they are members of security groups when editing the user, i email security@mozilla.org and they update where appropriate.
This kind of thing should be made available upstream.  Giving admins good tools for managing groups just sounds like a good idea.
(Assignee)

Updated

5 years ago
Assignee: nobody → glob
(Assignee)

Comment 3

5 years ago
Created attachment 683996 [details] [diff] [review]
patch v1

add a group membership report (both html and json output).
Attachment #683996 - Flags: review?(dkl)
Comment on attachment 683996 [details] [diff] [review]
patch v1

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

Looks good and works well. Just a suggestion for group_members.json.tmpl but not a big deal.

r=dkl

::: extensions/BMO/template/en/default/pages/group_members.json.tmpl
@@ +13,5 @@
> +    [% FOREACH member = type.members %]
> +      "[% member.email FILTER json %]"[% "," UNLESS loop.last %]
> +    [% END %]
> +    [% LAST %]
> +  [% END %]

Maybe instead of adding complexity by performing too loops over the same data, instead do earlier:

[% direct = [] %]
[% indirect = [] %]

[% FOREACH type = types %]
  [% direct.push(type) IF type.name == 'direct' %]
  [% indirect.push(type) IF type.name != 'direct' %]
[% END %]

{
  "group": "[% group FILTER json %]",
  "direct": [
  [% FOREACH type = direct %]
    [% FOREACH member = type.members %]
      "[% member.email FILTER json %]"[% "," UNLESS loop.last %]
    [% END %]
    [% LAST %]
  [% END %]
  ][% "," IF indirect.size > 1 %]
  [% IF indirect.size > 1 %]
    "indirect": {
    [% FOREACH type = indirect %]
      "[% type.name FILTER json %]": [
        [% FOREACH member = type.members %]
          "[% member.email FILTER json %]"[% "," UNLESS loop.last %]
        [% END %]
        ]
      [% END %]
    }
  [% END %]
}
Attachment #683996 - Flags: review?(dkl) → review+
Comment on attachment 683996 [details] [diff] [review]
patch v1

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

::: extensions/BMO/template/en/default/pages/group_members.html.tmpl
@@ +61,5 @@
> +            [% type.members.size FILTER html %]
> +          </td>
> +          <td valign="top" width="100%">
> +            [% FOREACH member = type.members %]
> +              <a href="editusers.cgi?action=edit&userid=[% member.id FILTER html %]"

&amp;

FILTER url_quote

@@ +64,5 @@
> +            [% FOREACH member = type.members %]
> +              <a href="editusers.cgi?action=edit&userid=[% member.id FILTER html %]"
> +                 target="_blank">
> +                <span [% 'class="bz_inactive"' UNLESS member.is_enabled %]>
> +                  [% member.name FILTER html %] &lt;[% member.email FILTER html %]&gt;

FILTER email FILTER html

I wonder if any of this stuff can reuse template/en/default/global/user.html.tmpl code ...

::: extensions/BMO/template/en/default/pages/group_members.json.tmpl
@@ +10,5 @@
> +  "direct": [
> +  [% FOREACH type = types %]
> +    [% NEXT UNLESS type.name == "direct" %]
> +    [% FOREACH member = type.members %]
> +      "[% member.email FILTER json %]"[% "," UNLESS loop.last %]

perhaps FILTER email FILTER json ?

@@ +21,5 @@
> +      [% FOREACH type = types %]
> +        [% NEXT IF type.name == "direct" %]
> +        "[% type.name FILTER json %]": [
> +          [% FOREACH member = type.members %]
> +            "[% member.email FILTER json %]"[% "," UNLESS loop.last %]

Same, FILTER email FILTER json ??
Attachment #683996 - Flags: review-
(Assignee)

Comment 6

5 years ago
(In reply to Reed Loden [:reed] from comment #5)
> I wonder if any of this stuff can reuse
> template/en/default/global/user.html.tmpl code ...

i had that initially, however it displays the identity only.  i found it much more useful to display both the real name and email address when auditing group membership.


will fix the other points.  i've already realised that the json filter is from the bzapi extension, so i'll switch to using the js filter.
(Assignee)

Comment 7

5 years ago
Created attachment 685138 [details] [diff] [review]
patch v2
Attachment #685138 - Flags: review?(dkl)
(Assignee)

Comment 8

5 years ago
Created attachment 691930 [details] [diff] [review]
patch v3

adds a "last seen" column (see bug 821341)
Attachment #683996 - Attachment is obsolete: true
Attachment #685138 - Attachment is obsolete: true
Attachment #685138 - Flags: review?(dkl)
Attachment #691930 - Flags: review?(dkl)
Comment on attachment 691930 [details] [diff] [review]
patch v3

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

Looks good and works well. r=dkl

::: extensions/BMO/lib/Reports.pm
@@ +565,2 @@
>          || ThrowUserError('auth_failure', { group  => 'editusers', 
>                                              action => 'run', 

nit: remove whitespace on commit

::: extensions/BMO/template/en/default/pages/group_members.json.tmpl
@@ +14,5 @@
> +  [% SET i = 0 %]
> +  [% FOREACH type = types %]
> +    [% FOREACH member = type.members %]
> +      [% SET i = i + 1 %]
> +      { "login": "[% member.login FILTER email FILTER js %]",

FWIW, in 4.2, we have the json filter which would be better suited here.
Attachment #691930 - Flags: review?(dkl) → review+
(Assignee)

Comment 10

5 years ago
(In reply to David Lawrence [:dkl] from comment #9)
> FWIW, in 4.2, we have the json filter which would be better suited here.

the json filter is injected by the bzapi extension, and shouldn't be used outside of it.
(Assignee)

Comment 11

5 years ago
Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bmo/4.0/
modified extensions/BMO/Extension.pm
modified extensions/BMO/lib/Reports.pm
modified extensions/BMO/template/en/default/hook/reports/menu-end.html.tmpl
modified extensions/BMO/template/en/default/pages/email_queue.html.tmpl
modified extensions/BMO/template/en/default/pages/group_admins.html.tmpl
added extensions/BMO/template/en/default/pages/group_members.html.tmpl
added extensions/BMO/template/en/default/pages/group_members.json.tmpl
modified extensions/BMO/template/en/default/pages/group_membership.html.tmpl
modified extensions/BMO/template/en/default/pages/user_activity.html.tmpl
modified extensions/BMO/web/styles/reports.css
Committed revision 8448.

Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bmo/4.2/
modified extensions/BMO/Extension.pm
modified extensions/BMO/lib/Reports.pm
modified extensions/BMO/template/en/default/hook/reports/menu-end.html.tmpl
modified extensions/BMO/template/en/default/pages/email_queue.html.tmpl
modified extensions/BMO/template/en/default/pages/group_admins.html.tmpl
added extensions/BMO/template/en/default/pages/group_members.html.tmpl
added extensions/BMO/template/en/default/pages/group_members.json.tmpl
modified extensions/BMO/template/en/default/pages/group_membership.html.tmpl
modified extensions/BMO/template/en/default/pages/user_activity.html.tmpl
modified extensions/BMO/web/styles/reports.css
Committed revision 8512.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(In reply to Byron Jones ‹:glob› from comment #10)
> (In reply to David Lawrence [:dkl] from comment #9)
> > FWIW, in 4.2, we have the json filter which would be better suited here.
> 
> the json filter is injected by the bzapi extension, and shouldn't be used
> outside of it.

I concede that we made a customization and the filter is not in the upstream code, but the filter does exist outside of the bzapi extension if we would like to use it.

http://bzr.mozilla.org/bmo/4.2/annotate/head:/Bugzilla/Template.pm#L648

dkl
(Assignee)

Comment 13

5 years ago
https://bugzilla.mozilla.org/page.cgi?id=group_members.html is now live.
Does this bug need to continue to be confidential?

I'd like to reference it in another bug report; I'd like to discuss whether we should make this report available to everyone.

Mozilla does closed bugs as a sad tradeoff of security against openness. But I don't think there's any reason to keep a secret of _who_ can see secure bugs. That information could probably be worked out anyway by someone paying attention. And making the lists public is a useful accountability measure to help us keep secrecy to the necessary minimum.

Gerv
(In reply to Gervase Markham [:gerv] from comment #14)
> I'd like to reference it in another bug report; I'd like to discuss whether
> we should make this report available to everyone.

That's not particularly clear :-) Please read:

"I'd like to reference this bug in another bug, which would discuss whether
we should make the group membership report available to everyone."
 
Gerv
(Assignee)

Comment 16

5 years ago
(In reply to Gervase Markham [:gerv] from comment #14)
> Does this bug need to continue to be confidential?

nope; i wasn't exactly sure where the discussion would take it.  public this bug now is.
Group: mozilla-corporation-confidential
You need to log in before you can comment on or make changes to this bug.