Skip to content

Organizations: show audit logs #8588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Nov 8, 2021
Merged
16 changes: 15 additions & 1 deletion readthedocs/audit/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,18 @@ class UserSecurityLogFilter(FilterSet):

class Meta:
model = AuditLog
fields = ['ip', 'project', 'action']
fields = []
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this wasn't needed, as these values are defined as attributes, but you still need to declare fields



class OrganizationSecurityLogFilter(UserSecurityLogFilter):

action = ChoiceFilter(
field_name='action',
lookup_expr='exact',
choices=[
(AuditLog.AUTHN, _('Authentication success')),
(AuditLog.AUTHN_FAILURE, _('Authentication failure')),
(AuditLog.PAGEVIEW, _('Page view')),
],
)
user = CharFilter(field_name='log_user_username', lookup_expr='exact')
18 changes: 18 additions & 0 deletions readthedocs/audit/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,5 +190,23 @@ def save(self, **kwargs):
self.log_organization_slug = organization.slug
super().save(**kwargs)

def auth_backend_display(self):
"""
Get a string representation for backends that aren't part of the normal login.
.. note::
The backends listed here are implemented on .com only.
"""
backend = self.auth_backend or ''
backend_displays = {
'TemporaryAccessTokenBackend': _('shared link'),
'TemporaryAccessPasswordBackend': _('shared password'),
}
for name, display in backend_displays.items():
if name in backend:
return display
return ''

def __str__(self):
return self.action
83 changes: 83 additions & 0 deletions readthedocs/audit/templates/audit/list_logs.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
{% load i18n %}

{% comment %}
If `omit_user` is given, the username attached to the log isn't shown.
This is useful when listing logs for the same user.
{% endcomment %}

<div class="module-list">
<div class="module-list-wrapper">
<ul>
{% for log in object_list %}
<li class="module-item">
{% if log.action == AuditLog.AUTHN %}
{% if omit_user %}
{% blocktrans trimmed with action=log.action method=log.auth_backend_display %}
<a href="?action={{ action }}" title="{{ method }}">Authenticated</a>
{% endblocktrans %}
{% elif log.log_user_id %}
{% blocktrans trimmed with action=log.action user=log.log_user_username method=log.auth_backend_display %}
<a href="?user={{ user }}">
<code>{{ user }}</code>
</a>
<a href="?action={{ action }}" title="{{ method }}">authenticated</a>
{% endblocktrans %}
{% else %}
{% blocktrans trimmed with action=log.action method=log.auth_backend_display %}
User <a href="?action={{ action }}" title="{{ method }}">authenticated</a>
{% endblocktrans %}
{% endif %}
{% elif log.action == AuditLog.AUTHN_FAILURE %}
{% blocktrans trimmed with action=log.action method=log.auth_backend_display %}
<a href="?action={{ action }}" title="{{ method }}">Authentication attempt</a>
{% endblocktrans %}
{% elif log.action == AuditLog.PAGEVIEW %}
{% if log.log_user_id %}
{% blocktrans trimmed with action=log.action user=log.log_user_username path=log.resource %}
<a href="?user={{ user }}">
<code>{{ user }}</code>
</a>
<a href="?action={{ action }}" title="{{ path }}">visited</a> a page
{% endblocktrans %}
{% else %}
{% blocktrans trimmed with action=log.action user=log.log_user_username path=log.resource %}
A user <a href="?action={{ action }}" title="{{ path }}">visited</a> a page
{% endblocktrans %}
{% endif %}
{% endif %}

{% trans "from" %}

<a href="?ip={{ log.ip }}" title="{{ log.browser }}">
<code>{{ log.ip }}</code>
</a>

