Skip to content
This repository has been archived by the owner on Dec 15, 2018. It is now read-only.

Commit

Permalink
project-level admin backend implementation
Browse files Browse the repository at this point in the history
Summary: This implements project-level admin in the backend (API calls). Now, in addition to global admins, project admins will be able to modify the project's metadata and build plans. It does not contain any UI change, and existing admins should be able to continue using Changes as usual.

Test Plan:
unit tests + ran locally

some screenshots of what happens when an unauthenticated user tries to make changes:
{F499393}

{F499394}

Reviewers: anupc

Reviewed By: anupc

Subscribers: changesbot, kylec

Differential Revision: https://tails.corp.dropbox.com/D222313
  • Loading branch information
Naphat Sanguansin committed Aug 25, 2016
1 parent c802d9b commit 5eb5064
Show file tree
Hide file tree
Showing 16 changed files with 127 additions and 28 deletions.
55 changes: 55 additions & 0 deletions changes/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
from functools import wraps
from typing import Optional # NOQA

from changes.models.plan import Plan
from changes.models.project import Project
from changes.models.step import Step
from changes.models.user import User


Expand Down Expand Up @@ -56,6 +59,58 @@ class ResourceNotFound(Exception):
pass


def get_project_slug_from_project_id(*args, **kwargs):
"""
Get the project slug from the project ID. This function assumes that
the project ID is passed as the keyword argument `project_id`.
Returns:
basestring - project slug
Raises:
ResourceNotFound - if the project is not found
"""
project_id = kwargs['project_id']
# use our custom .get() function instead of .query.get()
project = Project.get(project_id)
if project is None:
raise ResourceNotFound('Project with ID {} not found.'.format(project_id))
return project.slug


def get_project_slug_from_plan_id(*args, **kwargs):
"""
Get the project slug from the plan ID. This function assumes that
the plan ID is passed as the keyword argument `plan_id`.
Returns:
basestring - project slug
Raises:
ResourceNotFound - if the plan is not found
"""
plan_id = kwargs['plan_id']
plan = Plan.query.get(plan_id)
if plan is None:
raise ResourceNotFound('Plan with ID {} not found.'.format(plan_id))
return plan.project.slug


def get_project_slug_from_step_id(*args, **kwargs):
"""
Get the project slug from the step ID. This function assumes that
the step ID is passed as the keyword argument `step_id`.
Returns:
basestring - project slug
Raises:
ResourceNotFound - if the step is not found
"""
step_id = kwargs['step_id']
step = Step.query.get(step_id)
if step is None:
raise ResourceNotFound('Step with ID {} not found.'.format(step_id))
return step.plan.project.slug


