diff --git a/alembic/versions/4f46aecb337f_add_columns_to_auditevent.py b/alembic/versions/4f46aecb337f_add_columns_to_auditevent.py new file mode 100644 index 00000000..03548f42 --- /dev/null +++ b/alembic/versions/4f46aecb337f_add_columns_to_auditevent.py @@ -0,0 +1,30 @@ +"""Add columns to AuditEvent + +Revision ID: 4f46aecb337f +Revises: 4c0b8263d800 +Create Date: 2018-11-12 16:03:55.281648 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = '4f46aecb337f' +down_revision = '4c0b8263d800' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('audit_events', sa.Column('changed_state', postgresql.JSON(astext_type=sa.Text()), nullable=True)) + op.add_column('audit_events', sa.Column('event_details', postgresql.JSON(astext_type=sa.Text()), nullable=True)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('audit_events', 'event_details') + op.drop_column('audit_events', 'changed_state') + # ### end Alembic commands ### diff --git a/atst/domain/audit_log.py b/atst/domain/audit_log.py index b5f8522c..a334cc7d 100644 --- a/atst/domain/audit_log.py +++ b/atst/domain/audit_log.py @@ -14,16 +14,6 @@ class AuditEventQuery(Query): class AuditLog(object): - @classmethod - def log_event(cls, user, resource, action): - return cls._log(user=user, resource=resource, action=action) - - @classmethod - def log_workspace_event(cls, user, workspace, resource, action): - return cls._log( - user=user, workspace_id=workspace.id, resource=resource, action=action - ) - @classmethod def log_system_event(cls, resource, action): return cls._log(resource=resource, action=action) diff --git a/atst/domain/workspace_roles.py b/atst/domain/workspace_roles.py index f6942e2f..ff9604fa 100644 --- a/atst/domain/workspace_roles.py +++ b/atst/domain/workspace_roles.py @@ -101,9 +101,8 @@ class WorkspaceRoles(object): return new_workspace_role @classmethod - def update_role(cls, member, workspace_id, role_name): + def update_role(cls, workspace_role, role_name): new_role = Roles.get(role_name) - workspace_role = WorkspaceRoles._get_workspace_role(member.user, workspace_id) workspace_role.role = new_role db.session.add(workspace_role) diff --git a/atst/domain/workspaces/workspaces.py b/atst/domain/workspaces/workspaces.py index 453a1eed..6c43bc3b 100644 --- a/atst/domain/workspaces/workspaces.py +++ b/atst/domain/workspaces/workspaces.py @@ -119,7 +119,7 @@ class Workspaces(object): "edit workspace member", ) - return WorkspaceRoles.update_role(member, workspace.id, role_name) + return WorkspaceRoles.update_role(member, role_name) @classmethod def _create_workspace_role( diff --git a/atst/models/audit_event.py b/atst/models/audit_event.py index 0c6403b9..dd5f94a2 100644 --- a/atst/models/audit_event.py +++ b/atst/models/audit_event.py @@ -1,5 +1,5 @@ from sqlalchemy import String, Column, ForeignKey, inspect -from sqlalchemy.dialects.postgresql import UUID +from sqlalchemy.dialects.postgresql import UUID, JSONB from sqlalchemy.orm import relationship from atst.models import Base, types @@ -20,6 +20,9 @@ class AuditEvent(Base, TimestampsMixin): request_id = Column(UUID(as_uuid=True), ForeignKey("requests.id"), index=True) request = relationship("Request", backref="audit_events") + changed_state = Column(JSONB()) + event_details = Column(JSONB()) + resource_type = Column(String(), nullable=False) resource_id = Column(UUID(as_uuid=True), index=True, nullable=False) display_name = Column(String()) diff --git a/atst/models/mixins/auditable.py b/atst/models/mixins/auditable.py index 1b6ae3d2..8aee817a 100644 --- a/atst/models/mixins/auditable.py +++ b/atst/models/mixins/auditable.py @@ -1,4 +1,4 @@ -from sqlalchemy import event +from sqlalchemy import event, inspect from flask import g from atst.models.audit_event import AuditEvent @@ -17,6 +17,8 @@ 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() audit_event = AuditEvent( user_id=user_id, @@ -26,6 +28,8 @@ class AuditableMixin(object): resource_id=resource.id, display_name=display_name, action=action, + changed_state=changed_state, + event_details=event_details, ) audit_event.save(connection) @@ -50,6 +54,32 @@ class AuditableMixin(object): def audit_update(mapper, connection, 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]}, + 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. + + There may be more than one item in the dictionary, but that is not expected. + """ + previous_state = {} + attrs = inspect(self).mapper.column_attrs + 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()] + return previous_state + + def auditable_changed_state(self): + return getattr_path(self, "history") + + def auditable_event_details(self): + return getattr_path(self, "event_details") + def auditable_resource_type(self): return camel_to_snake(type(self).__name__) diff --git a/atst/models/workspace_role.py b/atst/models/workspace_role.py index c006b87a..21d17a90 100644 --- a/atst/models/workspace_role.py +++ b/atst/models/workspace_role.py @@ -10,6 +10,7 @@ from atst.database import db from atst.models.environment_role import EnvironmentRole from atst.models.project import Project from atst.models.environment import Environment +from atst.models.role import Role class Status(Enum): @@ -34,13 +35,35 @@ 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)) + 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 ) + @property + def history(self): + previous_state = self.get_changes() + change_set = {} + if "role_id" in previous_state: + from_role_id = previous_state["role_id"][0] + from_role = db.session.query(Role).filter(Role.id == from_role_id).one() + to_role = self.role_name + change_set["role"] = [from_role.name, to_role] + if "status" in previous_state: + from_status = previous_state["status"][0].value + to_status = self.status.value + change_set["status"] = [from_status, to_status] + return change_set + + @property + def event_details(self): + return { + "updated_user_name": self.user_name, + "updated_user_id": str(self.user_id), + } + @property def latest_invitation(self): if self.invitations: diff --git a/templates/audit_log.html b/templates/audit_log.html index 8455276f..5e679dfe 100644 --- a/templates/audit_log.html +++ b/templates/audit_log.html @@ -28,6 +28,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] }} +
+ {% endif %} + {% if event.workspace %} in Workspace {{ event.workspace_id }} ({{ event.workspace.name }}) {% elif event.request %} diff --git a/tests/domain/test_audit_log.py b/tests/domain/test_audit_log.py index ab03c48c..3e000dde 100644 --- a/tests/domain/test_audit_log.py +++ b/tests/domain/test_audit_log.py @@ -20,7 +20,7 @@ def test_non_admin_cannot_view_audit_log(developer): AuditLog.get_all_events(developer) -def test_ccpo_can_iview_audit_log(ccpo): +def test_ccpo_can_view_audit_log(ccpo): AuditLog.get_all_events(ccpo) diff --git a/tests/factories.py b/tests/factories.py index 8d62a22d..7c7bdfd8 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -326,6 +326,7 @@ class WorkspaceRoleFactory(Base): workspace = factory.SubFactory(WorkspaceFactory) role = factory.SubFactory(RoleFactory) user = factory.SubFactory(UserFactory) + status = WorkspaceRoleStatus.PENDING class EnvironmentRoleFactory(Base): diff --git a/tests/models/test_workspace_role.py b/tests/models/test_workspace_role.py index 3622e88a..d59b85cc 100644 --- a/tests/models/test_workspace_role.py +++ b/tests/models/test_workspace_role.py @@ -5,7 +5,9 @@ from atst.domain.workspaces import Workspaces from atst.domain.projects import Projects from atst.domain.workspace_roles import WorkspaceRoles from atst.models.workspace_role import Status +from atst.models.role import Role from atst.models.invitation import Status as InvitationStatus +from atst.models.audit_event import AuditEvent from tests.factories import ( RequestFactory, UserFactory, @@ -14,6 +16,82 @@ from tests.factories import ( ) +def test_has_no_history(session): + owner = UserFactory.create() + user = UserFactory.create() + + workspace = Workspaces.create(RequestFactory.create(creator=owner)) + workspace_role = WorkspaceRoles.add(user, workspace.id, "developer") + create_event = ( + session.query(AuditEvent) + .filter( + AuditEvent.resource_id == workspace_role.id, AuditEvent.action == "create" + ) + .one() + ) + + assert not create_event.changed_state + + +def test_has_role_history(session): + owner = UserFactory.create() + user = UserFactory.create() + + workspace = Workspaces.create(RequestFactory.create(creator=owner)) + role = session.query(Role).filter(Role.name == "developer").one() + # in order to get the history, we don't want the WorkspaceRoleFactory + # to commit after create() + WorkspaceRoleFactory._meta.sqlalchemy_session_persistence = "flush" + workspace_role = WorkspaceRoleFactory.create( + workspace=workspace, user=user, role=role + ) + WorkspaceRoles.update_role(workspace_role, "admin") + changed_events = ( + session.query(AuditEvent) + .filter( + AuditEvent.resource_id == workspace_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_has_status_history(session): + owner = UserFactory.create() + user = UserFactory.create() + + workspace = Workspaces.create(RequestFactory.create(creator=owner)) + # in order to get the history, we don't want the WorkspaceRoleFactory + # to commit after create() + WorkspaceRoleFactory._meta.sqlalchemy_session_persistence = "flush" + workspace_role = WorkspaceRoleFactory.create(workspace=workspace, user=user) + WorkspaceRoles.enable(workspace_role) + changed_events = ( + session.query(AuditEvent) + .filter( + AuditEvent.resource_id == workspace_role.id, AuditEvent.action == "update" + ) + .all() + ) + + # changed_state["status"] returns a list [previous status, current status] + assert changed_events[0].changed_state["status"][0] == "pending" + assert changed_events[0].changed_state["status"][1] == "active" + + +def test_event_details(): + owner = UserFactory.create() + user = UserFactory.create() + + workspace = Workspaces.create(RequestFactory.create(creator=owner)) + workspace_role = WorkspaceRoles.add(user, workspace.id, "developer") + + assert workspace_role.event_details["updated_user_name"] == user.displayname + assert workspace_role.event_details["updated_user_id"] == str(user.id) + + def test_has_no_environment_roles(): owner = UserFactory.create() developer_data = {