Grouper integration with user admin status

Summary:
Historically, making/revoking an admin requires going through the Changes UI. We want to do this on Grouper so that we can say things like, "All members of Team A should be an admin".

This diff creates a recurring task every 1 minute that connects to Grouper, gets the list of admin users, and makes sure that those users and only those users have admin access.

This also updates the web UI to remove the "Make Admin" button from the users page.

The API endpoint that can manipulate the admin status of the user is purposely kept for compatability reasons. It is not a serious risk, as the Grouper sync will override any changes every 1 minute.

Test Plan:
unit test, manual testing on the VM

screenshot of UI change:
{F492038}

Reviewers: anupc

Reviewed By: anupc

Subscribers: changesbot, herb, kylec

Tags: #changes_ui

Differential Revision: https://tails.corp.dropbox.com/D219947

changes/config.py

@@ -87,6 +87,9 @@ def create_app(_read_config=True, **config):
     app.config['SQLALCHEMY_RECORD_QUERIES'] = True
 
     app.config['REDIS_URL'] = 'redis://localhost/0'
+    app.config['GROUPER_API_URL'] = 'localhost:80'
+    app.config['GROUPER_PERMISSIONS_ADMIN'] = 'changes.prod.admin'
+    app.config['GROUPER_EXCLUDED_ROLES'] = ['np-owner']
     app.config['DEBUG'] = True
     app.config['HTTP_PORT'] = 5000
     app.config['SEND_FILE_MAX_AGE_DEFAULT'] = 0
@@ -184,6 +187,7 @@ def create_app(_read_config=True, **config):
         Queue('events', routing_key='events'),
         Queue('default', routing_key='default'),
         Queue('repo.sync', Exchange('fanout', 'fanout'), routing_key='repo.sync'),
+        Queue('grouper.sync', routing_key='grouper.sync'),
         Broadcast('repo.update'),
     )
     app.config['CELERY_ROUTES'] = {
@@ -207,6 +211,10 @@ def create_app(_read_config=True, **config):
             'queue': 'repo.sync',
             'routing_key': 'repo.sync',
         },
+        'sync_grouper': {
+            'queue': 'grouper.sync',
+            'routing_key': 'grouper.sync',
+        },
         'sync_repo': {
             'queue': 'repo.sync',
             'routing_key': 'repo.sync',
@@ -263,6 +271,10 @@ def create_app(_read_config=True, **config):
             'task': 'check_repos',
             'schedule': timedelta(minutes=2),
         },
+        'sync-grouper': {
+            'task': 'sync_grouper',
+            'schedule': timedelta(minutes=1),
+        },
         'aggregate-flaky-tests': {
             'task': 'aggregate_flaky_tests',
             # Hour 7 GMT is midnight PST, hopefully a time of low load
@@ -827,6 +839,7 @@ def configure_jobs(app):
     )
     from changes.jobs.sync_artifact import sync_artifact
     from changes.jobs.sync_build import sync_build
+    from changes.jobs.sync_grouper import sync_grouper
     from changes.jobs.sync_job import sync_job
     from changes.jobs.sync_job_step import sync_job_step
     from changes.jobs.sync_repo import sync_repo
@@ -844,6 +857,7 @@ def configure_jobs(app):
     queue.register('run_event_listener', run_event_listener)
     queue.register('sync_artifact', sync_artifact)
     queue.register('sync_build', sync_build)
+    queue.register('sync_grouper', sync_grouper)
     queue.register('sync_job', sync_job)
     queue.register('sync_job_step', sync_job_step)
     queue.register('sync_repo', sync_repo)

changes/jobs/sync_grouper.py