def requires_project_admin(get_project_slug):
"""
Require an authenticated user with project admin privileges.
Expand Down
4 changes: 2 additions & 2 deletions changes/api/plan_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from flask.ext.restful import reqparse

from changes.api.auth import requires_admin
from changes.api.auth import get_project_slug_from_plan_id, requires_project_admin
from changes.api.base import APIView
from changes.config import db
from changes.models.plan import Plan, PlanStatus
Expand All @@ -27,7 +27,7 @@ def get(self, plan_id):

return self.respond(context)

@requires_admin
@requires_project_admin(get_project_slug_from_plan_id)
def post(self, plan_id):
plan = Plan.query.get(plan_id)
if plan is None:
Expand Down
4 changes: 2 additions & 2 deletions changes/api/plan_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from flask.ext.restful.reqparse import RequestParser

from changes.api.base import APIView, error
from changes.api.auth import requires_admin
from changes.api.auth import get_project_slug_from_plan_id, requires_project_admin
from changes.db.utils import create_or_update
from changes.models.option import ItemOption
from changes.models.plan import Plan
Expand Down Expand Up @@ -41,7 +41,7 @@ def get(self, plan_id):
post_parser.add_argument('snapshot.allow')
post_parser.add_argument('snapshot.require')

@requires_admin
@requires_project_admin(get_project_slug_from_plan_id)
def post(self, plan_id):
plan = Plan.query.get(plan_id)
if plan is None:
Expand Down
4 changes: 2 additions & 2 deletions changes/api/plan_step_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from copy import deepcopy
from flask.ext.restful import reqparse

from changes.api.auth import requires_admin
from changes.api.auth import get_project_slug_from_plan_id, requires_project_admin
from changes.api.base import APIView, error
from changes.config import db
from changes.constants import IMPLEMENTATION_CHOICES
Expand All @@ -31,7 +31,7 @@ def get(self, plan_id):

return self.respond(list(plan.steps))

@requires_admin
@requires_project_admin(get_project_slug_from_plan_id)
def post(self, plan_id):
plan = Plan.query.get(plan_id)
if plan is None:
Expand Down
4 changes: 2 additions & 2 deletions changes/api/project_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from sqlalchemy.orm import contains_eager, joinedload
from sqlalchemy.sql import func

from changes.api.auth import requires_admin
from changes.api.auth import get_project_slug_from_project_id, requires_project_admin
from changes.api.base import APIView
from changes.config import db
from changes.constants import Result, Status
Expand Down Expand Up @@ -162,7 +162,7 @@ def get(self, project_id):

return self.respond(data)

@requires_admin
@requires_project_admin(get_project_slug_from_project_id)
def post(self, project_id):
project = Project.get(project_id)
if project is None:
Expand Down
4 changes: 2 additions & 2 deletions changes/api/project_options_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from changes.config import statsreporter
from changes.api.base import APIView, error
from changes.api.auth import requires_admin
from changes.api.auth import get_project_slug_from_project_id, requires_project_admin
from changes.db.utils import create_or_update
from changes.models.project import Project, ProjectOption
from changes.models.snapshot import Snapshot, SnapshotStatus
Expand Down Expand Up @@ -62,7 +62,7 @@ def _get_project(self, project_id):
).get(as_uuid)
return project

@requires_admin
@requires_project_admin(get_project_slug_from_project_id)
def post(self, project_id):
project = self._get_project(project_id)
if project is None:
Expand Down
4 changes: 2 additions & 2 deletions changes/api/project_plan_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from flask.ext.restful import reqparse
from sqlalchemy.sql import func

from changes.api.auth import requires_admin
from changes.api.auth import get_project_slug_from_project_id, requires_project_admin
from changes.api.base import APIView
from changes.config import db
from changes.models.plan import Plan, PlanStatus
Expand Down Expand Up @@ -54,7 +54,7 @@ def get(self, project_id):

return self.paginate(queryset)

@requires_admin
@requires_project_admin(get_project_slug_from_project_id)
def post(self, project_id):
project = Project.get(project_id)
if project is None:
Expand Down
6 changes: 3 additions & 3 deletions changes/api/step_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from flask.ext.restful import reqparse

from changes.api.base import APIView, error
from changes.api.auth import requires_admin
from changes.api.auth import get_project_slug_from_step_id, requires_project_admin
from changes.config import db
from changes.constants import IMPLEMENTATION_CHOICES
from changes.db.utils import create_or_update
Expand All @@ -30,7 +30,7 @@ def get(self, step_id):

return self.respond(step)

@requires_admin
@requires_project_admin(get_project_slug_from_step_id)
def post(self, step_id):
step = Step.query.get(step_id)
if step is None:
Expand Down Expand Up @@ -87,7 +87,7 @@ def post(self, step_id):
db.session.commit()
return self.respond(step)

@requires_admin
@requires_project_admin(get_project_slug_from_step_id)
def delete(self, step_id):
step = Step.query.get(step_id)
if step is None:
Expand Down
12 changes: 12 additions & 0 deletions changes/testutils/cases.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from __future__ import absolute_import

import json
import random as insecure_random
import string
import unittest

from exam import Exam, fixture
Expand Down Expand Up @@ -45,6 +47,16 @@ def login_default(self):
def login_default_admin(self):
return self.login(self.default_admin)

def create_and_login_project_admin(self, project_permissions):
username = ''.join(insecure_random.choice(string.ascii_lowercase + string.digits) for _ in range(50))
user = User(
email='{}@example.com'.format(username),
project_permissions=project_permissions,
)
db.session.add(user)
db.session.commit()
return self.login(user)


class TestCase(Exam, unittest.TestCase, Fixtures, AuthMixin):
def setUp(self):
Expand Down
34 changes: 34 additions & 0 deletions tests/changes/api/test_auth.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,46 @@
import mock
import pytest

from uuid import uuid4

from changes.api.auth import (
get_project_slug_from_project_id, get_project_slug_from_plan_id,
get_project_slug_from_step_id,
ResourceNotFound, requires_project_admin,
)
from changes.testutils import TestCase


class ProjectAdminHelpersTestCase(TestCase):

def test_get_project_slug_from_project_id_success(self):
project = self.create_project()
assert project.slug == get_project_slug_from_project_id(1, 2, project_id=project.slug, other_args='test')

def test_get_project_slug_from_project_id_not_found(self):
with pytest.raises(ResourceNotFound):
get_project_slug_from_project_id(project_id='does-not-exist')

def test_get_project_slug_from_plan_id(self):
project = self.create_project()
plan = self.create_plan(project)
assert project.slug == get_project_slug_from_plan_id(1, 2, plan_id=plan.id, other_args='test')

def test_get_project_slug_from_plan_id_not_found(self):
with pytest.raises(ResourceNotFound):
get_project_slug_from_plan_id(plan_id=uuid4())

def test_get_project_slug_from_step_id(self):
project = self.create_project()
plan = self.create_plan(project)
step = self.create_step(plan)
assert project.slug == get_project_slug_from_step_id(1, 2, step_id=step.id, other_args='test')

def test_get_project_slug_from_step_id_not_found(self):
with pytest.raises(ResourceNotFound):
get_project_slug_from_step_id(step_id=uuid4())


class ProjectAdminTestCase(TestCase):

_project_slug = 'other:project-a'
Expand Down
4 changes: 2 additions & 2 deletions tests/changes/api/test_plan_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def test_simple(self):
})
assert resp.status_code == 403

self.login_default_admin()
self.create_and_login_project_admin([project.slug])

# test valid params
resp = self.client.post(path, data={
Expand All @@ -58,7 +58,7 @@ def test_set_snapshot_plan_id(self):

path = '/api/0/plans/{0}/'.format(plan.id.hex)

self.login_default_admin()
self.create_and_login_project_admin([project.slug])
resp = self.client.post(path, data={
'snapshot_plan_id': other_plan.id.hex,
})
Expand Down
2 changes: 1 addition & 1 deletion tests/changes/api/test_plan_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def test_simple(self):
})
assert resp.status_code == 403

self.login_default_admin()
self.create_and_login_project_admin([project.slug])

resp = self.client.post(path, data={
'build.expect-tests': '1',
Expand Down
2 changes: 1 addition & 1 deletion tests/changes/api/test_plan_step_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def test_simple(self):
project = self.create_project()
plan = self.create_plan(project, label='Foo')

self.login_default_admin()
self.create_and_login_project_admin([project.slug])

path = '/api/0/plans/{0}/steps/'.format(plan.id.hex)

Expand Down
4 changes: 2 additions & 2 deletions tests/changes/api/test_project_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def test_update(self):
})
assert resp.status_code == 403

self.login_default_admin()
self.create_and_login_project_admin([project.slug, 'details*'])

resp = self.client.post(path, data={
'name': 'details test project',
Expand Down Expand Up @@ -75,7 +75,7 @@ def test_update_by_slug(self):
path = '/api/0/projects/{0}/'.format(
project.slug)

self.login_default_admin()
self.create_and_login_project_admin([project.slug, 'details*'])

resp = self.client.post(path, data={
'name': 'details test project',
Expand Down
6 changes: 3 additions & 3 deletions tests/changes/api/test_project_options_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def test_simple(self):
})
assert resp.status_code == 403

self.login_default_admin()
self.create_and_login_project_admin([project.slug])

resp = self.client.post(path, data={
'mail.notify-author': '0',
Expand Down Expand Up @@ -65,7 +65,7 @@ def test_simple(self):
def test_nonsense_project(self):
project = self.create_project()
path_fmt = '/api/0/projects/{0}/options/'
self.login_default_admin()
self.create_and_login_project_admin([project.slug])
resp = self.client.post(path_fmt.format('project-that-doesnt-exist'), data={
'mail.notify-author': '0',
})
Expand All @@ -80,7 +80,7 @@ def test_nonsense_project(self):
def test_report_rollback(self):
project = self.create_project()
path = '/api/0/projects/{0}/options/'.format(project.slug)
self.login_default_admin()
self.create_and_login_project_admin([project.slug])

now = datetime(2013, 9, 19, 22, 15, 22)
earlier = now - timedelta(days=1)
Expand Down
6 changes: 2 additions & 4 deletions tests/changes/api/test_step_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,11 @@ def test_requires_auth(self):
assert resp.status_code == 401

def test_simple(self):
self.login_default_admin()

project = self.create_project()
plan = self.create_plan(project, label='Foo')
step = self.create_step(plan=plan)

self.login_default_admin()
self.create_and_login_project_admin([project.slug])

path = '/api/0/steps/{0}/'.format(step.id.hex)

Expand Down Expand Up @@ -87,7 +85,7 @@ def test_simple(self):
plan = self.create_plan(project, label='Foo')
step = self.create_step(plan=plan)

self.login_default_admin()
self.create_and_login_project_admin([project.slug])

path = '/api/0/steps/{0}/'.format(step.id.hex)

Expand Down

0 comments on commit 5eb5064

Please sign in to comment.