diff --git a/alembic/versions/67955a4abaef_remove_accepted_from_workspace_role.py b/alembic/versions/67955a4abaef_remove_accepted_from_workspace_role.py new file mode 100644 index 00000000..749ca512 --- /dev/null +++ b/alembic/versions/67955a4abaef_remove_accepted_from_workspace_role.py @@ -0,0 +1,28 @@ +"""remove accepted from workspace_role + +Revision ID: 67955a4abaef +Revises: e62bcc460c26 +Create Date: 2018-10-26 13:57:29.480236 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '67955a4abaef' +down_revision = 'e62bcc460c26' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('workspace_roles', 'accepted') + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('workspace_roles', sa.Column('accepted', sa.BOOLEAN(), autoincrement=False, nullable=True)) + # ### end Alembic commands ### diff --git a/atst/domain/authz.py b/atst/domain/authz.py index ce422736..fc4b4c32 100644 --- a/atst/domain/authz.py +++ b/atst/domain/authz.py @@ -6,8 +6,7 @@ from atst.domain.exceptions import UnauthorizedError class Authorization(object): @classmethod def has_workspace_permission(cls, user, workspace, permission): - workspace_user = WorkspaceUsers.get(workspace.id, user.id) - return permission in workspace_user.permissions() + return permission in WorkspaceUsers.workspace_user_permissions(workspace, user) @classmethod def has_atat_permission(cls, user, permission): diff --git a/atst/domain/invitations.py b/atst/domain/invitations.py index fc54c43c..166bc841 100644 --- a/atst/domain/invitations.py +++ b/atst/domain/invitations.py @@ -43,6 +43,20 @@ class Invitations(object): return invite + @classmethod + def create_for_owner(cls, workspace, user): + invite = Invitation( + workspace=workspace, + inviter=user, + user=user, + status=InvitationStatus.ACCEPTED, + expiration_time=Invitations.current_expiration_time(), + ) + db.session.add(invite) + db.session.commit() + + return invite + @classmethod def accept(cls, invite_id): invite = Invitations._get(invite_id) diff --git a/atst/domain/workspace_users.py b/atst/domain/workspace_users.py index 84e20c50..ddd234ed 100644 --- a/atst/domain/workspace_users.py +++ b/atst/domain/workspace_users.py @@ -4,6 +4,7 @@ from atst.database import db from atst.models.workspace_role import WorkspaceRole from atst.models.workspace_user import WorkspaceUser from atst.models.user import User +from atst.models.invitation import Invitation, Status as InvitationStatus from .roles import Roles from .users import Users @@ -30,6 +31,32 @@ class WorkspaceUsers(object): return WorkspaceUser(user, workspace_role) + @classmethod + def _get_active_workspace_role(cls, workspace_id, user_id): + try: + return ( + db.session.query(WorkspaceRole) + .join(User) + .filter(User.id == user_id, WorkspaceRole.workspace_id == workspace_id) + .join(Invitation, WorkspaceRole.workspace_id == Invitation.workspace_id) + .filter(Invitation.user_id == WorkspaceRole.user_id) + .filter(Invitation.status == InvitationStatus.ACCEPTED) + .one() + ) + except NoResultFound: + return None + + @classmethod + def workspace_user_permissions(cls, workspace, user): + workspace_role = WorkspaceUsers._get_active_workspace_role( + workspace.id, user.id + ) + atat_permissions = set(user.atat_role.permissions) + workspace_permissions = ( + [] if workspace_role is None else workspace_role.role.permissions + ) + return set(workspace_permissions).union(atat_permissions) + @classmethod def _get_workspace_role(cls, user, workspace_id): try: @@ -63,7 +90,7 @@ class WorkspaceUsers(object): new_workspace_role.role = role except NoResultFound: new_workspace_role = WorkspaceRole( - user=user, role_id=role.id, workspace_id=workspace_id, accepted=False + user=user, role_id=role.id, workspace_id=workspace_id ) user.workspace_roles.append(new_workspace_role) diff --git a/atst/domain/workspaces/query.py b/atst/domain/workspaces/query.py index 28a5aae8..7dc1bc88 100644 --- a/atst/domain/workspaces/query.py +++ b/atst/domain/workspaces/query.py @@ -5,6 +5,7 @@ from atst.domain.common import Query from atst.domain.exceptions import NotFoundError from atst.models.workspace import Workspace from atst.models.workspace_role import WorkspaceRole +from atst.models.invitation import Invitation, Status as InvitationStatus class WorkspacesQuery(Query): @@ -24,8 +25,10 @@ class WorkspacesQuery(Query): return ( db.session.query(Workspace) .join(WorkspaceRole) + .join(Invitation) .filter(WorkspaceRole.user == user) - .filter(WorkspaceRole.accepted == True) + .filter(Invitation.user == user) + .filter(Invitation.status == InvitationStatus.ACCEPTED) .all() ) diff --git a/atst/domain/workspaces/workspaces.py b/atst/domain/workspaces/workspaces.py index d5e91a52..14397714 100644 --- a/atst/domain/workspaces/workspaces.py +++ b/atst/domain/workspaces/workspaces.py @@ -3,6 +3,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 atst.domain.invitations import Invitations from .query import WorkspacesQuery from .scopes import ScopedWorkspace @@ -13,9 +14,8 @@ 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", accepted=True - ) + Workspaces._create_workspace_role(request.creator, workspace, "owner") + Invitations.create_for_owner(workspace, request.creator) WorkspacesQuery.add_and_commit(workspace) return workspace @@ -109,11 +109,9 @@ class Workspaces(object): return WorkspaceUsers.update_role(member, workspace.id, role_name) @classmethod - def _create_workspace_role(cls, user, workspace, role_name, accepted=False): + def _create_workspace_role(cls, user, workspace, role_name): role = Roles.get(role_name) - workspace_role = WorkspacesQuery.create_workspace_role( - user, role, workspace, accepted=accepted - ) + workspace_role = WorkspacesQuery.create_workspace_role(user, role, workspace) WorkspacesQuery.add_and_commit(workspace_role) return workspace_role @@ -123,11 +121,3 @@ class Workspaces(object): workspace.name = new_data["name"] WorkspacesQuery.add_and_commit(workspace) - - @classmethod - def accept_workspace_role(cls, user, workspace): - workspace_role = WorkspacesQuery.get_role_for_workspace_and_user( - workspace, user - ) - workspace_role.accepted = True - WorkspacesQuery.add_and_commit(workspace_role) diff --git a/atst/models/workspace.py b/atst/models/workspace.py index 11f32e27..659dca6a 100644 --- a/atst/models/workspace.py +++ b/atst/models/workspace.py @@ -49,6 +49,6 @@ class Workspace(Base, mixins.TimestampsMixin, mixins.AuditableMixin): return self.id def __repr__(self): - return "".format( - self.name, self.request_id, self.task_order.number, self.user_count, self.id + return "".format( + self.name, self.request_id, self.user_count, self.id ) diff --git a/atst/models/workspace_role.py b/atst/models/workspace_role.py index 93da1488..87c8f46f 100644 --- a/atst/models/workspace_role.py +++ b/atst/models/workspace_role.py @@ -1,4 +1,4 @@ -from sqlalchemy import Index, ForeignKey, Column, Boolean +from sqlalchemy import Index, ForeignKey, Column from sqlalchemy.dialects.postgresql import UUID from sqlalchemy.orm import relationship @@ -22,8 +22,6 @@ class WorkspaceRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): UUID(as_uuid=True), ForeignKey("users.id"), index=True, nullable=False ) - accepted = Column(Boolean) - def __repr__(self): return "".format( self.role.name, self.workspace.name, self.user_id, self.id diff --git a/atst/models/workspace_user.py b/atst/models/workspace_user.py index 8d28c7b0..fb79ebbd 100644 --- a/atst/models/workspace_user.py +++ b/atst/models/workspace_user.py @@ -12,9 +12,7 @@ class WorkspaceUser(object): def permissions(self): atat_permissions = set(self.user.atat_role.permissions) workspace_permissions = ( - [] - if self.workspace_role is None or not self.is_accepted - else self.workspace_role.role.permissions + [] if self.workspace_role is None else self.workspace_role.role.permissions ) return set(workspace_permissions).union(atat_permissions) @@ -81,10 +79,3 @@ class WorkspaceUser(object): 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/errors.py b/atst/routes/errors.py index 34830911..ef1b3397 100644 --- a/atst/routes/errors.py +++ b/atst/routes/errors.py @@ -48,7 +48,7 @@ def make_error_pages(app): log_error(e) return ( render_template( - "error.html", message="The invitation you followed has expired." + "error.html", message="The invitation link you clicked is invalid." ), 404, ) diff --git a/atst/routes/workspaces.py b/atst/routes/workspaces.py index 6dd60b6c..64ad272c 100644 --- a/atst/routes/workspaces.py +++ b/atst/routes/workspaces.py @@ -340,10 +340,10 @@ def update_member(workspace_id, member_id): @bp.route("/workspaces/invitation/", methods=["GET"]) def accept_invitation(invite_id): + # TODO: check that the current_user DOD ID matches the user associated with + # the invitation invite = Invitations.accept(invite_id) - Workspaces.accept_workspace_role(invite.user, invite.workspace) - return redirect( url_for("workspaces.show_workspace", workspace_id=invite.workspace.id) ) diff --git a/tests/domain/test_workspaces.py b/tests/domain/test_workspaces.py index f3479ea5..0365ed5f 100644 --- a/tests/domain/test_workspaces.py +++ b/tests/domain/test_workspaces.py @@ -6,8 +6,9 @@ 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 atst.models.invitation import Status as InvitationStatus -from tests.factories import RequestFactory, UserFactory +from tests.factories import RequestFactory, UserFactory, InvitationFactory @pytest.fixture(scope="function") @@ -219,7 +220,12 @@ 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) + InvitationFactory.create( + user=admin, + inviter=workspace.owner, + workspace=workspace, + status=InvitationStatus.ACCEPTED, + ) scoped_workspace = Workspaces.get(admin, workspace.id) assert len(scoped_workspace.projects) == 5 @@ -248,7 +254,12 @@ def test_for_user_returns_assigned_workspaces_for_user(workspace, workspace_owne bob = UserFactory.from_atat_role("default") Workspaces.add_member(workspace, bob, "developer") Workspaces.create(RequestFactory.create()) - Workspaces.accept_workspace_role(bob, workspace) + InvitationFactory.create( + user=bob, + inviter=workspace.owner, + workspace=workspace, + status=InvitationStatus.ACCEPTED, + ) bobs_workspaces = Workspaces.for_user(bob) @@ -275,12 +286,23 @@ def test_for_user_returns_all_workspaces_for_ccpo(workspace, workspace_owner): def test_get_for_update_information(): workspace_owner = UserFactory.create() workspace = Workspaces.create(RequestFactory.create(creator=workspace_owner)) + InvitationFactory.create( + user=workspace_owner, + inviter=workspace_owner, + workspace=workspace, + status=InvitationStatus.ACCEPTED, + ) owner_ws = Workspaces.get_for_update_information(workspace_owner, workspace.id) assert workspace == owner_ws admin = UserFactory.create() Workspaces.add_member(workspace, admin, "admin") - Workspaces.accept_workspace_role(admin, workspace) + InvitationFactory.create( + user=admin, + inviter=workspace_owner, + workspace=workspace, + status=InvitationStatus.ACCEPTED, + ) 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 babe6595..beb02364 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -257,7 +257,13 @@ class WorkspaceFactory(Base): workspace.request.creator = owner WorkspaceRoleFactory.create( - workspace=workspace, role=Roles.get("owner"), user=owner, accepted=True + workspace=workspace, role=Roles.get("owner"), user=owner + ) + InvitationFactory.create( + user=owner, + inviter=owner, + workspace=workspace, + status=InvitationStatus.ACCEPTED, ) for member in members: @@ -275,7 +281,12 @@ class WorkspaceFactory(Base): user = UserFactory.create() workspace = WorkspaceFactory.create() Workspaces._create_workspace_role(user, workspace, role) - Workspaces.accept_workspace_role(user, workspace) + InvitationFactory.create( + user=user, + inviter=workspace.owner, + workspace=workspace, + status=InvitationStatus.ACCEPTED, + ) return user, workspace diff --git a/tests/routes/test_home.py b/tests/routes/test_home.py index 6a8a2ad5..877845a3 100644 --- a/tests/routes/test_home.py +++ b/tests/routes/test_home.py @@ -1,12 +1,23 @@ -from tests.factories import UserFactory, WorkspaceFactory, RequestFactory +from tests.factories import ( + UserFactory, + WorkspaceFactory, + RequestFactory, + InvitationFactory, +) from atst.domain.workspaces import Workspaces +from atst.models.invitation import Status as InvitationStatus 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) + InvitationFactory.create( + user=user, + inviter=workspace.owner, + workspace=workspace, + status=InvitationStatus.ACCEPTED, + ) 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 d90203de..cfc56f6f 100644 --- a/tests/routes/test_workspaces.py +++ b/tests/routes/test_workspaces.py @@ -341,11 +341,16 @@ def test_new_member_accept_invalid_invite(client, user_session): 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()}, + Invitations.create_for_owner(workspace, workspace.owner) + + # create user in workspace with invitation + user_session(workspace.owner) + response = client.post( + url_for("workspaces.create_member", workspace_id=workspace.id), + data={"workspace_role": "developer", **user.to_dictionary()}, ) + + # user tries to view workspace before accepting invitation user_session(user) response = client.get("/workspaces/{}/projects".format(workspace.id)) assert response.status_code == 404