From b3ecd1658c835720374c31bfc4caf13e9b12f688 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 15 May 2019 18:14:26 -0400 Subject: [PATCH 1/8] Add application_id column to AuditEvent Use application_id and portfolio_id if the resource is a portfolio in AuditableMixin Clean up some residual references to workspace --- ...a1826be_add_appliction_id_to_auditevent.py | 32 +++++++++++++++++++ atst/domain/audit_log.py | 12 +++---- atst/models/audit_event.py | 7 ++++ atst/models/mixins/auditable.py | 16 +++++++++- tests/domain/test_audit_log.py | 27 ++++++++++++++-- 5 files changed, 83 insertions(+), 11 deletions(-) create mode 100644 alembic/versions/c5deba1826be_add_appliction_id_to_auditevent.py diff --git a/alembic/versions/c5deba1826be_add_appliction_id_to_auditevent.py b/alembic/versions/c5deba1826be_add_appliction_id_to_auditevent.py new file mode 100644 index 00000000..0dcd9f08 --- /dev/null +++ b/alembic/versions/c5deba1826be_add_appliction_id_to_auditevent.py @@ -0,0 +1,32 @@ +"""add_appliction_id_to_auditevent + +Revision ID: c5deba1826be +Revises: 404bb5bb3a0e +Create Date: 2019-05-15 16:30:27.981456 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = 'c5deba1826be' +down_revision = '404bb5bb3a0e' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('audit_events', sa.Column('application_id', postgresql.UUID(as_uuid=True), nullable=True)) + op.create_index(op.f('ix_audit_events_application_id'), 'audit_events', ['application_id'], unique=False) + op.create_foreign_key('audit_events_application_application_id', 'audit_events', 'applications', ['application_id'], ['id']) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint('audit_events_application_application_id', 'audit_events', type_='foreignkey') + op.drop_index(op.f('ix_audit_events_application_id'), table_name='audit_events') + op.drop_column('audit_events', 'application_id') + # ### end Alembic commands ### diff --git a/atst/domain/audit_log.py b/atst/domain/audit_log.py index 8fa6e517..cb7354db 100644 --- a/atst/domain/audit_log.py +++ b/atst/domain/audit_log.py @@ -14,15 +14,10 @@ class AuditEventQuery(Query): return cls.paginate(query, pagination_opts) @classmethod - def get_ws_events(cls, portfolio_id, pagination_opts): + def get_portfolio_events(cls, portfolio_id, pagination_opts): query = ( db.session.query(cls.model) - .filter( - or_( - cls.model.portfolio_id == portfolio_id, - cls.model.resource_id == portfolio_id, - ) - ) + .filter(cls.model.portfolio_id == portfolio_id) .order_by(cls.model.time_created.desc()) ) return cls.paginate(query, pagination_opts) @@ -39,7 +34,8 @@ class AuditLog(object): @classmethod def get_portfolio_events(cls, portfolio, pagination_opts=None): - return AuditEventQuery.get_ws_events(portfolio.id, pagination_opts) + return AuditEventQuery.get_portfolio_events(portfolio.id, pagination_opts) + @classmethod def get_by_resource(cls, resource_id): diff --git a/atst/models/audit_event.py b/atst/models/audit_event.py index 2d8d4ea1..90c69ee1 100644 --- a/atst/models/audit_event.py +++ b/atst/models/audit_event.py @@ -17,11 +17,17 @@ class AuditEvent(Base, TimestampsMixin): portfolio_id = Column(UUID(as_uuid=True), ForeignKey("portfolios.id"), index=True) portfolio = relationship("Portfolio", backref="audit_events") + application_id = Column( + UUID(as_uuid=True), ForeignKey("applications.id"), index=True + ) + application = relationship("Application", 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()) action = Column(String(), nullable=False) @@ -29,6 +35,7 @@ class AuditEvent(Base, TimestampsMixin): def log(self): return { "portfolio_id": str(self.portfolio_id), + "application_id": str(self.application_id), "changed_state": self.changed_state, "event_details": self.event_details, "resource_type": self.resource_type, diff --git a/atst/models/mixins/auditable.py b/atst/models/mixins/auditable.py index 091dc411..70c462a2 100644 --- a/atst/models/mixins/auditable.py +++ b/atst/models/mixins/auditable.py @@ -13,17 +13,27 @@ class AuditableMixin(object): @staticmethod def create_audit_event(connection, resource, action, changed_state=None): user_id = getattr_path(g, "current_user.id") - portfolio_id = resource.portfolio_id resource_type = resource.resource_type display_name = resource.displayname event_details = resource.event_details + if resource_type == "portfolio": + portfolio_id = resource.id + else: + portfolio_id = resource.portfolio_id + + if resource_type == "application": + application_id = resource.id + else: + application_id = resource.application_id + if changed_state is None: changed_state = resource.history if action == ACTION_UPDATE else None audit_event = AuditEvent( user_id=user_id, portfolio_id=portfolio_id, + application_id=application_id, resource_type=resource_type, resource_id=resource.id, display_name=display_name, @@ -95,6 +105,10 @@ class AuditableMixin(object): def portfolio_id(self): return None + @property + def application_id(self): + return None + @property def displayname(self): return None diff --git a/tests/domain/test_audit_log.py b/tests/domain/test_audit_log.py index af4f76e0..687b2395 100644 --- a/tests/domain/test_audit_log.py +++ b/tests/domain/test_audit_log.py @@ -1,5 +1,6 @@ import pytest +from atst.domain.applications import Applications from atst.domain.audit_log import AuditLog from atst.domain.exceptions import UnauthorizedError from atst.domain.permission_sets import PermissionSets @@ -45,7 +46,7 @@ def test_paginate_ws_audit_log(): assert len(events) == 25 -def test_ws_audit_log_only_includes_current_ws_events(): +def test_portfolio_audit_log_only_includes_current_portfolio_events(): owner = UserFactory.create() portfolio = PortfolioFactory.create(owner=owner) other_portfolio = PortfolioFactory.create(owner=owner) @@ -55,8 +56,30 @@ def test_ws_audit_log_only_includes_current_ws_events(): events = AuditLog.get_portfolio_events(portfolio) for event in events: - assert event.portfolio_id == portfolio.id or event.resource_id == portfolio.id + assert event.portfolio_id == portfolio.id assert ( not event.portfolio_id == other_portfolio.id or event.resource_id == other_portfolio.id ) + + +def test_portfolio_audit_log_includes_app_and_env_events(): + # TODO: add in events for + # creating/updating/deleting env_role and app_role + # creating an app_invitation + owner = UserFactory.create() + portfolio = PortfolioFactory.create(owner=owner) + application = ApplicationFactory.create(portfolio=portfolio) + Applications.update(application, {"name": "Star Cruiser"}) + + events = AuditLog.get_portfolio_events(portfolio) + + for event in events: + assert event.portfolio_id == portfolio.id + if event.resource_type == 'application': + assert event.application_id == application.id + pass + + +def test_application_audit_log_does_not_include_portfolio_events(): + pass From 3e07d959691a8059153f4db31c4953a88dce0e2d Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 15 May 2019 18:53:27 -0400 Subject: [PATCH 2/8] Refactor Pagination macro --- templates/audit_log/audit_log.html | 2 +- templates/components/pagination.html | 29 ++++++++++++++-------------- templates/portfolios/admin.html | 2 +- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/templates/audit_log/audit_log.html b/templates/audit_log/audit_log.html index 6c2cf5c4..803d70ad 100644 --- a/templates/audit_log/audit_log.html +++ b/templates/audit_log/audit_log.html @@ -4,6 +4,6 @@ {% block content %}
{% include "fragments/audit_events_log.html" %} - {{ Pagination(audit_events, 'atst.activity_history')}} + {{ Pagination(audit_events, url_for('atst.activity_history'))}}
{% endblock %} diff --git a/templates/components/pagination.html b/templates/components/pagination.html index 8b086588..e85904ca 100644 --- a/templates/components/pagination.html +++ b/templates/components/pagination.html @@ -1,4 +1,4 @@ -{% macro Page(pagination, route, i, label=None, disabled=False, portfolio_id=None) -%} +{% macro Page(pagination, url, i, label=None, disabled=False) -%} {% set label = label or i %} {% set button_class = "page usa-button " %} @@ -11,44 +11,45 @@ {% set button_class = button_class + "usa-button-secondary" %} {% endif %} - {{ label }} + {{ label }} {%- endmacro %} -{% macro Pagination(pagination, route, portfolio_id=None) -%} +{% macro Pagination(pagination, url) -%} + diff --git a/templates/portfolios/admin.html b/templates/portfolios/admin.html index 6ba37cb0..40010a53 100644 --- a/templates/portfolios/admin.html +++ b/templates/portfolios/admin.html @@ -52,7 +52,7 @@ {% if user_can(permissions.VIEW_PORTFOLIO_ACTIVITY_LOG) %} {% include "fragments/audit_events_log.html" %} - {{ Pagination(audit_events, 'portfolios.admin', portfolio_id=portfolio.id) }} + {{ Pagination(audit_events, url_for('portfolios.admin', portfolio_id=portfolio.id)) }} {% endif %} {% endblock %} From 927d1b7925c79dc6c4723c4195d5771f1764dae8 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 15 May 2019 19:10:18 -0400 Subject: [PATCH 3/8] Add audit log to app settings page --- atst/domain/audit_log.py | 12 ++++++++++++ atst/routes/applications/settings.py | 5 +++++ templates/portfolios/applications/settings.html | 6 ++++++ tests/routes/applications/test_settings.py | 6 ++++-- 4 files changed, 27 insertions(+), 2 deletions(-) diff --git a/atst/domain/audit_log.py b/atst/domain/audit_log.py index cb7354db..97bb6603 100644 --- a/atst/domain/audit_log.py +++ b/atst/domain/audit_log.py @@ -22,6 +22,15 @@ class AuditEventQuery(Query): ) return cls.paginate(query, pagination_opts) + @classmethod + def get_application_events(cls, application_id, pagination_opts): + query = ( + db.session.query(cls.model) + .filter(cls.model.application_id == application_id) + .order_by(cls.model.time_created.desc()) + ) + return cls.paginate(query, pagination_opts) + class AuditLog(object): @classmethod @@ -36,6 +45,9 @@ class AuditLog(object): def get_portfolio_events(cls, portfolio, pagination_opts=None): return AuditEventQuery.get_portfolio_events(portfolio.id, pagination_opts) + @classmethod + def get_application_events(cls, application, pagination_opts=None): + return AuditEventQuery.get_application_events(application.id, pagination_opts) @classmethod def get_by_resource(cls, resource_id): diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index 935aaaf4..a50e04ae 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -3,6 +3,8 @@ from flask import redirect, render_template, request as http_request, url_for from . import applications_bp from atst.domain.environments import Environments from atst.domain.applications import Applications +from atst.domain.audit_log import AuditLog +from atst.domain.common import Paginator from atst.forms.app_settings import AppEnvRolesForm from atst.forms.application import ApplicationForm, EditEnvironmentForm from atst.forms.data import ENV_ROLE_NO_ACCESS as NO_ACCESS @@ -90,6 +92,8 @@ def render_settings_page(application, **kwargs): environments_obj = get_environments_obj_for_app(application=application) members_form = AppEnvRolesForm(data=data_for_app_env_roles_form(application)) new_env_form = EditEnvironmentForm() + pagination_opts = Paginator.get_pagination_opts(http_request) + audit_events = AuditLog.get_application_events(application, pagination_opts) if "application_form" not in kwargs: kwargs["application_form"] = ApplicationForm( @@ -102,6 +106,7 @@ def render_settings_page(application, **kwargs): environments_obj=environments_obj, members_form=members_form, new_env_form=new_env_form, + audit_events=audit_events, **kwargs, ) diff --git a/templates/portfolios/applications/settings.html b/templates/portfolios/applications/settings.html index 8fe46732..65b14f4a 100644 --- a/templates/portfolios/applications/settings.html +++ b/templates/portfolios/applications/settings.html @@ -4,6 +4,7 @@ {% from "components/delete_confirmation.html" import DeleteConfirmation %} {% from "components/icon.html" import Icon %} {% from "components/modal.html" import Modal %} +{% from "components/pagination.html" import Pagination %} {% from "components/text_input.html" import TextInput %} {% set secondary_breadcrumb = 'portfolios.applications.existing_application_title' | translate({ "application_name": application.name }) %} @@ -102,4 +103,9 @@ {% endcall %} {% endif %} + {% if user_can(permissions.VIEW_APPLICATION) %} + {% include "fragments/audit_events_log.html" %} + {{ Pagination(audit_events, url=url_for('applications.settings', application_id=application.id)) }} + {% endif %} + {% endblock %} diff --git a/tests/routes/applications/test_settings.py b/tests/routes/applications/test_settings.py index 40f758ed..ff0b8d34 100644 --- a/tests/routes/applications/test_settings.py +++ b/tests/routes/applications/test_settings.py @@ -15,6 +15,7 @@ from atst.routes.applications.settings import check_users_are_in_application from atst.domain.applications import Applications from atst.domain.environment_roles import EnvironmentRoles from atst.domain.environments import Environments +from atst.domain.common import Paginator from atst.domain.permission_sets import PermissionSets from atst.domain.portfolios import Portfolios from atst.domain.exceptions import NotFoundError @@ -116,7 +117,7 @@ def test_edit_application_environments_obj(app, client, user_session): ) assert response.status_code == 200 - _, context = templates[0] + _, context = templates[-1] assert isinstance(context["members_form"], AppEnvRolesForm) env_obj = context["environments_obj"][0] @@ -127,6 +128,7 @@ def test_edit_application_environments_obj(app, client, user_session): env_obj["members"].sort() == [env_role1.user.full_name, env_role2.user.full_name].sort() ) + assert isinstance(context["audit_events"], Paginator) def test_data_for_app_env_roles_form(app, client, user_session): @@ -156,7 +158,7 @@ def test_data_for_app_env_roles_form(app, client, user_session): ) assert response.status_code == 200 - _, context = templates[0] + _, context = templates[-1] members_form = context["members_form"] assert isinstance(members_form, AppEnvRolesForm) From a1eb7ec935ca07e73d1f9f3414124117afc352f1 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 16 May 2019 16:58:41 -0400 Subject: [PATCH 4/8] Make sure all log events have portfolio and app ids (where applicable) --- atst/domain/application_roles.py | 4 +- atst/domain/audit_log.py | 2 - atst/models/application_invitation.py | 4 ++ atst/models/application_role.py | 3 ++ atst/models/environment.py | 3 +- atst/models/environment_role.py | 8 ++++ atst/models/mixins/auditable.py | 23 +++------- tests/domain/test_audit_log.py | 62 +++++++++++++++++++++------ 8 files changed, 73 insertions(+), 36 deletions(-) diff --git a/atst/domain/application_roles.py b/atst/domain/application_roles.py index e1956aba..c0e8e79b 100644 --- a/atst/domain/application_roles.py +++ b/atst/domain/application_roles.py @@ -14,7 +14,9 @@ class ApplicationRoles(object): @classmethod def create(cls, user, application, permission_set_names): - application_role = ApplicationRole(user=user, application_id=application.id) + application_role = ApplicationRole( + user=user, application_id=application.id, application=application + ) application_role.permission_sets = ApplicationRoles._permission_sets_for_names( permission_set_names diff --git a/atst/domain/audit_log.py b/atst/domain/audit_log.py index 97bb6603..4090ccfd 100644 --- a/atst/domain/audit_log.py +++ b/atst/domain/audit_log.py @@ -1,5 +1,3 @@ -from sqlalchemy import or_ - from atst.database import db from atst.domain.common import Query from atst.models.audit_event import AuditEvent diff --git a/atst/models/application_invitation.py b/atst/models/application_invitation.py index 9f1db550..328ac59e 100644 --- a/atst/models/application_invitation.py +++ b/atst/models/application_invitation.py @@ -26,6 +26,10 @@ class ApplicationInvitation(Base, TimestampsMixin, AuditableMixin, InvitesMixin) def application_id(self): return self.role.application_id + @property + def portfolio_id(self): + return self.role.portfolio_id + @property def event_details(self): return {"email": self.email, "dod_id": self.user_dod_id} diff --git a/atst/models/application_role.py b/atst/models/application_role.py index fe56a37d..b98f57a7 100644 --- a/atst/models/application_role.py +++ b/atst/models/application_role.py @@ -75,6 +75,9 @@ class ApplicationRole( lambda prms: prms.name == perm_set_name, self.permission_sets ) + @property + def portfolio_id(self): + return self.application.portfolio_id Index( "application_role_user_application", diff --git a/atst/models/environment.py b/atst/models/environment.py index 1817517b..e15373b4 100644 --- a/atst/models/environment.py +++ b/atst/models/environment.py @@ -35,7 +35,8 @@ class Environment( def portfolio(self): return self.application.portfolio - def auditable_portfolio_id(self): + @property + def portfolio_id(self): return self.application.portfolio_id def __repr__(self): diff --git a/atst/models/environment_role.py b/atst/models/environment_role.py index 55cf742e..5ad67f6c 100644 --- a/atst/models/environment_role.py +++ b/atst/models/environment_role.py @@ -38,6 +38,14 @@ class EnvironmentRole( def history(self): return self.get_changes() + @property + def portfolio_id(self): + return self.environment.application.portfolio_id + + @property + def application_id(self): + return self.environment.application_id + @property def displayname(self): return self.role diff --git a/atst/models/mixins/auditable.py b/atst/models/mixins/auditable.py index 70c462a2..1d83da82 100644 --- a/atst/models/mixins/auditable.py +++ b/atst/models/mixins/auditable.py @@ -13,33 +13,20 @@ class AuditableMixin(object): @staticmethod def create_audit_event(connection, resource, action, changed_state=None): user_id = getattr_path(g, "current_user.id") - resource_type = resource.resource_type - display_name = resource.displayname - event_details = resource.event_details - - if resource_type == "portfolio": - portfolio_id = resource.id - else: - portfolio_id = resource.portfolio_id - - if resource_type == "application": - application_id = resource.id - else: - application_id = resource.application_id if changed_state is None: changed_state = resource.history if action == ACTION_UPDATE else None audit_event = AuditEvent( user_id=user_id, - portfolio_id=portfolio_id, - application_id=application_id, - resource_type=resource_type, + portfolio_id=resource.portfolio_id, + application_id=resource.application_id, + resource_type=resource.resource_type, resource_id=resource.id, - display_name=display_name, + display_name=resource.displayname, action=action, changed_state=changed_state, - event_details=event_details, + event_details=resource.event_details, ) app.logger.info( diff --git a/tests/domain/test_audit_log.py b/tests/domain/test_audit_log.py index 687b2395..da74e7b0 100644 --- a/tests/domain/test_audit_log.py +++ b/tests/domain/test_audit_log.py @@ -4,12 +4,17 @@ from atst.domain.applications import Applications from atst.domain.audit_log import AuditLog from atst.domain.exceptions import UnauthorizedError from atst.domain.permission_sets import PermissionSets +from atst.domain.portfolios import Portfolios from atst.models.portfolio_role import Status as PortfolioRoleStatus from tests.factories import ( - UserFactory, + ApplicationFactory, + ApplicationInvitationFactory, + ApplicationRoleFactory, + EnvironmentFactory, + EnvironmentRoleFactory, PortfolioFactory, PortfolioRoleFactory, - ApplicationFactory, + UserFactory, ) @@ -63,23 +68,52 @@ def test_portfolio_audit_log_only_includes_current_portfolio_events(): ) -def test_portfolio_audit_log_includes_app_and_env_events(): - # TODO: add in events for - # creating/updating/deleting env_role and app_role - # creating an app_invitation +def test_get_portfolio_events_includes_app_and_env_events(): owner = UserFactory.create() + # add portfolio level events portfolio = PortfolioFactory.create(owner=owner) + portfolio_events = AuditLog.get_portfolio_events(portfolio) + + # add application level events application = ApplicationFactory.create(portfolio=portfolio) Applications.update(application, {"name": "Star Cruiser"}) + app_role = ApplicationRoleFactory.create(application=application) + app_invite = ApplicationInvitationFactory.create(role=app_role) + portfolio_and_app_events = AuditLog.get_portfolio_events(portfolio) + assert len(portfolio_events) < len(portfolio_and_app_events) - events = AuditLog.get_portfolio_events(portfolio) + # add environment level events + env = EnvironmentFactory.create(application=application) + env_role = EnvironmentRoleFactory.create(environment=env, user=app_role.user) + portfolio_app_and_env_events = AuditLog.get_portfolio_events(portfolio) + assert len(portfolio_and_app_events) < len(portfolio_app_and_env_events) + resource_types = [event.resource_type for event in portfolio_app_and_env_events] + assert "application" in resource_types + assert "application_role" in resource_types + assert "application_invitation" in resource_types + assert "environment" in resource_types + assert "environment_role" in resource_types + + +def test_get_application_events(): + # add in some portfolio level events + portfolio = PortfolioFactory.create() + Portfolios.update(portfolio, {"name": "New Name"}) + # add app level events + application = ApplicationFactory.create(portfolio=portfolio) + Applications.update(application, {"name": "Star Cruiser"}) + app_role = ApplicationRoleFactory.create(application=application) + app_invite = ApplicationInvitationFactory.create(role=app_role) + env = EnvironmentFactory.create(application=application) + env_role = EnvironmentRoleFactory.create(environment=env, user=app_role.user) + # add rando app + rando_app = ApplicationFactory.create(portfolio=portfolio) + + events = AuditLog.get_application_events(application) for event in events: - assert event.portfolio_id == portfolio.id - if event.resource_type == 'application': - assert event.application_id == application.id - pass + assert event.application_id == application.id + assert not event.application_id == rando_app.id - -def test_application_audit_log_does_not_include_portfolio_events(): - pass + resource_types = [event.resource_type for event in events] + assert "portfolio" not in resource_types From a65f758894fbb06dcede6639cb9cf6467bb4a8c5 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Fri, 17 May 2019 11:19:38 -0400 Subject: [PATCH 5/8] Add templates to display new audit events --- atst/models/application_role.py | 15 +++++++++++++- atst/models/mixins/invites.py | 20 +++++++++++++++++++ atst/models/portfolio_invitation.py | 16 +-------------- .../events/application_invitation.html | 16 +++++++++++++++ .../audit_log/events/application_role.html | 9 +++++++++ 5 files changed, 60 insertions(+), 16 deletions(-) create mode 100644 templates/audit_log/events/application_invitation.html create mode 100644 templates/audit_log/events/application_role.html diff --git a/atst/models/application_role.py b/atst/models/application_role.py index b98f57a7..7b343cd8 100644 --- a/atst/models/application_role.py +++ b/atst/models/application_role.py @@ -53,7 +53,10 @@ class ApplicationRole( @property def user_name(self): - return self.user.full_name + if self.user: + return self.user.full_name + else: + return None def __repr__(self): return "".format( @@ -79,6 +82,16 @@ class ApplicationRole( def portfolio_id(self): return self.application.portfolio_id + @property + def event_details(self): + return { + "updated_user_name": self.user_name, + "updated_user_id": str(self.user_id), + "application": self.application.name, + "portfolio": self.application.portfolio.name, + } + + Index( "application_role_user_application", ApplicationRole.user_id, diff --git a/atst/models/mixins/invites.py b/atst/models/mixins/invites.py index 39bedc08..f7212850 100644 --- a/atst/models/mixins/invites.py +++ b/atst/models/mixins/invites.py @@ -105,3 +105,23 @@ class InvitesMixin(object): @property def user_dod_id(self): return self.user.dod_id if self.user is not None else None + + @property + def event_details(self): + """Overrides the same property in AuditableMixin. + Provides the event details for an invite that are required for the audit log + """ + return {"email": self.email, "dod_id": self.user_dod_id} + + @property + def history(self): + """Overrides the same property in AuditableMixin + Determines whether or not invite status has been updated + """ + changes = self.get_changes() + change_set = {} + + if "status" in changes: + change_set["status"] = [s.name for s in changes["status"]] + + return change_set diff --git a/atst/models/portfolio_invitation.py b/atst/models/portfolio_invitation.py index 1c1e43dd..33aaadb6 100644 --- a/atst/models/portfolio_invitation.py +++ b/atst/models/portfolio_invitation.py @@ -6,7 +6,7 @@ from atst.models import Base from atst.models.mixins import TimestampsMixin, AuditableMixin, InvitesMixin -class PortfolioInvitation(Base, TimestampsMixin, AuditableMixin, InvitesMixin): +class PortfolioInvitation(Base, TimestampsMixin, InvitesMixin, AuditableMixin): __tablename__ = "portfolio_invitations" portfolio_role_id = Column( @@ -25,17 +25,3 @@ class PortfolioInvitation(Base, TimestampsMixin, AuditableMixin, InvitesMixin): @property def portfolio_id(self): return self.role.portfolio_id - - @property - def event_details(self): - return {"email": self.email, "dod_id": self.user_dod_id} - - @property - def history(self): - changes = self.get_changes() - change_set = {} - - if "status" in changes: - change_set["status"] = [s.name for s in changes["status"]] - - return change_set diff --git a/templates/audit_log/events/application_invitation.html b/templates/audit_log/events/application_invitation.html new file mode 100644 index 00000000..a68e3d03 --- /dev/null +++ b/templates/audit_log/events/application_invitation.html @@ -0,0 +1,16 @@ +{% extends 'audit_log/events/_base.html' %} + +{% block content %} + {% set accepted = event.changed_state.status and event.changed_state.status.1 == "ACCEPTED" %} + {% if accepted %} + accepted by {{ event.event_details.email }} (DOD {{ event.event_details.dod_id }}) +
+ {% endif %} + {% if event.action == "create" %} + invited {{ event.event_details.email }} (DOD {{ event.event_details.dod_id }}) +
+ {% endif %} + in Application {{ event.application_id }} ({{ event.application.name }}) +
+ in Portfolio {{ event.portfolio_id }} ({{ event.portfolio.name }}) +{% endblock %} diff --git a/templates/audit_log/events/application_role.html b/templates/audit_log/events/application_role.html new file mode 100644 index 00000000..ff1ee6f9 --- /dev/null +++ b/templates/audit_log/events/application_role.html @@ -0,0 +1,9 @@ +{% extends 'audit_log/events/_base.html' %} + +{% block content %} + for User {{ event.event_details.updated_user_id }} ({{ event.event_details.updated_user_name }}) +
+ in Application {{ event.application_id}} ({{ event.event_details.application }}) +
+ in Portfolio {{ event.portfolio_id }} ({{ event.event_details.portfolio }}) +{% endblock %} From 5e2f00b1c298098035e79a68c4ff1da360273a8d Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 21 May 2019 14:13:42 -0400 Subject: [PATCH 6/8] Add perms for viewing application log --- atst/domain/permission_sets.py | 1 + atst/models/permissions.py | 1 + templates/portfolios/applications/settings.html | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/atst/domain/permission_sets.py b/atst/domain/permission_sets.py index d0d99031..1336bc92 100644 --- a/atst/domain/permission_sets.py +++ b/atst/domain/permission_sets.py @@ -209,6 +209,7 @@ _APPLICATION_TEAM_PERMISSION_SET = { Permissions.DELETE_APPLICATION_MEMBER, Permissions.CREATE_APPLICATION_MEMBER, Permissions.ASSIGN_ENVIRONMENT_MEMBER, + Permissions.VIEW_APPLICATION_ACTIVITY_LOG, ], } diff --git a/atst/models/permissions.py b/atst/models/permissions.py index e4b797e4..75991a64 100644 --- a/atst/models/permissions.py +++ b/atst/models/permissions.py @@ -18,6 +18,7 @@ class Permissions(object): CREATE_ENVIRONMENT = "create_environment" DELETE_ENVIRONMENT = "delete_environment" ASSIGN_ENVIRONMENT_MEMBER = "assign_environment_member" + VIEW_APPLICATION_ACTIVITY_LOG = "view_application_activity_log" # funding VIEW_PORTFOLIO_FUNDING = "view_portfolio_funding" # TO summary page diff --git a/templates/portfolios/applications/settings.html b/templates/portfolios/applications/settings.html index 65b14f4a..e43a9c34 100644 --- a/templates/portfolios/applications/settings.html +++ b/templates/portfolios/applications/settings.html @@ -103,7 +103,7 @@ {% endcall %} {% endif %} - {% if user_can(permissions.VIEW_APPLICATION) %} + {% if user_can(permissions.VIEW_APPLICATION_ACTIVITY_LOG) %} {% include "fragments/audit_events_log.html" %} {{ Pagination(audit_events, url=url_for('applications.settings', application_id=application.id)) }} {% endif %} From 9f66bbafe5ff446883d1082b91187172cc5ea4a9 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 21 May 2019 16:55:55 -0400 Subject: [PATCH 7/8] Raise NotImplementedError in AuditableMixin if the model does not have the properties portfolio_id or application_id --- atst/models/application.py | 4 ++++ atst/models/mixins/auditable.py | 4 ++-- atst/models/portfolio.py | 7 ++++++- atst/models/portfolio_invitation.py | 4 ++++ atst/models/portfolio_role.py | 4 ++++ atst/models/user.py | 8 ++++++++ 6 files changed, 28 insertions(+), 3 deletions(-) diff --git a/atst/models/application.py b/atst/models/application.py index ef783ff6..d1187aef 100644 --- a/atst/models/application.py +++ b/atst/models/application.py @@ -52,6 +52,10 @@ class Application( def displayname(self): return self.name + @property + def application_id(self): + return self.id + def __repr__(self): # pragma: no cover return "".format( self.name, self.description, self.portfolio.name, self.id diff --git a/atst/models/mixins/auditable.py b/atst/models/mixins/auditable.py index 1d83da82..acfa6b2e 100644 --- a/atst/models/mixins/auditable.py +++ b/atst/models/mixins/auditable.py @@ -90,11 +90,11 @@ class AuditableMixin(object): @property def portfolio_id(self): - return None + raise NotImplementedError() @property def application_id(self): - return None + raise NotImplementedError() @property def displayname(self): diff --git a/atst/models/portfolio.py b/atst/models/portfolio.py index 0e65b146..7f2bbe84 100644 --- a/atst/models/portfolio.py +++ b/atst/models/portfolio.py @@ -68,9 +68,14 @@ class Portfolio(Base, mixins.TimestampsMixin, mixins.AuditableMixin): def all_environments(self): return list(chain.from_iterable(p.environments for p in self.applications)) - def auditable_portfolio_id(self): + @property + def portfolio_id(self): return self.id + @property + def application_id(self): + return None + def __repr__(self): return "".format( self.name, self.user_count, self.id diff --git a/atst/models/portfolio_invitation.py b/atst/models/portfolio_invitation.py index 33aaadb6..a6e070d4 100644 --- a/atst/models/portfolio_invitation.py +++ b/atst/models/portfolio_invitation.py @@ -25,3 +25,7 @@ class PortfolioInvitation(Base, TimestampsMixin, InvitesMixin, AuditableMixin): @property def portfolio_id(self): return self.role.portfolio_id + + @property + def application_id(self): + return None diff --git a/atst/models/portfolio_role.py b/atst/models/portfolio_role.py index 3b5859cf..2d26f636 100644 --- a/atst/models/portfolio_role.py +++ b/atst/models/portfolio_role.py @@ -164,6 +164,10 @@ class PortfolioRole( def full_name(self): return self.user.full_name + @property + def application_id(self): + return None + Index( "portfolio_role_user_portfolio", diff --git a/atst/models/user.py b/atst/models/user.py index d40501a4..c1693a37 100644 --- a/atst/models/user.py +++ b/atst/models/user.py @@ -96,6 +96,14 @@ class User( def displayname(self): return self.full_name + @property + def portfolio_id(self): + return None + + @property + def application_id(self): + return None + def __repr__(self): return "".format( self.full_name, self.dod_id, self.email, self.has_portfolios, self.id From 7756ab8c281c71a7b6ca4db1aaf364ecb1ccbf11 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Fri, 17 May 2019 11:20:23 -0400 Subject: [PATCH 8/8] Notes about unused methods that need more research --- atst/domain/audit_log.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/atst/domain/audit_log.py b/atst/domain/audit_log.py index 4090ccfd..42a996d3 100644 --- a/atst/domain/audit_log.py +++ b/atst/domain/audit_log.py @@ -32,6 +32,7 @@ class AuditEventQuery(Query): class AuditLog(object): @classmethod + # TODO: see if this is being used anywhere and remove if not def log_system_event(cls, resource, action, portfolio=None): return cls._log(resource=resource, action=action, portfolio=portfolio) @@ -61,6 +62,7 @@ class AuditLog(object): return type(resource).__name__.lower() @classmethod + # TODO: see if this is being used anywhere and remove if not def _log(cls, user=None, portfolio=None, resource=None, action=None): resource_id = resource.id if resource else None resource_type = cls._resource_type(resource) if resource else None