931242 - More fine grained permissions for access to sensitive data

Status: VERIFIED FIXED

(Socorro :: Webapp, task)

Description

[Peter Bengtsson [:peterbe]]

At the moment, all logic for whether you can access certain things (emails, URLs, Admin UI, admin UI actions) is based on whether you succeeded to log in or not. 

Instead we should allow anybody to log in but all access should be blocked by permissions. 

We also need to attach permissions to groups and groups to people. For this we need an Admin UI.

Comment 1

[Peter Bengtsson [:peterbe]]

Attachment: List of all users

Comment 2

[Peter Bengtsson [:peterbe]]

Attachment: Editing a single user

Comment 3

[Peter Bengtsson [:peterbe]]

I need some brainstorming guidance on this now...

I've thrown out all the LDAP stuff and built an admin view for managing users which makes it possible to promote a "nobody" user to belong to a group or groups. These groups then map to permissions and it's these permissions we refer to within the business logic throughout the site. E.g.::

 {% if request.user.has_perm('view_email') %}
    Email: {{ crash.email }}
 {% endif %}

What I need help with is the initial set of permissions and groups that we're going to ship and attach the business logic to. Here's what I've got so far::

 PERMISSIONS = {
    'view_pii': 'View Personal Identifyable Information',
    'view_rawdump': 'View Raw Dumps',
 }

 GROUPS = (
    ('Hackers', ('view_pii', 'view_rawdump')),
 )

Hopefully the names of that will make sense. This basically makes 1 group available called "Hackers". 
Not a great name, is it? 
Are there other group configurations you can imagine we should ship by default?

As for the Admin UI, I made ALL of the admin related views and actions depend on you being a superuser so that won't depend on groups. Is that good enough?

Comment 4

[Robert Kaiser]

Right now, IMHO, we should have an "Admin" group for people who can access the admin UI, a "PII" group (for lack of a better name) that can view URLs and emails, and a "Rawdump" group (we could name it "developers" if you like but I find it confusing if the group name doesn't tell you which kind of permissions is behind it) for people who can access the raw dump. The latter may still need to correspond to an LDAP flag as AFAIK Apache is guarding that access ultimately right now.

Comment 5

[Benjamin Smedberg]

We will want a group for "can view exploitability results".

Comment 6

[Peter Bengtsson [:peterbe]]

(In reply to Robert Kaiser (:[e-mail]) from comment #4)
> Right now, IMHO, we should have an "Admin" group for people who can access
> the admin UI, a "PII" group (for lack of a better name) that can view URLs
> and emails, and a "Rawdump" group (we could name it "developers" if you like
> but I find it confusing if the group name doesn't tell you which kind of
> permissions is behind it) for people who can access the raw dump. The latter
> may still need to correspond to an LDAP flag as AFAIK Apache is guarding
> that access ultimately right now.

Thanks. That helps. 
But do you mind that I use a "is superuser" attribute instead of doing group membership for accessing any /admin/* URLs.

Comment 7

[Peter Bengtsson [:peterbe]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #5)
> We will want a group for "can view exploitability results".

Excellent idea! 
That's a great permission name. Any preference what we should call this group?

Comment 8

[Robert Kaiser]

(In reply to Peter Bengtsson [:peterbe] from comment #6)
> But do you mind that I use a "is superuser" attribute instead of doing group
> membership for accessing any /admin/* URLs.

I don't care too much as long as we can have multiple people who have access to the admin UI.

Comment 9

[Peter Bengtsson [:peterbe]]

(In reply to Robert Kaiser (:[e-mail]) from comment #8)
> (In reply to Peter Bengtsson [:peterbe] from comment #6)
> > But do you mind that I use a "is superuser" attribute instead of doing group
> > membership for accessing any /admin/* URLs.
> 
> I don't care too much as long as we can have multiple people who have access
> to the admin UI.

Being a supervisor is like being root on a UNIX systems. No questions asked basically. But like with root, you can't say, for example, "you need this or that permission to write to /some/special/place"

Comment 10

[Peter Bengtsson [:peterbe]]

Attachment: Screen Shot 2013-11-04 at 3.15.37 PM.png

So, instead of nailing the actual groups here and now I built an admin UI thing so you can CRUD groups instead. 

The permissions are more or less hardcoded because that's what we use in the business logic and templates to protect things.

Comment 11

[Peter Bengtsson [:peterbe]]

Attachment: Github PR

Comment 12

[Peter Bengtsson [:peterbe]]

CC'ing Adrian because I think you would be the best person to review this monster.

Comment 13

[Brandon Burton [:solarce]]

Once this has landed webops needs to do 935594

Comment 14

[[github robot]]

Commit pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/e13bd1df32ef36c4981c0f5560e43faa5d2bd4af
fixes bug 931242 - more fine grained permissions on data access, r=adrian

Comment 15

[Peter Bengtsson [:peterbe]]

Steps to "reproduce":

1. Go to the home page
2. Click "Sign-in" with Persona. Use a non-mozilla email address that you know is not connected to LDAP. 
3. Click on a signature from the Top Crashers, click Report tab. Click on a crash. 

Expect:

* You should be able to sign in
* You should have a link in the footer that says "Your permissions". Click that. Expect to see No, No, No on all permissions. 
* When looking at a crash you should not be able to see any PII. 

The next step would be for someone (e.g. peterbe) to make this user belong to a group with the appropriate permissions and now PII should re-appear and the "Your permissions" page should change.

Comment 16

[[DEACTIVATED] Adrian Gaudebert]

One way to check the PII permission would be to go to super search, add a new line and try to use the "email" or "url" fields. If they do not appear, that means you don't have the permission to see PII.

Comment 17

[Matt Brandt [:mbrandt]]

QA verified on stage - 
- /permissions/ link exists in the footer
- https://crash-stats.allizom.org/permissions/ the new permissions panel correctly updates yes/no for the 3 permission levels (exploitability reports, PII, and Raw Dumps)
- Users without permission are not given the filter options in /search/ to query by email or url. The inverse is true; if the account has permission the user will see these options
- Top crasher reports only show the url tab to users with the correct privilege constellation
- individual crashes allow the downloading of raw dumps only to users with those privileges

Not verified - permissions groups other than the "Hackers" group work.

Comment 18

[Matt Brandt [:mbrandt]]

Bumping to QA verified - I created several other groups that represent the available permissions constellations and lgtm.