From 5c2d466049cf186c76320660d567c37159e2a06b Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 25 Oct 2018 15:14:14 -0400 Subject: [PATCH] workspace users who have not accepted invite should not have permissions --- atst/domain/workspaces/query.py | 4 +- atst/domain/workspaces/workspaces.py | 10 +++- atst/models/workspace_user.py | 15 ++++- atst/routes/workspaces.py | 1 + tests/domain/test_workspaces.py | 2 + tests/factories.py | 20 +++++-- tests/routes/test_home.py | 1 + tests/routes/test_workspaces.py | 89 ++++++++++++---------------- 8 files changed, 78 insertions(+), 64 deletions(-) diff --git a/atst/domain/workspaces/query.py b/atst/domain/workspaces/query.py index 45f29b3b..28a5aae8 100644 --- a/atst/domain/workspaces/query.py +++ b/atst/domain/workspaces/query.py @@ -30,8 +30,8 @@ class WorkspacesQuery(Query): ) @classmethod - def create_workspace_role(cls, user, role, workspace): - return WorkspaceRole(user=user, role=role, workspace=workspace) + def create_workspace_role(cls, user, role, workspace, **kwargs): + return WorkspaceRole(user=user, role=role, workspace=workspace, **kwargs) @classmethod def get_role_for_workspace_and_user(cls, workspace, user): diff --git a/atst/domain/workspaces/workspaces.py b/atst/domain/workspaces/workspaces.py index b3884ed2..d5e91a52 100644 --- a/atst/domain/workspaces/workspaces.py +++ b/atst/domain/workspaces/workspaces.py @@ -13,7 +13,9 @@ class Workspaces(object): def create(cls, request, name=None): name = name or request.displayname workspace = WorkspacesQuery.create(request=request, name=name) - Workspaces._create_workspace_role(request.creator, workspace, "owner") + Workspaces._create_workspace_role( + request.creator, workspace, "owner", accepted=True + ) WorkspacesQuery.add_and_commit(workspace) return workspace @@ -107,9 +109,11 @@ class Workspaces(object): return WorkspaceUsers.update_role(member, workspace.id, role_name) @classmethod - def _create_workspace_role(cls, user, workspace, role_name): + def _create_workspace_role(cls, user, workspace, role_name, accepted=False): role = Roles.get(role_name) - workspace_role = WorkspacesQuery.create_workspace_role(user, role, workspace) + workspace_role = WorkspacesQuery.create_workspace_role( + user, role, workspace, accepted=accepted + ) WorkspacesQuery.add_and_commit(workspace_role) return workspace_role diff --git a/atst/models/workspace_user.py b/atst/models/workspace_user.py index e61182e4..8d28c7b0 100644 --- a/atst/models/workspace_user.py +++ b/atst/models/workspace_user.py @@ -12,7 +12,9 @@ class WorkspaceUser(object): def permissions(self): atat_permissions = set(self.user.atat_role.permissions) workspace_permissions = ( - [] if self.workspace_role is None else self.workspace_role.role.permissions + [] + if self.workspace_role is None or not self.is_accepted + else self.workspace_role.role.permissions ) return set(workspace_permissions).union(atat_permissions) @@ -74,8 +76,15 @@ class WorkspaceUser(object): def __repr__(self): return "".format( - self.user_name, - self.role.name, + self.user.full_name, + self.role, self.workspace.name, self.num_environment_roles, ) + + @property + def is_accepted(self): + if self.workspace_role: + return self.workspace_role.accepted + + return False diff --git a/atst/routes/workspaces.py b/atst/routes/workspaces.py index cc2f9f48..fbd14ceb 100644 --- a/atst/routes/workspaces.py +++ b/atst/routes/workspaces.py @@ -341,6 +341,7 @@ def update_member(workspace_id, member_id): @bp.route("/workspaces/invitation/", methods=["GET"]) def accept_invitation(invite_id): invite = Invitations.accept(invite_id) + Workspaces.accept_workspace_role(invite.user, invite.workspace) return redirect( diff --git a/tests/domain/test_workspaces.py b/tests/domain/test_workspaces.py index 6141f9bf..f3479ea5 100644 --- a/tests/domain/test_workspaces.py +++ b/tests/domain/test_workspaces.py @@ -219,6 +219,7 @@ def test_scoped_workspace_returns_all_projects_for_workspace_admin( admin = Workspaces.add_member( workspace, UserFactory.from_atat_role("default"), "admin" ).user + Workspaces.accept_workspace_role(admin, workspace) scoped_workspace = Workspaces.get(admin, workspace.id) assert len(scoped_workspace.projects) == 5 @@ -279,6 +280,7 @@ def test_get_for_update_information(): admin = UserFactory.create() Workspaces.add_member(workspace, admin, "admin") + Workspaces.accept_workspace_role(admin, workspace) admin_ws = Workspaces.get_for_update_information(admin, workspace.id) assert workspace == admin_ws diff --git a/tests/factories.py b/tests/factories.py index ca40e61c..0db8128f 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -21,6 +21,7 @@ from atst.domain.roles import Roles from atst.models.workspace_role import WorkspaceRole from atst.models.environment_role import EnvironmentRole from atst.models.invitation import Invitation +from atst.domain.workspaces import Workspaces class Base(factory.alchemy.SQLAlchemyModelFactory): @@ -244,7 +245,7 @@ class WorkspaceFactory(Base): @classmethod def _create(cls, model_class, *args, **kwargs): with_projects = kwargs.pop("projects", []) - owner = kwargs.pop("owner", None) + owner = kwargs.pop("owner", UserFactory.create()) members = kwargs.pop("members", []) workspace = super()._create(model_class, *args, **kwargs) @@ -253,11 +254,10 @@ class WorkspaceFactory(Base): ProjectFactory.create(workspace=workspace, **p) for p in with_projects ] - if owner: - workspace.request.creator = owner - WorkspaceRoleFactory.create( - workspace=workspace, role=Roles.get("owner"), user=owner - ) + workspace.request.creator = owner + WorkspaceRoleFactory.create( + workspace=workspace, role=Roles.get("owner"), user=owner, accepted=True + ) for member in members: user = member.get("user", UserFactory.create()) @@ -269,6 +269,14 @@ class WorkspaceFactory(Base): workspace.projects = projects return workspace + @classmethod + def create_user_and_workspace_with_role(cls, role="owner"): + user = UserFactory.create() + workspace = WorkspaceFactory.create() + Workspaces._create_workspace_role(user, workspace, role) + Workspaces.accept_workspace_role(user, workspace) + return user, workspace + class ProjectFactory(Base): class Meta: diff --git a/tests/routes/test_home.py b/tests/routes/test_home.py index 3bcc381d..6a8a2ad5 100644 --- a/tests/routes/test_home.py +++ b/tests/routes/test_home.py @@ -6,6 +6,7 @@ def test_user_with_workspaces_has_workspaces_nav(client, user_session): user = UserFactory.create() workspace = WorkspaceFactory.create() Workspaces._create_workspace_role(user, workspace, "developer") + Workspaces.accept_workspace_role(user, workspace) user_session(user) response = client.get("/home", follow_redirects=True) diff --git a/tests/routes/test_workspaces.py b/tests/routes/test_workspaces.py index 2edaa0ee..f9d460a5 100644 --- a/tests/routes/test_workspaces.py +++ b/tests/routes/test_workspaces.py @@ -12,9 +12,7 @@ from atst.queue import queue def test_user_with_permission_has_budget_report_link(client, user_session): - user = UserFactory.create() - workspace = WorkspaceFactory.create() - Workspaces._create_workspace_role(user, workspace, "owner") + user, workspace = WorkspaceFactory.create_user_and_workspace_with_role("owner") user_session(user) response = client.get("/workspaces/{}/projects".format(workspace.id)) @@ -24,9 +22,8 @@ def test_user_with_permission_has_budget_report_link(client, user_session): def test_user_without_permission_has_no_budget_report_link(client, user_session): - user = UserFactory.create() - workspace = WorkspaceFactory.create() - Workspaces._create_workspace_role(user, workspace, "developer") + user, workspace = WorkspaceFactory.create_user_and_workspace_with_role("developer") + user_session(user) response = client.get("/workspaces/{}/projects".format(workspace.id)) assert ( @@ -36,9 +33,7 @@ def test_user_without_permission_has_no_budget_report_link(client, user_session) def test_user_with_permission_has_add_project_link(client, user_session): - user = UserFactory.create() - workspace = WorkspaceFactory.create() - Workspaces._create_workspace_role(user, workspace, "owner") + user, workspace = WorkspaceFactory.create_user_and_workspace_with_role("owner") user_session(user) response = client.get("/workspaces/{}/projects".format(workspace.id)) @@ -49,9 +44,8 @@ def test_user_with_permission_has_add_project_link(client, user_session): def test_user_without_permission_has_no_add_project_link(client, user_session): - user = UserFactory.create() - workspace = WorkspaceFactory.create() - Workspaces._create_workspace_role(user, workspace, "developer") + user, workspace = WorkspaceFactory.create_user_and_workspace_with_role("developer") + user_session(user) response = client.get("/workspaces/{}/projects".format(workspace.id)) assert ( @@ -61,9 +55,7 @@ def test_user_without_permission_has_no_add_project_link(client, user_session): def test_user_with_permission_has_add_member_link(client, user_session): - user = UserFactory.create() - workspace = WorkspaceFactory.create() - Workspaces._create_workspace_role(user, workspace, "owner") + user, workspace = WorkspaceFactory.create_user_and_workspace_with_role("owner") user_session(user) response = client.get("/workspaces/{}/members".format(workspace.id)) @@ -74,9 +66,8 @@ def test_user_with_permission_has_add_member_link(client, user_session): def test_user_without_permission_has_no_add_member_link(client, user_session): - user = UserFactory.create() - workspace = WorkspaceFactory.create() - Workspaces._create_workspace_role(user, workspace, "developer") + user, workspace = WorkspaceFactory.create_user_and_workspace_with_role("developer") + user_session(user) response = client.get("/workspaces/{}/members".format(workspace.id)) assert ( @@ -86,9 +77,8 @@ def test_user_without_permission_has_no_add_member_link(client, user_session): def test_update_workspace_name(client, user_session): - user = UserFactory.create() - workspace = WorkspaceFactory.create() - Workspaces._create_workspace_role(user, workspace, "admin") + user, workspace = WorkspaceFactory.create_user_and_workspace_with_role("admin") + user_session(user) response = client.post( url_for("workspaces.edit_workspace", workspace_id=workspace.id), @@ -100,9 +90,8 @@ def test_update_workspace_name(client, user_session): def test_view_edit_project(client, user_session): - owner = UserFactory.create() - workspace = WorkspaceFactory.create() - Workspaces._create_workspace_role(owner, workspace, "admin") + owner, workspace = WorkspaceFactory.create_user_and_workspace_with_role("admin") + project = Projects.create( owner, workspace, @@ -178,13 +167,12 @@ def test_user_without_permission_cannot_update_project(client, user_session): def test_create_member(client, user_session): - owner = UserFactory.create() - user = UserFactory.create() - workspace = WorkspaceFactory.create() - Workspaces._create_workspace_role(owner, workspace, "admin") + owner, workspace = WorkspaceFactory.create_user_and_workspace_with_role("admin") + user_session(owner) queue_length = len(queue.get_queue()) + user = UserFactory.create() response = client.post( url_for("workspaces.create_member", workspace_id=workspace.id), data={ @@ -204,10 +192,8 @@ def test_create_member(client, user_session): def test_permissions_for_view_member(client, user_session): - user = UserFactory.create() - workspace = WorkspaceFactory.create() - Workspaces._create_workspace_role(user, workspace, "developer") - member = WorkspaceUsers.add(user, workspace.id, "developer") + user, workspace = WorkspaceFactory.create_user_and_workspace_with_role("developer") + user_session(user) response = client.post( url_for("workspaces.view_member", workspace_id=workspace.id, member_id=user.id), @@ -217,9 +203,8 @@ def test_permissions_for_view_member(client, user_session): def test_update_member_workspace_role(client, user_session): - owner = UserFactory.create() - workspace = WorkspaceFactory.create() - Workspaces._create_workspace_role(owner, workspace, "admin") + owner, workspace = WorkspaceFactory.create_user_and_workspace_with_role("admin") + user = UserFactory.create() member = WorkspaceUsers.add(user, workspace.id, "developer") user_session(owner) @@ -235,9 +220,8 @@ def test_update_member_workspace_role(client, user_session): def test_update_member_workspace_role_with_no_data(client, user_session): - owner = UserFactory.create() - workspace = WorkspaceFactory.create() - Workspaces._create_workspace_role(owner, workspace, "admin") + owner, workspace = WorkspaceFactory.create_user_and_workspace_with_role("admin") + user = UserFactory.create() member = WorkspaceUsers.add(user, workspace.id, "developer") user_session(owner) @@ -253,9 +237,7 @@ def test_update_member_workspace_role_with_no_data(client, user_session): def test_update_member_environment_role(client, user_session): - owner = UserFactory.create() - workspace = WorkspaceFactory.create() - Workspaces._create_workspace_role(owner, workspace, "admin") + owner, workspace = WorkspaceFactory.create_user_and_workspace_with_role("admin") user = UserFactory.create() member = WorkspaceUsers.add(user, workspace.id, "developer") @@ -288,9 +270,7 @@ def test_update_member_environment_role(client, user_session): def test_update_member_environment_role_with_no_data(client, user_session): - owner = UserFactory.create() - workspace = WorkspaceFactory.create() - Workspaces._create_workspace_role(owner, workspace, "admin") + owner, workspace = WorkspaceFactory.create_user_and_workspace_with_role("admin") user = UserFactory.create() member = WorkspaceUsers.add(user, workspace.id, "developer") @@ -317,9 +297,7 @@ def test_update_member_environment_role_with_no_data(client, user_session): def test_new_member_accepts_valid_invite(client, user_session): - owner = UserFactory.create() - workspace = WorkspaceFactory.create() - Workspaces._create_workspace_role(owner, workspace, "admin") + owner, workspace = WorkspaceFactory.create_user_and_workspace_with_role("admin") user = UserFactory.create() member = WorkspaceUsers.add(user, workspace.id, "developer") @@ -344,9 +322,7 @@ def test_new_member_accepts_valid_invite(client, user_session): def test_new_member_accept_invalid_invite(client, user_session): - owner = UserFactory.create() - workspace = WorkspaceFactory.create() - Workspaces._create_workspace_role(owner, workspace, "admin") + owner, workspace = WorkspaceFactory.create_user_and_workspace_with_role("admin") user = UserFactory.create() member = WorkspaceUsers.add(user, workspace.id, "developer") @@ -357,3 +333,16 @@ def test_new_member_accept_invalid_invite(client, user_session): response = client.get(url_for("workspaces.accept_invitation", invite_id=invite.id)) assert response.status_code == 404 + + +def test_user_who_has_not_accepted_workspace_invite_cannot_view(client, user_session): + user = UserFactory.create() + workspace = WorkspaceFactory.create() + Workspaces.create_member( + workspace.owner, + workspace, + {"workspace_role": "developer", **user.to_dictionary()}, + ) + user_session(user) + response = client.get("/workspaces/{}/projects".format(workspace.id)) + assert response.status_code == 404