From bf22afa7afe8b10a47bed7481f257d91611121e0 Mon Sep 17 00:00:00 2001 From: Montana Date: Mon, 7 Jan 2019 11:30:59 -0500 Subject: [PATCH 01/14] Add Activity Log tab for a workspace --- atst/routes/workspaces/index.py | 14 ++++++++++++++ templates/navigation/workspace_navigation.html | 9 +++++++++ templates/workspaces/activity.html | 1 + translations.yaml | 1 + 4 files changed, 25 insertions(+) create mode 100644 templates/workspaces/activity.html diff --git a/atst/routes/workspaces/index.py b/atst/routes/workspaces/index.py index 2bdcd6d2..30e11ab1 100644 --- a/atst/routes/workspaces/index.py +++ b/atst/routes/workspaces/index.py @@ -80,3 +80,17 @@ def workspace_reports(workspace_id): expiration_date=expiration_date, remaining_days=remaining_days, ) + + +@workspaces_bp.route("/workspaces//activity") +def workspace_activity(workspace_id): + workspace = Workspaces.get(g.current_user, workspace_id) + Authorization.check_workspace_permission( + g.current_user, + workspace, + # TODO: diff permission + Permissions.VIEW_USAGE_DOLLARS, + "view workspace reports", + ) + + return render_template("workspaces/activity.html", workspace_name=workspace.name) diff --git a/templates/navigation/workspace_navigation.html b/templates/navigation/workspace_navigation.html index 3a36bff5..698f193f 100644 --- a/templates/navigation/workspace_navigation.html +++ b/templates/navigation/workspace_navigation.html @@ -56,5 +56,14 @@ ) }} {% endif %} + + {% if user_can(permissions.VIEW_USAGE_DOLLARS) %} + {{ SidenavItem( + ("navigation.workspace_navigation.activity_log" | translate), + href=url_for("workspaces.workspace_activity", workspace_id=workspace.id), + active=request.url_rule.rule.startswith('/workspaces//activity') + ) }} + {% endif %} + diff --git a/templates/workspaces/activity.html b/templates/workspaces/activity.html new file mode 100644 index 00000000..acd43b4e --- /dev/null +++ b/templates/workspaces/activity.html @@ -0,0 +1 @@ +

Activity Log for Workspace {{workspace_name}}

diff --git a/translations.yaml b/translations.yaml index d4268246..61859980 100644 --- a/translations.yaml +++ b/translations.yaml @@ -202,6 +202,7 @@ navigation: add_new_member_label: Add New Member add_new_project_label: Add New Project budget_report: Budget Report + activity_log: Activity Log members: Members projects: Projects task_orders: Task Orders From 3731dd876e253362cb0d979fba771536ca62e88f Mon Sep 17 00:00:00 2001 From: Montana Date: Mon, 7 Jan 2019 14:46:41 -0500 Subject: [PATCH 02/14] Display audit logs for current workspace --- atst/domain/audit_log.py | 9 +++++++++ atst/routes/workspaces/index.py | 10 ++++++++-- templates/workspaces/activity.html | 1 - templates/workspaces/activity/index.html | 23 +++++++++++++++++++++++ 4 files changed, 40 insertions(+), 3 deletions(-) delete mode 100644 templates/workspaces/activity.html create mode 100644 templates/workspaces/activity/index.html diff --git a/atst/domain/audit_log.py b/atst/domain/audit_log.py index 6dc42a50..0292920a 100644 --- a/atst/domain/audit_log.py +++ b/atst/domain/audit_log.py @@ -25,6 +25,15 @@ class AuditLog(object): ) return AuditEventQuery.get_all(pagination_opts) + @classmethod + def get_workspace_events(cls, workspace_id): + return ( + db.session.query(AuditEvent) + .filter(AuditEvent.workspace_id == workspace_id) + .order_by(AuditEvent.time_created.desc()) + .all() + ) + @classmethod def get_by_resource(cls, resource_id): return ( diff --git a/atst/routes/workspaces/index.py b/atst/routes/workspaces/index.py index 30e11ab1..cad5e5b2 100644 --- a/atst/routes/workspaces/index.py +++ b/atst/routes/workspaces/index.py @@ -5,8 +5,9 @@ from flask import render_template, request as http_request, g, redirect, url_for from . import workspaces_bp from atst.domain.reports import Reports from atst.domain.workspaces import Workspaces -from atst.forms.workspace import WorkspaceForm +from atst.domain.audit_log import AuditLog from atst.domain.authz import Authorization +from atst.forms.workspace import WorkspaceForm from atst.models.permissions import Permissions @@ -92,5 +93,10 @@ def workspace_activity(workspace_id): Permissions.VIEW_USAGE_DOLLARS, "view workspace reports", ) + audit_events = AuditLog.get_workspace_events(workspace_id) - return render_template("workspaces/activity.html", workspace_name=workspace.name) + return render_template( + "workspaces/activity/index.html", + workspace_name=workspace.name, + audit_events=audit_events, + ) diff --git a/templates/workspaces/activity.html b/templates/workspaces/activity.html deleted file mode 100644 index acd43b4e..00000000 --- a/templates/workspaces/activity.html +++ /dev/null @@ -1 +0,0 @@ -