@@ -0,0 +1,78 @@
+import logging
+
+from flask import current_app
+from groupy.client import Groupy
+from typing import Iterable, Set  # NOQA
+
+from changes.config import db, statsreporter
+from changes.models.user import User
+
+
+logger = logging.getLogger('grouper.sync')
+
+
+def sync_grouper():
+    # type: () -> None
+    """This function is meant as a Celery task. It connects to Grouper, gets
+    all users who should be admin, and makes sure that those users and only
+    those users are admin.
+    """
+    try:
+        admin_emails = _get_admin_emails_from_grouper()
+        _sync_admin_users(admin_emails)
+    except Exception:
+        logger.exception("An error occurred during Grouper sync.")
+        statsreporter.stats().set_gauge('grouper_sync_error', 1)
+        raise
+    else:
+        statsreporter.stats().set_gauge('grouper_sync_error', 0)
+
+
+def _get_admin_emails_from_grouper():
+    # type: () -> Set[str]
+    """This function connects to Grouper and retrieves the list of emails of
+    users with admin permission.
+
+    Returns:
+        set[basestring]: a set of emails of admin users
+    """
+    grouper_api_url = current_app.config['GROUPER_API_URL']
+    grouper_permissions_admin = current_app.config['GROUPER_PERMISSIONS_ADMIN']
+    grclient = Groupy(grouper_api_url)
+    groups = grclient.permissions.get(grouper_permissions_admin).groups
+
+    admin_users = set()
+    for _, group in groups.iteritems():
+        for email, user in group.users.iteritems():
+            if user['rolename'] not in current_app.config['GROUPER_EXCLUDED_ROLES']:
+                admin_users.add(email)
+    return admin_users
+
+
+def _sync_admin_users(admin_emails):
+    # type: (Iterable[str]) -> None
+    """Take a look at the Changes user database. Every user with email in
+    `admin_emails` should become a Changes admin, and every user already
+    an admin whose email is not in `admin_emails` will have their
+    admin privileges revoked.
+
+    Args:
+        admin_emails (iterable[basestring]): an iterable of usernames of
+            people who should be admin.
+    """
+    # revoke access for people who should not have admin access
+    User.query.filter(
+        ~User.email.in_(admin_emails),
+        User.is_admin.is_(True),
+    ).update({
+        'is_admin': False,
+    }, synchronize_session=False)
+
+    # give access for people who should have access
+    User.query.filter(
+        User.email.in_(admin_emails),
+        User.is_admin.is_(False),
+    ).update({
+        'is_admin': True,
+    }, synchronize_session=False)
+    db.session.commit()

conftest.py

@@ -35,6 +35,7 @@ def app(request, session_config):
         TESTING=True,
         SQLALCHEMY_DATABASE_URI='postgresql:///' + session_config['db_name'],
         REDIS_URL='redis://localhost/' + session_config['redis_db'],
+        GROUPER_API_URL='https://localhost/',
         WEB_BASE_URI='http://example.com',
         INTERNAL_BASE_URI='http://changes-int.example.com',
         REPO_ROOT='/tmp',

requirements.txt

@@ -17,6 +17,8 @@ flask-debugtoolbar>=0.9.0,<0.10.0
 flask-mail>=0.9.0,<0.10.0
 flask-restful>=0.2.10,<0.2.11
 flask-sqlalchemy>=1.0,<1.1
+# 0.1.0 had a ValueError on making any kind of query
+groupy==0.0.9
 lxml>=3.2.3,<3.3.0
 kazoo==2.2.1
 raven>=5.3.0,<5.4.0

setup.py

@@ -45,6 +45,7 @@
     'flask-mail>=0.9.0,<0.10.0',
     'flask-restful>=0.2.10,<0.2.11',
     'flask-sqlalchemy>=1.0,<1.1',
+    'groupy==0.0.9', # 0.1.0 had a ValueError on making any kind of query
     'lxml>=3.2.3,<3.3.0',
     'kazoo==2.2.1',
     'raven>=5.3.0,<5.4.0',

tests/changes/jobs/test_sync_grouper.py

