diff --git a/alembic/versions/02d11579a581_add_email_to_invite.py b/alembic/versions/02d11579a581_add_email_to_invite.py new file mode 100644 index 00000000..cd21df51 --- /dev/null +++ b/alembic/versions/02d11579a581_add_email_to_invite.py @@ -0,0 +1,50 @@ +"""Add email to invite + +Revision ID: 02d11579a581 +Revises: 4f46aecb337f +Create Date: 2018-11-19 14:51:33.178358 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.sql import text + + +# revision identifiers, used by Alembic. +revision = '02d11579a581' +down_revision = '4f46aecb337f' +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column('invitations', sa.Column('email', sa.String())) + conn = op.get_bind() + # Add non-null value to email column + conn.execute( + text( + """ + insert into invitations (email) + select u.email + from invitations i + inner join workspace_roles wr on i.workspace_role_id = wr.id + inner join users u on wr.user_id = u.id + where i.email is null; + """ + ) + ) + conn.execute( + text( + """ + update invitations + set email = 'example@example.com' + where email is null; + """ + ) + ) + op.alter_column('invitations', 'email', nullable=False) + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('invitations', 'email') + # ### end Alembic commands ### diff --git a/atst/domain/invitations.py b/atst/domain/invitations.py index 738525ab..5f69e039 100644 --- a/atst/domain/invitations.py +++ b/atst/domain/invitations.py @@ -54,13 +54,14 @@ class Invitations(object): return invite @classmethod - def create(cls, inviter, workspace_role): + def create(cls, inviter, workspace_role, email): invite = Invitation( workspace_role=workspace_role, inviter=inviter, user=workspace_role.user, status=InvitationStatus.PENDING, expiration_time=Invitations.current_expiration_time(), + email=email, ) db.session.add(invite) db.session.commit() @@ -120,4 +121,6 @@ class Invitations(object): previous_invitation = Invitations._get(token) Invitations._update_status(previous_invitation, InvitationStatus.REVOKED) - return Invitations.create(user, previous_invitation.workspace_role) + return Invitations.create( + user, previous_invitation.workspace_role, previous_invitation.email + ) diff --git a/atst/models/environment_role.py b/atst/models/environment_role.py index b236617d..6b46bef5 100644 --- a/atst/models/environment_role.py +++ b/atst/models/environment_role.py @@ -10,7 +10,7 @@ class CSPRole(Enum): NONSENSE_ROLE = "nonsense_role" -class EnvironmentRole(Base, mixins.TimestampsMixin): +class EnvironmentRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): __tablename__ = "environment_roles" id = types.Id() @@ -29,6 +29,23 @@ class EnvironmentRole(Base, mixins.TimestampsMixin): self.role, self.user.full_name, self.environment.name, self.id ) + @property + def history(self): + return self.get_changes() + + @property + def event_details(self): + return { + "updated_user_name": self.user.displayname, + "updated_user_id": str(self.user_id), + "environment": self.environment.displayname, + "environment_id": str(self.environment_id), + "project": self.environment.project.name, + "project_id": str(self.environment.project_id), + "workspace": self.environment.project.workspace.name, + "workspace_id": str(self.environment.project.workspace.id), + } + Index( "environments_role_user_environment", diff --git a/atst/models/invitation.py b/atst/models/invitation.py index 3768849c..6097361a 100644 --- a/atst/models/invitation.py +++ b/atst/models/invitation.py @@ -42,11 +42,13 @@ class Invitation(Base, TimestampsMixin, AuditableMixin): expiration_time = Column(TIMESTAMP(timezone=True)) - token = Column(String(), index=True, default=lambda: secrets.token_urlsafe()) + token = Column(String, index=True, default=lambda: secrets.token_urlsafe()) + + email = Column(String, nullable=False) def __repr__(self): - return "".format( - self.user_id, self.workspace_role_id, self.id + return "".format( + self.user_id, self.workspace_role_id, self.id, self.email ) @property @@ -82,10 +84,6 @@ class Invitation(Base, TimestampsMixin, AuditableMixin): if self.workspace_role: return self.workspace_role.workspace - @property - def user_email(self): - return self.workspace_role.user.email - @property def user_name(self): return self.workspace_role.user.full_name diff --git a/atst/models/mixins/auditable.py b/atst/models/mixins/auditable.py index 8aee817a..b39c8768 100644 --- a/atst/models/mixins/auditable.py +++ b/atst/models/mixins/auditable.py @@ -17,9 +17,12 @@ class AuditableMixin(object): request_id = resource.auditable_request_id() resource_type = resource.auditable_resource_type() display_name = resource.auditable_displayname() - changed_state = resource.auditable_changed_state() event_details = resource.auditable_event_details() + changed_state = ( + resource.auditable_changed_state() if action == ACTION_UPDATE else None + ) + audit_event = AuditEvent( user_id=user_id, workspace_id=workspace_id, @@ -52,14 +55,12 @@ class AuditableMixin(object): @staticmethod def audit_update(mapper, connection, target): - target.create_audit_event(connection, target, ACTION_UPDATE) + if AuditableMixin.get_changes(target): + target.create_audit_event(connection, target, ACTION_UPDATE) def get_changes(self): """ - This function borrows largely from a gist: - https://gist.github.com/ngse/c20058116b8044c65d3fbceda3fdf423#file-audit_mixin-py-L106-L120 - - It returns a dictionary of the form {item: [from_value, to_value]}, + This function returns a dictionary of the form {item: [from_value, to_value]}, where 'item' is the attribute on the target that has been updated, 'from_value' is the value of the attribute before it was updated, and 'to_value' is the current value of the attribute. @@ -71,7 +72,9 @@ class AuditableMixin(object): for attr in attrs: history = getattr(inspect(self).attrs, attr.key).history if history.has_changes(): - previous_state[attr.key] = [history.deleted.pop(), history.added.pop()] + deleted = history.deleted.pop() if history.deleted else None + added = history.added.pop() if history.added else None + previous_state[attr.key] = [deleted, added] return previous_state def auditable_changed_state(self): diff --git a/atst/routes/workspaces.py b/atst/routes/workspaces.py index c216d5ee..c0a0cab6 100644 --- a/atst/routes/workspaces.py +++ b/atst/routes/workspaces.py @@ -262,10 +262,8 @@ def create_member(workspace_id): if form.validate(): try: new_member = Workspaces.create_member(user, workspace, form.data) - invite = Invitations.create(user, new_member) - send_invite_email( - g.current_user.full_name, invite.token, new_member.user.email - ) + invite = Invitations.create(user, new_member, form.data["email"]) + send_invite_email(g.current_user.full_name, invite.token, invite.email) return redirect( url_for( @@ -381,7 +379,7 @@ def revoke_invitation(workspace_id, token): @bp.route("/workspaces//invitations//resend", methods=["POST"]) def resend_invitation(workspace_id, token): invite = Invitations.resend(g.current_user, workspace_id, token) - send_invite_email(g.current_user.full_name, invite.token, invite.user_email) + send_invite_email(g.current_user.full_name, invite.token, invite.email) return redirect( url_for( "workspaces.workspace_members", diff --git a/js/components/__tests__/__snapshots__/confirmation_popover.test.js.snap b/js/components/__tests__/__snapshots__/confirmation_popover.test.js.snap new file mode 100644 index 00000000..2146b1aa --- /dev/null +++ b/js/components/__tests__/__snapshots__/confirmation_popover.test.js.snap @@ -0,0 +1,3 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`ConfirmationPopover matches snapshot 1`] = ` `; diff --git a/js/components/__tests__/confirmation_popover.test.js b/js/components/__tests__/confirmation_popover.test.js new file mode 100644 index 00000000..cf31b253 --- /dev/null +++ b/js/components/__tests__/confirmation_popover.test.js @@ -0,0 +1,32 @@ +import { createLocalVue, shallowMount } from '@vue/test-utils' +import VTooltip from 'v-tooltip' + +import ConfirmationPopover from '../confirmation_popover' + +const localVue = createLocalVue() +localVue.use(VTooltip) + +describe('ConfirmationPopover', () => { + const wrapper = shallowMount(ConfirmationPopover, { + localVue, + propsData: { + action: '/some-url', + btn_text: 'Do something dangerous', + cancel_btn_text: 'Cancel', + confirm_btn_text: 'Confirm', + confirm_msg: 'Are you sure you want to do that?', + csrf_token: '42' + } + }) + + it('matches snapshot', () => { + expect(wrapper).toMatchSnapshot() + }) + + it('renders form with hidden csrf input', () => { + const input = wrapper.find('input[type=hidden]') + expect(input.exists()).toBe(true) + expect(input.attributes('value')).toBe('42') + }) +}) + diff --git a/js/components/confirmation_popover.js b/js/components/confirmation_popover.js new file mode 100644 index 00000000..5bccc719 --- /dev/null +++ b/js/components/confirmation_popover.js @@ -0,0 +1,32 @@ +export default { + name: 'confirmation-popover', + + props: { + action: String, + btn_text: String, + cancel_btn_text: String, + confirm_btn_text: String, + confirm_msg: String, + csrf_token: String + }, + + template: ` + + + + + ` +} diff --git a/js/index.js b/js/index.js index 01005a94..c31f8273 100644 --- a/js/index.js +++ b/js/index.js @@ -23,6 +23,7 @@ import CcpoApproval from './components/forms/ccpo_approval' import MembersList from './components/members_list' import LocalDatetime from './components/local_datetime' import RequestsList from './components/requests_list' +import ConfirmationPopover from './components/confirmation_popover' Vue.config.productionTip = false @@ -50,6 +51,7 @@ const app = new Vue({ EditEnvironmentRole, EditProjectRoles, RequestsList, + ConfirmationPopover, }, mounted: function() { diff --git a/templates/audit_log.html b/templates/audit_log.html index 5e679dfe..b4b41daa 100644 --- a/templates/audit_log.html +++ b/templates/audit_log.html @@ -30,12 +30,16 @@ {% if event.event_details %} for User {{ event.event_details.updated_user_id }} ({{ event.event_details.updated_user_name }}) -
- {% endif %} - {% if event.changed_state %} - from {{ event.changed_state.role[0] }} to {{ event.changed_state.role[1] }} -
+ {% if event.event_details["environment"] %} +
+ in Environment {{ event.event_details["environment_id"] }} ({{ event.event_details["environment"] }}) +
+ in Project {{ event.event_details["project_id"] }} ({{ event.event_details["project"] }}) +
+ in Workspace {{ event.event_details["workspace_id"] }} ({{ event.event_details["workspace"] }}) + {% endif %} +
{% endif %} {% if event.workspace %} @@ -43,6 +47,11 @@ {% elif event.request %} on Request {{ event.request_id }} ({{ event.request.displayname }}) {% endif %} + + {% if event.changed_state.role %} + from {{ event.changed_state.role[0] }} to {{ event.changed_state.role[1] }} +
+ {% endif %} diff --git a/templates/components/confirmation_button.html b/templates/components/confirmation_button.html index 71b15a3b..31db3355 100644 --- a/templates/components/confirmation_button.html +++ b/templates/components/confirmation_button.html @@ -1,19 +1,10 @@ -{% macro ConfirmationButton(btn_text, action, csrf_token, confirm_msg="Are you sure?", confirm_btn="Confirm", cancel_btn="Cancel") -%} - - - - +{% macro ConfirmationButton(btn_text, action, confirm_msg="Are you sure?", confirm_btn="Confirm", cancel_btn="Cancel") -%} + + {%- endmacro %} diff --git a/templates/navigation/global_navigation.html b/templates/navigation/global_navigation.html index 245474cb..093b86bd 100644 --- a/templates/navigation/global_navigation.html +++ b/templates/navigation/global_navigation.html @@ -2,17 +2,6 @@
    - {% if g.dev %} - {{ SidenavItem("Styleguide", - href="/styleguide", - icon="visible", - active=g.matchesPath('/styleguide'), - subnav=[ - {"label":"Subnav 1", "href":"/styleguide?subnav1", "icon": "plus", "active": g.matchesPath('/styleguide?subnav1')}, - {"label":"Subnav 2", "href":"/styleguide?subnav2", "active": g.matchesPath('/styleguide?subnav2')}, - ]) }} - {% endif %} - {{ SidenavItem("Requests", href="/requests", icon="document", diff --git a/templates/workspaces/members/edit.html b/templates/workspaces/members/edit.html index 0c1e8f7a..0b609e5b 100644 --- a/templates/workspaces/members/edit.html +++ b/templates/workspaces/members/edit.html @@ -44,14 +44,12 @@ {{ ConfirmationButton( "Revoke Invitation", url_for("workspaces.revoke_invitation", workspace_id=workspace.id, token=member.latest_invitation.token), - form.csrf_token ) }} {% endif %} {% if member.can_resend_invitation %} {{ ConfirmationButton ( "Resend Invitation", url_for("workspaces.resend_invitation", workspace_id=workspace.id, token=member.latest_invitation.token), - form.csrf_token, confirm_msg="Are you sure? This will send an email to invite the user to join this workspace." )}} {% endif %} diff --git a/tests/domain/test_invitations.py b/tests/domain/test_invitations.py index e63258e8..4f4a1492 100644 --- a/tests/domain/test_invitations.py +++ b/tests/domain/test_invitations.py @@ -22,7 +22,7 @@ def test_create_invitation(): workspace = WorkspaceFactory.create() user = UserFactory.create() ws_role = WorkspaceRoleFactory.create(user=user, workspace=workspace) - invite = Invitations.create(workspace.owner, ws_role) + invite = Invitations.create(workspace.owner, ws_role, user.email) assert invite.user == user assert invite.workspace_role == ws_role assert invite.inviter == workspace.owner @@ -34,7 +34,7 @@ def test_accept_invitation(): workspace = WorkspaceFactory.create() user = UserFactory.create() ws_role = WorkspaceRoleFactory.create(user=user, workspace=workspace) - invite = Invitations.create(workspace.owner, ws_role) + invite = Invitations.create(workspace.owner, ws_role, user.email) assert invite.is_pending accepted_invite = Invitations.accept(user, invite.token) assert accepted_invite.is_accepted @@ -89,7 +89,7 @@ def test_accept_invitation_twice(): workspace = WorkspaceFactory.create() user = UserFactory.create() ws_role = WorkspaceRoleFactory.create(user=user, workspace=workspace) - invite = Invitations.create(workspace.owner, ws_role) + invite = Invitations.create(workspace.owner, ws_role, user.email) Invitations.accept(user, invite.token) with pytest.raises(InvitationError): Invitations.accept(user, invite.token) @@ -99,7 +99,7 @@ def test_revoke_invitation(): workspace = WorkspaceFactory.create() user = UserFactory.create() ws_role = WorkspaceRoleFactory.create(user=user, workspace=workspace) - invite = Invitations.create(workspace.owner, ws_role) + invite = Invitations.create(workspace.owner, ws_role, user.email) assert invite.is_pending Invitations.revoke(invite.token) assert invite.is_revoked @@ -109,7 +109,7 @@ def test_resend_invitation(): workspace = WorkspaceFactory.create() user = UserFactory.create() ws_role = WorkspaceRoleFactory.create(user=user, workspace=workspace) - invite = Invitations.create(workspace.owner, ws_role) + invite = Invitations.create(workspace.owner, ws_role, user.email) Invitations.resend(workspace.owner, workspace.id, invite.token) assert ws_role.invitations[0].is_revoked assert ws_role.invitations[1].is_pending diff --git a/tests/domain/test_workspaces.py b/tests/domain/test_workspaces.py index efec7eb3..901089b0 100644 --- a/tests/domain/test_workspaces.py +++ b/tests/domain/test_workspaces.py @@ -134,7 +134,8 @@ def test_update_workspace_role_role(workspace, workspace_owner): "workspace_role": "developer", "dod_id": "1234567890", } - member = Workspaces.create_member(workspace_owner, workspace, user_data) + WorkspaceRoleFactory._meta.sqlalchemy_session_persistence = "flush" + member = WorkspaceRoleFactory.create(workspace=workspace) role_name = "admin" updated_member = Workspaces.update_member( diff --git a/tests/factories.py b/tests/factories.py index 7c7bdfd8..96e9c22c 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -342,5 +342,6 @@ class InvitationFactory(Base): class Meta: model = Invitation + email = factory.Faker("email") status = InvitationStatus.PENDING expiration_time = Invitations.current_expiration_time() diff --git a/tests/models/test_workspace_role.py b/tests/models/test_workspace_role.py index d59b85cc..990c8e0b 100644 --- a/tests/models/test_workspace_role.py +++ b/tests/models/test_workspace_role.py @@ -13,10 +13,13 @@ from tests.factories import ( UserFactory, InvitationFactory, WorkspaceRoleFactory, + EnvironmentFactory, + EnvironmentRoleFactory, + ProjectFactory, ) -def test_has_no_history(session): +def test_has_no_ws_role_history(session): owner = UserFactory.create() user = UserFactory.create() @@ -33,7 +36,7 @@ def test_has_no_history(session): assert not create_event.changed_state -def test_has_role_history(session): +def test_has_ws_role_history(session): owner = UserFactory.create() user = UserFactory.create() @@ -58,7 +61,7 @@ def test_has_role_history(session): assert changed_events[0].changed_state["role"][1] == "admin" -def test_has_status_history(session): +def test_has_ws_status_history(session): owner = UserFactory.create() user = UserFactory.create() @@ -81,6 +84,49 @@ def test_has_status_history(session): assert changed_events[0].changed_state["status"][1] == "active" +def test_has_no_env_role_history(session): + owner = UserFactory.create() + user = UserFactory.create() + workspace = Workspaces.create(RequestFactory.create(creator=owner)) + project = ProjectFactory.create(workspace=workspace) + environment = EnvironmentFactory.create(project=project, name="new environment!") + + env_role = EnvironmentRoleFactory.create( + user=user, environment=environment, role="developer" + ) + create_event = ( + session.query(AuditEvent) + .filter(AuditEvent.resource_id == env_role.id, AuditEvent.action == "create") + .one() + ) + + assert not create_event.changed_state + + +def test_has_env_role_history(session): + owner = UserFactory.create() + user = UserFactory.create() + workspace = Workspaces.create(RequestFactory.create(creator=owner)) + workspace_role = WorkspaceRoleFactory.create(workspace=workspace, user=user) + project = ProjectFactory.create(workspace=workspace) + environment = EnvironmentFactory.create(project=project, name="new environment!") + + env_role = EnvironmentRoleFactory.create( + user=user, environment=environment, role="developer" + ) + Environments.update_environment_roles( + owner, workspace, workspace_role, [{"role": "admin", "id": environment.id}] + ) + changed_events = ( + session.query(AuditEvent) + .filter(AuditEvent.resource_id == env_role.id, AuditEvent.action == "update") + .all() + ) + # changed_state["role"] returns a list [previous role, current role] + assert changed_events[0].changed_state["role"][0] == "developer" + assert changed_events[0].changed_state["role"][1] == "admin" + + def test_event_details(): owner = UserFactory.create() user = UserFactory.create() diff --git a/tests/routes/test_workspaces.py b/tests/routes/test_workspaces.py index 3737042e..c15ee2c5 100644 --- a/tests/routes/test_workspaces.py +++ b/tests/routes/test_workspaces.py @@ -1,5 +1,6 @@ import datetime from flask import url_for +import pytest from tests.factories import ( UserFactory, @@ -339,6 +340,29 @@ def test_existing_member_accepts_valid_invite(client, user_session): assert len(Workspaces.for_user(user)) == 1 +def test_existing_member_invite_sent_to_email_submitted_in_form( + client, user_session, queue +): + workspace = WorkspaceFactory.create() + user = UserFactory.create() + member_form_data = { + "dod_id": user.dod_id, + "first_name": user.first_name, + "last_name": user.last_name, + "workspace_role": "developer", + "email": "example@example.com", + } + user_session(workspace.owner) + client.post( + url_for("workspaces.create_member", workspace_id=workspace.id), + data={**member_form_data}, + ) + + assert user.email != "example@example.com" + assert len(queue.get_queue().jobs[0].args[0]) == 1 + assert queue.get_queue().jobs[0].args[0][0] == "example@example.com" + + def test_new_member_accepts_valid_invite(monkeypatch, client, user_session): workspace = WorkspaceFactory.create() user_info = UserFactory.dictionary() @@ -478,3 +502,32 @@ def test_resend_invitation_sends_email(client, user_session, queue): ) assert len(queue.get_queue()) == 1 + + +def test_existing_member_invite_resent_to_email_submitted_in_form( + client, user_session, queue +): + workspace = WorkspaceFactory.create() + user = UserFactory.create() + ws_role = WorkspaceRoleFactory.create( + user=user, workspace=workspace, status=WorkspaceRoleStatus.PENDING + ) + invite = InvitationFactory.create( + user_id=user.id, + workspace_role_id=ws_role.id, + status=InvitationStatus.PENDING, + email="example@example.com", + ) + user_session(workspace.owner) + client.post( + url_for( + "workspaces.resend_invitation", + workspace_id=workspace.id, + token=invite.token, + ) + ) + + send_mail_job = queue.get_queue().jobs[0] + assert user.email != "example@example.com" + assert send_mail_job.func.__func__.__name__ == "_send_mail" + assert send_mail_job.args[0] == ["example@example.com"]