From a85c475642db10a255b675061e1ad208a92c17b8 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 30 Oct 2018 11:29:27 -0400 Subject: [PATCH 01/23] update requests --- Pipfile.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index cf9730d8..11faecf5 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -322,11 +322,11 @@ }, "requests": { "hashes": [ - "sha256:63b52e3c866428a224f97cab011de738c36aec0185aa91cfacd418b5d58911d1", - "sha256:ec22d826a36ed72a7358ff3fe56cbd4ba69dd7a6718ffd450ff0e9df7a47ce6a" + "sha256:99dcfdaaeb17caf6e526f32b6a7b780461512ab3f1d992187801694cba42770c", + "sha256:a84b8c9ab6239b578f22d1c21d51b696dcfe004032bb80ea832398d6909d7279" ], "index": "pypi", - "version": "==2.19.1" + "version": "==2.20.0" }, "rq": { "hashes": [ From b8fc92cd14e228b973ce4683c8b9148ee60973c0 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 23 Oct 2018 15:49:51 -0400 Subject: [PATCH 02/23] basic invitation model with domain class --- .../versions/25bcba9b99a9_add_invitiations.py | 42 +++++++++++++ atst/domain/invitations.py | 62 +++++++++++++++++++ atst/models/__init__.py | 1 + atst/models/invitation.py | 25 ++++++++ tests/domain/test_invitations.py | 48 ++++++++++++++ tests/factories.py | 6 ++ 6 files changed, 184 insertions(+) create mode 100644 alembic/versions/25bcba9b99a9_add_invitiations.py create mode 100644 atst/domain/invitations.py create mode 100644 atst/models/invitation.py create mode 100644 tests/domain/test_invitations.py diff --git a/alembic/versions/25bcba9b99a9_add_invitiations.py b/alembic/versions/25bcba9b99a9_add_invitiations.py new file mode 100644 index 00000000..c2681435 --- /dev/null +++ b/alembic/versions/25bcba9b99a9_add_invitiations.py @@ -0,0 +1,42 @@ +"""add invitiations + +Revision ID: 25bcba9b99a9 +Revises: c99026ab9918 +Create Date: 2018-10-23 15:03:12.641069 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = '25bcba9b99a9' +down_revision = 'c99026ab9918' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('invitations', + sa.Column('time_created', sa.TIMESTAMP(timezone=True), server_default=sa.text('now()'), nullable=False), + sa.Column('time_updated', sa.TIMESTAMP(timezone=True), server_default=sa.text('now()'), nullable=False), + sa.Column('id', postgresql.UUID(as_uuid=True), server_default=sa.text('uuid_generate_v4()'), nullable=False), + sa.Column('user_id', postgresql.UUID(as_uuid=True), nullable=True), + sa.Column('workspace_id', postgresql.UUID(as_uuid=True), nullable=True), + sa.Column('valid', sa.Boolean(), nullable=True), + sa.ForeignKeyConstraint(['user_id'], ['users.id'], ), + sa.ForeignKeyConstraint(['workspace_id'], ['workspaces.id'], ), + sa.PrimaryKeyConstraint('id') + ) + op.create_index(op.f('ix_invitations_user_id'), 'invitations', ['user_id'], unique=False) + op.create_index(op.f('ix_invitations_workspace_id'), 'invitations', ['workspace_id'], unique=False) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_index(op.f('ix_invitations_workspace_id'), table_name='invitations') + op.drop_index(op.f('ix_invitations_user_id'), table_name='invitations') + op.drop_table('invitations') + # ### end Alembic commands ### diff --git a/atst/domain/invitations.py b/atst/domain/invitations.py new file mode 100644 index 00000000..2f067b36 --- /dev/null +++ b/atst/domain/invitations.py @@ -0,0 +1,62 @@ +import datetime +from sqlalchemy.orm.exc import NoResultFound + +from atst.database import db +from atst.models import Invitation + +from .exceptions import NotFoundError + + +class InvitationExpired(Exception): + def __init__(self, invite_id): + self.invite_id = invite_id + + @property + def message(self): + return "{} has expired".format(self.invite_id) + + +class Invitations(object): + EXPIRATION_LIMIT = 360 + + @classmethod + def _get(cls, invite_id): + try: + invite = db.session.query(Invitation).filter_by(id=invite_id).one() + except NoResultFound: + raise NotFoundError("invite") + + return invite + + @classmethod + def create(cls, workspace, user): + invite = Invitation(workspace=workspace, user=user, valid=True) + db.session.add(invite) + db.session.commit() + + return invite + + @classmethod + def accept(cls, invite_id): + invite = Invitations._get(invite_id) + valid = Invitations._is_valid(invite) + + invite.valid = False + db.session.add(invite) + db.session.commit() + + if not valid: + raise InvitationExpired(invite_id) + + return invite + + @classmethod + def _is_valid(cls, invite): + if not invite.valid: + return False + else: + time_created = invite.time_created + expiration = datetime.datetime.now( + time_created.tzinfo + ) - datetime.timedelta(minutes=Invitations.EXPIRATION_LIMIT) + return invite.time_created > expiration diff --git a/atst/models/__init__.py b/atst/models/__init__.py index 91cee016..27afc327 100644 --- a/atst/models/__init__.py +++ b/atst/models/__init__.py @@ -18,3 +18,4 @@ from .request_revision import RequestRevision from .request_review import RequestReview from .request_internal_comment import RequestInternalComment from .audit_event import AuditEvent +from .invitation import Invitation diff --git a/atst/models/invitation.py b/atst/models/invitation.py new file mode 100644 index 00000000..97158370 --- /dev/null +++ b/atst/models/invitation.py @@ -0,0 +1,25 @@ +from sqlalchemy import Column, ForeignKey, Boolean +from sqlalchemy.dialects.postgresql import UUID +from sqlalchemy.orm import relationship + +from atst.models import Base, types +from atst.models.mixins.timestamps import TimestampsMixin + + +class Invitation(Base, TimestampsMixin): + __tablename__ = "invitations" + + id = types.Id() + + user_id = Column(UUID(as_uuid=True), ForeignKey("users.id"), index=True) + user = relationship("User", backref="invitations") + + workspace_id = Column(UUID(as_uuid=True), ForeignKey("workspaces.id"), index=True) + workspace = relationship("Workspace", backref="invitations") + + valid = Column(Boolean, default=True) + + def __repr__(self): + return "".format( + self.user.id, self.workspace.id, self.id + ) diff --git a/tests/domain/test_invitations.py b/tests/domain/test_invitations.py new file mode 100644 index 00000000..b755bc3a --- /dev/null +++ b/tests/domain/test_invitations.py @@ -0,0 +1,48 @@ +import datetime +import pytest + +from atst.domain.invitations import Invitations, InvitationExpired + +from tests.factories import WorkspaceFactory, UserFactory, InvitationFactory + + +def test_create_invitation(): + workspace = WorkspaceFactory.create() + user = UserFactory.create() + invite = Invitations.create(workspace, user) + assert invite.user == user + assert invite.workspace == workspace + assert invite.valid + + +def test_accept_invitation(): + workspace = WorkspaceFactory.create() + user = UserFactory.create() + invite = Invitations.create(workspace, user) + assert invite.valid + accepted_invite = Invitations.accept(invite.id) + assert not accepted_invite.valid + + +def test_accept_expired_invitation(): + workspace = WorkspaceFactory.create() + user = UserFactory.create() + increment = Invitations.EXPIRATION_LIMIT + 1 + created_at = datetime.datetime.now() - datetime.timedelta(minutes=increment) + invite = InvitationFactory.create( + workspace_id=workspace.id, user_id=user.id, time_created=created_at, valid=True + ) + with pytest.raises(InvitationExpired): + Invitations.accept(invite.id) + + assert not invite.valid + + +def test_accept_invalid_invite(): + workspace = WorkspaceFactory.create() + user = UserFactory.create() + invite = InvitationFactory.create( + workspace_id=workspace.id, user_id=user.id, valid=False + ) + with pytest.raises(InvitationExpired): + Invitations.accept(invite.id) diff --git a/tests/factories.py b/tests/factories.py index b2338a06..ca40e61c 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -20,6 +20,7 @@ from atst.models.workspace import Workspace 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 class Base(factory.alchemy.SQLAlchemyModelFactory): @@ -325,3 +326,8 @@ class EnvironmentRoleFactory(Base): environment = factory.SubFactory(EnvironmentFactory) role = factory.Faker("name") user = factory.SubFactory(UserFactory) + + +class InvitationFactory(Base): + class Meta: + model = Invitation From 1c444c726cee199f69b5331dbace5465a31b1803 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 23 Oct 2018 16:54:51 -0400 Subject: [PATCH 03/23] When new member is added, an invite is sent to the new member --- atst/routes/workspaces.py | 23 +++++++++++++++++++++++ templates/emails/invitation.txt | 12 ++++++++++++ tests/routes/test_workspaces.py | 5 +++++ 3 files changed, 40 insertions(+) create mode 100644 templates/emails/invitation.txt diff --git a/atst/routes/workspaces.py b/atst/routes/workspaces.py index 5e295b70..f12a81ce 100644 --- a/atst/routes/workspaces.py +++ b/atst/routes/workspaces.py @@ -24,6 +24,8 @@ from atst.forms.workspace import WorkspaceForm from atst.forms.data import ENVIRONMENT_ROLES, ENV_ROLE_MODAL_DESCRIPTION from atst.domain.authz import Authorization from atst.models.permissions import Permissions +from atst.domain.invitations import Invitations +from atst.queue import queue bp = Blueprint("workspaces", __name__) @@ -218,6 +220,17 @@ def new_member(workspace_id): ) +def send_invite_email(owner_name, invite_id, new_member_email): + body = render_template( + "emails/invitation.txt", owner=owner_name, invite_id=invite_id + ) + queue.send_mail( + [new_member_email], + "{} has invited you to a JEDI Cloud Workspace".format(owner_name), + body, + ) + + @bp.route("/workspaces//members/new", methods=["POST"]) def create_member(workspace_id): workspace = Workspaces.get(g.current_user, workspace_id) @@ -226,6 +239,11 @@ def create_member(workspace_id): if form.validate(): try: new_member = Workspaces.create_member(g.current_user, workspace, form.data) + invite = Invitations.create(workspace, new_member.user) + send_invite_email( + g.current_user.full_name, invite.id, new_member.user.email + ) + return redirect( url_for( "workspaces.workspace_members", @@ -318,3 +336,8 @@ def update_member(workspace_id, member_id): workspace=workspace, member=member, ) + + +@bp.route("/workspaces/invitation/", methods=["GET"]) +def accept_invitation(invite_id): + pass diff --git a/templates/emails/invitation.txt b/templates/emails/invitation.txt new file mode 100644 index 00000000..789da574 --- /dev/null +++ b/templates/emails/invitation.txt @@ -0,0 +1,12 @@ +Join this JEDI Cloud Workspace +{{ owner }} has invited you to join a JEDI Cloud Workspace. Login now to view or use your JEDI Cloud resources. + +{{ url_for("workspaces.accept_invitation", invite_id=invite_id, _external=True) }} + +What is JEDI Cloud? +JEDI Cloud is a DoD enterprise-wide solution for commercial cloud services. + +What is a JEDI Cloud Workspace? +A JEDI Cloud Workspace is where you may access and manage the cloud resources associated with your projects and environments. + +JEDI Cloud is managed by the Cloud Computing Program Office. Learn more at jedi.cloud. diff --git a/tests/routes/test_workspaces.py b/tests/routes/test_workspaces.py index f345f462..75fe0662 100644 --- a/tests/routes/test_workspaces.py +++ b/tests/routes/test_workspaces.py @@ -7,6 +7,7 @@ from atst.domain.projects import Projects from atst.domain.environments import Environments from atst.domain.environment_roles import EnvironmentRoles from atst.models.workspace_user import WorkspaceUser +from atst.queue import queue def test_user_with_permission_has_budget_report_link(client, user_session): @@ -181,6 +182,8 @@ def test_create_member(client, user_session): workspace = WorkspaceFactory.create() Workspaces._create_workspace_role(owner, workspace, "admin") user_session(owner) + queue_length = len(queue.get_queue()) + response = client.post( url_for("workspaces.create_member", workspace_id=workspace.id), data={ @@ -195,6 +198,8 @@ def test_create_member(client, user_session): assert response.status_code == 200 assert user.has_workspaces + assert user.invitations + assert len(queue.get_queue()) == queue_length + 1 def test_permissions_for_view_member(client, user_session): From 370b037d991f6447d707f3394584ebd68b3fe108 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 25 Oct 2018 10:15:02 -0400 Subject: [PATCH 04/23] Create route for accepting an invite --- atst/routes/errors.py | 12 +++++++++++ atst/routes/workspaces.py | 6 +++++- tests/routes/test_workspaces.py | 38 ++++++++++++++++++++++++++++++++- 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/atst/routes/errors.py b/atst/routes/errors.py index b3e7effd..b579996d 100644 --- a/atst/routes/errors.py +++ b/atst/routes/errors.py @@ -3,6 +3,7 @@ from flask_wtf.csrf import CSRFError import werkzeug.exceptions as werkzeug_exceptions import atst.domain.exceptions as exceptions +from atst.domain.invitations import InvitationExpired def make_error_pages(app): @@ -41,4 +42,15 @@ def make_error_pages(app): 500, ) + @app.errorhandler(InvitationExpired) + # pylint: disable=unused-variable + def expired_invitation(e): + log_error(e) + return ( + render_template( + "error.html", message="The invitation you followed has expired." + ), + 404, + ) + return app diff --git a/atst/routes/workspaces.py b/atst/routes/workspaces.py index f12a81ce..674f9dd3 100644 --- a/atst/routes/workspaces.py +++ b/atst/routes/workspaces.py @@ -340,4 +340,8 @@ def update_member(workspace_id, member_id): @bp.route("/workspaces/invitation/", methods=["GET"]) def accept_invitation(invite_id): - pass + invite = Invitations.accept(invite_id) + + return redirect( + url_for("workspaces.show_workspace", workspace_id=invite.workspace.id) + ) diff --git a/tests/routes/test_workspaces.py b/tests/routes/test_workspaces.py index 75fe0662..32a85f8e 100644 --- a/tests/routes/test_workspaces.py +++ b/tests/routes/test_workspaces.py @@ -1,11 +1,12 @@ from flask import url_for -from tests.factories import UserFactory, WorkspaceFactory +from tests.factories import UserFactory, WorkspaceFactory, InvitationFactory 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.domain.environment_roles import EnvironmentRoles +from atst.domain.invitations import Invitations from atst.models.workspace_user import WorkspaceUser from atst.queue import queue @@ -313,3 +314,38 @@ def test_update_member_environment_role_with_no_data(client, user_session): ) assert response.status_code == 200 assert EnvironmentRoles.get(user.id, env1_id).role == "developer" + + +def test_new_member_accept_valid_invite(client, user_session): + owner = UserFactory.create() + workspace = WorkspaceFactory.create() + Workspaces._create_workspace_role(owner, workspace, "admin") + + user = UserFactory.create() + member = WorkspaceUsers.add(user, workspace.id, "developer") + invite = InvitationFactory.create(user_id=member.user.id, workspace_id=workspace.id) + user_session(user) + response = client.get(url_for("workspaces.accept_invitation", invite_id=invite.id)) + + assert response.status_code == 302 + assert ( + url_for("workspaces.show_workspace", workspace_id=invite.workspace.id) + in response.headers["Location"] + ) + assert invite.valid == False + + +def test_new_member_accept_invalid_invite(client, user_session): + owner = UserFactory.create() + workspace = WorkspaceFactory.create() + Workspaces._create_workspace_role(owner, workspace, "admin") + + user = UserFactory.create() + member = WorkspaceUsers.add(user, workspace.id, "developer") + invite = InvitationFactory.create( + user_id=member.user.id, workspace_id=workspace.id, valid=False + ) + user_session(user) + response = client.get(url_for("workspaces.accept_invitation", invite_id=invite.id)) + + assert response.status_code == 404 From 8f146b2feee3677267908c6dd6bfa2285d4707c7 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 25 Oct 2018 10:41:20 -0400 Subject: [PATCH 05/23] break out expiration method in Invitations domain --- atst/domain/invitations.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/atst/domain/invitations.py b/atst/domain/invitations.py index 2f067b36..777dbc10 100644 --- a/atst/domain/invitations.py +++ b/atst/domain/invitations.py @@ -39,7 +39,7 @@ class Invitations(object): @classmethod def accept(cls, invite_id): invite = Invitations._get(invite_id) - valid = Invitations._is_valid(invite) + valid = Invitations.is_valid(invite) invite.valid = False db.session.add(invite) @@ -51,12 +51,13 @@ class Invitations(object): return invite @classmethod - def _is_valid(cls, invite): - if not invite.valid: - return False - else: - time_created = invite.time_created - expiration = datetime.datetime.now( - time_created.tzinfo - ) - datetime.timedelta(minutes=Invitations.EXPIRATION_LIMIT) - return invite.time_created > expiration + def is_valid(cls, invite): + return invite.valid and not Invitations.is_expired(invite) + + @classmethod + def is_expired(cls, invite): + time_created = invite.time_created + expiration = datetime.datetime.now(time_created.tzinfo) - datetime.timedelta( + minutes=Invitations.EXPIRATION_LIMIT + ) + return invite.time_created < expiration From 49f5edfe29e63aa1c41a5c93be15da3882c75f62 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 25 Oct 2018 11:25:58 -0400 Subject: [PATCH 06/23] new workspace members are provisional until they log in for the first time --- ...c1ac77c_add_provisional_column_to_users.py | 28 +++++++++++++++++++ atst/domain/users.py | 9 ++++++ atst/domain/workspaces/workspaces.py | 1 + atst/models/user.py | 4 ++- atst/routes/__init__.py | 4 +++ tests/domain/test_workspaces.py | 17 +++++++++++ tests/test_auth.py | 14 ++++++++++ 7 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 alembic/versions/5284ac1ac77c_add_provisional_column_to_users.py diff --git a/alembic/versions/5284ac1ac77c_add_provisional_column_to_users.py b/alembic/versions/5284ac1ac77c_add_provisional_column_to_users.py new file mode 100644 index 00000000..8ad52723 --- /dev/null +++ b/alembic/versions/5284ac1ac77c_add_provisional_column_to_users.py @@ -0,0 +1,28 @@ +"""add provisional column to users + +Revision ID: 5284ac1ac77c +Revises: 25bcba9b99a9 +Create Date: 2018-10-25 11:04:49.879393 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '5284ac1ac77c' +down_revision = '25bcba9b99a9' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('users', sa.Column('provisional', sa.Boolean(), nullable=True)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('users', 'provisional') + # ### end Alembic commands ### diff --git a/atst/domain/users.py b/atst/domain/users.py index 7494f5f4..97947080 100644 --- a/atst/domain/users.py +++ b/atst/domain/users.py @@ -89,3 +89,12 @@ class Users(object): db.session.commit() return user + + @classmethod + def finalize(cls, user): + user.provisional = False + + db.session.add(user) + db.session.commit() + + return user diff --git a/atst/domain/workspaces/workspaces.py b/atst/domain/workspaces/workspaces.py index f8e2c560..dc2f4202 100644 --- a/atst/domain/workspaces/workspaces.py +++ b/atst/domain/workspaces/workspaces.py @@ -86,6 +86,7 @@ class Workspaces(object): last_name=data["last_name"], email=data["email"], atat_role_name="default", + provisional=True, ) return Workspaces.add_member(workspace, new_user, data["workspace_role"]) diff --git a/atst/models/user.py b/atst/models/user.py index bb89c51b..5f8765dd 100644 --- a/atst/models/user.py +++ b/atst/models/user.py @@ -1,4 +1,4 @@ -from sqlalchemy import String, ForeignKey, Column, Date +from sqlalchemy import String, ForeignKey, Column, Date, Boolean from sqlalchemy.orm import relationship from sqlalchemy.dialects.postgresql import UUID @@ -26,6 +26,8 @@ class User(Base, mixins.TimestampsMixin, mixins.AuditableMixin): designation = Column(String) date_latest_training = Column(Date) + provisional = Column(Boolean) + @property def atat_permissions(self): return self.atat_role.permissions diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index 5051ad5a..0149ace5 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -101,6 +101,10 @@ def login_redirect(): auth_context = _make_authentication_context() auth_context.authenticate() user = auth_context.get_user() + + if user.provisional: + Users.finalize(user) + session["user_id"] = user.id return redirect(redirect_after_login_url()) diff --git a/tests/domain/test_workspaces.py b/tests/domain/test_workspaces.py index 86064c1e..79d8c57b 100644 --- a/tests/domain/test_workspaces.py +++ b/tests/domain/test_workspaces.py @@ -86,6 +86,23 @@ def test_can_create_workspace_user(workspace, workspace_owner): new_member = Workspaces.create_member(workspace_owner, workspace, user_data) assert new_member.workspace == workspace + assert new_member.user.provisional + + +def test_can_add_existing_user_to_workspace(workspace, workspace_owner): + user = UserFactory.create() + user_data = { + "first_name": "New", + "last_name": "User", + "email": "new.user@mail.com", + "workspace_role": "developer", + "dod_id": user.dod_id, + } + + new_member = Workspaces.create_member(workspace_owner, workspace, user_data) + assert new_member.workspace == workspace + assert new_member.user.email == user.email + assert not new_member.user.provisional def test_need_permission_to_create_workspace_user(workspace, workspace_owner): diff --git a/tests/test_auth.py b/tests/test_auth.py index 099dc4ec..13a6896c 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -211,3 +211,17 @@ def test_redirected_on_login(client, monkeypatch): target_route = url_for("requests.requests_form_new", screen=1) response = _login(client, next=target_route) assert target_route in response.headers.get("Location") + + +def test_invited_user_finalized_on_login(monkeypatch, client): + user = UserFactory.create(provisional=True) + monkeypatch.setattr( + "atst.domain.authnid.AuthenticationContext.authenticate", lambda *args: True + ) + monkeypatch.setattr( + "atst.domain.authnid.AuthenticationContext.get_user", lambda *args: user + ) + + resp = _login(client) + + assert not user.provisional From 3e19c75c801b1a8901ced2030e690941e8345dc1 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 25 Oct 2018 13:21:17 -0400 Subject: [PATCH 07/23] workspace role marked as accepted when user accepts invitation --- ...c_add_accepted_column_to_workspace_role.py | 28 +++++++++++++++++++ atst/domain/workspace_users.py | 2 +- atst/domain/workspaces/query.py | 10 +++++++ atst/domain/workspaces/workspaces.py | 8 ++++++ atst/models/workspace_role.py | 4 ++- atst/routes/workspaces.py | 1 + tests/domain/test_workspaces.py | 11 ++++++++ tests/routes/test_workspaces.py | 10 ++++++- 8 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 alembic/versions/c7feaa7b6b0c_add_accepted_column_to_workspace_role.py diff --git a/alembic/versions/c7feaa7b6b0c_add_accepted_column_to_workspace_role.py b/alembic/versions/c7feaa7b6b0c_add_accepted_column_to_workspace_role.py new file mode 100644 index 00000000..9a1fb904 --- /dev/null +++ b/alembic/versions/c7feaa7b6b0c_add_accepted_column_to_workspace_role.py @@ -0,0 +1,28 @@ +"""add accepted column to workspace role + +Revision ID: c7feaa7b6b0c +Revises: 5284ac1ac77c +Create Date: 2018-10-25 11:44:17.654892 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'c7feaa7b6b0c' +down_revision = '5284ac1ac77c' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('workspace_roles', sa.Column('accepted', sa.Boolean(), nullable=True)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('workspace_roles', 'accepted') + # ### end Alembic commands ### diff --git a/atst/domain/workspace_users.py b/atst/domain/workspace_users.py index d37ec736..84e20c50 100644 --- a/atst/domain/workspace_users.py +++ b/atst/domain/workspace_users.py @@ -63,7 +63,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 + user=user, role_id=role.id, workspace_id=workspace_id, accepted=False ) user.workspace_roles.append(new_workspace_role) diff --git a/atst/domain/workspaces/query.py b/atst/domain/workspaces/query.py index ec32c73c..45f29b3b 100644 --- a/atst/domain/workspaces/query.py +++ b/atst/domain/workspaces/query.py @@ -25,9 +25,19 @@ class WorkspacesQuery(Query): db.session.query(Workspace) .join(WorkspaceRole) .filter(WorkspaceRole.user == user) + .filter(WorkspaceRole.accepted == True) .all() ) @classmethod def create_workspace_role(cls, user, role, workspace): return WorkspaceRole(user=user, role=role, workspace=workspace) + + @classmethod + def get_role_for_workspace_and_user(cls, workspace, user): + return ( + db.session.query(WorkspaceRole) + .filter(WorkspaceRole.user == user) + .filter(WorkspaceRole.workspace == workspace) + .one() + ) diff --git a/atst/domain/workspaces/workspaces.py b/atst/domain/workspaces/workspaces.py index dc2f4202..b3884ed2 100644 --- a/atst/domain/workspaces/workspaces.py +++ b/atst/domain/workspaces/workspaces.py @@ -119,3 +119,11 @@ 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_role.py b/atst/models/workspace_role.py index 87c8f46f..93da1488 100644 --- a/atst/models/workspace_role.py +++ b/atst/models/workspace_role.py @@ -1,4 +1,4 @@ -from sqlalchemy import Index, ForeignKey, Column +from sqlalchemy import Index, ForeignKey, Column, Boolean from sqlalchemy.dialects.postgresql import UUID from sqlalchemy.orm import relationship @@ -22,6 +22,8 @@ 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/routes/workspaces.py b/atst/routes/workspaces.py index 674f9dd3..cc2f9f48 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( 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 79d8c57b..6141f9bf 100644 --- a/tests/domain/test_workspaces.py +++ b/tests/domain/test_workspaces.py @@ -247,11 +247,22 @@ 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) + bobs_workspaces = Workspaces.for_user(bob) assert len(bobs_workspaces) == 1 +def test_for_user_does_not_return_unaccepted_workspaces(workspace, workspace_owner): + bob = UserFactory.from_atat_role("default") + Workspaces.add_member(workspace, bob, "developer") + Workspaces.create(RequestFactory.create()) + bobs_workspaces = Workspaces.for_user(bob) + + assert len(bobs_workspaces) == 0 + + def test_for_user_returns_all_workspaces_for_ccpo(workspace, workspace_owner): sam = UserFactory.from_atat_role("ccpo") Workspaces.create(RequestFactory.create()) diff --git a/tests/routes/test_workspaces.py b/tests/routes/test_workspaces.py index 32a85f8e..2edaa0ee 100644 --- a/tests/routes/test_workspaces.py +++ b/tests/routes/test_workspaces.py @@ -316,7 +316,7 @@ def test_update_member_environment_role_with_no_data(client, user_session): assert EnvironmentRoles.get(user.id, env1_id).role == "developer" -def test_new_member_accept_valid_invite(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") @@ -324,15 +324,23 @@ def test_new_member_accept_valid_invite(client, user_session): user = UserFactory.create() member = WorkspaceUsers.add(user, workspace.id, "developer") invite = InvitationFactory.create(user_id=member.user.id, workspace_id=workspace.id) + + # the user does not have access to the workspace before accepting the invite + assert len(Workspaces.for_user(user)) == 0 + user_session(user) response = client.get(url_for("workspaces.accept_invitation", invite_id=invite.id)) + # user is redirected to the workspace view assert response.status_code == 302 assert ( url_for("workspaces.show_workspace", workspace_id=invite.workspace.id) in response.headers["Location"] ) + # the one-time use invite is no longer usable assert invite.valid == False + # the user has access to the workspace + assert len(Workspaces.for_user(user)) == 1 def test_new_member_accept_invalid_invite(client, user_session): From 5c2d466049cf186c76320660d567c37159e2a06b Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 25 Oct 2018 15:14:14 -0400 Subject: [PATCH 08/23] 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 From edede87108cbfeb7538071ebfff23bbdb18e9756 Mon Sep 17 00:00:00 2001 From: dandds Date: Fri, 26 Oct 2018 11:38:43 -0400 Subject: [PATCH 09/23] specify unit for invitation expiration --- atst/domain/invitations.py | 5 +++-- tests/domain/test_invitations.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/atst/domain/invitations.py b/atst/domain/invitations.py index 777dbc10..fb4e30dc 100644 --- a/atst/domain/invitations.py +++ b/atst/domain/invitations.py @@ -17,7 +17,8 @@ class InvitationExpired(Exception): class Invitations(object): - EXPIRATION_LIMIT = 360 + # number of minutes a given invitation is considered valid + EXPIRATION_LIMIT_MINUTES = 360 @classmethod def _get(cls, invite_id): @@ -58,6 +59,6 @@ class Invitations(object): def is_expired(cls, invite): time_created = invite.time_created expiration = datetime.datetime.now(time_created.tzinfo) - datetime.timedelta( - minutes=Invitations.EXPIRATION_LIMIT + minutes=Invitations.EXPIRATION_LIMIT_MINUTES ) return invite.time_created < expiration diff --git a/tests/domain/test_invitations.py b/tests/domain/test_invitations.py index b755bc3a..5a51ec2b 100644 --- a/tests/domain/test_invitations.py +++ b/tests/domain/test_invitations.py @@ -27,7 +27,7 @@ def test_accept_invitation(): def test_accept_expired_invitation(): workspace = WorkspaceFactory.create() user = UserFactory.create() - increment = Invitations.EXPIRATION_LIMIT + 1 + increment = Invitations.EXPIRATION_LIMIT_MINUTES + 1 created_at = datetime.datetime.now() - datetime.timedelta(minutes=increment) invite = InvitationFactory.create( workspace_id=workspace.id, user_id=user.id, time_created=created_at, valid=True From 6125041a93edf17403fa80660012fceb318f3c7a Mon Sep 17 00:00:00 2001 From: dandds Date: Fri, 26 Oct 2018 12:54:28 -0400 Subject: [PATCH 10/23] record inviter on invitation --- ..._add_inviter_relationship_to_invitation.py | 32 +++++++++++++++++++ atst/domain/invitations.py | 4 +-- atst/models/invitation.py | 5 ++- atst/routes/workspaces.py | 2 +- tests/domain/test_invitations.py | 5 +-- 5 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 alembic/versions/2bec1868a22a_add_inviter_relationship_to_invitation.py diff --git a/alembic/versions/2bec1868a22a_add_inviter_relationship_to_invitation.py b/alembic/versions/2bec1868a22a_add_inviter_relationship_to_invitation.py new file mode 100644 index 00000000..23c75457 --- /dev/null +++ b/alembic/versions/2bec1868a22a_add_inviter_relationship_to_invitation.py @@ -0,0 +1,32 @@ +"""add inviter relationship to invitation + +Revision ID: 2bec1868a22a +Revises: c7feaa7b6b0c +Create Date: 2018-10-26 12:45:13.192062 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = '2bec1868a22a' +down_revision = 'c7feaa7b6b0c' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('invitations', sa.Column('inviter_id', postgresql.UUID(as_uuid=True), nullable=True)) + op.create_index(op.f('ix_invitations_inviter_id'), 'invitations', ['inviter_id'], unique=False) + op.create_foreign_key(None, 'invitations', 'users', ['inviter_id'], ['id']) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint(None, 'invitations', type_='foreignkey') + op.drop_index(op.f('ix_invitations_inviter_id'), table_name='invitations') + op.drop_column('invitations', 'inviter_id') + # ### end Alembic commands ### diff --git a/atst/domain/invitations.py b/atst/domain/invitations.py index fb4e30dc..0f36cecb 100644 --- a/atst/domain/invitations.py +++ b/atst/domain/invitations.py @@ -30,8 +30,8 @@ class Invitations(object): return invite @classmethod - def create(cls, workspace, user): - invite = Invitation(workspace=workspace, user=user, valid=True) + def create(cls, workspace, inviter, user): + invite = Invitation(workspace=workspace, inviter=inviter, user=user, valid=True) db.session.add(invite) db.session.commit() diff --git a/atst/models/invitation.py b/atst/models/invitation.py index 97158370..95d170ac 100644 --- a/atst/models/invitation.py +++ b/atst/models/invitation.py @@ -12,11 +12,14 @@ class Invitation(Base, TimestampsMixin): id = types.Id() user_id = Column(UUID(as_uuid=True), ForeignKey("users.id"), index=True) - user = relationship("User", backref="invitations") + user = relationship("User", backref="invitations", foreign_keys=[user_id]) workspace_id = Column(UUID(as_uuid=True), ForeignKey("workspaces.id"), index=True) workspace = relationship("Workspace", backref="invitations") + inviter_id = Column(UUID(as_uuid=True), ForeignKey("users.id"), index=True) + inviter = relationship("User", backref="sent_invites", foreign_keys=[inviter_id]) + valid = Column(Boolean, default=True) def __repr__(self): diff --git a/atst/routes/workspaces.py b/atst/routes/workspaces.py index fbd14ceb..6dd60b6c 100644 --- a/atst/routes/workspaces.py +++ b/atst/routes/workspaces.py @@ -239,7 +239,7 @@ def create_member(workspace_id): if form.validate(): try: new_member = Workspaces.create_member(g.current_user, workspace, form.data) - invite = Invitations.create(workspace, new_member.user) + invite = Invitations.create(workspace, g.current_user, new_member.user) send_invite_email( g.current_user.full_name, invite.id, new_member.user.email ) diff --git a/tests/domain/test_invitations.py b/tests/domain/test_invitations.py index 5a51ec2b..c0fc8901 100644 --- a/tests/domain/test_invitations.py +++ b/tests/domain/test_invitations.py @@ -9,16 +9,17 @@ from tests.factories import WorkspaceFactory, UserFactory, InvitationFactory def test_create_invitation(): workspace = WorkspaceFactory.create() user = UserFactory.create() - invite = Invitations.create(workspace, user) + invite = Invitations.create(workspace, workspace.owner, user) assert invite.user == user assert invite.workspace == workspace + assert invite.inviter == workspace.owner assert invite.valid def test_accept_invitation(): workspace = WorkspaceFactory.create() user = UserFactory.create() - invite = Invitations.create(workspace, user) + invite = Invitations.create(workspace, workspace.owner, user) assert invite.valid accepted_invite = Invitations.accept(invite.id) assert not accepted_invite.valid From d5998ed3700cc3161af2024a342bcf4bdf0984d3 Mon Sep 17 00:00:00 2001 From: dandds Date: Fri, 26 Oct 2018 13:31:21 -0400 Subject: [PATCH 11/23] track invitation state by status enum --- .../03654d08f5ff_add_status_to_invitation.py | 30 ++++++++++++++++ atst/domain/invitations.py | 34 +++++++++++-------- atst/models/invitation.py | 29 ++++++++++++++-- atst/routes/errors.py | 4 +-- tests/domain/test_invitations.py | 34 +++++++++++++------ tests/factories.py | 4 ++- tests/routes/test_workspaces.py | 7 ++-- 7 files changed, 110 insertions(+), 32 deletions(-) create mode 100644 alembic/versions/03654d08f5ff_add_status_to_invitation.py diff --git a/alembic/versions/03654d08f5ff_add_status_to_invitation.py b/alembic/versions/03654d08f5ff_add_status_to_invitation.py new file mode 100644 index 00000000..0d06b1e5 --- /dev/null +++ b/alembic/versions/03654d08f5ff_add_status_to_invitation.py @@ -0,0 +1,30 @@ +"""add status to invitation + +Revision ID: 03654d08f5ff +Revises: 2bec1868a22a +Create Date: 2018-10-26 12:59:16.709080 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '03654d08f5ff' +down_revision = '2bec1868a22a' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('invitations', sa.Column('status', sa.Enum('ACCEPTED', 'REVOKED', 'PENDING', 'REJECTED', name='status', native_enum=False), nullable=True)) + op.drop_column('invitations', 'valid') + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('invitations', sa.Column('valid', sa.BOOLEAN(), autoincrement=False, nullable=True)) + op.drop_column('invitations', 'status') + # ### end Alembic commands ### diff --git a/atst/domain/invitations.py b/atst/domain/invitations.py index 0f36cecb..c2e2be2e 100644 --- a/atst/domain/invitations.py +++ b/atst/domain/invitations.py @@ -2,18 +2,18 @@ import datetime from sqlalchemy.orm.exc import NoResultFound from atst.database import db -from atst.models import Invitation +from atst.models.invitation import Invitation, Status as InvitationStatus from .exceptions import NotFoundError -class InvitationExpired(Exception): - def __init__(self, invite_id): - self.invite_id = invite_id +class InvitationError(Exception): + def __init__(self, invite): + self.invite = invite @property def message(self): - return "{} has expired".format(self.invite_id) + return "{} has a status of {}".format(self.invite.id, self.invite.status.value) class Invitations(object): @@ -31,7 +31,12 @@ class Invitations(object): @classmethod def create(cls, workspace, inviter, user): - invite = Invitation(workspace=workspace, inviter=inviter, user=user, valid=True) + invite = Invitation( + workspace=workspace, + inviter=inviter, + user=user, + status=InvitationStatus.PENDING, + ) db.session.add(invite) db.session.commit() @@ -40,23 +45,22 @@ class Invitations(object): @classmethod def accept(cls, invite_id): invite = Invitations._get(invite_id) - valid = Invitations.is_valid(invite) - invite.valid = False + if Invitations._is_expired(invite): + invite.status = InvitationStatus.REJECTED + elif invite.is_pending: + invite.status = InvitationStatus.ACCEPTED + db.session.add(invite) db.session.commit() - if not valid: - raise InvitationExpired(invite_id) + if invite.is_revoked or invite.is_rejected: + raise InvitationError(invite) return invite @classmethod - def is_valid(cls, invite): - return invite.valid and not Invitations.is_expired(invite) - - @classmethod - def is_expired(cls, invite): + def _is_expired(cls, invite): time_created = invite.time_created expiration = datetime.datetime.now(time_created.tzinfo) - datetime.timedelta( minutes=Invitations.EXPIRATION_LIMIT_MINUTES diff --git a/atst/models/invitation.py b/atst/models/invitation.py index 95d170ac..0d8bc8d8 100644 --- a/atst/models/invitation.py +++ b/atst/models/invitation.py @@ -1,4 +1,6 @@ -from sqlalchemy import Column, ForeignKey, Boolean +from enum import Enum + +from sqlalchemy import Column, ForeignKey, Enum as SQLAEnum from sqlalchemy.dialects.postgresql import UUID from sqlalchemy.orm import relationship @@ -6,6 +8,13 @@ from atst.models import Base, types from atst.models.mixins.timestamps import TimestampsMixin +class Status(Enum): + ACCEPTED = "accepted" + REVOKED = "revoked" + PENDING = "pending" + REJECTED = "rejected" + + class Invitation(Base, TimestampsMixin): __tablename__ = "invitations" @@ -20,9 +29,25 @@ class Invitation(Base, TimestampsMixin): inviter_id = Column(UUID(as_uuid=True), ForeignKey("users.id"), index=True) inviter = relationship("User", backref="sent_invites", foreign_keys=[inviter_id]) - valid = Column(Boolean, default=True) + status = Column(SQLAEnum(Status, native_enum=False, default=Status.PENDING)) def __repr__(self): return "".format( self.user.id, self.workspace.id, self.id ) + + @property + def is_accepted(self): + return self.status == Status.ACCEPTED + + @property + def is_revoked(self): + return self.status == Status.REVOKED + + @property + def is_pending(self): + return self.status == Status.PENDING + + @property + def is_rejected(self): + return self.status == Status.REJECTED diff --git a/atst/routes/errors.py b/atst/routes/errors.py index b579996d..34830911 100644 --- a/atst/routes/errors.py +++ b/atst/routes/errors.py @@ -3,7 +3,7 @@ from flask_wtf.csrf import CSRFError import werkzeug.exceptions as werkzeug_exceptions import atst.domain.exceptions as exceptions -from atst.domain.invitations import InvitationExpired +from atst.domain.invitations import InvitationError def make_error_pages(app): @@ -42,7 +42,7 @@ def make_error_pages(app): 500, ) - @app.errorhandler(InvitationExpired) + @app.errorhandler(InvitationError) # pylint: disable=unused-variable def expired_invitation(e): log_error(e) diff --git a/tests/domain/test_invitations.py b/tests/domain/test_invitations.py index c0fc8901..864fa79d 100644 --- a/tests/domain/test_invitations.py +++ b/tests/domain/test_invitations.py @@ -1,7 +1,8 @@ import datetime import pytest -from atst.domain.invitations import Invitations, InvitationExpired +from atst.domain.invitations import Invitations, InvitationError +from atst.models.invitation import Status from tests.factories import WorkspaceFactory, UserFactory, InvitationFactory @@ -13,16 +14,16 @@ def test_create_invitation(): assert invite.user == user assert invite.workspace == workspace assert invite.inviter == workspace.owner - assert invite.valid + assert invite.status == Status.PENDING def test_accept_invitation(): workspace = WorkspaceFactory.create() user = UserFactory.create() invite = Invitations.create(workspace, workspace.owner, user) - assert invite.valid + assert invite.is_pending accepted_invite = Invitations.accept(invite.id) - assert not accepted_invite.valid + assert accepted_invite.is_accepted def test_accept_expired_invitation(): @@ -31,19 +32,32 @@ def test_accept_expired_invitation(): increment = Invitations.EXPIRATION_LIMIT_MINUTES + 1 created_at = datetime.datetime.now() - datetime.timedelta(minutes=increment) invite = InvitationFactory.create( - workspace_id=workspace.id, user_id=user.id, time_created=created_at, valid=True + workspace_id=workspace.id, + user_id=user.id, + time_created=created_at, + status=Status.PENDING, ) - with pytest.raises(InvitationExpired): + with pytest.raises(InvitationError): Invitations.accept(invite.id) - assert not invite.valid + assert invite.is_rejected -def test_accept_invalid_invite(): +def test_accept_rejected_invite(): workspace = WorkspaceFactory.create() user = UserFactory.create() invite = InvitationFactory.create( - workspace_id=workspace.id, user_id=user.id, valid=False + workspace_id=workspace.id, user_id=user.id, status=Status.REJECTED ) - with pytest.raises(InvitationExpired): + with pytest.raises(InvitationError): + Invitations.accept(invite.id) + + +def test_accept_revoked_invite(): + workspace = WorkspaceFactory.create() + user = UserFactory.create() + invite = InvitationFactory.create( + workspace_id=workspace.id, user_id=user.id, status=Status.REVOKED + ) + with pytest.raises(InvitationError): Invitations.accept(invite.id) diff --git a/tests/factories.py b/tests/factories.py index 0db8128f..eac728a9 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -20,7 +20,7 @@ from atst.models.workspace import Workspace 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.models.invitation import Invitation, Status as InvitationStatus from atst.domain.workspaces import Workspaces @@ -339,3 +339,5 @@ class EnvironmentRoleFactory(Base): class InvitationFactory(Base): class Meta: model = Invitation + + status = InvitationStatus.PENDING diff --git a/tests/routes/test_workspaces.py b/tests/routes/test_workspaces.py index f9d460a5..d90203de 100644 --- a/tests/routes/test_workspaces.py +++ b/tests/routes/test_workspaces.py @@ -8,6 +8,7 @@ from atst.domain.environments import Environments from atst.domain.environment_roles import EnvironmentRoles from atst.domain.invitations import Invitations from atst.models.workspace_user import WorkspaceUser +from atst.models.invitation import Status as InvitationStatus from atst.queue import queue @@ -316,7 +317,7 @@ def test_new_member_accepts_valid_invite(client, user_session): in response.headers["Location"] ) # the one-time use invite is no longer usable - assert invite.valid == False + assert invite.is_accepted # the user has access to the workspace assert len(Workspaces.for_user(user)) == 1 @@ -327,7 +328,9 @@ def test_new_member_accept_invalid_invite(client, user_session): user = UserFactory.create() member = WorkspaceUsers.add(user, workspace.id, "developer") invite = InvitationFactory.create( - user_id=member.user.id, workspace_id=workspace.id, valid=False + user_id=member.user.id, + workspace_id=workspace.id, + status=InvitationStatus.REJECTED, ) user_session(user) response = client.get(url_for("workspaces.accept_invitation", invite_id=invite.id)) From 5c5f9c6c9ccd0e4372fb83fa83f97171ebba1cfd Mon Sep 17 00:00:00 2001 From: dandds Date: Fri, 26 Oct 2018 13:43:40 -0400 Subject: [PATCH 12/23] record expiration time on the invitation --- ...60c26_add_expiration_time_to_invitation.py | 28 +++++++++++++++++++ atst/domain/invitations.py | 9 +++--- atst/models/invitation.py | 9 +++++- tests/domain/test_invitations.py | 4 +-- tests/factories.py | 2 ++ 5 files changed, 44 insertions(+), 8 deletions(-) create mode 100644 alembic/versions/e62bcc460c26_add_expiration_time_to_invitation.py diff --git a/alembic/versions/e62bcc460c26_add_expiration_time_to_invitation.py b/alembic/versions/e62bcc460c26_add_expiration_time_to_invitation.py new file mode 100644 index 00000000..9b6fc4d1 --- /dev/null +++ b/alembic/versions/e62bcc460c26_add_expiration_time_to_invitation.py @@ -0,0 +1,28 @@ +"""add expiration_time to invitation + +Revision ID: e62bcc460c26 +Revises: 03654d08f5ff +Create Date: 2018-10-26 13:37:39.015003 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'e62bcc460c26' +down_revision = '03654d08f5ff' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('invitations', sa.Column('expiration_time', sa.TIMESTAMP(timezone=True), nullable=True)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('invitations', 'expiration_time') + # ### end Alembic commands ### diff --git a/atst/domain/invitations.py b/atst/domain/invitations.py index c2e2be2e..fc54c43c 100644 --- a/atst/domain/invitations.py +++ b/atst/domain/invitations.py @@ -36,6 +36,7 @@ class Invitations(object): inviter=inviter, user=user, status=InvitationStatus.PENDING, + expiration_time=Invitations.current_expiration_time(), ) db.session.add(invite) db.session.commit() @@ -46,7 +47,7 @@ class Invitations(object): def accept(cls, invite_id): invite = Invitations._get(invite_id) - if Invitations._is_expired(invite): + if invite.is_expired: invite.status = InvitationStatus.REJECTED elif invite.is_pending: invite.status = InvitationStatus.ACCEPTED @@ -60,9 +61,7 @@ class Invitations(object): return invite @classmethod - def _is_expired(cls, invite): - time_created = invite.time_created - expiration = datetime.datetime.now(time_created.tzinfo) - datetime.timedelta( + def current_expiration_time(cls): + return datetime.datetime.now() + datetime.timedelta( minutes=Invitations.EXPIRATION_LIMIT_MINUTES ) - return invite.time_created < expiration diff --git a/atst/models/invitation.py b/atst/models/invitation.py index 0d8bc8d8..4f68e332 100644 --- a/atst/models/invitation.py +++ b/atst/models/invitation.py @@ -1,6 +1,7 @@ +import datetime from enum import Enum -from sqlalchemy import Column, ForeignKey, Enum as SQLAEnum +from sqlalchemy import Column, ForeignKey, Enum as SQLAEnum, TIMESTAMP from sqlalchemy.dialects.postgresql import UUID from sqlalchemy.orm import relationship @@ -31,6 +32,8 @@ class Invitation(Base, TimestampsMixin): status = Column(SQLAEnum(Status, native_enum=False, default=Status.PENDING)) + expiration_time = Column(TIMESTAMP(timezone=True)) + def __repr__(self): return "".format( self.user.id, self.workspace.id, self.id @@ -51,3 +54,7 @@ class Invitation(Base, TimestampsMixin): @property def is_rejected(self): return self.status == Status.REJECTED + + @property + def is_expired(self): + return datetime.datetime.now(self.expiration_time.tzinfo) > self.expiration_time diff --git a/tests/domain/test_invitations.py b/tests/domain/test_invitations.py index 864fa79d..cbcb43d4 100644 --- a/tests/domain/test_invitations.py +++ b/tests/domain/test_invitations.py @@ -30,11 +30,11 @@ def test_accept_expired_invitation(): workspace = WorkspaceFactory.create() user = UserFactory.create() increment = Invitations.EXPIRATION_LIMIT_MINUTES + 1 - created_at = datetime.datetime.now() - datetime.timedelta(minutes=increment) + expiration_time = datetime.datetime.now() - datetime.timedelta(minutes=increment) invite = InvitationFactory.create( workspace_id=workspace.id, user_id=user.id, - time_created=created_at, + expiration_time=expiration_time, status=Status.PENDING, ) with pytest.raises(InvitationError): diff --git a/tests/factories.py b/tests/factories.py index eac728a9..babe6595 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -22,6 +22,7 @@ from atst.models.workspace_role import WorkspaceRole from atst.models.environment_role import EnvironmentRole from atst.models.invitation import Invitation, Status as InvitationStatus from atst.domain.workspaces import Workspaces +from atst.domain.invitations import Invitations class Base(factory.alchemy.SQLAlchemyModelFactory): @@ -341,3 +342,4 @@ class InvitationFactory(Base): model = Invitation status = InvitationStatus.PENDING + expiration_time = Invitations.current_expiration_time() From b81a831c85ae3129e7a04b8092cd32bebdf8af0a Mon Sep 17 00:00:00 2001 From: dandds Date: Fri, 26 Oct 2018 16:07:10 -0400 Subject: [PATCH 13/23] remove accepted column from workspace_roles --- ...aef_remove_accepted_from_workspace_role.py | 28 +++++++++++++++++ atst/domain/authz.py | 3 +- atst/domain/invitations.py | 14 +++++++++ atst/domain/workspace_users.py | 29 +++++++++++++++++- atst/domain/workspaces/query.py | 5 +++- atst/domain/workspaces/workspaces.py | 20 ++++--------- atst/models/workspace.py | 4 +-- atst/models/workspace_role.py | 4 +-- atst/models/workspace_user.py | 11 +------ atst/routes/errors.py | 2 +- atst/routes/workspaces.py | 4 +-- tests/domain/test_workspaces.py | 30 ++++++++++++++++--- tests/factories.py | 15 ++++++++-- tests/routes/test_home.py | 15 ++++++++-- tests/routes/test_workspaces.py | 13 +++++--- 15 files changed, 148 insertions(+), 49 deletions(-) create mode 100644 alembic/versions/67955a4abaef_remove_accepted_from_workspace_role.py 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 From 151d5be5eabad7ec85f18b3b196c4acb54a6267e Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 29 Oct 2018 09:59:34 -0400 Subject: [PATCH 14/23] use invite token instead of id for invitation url --- .../81e6babf5136_add_token_to_invitation.py | 30 +++++++++++++++++++ atst/domain/invitations.py | 8 ++--- atst/models/invitation.py | 5 +++- atst/routes/workspaces.py | 14 ++++----- templates/emails/invitation.txt | 2 +- tests/domain/test_invitations.py | 10 ++++--- tests/routes/test_workspaces.py | 4 +-- 7 files changed, 53 insertions(+), 20 deletions(-) create mode 100644 alembic/versions/81e6babf5136_add_token_to_invitation.py diff --git a/alembic/versions/81e6babf5136_add_token_to_invitation.py b/alembic/versions/81e6babf5136_add_token_to_invitation.py new file mode 100644 index 00000000..7206efc2 --- /dev/null +++ b/alembic/versions/81e6babf5136_add_token_to_invitation.py @@ -0,0 +1,30 @@ +"""add token to invitation + +Revision ID: 81e6babf5136 +Revises: 67955a4abaef +Create Date: 2018-10-29 09:26:30.728348 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '81e6babf5136' +down_revision = '67955a4abaef' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('invitations', sa.Column('token', sa.String(), nullable=True)) + op.create_index(op.f('ix_invitations_token'), 'invitations', ['token'], unique=True) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_index(op.f('ix_invitations_token'), table_name='invitations') + op.drop_column('invitations', 'token') + # ### end Alembic commands ### diff --git a/atst/domain/invitations.py b/atst/domain/invitations.py index 166bc841..9181d682 100644 --- a/atst/domain/invitations.py +++ b/atst/domain/invitations.py @@ -21,9 +21,9 @@ class Invitations(object): EXPIRATION_LIMIT_MINUTES = 360 @classmethod - def _get(cls, invite_id): + def _get(cls, token): try: - invite = db.session.query(Invitation).filter_by(id=invite_id).one() + invite = db.session.query(Invitation).filter_by(token=token).one() except NoResultFound: raise NotFoundError("invite") @@ -58,8 +58,8 @@ class Invitations(object): return invite @classmethod - def accept(cls, invite_id): - invite = Invitations._get(invite_id) + def accept(cls, token): + invite = Invitations._get(token) if invite.is_expired: invite.status = InvitationStatus.REJECTED diff --git a/atst/models/invitation.py b/atst/models/invitation.py index 4f68e332..50ae3351 100644 --- a/atst/models/invitation.py +++ b/atst/models/invitation.py @@ -1,7 +1,8 @@ import datetime from enum import Enum +import secrets -from sqlalchemy import Column, ForeignKey, Enum as SQLAEnum, TIMESTAMP +from sqlalchemy import Column, ForeignKey, Enum as SQLAEnum, TIMESTAMP, String from sqlalchemy.dialects.postgresql import UUID from sqlalchemy.orm import relationship @@ -34,6 +35,8 @@ class Invitation(Base, TimestampsMixin): expiration_time = Column(TIMESTAMP(timezone=True)) + token = Column(String(), index=True, default=lambda: secrets.token_urlsafe()) + def __repr__(self): return "".format( self.user.id, self.workspace.id, self.id diff --git a/atst/routes/workspaces.py b/atst/routes/workspaces.py index 64ad272c..0a6604e9 100644 --- a/atst/routes/workspaces.py +++ b/atst/routes/workspaces.py @@ -220,10 +220,8 @@ def new_member(workspace_id): ) -def send_invite_email(owner_name, invite_id, new_member_email): - body = render_template( - "emails/invitation.txt", owner=owner_name, invite_id=invite_id - ) +def send_invite_email(owner_name, token, new_member_email): + body = render_template("emails/invitation.txt", owner=owner_name, token=token) queue.send_mail( [new_member_email], "{} has invited you to a JEDI Cloud Workspace".format(owner_name), @@ -241,7 +239,7 @@ def create_member(workspace_id): new_member = Workspaces.create_member(g.current_user, workspace, form.data) invite = Invitations.create(workspace, g.current_user, new_member.user) send_invite_email( - g.current_user.full_name, invite.id, new_member.user.email + g.current_user.full_name, invite.token, new_member.user.email ) return redirect( @@ -338,11 +336,11 @@ def update_member(workspace_id, member_id): ) -@bp.route("/workspaces/invitation/", methods=["GET"]) -def accept_invitation(invite_id): +@bp.route("/workspaces/invitation/", methods=["GET"]) +def accept_invitation(token): # TODO: check that the current_user DOD ID matches the user associated with # the invitation - invite = Invitations.accept(invite_id) + invite = Invitations.accept(token) return redirect( url_for("workspaces.show_workspace", workspace_id=invite.workspace.id) diff --git a/templates/emails/invitation.txt b/templates/emails/invitation.txt index 789da574..02fdd49d 100644 --- a/templates/emails/invitation.txt +++ b/templates/emails/invitation.txt @@ -1,7 +1,7 @@ Join this JEDI Cloud Workspace {{ owner }} has invited you to join a JEDI Cloud Workspace. Login now to view or use your JEDI Cloud resources. -{{ url_for("workspaces.accept_invitation", invite_id=invite_id, _external=True) }} +{{ url_for("workspaces.accept_invitation", token=token, _external=True) }} What is JEDI Cloud? JEDI Cloud is a DoD enterprise-wide solution for commercial cloud services. diff --git a/tests/domain/test_invitations.py b/tests/domain/test_invitations.py index cbcb43d4..42f61e1e 100644 --- a/tests/domain/test_invitations.py +++ b/tests/domain/test_invitations.py @@ -1,5 +1,6 @@ import datetime import pytest +import re from atst.domain.invitations import Invitations, InvitationError from atst.models.invitation import Status @@ -15,6 +16,7 @@ def test_create_invitation(): assert invite.workspace == workspace assert invite.inviter == workspace.owner assert invite.status == Status.PENDING + assert re.match(r"^[\w\-_]+$", invite.token) def test_accept_invitation(): @@ -22,7 +24,7 @@ def test_accept_invitation(): user = UserFactory.create() invite = Invitations.create(workspace, workspace.owner, user) assert invite.is_pending - accepted_invite = Invitations.accept(invite.id) + accepted_invite = Invitations.accept(invite.token) assert accepted_invite.is_accepted @@ -38,7 +40,7 @@ def test_accept_expired_invitation(): status=Status.PENDING, ) with pytest.raises(InvitationError): - Invitations.accept(invite.id) + Invitations.accept(invite.token) assert invite.is_rejected @@ -50,7 +52,7 @@ def test_accept_rejected_invite(): workspace_id=workspace.id, user_id=user.id, status=Status.REJECTED ) with pytest.raises(InvitationError): - Invitations.accept(invite.id) + Invitations.accept(invite.token) def test_accept_revoked_invite(): @@ -60,4 +62,4 @@ def test_accept_revoked_invite(): workspace_id=workspace.id, user_id=user.id, status=Status.REVOKED ) with pytest.raises(InvitationError): - Invitations.accept(invite.id) + Invitations.accept(invite.token) diff --git a/tests/routes/test_workspaces.py b/tests/routes/test_workspaces.py index cfc56f6f..1cd0e4c5 100644 --- a/tests/routes/test_workspaces.py +++ b/tests/routes/test_workspaces.py @@ -308,7 +308,7 @@ def test_new_member_accepts_valid_invite(client, user_session): assert len(Workspaces.for_user(user)) == 0 user_session(user) - response = client.get(url_for("workspaces.accept_invitation", invite_id=invite.id)) + response = client.get(url_for("workspaces.accept_invitation", token=invite.token)) # user is redirected to the workspace view assert response.status_code == 302 @@ -333,7 +333,7 @@ def test_new_member_accept_invalid_invite(client, user_session): status=InvitationStatus.REJECTED, ) user_session(user) - response = client.get(url_for("workspaces.accept_invitation", invite_id=invite.id)) + response = client.get(url_for("workspaces.accept_invitation", token=invite.token)) assert response.status_code == 404 From ead84834d02e52a01686663b0877da0c6279a48f Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 29 Oct 2018 10:23:42 -0400 Subject: [PATCH 15/23] squash invitation migrations --- .../03654d08f5ff_add_status_to_invitation.py | 30 ----------------- .../versions/25bcba9b99a9_add_invitiations.py | 8 ++++- ..._add_inviter_relationship_to_invitation.py | 32 ------------------- ...aef_remove_accepted_from_workspace_role.py | 28 ---------------- .../81e6babf5136_add_token_to_invitation.py | 30 ----------------- ...c_add_accepted_column_to_workspace_role.py | 28 ---------------- ...60c26_add_expiration_time_to_invitation.py | 28 ---------------- 7 files changed, 7 insertions(+), 177 deletions(-) delete mode 100644 alembic/versions/03654d08f5ff_add_status_to_invitation.py delete mode 100644 alembic/versions/2bec1868a22a_add_inviter_relationship_to_invitation.py delete mode 100644 alembic/versions/67955a4abaef_remove_accepted_from_workspace_role.py delete mode 100644 alembic/versions/81e6babf5136_add_token_to_invitation.py delete mode 100644 alembic/versions/c7feaa7b6b0c_add_accepted_column_to_workspace_role.py delete mode 100644 alembic/versions/e62bcc460c26_add_expiration_time_to_invitation.py diff --git a/alembic/versions/03654d08f5ff_add_status_to_invitation.py b/alembic/versions/03654d08f5ff_add_status_to_invitation.py deleted file mode 100644 index 0d06b1e5..00000000 --- a/alembic/versions/03654d08f5ff_add_status_to_invitation.py +++ /dev/null @@ -1,30 +0,0 @@ -"""add status to invitation - -Revision ID: 03654d08f5ff -Revises: 2bec1868a22a -Create Date: 2018-10-26 12:59:16.709080 - -""" -from alembic import op -import sqlalchemy as sa - - -# revision identifiers, used by Alembic. -revision = '03654d08f5ff' -down_revision = '2bec1868a22a' -branch_labels = None -depends_on = None - - -def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.add_column('invitations', sa.Column('status', sa.Enum('ACCEPTED', 'REVOKED', 'PENDING', 'REJECTED', name='status', native_enum=False), nullable=True)) - op.drop_column('invitations', 'valid') - # ### end Alembic commands ### - - -def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.add_column('invitations', sa.Column('valid', sa.BOOLEAN(), autoincrement=False, nullable=True)) - op.drop_column('invitations', 'status') - # ### end Alembic commands ### diff --git a/alembic/versions/25bcba9b99a9_add_invitiations.py b/alembic/versions/25bcba9b99a9_add_invitiations.py index c2681435..80883715 100644 --- a/alembic/versions/25bcba9b99a9_add_invitiations.py +++ b/alembic/versions/25bcba9b99a9_add_invitiations.py @@ -24,13 +24,19 @@ def upgrade(): sa.Column('id', postgresql.UUID(as_uuid=True), server_default=sa.text('uuid_generate_v4()'), nullable=False), sa.Column('user_id', postgresql.UUID(as_uuid=True), nullable=True), sa.Column('workspace_id', postgresql.UUID(as_uuid=True), nullable=True), - sa.Column('valid', sa.Boolean(), nullable=True), + sa.Column('status', sa.Enum('ACCEPTED', 'REVOKED', 'PENDING', 'REJECTED', name='status', native_enum=False), nullable=True), + sa.Column('inviter_id', postgresql.UUID(as_uuid=True), nullable=True), + sa.Column('token', sa.String(), nullable=True), + sa.Column('expiration_time', sa.TIMESTAMP(timezone=True), nullable=True), sa.ForeignKeyConstraint(['user_id'], ['users.id'], ), + sa.ForeignKeyConstraint(['inviter_id'], ['users.id'], ), sa.ForeignKeyConstraint(['workspace_id'], ['workspaces.id'], ), sa.PrimaryKeyConstraint('id') ) op.create_index(op.f('ix_invitations_user_id'), 'invitations', ['user_id'], unique=False) + op.create_index(op.f('ix_invitations_inviter_id'), 'invitations', ['inviter_id'], unique=False) op.create_index(op.f('ix_invitations_workspace_id'), 'invitations', ['workspace_id'], unique=False) + op.create_index(op.f('ix_invitations_token'), 'invitations', ['token'], unique=True) # ### end Alembic commands ### diff --git a/alembic/versions/2bec1868a22a_add_inviter_relationship_to_invitation.py b/alembic/versions/2bec1868a22a_add_inviter_relationship_to_invitation.py deleted file mode 100644 index 23c75457..00000000 --- a/alembic/versions/2bec1868a22a_add_inviter_relationship_to_invitation.py +++ /dev/null @@ -1,32 +0,0 @@ -"""add inviter relationship to invitation - -Revision ID: 2bec1868a22a -Revises: c7feaa7b6b0c -Create Date: 2018-10-26 12:45:13.192062 - -""" -from alembic import op -import sqlalchemy as sa -from sqlalchemy.dialects import postgresql - -# revision identifiers, used by Alembic. -revision = '2bec1868a22a' -down_revision = 'c7feaa7b6b0c' -branch_labels = None -depends_on = None - - -def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.add_column('invitations', sa.Column('inviter_id', postgresql.UUID(as_uuid=True), nullable=True)) - op.create_index(op.f('ix_invitations_inviter_id'), 'invitations', ['inviter_id'], unique=False) - op.create_foreign_key(None, 'invitations', 'users', ['inviter_id'], ['id']) - # ### end Alembic commands ### - - -def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.drop_constraint(None, 'invitations', type_='foreignkey') - op.drop_index(op.f('ix_invitations_inviter_id'), table_name='invitations') - op.drop_column('invitations', 'inviter_id') - # ### end Alembic commands ### diff --git a/alembic/versions/67955a4abaef_remove_accepted_from_workspace_role.py b/alembic/versions/67955a4abaef_remove_accepted_from_workspace_role.py deleted file mode 100644 index 749ca512..00000000 --- a/alembic/versions/67955a4abaef_remove_accepted_from_workspace_role.py +++ /dev/null @@ -1,28 +0,0 @@ -"""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/alembic/versions/81e6babf5136_add_token_to_invitation.py b/alembic/versions/81e6babf5136_add_token_to_invitation.py deleted file mode 100644 index 7206efc2..00000000 --- a/alembic/versions/81e6babf5136_add_token_to_invitation.py +++ /dev/null @@ -1,30 +0,0 @@ -"""add token to invitation - -Revision ID: 81e6babf5136 -Revises: 67955a4abaef -Create Date: 2018-10-29 09:26:30.728348 - -""" -from alembic import op -import sqlalchemy as sa - - -# revision identifiers, used by Alembic. -revision = '81e6babf5136' -down_revision = '67955a4abaef' -branch_labels = None -depends_on = None - - -def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.add_column('invitations', sa.Column('token', sa.String(), nullable=True)) - op.create_index(op.f('ix_invitations_token'), 'invitations', ['token'], unique=True) - # ### end Alembic commands ### - - -def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.drop_index(op.f('ix_invitations_token'), table_name='invitations') - op.drop_column('invitations', 'token') - # ### end Alembic commands ### diff --git a/alembic/versions/c7feaa7b6b0c_add_accepted_column_to_workspace_role.py b/alembic/versions/c7feaa7b6b0c_add_accepted_column_to_workspace_role.py deleted file mode 100644 index 9a1fb904..00000000 --- a/alembic/versions/c7feaa7b6b0c_add_accepted_column_to_workspace_role.py +++ /dev/null @@ -1,28 +0,0 @@ -"""add accepted column to workspace role - -Revision ID: c7feaa7b6b0c -Revises: 5284ac1ac77c -Create Date: 2018-10-25 11:44:17.654892 - -""" -from alembic import op -import sqlalchemy as sa - - -# revision identifiers, used by Alembic. -revision = 'c7feaa7b6b0c' -down_revision = '5284ac1ac77c' -branch_labels = None -depends_on = None - - -def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.add_column('workspace_roles', sa.Column('accepted', sa.Boolean(), nullable=True)) - # ### end Alembic commands ### - - -def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.drop_column('workspace_roles', 'accepted') - # ### end Alembic commands ### diff --git a/alembic/versions/e62bcc460c26_add_expiration_time_to_invitation.py b/alembic/versions/e62bcc460c26_add_expiration_time_to_invitation.py deleted file mode 100644 index 9b6fc4d1..00000000 --- a/alembic/versions/e62bcc460c26_add_expiration_time_to_invitation.py +++ /dev/null @@ -1,28 +0,0 @@ -"""add expiration_time to invitation - -Revision ID: e62bcc460c26 -Revises: 03654d08f5ff -Create Date: 2018-10-26 13:37:39.015003 - -""" -from alembic import op -import sqlalchemy as sa - - -# revision identifiers, used by Alembic. -revision = 'e62bcc460c26' -down_revision = '03654d08f5ff' -branch_labels = None -depends_on = None - - -def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.add_column('invitations', sa.Column('expiration_time', sa.TIMESTAMP(timezone=True), nullable=True)) - # ### end Alembic commands ### - - -def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.drop_column('invitations', 'expiration_time') - # ### end Alembic commands ### From 4c03a403f7b4937d1fb8f7049286e3f8ac37bfb1 Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 29 Oct 2018 10:39:19 -0400 Subject: [PATCH 16/23] cleanup, add test for workspace user permissions --- atst/models/workspace_user.py | 7 ------- tests/domain/test_workspace_users.py | 25 ++++++++++++++++++++++++- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/atst/models/workspace_user.py b/atst/models/workspace_user.py index fb79ebbd..216b07aa 100644 --- a/atst/models/workspace_user.py +++ b/atst/models/workspace_user.py @@ -9,13 +9,6 @@ class WorkspaceUser(object): self.user = user self.workspace_role = workspace_role - 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 - ) - return set(workspace_permissions).union(atat_permissions) - @property def workspace(self): return self.workspace_role.workspace diff --git a/tests/domain/test_workspace_users.py b/tests/domain/test_workspace_users.py index 6e3aa710..4cea3970 100644 --- a/tests/domain/test_workspace_users.py +++ b/tests/domain/test_workspace_users.py @@ -1,6 +1,8 @@ from atst.domain.workspace_users import WorkspaceUsers from atst.domain.users import Users -from tests.factories import WorkspaceFactory, UserFactory +from atst.models.invitation import Status as InvitationStatus + +from tests.factories import WorkspaceFactory, UserFactory, InvitationFactory def test_can_create_new_workspace_user(): @@ -34,3 +36,24 @@ def test_can_update_existing_workspace_user(): workspace_users[0].workspace_role.role.name == new_user.workspace_roles[0].role.name ) + + +def test_workspace_user_permissions(): + workspace_one = WorkspaceFactory.create() + workspace_two = WorkspaceFactory.create() + new_user = UserFactory.create() + WorkspaceUsers.add_many( + workspace_one.id, [{"id": new_user.id, "workspace_role": "developer"}] + ) + WorkspaceUsers.add_many( + workspace_two.id, [{"id": new_user.id, "workspace_role": "developer"}] + ) + InvitationFactory.create( + workspace=workspace_one, + user=new_user, + inviter=workspace_one.owner, + status=InvitationStatus.ACCEPTED, + ) + + assert WorkspaceUsers.workspace_user_permissions(workspace_one, new_user) + assert not WorkspaceUsers.workspace_user_permissions(workspace_two, new_user) From b2b23519f7bad3054a8ff147d60902200f3430a5 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 29 Oct 2018 11:02:28 -0400 Subject: [PATCH 17/23] Fix migration conflict --- alembic/versions/25bcba9b99a9_add_invitiations.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/alembic/versions/25bcba9b99a9_add_invitiations.py b/alembic/versions/25bcba9b99a9_add_invitiations.py index 80883715..7b669506 100644 --- a/alembic/versions/25bcba9b99a9_add_invitiations.py +++ b/alembic/versions/25bcba9b99a9_add_invitiations.py @@ -1,7 +1,7 @@ """add invitiations Revision ID: 25bcba9b99a9 -Revises: c99026ab9918 +Revises: 9c24c609878a Create Date: 2018-10-23 15:03:12.641069 """ @@ -11,7 +11,7 @@ from sqlalchemy.dialects import postgresql # revision identifiers, used by Alembic. revision = '25bcba9b99a9' -down_revision = 'c99026ab9918' +down_revision = '9c24c609878a' branch_labels = None depends_on = None From 4255dbe292135e3ec483e15ba80d92c30c1277ad Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 30 Oct 2018 09:38:22 -0400 Subject: [PATCH 18/23] fix invitation migration --- ...a9_add_invitiations.py => 25bcba9b99a9_add_invitations.py} | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) rename alembic/versions/{25bcba9b99a9_add_invitiations.py => 25bcba9b99a9_add_invitations.py} (92%) diff --git a/alembic/versions/25bcba9b99a9_add_invitiations.py b/alembic/versions/25bcba9b99a9_add_invitations.py similarity index 92% rename from alembic/versions/25bcba9b99a9_add_invitiations.py rename to alembic/versions/25bcba9b99a9_add_invitations.py index 7b669506..8aafdfcb 100644 --- a/alembic/versions/25bcba9b99a9_add_invitiations.py +++ b/alembic/versions/25bcba9b99a9_add_invitations.py @@ -1,4 +1,4 @@ -"""add invitiations +"""add invitations Revision ID: 25bcba9b99a9 Revises: 9c24c609878a @@ -44,5 +44,7 @@ def downgrade(): # ### commands auto generated by Alembic - please adjust! ### op.drop_index(op.f('ix_invitations_workspace_id'), table_name='invitations') op.drop_index(op.f('ix_invitations_user_id'), table_name='invitations') + op.drop_index(op.f('ix_invitations_inviter_id'), table_name='invitations') + op.drop_index(op.f('ix_invitations_token'), table_name='invitations') op.drop_table('invitations') # ### end Alembic commands ### From 848bbf9c124042ec815a6b05905589c1b62db3d4 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 30 Oct 2018 15:22:07 -0400 Subject: [PATCH 19/23] Updates to workspace role permissions, invitations --- ...e4be_change_invitation_relationship_to_.py | 42 +++++++ ...c3cd315c1_add_status_to_workspace_roles.py | 28 +++++ atst/domain/invitations.py | 21 +--- atst/domain/workspace_users.py | 14 ++- atst/domain/workspaces/query.py | 7 +- atst/domain/workspaces/workspaces.py | 15 ++- atst/models/invitation.py | 11 +- atst/models/workspace_role.py | 11 +- atst/routes/workspaces.py | 4 +- tests/domain/test_invitations.py | 31 +++-- tests/domain/test_workspace_users.py | 28 +++-- tests/domain/test_workspaces.py | 48 +++----- tests/factories.py | 29 ++--- tests/routes/test_home.py | 19 +-- tests/routes/test_workspaces.py | 111 +++++++++--------- tests/test_auth.py | 14 --- 16 files changed, 232 insertions(+), 201 deletions(-) create mode 100644 alembic/versions/d1ea7f3ee4be_change_invitation_relationship_to_.py create mode 100644 alembic/versions/e0fc3cd315c1_add_status_to_workspace_roles.py diff --git a/alembic/versions/d1ea7f3ee4be_change_invitation_relationship_to_.py b/alembic/versions/d1ea7f3ee4be_change_invitation_relationship_to_.py new file mode 100644 index 00000000..22175d4d --- /dev/null +++ b/alembic/versions/d1ea7f3ee4be_change_invitation_relationship_to_.py @@ -0,0 +1,42 @@ +"""change invitation relationship to workspace role + +Revision ID: d1ea7f3ee4be +Revises: 5284ac1ac77c +Create Date: 2018-10-30 14:09:42.277467 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = 'd1ea7f3ee4be' +down_revision = '5284ac1ac77c' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('invitations', sa.Column('workspace_role_id', postgresql.UUID(as_uuid=True), nullable=True)) + op.create_index(op.f('ix_invitations_workspace_role_id'), 'invitations', ['workspace_role_id'], unique=False) + op.drop_index('ix_invitations_token', table_name='invitations') + op.create_index(op.f('ix_invitations_token'), 'invitations', ['token'], unique=False) + op.drop_index('ix_invitations_workspace_id', table_name='invitations') + op.drop_constraint('invitations_workspace_id_fkey', 'invitations', type_='foreignkey') + op.create_foreign_key(None, 'invitations', 'workspace_roles', ['workspace_role_id'], ['id']) + op.drop_column('invitations', 'workspace_id') + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('invitations', sa.Column('workspace_id', postgresql.UUID(), autoincrement=False, nullable=True)) + op.drop_constraint(None, 'invitations', type_='foreignkey') + op.create_foreign_key('invitations_workspace_id_fkey', 'invitations', 'workspaces', ['workspace_id'], ['id']) + op.create_index('ix_invitations_workspace_id', 'invitations', ['workspace_id'], unique=False) + op.drop_index(op.f('ix_invitations_token'), table_name='invitations') + op.create_index('ix_invitations_token', 'invitations', ['token'], unique=True) + op.drop_index(op.f('ix_invitations_workspace_role_id'), table_name='invitations') + op.drop_column('invitations', 'workspace_role_id') + # ### end Alembic commands ### diff --git a/alembic/versions/e0fc3cd315c1_add_status_to_workspace_roles.py b/alembic/versions/e0fc3cd315c1_add_status_to_workspace_roles.py new file mode 100644 index 00000000..810b5691 --- /dev/null +++ b/alembic/versions/e0fc3cd315c1_add_status_to_workspace_roles.py @@ -0,0 +1,28 @@ +"""add status to workspace_roles + +Revision ID: e0fc3cd315c1 +Revises: d1ea7f3ee4be +Create Date: 2018-10-30 14:36:51.047876 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'e0fc3cd315c1' +down_revision = 'd1ea7f3ee4be' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('workspace_roles', sa.Column('status', sa.Enum('ACTIVE', 'DISABLED', 'PENDING', name='status', native_enum=False), nullable=True)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('workspace_roles', 'status') + # ### end Alembic commands ### diff --git a/atst/domain/invitations.py b/atst/domain/invitations.py index 9181d682..ee518c4e 100644 --- a/atst/domain/invitations.py +++ b/atst/domain/invitations.py @@ -3,6 +3,7 @@ from sqlalchemy.orm.exc import NoResultFound from atst.database import db from atst.models.invitation import Invitation, Status as InvitationStatus +from atst.domain.workspace_users import WorkspaceUsers from .exceptions import NotFoundError @@ -30,9 +31,9 @@ class Invitations(object): return invite @classmethod - def create(cls, workspace, inviter, user): + def create(cls, workspace_role, inviter, user): invite = Invitation( - workspace=workspace, + workspace_role=workspace_role, inviter=inviter, user=user, status=InvitationStatus.PENDING, @@ -43,20 +44,6 @@ 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, token): invite = Invitations._get(token) @@ -72,6 +59,8 @@ class Invitations(object): if invite.is_revoked or invite.is_rejected: raise InvitationError(invite) + WorkspaceUsers.enable(invite.workspace_role) + return invite @classmethod diff --git a/atst/domain/workspace_users.py b/atst/domain/workspace_users.py index ddd234ed..c5f4075d 100644 --- a/atst/domain/workspace_users.py +++ b/atst/domain/workspace_users.py @@ -1,10 +1,9 @@ from sqlalchemy.orm.exc import NoResultFound from atst.database import db -from atst.models.workspace_role import WorkspaceRole +from atst.models.workspace_role import WorkspaceRole, Status as WorkspaceRoleStatus 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 @@ -38,9 +37,7 @@ class WorkspaceUsers(object): 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) + .filter(WorkspaceRole.status == WorkspaceRoleStatus.ACTIVE) .one() ) except NoResultFound: @@ -150,3 +147,10 @@ class WorkspaceUsers(object): db.session.commit() return workspace_users + + @classmethod + def enable(cls, workspace_role): + workspace_role.status = WorkspaceRoleStatus.ACTIVE + + db.session.add(workspace_role) + db.session.commit() diff --git a/atst/domain/workspaces/query.py b/atst/domain/workspaces/query.py index 7dc1bc88..021edf9d 100644 --- a/atst/domain/workspaces/query.py +++ b/atst/domain/workspaces/query.py @@ -4,8 +4,7 @@ from atst.database import db 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 +from atst.models.workspace_role import WorkspaceRole, Status as WorkspaceRoleStatus class WorkspacesQuery(Query): @@ -25,10 +24,8 @@ class WorkspacesQuery(Query): return ( db.session.query(Workspace) .join(WorkspaceRole) - .join(Invitation) .filter(WorkspaceRole.user == user) - .filter(Invitation.user == user) - .filter(Invitation.status == InvitationStatus.ACCEPTED) + .filter(WorkspaceRole.status == WorkspaceRoleStatus.ACTIVE) .all() ) diff --git a/atst/domain/workspaces/workspaces.py b/atst/domain/workspaces/workspaces.py index 14397714..aecfbc00 100644 --- a/atst/domain/workspaces/workspaces.py +++ b/atst/domain/workspaces/workspaces.py @@ -3,7 +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 atst.models.workspace_role import Status as WorkspaceRoleStatus from .query import WorkspacesQuery from .scopes import ScopedWorkspace @@ -14,8 +14,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") - Invitations.create_for_owner(workspace, request.creator) + Workspaces._create_workspace_role( + request.creator, workspace, "owner", status=WorkspaceRoleStatus.ACTIVE + ) WorkspacesQuery.add_and_commit(workspace) return workspace @@ -109,9 +110,13 @@ 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, status=WorkspaceRoleStatus.PENDING + ): role = Roles.get(role_name) - workspace_role = WorkspacesQuery.create_workspace_role(user, role, workspace) + workspace_role = WorkspacesQuery.create_workspace_role( + user, role, workspace, status=status + ) WorkspacesQuery.add_and_commit(workspace_role) return workspace_role diff --git a/atst/models/invitation.py b/atst/models/invitation.py index 50ae3351..3ac1a4bf 100644 --- a/atst/models/invitation.py +++ b/atst/models/invitation.py @@ -25,8 +25,10 @@ class Invitation(Base, TimestampsMixin): user_id = Column(UUID(as_uuid=True), ForeignKey("users.id"), index=True) user = relationship("User", backref="invitations", foreign_keys=[user_id]) - workspace_id = Column(UUID(as_uuid=True), ForeignKey("workspaces.id"), index=True) - workspace = relationship("Workspace", backref="invitations") + workspace_role_id = Column( + UUID(as_uuid=True), ForeignKey("workspace_roles.id"), index=True + ) + workspace_role = relationship("WorkspaceRole", backref="invitations") inviter_id = Column(UUID(as_uuid=True), ForeignKey("users.id"), index=True) inviter = relationship("User", backref="sent_invites", foreign_keys=[inviter_id]) @@ -61,3 +63,8 @@ class Invitation(Base, TimestampsMixin): @property def is_expired(self): return datetime.datetime.now(self.expiration_time.tzinfo) > self.expiration_time + + @property + def workspace(self): + if self.workspace_role: + return self.workspace_role.workspace diff --git a/atst/models/workspace_role.py b/atst/models/workspace_role.py index 87c8f46f..97074602 100644 --- a/atst/models/workspace_role.py +++ b/atst/models/workspace_role.py @@ -1,4 +1,5 @@ -from sqlalchemy import Index, ForeignKey, Column +from enum import Enum +from sqlalchemy import Index, ForeignKey, Column, Enum as SQLAEnum from sqlalchemy.dialects.postgresql import UUID from sqlalchemy.orm import relationship @@ -6,6 +7,12 @@ from atst.models import Base, mixins from .types import Id +class Status(Enum): + ACTIVE = "active" + DISABLED = "disabled" + PENDING = "pending" + + class WorkspaceRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): __tablename__ = "workspace_roles" @@ -22,6 +29,8 @@ class WorkspaceRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): UUID(as_uuid=True), ForeignKey("users.id"), index=True, nullable=False ) + status = Column(SQLAEnum(Status, native_enum=False, default=Status.PENDING)) + def __repr__(self): return "".format( self.role.name, self.workspace.name, self.user_id, self.id diff --git a/atst/routes/workspaces.py b/atst/routes/workspaces.py index 0a6604e9..b0c751a6 100644 --- a/atst/routes/workspaces.py +++ b/atst/routes/workspaces.py @@ -237,7 +237,9 @@ def create_member(workspace_id): if form.validate(): try: new_member = Workspaces.create_member(g.current_user, workspace, form.data) - invite = Invitations.create(workspace, g.current_user, new_member.user) + invite = Invitations.create( + new_member.workspace_role, g.current_user, new_member.user + ) send_invite_email( g.current_user.full_name, invite.token, new_member.user.email ) diff --git a/tests/domain/test_invitations.py b/tests/domain/test_invitations.py index 42f61e1e..2ec08a9f 100644 --- a/tests/domain/test_invitations.py +++ b/tests/domain/test_invitations.py @@ -5,15 +5,21 @@ import re from atst.domain.invitations import Invitations, InvitationError from atst.models.invitation import Status -from tests.factories import WorkspaceFactory, UserFactory, InvitationFactory +from tests.factories import ( + WorkspaceFactory, + WorkspaceRoleFactory, + UserFactory, + InvitationFactory, +) def test_create_invitation(): workspace = WorkspaceFactory.create() user = UserFactory.create() - invite = Invitations.create(workspace, workspace.owner, user) + ws_role = WorkspaceRoleFactory.create(user=user, workspace=workspace) + invite = Invitations.create(ws_role, workspace.owner, user) assert invite.user == user - assert invite.workspace == workspace + assert invite.workspace_role == ws_role assert invite.inviter == workspace.owner assert invite.status == Status.PENDING assert re.match(r"^[\w\-_]+$", invite.token) @@ -22,22 +28,19 @@ def test_create_invitation(): def test_accept_invitation(): workspace = WorkspaceFactory.create() user = UserFactory.create() - invite = Invitations.create(workspace, workspace.owner, user) + ws_role = WorkspaceRoleFactory.create(user=user, workspace=workspace) + invite = Invitations.create(ws_role, workspace.owner, user) assert invite.is_pending accepted_invite = Invitations.accept(invite.token) assert accepted_invite.is_accepted def test_accept_expired_invitation(): - workspace = WorkspaceFactory.create() user = UserFactory.create() increment = Invitations.EXPIRATION_LIMIT_MINUTES + 1 expiration_time = datetime.datetime.now() - datetime.timedelta(minutes=increment) invite = InvitationFactory.create( - workspace_id=workspace.id, - user_id=user.id, - expiration_time=expiration_time, - status=Status.PENDING, + user_id=user.id, expiration_time=expiration_time, status=Status.PENDING ) with pytest.raises(InvitationError): Invitations.accept(invite.token) @@ -46,20 +49,14 @@ def test_accept_expired_invitation(): def test_accept_rejected_invite(): - workspace = WorkspaceFactory.create() user = UserFactory.create() - invite = InvitationFactory.create( - workspace_id=workspace.id, user_id=user.id, status=Status.REJECTED - ) + invite = InvitationFactory.create(user_id=user.id, status=Status.REJECTED) with pytest.raises(InvitationError): Invitations.accept(invite.token) def test_accept_revoked_invite(): - workspace = WorkspaceFactory.create() user = UserFactory.create() - invite = InvitationFactory.create( - workspace_id=workspace.id, user_id=user.id, status=Status.REVOKED - ) + invite = InvitationFactory.create(user_id=user.id, status=Status.REVOKED) with pytest.raises(InvitationError): Invitations.accept(invite.token) diff --git a/tests/domain/test_workspace_users.py b/tests/domain/test_workspace_users.py index 4cea3970..c8d6aa5d 100644 --- a/tests/domain/test_workspace_users.py +++ b/tests/domain/test_workspace_users.py @@ -1,8 +1,14 @@ from atst.domain.workspace_users import WorkspaceUsers from atst.domain.users import Users -from atst.models.invitation import Status as InvitationStatus +from atst.models.workspace_role import Status as WorkspaceRoleStatus +from atst.domain.roles import Roles -from tests.factories import WorkspaceFactory, UserFactory, InvitationFactory +from tests.factories import ( + WorkspaceFactory, + UserFactory, + InvitationFactory, + WorkspaceRoleFactory, +) def test_can_create_new_workspace_user(): @@ -42,17 +48,17 @@ def test_workspace_user_permissions(): workspace_one = WorkspaceFactory.create() workspace_two = WorkspaceFactory.create() new_user = UserFactory.create() - WorkspaceUsers.add_many( - workspace_one.id, [{"id": new_user.id, "workspace_role": "developer"}] - ) - WorkspaceUsers.add_many( - workspace_two.id, [{"id": new_user.id, "workspace_role": "developer"}] - ) - InvitationFactory.create( + WorkspaceRoleFactory.create( workspace=workspace_one, user=new_user, - inviter=workspace_one.owner, - status=InvitationStatus.ACCEPTED, + role=Roles.get("developer"), + status=WorkspaceRoleStatus.ACTIVE, + ) + WorkspaceRoleFactory.create( + workspace=workspace_two, + user=new_user, + role=Roles.get("developer"), + status=WorkspaceRoleStatus.PENDING, ) assert WorkspaceUsers.workspace_user_permissions(workspace_one, new_user) diff --git a/tests/domain/test_workspaces.py b/tests/domain/test_workspaces.py index 0365ed5f..97e13ad8 100644 --- a/tests/domain/test_workspaces.py +++ b/tests/domain/test_workspaces.py @@ -6,9 +6,14 @@ 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 atst.models.workspace_role import Status as WorkspaceRoleStatus -from tests.factories import RequestFactory, UserFactory, InvitationFactory +from tests.factories import ( + RequestFactory, + UserFactory, + InvitationFactory, + WorkspaceRoleFactory, +) @pytest.fixture(scope="function") @@ -217,14 +222,9 @@ def test_scoped_workspace_returns_all_projects_for_workspace_admin( ["dev", "staging", "prod"], ) - admin = Workspaces.add_member( - workspace, UserFactory.from_atat_role("default"), "admin" - ).user - InvitationFactory.create( - user=admin, - inviter=workspace.owner, - workspace=workspace, - status=InvitationStatus.ACCEPTED, + admin = UserFactory.from_atat_role("default") + Workspaces._create_workspace_role( + admin, workspace, "admin", status=WorkspaceRoleStatus.ACTIVE ) scoped_workspace = Workspaces.get(admin, workspace.id) @@ -250,23 +250,19 @@ def test_scoped_workspace_returns_all_projects_for_workspace_owner( assert len(scoped_workspace.projects[0].environments) == 3 -def test_for_user_returns_assigned_workspaces_for_user(workspace, workspace_owner): +def test_for_user_returns_active_workspaces_for_user(workspace, workspace_owner): bob = UserFactory.from_atat_role("default") - Workspaces.add_member(workspace, bob, "developer") - Workspaces.create(RequestFactory.create()) - InvitationFactory.create( - user=bob, - inviter=workspace.owner, - workspace=workspace, - status=InvitationStatus.ACCEPTED, + WorkspaceRoleFactory.create( + user=bob, workspace=workspace, status=WorkspaceRoleStatus.ACTIVE ) + Workspaces.create(RequestFactory.create()) bobs_workspaces = Workspaces.for_user(bob) assert len(bobs_workspaces) == 1 -def test_for_user_does_not_return_unaccepted_workspaces(workspace, workspace_owner): +def test_for_user_does_not_return_inactive_workspaces(workspace, workspace_owner): bob = UserFactory.from_atat_role("default") Workspaces.add_member(workspace, bob, "developer") Workspaces.create(RequestFactory.create()) @@ -286,22 +282,12 @@ 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") - InvitationFactory.create( - user=admin, - inviter=workspace_owner, - workspace=workspace, - status=InvitationStatus.ACCEPTED, + Workspaces._create_workspace_role( + admin, workspace, "admin", status=WorkspaceRoleStatus.ACTIVE ) 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 beb02364..f790579b 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -18,7 +18,7 @@ from atst.models.user import User from atst.models.role import Role from atst.models.workspace import Workspace from atst.domain.roles import Roles -from atst.models.workspace_role import WorkspaceRole +from atst.models.workspace_role import WorkspaceRole, Status as WorkspaceRoleStatus from atst.models.environment_role import EnvironmentRole from atst.models.invitation import Invitation, Status as InvitationStatus from atst.domain.workspaces import Workspaces @@ -257,38 +257,25 @@ class WorkspaceFactory(Base): workspace.request.creator = owner WorkspaceRoleFactory.create( - workspace=workspace, role=Roles.get("owner"), user=owner - ) - InvitationFactory.create( - user=owner, - inviter=owner, workspace=workspace, - status=InvitationStatus.ACCEPTED, + role=Roles.get("owner"), + user=owner, + status=WorkspaceRoleStatus.ACTIVE, ) for member in members: user = member.get("user", UserFactory.create()) role_name = member["role_name"] WorkspaceRoleFactory.create( - workspace=workspace, role=Roles.get(role_name), user=user + workspace=workspace, + role=Roles.get(role_name), + user=user, + status=WorkspaceRoleStatus.ACTIVE, ) 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) - InvitationFactory.create( - user=user, - inviter=workspace.owner, - workspace=workspace, - status=InvitationStatus.ACCEPTED, - ) - return user, workspace - class ProjectFactory(Base): class Meta: diff --git a/tests/routes/test_home.py b/tests/routes/test_home.py index 877845a3..83ac2eef 100644 --- a/tests/routes/test_home.py +++ b/tests/routes/test_home.py @@ -1,25 +1,10 @@ -from tests.factories import ( - UserFactory, - WorkspaceFactory, - RequestFactory, - InvitationFactory, -) +from tests.factories import UserFactory, WorkspaceFactory, RequestFactory 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") - InvitationFactory.create( - user=user, - inviter=workspace.owner, - workspace=workspace, - status=InvitationStatus.ACCEPTED, - ) - - user_session(user) + user_session(workspace.owner) response = client.get("/home", follow_redirects=True) assert b'href="/workspaces"' in response.data diff --git a/tests/routes/test_workspaces.py b/tests/routes/test_workspaces.py index 1cd0e4c5..d4f543de 100644 --- a/tests/routes/test_workspaces.py +++ b/tests/routes/test_workspaces.py @@ -1,21 +1,25 @@ from flask import url_for -from tests.factories import UserFactory, WorkspaceFactory, InvitationFactory +from tests.factories import ( + UserFactory, + WorkspaceFactory, + WorkspaceRoleFactory, + InvitationFactory, +) 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.domain.environment_roles import EnvironmentRoles -from atst.domain.invitations import Invitations from atst.models.workspace_user import WorkspaceUser +from atst.models.workspace_role import Status as WorkspaceRoleStatus from atst.models.invitation import Status as InvitationStatus from atst.queue import queue def test_user_with_permission_has_budget_report_link(client, user_session): - user, workspace = WorkspaceFactory.create_user_and_workspace_with_role("owner") - - user_session(user) + workspace = WorkspaceFactory.create() + user_session(workspace.owner) response = client.get("/workspaces/{}/projects".format(workspace.id)) assert ( 'href="/workspaces/{}/reports"'.format(workspace.id).encode() in response.data @@ -23,8 +27,11 @@ 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, workspace = WorkspaceFactory.create_user_and_workspace_with_role("developer") - + user = UserFactory.create() + workspace = WorkspaceFactory.create() + Workspaces._create_workspace_role( + user, workspace, "developer", status=WorkspaceRoleStatus.ACTIVE + ) user_session(user) response = client.get("/workspaces/{}/projects".format(workspace.id)) assert ( @@ -34,9 +41,8 @@ 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, workspace = WorkspaceFactory.create_user_and_workspace_with_role("owner") - - user_session(user) + workspace = WorkspaceFactory.create() + user_session(workspace.owner) response = client.get("/workspaces/{}/projects".format(workspace.id)) assert ( 'href="/workspaces/{}/projects/new"'.format(workspace.id).encode() @@ -45,8 +51,9 @@ 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, workspace = WorkspaceFactory.create_user_and_workspace_with_role("developer") - + user = UserFactory.create() + workspace = WorkspaceFactory.create() + Workspaces._create_workspace_role(user, workspace, "developer") user_session(user) response = client.get("/workspaces/{}/projects".format(workspace.id)) assert ( @@ -56,9 +63,8 @@ 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, workspace = WorkspaceFactory.create_user_and_workspace_with_role("owner") - - user_session(user) + workspace = WorkspaceFactory.create() + user_session(workspace.owner) response = client.get("/workspaces/{}/members".format(workspace.id)) assert ( 'href="/workspaces/{}/members/new"'.format(workspace.id).encode() @@ -67,8 +73,9 @@ 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, workspace = WorkspaceFactory.create_user_and_workspace_with_role("developer") - + user = UserFactory.create() + workspace = WorkspaceFactory.create() + Workspaces._create_workspace_role(user, workspace, "developer") user_session(user) response = client.get("/workspaces/{}/members".format(workspace.id)) assert ( @@ -78,9 +85,8 @@ def test_user_without_permission_has_no_add_member_link(client, user_session): def test_update_workspace_name(client, user_session): - user, workspace = WorkspaceFactory.create_user_and_workspace_with_role("admin") - - user_session(user) + workspace = WorkspaceFactory.create() + user_session(workspace.owner) response = client.post( url_for("workspaces.edit_workspace", workspace_id=workspace.id), data={"name": "a cool new name"}, @@ -91,16 +97,15 @@ def test_update_workspace_name(client, user_session): def test_view_edit_project(client, user_session): - owner, workspace = WorkspaceFactory.create_user_and_workspace_with_role("admin") - + workspace = WorkspaceFactory.create() project = Projects.create( - owner, + workspace.owner, workspace, "Snazzy Project", "A new project for me and my friends", {"env1", "env2"}, ) - user_session(owner) + user_session(workspace.owner) response = client.get( "/workspaces/{}/projects/{}/edit".format(workspace.id, project.id) ) @@ -168,12 +173,11 @@ def test_user_without_permission_cannot_update_project(client, user_session): def test_create_member(client, user_session): - owner, workspace = WorkspaceFactory.create_user_and_workspace_with_role("admin") - - user_session(owner) + user = UserFactory.create() + workspace = WorkspaceFactory.create() + user_session(workspace.owner) queue_length = len(queue.get_queue()) - user = UserFactory.create() response = client.post( url_for("workspaces.create_member", workspace_id=workspace.id), data={ @@ -193,8 +197,10 @@ def test_create_member(client, user_session): def test_permissions_for_view_member(client, user_session): - user, workspace = WorkspaceFactory.create_user_and_workspace_with_role("developer") - + user = UserFactory.create() + workspace = WorkspaceFactory.create() + Workspaces._create_workspace_role(user, workspace, "developer") + member = WorkspaceUsers.add(user, workspace.id, "developer") user_session(user) response = client.post( url_for("workspaces.view_member", workspace_id=workspace.id, member_id=user.id), @@ -204,11 +210,10 @@ def test_permissions_for_view_member(client, user_session): def test_update_member_workspace_role(client, user_session): - owner, workspace = WorkspaceFactory.create_user_and_workspace_with_role("admin") - + workspace = WorkspaceFactory.create() user = UserFactory.create() member = WorkspaceUsers.add(user, workspace.id, "developer") - user_session(owner) + user_session(workspace.owner) response = client.post( url_for( "workspaces.update_member", workspace_id=workspace.id, member_id=user.id @@ -221,11 +226,10 @@ def test_update_member_workspace_role(client, user_session): def test_update_member_workspace_role_with_no_data(client, user_session): - owner, workspace = WorkspaceFactory.create_user_and_workspace_with_role("admin") - + workspace = WorkspaceFactory.create() user = UserFactory.create() member = WorkspaceUsers.add(user, workspace.id, "developer") - user_session(owner) + user_session(workspace.owner) response = client.post( url_for( "workspaces.update_member", workspace_id=workspace.id, member_id=user.id @@ -238,12 +242,11 @@ def test_update_member_workspace_role_with_no_data(client, user_session): def test_update_member_environment_role(client, user_session): - owner, workspace = WorkspaceFactory.create_user_and_workspace_with_role("admin") - + workspace = WorkspaceFactory.create() user = UserFactory.create() member = WorkspaceUsers.add(user, workspace.id, "developer") project = Projects.create( - owner, + workspace.owner, workspace, "Snazzy Project", "A new project for me and my friends", @@ -253,7 +256,7 @@ def test_update_member_environment_role(client, user_session): env2_id = project.environments[1].id for env in project.environments: Environments.add_member(env, user, "developer") - user_session(owner) + user_session(workspace.owner) response = client.post( url_for( "workspaces.update_member", workspace_id=workspace.id, member_id=user.id @@ -271,12 +274,11 @@ def test_update_member_environment_role(client, user_session): def test_update_member_environment_role_with_no_data(client, user_session): - owner, workspace = WorkspaceFactory.create_user_and_workspace_with_role("admin") - + workspace = WorkspaceFactory.create() user = UserFactory.create() member = WorkspaceUsers.add(user, workspace.id, "developer") project = Projects.create( - owner, + workspace.owner, workspace, "Snazzy Project", "A new project for me and my friends", @@ -285,7 +287,7 @@ def test_update_member_environment_role_with_no_data(client, user_session): env1_id = project.environments[0].id for env in project.environments: Environments.add_member(env, user, "developer") - user_session(owner) + user_session(workspace.owner) response = client.post( url_for( "workspaces.update_member", workspace_id=workspace.id, member_id=user.id @@ -298,11 +300,12 @@ def test_update_member_environment_role_with_no_data(client, user_session): def test_new_member_accepts_valid_invite(client, user_session): - owner, workspace = WorkspaceFactory.create_user_and_workspace_with_role("admin") - + workspace = WorkspaceFactory.create() user = UserFactory.create() - member = WorkspaceUsers.add(user, workspace.id, "developer") - invite = InvitationFactory.create(user_id=member.user.id, workspace_id=workspace.id) + ws_role = WorkspaceRoleFactory.create( + workspace=workspace, user=user, status=WorkspaceRoleStatus.PENDING + ) + invite = InvitationFactory.create(user_id=user.id, workspace_role_id=ws_role.id) # the user does not have access to the workspace before accepting the invite assert len(Workspaces.for_user(user)) == 0 @@ -323,14 +326,13 @@ def test_new_member_accepts_valid_invite(client, user_session): def test_new_member_accept_invalid_invite(client, user_session): - owner, workspace = WorkspaceFactory.create_user_and_workspace_with_role("admin") - + workspace = WorkspaceFactory.create() user = UserFactory.create() - member = WorkspaceUsers.add(user, workspace.id, "developer") + ws_role = WorkspaceRoleFactory.create( + user=user, workspace=workspace, status=WorkspaceRoleStatus.PENDING + ) invite = InvitationFactory.create( - user_id=member.user.id, - workspace_id=workspace.id, - status=InvitationStatus.REJECTED, + user_id=user.id, workspace_role_id=ws_role.id, status=InvitationStatus.REJECTED ) user_session(user) response = client.get(url_for("workspaces.accept_invitation", token=invite.token)) @@ -341,7 +343,6 @@ 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() - Invitations.create_for_owner(workspace, workspace.owner) # create user in workspace with invitation user_session(workspace.owner) diff --git a/tests/test_auth.py b/tests/test_auth.py index 13a6896c..099dc4ec 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -211,17 +211,3 @@ def test_redirected_on_login(client, monkeypatch): target_route = url_for("requests.requests_form_new", screen=1) response = _login(client, next=target_route) assert target_route in response.headers.get("Location") - - -def test_invited_user_finalized_on_login(monkeypatch, client): - user = UserFactory.create(provisional=True) - monkeypatch.setattr( - "atst.domain.authnid.AuthenticationContext.authenticate", lambda *args: True - ) - monkeypatch.setattr( - "atst.domain.authnid.AuthenticationContext.get_user", lambda *args: user - ) - - resp = _login(client) - - assert not user.provisional From 2aa3fbb12999689f83b00638559dbe3374cfec45 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 30 Oct 2018 16:54:08 -0400 Subject: [PATCH 20/23] revise migration chain --- ...c1ac77c_add_provisional_column_to_users.py | 4 +- ...ons.py => 994a80ee92c9_add_invitations.py} | 26 ++++++------ ...e4be_change_invitation_relationship_to_.py | 42 ------------------- ...c3cd315c1_add_status_to_workspace_roles.py | 4 +- 4 files changed, 17 insertions(+), 59 deletions(-) rename alembic/versions/{25bcba9b99a9_add_invitations.py => 994a80ee92c9_add_invitations.py} (80%) delete mode 100644 alembic/versions/d1ea7f3ee4be_change_invitation_relationship_to_.py diff --git a/alembic/versions/5284ac1ac77c_add_provisional_column_to_users.py b/alembic/versions/5284ac1ac77c_add_provisional_column_to_users.py index 8ad52723..7078b713 100644 --- a/alembic/versions/5284ac1ac77c_add_provisional_column_to_users.py +++ b/alembic/versions/5284ac1ac77c_add_provisional_column_to_users.py @@ -1,7 +1,7 @@ """add provisional column to users Revision ID: 5284ac1ac77c -Revises: 25bcba9b99a9 +Revises: e0fc3cd315c1 Create Date: 2018-10-25 11:04:49.879393 """ @@ -11,7 +11,7 @@ import sqlalchemy as sa # revision identifiers, used by Alembic. revision = '5284ac1ac77c' -down_revision = '25bcba9b99a9' +down_revision = 'e0fc3cd315c1' branch_labels = None depends_on = None diff --git a/alembic/versions/25bcba9b99a9_add_invitations.py b/alembic/versions/994a80ee92c9_add_invitations.py similarity index 80% rename from alembic/versions/25bcba9b99a9_add_invitations.py rename to alembic/versions/994a80ee92c9_add_invitations.py index 8aafdfcb..c480ec9b 100644 --- a/alembic/versions/25bcba9b99a9_add_invitations.py +++ b/alembic/versions/994a80ee92c9_add_invitations.py @@ -1,8 +1,8 @@ """add invitations -Revision ID: 25bcba9b99a9 +Revision ID: 994a80ee92c9 Revises: 9c24c609878a -Create Date: 2018-10-23 15:03:12.641069 +Create Date: 2018-10-30 16:49:53.688621 """ from alembic import op @@ -10,7 +10,7 @@ import sqlalchemy as sa from sqlalchemy.dialects import postgresql # revision identifiers, used by Alembic. -revision = '25bcba9b99a9' +revision = '994a80ee92c9' down_revision = '9c24c609878a' branch_labels = None depends_on = None @@ -23,28 +23,28 @@ def upgrade(): sa.Column('time_updated', sa.TIMESTAMP(timezone=True), server_default=sa.text('now()'), nullable=False), sa.Column('id', postgresql.UUID(as_uuid=True), server_default=sa.text('uuid_generate_v4()'), nullable=False), sa.Column('user_id', postgresql.UUID(as_uuid=True), nullable=True), - sa.Column('workspace_id', postgresql.UUID(as_uuid=True), nullable=True), - sa.Column('status', sa.Enum('ACCEPTED', 'REVOKED', 'PENDING', 'REJECTED', name='status', native_enum=False), nullable=True), + sa.Column('workspace_role_id', postgresql.UUID(as_uuid=True), nullable=True), sa.Column('inviter_id', postgresql.UUID(as_uuid=True), nullable=True), - sa.Column('token', sa.String(), nullable=True), + sa.Column('status', sa.Enum('ACCEPTED', 'REVOKED', 'PENDING', 'REJECTED', name='status', native_enum=False), nullable=True), sa.Column('expiration_time', sa.TIMESTAMP(timezone=True), nullable=True), - sa.ForeignKeyConstraint(['user_id'], ['users.id'], ), + sa.Column('token', sa.String(), nullable=True), sa.ForeignKeyConstraint(['inviter_id'], ['users.id'], ), - sa.ForeignKeyConstraint(['workspace_id'], ['workspaces.id'], ), + sa.ForeignKeyConstraint(['user_id'], ['users.id'], ), + sa.ForeignKeyConstraint(['workspace_role_id'], ['workspace_roles.id'], ), sa.PrimaryKeyConstraint('id') ) - op.create_index(op.f('ix_invitations_user_id'), 'invitations', ['user_id'], unique=False) op.create_index(op.f('ix_invitations_inviter_id'), 'invitations', ['inviter_id'], unique=False) - op.create_index(op.f('ix_invitations_workspace_id'), 'invitations', ['workspace_id'], unique=False) - op.create_index(op.f('ix_invitations_token'), 'invitations', ['token'], unique=True) + op.create_index(op.f('ix_invitations_token'), 'invitations', ['token'], unique=False) + op.create_index(op.f('ix_invitations_user_id'), 'invitations', ['user_id'], unique=False) + op.create_index(op.f('ix_invitations_workspace_role_id'), 'invitations', ['workspace_role_id'], unique=False) # ### end Alembic commands ### def downgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.drop_index(op.f('ix_invitations_workspace_id'), table_name='invitations') + op.drop_index(op.f('ix_invitations_workspace_role_id'), table_name='invitations') op.drop_index(op.f('ix_invitations_user_id'), table_name='invitations') - op.drop_index(op.f('ix_invitations_inviter_id'), table_name='invitations') op.drop_index(op.f('ix_invitations_token'), table_name='invitations') + op.drop_index(op.f('ix_invitations_inviter_id'), table_name='invitations') op.drop_table('invitations') # ### end Alembic commands ### diff --git a/alembic/versions/d1ea7f3ee4be_change_invitation_relationship_to_.py b/alembic/versions/d1ea7f3ee4be_change_invitation_relationship_to_.py deleted file mode 100644 index 22175d4d..00000000 --- a/alembic/versions/d1ea7f3ee4be_change_invitation_relationship_to_.py +++ /dev/null @@ -1,42 +0,0 @@ -"""change invitation relationship to workspace role - -Revision ID: d1ea7f3ee4be -Revises: 5284ac1ac77c -Create Date: 2018-10-30 14:09:42.277467 - -""" -from alembic import op -import sqlalchemy as sa -from sqlalchemy.dialects import postgresql - -# revision identifiers, used by Alembic. -revision = 'd1ea7f3ee4be' -down_revision = '5284ac1ac77c' -branch_labels = None -depends_on = None - - -def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.add_column('invitations', sa.Column('workspace_role_id', postgresql.UUID(as_uuid=True), nullable=True)) - op.create_index(op.f('ix_invitations_workspace_role_id'), 'invitations', ['workspace_role_id'], unique=False) - op.drop_index('ix_invitations_token', table_name='invitations') - op.create_index(op.f('ix_invitations_token'), 'invitations', ['token'], unique=False) - op.drop_index('ix_invitations_workspace_id', table_name='invitations') - op.drop_constraint('invitations_workspace_id_fkey', 'invitations', type_='foreignkey') - op.create_foreign_key(None, 'invitations', 'workspace_roles', ['workspace_role_id'], ['id']) - op.drop_column('invitations', 'workspace_id') - # ### end Alembic commands ### - - -def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.add_column('invitations', sa.Column('workspace_id', postgresql.UUID(), autoincrement=False, nullable=True)) - op.drop_constraint(None, 'invitations', type_='foreignkey') - op.create_foreign_key('invitations_workspace_id_fkey', 'invitations', 'workspaces', ['workspace_id'], ['id']) - op.create_index('ix_invitations_workspace_id', 'invitations', ['workspace_id'], unique=False) - op.drop_index(op.f('ix_invitations_token'), table_name='invitations') - op.create_index('ix_invitations_token', 'invitations', ['token'], unique=True) - op.drop_index(op.f('ix_invitations_workspace_role_id'), table_name='invitations') - op.drop_column('invitations', 'workspace_role_id') - # ### end Alembic commands ### diff --git a/alembic/versions/e0fc3cd315c1_add_status_to_workspace_roles.py b/alembic/versions/e0fc3cd315c1_add_status_to_workspace_roles.py index 810b5691..a852e5ef 100644 --- a/alembic/versions/e0fc3cd315c1_add_status_to_workspace_roles.py +++ b/alembic/versions/e0fc3cd315c1_add_status_to_workspace_roles.py @@ -1,7 +1,7 @@ """add status to workspace_roles Revision ID: e0fc3cd315c1 -Revises: d1ea7f3ee4be +Revises: 994a80ee92c9 Create Date: 2018-10-30 14:36:51.047876 """ @@ -11,7 +11,7 @@ import sqlalchemy as sa # revision identifiers, used by Alembic. revision = 'e0fc3cd315c1' -down_revision = 'd1ea7f3ee4be' +down_revision = '994a80ee92c9' branch_labels = None depends_on = None From faa98fcb1728eead0b60a46c83f5bb160645141d Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 31 Oct 2018 08:55:57 -0400 Subject: [PATCH 21/23] include auditable mixin in invitation --- atst/models/invitation.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/atst/models/invitation.py b/atst/models/invitation.py index 3ac1a4bf..63fc74a9 100644 --- a/atst/models/invitation.py +++ b/atst/models/invitation.py @@ -8,6 +8,7 @@ from sqlalchemy.orm import relationship from atst.models import Base, types from atst.models.mixins.timestamps import TimestampsMixin +from atst.models.mixins.auditable import AuditableMixin class Status(Enum): @@ -17,7 +18,7 @@ class Status(Enum): REJECTED = "rejected" -class Invitation(Base, TimestampsMixin): +class Invitation(Base, TimestampsMixin, AuditableMixin): __tablename__ = "invitations" id = types.Id() From 3a5d8036a1d399209967a414e8ecf93dac3f9c1b Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 31 Oct 2018 09:21:29 -0400 Subject: [PATCH 22/23] some cleanup for invitation changes --- atst/domain/workspaces/query.py | 9 --------- atst/models/invitation.py | 4 ++-- atst/routes/errors.py | 2 +- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/atst/domain/workspaces/query.py b/atst/domain/workspaces/query.py index 021edf9d..6ac00d8a 100644 --- a/atst/domain/workspaces/query.py +++ b/atst/domain/workspaces/query.py @@ -32,12 +32,3 @@ class WorkspacesQuery(Query): @classmethod 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): - return ( - db.session.query(WorkspaceRole) - .filter(WorkspaceRole.user == user) - .filter(WorkspaceRole.workspace == workspace) - .one() - ) diff --git a/atst/models/invitation.py b/atst/models/invitation.py index 63fc74a9..60f03b3c 100644 --- a/atst/models/invitation.py +++ b/atst/models/invitation.py @@ -41,8 +41,8 @@ class Invitation(Base, TimestampsMixin, AuditableMixin): token = Column(String(), index=True, default=lambda: secrets.token_urlsafe()) def __repr__(self): - return "".format( - self.user.id, self.workspace.id, self.id + return "".format( + self.user_id, self.workspace_role_id, self.id ) @property diff --git a/atst/routes/errors.py b/atst/routes/errors.py index ef1b3397..4871f89d 100644 --- a/atst/routes/errors.py +++ b/atst/routes/errors.py @@ -44,7 +44,7 @@ def make_error_pages(app): @app.errorhandler(InvitationError) # pylint: disable=unused-variable - def expired_invitation(e): + def invalid_invitation(e): log_error(e) return ( render_template( From 8cbd3d0390605248af29b6f9d88a1ea413164e7f Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 31 Oct 2018 11:17:28 -0400 Subject: [PATCH 23/23] migration to update existing workspace_role statuses --- ...21c_status_for_existing_workspace_roles.py | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 alembic/versions/a9d8c6b6221c_status_for_existing_workspace_roles.py diff --git a/alembic/versions/a9d8c6b6221c_status_for_existing_workspace_roles.py b/alembic/versions/a9d8c6b6221c_status_for_existing_workspace_roles.py new file mode 100644 index 00000000..25ba77cc --- /dev/null +++ b/alembic/versions/a9d8c6b6221c_status_for_existing_workspace_roles.py @@ -0,0 +1,30 @@ +"""status for existing workspace roles + +Revision ID: a9d8c6b6221c +Revises: 5284ac1ac77c +Create Date: 2018-10-31 11:08:05.791739 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'a9d8c6b6221c' +down_revision = '5284ac1ac77c' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + conn = op.get_bind() + conn.execute("UPDATE workspace_roles set status = 'ACTIVE'") + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + conn = op.get_bind() + conn.execute("UPDATE workspace_roles set status = null") + # ### end Alembic commands ###