allow project admins to create projects

Summary: project admins will be able to create projects that match their patterns

Test Plan: unit tests

Reviewers: kylec

Reviewed By: kylec

Subscribers: changesbot, anupc, wwu

Tags: #changes_ui

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

changes/api/auth.py

@@ -5,6 +5,7 @@
 from functools import wraps
 from typing import Optional  # NOQA
 
+from changes.api.base import error
 from changes.models.plan import Plan
 from changes.models.project import Project
 from changes.models.step import Step
@@ -111,6 +112,21 @@ def get_project_slug_from_step_id(*args, **kwargs):
     return step.plan.project.slug
 
 
+def user_has_project_permission(user, project_slug):
+    # type: (User, str) -> bool
+    """
+    Given a user and a project slug, determine if the user has admin permission
+    for this project.
+    """
+    if user.is_admin:
+        return True
+    if user.project_permissions is not None:
+        for p in user.project_permissions:
+            if fnmatch(project_slug, p):
+                return True
+    return False
+
+
 def requires_project_admin(get_project_slug):
     """
     Require an authenticated user with project admin privileges.
@@ -130,25 +146,14 @@ def decorator(method):
         def wrapped(self, *args, **kwargs):
             user = get_current_user()
             if user is None:
-                return self.respond({
-                    'error': 'Not logged in.'
-                }, status_code=401)
-            if user.is_admin:
-                # global admins are automatically project admins
+                return error('Not logged in', http_code=401)
+            try:
+                slug = get_project_slug(self, *args, **kwargs)
+            except ResourceNotFound as e:
+                return error('{}'.format(e), http_code=404)
+            if user_has_project_permission(user, slug):
                 return method(self, *args, **kwargs)
-            if user.project_permissions is not None:
-                try:
-                    slug = get_project_slug(self, *args, **kwargs)
-                except ResourceNotFound as e:
-                    return self.respond({
-                        'error': '{}'.format(e)
-                    }, status_code=404)
-                for p in user.project_permissions:
-                    if fnmatch(slug, p):
-                        return method(self, *args, **kwargs)
-            return self.respond({
-                'error': 'User does not have access to this project.'
-            }, status_code=403)
+            return error('User does not have access to this project.', http_code=403)
         return wrapped
     return decorator
 

changes/api/project_index.py

@@ -7,8 +7,8 @@
 from sqlalchemy.orm import joinedload
 from sqlalchemy.sql import func
 
-from changes.api.base import APIView
-from changes.api.auth import requires_admin
+from changes.api.base import APIView, error
+from changes.api.auth import get_current_user, user_has_project_permission
 from changes.config import db, statsreporter
 from changes.constants import Result, Status, ProjectStatus
 from changes.lib import build_type
@@ -188,21 +188,26 @@ def get(self):
 
             return self.respond(context, serialize=False)
 
-    @requires_admin
     def post(self):
+        user = get_current_user()
+        if user is None:
+            return error('Not logged in.', http_code=401)
         args = self.post_parser.parse_args()
 
         slug = str(args.slug or args.name.replace(' ', '-').lower())
 
+        if not user_has_project_permission(user, slug):
+            return error('User does not have permission to create a project with slug {}.'.format(slug), http_code=403)
+
         match = Project.query.filter(
             Project.slug == slug,
         ).first()
         if match:
-            return '{"error": "Project with slug %r already exists"}' % (slug,), 400
+            return error('Project with slug {} already exists.'.format(slug), http_code=400)
 
         repository = Repository.get(args.repository)
         if repository is None:
-            return '{"error": "Repository with url %r does not exist"}' % (args.repository,), 400
+            return error('Repository with url {} does not exist.'.format(args.repository), http_code=400)
 
         project = Project(
             name=args.name,

tests/changes/api/test_auth.py

@@ -7,6 +7,7 @@
     get_project_slug_from_project_id, get_project_slug_from_plan_id,
     get_project_slug_from_step_id,
     ResourceNotFound, requires_project_admin,
+    user_has_project_permission,
 )
 from changes.testutils import TestCase
 
@@ -42,6 +43,32 @@ def test_get_project_slug_from_step_id_not_found(self):
 
 
 class ProjectAdminTestCase(TestCase):
+    def test_global_admin(self):
+        user = self.create_user(email='user1@example.com', is_admin=True)
+        assert user_has_project_permission(user, 'other:project-a')
+
+    def test_authenticated_exact(self):
+        user = self.create_user(email='user1@example.com', project_permissions=['someproject', 'other:project-a', 'otherproject'])
+        assert user_has_project_permission(user, 'other:project-a')
+
+    def test_authenticated_pattern_trailing(self):
+        user = self.create_user(email='user1@example.com', project_permissions=['someproject', 'other:*', 'otherproject'])
+        assert user_has_project_permission(user, 'other:project-a')
+
+    def test_authenticated_pattern_both(self):
+        user = self.create_user(email='user1@example.com', project_permissions=['someproject', '*other:*', 'otherproject'])
+        assert user_has_project_permission(user, 'other:project-a')
+
+    def test_not_authenticated_none(self):
+        user = self.create_user(email='user1@example.com')
+        assert not user_has_project_permission(user, 'other:project-a')
+
+    def test_not_authenticated_pattern(self):
+        user = self.create_user(email='user1@example.com', project_permissions=['someproject*', 'otherproject'])
+        assert not user_has_project_permission(user, 'other:project-a')
+
+
+class ProjectAdminDecoratorTestCase(TestCase):
 
     _project_slug = 'other:project-a'
 
@@ -62,8 +89,6 @@ def _sample_function(self):
     def _sample_function_error(self):
         raise self.DidExecute
 
-    respond = mock.MagicMock()
-
     def test_global_admin(self):
         user = self.create_user(email='user1@example.com', is_admin=True)
         with mock.patch('changes.api.auth.get_current_user') as mocked:
@@ -96,29 +121,25 @@ def test_not_authenticated_none(self):
         user = self.create_user(email='user1@example.com')
         with mock.patch('changes.api.auth.get_current_user') as mocked:
             mocked.return_value = user
-            self._sample_function()
-            _, kwargs = self.respond.call_args
-            assert kwargs['status_code'] == 403
+            _, status_code = self._sample_function()
+            assert status_code == 403
 
     def test_not_authenticated_pattern(self):
         user = self.create_user(email='user1@example.com', project_permissions=['someproject*', 'otherproject'])
         with mock.patch('changes.api.auth.get_current_user') as mocked:
             mocked.return_value = user
-            self._sample_function()
-            _, kwargs = self.respond.call_args
-            assert kwargs['status_code'] == 403
+            _, status_code = self._sample_function()
+            assert status_code == 403
 
     def test_no_user(self):
         with mock.patch('changes.api.auth.get_current_user') as mocked:
             mocked.return_value = None
-            self._sample_function()
-            _, kwargs = self.respond.call_args
-            assert kwargs['status_code'] == 401
+            _, status_code = self._sample_function()
+            assert status_code == 401
 
     def test_resource_not_found(self):
         user = self.create_user(email='user1@example.com', project_permissions=['someproject', 'other:project-a', 'otherproject'])
         with mock.patch('changes.api.auth.get_current_user') as mocked:
             mocked.return_value = user
-            status = self._sample_function_error()
-            _, kwargs = self.respond.call_args
-            assert kwargs['status_code'] == 404
+            _, status_code = self._sample_function_error()
+            assert status_code == 404

tests/changes/api/test_project_index.py

@@ -101,7 +101,16 @@ def test_simple(self):
         })
         assert resp.status_code == 403
 
-        self.login_default_admin()
+        # wrong admin permission
+        self.create_and_login_project_admin(['someotherproject'])
+
+        resp = self.client.post(path, data={
+            'name': 'Foobar',
+            'repository': 'ssh://example.com/foo',
+        })
+        assert resp.status_code == 403
+
+        self.create_and_login_project_admin(['foobar'])
 
         # invalid repo url
         resp = self.client.post(path, data={

webapp/pages/admin_page.js

@@ -5,6 +5,7 @@ import SectionHeader from 'es6!display/section_header';
 import { ChangesPage, APINotLoadedPage } from 'es6!display/page_chrome';
 import ChangesLinks from 'es6!display/changes/links';
 import * as FieldGroupMarkup from 'es6!display/field_group';
+import { FlashMessage, FAILURE } from 'es6!display/flash';
 import { Grid } from 'es6!display/grid';
 import Request from 'es6!display/request';
 import { Tabs, MenuUtils } from 'es6!display/menus';
@@ -268,7 +269,19 @@ let NewProjectFieldGroup = React.createClass({
     ];
 
     let fieldMarkup = FieldGroupMarkup.create(form, "Save Project", this, []);
-    return <div>{fieldMarkup}</div>;
+    let error = this.state.error;
+    if (error) {
+      error = JSON.parse(error);
+      if (error.error !== undefined) {
+        error = error.error;
+      }
+    }
+
+    let errorMessage = error ? <FlashMessage message={error} type={FAILURE} /> : null;
+    return <div>
+             <div>{fieldMarkup}</div>
+             {errorMessage}
+           </div>;
   },
 })