From 199c249f116d72820a467ea3908f286245cec950 Mon Sep 17 00:00:00 2001 From: shrouxm Date: Mon, 14 Oct 2024 17:10:56 -0700 Subject: [PATCH 1/3] cleanup: remove audit logs app --- terraso_backend/apps/audit_logs/api.py | 47 ------- .../0002_delete_log.py} | 17 ++- terraso_backend/apps/audit_logs/models.py | 46 ------ terraso_backend/apps/audit_logs/services.py | 131 ------------------ .../core/management/commands/harddelete.py | 2 - .../apps/graphql/schema/audit_logs.py | 71 ---------- .../apps/graphql/schema/commons.py | 17 +-- .../apps/graphql/schema/schema.graphql | 75 +--------- terraso_backend/apps/graphql/schema/schema.py | 2 - terraso_backend/apps/graphql/schema/sites.py | 39 ------ .../apps/project_management/apps.py | 3 - .../project_management/graphql/projects.py | 35 ----- .../apps/project_management/signals.py | 75 ---------- terraso_backend/config/settings.py | 3 - terraso_backend/tests/audit_logs/__init__.py | 0 .../tests/audit_logs/test_services.py | 78 ----------- terraso_backend/tests/graphql/conftest.py | 5 - .../tests/graphql/mutations/test_projects.py | 117 ---------------- .../tests/graphql/mutations/test_sites.py | 49 ------- .../tests/graphql/test_audit_log.py | 108 --------------- 20 files changed, 23 insertions(+), 897 deletions(-) delete mode 100644 terraso_backend/apps/audit_logs/api.py rename terraso_backend/apps/audit_logs/{admin.py => migrations/0002_delete_log.py} (71%) delete mode 100644 terraso_backend/apps/audit_logs/services.py delete mode 100644 terraso_backend/apps/graphql/schema/audit_logs.py delete mode 100644 terraso_backend/apps/project_management/signals.py delete mode 100644 terraso_backend/tests/audit_logs/__init__.py delete mode 100644 terraso_backend/tests/audit_logs/test_services.py delete mode 100644 terraso_backend/tests/graphql/test_audit_log.py diff --git a/terraso_backend/apps/audit_logs/api.py b/terraso_backend/apps/audit_logs/api.py deleted file mode 100644 index a46deb485..000000000 --- a/terraso_backend/apps/audit_logs/api.py +++ /dev/null @@ -1,47 +0,0 @@ -# Copyright © 2023 Technology Matters -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU Affero General Public License as published -# by the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU Affero General Public License for more details. -# -# You should have received a copy of the GNU Affero General Public License -# along with this program. If not, see https://www.gnu.org/licenses/. - -from datetime import datetime -from typing import Dict, Literal, Protocol - -from apps.core.models import User - -from . import models -from .models import Events - -# KeyValue represents a key-value pair -KeyValue = Dict[str, object | str | int | datetime] - -ACTIONS = Literal[Events.CREATE, Events.READ, Events.CHANGE, Events.DELETE] - -CREATE = models.Events.CREATE -READ = models.Events.READ -CHANGE = models.Events.CHANGE -DELETE = models.Events.DELETE - - -class AuditLog(Protocol): - """ - AuditLogProtocol is a protocol that defines the interface for the audit log - """ - - def log( - self, - user: User, - action: ACTIONS, - resource: object, - metadata: KeyValue, - client_time: datetime, - ) -> None: ... diff --git a/terraso_backend/apps/audit_logs/admin.py b/terraso_backend/apps/audit_logs/migrations/0002_delete_log.py similarity index 71% rename from terraso_backend/apps/audit_logs/admin.py rename to terraso_backend/apps/audit_logs/migrations/0002_delete_log.py index b4ced0b52..e6d9b8ab0 100644 --- a/terraso_backend/apps/audit_logs/admin.py +++ b/terraso_backend/apps/audit_logs/migrations/0002_delete_log.py @@ -13,8 +13,19 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see https://www.gnu.org/licenses/. -from django.contrib import admin +# Generated by Django 5.1.1 on 2024-10-15 00:10 -from .models import Log +from django.db import migrations -admin.site.register(Log) + +class Migration(migrations.Migration): + + dependencies = [ + ("audit_logs", "0001_initial"), + ] + + operations = [ + migrations.DeleteModel( + name="Log", + ), + ] diff --git a/terraso_backend/apps/audit_logs/models.py b/terraso_backend/apps/audit_logs/models.py index 44e0a80a4..959587a89 100644 --- a/terraso_backend/apps/audit_logs/models.py +++ b/terraso_backend/apps/audit_logs/models.py @@ -14,11 +14,6 @@ # along with this program. If not, see https://www.gnu.org/licenses/. from enum import Enum -from typing import Optional - -from django.conf import settings -from django.contrib.contenttypes.fields import GenericForeignKey -from django.db import models class Events(Enum): @@ -30,44 +25,3 @@ class Events(Enum): @classmethod def choices(cls): return [(key.value, key.name) for key in cls] - - -class Log(models.Model): - """ - Log model for audits logs - """ - - timestamp = models.DateTimeField(auto_now_add=True) - client_timestamp = models.DateTimeField() - user = models.ForeignKey( - settings.AUTH_USER_MODEL, on_delete=models.PROTECT, verbose_name="user" - ) - user_human_readable = models.CharField(max_length=255) - event = models.CharField(max_length=50, choices=Events.choices(), default=Events.CREATE) - - resource_id = models.UUIDField() - resource_content_type = models.ForeignKey( - "contenttypes.ContentType", - on_delete=models.CASCADE, - verbose_name="content type", - blank=True, - null=True, - ) - resource_object = GenericForeignKey("resource_content_type", "resource_id") - resource_json_repr = models.JSONField() - resource_human_readable = models.CharField(max_length=255) - - metadata = models.JSONField(default=dict) - - def __str__(self): - return str(self.client_timestamp) + " - " + str(self.metadata) - - def get_string(self, template: Optional[str] = None) -> str: - if template is None: - return str(self) - return template.format(**self.metadata) - - class Meta: - indexes = [ - models.Index(fields=["resource_content_type", "resource_id"]), - ] diff --git a/terraso_backend/apps/audit_logs/services.py b/terraso_backend/apps/audit_logs/services.py deleted file mode 100644 index 2a693c2f2..000000000 --- a/terraso_backend/apps/audit_logs/services.py +++ /dev/null @@ -1,131 +0,0 @@ -# Copyright © 2023 Technology Matters -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU Affero General Public License as published -# by the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU Affero General Public License for more details. -# -# You should have received a copy of the GNU Affero General Public License -# along with this program. If not, see https://www.gnu.org/licenses/. - -import typing -from datetime import datetime -from enum import Enum - -from django.conf import settings -from django.contrib.contenttypes.models import ContentType -from django.core.paginator import Paginator -from django.db import transaction -from django.db.models.query import QuerySet - -from apps.core.models import User - -from . import api, models - -TEMPLATE = "{client_time} - {user} {action} {resource}" - - -class _AuditLogService: - """ - AuditLogService implements the AuditLog protocol - """ - - def log( - self, - user: User, - action: api.ACTIONS, - resource: object, - metadata: typing.Optional[dict[str, any]] = None, - client_time: typing.Optional[datetime] = None, - ) -> None: - """ - log logs an action performed by a user on a resource - example: - log(user, "create", resource, client_time=1234567890) - :param client_time: - :param metadata: - :param action: - :param user: - :type resource: object - - """ - if not hasattr(user, "id"): - raise ValueError("Invalid user") - - get_user_readable = getattr(user, "human_readable", None) - user_readable = get_user_readable() if callable(get_user_readable) else user.full_name() - - if not isinstance(action, Enum) or not hasattr(models.Events, action.value): - raise ValueError("Invalid action") - - resource_id = resource.id if hasattr(resource, "id") else None - if resource_id is None: - raise ValueError("Invalid resource") - - get_resource_human_readable = getattr(resource, "human_readable", None) - if callable(get_resource_human_readable): - resource_human_readable = get_resource_human_readable() - else: - resource_human_readable = resource_id - - content_type = ContentType.objects.get_for_model(resource) - resource_obj = resource - - resource_repr = resource.__dict__.__str__() - - if metadata is None: - metadata = {} - - with transaction.atomic(): - log = models.Log( - user=user, - event=action.value, - resource_id=resource_id, - resource_content_type=content_type, - resource_object=resource_obj, - resource_json_repr=resource_repr, - resource_human_readable=str(resource_human_readable), - user_human_readable=str(user_readable), - ) - - if client_time is None: - client_time = datetime.now() - if settings.USE_TZ: - from django.utils.timezone import make_aware - - log.client_timestamp = make_aware(client_time) - else: - log.client_timestamp = client_time - - log.metadata = metadata - log.save() - - -class LogData: - """ - LazyPaginator implements the Paginator protocol - """ - - def __init__(self, data: QuerySet): - self.data = data - - def get_paginator(self, page_size: int = 10): - return Paginator(self.data, page_size) - - def __len__(self): - return len(self.data) - - def __iter__(self): - return iter(self.data) - - -def new_audit_logger() -> api.AuditLog: - """ - new_audit_logger creates a new audit log - """ - return _AuditLogService() diff --git a/terraso_backend/apps/core/management/commands/harddelete.py b/terraso_backend/apps/core/management/commands/harddelete.py index 38e3ed3ec..b1c015f24 100644 --- a/terraso_backend/apps/core/management/commands/harddelete.py +++ b/terraso_backend/apps/core/management/commands/harddelete.py @@ -43,8 +43,6 @@ def all_objects(cutoff_date): app_models = apps.get_models() objects = [] for model in app_models: - if model._meta.label in settings.DELETION_EXCEPTION_LIST: - continue for field in model._meta.fields: if field.name == "deleted_at" and isinstance(field, models.fields.DateTimeField): objects.extend( diff --git a/terraso_backend/apps/graphql/schema/audit_logs.py b/terraso_backend/apps/graphql/schema/audit_logs.py deleted file mode 100644 index e3026f025..000000000 --- a/terraso_backend/apps/graphql/schema/audit_logs.py +++ /dev/null @@ -1,71 +0,0 @@ -# Copyright © 2023 Technology Matters -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU Affero General Public License as published -# by the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU Affero General Public License for more details. -# -# You should have received a copy of the GNU Affero General Public License -# along with this program. If not, see https://www.gnu.org/licenses/. - -import graphene -from django_filters import FilterSet, OrderingFilter -from graphene import relay -from graphene.types.generic import GenericScalar -from graphene_django import DjangoObjectType - -from apps.audit_logs import models - -from .commons import TerrasoConnection - - -class AuditLogFilter(FilterSet): - """ - LogFilter is a filter that filters logs and orders them by client_timestamp - """ - - class Meta: - model = models.Log - fields = [ - "client_timestamp", - "user__id", - "event", - "resource_id", - "resource_content_type__model", - "resource_content_type", - ] - - order_by = OrderingFilter(fields=(("client_timestamp", "client_timestamp"),)) - - -class AuditLogNode(DjangoObjectType): - """ - AuditLogNode is a node that represents an audit log - """ - - id = graphene.ID(source="pk", required=True) - metadata = GenericScalar(required=True) - resource_json_repr = GenericScalar(required=True) - resource_content_type = graphene.String(required=True) - - class Meta: - model = models.Log - - fields = ( - "client_timestamp", - "user", - "event", - "resource_id", - ) - - interfaces = (relay.Node,) - connection_class = TerrasoConnection - filterset_class = AuditLogFilter - - def resolve_content_type(self, info): - return self.resource_content_type.model diff --git a/terraso_backend/apps/graphql/schema/commons.py b/terraso_backend/apps/graphql/schema/commons.py index c3a9a5988..f518e2a98 100644 --- a/terraso_backend/apps/graphql/schema/commons.py +++ b/terraso_backend/apps/graphql/schema/commons.py @@ -24,8 +24,6 @@ from graphene.types.generic import GenericScalar from graphql import get_nullable_type -from apps.audit_logs import api as audit_log_api -from apps.audit_logs import services as audit_log_services from apps.core.formatters import from_camel_to_snake_case from apps.graphql.exceptions import ( GraphQLNotAllowedException, @@ -179,18 +177,7 @@ def not_found(cls, model=None, field=None, msg=None): raise super().not_found(msg, field=field, model=model or cls.model_class) -class LoggerMixin: - logger: Optional[audit_log_api.AuditLog] = None - - @classmethod - def get_logger(cls): - """Returns the logger instance.""" - if not cls.logger: - cls.logger = audit_log_services.new_audit_logger() - return cls.logger - - -class BaseWriteMutation(BaseAuthenticatedMutation, LoggerMixin): +class BaseWriteMutation(BaseAuthenticatedMutation): skip_field_validation: Optional[str] = None @classmethod @@ -281,7 +268,7 @@ def remove_null_fields(kwargs, options=[str]): return kwargs -class BaseDeleteMutation(BaseAuthenticatedMutation, LoggerMixin): +class BaseDeleteMutation(BaseAuthenticatedMutation): @classmethod def mutate_and_get_payload(cls, root, info, **kwargs): if "model_instance" in kwargs: diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index 1337dbcd8..c3d5a5ea2 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -75,22 +75,6 @@ type Query { """Ordering""" orderBy: String ): SiteNodeConnection! - auditLogs( - offset: Int - before: String - after: String - first: Int - last: Int - clientTimestamp: DateTime - user_Id: ID - event: AuditLogsLogEventChoices - resourceId: UUID - resourceContentType_Model: String - resourceContentType: ID - - """Ordering""" - orderBy: String - ): AuditLogNodeConnection sharedResource(shareUuid: String!): SharedResourceNode } @@ -1617,58 +1601,6 @@ type ProjectNodeEdge { cursor: String! } -type AuditLogNodeConnection { - """Pagination data for this connection.""" - pageInfo: PageInfo! - - """Contains the nodes in this connection.""" - edges: [AuditLogNodeEdge!]! - totalCount: Int! -} - -"""A Relay edge containing a `AuditLogNode` and its cursor.""" -type AuditLogNodeEdge { - """The item at the end of the edge""" - node: AuditLogNode! - - """A cursor for use in pagination""" - cursor: String! -} - -"""AuditLogNode is a node that represents an audit log""" -type AuditLogNode implements Node { - clientTimestamp: DateTime! - user: UserNode! - event: AuditLogsLogEventChoices! - resourceId: UUID! - id: ID! - metadata: GenericScalar! - resourceJsonRepr: GenericScalar! - resourceContentType: String! -} - -"""An enumeration.""" -enum AuditLogsLogEventChoices { - """CREATE""" - CREATE - - """READ""" - READ - - """CHANGE""" - CHANGE - - """DELETE""" - DELETE -} - -""" -The `GenericScalar` scalar type represents a generic -GraphQL scalar value that could be: -String, Boolean, Int, Float, List or Object. -""" -scalar GenericScalar - type Mutations { addGroup(input: GroupAddMutationInput!): GroupAddMutationPayload! addLandscape(input: LandscapeAddMutationInput!): LandscapeAddMutationPayload! @@ -1733,6 +1665,13 @@ type GroupAddMutationPayload { clientMutationId: String } +""" +The `GenericScalar` scalar type represents a generic +GraphQL scalar value that could be: +String, Boolean, Int, Float, List or Object. +""" +scalar GenericScalar + input GroupAddMutationInput { name: String! description: String diff --git a/terraso_backend/apps/graphql/schema/schema.py b/terraso_backend/apps/graphql/schema/schema.py index 1dc8e6fdf..8f02ab1de 100644 --- a/terraso_backend/apps/graphql/schema/schema.py +++ b/terraso_backend/apps/graphql/schema/schema.py @@ -43,7 +43,6 @@ ) from apps.soil_id.graphql.soil_id.endpoints import soil_id -from .audit_logs import AuditLogNode from .commons import TerrasoRelayNode from .data_entries import ( DataEntryAddMutation, @@ -140,7 +139,6 @@ class Query(graphene.ObjectType): projects = DjangoFilterConnectionField(ProjectNode, required=True) site = TerrasoRelayNode.Field(SiteNode) sites = DjangoFilterConnectionField(SiteNode, required=True) - audit_logs = DjangoFilterConnectionField(AuditLogNode) shared_resource = SharedResourceRelayNode.Field() soil_id = soil_id from .shared_resources import resolve_shared_resource diff --git a/terraso_backend/apps/graphql/schema/sites.py b/terraso_backend/apps/graphql/schema/sites.py index dc6a50f29..465fc310a 100644 --- a/terraso_backend/apps/graphql/schema/sites.py +++ b/terraso_backend/apps/graphql/schema/sites.py @@ -21,7 +21,6 @@ from graphene_django import DjangoObjectType from graphene_django.filter import TypedFilter -from apps.audit_logs import api as audit_log_api from apps.project_management.graphql.projects import ProjectNode from apps.project_management.models import Project, Site, sites from apps.project_management.permission_rules import Context @@ -121,7 +120,6 @@ class Input: @classmethod def mutate_and_get_payload(cls, root, info, create_soil_data=True, **kwargs): - log = cls.get_logger() user = info.context.user if not check_site_permission(user, SiteAction.CREATE, Context()): @@ -159,13 +157,6 @@ def mutate_and_get_payload(cls, root, info, create_soil_data=True, **kwargs): } if kwargs.get("project_id", None): metadata["project_id"] = kwargs["project_id"] - log.log( - user=user, - action=audit_log_api.CREATE, - resource=site, - metadata=metadata, - client_time=client_time, - ) return result @@ -200,7 +191,6 @@ class Input: @classmethod @transaction.atomic def mutate_and_get_payload(cls, root, info, **kwargs): - log = cls.get_logger() user = info.context.user site = cls.get_or_throw(Site, "id", kwargs["id"]) @@ -251,14 +241,6 @@ def mutate_and_get_payload(cls, root, info, **kwargs): if project_id is not None: metadata["project_id"] = str(project.id) - log.log( - user=user, - action=audit_log_api.CHANGE, - resource=site, - metadata=metadata, - client_time=client_time, - ) - return result @@ -273,7 +255,6 @@ class Input: @classmethod @transaction.atomic def mutate_and_get_payload(cls, root, info, **kwargs): - log = cls.get_logger() user = info.context.user site_id = kwargs["id"] site = cls.get_or_throw(Site, "id", site_id) @@ -281,14 +262,6 @@ def mutate_and_get_payload(cls, root, info, **kwargs): cls.not_allowed(MutationTypes.DELETE) result = super().mutate_and_get_payload(root, info, **kwargs) - metadata = {"project_id": str(site.project.id)} if site.project else {} - log.log( - user=user, - action=audit_log_api.DELETE, - resource=site, - metadata=metadata, - client_time=datetime.now(), - ) return result @@ -311,7 +284,6 @@ class Input: @classmethod def mutate_and_get_payload(cls, root, info, **kwargs): - log = cls.get_logger() user = info.context.user project = cls.get_or_throw(Project, "project_id", kwargs["project_id"]) @@ -345,17 +317,6 @@ def mutate_and_get_payload(cls, root, info, **kwargs): Site.bulk_change_project(to_change, project) - log.log( - user=user, - action=audit_log_api.CHANGE, - resource=project, - metadata={ - "project_id": str(project.id), - "transfered_sites": [str(site.id) for site in to_change], - }, - client_time=datetime.now(), - ) - return SiteTransferMutation( project=project, updated=[ diff --git a/terraso_backend/apps/project_management/apps.py b/terraso_backend/apps/project_management/apps.py index 54e4c85bb..94c0319d4 100644 --- a/terraso_backend/apps/project_management/apps.py +++ b/terraso_backend/apps/project_management/apps.py @@ -18,6 +18,3 @@ class ProjectManagementConfig(AppConfig): default_auto_field = "django.db.models.BigAutoField" name = "apps.project_management" - - def ready(self): - from .signals import handle_membership_added, handle_membership_updated # noqa diff --git a/terraso_backend/apps/project_management/graphql/projects.py b/terraso_backend/apps/project_management/graphql/projects.py index d5f461a85..7d876646c 100644 --- a/terraso_backend/apps/project_management/graphql/projects.py +++ b/terraso_backend/apps/project_management/graphql/projects.py @@ -23,7 +23,6 @@ from graphene_django import DjangoObjectType from graphene_django.filter import DjangoFilterConnectionField, TypedFilter -from apps.audit_logs import api as log_api from apps.collaboration.graphql.memberships import ( MembershipListNodeMixin, MembershipNodeMixin, @@ -175,7 +174,6 @@ class Input: @classmethod def mutate_and_get_payload(cls, root, info, create_soil_settings=True, **kwargs): - logger = cls.get_logger() user = info.context.user if not check_project_permission(user, ProjectAction.CREATE, Context()): @@ -192,19 +190,6 @@ def mutate_and_get_payload(cls, root, info, create_soil_settings=True, **kwargs) result.project.mark_seen_by(user) if not client_time: client_time = datetime.now() - action = log_api.CREATE - metadata = { - "name": result.project.name, - "privacy": result.project.privacy, - "description": result.project.description, - } - logger.log( - user=user, - action=action, - resource=result.project, - client_time=client_time, - metadata=metadata, - ) return result @@ -220,7 +205,6 @@ class Input: @classmethod @transaction.atomic def mutate_and_get_payload(cls, root, info, **kwargs): - logger = cls.get_logger() user = info.context.user project_id = kwargs["id"] project = cls.get_or_throw(Project, "id", project_id) @@ -242,16 +226,6 @@ def mutate_and_get_payload(cls, root, info, **kwargs): Site.objects.bulk_update(project_sites, ["project"]) result = super().mutate_and_get_payload(root, info, **kwargs) - logger.log( - user=user, - action=log_api.DELETE, - resource=result.project, - client_time=datetime.now(), - metadata={ - "name": project.name, - "privacy": project.privacy, - }, - ) return result @@ -296,7 +270,6 @@ class Input: @classmethod @transaction.atomic def mutate_and_get_payload(cls, root, info, **kwargs): - logger = cls.get_logger() user = info.context.user project_id = kwargs["id"] project = cls.get_or_throw(Project, "id", project_id) @@ -314,14 +287,6 @@ def mutate_and_get_payload(cls, root, info, **kwargs): if privacy := kwargs.get("privacy"): metadata["privacy"] = privacy.value - logger.log( - user=user, - action=log_api.CHANGE, - resource=project, - client_time=datetime.now(), - metadata=metadata, - ) - return super().mutate_and_get_payload(root, info, **kwargs) diff --git a/terraso_backend/apps/project_management/signals.py b/terraso_backend/apps/project_management/signals.py deleted file mode 100644 index 295c9f6f0..000000000 --- a/terraso_backend/apps/project_management/signals.py +++ /dev/null @@ -1,75 +0,0 @@ -# Copyright © 2023 Technology Matters -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU Affero General Public License as published -# by the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU Affero General Public License for more details. -# -# You should have received a copy of the GNU Affero General Public License -# along with this program. If not, see https://www.gnu.org/licenses/. - -from datetime import datetime - -from django.dispatch import receiver - -from apps.audit_logs import api as audit_log_api -from apps.audit_logs import services -from apps.graphql.signals import ( - membership_added_signal, - membership_deleted_signal, - membership_updated_signal, -) - -audit_logger = services.new_audit_logger() - - -def _handle_membership_log(user, action, membership, client_time): - try: - project = membership.membership_list.project - except Exception: - # No project for membership, do nothing - return - - audit_logger.log( - user=user, - action=action, - resource=membership, - metadata={ - "user_email": membership.user.email, - "user_role": membership.user_role, - "project_id": str(project.id), - }, - client_time=client_time, - ) - - -@receiver(membership_added_signal) -def handle_membership_added(sender, **kwargs): - membership = kwargs["membership"] - user = kwargs["user"] - client_time = datetime.now() - - _handle_membership_log(user, audit_log_api.CREATE, membership, client_time) - - -@receiver(membership_updated_signal) -def handle_membership_updated(sender, **kwargs): - membership = kwargs["membership"] - user = kwargs["user"] - client_time = datetime.now() - - _handle_membership_log(user, audit_log_api.CHANGE, membership, client_time) - - -@receiver(membership_deleted_signal) -def handle_membership_deleted(sender, **kwargs): - membership = kwargs["membership"] - user = kwargs["user"] - client_time = datetime.now() - - _handle_membership_log(user, audit_log_api.DELETE, membership, client_time) diff --git a/terraso_backend/config/settings.py b/terraso_backend/config/settings.py index d42aa0d4e..8708a7a5e 100644 --- a/terraso_backend/config/settings.py +++ b/terraso_backend/config/settings.py @@ -386,9 +386,6 @@ HARD_DELETE_DELETION_GAP = config("HARD_DELETE_DELETION_GAP_DAYS", default="30", cast=config.eval) -# Preserve users to ensure audit logs are readable. -DELETION_EXCEPTION_LIST = ["core.User"] - class JWTProvider(TypedDict): """Type hint to indicate correct config for JWT_EXCHANGE_PROVIDERS""" diff --git a/terraso_backend/tests/audit_logs/__init__.py b/terraso_backend/tests/audit_logs/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/terraso_backend/tests/audit_logs/test_services.py b/terraso_backend/tests/audit_logs/test_services.py deleted file mode 100644 index b36f495f6..000000000 --- a/terraso_backend/tests/audit_logs/test_services.py +++ /dev/null @@ -1,78 +0,0 @@ -# Copyright © 2023 Technology Matters -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU Affero General Public License as published -# by the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU Affero General Public License for more details. -# -# You should have received a copy of the GNU Affero General Public License -# along with this program. If not, see https://www.gnu.org/licenses/. - -import datetime - -from django.test import TestCase - -from apps.audit_logs import api, models, services -from apps.core.models import User - - -class AuditLogServiceTest(TestCase): - def test_create_log(self): - log = services.new_audit_logger() - user = User(email="a@a.com") - user.save() - resource = User(email="b@b.com") - resource.save() - - action = api.CREATE - time = datetime.datetime.now() - metadata = {"some_key": "some_value"} - log.log(user, action, resource, metadata, time) - - result = models.Log.objects.all() - assert len(result) == 1 - assert result[0].user.id == user.id - assert result[0].event == action.CREATE.value - assert result[0].resource_object == resource - assert result[0].user_human_readable == user.full_name() - assert result[0].resource_human_readable == str(resource.id) - assert result[0].metadata["some_key"] == "some_value" - - def test_create_log_invalid_user(self): - log = services.new_audit_logger() - user = object() - resource = User(email="b@b.com") - resource.save() - action = api.CREATE - time = datetime.datetime.now() - metadata = {"some_key": "some_value"} - with self.assertRaises(ValueError): - log.log(user, action, resource, metadata, time) - - def test_create_log_invalid_action(self): - log = services.new_audit_logger() - user = User(email="a@a.com") - user.save() - resource = User(email="b@b.com") - resource.save() - action = "INVALID" - time = datetime.datetime.now() - metadata = {"some_key": "some_value"} - with self.assertRaises(ValueError): - log.log(user, action, resource, metadata, time) - - def test_create_log_invalid_resource(self): - log = services.new_audit_logger() - user = User(email="a@a.com") - user.save() - resource = object() - action = api.CREATE - time = datetime.datetime.now() - metadata = {"some_key": "some_value"} - with self.assertRaises(ValueError): - log.log(user, action, resource, metadata, time) diff --git a/terraso_backend/tests/graphql/conftest.py b/terraso_backend/tests/graphql/conftest.py index 32e74c707..a6edf360f 100644 --- a/terraso_backend/tests/graphql/conftest.py +++ b/terraso_backend/tests/graphql/conftest.py @@ -558,11 +558,6 @@ def story_map_user_memberships_not_registered_approve_tokens( ] -@pytest.fixture -def audit_log_user(): - return mixer.blend(User) - - @pytest.fixture def site_with_depth_intervals(project): site = mixer.blend(Site, project=project) diff --git a/terraso_backend/tests/graphql/mutations/test_projects.py b/terraso_backend/tests/graphql/mutations/test_projects.py index c22affc02..f9f7e4555 100644 --- a/terraso_backend/tests/graphql/mutations/test_projects.py +++ b/terraso_backend/tests/graphql/mutations/test_projects.py @@ -19,8 +19,6 @@ from graphene_django.utils.testing import graphql_query from mixer.backend.django import mixer -from apps.audit_logs.api import CHANGE, CREATE, DELETE -from apps.audit_logs.models import Log from apps.core.models.users import User from apps.project_management.models import Project from apps.project_management.models.sites import Site @@ -66,18 +64,6 @@ def test_create_project_default_values(client, user): assert project.description == "A test project" assert project.soil_settings is not None - logs = Log.objects.all() - assert len(logs) == 1 - log_result = logs[0] - assert log_result.event == CREATE.value - assert log_result.resource_object == project - expected_metadata = { - "name": "testProject", - "privacy": "PRIVATE", - "description": "A test project", - } - assert log_result.metadata == expected_metadata - def test_create_project_values(client, user): client.force_login(user) @@ -103,18 +89,6 @@ def test_create_project_values(client, user): assert project.description == "A test project" assert project.soil_settings is not None - logs = Log.objects.all() - assert len(logs) == 1 - log_result = logs[0] - assert log_result.event == CREATE.value - assert log_result.resource_object == project - expected_metadata = { - "name": "testProject", - "privacy": "PUBLIC", - "description": "A test project", - } - assert log_result.metadata == expected_metadata - DELETE_PROJECT_GRAPHQL = """ mutation($input: ProjectDeleteMutationInput!) { @@ -138,12 +112,6 @@ def test_delete_project(project_with_sites, client, project_manager, project): assert not Project.objects.filter(id=project_with_sites.id).exists() assert not Site.objects.filter(id__in=site_ids).exists() - logs = Log.objects.all() - assert len(logs) == 1 - log_result = logs[0] - assert log_result.event == DELETE.value - assert log_result.resource_object == project - def test_delete_project_user_not_manager(project, client, project_user): input = {"id": str(project.id)} @@ -239,41 +207,6 @@ def test_update_project_user_is_manager(project, client, project_manager): assert content["data"]["updateProject"]["project"]["measurementUnits"] == "ENGLISH" -@pytest.mark.parametrize( - "metadata", - [ - { - "name": "test_name", - "privacy": "PRIVATE", - "description": "A test project", - }, - { - "name": "test_name", - "description": "A test project", - }, - ], -) -def test_update_project_audit_log(metadata, project, client, project_manager): - input = { - "id": str(project.id), - } - input.update(metadata) - client.force_login(project_manager) - - response = graphql_query(UPDATE_PROJECT_GRAPHQL, input_data=input, client=client) - - assert response.status_code == 200 - - logs = Log.objects.all() - assert len(logs) == 1 - log_result = logs[0] - assert log_result.event == CHANGE.value - assert log_result.user_human_readable == project_manager.full_name() - assert log_result.resource_object == project - expected_metadata = metadata - assert log_result.metadata == expected_metadata - - def test_update_project_user_not_manager(project, client, project_user): input = {"id": str(project.id), "name": "test_name", "privacy": "PRIVATE"} client.force_login(project_user) @@ -313,41 +246,6 @@ def test_add_user_to_project(project, project_manager, client): assert project.viewer_memberships.filter(user=user).exists() -def test_add_user_to_project_audit_log(client, project, project_manager, user): - client.force_login(project_manager) - - assert project_manager.id != user.id - - response = graphql_query( - ADD_USER_GRAPHQL, - variables={ - "input": { - "projectId": str(project.id), - "userId": str(user.id), - "role": "VIEWER", - } - }, - client=client, - ) - - assert response.status_code == 200 - - membership = project.viewer_memberships.first() - - logs = Log.objects.all() - assert len(logs) == 1 - log_result = logs[0] - assert log_result.event == CREATE.value - assert log_result.user_human_readable == project_manager.full_name() - assert log_result.resource_object == membership - expected_metadata = { - "user_email": user.email, - "user_role": "VIEWER", - "project_id": str(project.id), - } - assert log_result.metadata == expected_metadata - - def test_add_user_to_project_bad_roles(project, project_manager, client): user = mixer.blend(User) client.force_login(project_manager) @@ -497,21 +395,6 @@ def test_update_project_role_manager(project, project_manager, project_user, cli assert not project.is_viewer(project_user) assert response.status_code == 200 - membership = project.get_membership(user=project_user) - - logs = Log.objects.all() - assert len(logs) == 1 - log_result = logs[0] - assert log_result.event == CHANGE.value - assert log_result.user_human_readable == project_manager.full_name() - assert log_result.resource_object == membership - expected_metadata = { - "user_email": project_user.email, - "user_role": "CONTRIBUTOR", - "project_id": str(project.id), - } - assert log_result.metadata == expected_metadata - def test_update_project_role_not_manager(project, project_user, client): client.force_login(project_user) diff --git a/terraso_backend/tests/graphql/mutations/test_sites.py b/terraso_backend/tests/graphql/mutations/test_sites.py index 7718ed00c..fb4df2aa1 100644 --- a/terraso_backend/tests/graphql/mutations/test_sites.py +++ b/terraso_backend/tests/graphql/mutations/test_sites.py @@ -20,8 +20,6 @@ from mixer.backend.django import mixer from tests.utils import match_json -from apps.audit_logs.api import CHANGE, CREATE, DELETE -from apps.audit_logs.models import Log from apps.core.models import User from apps.project_management.collaboration_roles import ProjectRole from apps.project_management.models import Project, Site @@ -60,14 +58,6 @@ def test_site_creation(client_query, user): assert site.elevation == pytest.approx(site.elevation) assert site.owner == user assert site.privacy == "public" - logs = Log.objects.all() - assert len(logs) == 1 - log_result = logs[0] - assert log_result.event == CREATE.value - assert log_result.user == user - assert log_result.resource_object == site - expected_metadata = {"name": "Test Site", "latitude": 0.0, "longitude": 0.0, "elevation": 0.0} - assert log_result.metadata == expected_metadata def test_site_creation_without_elevation(client_query, user): @@ -84,14 +74,6 @@ def test_site_creation_without_elevation(client_query, user): assert site.elevation is None assert site.owner == user assert site.privacy == "public" - logs = Log.objects.all() - assert len(logs) == 1 - log_result = logs[0] - assert log_result.event == CREATE.value - assert log_result.user == user - assert log_result.resource_object == site - expected_metadata = {"name": "Test Site", "latitude": 0.0, "longitude": 0.0, "elevation": None} - assert log_result.metadata == expected_metadata @pytest.mark.parametrize("project_user_w_role", ["MANAGER", "CONTRIBUTOR"], indirect=True) @@ -105,15 +87,6 @@ def test_site_creation_in_project(client, project_user_w_role, project): id = content["data"]["addSite"]["site"]["id"] site = Site.objects.get(pk=id) assert site.project == project - logs = Log.objects.all() - assert len(logs) == 1 - log_result = logs[0] - assert log_result.event == CREATE.value - assert log_result.resource_object == site - expected_metadata = {"name": "Test Site", "latitude": 0.0, "longitude": 0.0} - assert log_result.metadata["name"] == expected_metadata["name"] - assert log_result.metadata["latitude"] == expected_metadata["latitude"] - assert log_result.metadata["longitude"] == expected_metadata["longitude"] UPDATE_SITE_QUERY = """ @@ -157,13 +130,6 @@ def test_update_site_in_project(client, project, project_manager, site_with_soil assert site.project.id == project.id assert site.privacy == "public" - logs = Log.objects.all() - assert len(logs) == 1 - log_result = logs[0] - assert log_result.event == CHANGE.value - assert log_result.resource_object == site - assert log_result.metadata["project_id"] == str(project.id) - @pytest.mark.parametrize("project_user_w_role", ["CONTRIBUTOR"], indirect=True) def test_update_site_settings_contributor(client, project, project_user_w_role, site): @@ -371,13 +337,6 @@ def test_delete_linked_site(client, linked_site, project_manager): assert response.json()["data"]["deleteSite"]["errors"] is None assert len(Site.objects.filter(id=linked_site.id)) == 0 - logs = Log.objects.all() - assert len(logs) == 1 - log_result = logs[0] - assert log_result.event == DELETE.value - assert log_result.resource_object == linked_site - assert log_result.metadata["project_id"] == str(linked_site.project.id) - @pytest.mark.parametrize("linked_site", ["linked", "MANAGER"], indirect=True) def test_site_transfer_success(linked_site, client, project, project_manager): @@ -393,14 +352,6 @@ def test_site_transfer_success(linked_site, client, project, project_manager): linked_site.refresh_from_db() assert linked_site.project == project - logs = Log.objects.all() - assert len(logs) == 1 - log_result = logs[0] - assert log_result.event == CHANGE.value - assert log_result.resource_object == project - assert log_result.metadata["project_id"] == str(project.id) - assert log_result.metadata["transfered_sites"] == [str(linked_site.id)] - def test_site_transfer_unlinked_site_user_contributor_success(client, user, site, project): project.add_contributor(user) diff --git a/terraso_backend/tests/graphql/test_audit_log.py b/terraso_backend/tests/graphql/test_audit_log.py deleted file mode 100644 index 0b6d60517..000000000 --- a/terraso_backend/tests/graphql/test_audit_log.py +++ /dev/null @@ -1,108 +0,0 @@ -# Copyright © 2023 Technology Matters -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU Affero General Public License as published -# by the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU Affero General Public License for more details. -# -# You should have received a copy of the GNU Affero General Public License -# along with this program. If not, see https://www.gnu.org/licenses/. - -import pytest - -from apps.audit_logs import api, services - -pytestmark = pytest.mark.django_db - - -def test_audit_log_query(client_query, audit_log_user, site): - logger = services.new_audit_logger() - metadata = {"some_key": "some_value"} - logger.log(user=audit_log_user, action=api.CREATE, resource=site, metadata=metadata) - response = client_query( - """ - { - auditLogs { - edges { - node { - clientTimestamp - resourceId - event - metadata - resourceContentType - user { - email - } - - } - } - } - } - """, - ) - expected_metadata = { - "some_key": "some_value", - } - edges = response.json()["data"]["auditLogs"]["edges"] - assert len(edges) == 1 - assert edges[0]["node"]["user"] == {"email": audit_log_user.email} - assert edges[0]["node"]["resourceId"] == str(site.id) - assert edges[0]["node"]["metadata"]["some_key"] == expected_metadata["some_key"] - - logger.log(user=audit_log_user, action=api.CHANGE, resource=site) - response = client_query( - """ - { - auditLogs { - edges { - node { - clientTimestamp - resourceId - event - metadata - resourceContentType - user { - email - } - - } - } - } - } - """, - ) - - edges = response.json()["data"]["auditLogs"]["edges"] - assert len(edges) == 2 - assert edges[0]["node"]["event"] == "CREATE" - - response = client_query( - """ - { - auditLogs(orderBy: "-clientTimestamp") { - edges { - node { - clientTimestamp - resourceId - event - metadata - resourceContentType - user { - email - } - - } - } - } - } - """, - ) - - edges = response.json()["data"]["auditLogs"]["edges"] - assert len(edges) == 2 - assert edges[0]["node"]["event"] == "CHANGE" From d13bdae21e9d3152e248d88965b7e1f1f3da402c Mon Sep 17 00:00:00 2001 From: Paul Schreiber Date: Mon, 14 Oct 2024 21:59:26 -0400 Subject: [PATCH 2/3] docs: fix copyright year --- terraso_backend/apps/audit_logs/migrations/0002_delete_log.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraso_backend/apps/audit_logs/migrations/0002_delete_log.py b/terraso_backend/apps/audit_logs/migrations/0002_delete_log.py index e6d9b8ab0..ce1def3b7 100644 --- a/terraso_backend/apps/audit_logs/migrations/0002_delete_log.py +++ b/terraso_backend/apps/audit_logs/migrations/0002_delete_log.py @@ -1,4 +1,4 @@ -# Copyright © 2023 Technology Matters +# Copyright © 2024 Technology Matters # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU Affero General Public License as published From 5d5d8e7edaa838a6754f64d09a3630257921fbb9 Mon Sep 17 00:00:00 2001 From: shrouxm Date: Tue, 15 Oct 2024 11:59:33 -0700 Subject: [PATCH 3/3] tests: fix tests --- .../tests/core/commands/test_harddelete.py | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/terraso_backend/tests/core/commands/test_harddelete.py b/terraso_backend/tests/core/commands/test_harddelete.py index 38f740f4a..d647cc319 100644 --- a/terraso_backend/tests/core/commands/test_harddelete.py +++ b/terraso_backend/tests/core/commands/test_harddelete.py @@ -23,7 +23,7 @@ pytestmark = pytest.mark.django_db -@pytest.mark.parametrize("model", [Group, DataEntry]) +@pytest.mark.parametrize("model", [User, Group, DataEntry]) def test_delete_model_deleted(model, delete_date): obj = mixer.blend(model) obj.delete() @@ -35,18 +35,6 @@ def test_delete_model_deleted(model, delete_date): ), "Model should be deleted" -@pytest.mark.parametrize("model", [User]) -def test_delete_user_not_deleted(model, delete_date): - obj = mixer.blend(model) - obj.delete() - obj.deleted_at = delete_date - obj.save(keep_deleted=True) - call_command("harddelete") - assert ( - model.objects.all(force_visibility=True).filter(id=obj.id).exists() - ), "Model should not be deleted" - - @pytest.mark.parametrize("model", [User, Group, DataEntry]) def test_delete_model_not_deleted(model, no_delete_date): obj = mixer.blend(model)