From ddc2e2fad7960233a925fcb1be4195f822701ba6 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Wed, 19 Sep 2018 11:41:27 -0400 Subject: [PATCH 01/15] Automatic audit logging using SQLA events --- ...8cca588a1_add_view_audit_log_permission.py | 39 +++++++++++ .../875841fac207_add_audit_events_table.py | 44 +++++++++++++ atst/app.py | 14 ++-- atst/domain/audit_log.py | 53 +++++++++++++++ atst/domain/auth.py | 4 ++ atst/domain/authz.py | 9 +++ atst/models/__init__.py | 1 + atst/models/audit_event.py | 44 +++++++++++++ atst/models/environment.py | 4 +- atst/models/mixins/__init__.py | 2 + atst/models/mixins/auditable.py | 66 +++++++++++++++++++ .../{mixins.py => mixins/timestamps.py} | 0 atst/models/permissions.py | 1 + atst/models/project.py | 4 +- atst/models/request.py | 2 +- atst/models/request_review.py | 2 +- atst/models/request_revision.py | 4 +- atst/models/request_status_event.py | 2 +- atst/models/workspace.py | 7 +- atst/models/workspace_role.py | 2 +- atst/routes/__init__.py | 12 +++- templates/audit_log.html | 22 +++++++ templates/navigation/global_navigation.html | 4 ++ tests/conftest.py | 3 +- tests/domain/test_audit_log.py | 20 ++++++ tests/domain/test_projects.py | 5 +- tests/routes/test_financial_verification.py | 2 +- 27 files changed, 346 insertions(+), 26 deletions(-) create mode 100644 alembic/versions/7958cca588a1_add_view_audit_log_permission.py create mode 100644 alembic/versions/875841fac207_add_audit_events_table.py create mode 100644 atst/domain/audit_log.py create mode 100644 atst/models/audit_event.py create mode 100644 atst/models/mixins/__init__.py create mode 100644 atst/models/mixins/auditable.py rename atst/models/{mixins.py => mixins/timestamps.py} (100%) create mode 100644 templates/audit_log.html create mode 100644 tests/domain/test_audit_log.py diff --git a/alembic/versions/7958cca588a1_add_view_audit_log_permission.py b/alembic/versions/7958cca588a1_add_view_audit_log_permission.py new file mode 100644 index 00000000..9aa1a0ee --- /dev/null +++ b/alembic/versions/7958cca588a1_add_view_audit_log_permission.py @@ -0,0 +1,39 @@ +"""add view_audit_log permission + +Revision ID: 7958cca588a1 +Revises: 875841fac207 +Create Date: 2018-09-14 10:20:20.016575 + +""" +from alembic import op +from sqlalchemy.orm.session import Session + +from atst.models.role import Role +from atst.models.permissions import Permissions + + +# revision identifiers, used by Alembic. +revision = '7958cca588a1' +down_revision = '875841fac207' +branch_labels = None +depends_on = None + + +def upgrade(): + session = Session(bind=op.get_bind()) + admin_roles = session.query(Role).filter(Role.name.in_(["ccpo", "security_auditor"])).all() + for role in admin_roles: + role.add_permission(Permissions.VIEW_AUDIT_LOG) + session.add(role) + + session.commit() + + +def downgrade(): + session = Session(bind=op.get_bind()) + admin_roles = session.query(Role).filter(Role.name.in_(["ccpo", "security_auditor"])).all() + for role in admin_roles: + role.remove_permission(Permissions.VIEW_AUDIT_LOG) + session.add(role) + + session.commit() diff --git a/alembic/versions/875841fac207_add_audit_events_table.py b/alembic/versions/875841fac207_add_audit_events_table.py new file mode 100644 index 00000000..8341fc68 --- /dev/null +++ b/alembic/versions/875841fac207_add_audit_events_table.py @@ -0,0 +1,44 @@ +"""add audit_events table + +Revision ID: 875841fac207 +Revises: 2572be7fb7fc +Create Date: 2018-09-13 15:34:18.815205 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = '875841fac207' +down_revision = '359caaf8c5f1' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('audit_events', + 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('resource_name', sa.String(), nullable=False), + sa.Column('resource_id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('action', sa.String(), nullable=False), + sa.Column('workspace_id', postgresql.UUID(as_uuid=True), nullable=True), + sa.ForeignKeyConstraint(['user_id'], ['users.id']), + sa.ForeignKeyConstraint(['workspace_id'], ['workspaces.id']), + sa.PrimaryKeyConstraint('id') + ) + op.create_index(op.f('ix_audit_events_resource_id'), 'audit_events', ['resource_id'], unique=False) + op.create_index(op.f('ix_audit_events_user_id'), 'audit_events', ['user_id'], unique=False) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_index(op.f('ix_audit_events_user_id'), table_name='audit_events') + op.drop_index(op.f('ix_audit_events_resource_id'), table_name='audit_events') + op.drop_table('audit_events') + # ### end Alembic commands ### diff --git a/atst/app.py b/atst/app.py index c14b6a1c..c30baa88 100644 --- a/atst/app.py +++ b/atst/app.py @@ -18,6 +18,7 @@ from atst.routes.dev import bp as dev_routes from atst.routes.errors import make_error_pages from atst.domain.authnid.crl import CRLCache from atst.domain.auth import apply_authentication +from atst.domain.authz import Authorization from atst.eda_client import MockEDAClient from atst.uploader import Uploader @@ -69,13 +70,12 @@ def make_flask_callbacks(app): g.dev = os.getenv("FLASK_ENV", "dev") == "dev" g.matchesPath = lambda href: re.match("^" + href, request.path) g.modal = request.args.get("modal", None) - g.current_user = { - "id": "cce17030-4109-4719-b958-ed109dbb87c8", - "first_name": "Amanda", - "last_name": "Adamson", - "atat_role": "default", - "atat_permissions": [], - } + g.Authorization = Authorization + + @app.after_request + def _cleanup(response): + g.pop("current_user", None) + return response def map_config(config): diff --git a/atst/domain/audit_log.py b/atst/domain/audit_log.py new file mode 100644 index 00000000..6a656b07 --- /dev/null +++ b/atst/domain/audit_log.py @@ -0,0 +1,53 @@ +from atst.database import db +from atst.domain.common import Query +from atst.domain.authz import Authorization, Permissions +from atst.models.audit_event import AuditEvent + + +class AuditEventQuery(Query): + model = AuditEvent + + @classmethod + def get_all(cls): + return db.session.query(cls.model).order_by(cls.model.time_created.desc()).all() + + +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) + + @classmethod + def get_all_events(cls, user): + Authorization.check_atat_permission( + user, Permissions.VIEW_AUDIT_LOG, "view audit log" + ) + return AuditEventQuery.get_all() + + @classmethod + def _resource_name(cls, resource): + return type(resource).__name__.lower() + + @classmethod + def _log(cls, user=None, workspace_id=None, resource=None, action=None): + resource_id = resource.id if resource else None + resource_name = cls._resource_name(resource) if resource else None + + audit_event = AuditEventQuery.create( + user=user, + workspace_id=workspace_id, + resource_id=resource_id, + resource_name=resource_name, + action=action, + ) + return AuditEventQuery.add_and_commit(audit_event) diff --git a/atst/domain/auth.py b/atst/domain/auth.py index 904708d6..0d4844c9 100644 --- a/atst/domain/auth.py +++ b/atst/domain/auth.py @@ -34,6 +34,10 @@ def get_current_user(): else: return False +def logout(): + if session.get("user_id"): + del (session["user_id"]) + def _unprotected_route(request): if request.endpoint in UNPROTECTED_ROUTES: diff --git a/atst/domain/authz.py b/atst/domain/authz.py index cdfafdac..ae7e2b38 100644 --- a/atst/domain/authz.py +++ b/atst/domain/authz.py @@ -43,3 +43,12 @@ class Authorization(object): def check_workspace_permission(cls, user, workspace, permission, message): if not Authorization.has_workspace_permission(user, workspace, permission): raise UnauthorizedError(user, message) + + @classmethod + def check_atat_permission(cls, user, permission, message): + if not Authorization.has_atat_permission(user, permission): + raise UnauthorizedError(user, message) + + @classmethod + def can_view_audit_log(cls, user): + return Authorization.has_atat_permission(user, Permissions.VIEW_AUDIT_LOG) diff --git a/atst/models/__init__.py b/atst/models/__init__.py index 0b8a95df..91cee016 100644 --- a/atst/models/__init__.py +++ b/atst/models/__init__.py @@ -17,3 +17,4 @@ from .attachment import Attachment from .request_revision import RequestRevision from .request_review import RequestReview from .request_internal_comment import RequestInternalComment +from .audit_event import AuditEvent diff --git a/atst/models/audit_event.py b/atst/models/audit_event.py new file mode 100644 index 00000000..7ed50d5b --- /dev/null +++ b/atst/models/audit_event.py @@ -0,0 +1,44 @@ +from sqlalchemy import String, Column, ForeignKey, inspect +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 AuditEvent(Base, TimestampsMixin): + __tablename__ = "audit_events" + + id = types.Id() + + user_id = Column(UUID(as_uuid=True), ForeignKey("users.id"), index=True) + user = relationship("User", backref="audit_events") + + workspace_id = Column(UUID(as_uuid=True), ForeignKey("workspaces.id"), index=True) + workspace = relationship("Workspace", backref="audit_events") + + resource_name = Column(String(), nullable=False) + resource_id = Column(UUID(as_uuid=True), index=True, nullable=False) + action = Column(String(), nullable=False) + + def __str__(self): + full_action = "{} on {} {}".format( + self.action, self.resource_name, self.resource_id + ) + + if self.user and self.workspace: + return "{} performed {} in workspace {}".format( + self.user.full_name, full_action, self.workspace_id + ) + if self.user: + return "{} performed {}".format(self.user.full_name, full_action) + else: + return "ATAT System performed {}".format(full_action) + + def save(self, connection): + attrs = inspect(self).dict + + connection.execute( + self.__table__.insert(), + **attrs + ) diff --git a/atst/models/environment.py b/atst/models/environment.py index 3f65b67a..a8edeae1 100644 --- a/atst/models/environment.py +++ b/atst/models/environment.py @@ -3,10 +3,10 @@ from sqlalchemy.orm import relationship from atst.models import Base from atst.models.types import Id -from atst.models.mixins import TimestampsMixin +from atst.models import mixins -class Environment(Base, TimestampsMixin): +class Environment(Base, mixins.TimestampsMixin, mixins.AuditableMixin): __tablename__ = "environments" id = Id() diff --git a/atst/models/mixins/__init__.py b/atst/models/mixins/__init__.py new file mode 100644 index 00000000..fc0dc51f --- /dev/null +++ b/atst/models/mixins/__init__.py @@ -0,0 +1,2 @@ +from .timestamps import TimestampsMixin +from .auditable import AuditableMixin diff --git a/atst/models/mixins/auditable.py b/atst/models/mixins/auditable.py new file mode 100644 index 00000000..6079e6d5 --- /dev/null +++ b/atst/models/mixins/auditable.py @@ -0,0 +1,66 @@ +from sqlalchemy import event +from flask import g +import re + +from atst.models.audit_event import AuditEvent + +ACTION_CREATE = "create" +ACTION_UPDATE = "update" +ACTION_DELETE = "delete" + + +def getattr_path(obj, path, default=None): + _obj = obj + for item in path.split('.'): + _obj = getattr(_obj, item, default) + return _obj + + +def camel_to_snake(camel_cased): + s1 = re.sub('(.)([A-Z][a-z]+)', r'\1_\2', camel_cased) + return re.sub('([a-z0-9])([A-Z])', r'\1_\2', s1).lower() + + +class AuditableMixin(object): + + @staticmethod + def create_audit_event(connection, resource, action): + user_id = getattr_path(g, "current_user.id") + workspace_id = resource.auditable_workspace_id() + resource_name = resource.auditable_resource_name() + + audit_event = AuditEvent( + user_id=user_id, + workspace_id=workspace_id, + resource_name=resource_name, + resource_id=resource.id, + action=action, + ) + + audit_event.save(connection) + + @classmethod + def __declare_last__(cls): + event.listen(cls, 'after_insert', cls.audit_insert) + event.listen(cls, 'after_delete', cls.audit_delete) + event.listen(cls, 'after_update', cls.audit_update) + + @staticmethod + def audit_insert(mapper, connection, target): + """Listen for the `after_insert` event and create an AuditLog entry""" + target.create_audit_event(connection, target, ACTION_CREATE) + + @staticmethod + def audit_delete(mapper, connection, target): + """Listen for the `after_delete` event and create an AuditLog entry""" + target.create_audit_event(connection, target, ACTION_DELETE) + + @staticmethod + def audit_update(mapper, connection, target): + target.create_audit_event(connection, target, ACTION_UPDATE) + + def auditable_resource_name(self): + return camel_to_snake(type(self).__name__) + + def auditable_workspace_id(self): + return getattr_path(self, "workspace.id") diff --git a/atst/models/mixins.py b/atst/models/mixins/timestamps.py similarity index 100% rename from atst/models/mixins.py rename to atst/models/mixins/timestamps.py diff --git a/atst/models/permissions.py b/atst/models/permissions.py index 3888fe2b..f7adc406 100644 --- a/atst/models/permissions.py +++ b/atst/models/permissions.py @@ -1,4 +1,5 @@ class Permissions(object): + VIEW_AUDIT_LOG = "view_audit_log" REQUEST_JEDI_WORKSPACE = "request_jedi_workspace" VIEW_ORIGINAL_JEDI_REQEUST = "view_original_jedi_request" REVIEW_AND_APPROVE_JEDI_WORKSPACE_REQUEST = ( diff --git a/atst/models/project.py b/atst/models/project.py index 46562e82..c276b240 100644 --- a/atst/models/project.py +++ b/atst/models/project.py @@ -3,10 +3,10 @@ from sqlalchemy.orm import relationship from atst.models import Base from atst.models.types import Id -from atst.models.mixins import TimestampsMixin +from atst.models import mixins -class Project(Base, TimestampsMixin): +class Project(Base, mixins.TimestampsMixin, mixins.AuditableMixin): __tablename__ = "projects" id = Id() diff --git a/atst/models/request.py b/atst/models/request.py index dff1db00..ddb84762 100644 --- a/atst/models/request.py +++ b/atst/models/request.py @@ -24,7 +24,7 @@ def update_dict_with_properties(instance, body, top_level_key, properties): return body -class Request(Base, mixins.TimestampsMixin): +class Request(Base, mixins.TimestampsMixin, mixins.AuditableMixin): __tablename__ = "requests" id = types.Id() diff --git a/atst/models/request_review.py b/atst/models/request_review.py index c38832f7..a69d62d2 100644 --- a/atst/models/request_review.py +++ b/atst/models/request_review.py @@ -4,7 +4,7 @@ from sqlalchemy.orm import relationship from atst.models import Base, mixins, types -class RequestReview(Base, mixins.TimestampsMixin): +class RequestReview(Base, mixins.TimestampsMixin, mixins.AuditableMixin): __tablename__ = "request_reviews" id = types.Id() diff --git a/atst/models/request_revision.py b/atst/models/request_revision.py index 2cfee3c4..be333eea 100644 --- a/atst/models/request_revision.py +++ b/atst/models/request_revision.py @@ -12,11 +12,11 @@ from sqlalchemy.orm import relationship from sqlalchemy.dialects.postgresql import ARRAY from atst.models import Base -from atst.models.mixins import TimestampsMixin +from atst.models import mixins from atst.models.types import Id -class RequestRevision(Base, TimestampsMixin): +class RequestRevision(Base, mixins.TimestampsMixin, mixins.AuditableMixin): __tablename__ = "request_revisions" id = Id() diff --git a/atst/models/request_status_event.py b/atst/models/request_status_event.py index 8b56dfeb..68b60729 100644 --- a/atst/models/request_status_event.py +++ b/atst/models/request_status_event.py @@ -22,7 +22,7 @@ class RequestStatus(Enum): DELETED = "Deleted" -class RequestStatusEvent(Base, mixins.TimestampsMixin): +class RequestStatusEvent(Base, mixins.TimestampsMixin, mixins.AuditableMixin): __tablename__ = "request_status_events" id = Id() diff --git a/atst/models/workspace.py b/atst/models/workspace.py index 1a3a13e0..6ae938b2 100644 --- a/atst/models/workspace.py +++ b/atst/models/workspace.py @@ -3,12 +3,12 @@ from sqlalchemy.orm import relationship from atst.models import Base from atst.models.types import Id -from atst.models.mixins import TimestampsMixin +from atst.models import mixins from atst.models.workspace_user import WorkspaceUser from atst.utils import first_or_none -class Workspace(Base, TimestampsMixin): +class Workspace(Base, mixins.TimestampsMixin, mixins.AuditableMixin): __tablename__ = "workspaces" id = Id() @@ -40,3 +40,6 @@ class Workspace(Base, TimestampsMixin): @property def members(self): return [WorkspaceUser(role.user, role) for role in self.roles] + + def auditable_workspace_id(self): + return self.id diff --git a/atst/models/workspace_role.py b/atst/models/workspace_role.py index da6b0ea4..e95ce350 100644 --- a/atst/models/workspace_role.py +++ b/atst/models/workspace_role.py @@ -6,7 +6,7 @@ from atst.models import Base, mixins from .types import Id -class WorkspaceRole(Base, mixins.TimestampsMixin): +class WorkspaceRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): __tablename__ = "workspace_roles" id = Id() diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index 32995091..c2790196 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -5,6 +5,8 @@ import pendulum from atst.domain.requests import Requests from atst.domain.users import Users from atst.domain.authnid import AuthenticationContext +from atst.domain.audit_log import AuditLog +from atst.domain.auth import logout as _logout bp = Blueprint("atst", __name__) @@ -79,7 +81,11 @@ def login_redirect(): @bp.route("/logout") def logout(): - if session.get("user_id"): - del (session["user_id"]) - + _logout() return redirect(url_for(".home")) + + +@bp.route("/activity-history") +def activity_history(): + audit_events = AuditLog.get_all_events(g.current_user) + return render_template("audit_log.html", audit_events=audit_events) diff --git a/templates/audit_log.html b/templates/audit_log.html new file mode 100644 index 00000000..68b54f80 --- /dev/null +++ b/templates/audit_log.html @@ -0,0 +1,22 @@ +{% extends "base.html" %} + +{% block content %} + + +
+
+
+

