Skip to content

Commit b5e0df4

Browse files
committed
AMOENG-2407 - Introduce a new serializer for the lookup API endpoint and account retrieval with the Users:Lookup permission, exposing email without other sensitive fields
1 parent b51c69b commit b5e0df4

4 files changed

Lines changed: 40 additions & 5 deletions

File tree

docs/topics/api/accounts.rst

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,9 @@ A developer is defined as a user who is listed as a developer or owner of one or
5252

5353
If you authenticate and access your own account by specifing your own ``user_id`` the following additional fields are returned.
5454
You can always access your account, regardless of whether you are a developer or not.
55-
If you have `Users:Edit` permission you will see these extra fields for all user accounts.
55+
If you have ``Users:Edit`` permission you will see these extra fields for all user accounts.
56+
If you have ``Users:Lookup`` permission (but not ``Users:Edit``) you will see all fields from
57+
the :ref:`account object <account-object>` plus ``email``, but not the other privileged fields below.
5658

5759
.. http:get:: /api/v5/accounts/account/(int:user_id|string:username)/
5860
@@ -110,15 +112,18 @@ Account Lookup
110112
.. note::
111113
This API requires :doc:`authentication <auth>` and ``Users:Lookup`` permission.
112114

113-
This endpoint looks up one or more accounts by email address and returns their full details.
115+
This endpoint looks up one or more accounts by email address.
114116
Since multiple accounts can share the same email address, the response is always a list.
115117
Deleted accounts are excluded from results.
116118

119+
The response includes all :ref:`account object <account-object>` fields plus ``email``,
120+
but excludes sensitive fields such as ``last_login_ip`` and ``permissions``.
121+
117122
.. http:get:: /api/v5/accounts/account/lookup/
118123
119124
:query string email: The email address to look up.
120125

121-
Returns a list of :ref:`account objects <account-object-self>`.
126+
Returns a list of :ref:`account objects <account-object>` augmented with ``email``.
122127

123128
:statuscode 200: account(s) found.
124129
:statuscode 400: ``email`` query parameter is missing.

src/olympia/accounts/serializers.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,12 @@ class Meta(BaseUserSerializer.Meta):
8181
read_only_fields = fields
8282

8383

84+
class LookupUserProfileSerializer(FullUserProfileSerializer):
85+
class Meta(FullUserProfileSerializer.Meta):
86+
fields = FullUserProfileSerializer.Meta.fields + ('email',)
87+
read_only_fields = fields
88+
89+
8490
class MinimalUserProfileSerializer(FullUserProfileSerializer):
8591
nullable_fields = (
8692
'biography',

src/olympia/accounts/tests/test_views.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1376,6 +1376,22 @@ def test_admin_view_slug(self):
13761376
assert response.data['email'] == self.random_user.email
13771377
assert response.data['url'] == absolutify(self.random_user.get_url_path())
13781378

1379+
def test_lookup_view(self):
1380+
self.grant_permission(self.user, 'Users:Lookup')
1381+
self.client.login_api(self.user)
1382+
random_user = user_factory(biography='something!')
1383+
random_user_profile_url = reverse_ns(
1384+
'account-detail', kwargs={'pk': random_user.pk}
1385+
)
1386+
response = self.client.get(random_user_profile_url)
1387+
assert response.status_code == 200
1388+
assert response.data['name'] == random_user.name
1389+
assert response.data['biography'] == random_user.biography
1390+
assert response.data['email'] == random_user.email
1391+
assert 'last_login_ip' not in response.data
1392+
assert 'permissions' not in response.data
1393+
assert response.data['url'] == absolutify(random_user.get_url_path())
1394+
13791395

13801396
class TestProfileViewWithJWT(APIKeyAuthTestMixin, TestCase):
13811397
"""This just tests JWT Auth (external) on the profile endpoint.
@@ -1409,8 +1425,9 @@ def test_lookup_by_email(self):
14091425
response = self.get(self.url, data={'email': self.target_user.email})
14101426
assert response.status_code == 200
14111427
assert len(response.data) == 1
1412-
assert response.data[0]['email'] == self.target_user.email
14131428
assert response.data[0]['id'] == self.target_user.pk
1429+
assert response.data[0]['email'] == self.target_user.email
1430+
assert 'last_login_ip' not in response.data[0]
14141431

14151432
def test_lookup_multiple_users_same_email(self):
14161433
# Multiple accounts can share the same email (no unique constraint).

src/olympia/accounts/views.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
from .serializers import (
6969
AccountSuperCreateSerializer,
7070
FullUserProfileSerializer,
71+
LookupUserProfileSerializer,
7172
MinimalUserProfileSerializer,
7273
SelfUserProfileSerializer,
7374
UserNotificationSerializer,
@@ -506,9 +507,15 @@ def self_view(self):
506507
def admin_viewing(self):
507508
return acl.action_allowed_for(self.request.user, amo.permissions.USERS_EDIT)
508509

510+
@property
511+
def lookup_viewing(self):
512+
return acl.action_allowed_for(self.request.user, amo.permissions.USERS_LOOKUP)
513+
509514
def get_serializer_class(self):
510515
if self.self_view or self.admin_viewing:
511516
return SelfUserProfileSerializer
517+
elif self.lookup_viewing:
518+
return LookupUserProfileSerializer
512519
elif self.get_object().has_full_profile:
513520
return FullUserProfileSerializer
514521
else:
@@ -559,7 +566,7 @@ def lookup(self, request):
559566
)
560567
if not users.exists():
561568
raise exceptions.NotFound()
562-
serializer = SelfUserProfileSerializer(
569+
serializer = LookupUserProfileSerializer(
563570
users, many=True, context={'request': request}
564571
)
565572
return Response(serializer.data)

0 commit comments

Comments
 (0)