{# If the authentication was on a doc domain. #}
{% if log.log_project_id %}
{% trans "on" %}
{% if log.project %}
<a href="{% url 'projects_detail' log.log_project_slug %}">
<code>{{ log.log_project_slug }}</code>
</a>
{% else %}
{# The original project has been deleted, don't link to it. #}
<code title="{{ log.resource }}">{{ log.log_project_slug }}</code>
{% endif %}
{% endif %}

<span class="quiet right" title="{{ log.created|date:"DATETIME_FORMAT" }}">
{% blocktrans trimmed with log.created|timesince as date %}
{{ date }} ago
{% endblocktrans %}
</span>
</li>
{% empty %}
<li class="module-item">
<p class="quiet">
{% trans 'No activity logged yet' %}
</p>
</li>
{% endfor %}
</ul>
</div>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
<ul>
<li class="{% block organization-admin-details %}{% endblock %}"><a href="{% url 'organization_edit' organization.slug %}">{% trans "Details" %}</a></li>
<li class="{% block organization-admin-owners %}{% endblock %}"><a href="{% url 'organization_owners' organization.slug %}">{% trans "Owners" %}</a></li>
<li class="{% block organization-admin-security-log %}{% endblock %}"><a href="{% url 'organization_security_log' organization.slug %}">{% trans "Security Log" %}</a></li>
</ul>
<div>
<h2>{% block edit_content_header %}{% endblock %}</h2>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
{% extends "organizations/admin/base.html" %}

{% load i18n %}
{% load pagination_tags %}

{% block title %}{% trans "Security Log" %}{% endblock %}

{% block organization-admin-security-log %}active{% endblock %}

{% block edit_content_header %} {% trans "Security Log" %} {% endblock %}

{% block edit_content %}

{% if not enabled %}
{% include 'projects/includes/feature_disabled.html' with organization=organization %}
Copy link
Member

@ericholscher ericholscher Oct 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be passing in something like plan=Pro in order to clarify what plan is required?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could pass the feature type and check from the other template what plan is required?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, not a huge priority though. 👍

{% elif days_limit and days_limit > 0 %}
<p>
{% blocktrans trimmed with days_limit as days_limit %}
Showing logs from the last {{ days_limit }} days.
{% endblocktrans %}
</p>
{% else %}
<p>{% trans "Showing logs from all time." %}</p>
{% endif %}
</p>

{% autopaginate object_list 15 %}

<div class="module">
<form method="get">
<button type="submit" name="download" value="true">{% trans "Download all data" %}</button>
</form>

{% include "audit/list_logs.html" with omit_user=False %}
</div>
{% paginate %}
{% endblock %}
127 changes: 127 additions & 0 deletions readthedocs/organizations/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
import csv
import itertools

import django_dynamic_fixture as fixture
from allauth.account.views import SignupView
from django.contrib.auth.models import User
from django.core.cache import cache
from django.test import TestCase
from django.test.utils import override_settings
from django.urls import reverse
from django_dynamic_fixture import get

from readthedocs.audit.models import AuditLog
from readthedocs.organizations.models import (
Organization,
Team,
Expand Down Expand Up @@ -54,6 +59,128 @@ def test_delete(self):
.exists())


@override_settings(RTD_ALLOW_ORGANIZATIONS=True)
class OrganizationSecurityLogTests(TestCase):

def setUp(self):
self.owner = get(User, username='owner')
self.member = get(User, username='member')
self.project = get(Project, slug='project')
self.project_b = get(Project, slug='project-b')
self.organization = get(
Organization,
owners=[self.owner],
projects=[self.project, self.project_b],
)
self.team = get(
Team,
organization=self.organization,
members=[self.member],
)

self.another_owner = get(User, username='another-owner')
self.another_member = get(User, username='another-member')
self.another_project = get(Project, slug='another-project')
self.another_organization = get(
Organization,
owners=[self.another_owner],
projects=[self.another_project],
)
self.another_team = get(
Team,
organization=self.another_organization,
members=[self.another_member],
)
self.client.force_login(self.owner)

actions = [
AuditLog.AUTHN,
AuditLog.AUTHN_FAILURE,
AuditLog.LOGOUT,
AuditLog.PAGEVIEW,
]
ips = [
'10.10.10.1',
'10.10.10.2',
]
users = [self.owner, self.member, self.another_owner, self.another_member]
AuditLog.objects.all().delete()
for action, ip, user in itertools.product(actions, ips, users):
get(
AuditLog,
user=user,
action=action,
ip=ip,
)
for project in [self.project, self.project_b, self.another_project]:
get(
AuditLog,
user=user,
action=action,
project=project,
ip=ip,
)

self.url = reverse('organization_security_log', args=[self.organization.slug])

def test_list_security_logs(self):
self.assertEqual(AuditLog.objects.count(), 128)

# Show logs for self.organization only.
resp = self.client.get(self.url)
self.assertEqual(resp.status_code, 200)
auditlogs = resp.context_data['object_list']
self.assertEqual(auditlogs.count(), 48)

# Show logs filtered by project.
resp = self.client.get(self.url + '?project=project')
self.assertEqual(resp.status_code, 200)
auditlogs = resp.context_data['object_list']
self.assertEqual(auditlogs.count(), 24)

resp = self.client.get(self.url + '?project=another-project')
self.assertEqual(resp.status_code, 200)
auditlogs = resp.context_data['object_list']
self.assertEqual(auditlogs.count(), 0)

# Show logs filtered by IP.
resp = self.client.get(self.url + '?ip=10.10.10.2')
self.assertEqual(resp.status_code, 200)
auditlogs = resp.context_data['object_list']
self.assertEqual(auditlogs.count(), 24)

# Show logs filtered by action.
for action in [AuditLog.AUTHN, AuditLog.AUTHN_FAILURE, AuditLog.PAGEVIEW]:
resp = self.client.get(self.url + f'?action={action}')
self.assertEqual(resp.status_code, 200)
auditlogs = resp.context_data['object_list']
self.assertEqual(auditlogs.count(), 16)

# Show logs filtered by user.
resp = self.client.get(self.url + '?user=member')
self.assertEqual(resp.status_code, 200)
auditlogs = resp.context_data['object_list']
self.assertEqual(auditlogs.count(), 12)

def test_download_csv(self):
self.assertEqual(AuditLog.objects.count(), 128)
resp = self.client.get(
self.url,
{'download': 'true'}
)
self.assertEqual(resp.status_code, 200)
self.assertEqual(resp['Content-Type'], 'text/csv')

# convert streaming data to csv format
content = [
line.decode()
for line in b''.join(resp.streaming_content).splitlines()
]
csv_data = list(csv.reader(content))
# All records + the header.
self.assertEqual(len(csv_data), 48 + 1)


@override_settings(RTD_ALLOW_ORGANIZATIONS=True)
class OrganizationInviteViewTests(RequestFactoryTestMixin, TestCase):

Expand Down
5 changes: 5 additions & 0 deletions readthedocs/organizations/urls/private.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@
views.DeleteOrganization.as_view(),
name='organization_delete',
),
url(
r'^(?P<slug>[\w.-]+)/security-log/$',
views.OrganizationSecurityLog.as_view(),
name='organization_security_log',
),
# Owners
url(
r'^(?P<slug>[\w.-]+)/owners/(?P<owner>\d+)/delete/$',
Expand Down
Loading