Distinguish between FxA logins and API token logins in ActivityLog (#20044)

* Distinguish between FxA logins and API token logins in ActivityLog
This commit is contained in:
Mathieu Pillard 2022-12-05 11:58:30 +01:00 коммит произвёл GitHub
Родитель f62f8ffa5e
Коммит 9d5eff4ad1
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
6 изменённых файлов: 74 добавлений и 14 удалений

Просмотреть файл

@ -213,6 +213,12 @@ class TestLoginUserAndRegisterUser(TestCase):
ActivityLog.objects.filter(user=self.user, action=amo.LOG.LOG_IN.id).count()
== 1
)
assert (
ActivityLog.objects.filter(
user=self.user, action=amo.LOG.LOG_IN_API_TOKEN.id
).count()
== 0
)
assert IPLog.objects.all().count() == 1
assert IPLog.objects.get().ip_address_binary == IPv4Address('8.8.8.8')

Просмотреть файл

@ -1604,7 +1604,11 @@ class TestAddonViewSetUpdate(AddonViewSetCreateUpdateMixin, TestCase):
).to_representation(self.addon)
assert self.addon.summary == 'summary update!'
alog = ActivityLog.objects.exclude(
action__in=(amo.LOG.ADD_VERSION.id, amo.LOG.LOG_IN.id)
action__in=(
amo.LOG.ADD_VERSION.id,
amo.LOG.LOG_IN.id,
amo.LOG.LOG_IN_API_TOKEN.id,
)
).get()
assert alog.user == self.user
assert alog.action == amo.LOG.EDIT_PROPERTIES.id
@ -1853,7 +1857,11 @@ class TestAddonViewSetUpdate(AddonViewSetCreateUpdateMixin, TestCase):
assert data['support_url']['url'] == {'en-US': 'https://my.home.page/support/'}
assert addon.support_url == 'https://my.home.page/support/'
alog = ActivityLog.objects.exclude(
action__in=(amo.LOG.ADD_VERSION.id, amo.LOG.LOG_IN.id)
action__in=(
amo.LOG.ADD_VERSION.id,
amo.LOG.LOG_IN.id,
amo.LOG.LOG_IN_API_TOKEN.id,
)
).get()
assert alog.user == self.user
assert alog.action == amo.LOG.EDIT_PROPERTIES.id
@ -1871,7 +1879,11 @@ class TestAddonViewSetUpdate(AddonViewSetCreateUpdateMixin, TestCase):
assert addon.is_disabled is True
assert addon.disabled_by_user is True # sets the user property
alog = ActivityLog.objects.exclude(
action__in=(amo.LOG.ADD_VERSION.id, amo.LOG.LOG_IN.id)
action__in=(
amo.LOG.ADD_VERSION.id,
amo.LOG.LOG_IN.id,
amo.LOG.LOG_IN_API_TOKEN.id,
)
).get()
assert alog.user == self.user
assert alog.action == amo.LOG.USER_DISABLE.id
@ -1903,7 +1915,11 @@ class TestAddonViewSetUpdate(AddonViewSetCreateUpdateMixin, TestCase):
assert addon.is_disabled is False
assert addon.disabled_by_user is False # sets the user property
alog = ActivityLog.objects.exclude(
action__in=(amo.LOG.ADD_VERSION.id, amo.LOG.LOG_IN.id)
action__in=(
amo.LOG.ADD_VERSION.id,
amo.LOG.LOG_IN.id,
amo.LOG.LOG_IN_API_TOKEN.id,
)
).get()
assert alog.user == self.user
assert alog.action == amo.LOG.USER_ENABLE.id
@ -1975,7 +1991,11 @@ class TestAddonViewSetUpdate(AddonViewSetCreateUpdateMixin, TestCase):
self.addon.reload()
assert [tag.tag_text for tag in self.addon.tags.all()] == ['music', 'zoom']
alogs = ActivityLog.objects.exclude(
action__in=(amo.LOG.ADD_VERSION.id, amo.LOG.LOG_IN.id)
action__in=(
amo.LOG.ADD_VERSION.id,
amo.LOG.LOG_IN.id,
amo.LOG.LOG_IN_API_TOKEN.id,
)
)
assert len(alogs) == 2, [(a.action, a.details) for a in alogs]
assert alogs[0].action == amo.LOG.REMOVE_TAG.id
@ -2032,7 +2052,9 @@ class TestAddonViewSetUpdate(AddonViewSetCreateUpdateMixin, TestCase):
assert os.path.exists(
os.path.join(self.addon.get_icon_dir(), f'{self.addon.id}-original.png')
)
alog = ActivityLog.objects.exclude(action=amo.LOG.LOG_IN.id).get()
alog = ActivityLog.objects.exclude(
action__in=(amo.LOG.LOG_IN.id, amo.LOG.LOG_IN_API_TOKEN.id)
).get()
assert alog.user == self.user
assert alog.action == amo.LOG.CHANGE_MEDIA.id
@ -2051,7 +2073,11 @@ class TestAddonViewSetUpdate(AddonViewSetCreateUpdateMixin, TestCase):
assert self.addon.icon_type == ''
remove_icons_mock.assert_called()
alog = ActivityLog.objects.exclude(
action__in=(amo.LOG.ADD_VERSION.id, amo.LOG.LOG_IN.id)
action__in=(
amo.LOG.ADD_VERSION.id,
amo.LOG.LOG_IN.id,
amo.LOG.LOG_IN_API_TOKEN.id,
)
).get()
assert alog.user == self.user
assert alog.action == amo.LOG.CHANGE_MEDIA.id
@ -3552,7 +3578,9 @@ class TestVersionViewSetUpdate(UploadMixin, VersionViewSetCreateUpdateMixin, Tes
+ reverse('addons.license', args=[self.addon.slug]),
'slug': None,
}
alog = ActivityLog.objects.exclude(action=amo.LOG.LOG_IN.id).get()
alog = ActivityLog.objects.exclude(
action__in=(amo.LOG.LOG_IN.id, amo.LOG.LOG_IN_API_TOKEN.id)
).get()
assert alog.user == self.user
assert alog.action == amo.LOG.CHANGE_LICENSE.id
@ -3582,7 +3610,7 @@ class TestVersionViewSetUpdate(UploadMixin, VersionViewSetCreateUpdateMixin, Tes
alog2 = (
ActivityLog.objects.exclude(id=alog.id)
.exclude(action=amo.LOG.LOG_IN.id)
.exclude(action__in=(amo.LOG.LOG_IN.id, amo.LOG.LOG_IN_API_TOKEN.id))
.get()
)
assert alog2.user == self.user
@ -3615,7 +3643,9 @@ class TestVersionViewSetUpdate(UploadMixin, VersionViewSetCreateUpdateMixin, Tes
+ reverse('addons.license', args=[self.addon.slug]),
'slug': None,
}
alog = ActivityLog.objects.exclude(action=amo.LOG.LOG_IN.id).get()
alog = ActivityLog.objects.exclude(
action__in=(amo.LOG.LOG_IN.id, amo.LOG.LOG_IN_API_TOKEN.id)
).get()
assert alog.user == self.user
assert alog.action == amo.LOG.CHANGE_LICENSE.id
@ -3635,7 +3665,7 @@ class TestVersionViewSetUpdate(UploadMixin, VersionViewSetCreateUpdateMixin, Tes
assert data['license']['url'] == builtin_license.url
alog2 = (
ActivityLog.objects.exclude(id=alog.id)
.exclude(action=amo.LOG.LOG_IN.id)
.exclude(action__in=(amo.LOG.LOG_IN.id, amo.LOG.LOG_IN_API_TOKEN.id))
.get()
)
assert alog2.user == self.user

Просмотреть файл

@ -204,7 +204,9 @@ class JWTKeyAuthentication(BaseAuthentication):
# Send user_logged_in signal when JWT is used to authenticate an user.
# Otherwise, we'd never update the last_login information for users
# who never visit the site but do use the API to upload new add-ons.
user_logged_in.send(sender=self.__class__, request=request, user=user)
user_logged_in.send(
sender=self.__class__, request=request, user=user, using_api_token=True
)
return (user, jwt_value)
def authenticate_credentials(self, payload):

Просмотреть файл

@ -17,8 +17,9 @@ from rest_framework.permissions import IsAuthenticated
from rest_framework.response import Response
from rest_framework.views import APIView
from olympia import core
from olympia import amo, core
from olympia.accounts.verify import IdentificationError
from olympia.activity.models import ActivityLog
from olympia.amo.templatetags.jinja_helpers import absolutify
from olympia.amo.tests import (
APITestClientSessionID,
@ -73,6 +74,12 @@ class TestJWTKeyAuthentication(JWTAuthKeyTester, TestCase):
assert user == self.user
assert user.last_login_ip == '15.16.23.42'
self.assertCloseToNow(user.last_login)
assert (
ActivityLog.objects.filter(
action=amo.LOG.LOG_IN_API_TOKEN.id, user=user
).count()
== 1
)
def test_authenticate_header(self):
request = self.factory.post('/api/v4/whatever/')

Просмотреть файл

@ -855,6 +855,16 @@ class UNREJECT_VERSION(_LOG):
reviewer_review_action = True
class LOG_IN_API_TOKEN(_LOG):
id = 172
# Note: clear_old_user_data cron would delete the IPLog when needed, so we
# can keep the activity log, it just won't have any IP associated with it.
keep = True
admin_event = True
store_ip = True
format = _('{user} authenticated through an API token.')
LOGS = [x for x in vars().values() if isclass(x) and issubclass(x, _LOG) and x != _LOG]
# Make sure there's no duplicate IDs.
assert len(LOGS) == len({log.id for log in LOGS})

Просмотреть файл

@ -579,7 +579,12 @@ class UserProfile(OnChangeMixin, ModelBase, AbstractBaseUser):
# The following log statement is used by foxsec-pipeline.
log.info('User (%s) logged in successfully', user, extra={'email': user.email})
user.update(last_login_ip=core.get_remote_addr() or '')
activity.log_create(amo.LOG.LOG_IN, user=user)
action = (
amo.LOG.LOG_IN_API_TOKEN
if kwargs.get('using_api_token')
else amo.LOG.LOG_IN
)
activity.log_create(action, user=user)
class UserNotification(ModelBase):