@@ -0,0 +1,114 @@
+import mock
+import pytest
+
+from flask import current_app
+
+from changes.config import db
+from changes.jobs.sync_grouper import (
+    _get_admin_emails_from_grouper, _sync_admin_users, sync_grouper
+)
+from changes.testutils import TestCase
+
+
+class SyncGrouperAdminTestCase(TestCase):
+
+    def test_get_admin_emails_from_grouper_correct(self):
+        mock_groupy = mock.MagicMock()
+        mock_groupy.permissions.get().groups = {
+            'group1': mock.MagicMock(),
+            'group2': mock.MagicMock(),
+        }
+        mock_groupy.permissions.get().groups['group1'].users = {
+            'user1@dropbox.com': {'rolename': 'owner'},
+            'user2@dropbox.com': {'rolename': 'member'},
+            'user3@dropbox.com': {'rolename': 'member'},
+        }
+        mock_groupy.permissions.get().groups['group2'].users = {
+            'user3@dropbox.com': {'rolename': 'member'},
+            'user4@dropbox.com': {'rolename': 'member'},
+        }
+        with mock.patch('changes.jobs.sync_grouper.Groupy') as mock_groupy_init:
+            mock_groupy_init.return_value = mock_groupy
+            admin_usernames = _get_admin_emails_from_grouper()
+        mock_groupy_init.assert_called_once_with(
+            current_app.config['GROUPER_API_URL'])
+
+        # can't use assert_called_once_with because we called it 3 times when
+        # setting up the mock object itself
+        mock_groupy.permissions.get.assert_called_with(
+            current_app.config['GROUPER_PERMISSIONS_ADMIN'])
+        assert admin_usernames == set([
+            'user1@dropbox.com',
+            'user2@dropbox.com',
+            'user3@dropbox.com',
+            'user4@dropbox.com',
+        ])
+
+    def test_get_admin_emails_from_grouper_np_owner(self):
+        mock_groupy = mock.MagicMock()
+        mock_groupy.permissions.get().groups = {
+            'group1': mock.MagicMock(),
+            'group2': mock.MagicMock(),
+        }
+        mock_groupy.permissions.get().groups['group1'].users = {
+            'user1@dropbox.com': {'rolename': 'np-owner'},
+            'user2@dropbox.com': {'rolename': 'member'},
+            'user3@dropbox.com': {'rolename': 'member'},
+        }
+        mock_groupy.permissions.get().groups['group2'].users = {
+            'user3@dropbox.com': {'rolename': 'member'},
+            'user4@dropbox.com': {'rolename': 'member'},
+        }
+        with mock.patch('changes.jobs.sync_grouper.Groupy') as mock_groupy_init:
+            mock_groupy_init.return_value = mock_groupy
+            admin_usernames = _get_admin_emails_from_grouper()
+        mock_groupy_init.assert_called_once_with(
+            current_app.config['GROUPER_API_URL'])
+        mock_groupy.permissions.get.assert_called_with(
+            current_app.config['GROUPER_PERMISSIONS_ADMIN'])
+        assert admin_usernames == set([
+            'user2@dropbox.com',
+            'user3@dropbox.com',
+            'user4@dropbox.com',
+        ])
+
+    def test_sync_admin_users_correct(self):
+        admin_user1 = self.create_user(
+            email='user1@dropbox.com', is_admin=True)
+        admin_user2 = self.create_user(
+            email='user2@dropbox.com', is_admin=True)
+        admin_user3 = self.create_user(
+            email='user3@dropbox.com', is_admin=True)
+
+        user4 = self.create_user(email='user4@dropbox.com', is_admin=False)
+        user5 = self.create_user(email='user5@dropbox.com', is_admin=False)
+
+        _sync_admin_users(
+            set([u'user2@dropbox.com', u'user3@dropbox.com', u'user5@dropbox.com']))
+        db.session.expire_all()
+
+        assert admin_user1.is_admin is False
+
+        assert admin_user2.is_admin is True
+
+        assert admin_user3.is_admin is True
+
+        assert user4.is_admin is False
+
+        assert user5.is_admin is True
+
+    def test_sync_grouper_stats_succeeded(self):
+        with mock.patch('changes.jobs.sync_grouper._get_admin_emails_from_grouper'):
+            with mock.patch('changes.jobs.sync_grouper._sync_admin_users'):
+                with mock.patch('changes.jobs.sync_grouper.statsreporter') as mock_statsreporter:
+                    sync_grouper()
+        mock_statsreporter.stats().set_gauge.assert_called_once_with('grouper_sync_error', 0)
+
+    def test_sync_grouper_stats_failure(self):
+        with mock.patch('changes.jobs.sync_grouper._get_admin_emails_from_grouper') as mock_call:
+            with mock.patch('changes.jobs.sync_grouper._sync_admin_users'):
+                with mock.patch('changes.jobs.sync_grouper.statsreporter') as mock_statsreporter:
+                    mock_call.side_effect = Exception
+                    with pytest.raises(Exception):
+                        sync_grouper()
+        mock_statsreporter.stats().set_gauge.assert_called_once_with('grouper_sync_error', 1)

webapp/pages/admin_page.js

@@ -2,7 +2,6 @@ import React, { PropTypes } from 'react';
 
 import APINotLoaded from 'es6!display/not_loaded';
 import SectionHeader from 'es6!display/section_header';
-import { Button } from 'es6!display/button';
 import { ChangesPage, APINotLoadedPage } from 'es6!display/page_chrome';
 import ChangesLinks from 'es6!display/changes/links';
 import * as FieldGroupMarkup from 'es6!display/field_group';
@@ -190,7 +189,7 @@ let AdminPage = React.createClass({
 
     _.each(users, user => {
       let params = {};
-      let action = user.isAdmin ? 'Remove Admin' : 'Make Admin';
+      let isAdmin = user.isAdmin ? 'Yes' : 'No';
       params['is_admin'] = !user.isAdmin;
       let endpoint = `/api/0/users/${user.id}/`;
       let post = <Request
@@ -199,9 +198,9 @@ let AdminPage = React.createClass({
                     endpoint={endpoint}
                     method="post"
                     params={params}>
-                      <Button type="blue">
-                        <span>{action}</span>
-                      </Button>
+                      <span>
+                        {isAdmin}
+                      </span>
                  </Request>;
 
       rows.push([