Activity Log for Workspace {{workspace_name}}

diff --git a/templates/workspaces/activity/index.html b/templates/workspaces/activity/index.html new file mode 100644 index 00000000..08f2a583 --- /dev/null +++ b/templates/workspaces/activity/index.html @@ -0,0 +1,23 @@ +{% extends "workspaces/base.html" %} + +{% block workspace_content %} + +
+
+
+

{{ "audit_log.header_title" | translate }}

+
+ +
    + {% for event in audit_events %} +
  • + {% autoescape false %} + {{ event | renderAuditEvent }} + {% endautoescape %} +
    • + {% endfor %} +
+
+ +
+{% endblock %} From ef6d9a2c5fab5649d8dd799df62f92ce61f8b422 Mon Sep 17 00:00:00 2001 From: Montana Date: Mon, 7 Jan 2019 15:54:07 -0500 Subject: [PATCH 03/14] Add pagination --- atst/domain/audit_log.py | 19 ++++++---- atst/domain/common/__init__.py | 1 + atst/domain/common/query.py | 7 ++++ atst/routes/__init__.py | 10 ++---- atst/routes/workspaces/index.py | 13 +++---- templates/components/pagination.html | 36 +++++++++---------- .../navigation/workspace_navigation.html | 3 +- templates/workspaces/activity/index.html | 2 ++ 8 files changed, 51 insertions(+), 40 deletions(-) diff --git a/atst/domain/audit_log.py b/atst/domain/audit_log.py index 0292920a..b40dc304 100644 --- a/atst/domain/audit_log.py +++ b/atst/domain/audit_log.py @@ -12,6 +12,15 @@ class AuditEventQuery(Query): query = db.session.query(cls.model).order_by(cls.model.time_created.desc()) return cls.paginate(query, pagination_opts) + @classmethod + def get_ws_events(cls, workspace_id, pagination_opts): + query = ( + db.session.query(cls.model) + .filter(cls.model.workspace_id == workspace_id) + .order_by(cls.model.time_created.desc()) + ) + return cls.paginate(query, pagination_opts) + class AuditLog(object): @classmethod @@ -26,13 +35,11 @@ class AuditLog(object): return AuditEventQuery.get_all(pagination_opts) @classmethod - def get_workspace_events(cls, workspace_id): - return ( - db.session.query(AuditEvent) - .filter(AuditEvent.workspace_id == workspace_id) - .order_by(AuditEvent.time_created.desc()) - .all() + def get_workspace_events(cls, user, workspace_id, pagination_opts): + Authorization.check_atat_permission( + user, Permissions.VIEW_AUDIT_LOG, "view audit log" ) + return AuditEventQuery.get_ws_events(workspace_id, pagination_opts) @classmethod def get_by_resource(cls, resource_id): diff --git a/atst/domain/common/__init__.py b/atst/domain/common/__init__.py index f829496f..684d4ce6 100644 --- a/atst/domain/common/__init__.py +++ b/atst/domain/common/__init__.py @@ -1 +1,2 @@ from .query import Query +from .query import Paginator diff --git a/atst/domain/common/query.py b/atst/domain/common/query.py index 07db46ef..0846387c 100644 --- a/atst/domain/common/query.py +++ b/atst/domain/common/query.py @@ -17,6 +17,13 @@ class Paginator(object): def __init__(self, query_set): self.query_set = query_set + @classmethod + def get_pagination_opts(cls, request, default_page=1, default_per_page=100): + return { + "page": int(request.args.get("page", default_page)), + "per_page": int(request.args.get("perPage", default_per_page)), + } + @classmethod def paginate(cls, query, pagination_opts=None): if pagination_opts is not None: diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index 7581eed3..07550af7 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -12,6 +12,7 @@ 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 +from atst.domain.common import Paginator from atst.utils.flash import formatted_flash as flash @@ -126,16 +127,9 @@ def logout(): return redirect(url_for(".root")) -def get_pagination_opts(request, default_page=1, default_per_page=100): - return { - "page": int(request.args.get("page", default_page)), - "per_page": int(request.args.get("perPage", default_per_page)), - } - - @bp.route("/activity-history") def activity_history(): - pagination_opts = get_pagination_opts(request) + pagination_opts = Paginator.get_pagination_opts(request) audit_events = AuditLog.get_all_events(g.current_user, pagination_opts) return render_template("audit_log/audit_log.html", audit_events=audit_events) diff --git a/atst/routes/workspaces/index.py b/atst/routes/workspaces/index.py index cad5e5b2..01e7b917 100644 --- a/atst/routes/workspaces/index.py +++ b/atst/routes/workspaces/index.py @@ -7,6 +7,7 @@ from atst.domain.reports import Reports from atst.domain.workspaces import Workspaces from atst.domain.audit_log import AuditLog from atst.domain.authz import Authorization +from atst.domain.common import Paginator from atst.forms.workspace import WorkspaceForm from atst.models.permissions import Permissions @@ -87,16 +88,16 @@ def workspace_reports(workspace_id): def workspace_activity(workspace_id): workspace = Workspaces.get(g.current_user, workspace_id) Authorization.check_workspace_permission( - g.current_user, - workspace, - # TODO: diff permission - Permissions.VIEW_USAGE_DOLLARS, - "view workspace reports", + g.current_user, workspace, Permissions.VIEW_AUDIT_LOG, "view workspace reports" + ) + pagination_opts = Paginator.get_pagination_opts(http_request) + audit_events = AuditLog.get_workspace_events( + g.current_user, workspace_id, pagination_opts ) - audit_events = AuditLog.get_workspace_events(workspace_id) return render_template( "workspaces/activity/index.html", workspace_name=workspace.name, + workspace_id=workspace_id, audit_events=audit_events, ) diff --git a/templates/components/pagination.html b/templates/components/pagination.html index 5b1c84a9..fe360a9f 100644 --- a/templates/components/pagination.html +++ b/templates/components/pagination.html @@ -1,4 +1,4 @@ -{% macro Page(pagination, route, i, label=None, disabled=False) -%} +{% macro Page(pagination, route, i, label=None, disabled=False, workspace_id=None) -%} {% set label = label or i %} {% set button_class = "page usa-button " %} @@ -11,38 +11,38 @@ {% set button_class = button_class + "usa-button-secondary" %} {% endif %} - {{ label }} + {{ label }} {%- endmacro %} -{% macro Pagination(pagination, route) -%} +{% macro Pagination(pagination, route, workspace_id=None) -%} {%- endmacro %} diff --git a/templates/navigation/workspace_navigation.html b/templates/navigation/workspace_navigation.html index 698f193f..d8c656a4 100644 --- a/templates/navigation/workspace_navigation.html +++ b/templates/navigation/workspace_navigation.html @@ -56,8 +56,7 @@ ) }} {% endif %} - - {% if user_can(permissions.VIEW_USAGE_DOLLARS) %} + {% if user_can(permissions.VIEW_AUDIT_LOG) %} {{ SidenavItem( ("navigation.workspace_navigation.activity_log" | translate), href=url_for("workspaces.workspace_activity", workspace_id=workspace.id), diff --git a/templates/workspaces/activity/index.html b/templates/workspaces/activity/index.html index 08f2a583..bce6217e 100644 --- a/templates/workspaces/activity/index.html +++ b/templates/workspaces/activity/index.html @@ -1,4 +1,5 @@ {% extends "workspaces/base.html" %} +{% from "components/pagination.html" import Pagination %} {% block workspace_content %} @@ -19,5 +20,6 @@ + {{ Pagination(audit_events, 'workspaces.workspace_activity', workspace_id=workspace_id) }} {% endblock %} From 81af9f1c99c5247f88325967802c8cb87a740ed3 Mon Sep 17 00:00:00 2001 From: Montana Date: Mon, 7 Jan 2019 16:11:07 -0500 Subject: [PATCH 04/14] Add new permission --- atst/domain/audit_log.py | 2 +- atst/domain/roles.py | 3 +++ atst/models/permissions.py | 1 + atst/routes/workspaces/index.py | 5 ++++- script/seed_roles.py | 1 + templates/navigation/workspace_navigation.html | 2 +- 6 files changed, 11 insertions(+), 3 deletions(-) mode change 100644 => 100755 script/seed_roles.py diff --git a/atst/domain/audit_log.py b/atst/domain/audit_log.py index b40dc304..5c2835b6 100644 --- a/atst/domain/audit_log.py +++ b/atst/domain/audit_log.py @@ -37,7 +37,7 @@ class AuditLog(object): @classmethod def get_workspace_events(cls, user, workspace_id, pagination_opts): Authorization.check_atat_permission( - user, Permissions.VIEW_AUDIT_LOG, "view audit log" + user, Permissions.VIEW_WORKSPACE_AUDIT_LOG, "view audit log" ) return AuditEventQuery.get_ws_events(workspace_id, pagination_opts) diff --git a/atst/domain/roles.py b/atst/domain/roles.py index 91137cbc..5f7e1ebb 100644 --- a/atst/domain/roles.py +++ b/atst/domain/roles.py @@ -44,6 +44,7 @@ ATAT_ROLES = [ Permissions.ADD_TAG_TO_WORKSPACE, Permissions.REMOVE_TAG_FROM_WORKSPACE, Permissions.VIEW_AUDIT_LOG, + Permissions.VIEW_WORKSPACE_AUDIT_LOG, ], }, { @@ -84,6 +85,7 @@ WORKSPACE_ROLES = [ Permissions.DEACTIVATE_ENVIRONMENT_IN_APPLICATION, Permissions.VIEW_ENVIRONMENT_IN_APPLICATION, Permissions.RENAME_ENVIRONMENT_IN_APPLICATION, + Permissions.VIEW_WORKSPACE_AUDIT_LOG, ], }, { @@ -111,6 +113,7 @@ WORKSPACE_ROLES = [ Permissions.DEACTIVATE_ENVIRONMENT_IN_APPLICATION, Permissions.VIEW_ENVIRONMENT_IN_APPLICATION, Permissions.RENAME_ENVIRONMENT_IN_APPLICATION, + Permissions.VIEW_WORKSPACE_AUDIT_LOG, ], }, { diff --git a/atst/models/permissions.py b/atst/models/permissions.py index f7adc406..231d65a2 100644 --- a/atst/models/permissions.py +++ b/atst/models/permissions.py @@ -1,5 +1,6 @@ class Permissions(object): VIEW_AUDIT_LOG = "view_audit_log" + VIEW_WORKSPACE_AUDIT_LOG = "view_workspace_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/routes/workspaces/index.py b/atst/routes/workspaces/index.py index 01e7b917..e89fe644 100644 --- a/atst/routes/workspaces/index.py +++ b/atst/routes/workspaces/index.py @@ -88,7 +88,10 @@ def workspace_reports(workspace_id): def workspace_activity(workspace_id): workspace = Workspaces.get(g.current_user, workspace_id) Authorization.check_workspace_permission( - g.current_user, workspace, Permissions.VIEW_AUDIT_LOG, "view workspace reports" + g.current_user, + workspace, + Permissions.VIEW_WORKSPACE_AUDIT_LOG, + "view workspace reports", ) pagination_opts = Paginator.get_pagination_opts(http_request) audit_events = AuditLog.get_workspace_events( diff --git a/script/seed_roles.py b/script/seed_roles.py old mode 100644 new mode 100755 index d1d7298b..cfe0337f --- a/script/seed_roles.py +++ b/script/seed_roles.py @@ -1,3 +1,4 @@ +#! .venv/bin/python # Add root project dir to the python path import os import sys diff --git a/templates/navigation/workspace_navigation.html b/templates/navigation/workspace_navigation.html index d8c656a4..9ef4dd86 100644 --- a/templates/navigation/workspace_navigation.html +++ b/templates/navigation/workspace_navigation.html @@ -56,7 +56,7 @@ ) }} {% endif %} - {% if user_can(permissions.VIEW_AUDIT_LOG) %} + {% if user_can(permissions.VIEW_WORKSPACE_AUDIT_LOG) %} {{ SidenavItem( ("navigation.workspace_navigation.activity_log" | translate), href=url_for("workspaces.workspace_activity", workspace_id=workspace.id), From 31d85be949687756daab12667ac2b14af9fd6c92 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 8 Jan 2019 09:41:16 -0500 Subject: [PATCH 05/14] Update to use workspace permissions --- atst/domain/audit_log.py | 11 +++++++---- atst/routes/workspaces/index.py | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/atst/domain/audit_log.py b/atst/domain/audit_log.py index 5c2835b6..b15af1c3 100644 --- a/atst/domain/audit_log.py +++ b/atst/domain/audit_log.py @@ -35,11 +35,14 @@ class AuditLog(object): return AuditEventQuery.get_all(pagination_opts) @classmethod - def get_workspace_events(cls, user, workspace_id, pagination_opts): - Authorization.check_atat_permission( - user, Permissions.VIEW_WORKSPACE_AUDIT_LOG, "view audit log" + def get_workspace_events(cls, user, workspace, pagination_opts=None): + Authorization.check_workspace_permission( + user, + workspace, + Permissions.VIEW_WORKSPACE_AUDIT_LOG, + "view workspace audit log", ) - return AuditEventQuery.get_ws_events(workspace_id, pagination_opts) + return AuditEventQuery.get_ws_events(workspace.id, pagination_opts) @classmethod def get_by_resource(cls, resource_id): diff --git a/atst/routes/workspaces/index.py b/atst/routes/workspaces/index.py index e89fe644..d8ba7159 100644 --- a/atst/routes/workspaces/index.py +++ b/atst/routes/workspaces/index.py @@ -95,7 +95,7 @@ def workspace_activity(workspace_id): ) pagination_opts = Paginator.get_pagination_opts(http_request) audit_events = AuditLog.get_workspace_events( - g.current_user, workspace_id, pagination_opts + g.current_user, workspace, pagination_opts ) return render_template( From 466f2565b5a31160659207080d2728267837763c Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 8 Jan 2019 10:50:27 -0500 Subject: [PATCH 06/14] Add tests --- atst/domain/audit_log.py | 7 ++- tests/domain/test_audit_log.py | 71 +++++++++++++++++++++++- tests/routes/workspaces/test_projects.py | 54 ++++++++++++++++++ 3 files changed, 128 insertions(+), 4 deletions(-) diff --git a/atst/domain/audit_log.py b/atst/domain/audit_log.py index b15af1c3..41d931fa 100644 --- a/atst/domain/audit_log.py +++ b/atst/domain/audit_log.py @@ -24,8 +24,8 @@ class AuditEventQuery(Query): class AuditLog(object): @classmethod - def log_system_event(cls, resource, action): - return cls._log(resource=resource, action=action) + def log_system_event(cls, resource, action, workspace=None): + return cls._log(resource=resource, action=action, workspace=workspace) @classmethod def get_all_events(cls, user, pagination_opts=None): @@ -58,9 +58,10 @@ class AuditLog(object): return type(resource).__name__.lower() @classmethod - def _log(cls, user=None, workspace_id=None, resource=None, action=None): + def _log(cls, user=None, workspace=None, resource=None, action=None): resource_id = resource.id if resource else None resource_type = cls._resource_type(resource) if resource else None + workspace_id = workspace.id if workspace else None audit_event = AuditEventQuery.create( user=user, diff --git a/tests/domain/test_audit_log.py b/tests/domain/test_audit_log.py index 3e000dde..dcda0eab 100644 --- a/tests/domain/test_audit_log.py +++ b/tests/domain/test_audit_log.py @@ -2,7 +2,14 @@ import pytest from atst.domain.audit_log import AuditLog from atst.domain.exceptions import UnauthorizedError -from tests.factories import UserFactory +from atst.domain.roles import Roles +from atst.models.workspace_role import Status as WorkspaceRoleStatus +from tests.factories import ( + UserFactory, + WorkspaceFactory, + WorkspaceRoleFactory, + ProjectFactory, +) @pytest.fixture(scope="function") @@ -31,3 +38,65 @@ def test_paginate_audit_log(ccpo): events = AuditLog.get_all_events(ccpo, pagination_opts={"per_page": 25, "page": 2}) assert len(events) == 25 + + +def test_ccpo_can_view_ws_audit_log(ccpo): + workspace = WorkspaceFactory.create() + AuditLog.get_workspace_events(ccpo, workspace) + + +def test_ws_admin_can_view_ws_audit_log(): + workspace = WorkspaceFactory.create() + admin = UserFactory.create() + WorkspaceRoleFactory.create( + workspace=workspace, + user=admin, + role=Roles.get("admin"), + status=WorkspaceRoleStatus.ACTIVE, + ) + AuditLog.get_workspace_events(admin, workspace) + + +def test_ws_owner_can_view_ws_audit_log(): + workspace = WorkspaceFactory.create() + AuditLog.get_workspace_events(workspace.owner, workspace) + + +def test_other_users_cannot_view_ws_audit_log(): + with pytest.raises(UnauthorizedError): + workspace = WorkspaceFactory.create() + dev = UserFactory.create() + WorkspaceRoleFactory.create( + workspace=workspace, + user=dev, + role=Roles.get("developer"), + status=WorkspaceRoleStatus.ACTIVE, + ) + AuditLog.get_workspace_events(dev, workspace) + + +def test_paginate_ws_audit_log(): + workspace = WorkspaceFactory.create() + project = ProjectFactory.create(workspace=workspace) + for _ in range(100): + AuditLog.log_system_event( + resource=project, action="create", workspace=workspace + ) + + events = AuditLog.get_workspace_events( + workspace.owner, workspace, pagination_opts={"per_page": 25, "page": 2} + ) + assert len(events) == 25 + + +def test_ws_audit_log_only_includes_current_ws_events(): + owner = UserFactory.create() + workspace = WorkspaceFactory.create(owner=owner) + other_workspace = WorkspaceFactory.create(owner=owner) + # Add some audit events + project_1 = ProjectFactory.create(workspace=workspace) + project_2 = ProjectFactory.create(workspace=other_workspace) + + events = AuditLog.get_workspace_events(workspace.owner, workspace) + for event in events: + assert event.workspace_id == workspace.id diff --git a/tests/routes/workspaces/test_projects.py b/tests/routes/workspaces/test_projects.py index af697c3c..7d8f5ae7 100644 --- a/tests/routes/workspaces/test_projects.py +++ b/tests/routes/workspaces/test_projects.py @@ -1,5 +1,6 @@ from flask import url_for +<<<<<<< HEAD from tests.factories import ( UserFactory, WorkspaceFactory, @@ -8,8 +9,12 @@ from tests.factories import ( EnvironmentFactory, ProjectFactory, ) +======= +from tests.factories import UserFactory, WorkspaceFactory, WorkspaceRoleFactory +>>>>>>> Add tests from atst.domain.projects import Projects from atst.domain.workspaces import Workspaces +from atst.domain.roles import Roles from atst.models.workspace_role import Status as WorkspaceRoleStatus @@ -36,6 +41,55 @@ def test_user_without_permission_has_no_budget_report_link(client, user_session) ) +def test_user_with_permission_has_activity_log_link(client, user_session): + workspace = WorkspaceFactory.create() + ccpo = UserFactory.from_atat_role("ccpo") + admin = UserFactory.create() + WorkspaceRoleFactory.create( + workspace=workspace, + user=admin, + role=Roles.get("admin"), + status=WorkspaceRoleStatus.ACTIVE, + ) + + user_session(workspace.owner) + response = client.get("/workspaces/{}/projects".format(workspace.id)) + assert ( + 'href="/workspaces/{}/activity"'.format(workspace.id).encode() in response.data + ) + + # logs out previous user before creating a new session + user_session(admin) + response = client.get("/workspaces/{}/projects".format(workspace.id)) + assert ( + 'href="/workspaces/{}/activity"'.format(workspace.id).encode() in response.data + ) + + user_session(ccpo) + response = client.get("/workspaces/{}/projects".format(workspace.id)) + assert ( + 'href="/workspaces/{}/activity"'.format(workspace.id).encode() in response.data + ) + + +def test_user_without_permission_has_no_activity_log_link(client, user_session): + workspace = WorkspaceFactory.create() + developer = UserFactory.create() + WorkspaceRoleFactory.create( + workspace=workspace, + user=developer, + role=Roles.get("developer"), + status=WorkspaceRoleStatus.ACTIVE, + ) + + user_session(developer) + response = client.get("/workspaces/{}/projects".format(workspace.id)) + assert ( + 'href="/workspaces/{}/activity"'.format(workspace.id).encode() + not in response.data + ) + + def test_user_with_permission_has_add_project_link(client, user_session): workspace = WorkspaceFactory.create() user_session(workspace.owner) From a48b814263f0721d82b50f5c80ff9553b086ac71 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 8 Jan 2019 15:00:41 -0500 Subject: [PATCH 07/14] Update query to also return events that update the workspace --- atst/domain/audit_log.py | 9 ++++++++- tests/domain/test_audit_log.py | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/atst/domain/audit_log.py b/atst/domain/audit_log.py index 41d931fa..00bbba17 100644 --- a/atst/domain/audit_log.py +++ b/atst/domain/audit_log.py @@ -1,3 +1,5 @@ +from sqlalchemy import or_ + from atst.database import db from atst.domain.common import Query from atst.domain.authz import Authorization, Permissions @@ -16,7 +18,12 @@ class AuditEventQuery(Query): def get_ws_events(cls, workspace_id, pagination_opts): query = ( db.session.query(cls.model) - .filter(cls.model.workspace_id == workspace_id) + .filter( + or_( + cls.model.workspace_id == workspace_id, + cls.model.resource_id == workspace_id, + ) + ) .order_by(cls.model.time_created.desc()) ) return cls.paginate(query, pagination_opts) diff --git a/tests/domain/test_audit_log.py b/tests/domain/test_audit_log.py index dcda0eab..45d382cd 100644 --- a/tests/domain/test_audit_log.py +++ b/tests/domain/test_audit_log.py @@ -99,4 +99,4 @@ def test_ws_audit_log_only_includes_current_ws_events(): events = AuditLog.get_workspace_events(workspace.owner, workspace) for event in events: - assert event.workspace_id == workspace.id + assert event.workspace_id == workspace.id or event.resource_id == workspace.id From 8700ba37d3010a48e4889eb920a966b491381304 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 9 Jan 2019 09:45:22 -0500 Subject: [PATCH 08/14] Create partial for duplicated div for audit log and activity history --- templates/audit_log/audit_log.html | 21 +-------------------- templates/fragments/audit_events_log.html | 21 +++++++++++++++++++++ templates/workspaces/activity/index.html | 21 +-------------------- 3 files changed, 23 insertions(+), 40 deletions(-) create mode 100644 templates/fragments/audit_events_log.html diff --git a/templates/audit_log/audit_log.html b/templates/audit_log/audit_log.html index 73a7443c..37edf908 100644 --- a/templates/audit_log/audit_log.html +++ b/templates/audit_log/audit_log.html @@ -2,24 +2,5 @@ {% from "components/pagination.html" import Pagination %} {% block content %} - -
-
-
-

{{ "audit_log.header_title" | translate }}

-
- -
    - {% for event in audit_events %} -
  • - {% autoescape false %} - {{ event | renderAuditEvent }} - {% endautoescape %} -
    • - {% endfor %} -
-
- - {{ Pagination(audit_events, 'atst.activity_history') }} -
+ {% include "fragments/audit_events_log.html" %} {% endblock %} diff --git a/templates/fragments/audit_events_log.html b/templates/fragments/audit_events_log.html new file mode 100644 index 00000000..2c4bb440 --- /dev/null +++ b/templates/fragments/audit_events_log.html @@ -0,0 +1,21 @@ +{% from "components/pagination.html" import Pagination %} + +
+
+
+

{{ "audit_log.header_title" | translate }}

+
+ +
    + {% for event in audit_events %} +
  • + {% autoescape false %} + {{ event | renderAuditEvent }} + {% endautoescape %} +
    • + {% endfor %} +
+
+ + {{ Pagination(audit_events, 'workspaces.workspace_activity', workspace_id=workspace_id) }} +
diff --git a/templates/workspaces/activity/index.html b/templates/workspaces/activity/index.html index bce6217e..e97d5d37 100644 --- a/templates/workspaces/activity/index.html +++ b/templates/workspaces/activity/index.html @@ -2,24 +2,5 @@ {% from "components/pagination.html" import Pagination %} {% block workspace_content %} - -
-
-
-

{{ "audit_log.header_title" | translate }}

-
- -
    - {% for event in audit_events %} -
  • - {% autoescape false %} - {{ event | renderAuditEvent }} - {% endautoescape %} -
    • - {% endfor %} -
-
- - {{ Pagination(audit_events, 'workspaces.workspace_activity', workspace_id=workspace_id) }} -
+ {% include "fragments/audit_events_log.html" %} {% endblock %} From 52bfb4b7d48982056687a54b079f76d77a6c89d5 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 9 Jan 2019 10:10:42 -0500 Subject: [PATCH 09/14] Add assertions to tests so they are more explicit --- tests/domain/test_audit_log.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/domain/test_audit_log.py b/tests/domain/test_audit_log.py index 45d382cd..a8a5e1c2 100644 --- a/tests/domain/test_audit_log.py +++ b/tests/domain/test_audit_log.py @@ -28,7 +28,8 @@ def test_non_admin_cannot_view_audit_log(developer): def test_ccpo_can_view_audit_log(ccpo): - AuditLog.get_all_events(ccpo) + events = AuditLog.get_all_events(ccpo) + assert len(events) > 0 def test_paginate_audit_log(ccpo): @@ -42,7 +43,8 @@ def test_paginate_audit_log(ccpo): def test_ccpo_can_view_ws_audit_log(ccpo): workspace = WorkspaceFactory.create() - AuditLog.get_workspace_events(ccpo, workspace) + events = AuditLog.get_workspace_events(ccpo, workspace) + assert len(events) > 0 def test_ws_admin_can_view_ws_audit_log(): @@ -54,12 +56,14 @@ def test_ws_admin_can_view_ws_audit_log(): role=Roles.get("admin"), status=WorkspaceRoleStatus.ACTIVE, ) - AuditLog.get_workspace_events(admin, workspace) + events = AuditLog.get_workspace_events(admin, workspace) + assert len(events) > 0 def test_ws_owner_can_view_ws_audit_log(): workspace = WorkspaceFactory.create() - AuditLog.get_workspace_events(workspace.owner, workspace) + events = AuditLog.get_workspace_events(workspace.owner, workspace) + assert len(events) > 0 def test_other_users_cannot_view_ws_audit_log(): @@ -100,3 +104,7 @@ def test_ws_audit_log_only_includes_current_ws_events(): events = AuditLog.get_workspace_events(workspace.owner, workspace) for event in events: assert event.workspace_id == workspace.id or event.resource_id == workspace.id + assert ( + not event.workspace_id == other_workspace.id + or event.resource_id == other_workspace.id + ) From 079d3c4e9cdf9d298e7772d66b394bee51746dcf Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 9 Jan 2019 10:13:18 -0500 Subject: [PATCH 10/14] Remove duplicated authorization check from the route --- atst/routes/workspaces/index.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/atst/routes/workspaces/index.py b/atst/routes/workspaces/index.py index d8ba7159..063f66f5 100644 --- a/atst/routes/workspaces/index.py +++ b/atst/routes/workspaces/index.py @@ -87,12 +87,6 @@ def workspace_reports(workspace_id): @workspaces_bp.route("/workspaces//activity") def workspace_activity(workspace_id): workspace = Workspaces.get(g.current_user, workspace_id) - Authorization.check_workspace_permission( - g.current_user, - workspace, - Permissions.VIEW_WORKSPACE_AUDIT_LOG, - "view workspace reports", - ) pagination_opts = Paginator.get_pagination_opts(http_request) audit_events = AuditLog.get_workspace_events( g.current_user, workspace, pagination_opts From 8e8adb8c6ab355dd69fdc335ee60e748b380294c Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 9 Jan 2019 10:48:15 -0500 Subject: [PATCH 11/14] Disable next and last links in pagination if there is only 1 page --- templates/components/pagination.html | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/templates/components/pagination.html b/templates/components/pagination.html index fe360a9f..6874d565 100644 --- a/templates/components/pagination.html +++ b/templates/components/pagination.html @@ -24,8 +24,14 @@ {% for i in range(1, max_page + 1) %} {{ Page(pagination, route, i, workspace_id=workspace_id) }} {% endfor %} - {{ Page(pagination, route, pagination.page + 1, label="next", workspace_id=workspace_id) }} - {{ Page(pagination, route, pagination.pages, label="last", workspace_id=workspace_id) }} + {% if pagination.per_page >= pagination.total %} + {{ Page(pagination, route, pagination.page + 1, label="next", disabled=True, workspace_id=workspace_id) }} + {{ Page(pagination, route, pagination.pages, label="last", disabled=True, workspace_id=workspace_id) }} + {% else %} + {{ Page(pagination, route, pagination.page + 1, label="next", workspace_id=workspace_id) }} + {{ Page(pagination, route, pagination.pages, label="last", workspace_id=workspace_id) }} + {% endif %} + {% elif pagination.page == pagination.pages %} {{ Page(pagination, route, 1, label="first", workspace_id=workspace_id) }} {{ Page(pagination, route, pagination.page - 1, label="prev", workspace_id=workspace_id) }} From 16febc2a77447f37a06aab75206f3ce171a231c8 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 9 Jan 2019 10:53:07 -0500 Subject: [PATCH 12/14] Move pagination macro out of audit log fragment --- templates/audit_log/audit_log.html | 5 +++- templates/fragments/audit_events_log.html | 32 ++++++++++------------- templates/workspaces/activity/index.html | 5 +++- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/templates/audit_log/audit_log.html b/templates/audit_log/audit_log.html index 37edf908..6c2cf5c4 100644 --- a/templates/audit_log/audit_log.html +++ b/templates/audit_log/audit_log.html @@ -2,5 +2,8 @@ {% from "components/pagination.html" import Pagination %} {% block content %} - {% include "fragments/audit_events_log.html" %} +
+ {% include "fragments/audit_events_log.html" %} + {{ Pagination(audit_events, 'atst.activity_history')}} +
{% endblock %} diff --git a/templates/fragments/audit_events_log.html b/templates/fragments/audit_events_log.html index 2c4bb440..4c9de82a 100644 --- a/templates/fragments/audit_events_log.html +++ b/templates/fragments/audit_events_log.html @@ -1,21 +1,17 @@ {% from "components/pagination.html" import Pagination %} -
-
-
-

{{ "audit_log.header_title" | translate }}

-
+
+
+

{{ "audit_log.header_title" | translate }}

+
-
    - {% for event in audit_events %} -
  • - {% autoescape false %} - {{ event | renderAuditEvent }} - {% endautoescape %} -
    • - {% endfor %} -
-
- - {{ Pagination(audit_events, 'workspaces.workspace_activity', workspace_id=workspace_id) }} -
+
    + {% for event in audit_events %} +
  • + {% autoescape false %} + {{ event | renderAuditEvent }} + {% endautoescape %} +
    • + {% endfor %} +
+ diff --git a/templates/workspaces/activity/index.html b/templates/workspaces/activity/index.html index e97d5d37..9c072af1 100644 --- a/templates/workspaces/activity/index.html +++ b/templates/workspaces/activity/index.html @@ -2,5 +2,8 @@ {% from "components/pagination.html" import Pagination %} {% block workspace_content %} - {% include "fragments/audit_events_log.html" %} +
+ {% include "fragments/audit_events_log.html" %} + {{ Pagination(audit_events, 'workspaces.workspace_activity', workspace_id=workspace_id) }} +
{% endblock %} From eff797d21bcf5f858c1345f63667c212de6291f1 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 9 Jan 2019 11:07:34 -0500 Subject: [PATCH 13/14] Refactor pagination macro --- templates/components/pagination.html | 29 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/templates/components/pagination.html b/templates/components/pagination.html index 6874d565..d7674d66 100644 --- a/templates/components/pagination.html +++ b/templates/components/pagination.html @@ -18,37 +18,36 @@ {%- endmacro %} From a9634edb95e33596d3c2facac78cac54def261fa Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 9 Jan 2019 13:31:11 -0500 Subject: [PATCH 14/14] Formatting fix from rebase --- tests/routes/workspaces/test_projects.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/routes/workspaces/test_projects.py b/tests/routes/workspaces/test_projects.py index 7d8f5ae7..a442e9fb 100644 --- a/tests/routes/workspaces/test_projects.py +++ b/tests/routes/workspaces/test_projects.py @@ -1,6 +1,5 @@ from flask import url_for -<<<<<<< HEAD from tests.factories import ( UserFactory, WorkspaceFactory, @@ -9,9 +8,7 @@ from tests.factories import ( EnvironmentFactory, ProjectFactory, ) -======= -from tests.factories import UserFactory, WorkspaceFactory, WorkspaceRoleFactory ->>>>>>> Add tests + from atst.domain.projects import Projects from atst.domain.workspaces import Workspaces from atst.domain.roles import Roles