From 996596f2b35f6e06918f74b7deac026d2277eee8 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 10 Sep 2018 13:35:16 -0400 Subject: [PATCH 01/10] Automatically assign workspace owner to environments --- atst/domain/projects.py | 5 ++++- atst/routes/workspaces.py | 1 + script/seed.py | 1 + tests/domain/test_projects.py | 2 +- tests/models/test_environments.py | 2 +- tests/models/test_workspace_user.py | 2 +- 6 files changed, 9 insertions(+), 4 deletions(-) diff --git a/atst/domain/projects.py b/atst/domain/projects.py index 611ee2f9..b3456ddf 100644 --- a/atst/domain/projects.py +++ b/atst/domain/projects.py @@ -8,10 +8,13 @@ from atst.models.project import Project class Projects(object): @classmethod - def create(cls, workspace, name, description, environment_names): + def create(cls, user, workspace, name, description, environment_names): project = Project(workspace=workspace, name=name, description=description) Environments.create_many(project, environment_names) + for environment in project.environments: + Environments.add_member(user, environment, user) + db.session.add(project) db.session.commit() diff --git a/atst/routes/workspaces.py b/atst/routes/workspaces.py index ecd97bac..e8d0df8d 100644 --- a/atst/routes/workspaces.py +++ b/atst/routes/workspaces.py @@ -112,6 +112,7 @@ def create_project(workspace_id): if form.validate(): project_data = form.data Projects.create( + g.current_user, workspace, project_data["name"], project_data["description"], diff --git a/script/seed.py b/script/seed.py index 01050427..cebe5528 100644 --- a/script/seed.py +++ b/script/seed.py @@ -68,6 +68,7 @@ def seed_db(): Workspaces.create_member(user, workspace, workspace_user) Projects.create( + user, workspace=workspace, name="First Project", description="This is our first project.", diff --git a/tests/domain/test_projects.py b/tests/domain/test_projects.py index 8845c04a..e959d00c 100644 --- a/tests/domain/test_projects.py +++ b/tests/domain/test_projects.py @@ -4,7 +4,7 @@ from tests.factories import WorkspaceFactory def test_create_project_with_multiple_environments(): workspace = WorkspaceFactory.create() - project = Projects.create(workspace, "My Test Project", "Test", ["dev", "prod"]) + project = Projects.create(workspace.owner, workspace, "My Test Project", "Test", ["dev", "prod"]) assert project.workspace == workspace assert project.name == "My Test Project" diff --git a/tests/models/test_environments.py b/tests/models/test_environments.py index 6bb4768f..05be623f 100644 --- a/tests/models/test_environments.py +++ b/tests/models/test_environments.py @@ -10,7 +10,7 @@ def test_add_user_to_environment(): workspace = Workspaces.create(RequestFactory.create(creator=owner)) project = Projects.create( - workspace, "my test project", "It's mine.", ["dev", "staging", "prod"] + owner, workspace, "my test project", "It's mine.", ["dev", "staging", "prod"] ) dev_environment = project.environments[0] diff --git a/tests/models/test_workspace_user.py b/tests/models/test_workspace_user.py index afa8c306..ef2c7749 100644 --- a/tests/models/test_workspace_user.py +++ b/tests/models/test_workspace_user.py @@ -33,7 +33,7 @@ def test_has_environment_roles(): workspace = Workspaces.create(RequestFactory.create(creator=owner)) workspace_user = Workspaces.create_member(owner, workspace, developer_data) project = Projects.create( - workspace, "my test project", "It's mine.", ["dev", "staging", "prod"] + owner, workspace, "my test project", "It's mine.", ["dev", "staging", "prod"] ) Environments.add_member(owner, project.environments[0], workspace_user.user) assert workspace_user.has_environment_roles From 89f6c903d1f803e1eaad333f9065f9916afb9b9b Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 10 Sep 2018 13:41:14 -0400 Subject: [PATCH 02/10] Created ScopedResource concept, create workspaces module --- atst/domain/workspaces/__init__.py | 1 + atst/domain/workspaces/scopes.py | 59 ++++++++++++++++++++++ atst/domain/{ => workspaces}/workspaces.py | 3 +- 3 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 atst/domain/workspaces/__init__.py create mode 100644 atst/domain/workspaces/scopes.py rename atst/domain/{ => workspaces}/workspaces.py (97%) diff --git a/atst/domain/workspaces/__init__.py b/atst/domain/workspaces/__init__.py new file mode 100644 index 00000000..a9757dc5 --- /dev/null +++ b/atst/domain/workspaces/__init__.py @@ -0,0 +1 @@ +from .workspaces import Workspaces diff --git a/atst/domain/workspaces/scopes.py b/atst/domain/workspaces/scopes.py new file mode 100644 index 00000000..640f75cf --- /dev/null +++ b/atst/domain/workspaces/scopes.py @@ -0,0 +1,59 @@ +from atst.domain.authz import Authorization +from atst.models.permissions import Permissions +from atst.domain.projects import Projects +from atst.domain.environments import Environments + + +class ScopedResource(object): + """ + An abstract class that represents a resource that is restricted + in some way by the priveleges of the user viewing that resource. + """ + + def __init__(self, user, resource): + self.user = user + self.resource = resource + + def __getattr__(self, name): + return getattr(self.resource, name) + + def __eq__(self, other): + return self.resource == other + + +class ScopedWorkspace(ScopedResource): + """ + An object that obeys the same API as a Workspace, but with the added + functionality that it only returns sub-resources (projects and environments) + that the given user is allowed to see. + """ + + @property + def projects(self): + if Authorization.has_workspace_permission( + self.user, self.resource, Permissions.VIEW_APPLICATION_IN_WORKSPACE + ): + projects = self.resource.projects + else: + projects = Projects.for_user(self.user, self.resource) + + return [ScopedProject(self.user, project) for project in projects] + + +class ScopedProject(ScopedResource): + """ + An object that obeys the same API as a Workspace, but with the added + functionality that it only returns sub-resources (environments) + that the given user is allowed to see. + """ + + @property + def environments(self): + if Authorization.has_workspace_permission( + self.user, self.resource, Permissions.VIEW_ENVIRONMENT_IN_APPLICATION + ): + environments = self.resource.environments + else: + environments = Environments.for_user(self.user, self.resource) + + return environments diff --git a/atst/domain/workspaces.py b/atst/domain/workspaces/workspaces.py similarity index 97% rename from atst/domain/workspaces.py rename to atst/domain/workspaces/workspaces.py index b21c8ca0..6c374188 100644 --- a/atst/domain/workspaces.py +++ b/atst/domain/workspaces/workspaces.py @@ -9,6 +9,7 @@ from atst.domain.authz import Authorization from atst.models.permissions import Permissions from atst.domain.users import Users from atst.domain.workspace_users import WorkspaceUsers +from .scopes import ScopedWorkspace class Workspaces(object): @@ -30,7 +31,7 @@ class Workspaces(object): user, workspace, Permissions.VIEW_WORKSPACE, "get workspace" ) - return workspace + return ScopedWorkspace(user, workspace) @classmethod def get_for_update(cls, user, workspace_id): From 39a1a5508c4cb2c44151d0644bbf09d44845d023 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 10 Sep 2018 13:41:48 -0400 Subject: [PATCH 03/10] Remove VIEW_{APPLICATION,ENVIRONMENT}_IN_WORKSPACE from some roles --- ...63_remove_view_project_and_environment_.py | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 alembic/versions/dea6b8e09d63_remove_view_project_and_environment_.py diff --git a/alembic/versions/dea6b8e09d63_remove_view_project_and_environment_.py b/alembic/versions/dea6b8e09d63_remove_view_project_and_environment_.py new file mode 100644 index 00000000..b665538a --- /dev/null +++ b/alembic/versions/dea6b8e09d63_remove_view_project_and_environment_.py @@ -0,0 +1,49 @@ +"""remove view project and environment permissions + +Revision ID: dea6b8e09d63 +Revises: ad30159ef19b +Create Date: 2018-09-10 11:06:00.017222 + +""" +from alembic import op +from sqlalchemy.orm.session import Session + +from atst.models.role import Role +from atst.models.permissions import Permissions + + +# revision identifiers, used by Alembic. +revision = "dea6b8e09d63" +down_revision = "ad30159ef19b" +branch_labels = None +depends_on = None + + +def upgrade(): + session = Session(bind=op.get_bind()) + + priveleged_role_names = ("owner", "admin", "ccpo") + non_priveleged_roles = ( + session.query(Role).filter(Role.name.notin_(priveleged_role_names)).all() + ) + for role in non_priveleged_roles: + role.remove_permission(Permissions.VIEW_APPLICATION_IN_WORKSPACE) + role.remove_permission(Permissions.VIEW_ENVIRONMENT_IN_APPLICATION) + session.add(role) + + session.commit() + + +def downgrade(): + session = Session(bind=op.get_bind()) + + priveleged_role_names = ("owner", "admin", "ccpo") + non_priveleged_roles = ( + session.query(Role).filter(not Role.name.in_(priveleged_role_names)).all() + ) + for role in non_priveleged_roles: + role.add_permission(Permissions.VIEW_APPLICATION_IN_WORKSPACE) + role.add_permission(Permissions.VIEW_ENVIRONMENT_IN_APPLICATION) + session.add(role) + + session.commit() From eb99e7265983d2aec8b8b2198dda338afe4c118f Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 10 Sep 2018 13:42:24 -0400 Subject: [PATCH 04/10] Create Projects.for_user and Environments.for_user --- atst/domain/environments.py | 16 ++++++++-- atst/domain/projects.py | 13 ++++++++ atst/models/workspace.py | 3 +- tests/domain/test_workspaces.py | 56 +++++++++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 3 deletions(-) diff --git a/atst/domain/environments.py b/atst/domain/environments.py index 853eadb7..3f15fb17 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -1,6 +1,7 @@ from atst.database import db from atst.models.environment import Environment from atst.models.environment_role import EnvironmentRole, CSPRole +from atst.models.project import Project class Environments(object): @@ -19,11 +20,22 @@ class Environments(object): db.session.commit() @classmethod - def add_member(cls, user, environment, member): + def add_member(cls, user, environment, member, role=CSPRole.NONSENSE_ROLE): environment_user = EnvironmentRole( - user=member, environment=environment, role=CSPRole.NONSENSE_ROLE.value + user=member, environment=environment, role=role.value ) db.session.add(environment_user) db.session.commit() return environment + + @classmethod + def for_user(cls, user, project): + return ( + db.session.query(Environment) + .join(EnvironmentRole) + .join(Project) + .filter(EnvironmentRole.user_id == user.id) + .filter(Project.id == Environment.project_id) + .all() + ) diff --git a/atst/domain/projects.py b/atst/domain/projects.py index b3456ddf..4d03f80f 100644 --- a/atst/domain/projects.py +++ b/atst/domain/projects.py @@ -4,6 +4,8 @@ from atst.domain.environments import Environments from atst.domain.exceptions import NotFoundError from atst.models.permissions import Permissions from atst.models.project import Project +from atst.models.environment import Environment +from atst.models.environment_role import EnvironmentRole class Projects(object): @@ -36,3 +38,14 @@ class Projects(object): raise NotFoundError("project") return project + + @classmethod + def for_user(self, user, workspace): + return ( + db.session.query(Project) + .join(Environment) + .join(EnvironmentRole) + .filter(Project.workspace_id == workspace.id) + .filter(EnvironmentRole.user_id == user.id) + .all() + ) diff --git a/atst/models/workspace.py b/atst/models/workspace.py index ad5f2948..dd48a7e2 100644 --- a/atst/models/workspace.py +++ b/atst/models/workspace.py @@ -22,7 +22,8 @@ class Workspace(Base, TimestampsMixin): def _is_workspace_owner(workspace_role): return workspace_role.role.name == "owner" - return first_or_none(_is_workspace_owner, self.roles) + owner = first_or_none(_is_workspace_owner, self.roles) + return owner.user if owner else None @property def users(self): diff --git a/tests/domain/test_workspaces.py b/tests/domain/test_workspaces.py index c95989d1..90e568bd 100644 --- a/tests/domain/test_workspaces.py +++ b/tests/domain/test_workspaces.py @@ -4,6 +4,8 @@ from uuid import uuid4 from atst.domain.exceptions import NotFoundError, UnauthorizedError from atst.domain.workspaces import Workspaces from atst.domain.workspace_users import WorkspaceUsers +from atst.domain.projects import Projects +from atst.domain.environments import Environments from tests.factories import WorkspaceFactory, RequestFactory, UserFactory @@ -179,3 +181,57 @@ def test_random_user_cannot_view_workspace_members(): with pytest.raises(UnauthorizedError): workspace = Workspaces.get_with_members(developer, workspace.id) + + +def test_scoped_workspace_only_returns_users_projects_and_environments(): + workspace = WorkspaceFactory.create() + new_project = Projects.create( + workspace.owner, + workspace, + "My Project", + "My project", + ["dev", "staging", "prod"], + ) + developer = UserFactory.from_atat_role("developer") + dev_environment = Environments.add_member(workspace.owner, new_project.environments[0], developer) + + scoped_workspace = Workspaces.get(developer, workspace.id) + + assert scoped_workspace.projects == [new_project] + assert scoped_workspace.projects[0].environments == [dev_environment] + + +def test_scoped_workspace_returns_all_projects_for_ccpo(): + workspace = Workspaces.create(RequestFactory.create()) + for _ in range(5): + Projects.create( + workspace.owner, + workspace, + "My Project", + "My project", + ["dev", "staging", "prod"], + ) + + ccpo = UserFactory.from_atat_role("ccpo") + scoped_workspace = Workspaces.get(ccpo, workspace.id) + + assert len(scoped_workspace.projects) == 5 + assert len(scoped_workspace.projects[0].environments) == 3 + + +def test_scoped_workspace_returns_all_projects_for_workspace_owner(): + workspace = Workspaces.create(RequestFactory.create()) + for _ in range(5): + Projects.create( + workspace.owner, + workspace, + "My Project", + "My project", + ["dev", "staging", "prod"], + ) + + owner = UserFactory.from_atat_role("owner") + scoped_workspace = Workspaces.get(owner, workspace.id) + + assert len(scoped_workspace.projects) == 5 + assert len(scoped_workspace.projects[0].environments) == 3 From 9e5d3094d02616a916b031c221b169db41dc10fb Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 10 Sep 2018 13:46:20 -0400 Subject: [PATCH 05/10] Extract variable for readability --- atst/domain/workspaces/scopes.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/atst/domain/workspaces/scopes.py b/atst/domain/workspaces/scopes.py index 640f75cf..3a719239 100644 --- a/atst/domain/workspaces/scopes.py +++ b/atst/domain/workspaces/scopes.py @@ -30,9 +30,11 @@ class ScopedWorkspace(ScopedResource): @property def projects(self): - if Authorization.has_workspace_permission( + can_view_all_projects = Authorization.has_workspace_permission( self.user, self.resource, Permissions.VIEW_APPLICATION_IN_WORKSPACE - ): + ) + + if can_view_all_projects: projects = self.resource.projects else: projects = Projects.for_user(self.user, self.resource) @@ -49,9 +51,11 @@ class ScopedProject(ScopedResource): @property def environments(self): - if Authorization.has_workspace_permission( + can_view_all_environments = Authorization.has_workspace_permission( self.user, self.resource, Permissions.VIEW_ENVIRONMENT_IN_APPLICATION - ): + ) + + if can_view_all_environments: environments = self.resource.environments else: environments = Environments.for_user(self.user, self.resource) From 7126670d564374815e0f7710e11e05fb4fd66823 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 10 Sep 2018 13:46:40 -0400 Subject: [PATCH 06/10] Formatting --- tests/domain/test_projects.py | 4 +++- tests/domain/test_workspaces.py | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/domain/test_projects.py b/tests/domain/test_projects.py index e959d00c..2c4788e6 100644 --- a/tests/domain/test_projects.py +++ b/tests/domain/test_projects.py @@ -4,7 +4,9 @@ from tests.factories import WorkspaceFactory def test_create_project_with_multiple_environments(): workspace = WorkspaceFactory.create() - project = Projects.create(workspace.owner, workspace, "My Test Project", "Test", ["dev", "prod"]) + project = Projects.create( + workspace.owner, workspace, "My Test Project", "Test", ["dev", "prod"] + ) assert project.workspace == workspace assert project.name == "My Test Project" diff --git a/tests/domain/test_workspaces.py b/tests/domain/test_workspaces.py index 90e568bd..5af3e4cb 100644 --- a/tests/domain/test_workspaces.py +++ b/tests/domain/test_workspaces.py @@ -193,7 +193,9 @@ def test_scoped_workspace_only_returns_users_projects_and_environments(): ["dev", "staging", "prod"], ) developer = UserFactory.from_atat_role("developer") - dev_environment = Environments.add_member(workspace.owner, new_project.environments[0], developer) + dev_environment = Environments.add_member( + workspace.owner, new_project.environments[0], developer + ) scoped_workspace = Workspaces.get(developer, workspace.id) From df84f1959a8e773eeac0a57d7a9f4aab1ebed7e0 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 10 Sep 2018 14:01:22 -0400 Subject: [PATCH 07/10] New Workspaces.add_member method --- atst/domain/workspaces/workspaces.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/atst/domain/workspaces/workspaces.py b/atst/domain/workspaces/workspaces.py index 6c374188..a4168c26 100644 --- a/atst/domain/workspaces/workspaces.py +++ b/atst/domain/workspaces/workspaces.py @@ -88,8 +88,12 @@ class Workspaces(object): last_name=data["last_name"], email=data["email"], ) + return Workspaces.add_member(workspace, new_user, data["workspace_role"]) + + @classmethod + def add_member(cls, workspace, member, role_name): workspace_user = WorkspaceUsers.add( - new_user, workspace.id, data["workspace_role"] + member, workspace.id, role_name ) return workspace_user From 79ba272f702b6c1f54f15371f215691e1f047eb6 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 10 Sep 2018 14:01:36 -0400 Subject: [PATCH 08/10] Check workspace permissions, not project --- atst/domain/workspaces/scopes.py | 2 +- tests/domain/test_workspaces.py | 16 +++++++--------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/atst/domain/workspaces/scopes.py b/atst/domain/workspaces/scopes.py index 3a719239..017a8be4 100644 --- a/atst/domain/workspaces/scopes.py +++ b/atst/domain/workspaces/scopes.py @@ -52,7 +52,7 @@ class ScopedProject(ScopedResource): @property def environments(self): can_view_all_environments = Authorization.has_workspace_permission( - self.user, self.resource, Permissions.VIEW_ENVIRONMENT_IN_APPLICATION + self.user, self.resource.workspace, Permissions.VIEW_ENVIRONMENT_IN_APPLICATION ) if can_view_all_environments: diff --git a/tests/domain/test_workspaces.py b/tests/domain/test_workspaces.py index 5af3e4cb..0433c214 100644 --- a/tests/domain/test_workspaces.py +++ b/tests/domain/test_workspaces.py @@ -203,7 +203,7 @@ def test_scoped_workspace_only_returns_users_projects_and_environments(): assert scoped_workspace.projects[0].environments == [dev_environment] -def test_scoped_workspace_returns_all_projects_for_ccpo(): +def test_scoped_workspace_returns_all_projects_for_workspace_admin(): workspace = Workspaces.create(RequestFactory.create()) for _ in range(5): Projects.create( @@ -214,8 +214,10 @@ def test_scoped_workspace_returns_all_projects_for_ccpo(): ["dev", "staging", "prod"], ) - ccpo = UserFactory.from_atat_role("ccpo") - scoped_workspace = Workspaces.get(ccpo, workspace.id) + admin = Workspaces.add_member( + workspace, UserFactory.from_atat_role("default"), "admin" + ).user + scoped_workspace = Workspaces.get(admin, workspace.id) assert len(scoped_workspace.projects) == 5 assert len(scoped_workspace.projects[0].environments) == 3 @@ -223,16 +225,12 @@ def test_scoped_workspace_returns_all_projects_for_ccpo(): def test_scoped_workspace_returns_all_projects_for_workspace_owner(): workspace = Workspaces.create(RequestFactory.create()) + owner = workspace.owner for _ in range(5): Projects.create( - workspace.owner, - workspace, - "My Project", - "My project", - ["dev", "staging", "prod"], + owner, workspace, "My Project", "My project", ["dev", "staging", "prod"] ) - owner = UserFactory.from_atat_role("owner") scoped_workspace = Workspaces.get(owner, workspace.id) assert len(scoped_workspace.projects) == 5 From 23c9c7d5ae8356bd3312d21a3ccb08d9d24617ef Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 10 Sep 2018 14:22:21 -0400 Subject: [PATCH 09/10] Formatting --- atst/domain/workspaces/scopes.py | 4 +++- atst/domain/workspaces/workspaces.py | 4 +--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/atst/domain/workspaces/scopes.py b/atst/domain/workspaces/scopes.py index 017a8be4..e03d1098 100644 --- a/atst/domain/workspaces/scopes.py +++ b/atst/domain/workspaces/scopes.py @@ -52,7 +52,9 @@ class ScopedProject(ScopedResource): @property def environments(self): can_view_all_environments = Authorization.has_workspace_permission( - self.user, self.resource.workspace, Permissions.VIEW_ENVIRONMENT_IN_APPLICATION + self.user, + self.resource.workspace, + Permissions.VIEW_ENVIRONMENT_IN_APPLICATION, ) if can_view_all_environments: diff --git a/atst/domain/workspaces/workspaces.py b/atst/domain/workspaces/workspaces.py index a4168c26..f87f3347 100644 --- a/atst/domain/workspaces/workspaces.py +++ b/atst/domain/workspaces/workspaces.py @@ -92,9 +92,7 @@ class Workspaces(object): @classmethod def add_member(cls, workspace, member, role_name): - workspace_user = WorkspaceUsers.add( - member, workspace.id, role_name - ) + workspace_user = WorkspaceUsers.add(member, workspace.id, role_name) return workspace_user @classmethod From 607effcd1e50674102c8a62dbc42c0a5e49b9064 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 10 Sep 2018 23:36:21 -0400 Subject: [PATCH 10/10] Rename and comment test for clarity --- tests/domain/test_workspaces.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/domain/test_workspaces.py b/tests/domain/test_workspaces.py index 0433c214..cf63facf 100644 --- a/tests/domain/test_workspaces.py +++ b/tests/domain/test_workspaces.py @@ -183,7 +183,7 @@ def test_random_user_cannot_view_workspace_members(): workspace = Workspaces.get_with_members(developer, workspace.id) -def test_scoped_workspace_only_returns_users_projects_and_environments(): +def test_scoped_workspace_only_returns_a_users_projects_and_environments(): workspace = WorkspaceFactory.create() new_project = Projects.create( workspace.owner, @@ -199,6 +199,8 @@ def test_scoped_workspace_only_returns_users_projects_and_environments(): scoped_workspace = Workspaces.get(developer, workspace.id) + # Should only return the project and environment in which the user has an + # environment role. assert scoped_workspace.projects == [new_project] assert scoped_workspace.projects[0].environments == [dev_environment]