Acitivity History

+
+ +
    + {% for event in audit_events %} +
  • {{ event }}
    • + {% endfor %} +
+ +
+ +
+ +{% endblock %} diff --git a/templates/navigation/global_navigation.html b/templates/navigation/global_navigation.html index 5314042e..51c81571 100644 --- a/templates/navigation/global_navigation.html +++ b/templates/navigation/global_navigation.html @@ -25,5 +25,9 @@ {% if g.current_user.has_workspaces %} {{ SidenavItem("Workspaces", href="/workspaces", icon="cloud", active=g.matchesPath('/workspaces')) }} {% endif %} + + {% if g.Authorization.can_view_audit_log(g.current_user) %} + {{ SidenavItem("Activity History", url_for('atst.activity_history'), icon="document", active=g.matchesPath('/activity-history')) }} + {% endif %} diff --git a/tests/conftest.py b/tests/conftest.py index 14539df6..947f054d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,6 +9,7 @@ from tempfile import TemporaryDirectory from atst.app import make_app, make_config from atst.database import db as _db +from atst.domain.auth import logout import tests.factories as factories from tests.mocks import PDF_FILENAME @@ -108,7 +109,7 @@ def user_session(monkeypatch, session): def set_user_session(user=None): monkeypatch.setattr( "atst.domain.auth.get_current_user", - lambda *args: user or factories.UserFactory.build(), + lambda *args: user or factories.UserFactory.create(), ) return set_user_session diff --git a/tests/domain/test_audit_log.py b/tests/domain/test_audit_log.py new file mode 100644 index 00000000..606e0fd7 --- /dev/null +++ b/tests/domain/test_audit_log.py @@ -0,0 +1,20 @@ +import pytest + +from atst.domain.audit_log import AuditLog +from atst.domain.exceptions import UnauthorizedError +from tests.factories import UserFactory + + +@pytest.fixture(scope="function") +def ccpo(): + return UserFactory.from_atat_role("ccpo") + + +@pytest.fixture(scope="function") +def developer(): + return UserFactory.from_atat_role("default") + + +def test_non_admin_cannot_view_audit_log(developer): + with pytest.raises(UnauthorizedError): + AuditLog.get_all_events(developer) diff --git a/tests/domain/test_projects.py b/tests/domain/test_projects.py index e09ab666..6a75a792 100644 --- a/tests/domain/test_projects.py +++ b/tests/domain/test_projects.py @@ -1,10 +1,11 @@ from atst.domain.projects import Projects -from atst.domain.workspaces import Workspaces from tests.factories import RequestFactory +from atst.domain.workspaces import Workspaces def test_create_project_with_multiple_environments(): - workspace = Workspaces.create(RequestFactory.create()) + request = RequestFactory.create() + workspace = Workspaces.create(request) project = Projects.create( workspace.owner, workspace, "My Test Project", "Test", ["dev", "prod"] ) diff --git a/tests/routes/test_financial_verification.py b/tests/routes/test_financial_verification.py index fe601c35..aaa9b54d 100644 --- a/tests/routes/test_financial_verification.py +++ b/tests/routes/test_financial_verification.py @@ -133,7 +133,7 @@ class TestPENumberInForm: request = RequestFactory.create(creator=user) monkeypatch.setattr("atst.domain.requests.Requests.get", lambda *args: request) monkeypatch.setattr("atst.forms.financial.validate_pe_id", lambda *args: True) - user_session() + user_session(user) data = {**self.required_data, **extended_financial_verification_data} data["task_order_number"] = "1234567" From 112f85a10e8018f1768d0e03b3c8b6f541017070 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Thu, 20 Sep 2018 15:17:42 -0400 Subject: [PATCH 02/15] Add audit log permission to CCPO --- script/seed_roles.py | 1 + 1 file changed, 1 insertion(+) diff --git a/script/seed_roles.py b/script/seed_roles.py index f84ff356..858e0419 100644 --- a/script/seed_roles.py +++ b/script/seed_roles.py @@ -48,6 +48,7 @@ roles = [ Permissions.RENAME_ENVIRONMENT_IN_APPLICATION, Permissions.ADD_TAG_TO_WORKSPACE, Permissions.REMOVE_TAG_FROM_WORKSPACE, + Permissions.VIEW_AUDIT_LOG ], ), Role( From 5a4cbb56bb56df85df09a22c8bcba61c9cf2dc30 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Fri, 21 Sep 2018 11:35:32 -0400 Subject: [PATCH 03/15] Formatting --- atst/domain/auth.py | 1 + atst/models/audit_event.py | 5 +---- atst/models/mixins/auditable.py | 13 ++++++------- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/atst/domain/auth.py b/atst/domain/auth.py index 0d4844c9..74c6d2d8 100644 --- a/atst/domain/auth.py +++ b/atst/domain/auth.py @@ -34,6 +34,7 @@ def get_current_user(): else: return False + def logout(): if session.get("user_id"): del (session["user_id"]) diff --git a/atst/models/audit_event.py b/atst/models/audit_event.py index 7ed50d5b..52a88cf6 100644 --- a/atst/models/audit_event.py +++ b/atst/models/audit_event.py @@ -38,7 +38,4 @@ class AuditEvent(Base, TimestampsMixin): def save(self, connection): attrs = inspect(self).dict - connection.execute( - self.__table__.insert(), - **attrs - ) + connection.execute(self.__table__.insert(), **attrs) diff --git a/atst/models/mixins/auditable.py b/atst/models/mixins/auditable.py index 6079e6d5..0758fa10 100644 --- a/atst/models/mixins/auditable.py +++ b/atst/models/mixins/auditable.py @@ -11,18 +11,17 @@ ACTION_DELETE = "delete" def getattr_path(obj, path, default=None): _obj = obj - for item in path.split('.'): + for item in path.split("."): _obj = getattr(_obj, item, default) return _obj def camel_to_snake(camel_cased): - s1 = re.sub('(.)([A-Z][a-z]+)', r'\1_\2', camel_cased) - return re.sub('([a-z0-9])([A-Z])', r'\1_\2', s1).lower() + s1 = re.sub("(.)([A-Z][a-z]+)", r"\1_\2", camel_cased) + return re.sub("([a-z0-9])([A-Z])", r"\1_\2", s1).lower() class AuditableMixin(object): - @staticmethod def create_audit_event(connection, resource, action): user_id = getattr_path(g, "current_user.id") @@ -41,9 +40,9 @@ class AuditableMixin(object): @classmethod def __declare_last__(cls): - event.listen(cls, 'after_insert', cls.audit_insert) - event.listen(cls, 'after_delete', cls.audit_delete) - event.listen(cls, 'after_update', cls.audit_update) + event.listen(cls, "after_insert", cls.audit_insert) + event.listen(cls, "after_delete", cls.audit_delete) + event.listen(cls, "after_update", cls.audit_update) @staticmethod def audit_insert(mapper, connection, target): From 22f02f604fc239d531d36e72338d39965d26232c Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 24 Sep 2018 10:47:50 -0400 Subject: [PATCH 04/15] Add test which ensures that CCPO can view the audit log --- tests/domain/test_audit_log.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/domain/test_audit_log.py b/tests/domain/test_audit_log.py index 606e0fd7..1a2d95f7 100644 --- a/tests/domain/test_audit_log.py +++ b/tests/domain/test_audit_log.py @@ -18,3 +18,7 @@ def developer(): def test_non_admin_cannot_view_audit_log(developer): with pytest.raises(UnauthorizedError): AuditLog.get_all_events(developer) + + +def test_ccpo_can_iview_audit_log(ccpo): + AuditLog.get_all_events(ccpo) From 870929afd6cfffd1a4bbb7bab2c92382a2dba8df Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 24 Sep 2018 10:51:49 -0400 Subject: [PATCH 05/15] Rename resource_name to resource_type --- alembic/versions/875841fac207_add_audit_events_table.py | 2 +- atst/domain/audit_log.py | 6 +++--- atst/models/audit_event.py | 4 ++-- atst/models/mixins/auditable.py | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/alembic/versions/875841fac207_add_audit_events_table.py b/alembic/versions/875841fac207_add_audit_events_table.py index 8341fc68..eecbbd29 100644 --- a/alembic/versions/875841fac207_add_audit_events_table.py +++ b/alembic/versions/875841fac207_add_audit_events_table.py @@ -23,7 +23,7 @@ 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('resource_name', sa.String(), nullable=False), + sa.Column('resource_type', sa.String(), nullable=False), sa.Column('resource_id', postgresql.UUID(as_uuid=True), nullable=False), sa.Column('action', sa.String(), nullable=False), sa.Column('workspace_id', postgresql.UUID(as_uuid=True), nullable=True), diff --git a/atst/domain/audit_log.py b/atst/domain/audit_log.py index 6a656b07..238cff2e 100644 --- a/atst/domain/audit_log.py +++ b/atst/domain/audit_log.py @@ -35,19 +35,19 @@ class AuditLog(object): return AuditEventQuery.get_all() @classmethod - def _resource_name(cls, resource): + def _resource_type(cls, resource): return type(resource).__name__.lower() @classmethod def _log(cls, user=None, workspace_id=None, resource=None, action=None): resource_id = resource.id if resource else None - resource_name = cls._resource_name(resource) if resource else None + resource_type = cls._resource_type(resource) if resource else None audit_event = AuditEventQuery.create( user=user, workspace_id=workspace_id, resource_id=resource_id, - resource_name=resource_name, + resource_type=resource_type, action=action, ) return AuditEventQuery.add_and_commit(audit_event) diff --git a/atst/models/audit_event.py b/atst/models/audit_event.py index 52a88cf6..288704a7 100644 --- a/atst/models/audit_event.py +++ b/atst/models/audit_event.py @@ -17,13 +17,13 @@ class AuditEvent(Base, TimestampsMixin): workspace_id = Column(UUID(as_uuid=True), ForeignKey("workspaces.id"), index=True) workspace = relationship("Workspace", backref="audit_events") - resource_name = Column(String(), nullable=False) + resource_type = Column(String(), nullable=False) resource_id = Column(UUID(as_uuid=True), index=True, nullable=False) action = Column(String(), nullable=False) def __str__(self): full_action = "{} on {} {}".format( - self.action, self.resource_name, self.resource_id + self.action, self.resource_type, self.resource_id ) if self.user and self.workspace: diff --git a/atst/models/mixins/auditable.py b/atst/models/mixins/auditable.py index 0758fa10..285bd17c 100644 --- a/atst/models/mixins/auditable.py +++ b/atst/models/mixins/auditable.py @@ -26,12 +26,12 @@ class AuditableMixin(object): def create_audit_event(connection, resource, action): user_id = getattr_path(g, "current_user.id") workspace_id = resource.auditable_workspace_id() - resource_name = resource.auditable_resource_name() + resource_type = resource.auditable_resource_type() audit_event = AuditEvent( user_id=user_id, workspace_id=workspace_id, - resource_name=resource_name, + resource_type=resource_type, resource_id=resource.id, action=action, ) @@ -58,7 +58,7 @@ class AuditableMixin(object): def audit_update(mapper, connection, target): target.create_audit_event(connection, target, ACTION_UPDATE) - def auditable_resource_name(self): + def auditable_resource_type(self): return camel_to_snake(type(self).__name__) def auditable_workspace_id(self): From 85d866956b12b20215e4423bf283282805c0f7d4 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 24 Sep 2018 11:41:19 -0400 Subject: [PATCH 06/15] Add display_name property to AuditEvent --- .../875841fac207_add_audit_events_table.py | 1 + atst/models/audit_event.py | 16 +++++++--------- atst/models/mixins/auditable.py | 5 +++++ 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/alembic/versions/875841fac207_add_audit_events_table.py b/alembic/versions/875841fac207_add_audit_events_table.py index eecbbd29..2b573a9e 100644 --- a/alembic/versions/875841fac207_add_audit_events_table.py +++ b/alembic/versions/875841fac207_add_audit_events_table.py @@ -24,6 +24,7 @@ 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('resource_type', sa.String(), nullable=False), + sa.Column('display_name', sa.String(), nullable=True), sa.Column('resource_id', postgresql.UUID(as_uuid=True), nullable=False), sa.Column('action', sa.String(), nullable=False), sa.Column('workspace_id', postgresql.UUID(as_uuid=True), nullable=True), diff --git a/atst/models/audit_event.py b/atst/models/audit_event.py index 288704a7..3d6c9a00 100644 --- a/atst/models/audit_event.py +++ b/atst/models/audit_event.py @@ -19,21 +19,19 @@ class AuditEvent(Base, TimestampsMixin): resource_type = Column(String(), nullable=False) resource_id = Column(UUID(as_uuid=True), index=True, nullable=False) + display_name = Column(String()) action = Column(String(), nullable=False) def __str__(self): - full_action = "{} on {} {}".format( + + user_str = "{} performed".format(self.user.full_name) if self.user else "ATAT System" + action_str = "{} on {} {}".format( self.action, self.resource_type, self.resource_id ) + display_name_str = "({})".format(self.display_name) if self.display_name else "" + workspace_str = "in workspace {}".format(self.workspace_id) if self.workspace_id else "" - if self.user and self.workspace: - return "{} performed {} in workspace {}".format( - self.user.full_name, full_action, self.workspace_id - ) - if self.user: - return "{} performed {}".format(self.user.full_name, full_action) - else: - return "ATAT System performed {}".format(full_action) + return " ".join([user_str, action_str, display_name_str, workspace_str]) def save(self, connection): attrs = inspect(self).dict diff --git a/atst/models/mixins/auditable.py b/atst/models/mixins/auditable.py index 285bd17c..89645186 100644 --- a/atst/models/mixins/auditable.py +++ b/atst/models/mixins/auditable.py @@ -27,12 +27,14 @@ class AuditableMixin(object): user_id = getattr_path(g, "current_user.id") workspace_id = resource.auditable_workspace_id() resource_type = resource.auditable_resource_type() + display_name = resource.auditable_displayname() audit_event = AuditEvent( user_id=user_id, workspace_id=workspace_id, resource_type=resource_type, resource_id=resource.id, + display_name=display_name, action=action, ) @@ -63,3 +65,6 @@ class AuditableMixin(object): def auditable_workspace_id(self): return getattr_path(self, "workspace.id") + + def auditable_displayname(self): + return getattr_path(self, "displayname") From 0c8d303bdd077d985e55fdfabbf81c8bb80378fa Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 24 Sep 2018 11:41:53 -0400 Subject: [PATCH 07/15] Faster (no join) default impl. of auditable_workspace_id --- atst/models/mixins/auditable.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atst/models/mixins/auditable.py b/atst/models/mixins/auditable.py index 89645186..e0fb3710 100644 --- a/atst/models/mixins/auditable.py +++ b/atst/models/mixins/auditable.py @@ -64,7 +64,7 @@ class AuditableMixin(object): return camel_to_snake(type(self).__name__) def auditable_workspace_id(self): - return getattr_path(self, "workspace.id") + return getattr_path(self, "workspace_id") def auditable_displayname(self): return getattr_path(self, "displayname") From 004985057c408b25c413e906a99602a24bb2eb5d Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 24 Sep 2018 11:42:30 -0400 Subject: [PATCH 08/15] auditable_workspace_id for Environment --- atst/models/environment.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/atst/models/environment.py b/atst/models/environment.py index a8edeae1..2d56f9da 100644 --- a/atst/models/environment.py +++ b/atst/models/environment.py @@ -22,3 +22,6 @@ class Environment(Base, mixins.TimestampsMixin, mixins.AuditableMixin): @property def num_users(self): return len(self.users) + + def auditable_workspace_id(self): + return self.project.workspace_id From b2c59f39ef6dc939b22394bfc427822171a99442 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 24 Sep 2018 11:56:50 -0400 Subject: [PATCH 09/15] Only display "in workspace x" resource isn't a workspace --- atst/models/audit_event.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/atst/models/audit_event.py b/atst/models/audit_event.py index 3d6c9a00..d95da99c 100644 --- a/atst/models/audit_event.py +++ b/atst/models/audit_event.py @@ -24,12 +24,18 @@ class AuditEvent(Base, TimestampsMixin): def __str__(self): - user_str = "{} performed".format(self.user.full_name) if self.user else "ATAT System" + user_str = ( + "{} performed".format(self.user.full_name) if self.user else "ATAT System" + ) action_str = "{} on {} {}".format( self.action, self.resource_type, self.resource_id ) display_name_str = "({})".format(self.display_name) if self.display_name else "" - workspace_str = "in workspace {}".format(self.workspace_id) if self.workspace_id else "" + workspace_str = ( + "in workspace {}".format(self.workspace_id) + if self.workspace_id and self.resource_type != "workspace" + else "" + ) return " ".join([user_str, action_str, display_name_str, workspace_str]) From 5e479feadc93334456c5b12d26e449967de901c0 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 24 Sep 2018 11:58:41 -0400 Subject: [PATCH 10/15] Implement Workspace.displayname --- atst/models/workspace.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/atst/models/workspace.py b/atst/models/workspace.py index 6ae938b2..d4f67491 100644 --- a/atst/models/workspace.py +++ b/atst/models/workspace.py @@ -41,5 +41,9 @@ class Workspace(Base, mixins.TimestampsMixin, mixins.AuditableMixin): def members(self): return [WorkspaceUser(role.user, role) for role in self.roles] + @property + def displayname(self): + return self.name + def auditable_workspace_id(self): return self.id From 4a5b9967ef70efe515c7f951eba3138c5469cd47 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 24 Sep 2018 13:35:49 -0400 Subject: [PATCH 11/15] Store request_id when available --- .../875841fac207_add_audit_events_table.py | 1 + atst/models/audit_event.py | 15 +++++++++------ atst/models/mixins/auditable.py | 5 +++++ 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/alembic/versions/875841fac207_add_audit_events_table.py b/alembic/versions/875841fac207_add_audit_events_table.py index 2b573a9e..56266c18 100644 --- a/alembic/versions/875841fac207_add_audit_events_table.py +++ b/alembic/versions/875841fac207_add_audit_events_table.py @@ -28,6 +28,7 @@ def upgrade(): sa.Column('resource_id', postgresql.UUID(as_uuid=True), nullable=False), sa.Column('action', sa.String(), nullable=False), sa.Column('workspace_id', postgresql.UUID(as_uuid=True), nullable=True), + sa.Column('request_id', postgresql.UUID(as_uuid=True), nullable=True), sa.ForeignKeyConstraint(['user_id'], ['users.id']), sa.ForeignKeyConstraint(['workspace_id'], ['workspaces.id']), sa.PrimaryKeyConstraint('id') diff --git a/atst/models/audit_event.py b/atst/models/audit_event.py index d95da99c..6b36b486 100644 --- a/atst/models/audit_event.py +++ b/atst/models/audit_event.py @@ -17,6 +17,8 @@ class AuditEvent(Base, TimestampsMixin): workspace_id = Column(UUID(as_uuid=True), ForeignKey("workspaces.id"), index=True) workspace = relationship("Workspace", backref="audit_events") + request_id = Column(UUID(as_uuid=True), ForeignKey("request.id"), index=True) + resource_type = Column(String(), nullable=False) resource_id = Column(UUID(as_uuid=True), index=True, nullable=False) display_name = Column(String()) @@ -31,13 +33,14 @@ class AuditEvent(Base, TimestampsMixin): self.action, self.resource_type, self.resource_id ) display_name_str = "({})".format(self.display_name) if self.display_name else "" - workspace_str = ( - "in workspace {}".format(self.workspace_id) - if self.workspace_id and self.resource_type != "workspace" - else "" - ) - return " ".join([user_str, action_str, display_name_str, workspace_str]) + scope_str = "" + if self.request_id and self.resource_type != "request": + scope_str = "for request {}".format(self.request_id) + elif self.workspace_id and self.resource_type != "workspace": + scope_str = "in workspace {}".format(self.request_id) + + return " ".join([user_str, action_str, display_name_str, scope_str]) def save(self, connection): attrs = inspect(self).dict diff --git a/atst/models/mixins/auditable.py b/atst/models/mixins/auditable.py index e0fb3710..2a47d1c9 100644 --- a/atst/models/mixins/auditable.py +++ b/atst/models/mixins/auditable.py @@ -26,12 +26,14 @@ class AuditableMixin(object): def create_audit_event(connection, resource, action): user_id = getattr_path(g, "current_user.id") workspace_id = resource.auditable_workspace_id() + request_id = resource.auditable_request_id() resource_type = resource.auditable_resource_type() display_name = resource.auditable_displayname() audit_event = AuditEvent( user_id=user_id, workspace_id=workspace_id, + request_id=request_id, resource_type=resource_type, resource_id=resource.id, display_name=display_name, @@ -66,5 +68,8 @@ class AuditableMixin(object): def auditable_workspace_id(self): return getattr_path(self, "workspace_id") + def auditable_request_id(self): + return getattr_path(self, "request_id") + def auditable_displayname(self): return getattr_path(self, "displayname") From 5aabf9231693cf979edf65245689f3a1a3a96cb5 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 25 Sep 2018 11:08:42 -0400 Subject: [PATCH 12/15] Rebase and fix migration path --- alembic/versions/875841fac207_add_audit_events_table.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/alembic/versions/875841fac207_add_audit_events_table.py b/alembic/versions/875841fac207_add_audit_events_table.py index 56266c18..bccaa8b4 100644 --- a/alembic/versions/875841fac207_add_audit_events_table.py +++ b/alembic/versions/875841fac207_add_audit_events_table.py @@ -11,7 +11,7 @@ from sqlalchemy.dialects import postgresql # revision identifiers, used by Alembic. revision = '875841fac207' -down_revision = '359caaf8c5f1' +down_revision = 'c19ae79d2521' branch_labels = None depends_on = None From ac3d070178c8394dcbca5aa249a49061d00b6762 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 25 Sep 2018 11:18:03 -0400 Subject: [PATCH 13/15] Use workspace_id when we have it --- atst/models/audit_event.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atst/models/audit_event.py b/atst/models/audit_event.py index 6b36b486..e402e631 100644 --- a/atst/models/audit_event.py +++ b/atst/models/audit_event.py @@ -38,7 +38,7 @@ class AuditEvent(Base, TimestampsMixin): if self.request_id and self.resource_type != "request": scope_str = "for request {}".format(self.request_id) elif self.workspace_id and self.resource_type != "workspace": - scope_str = "in workspace {}".format(self.request_id) + scope_str = "in workspace {}".format(self.workspace_id) return " ".join([user_str, action_str, display_name_str, scope_str]) From 0aeca336b7089ae3d256c1b2a779b6beebbe7088 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 25 Sep 2018 11:18:22 -0400 Subject: [PATCH 14/15] Add displayname to Project and Environment --- atst/models/environment.py | 4 ++++ atst/models/project.py | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/atst/models/environment.py b/atst/models/environment.py index 2d56f9da..61b5c910 100644 --- a/atst/models/environment.py +++ b/atst/models/environment.py @@ -23,5 +23,9 @@ class Environment(Base, mixins.TimestampsMixin, mixins.AuditableMixin): def num_users(self): return len(self.users) + @property + def displayname(self): + return self.name + def auditable_workspace_id(self): return self.project.workspace_id diff --git a/atst/models/project.py b/atst/models/project.py index c276b240..cd9ab019 100644 --- a/atst/models/project.py +++ b/atst/models/project.py @@ -16,3 +16,8 @@ class Project(Base, mixins.TimestampsMixin, mixins.AuditableMixin): workspace_id = Column(ForeignKey("workspaces.id"), nullable=False) workspace = relationship("Workspace") environments = relationship("Environment", back_populates="project") + + + @property + def displayname(self): + return self.name From 141cbedff569143aa8618cdc6513e3647fc2c07b Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 25 Sep 2018 11:20:50 -0400 Subject: [PATCH 15/15] Shuffle commits around to fix audit log ordering --- atst/domain/environments.py | 1 - atst/domain/projects.py | 7 +++++-- atst/models/project.py | 1 - 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/atst/domain/environments.py b/atst/domain/environments.py index eebc6c15..74976544 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -24,7 +24,6 @@ class Environments(object): for name in names: environment = Environment(project=project, name=name) db.session.add(environment) - db.session.commit() @classmethod def add_member(cls, environment, user, role): diff --git a/atst/domain/projects.py b/atst/domain/projects.py index 36c1168d..663a12f4 100644 --- a/atst/domain/projects.py +++ b/atst/domain/projects.py @@ -12,11 +12,14 @@ class Projects(object): @classmethod def create(cls, user, workspace, name, description, environment_names): project = Project(workspace=workspace, name=name, description=description) + db.session.add(project) + Environments.create_many(project, environment_names) - db.session.add(project) - db.session.commit() + for environment in project.environments: + Environments.add_member(user, environment, user) + db.session.commit() return project @classmethod diff --git a/atst/models/project.py b/atst/models/project.py index cd9ab019..4d795fea 100644 --- a/atst/models/project.py +++ b/atst/models/project.py @@ -17,7 +17,6 @@ class Project(Base, mixins.TimestampsMixin, mixins.AuditableMixin): workspace = relationship("Workspace") environments = relationship("Environment", back_populates="project") - @property def displayname(self): return self.name