Skip to content

fix: do not return secondary emails in member listing/details #83556

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 7 commits into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions src/sentry/api/serializers/models/organization_member/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ def get_attrs(
users_by_id: MutableMapping[str, Any] = {}
email_map: MutableMapping[str, str] = {}
for u in user_service.serialize_many(filter={"user_ids": users_set}):
# Filter out the emails from the user data
u.pop("emails", None)
users_by_id[u["id"]] = u
email_map[u["id"]] = u["email"]

Expand Down Expand Up @@ -96,17 +98,39 @@ def serialize(
user: User | RpcUser | AnonymousUser,
**kwargs: Any,
) -> OrganizationMemberResponse:
serialized_user = attrs["user"]
if obj.user_id:
# if the OrganizationMember has a user_id, the user has an account
# `email` on the OrganizationMember will be null, so we need to pull
# the email address from the user's actual account
email = serialized_user["email"] if serialized_user else obj.email
else:
# when there is no user_id, the OrganizationMember is an invited user
# and the email field on OrganizationMember will be populated, so we
# will use it directly
email = obj.email

# helping mypy - the email will always be populated at this point
# in the case that it is a user that has an account, we pull the email
# address above from the serialized_user. The email field on OrganizationMember
# is null in the case, so it is necessary.
#
# invited users do not yet have a full account and the email field
# on OrganizationMember will be populated in such cases
assert email is not None

inviter_name = None
if obj.inviter_id:
inviter = attrs["inviter"]
if inviter:
inviter_name = inviter.get_display_name()
serialized_user = attrs["user"]
email = attrs["email"]

name = serialized_user["name"] if serialized_user else email

data: OrganizationMemberResponse = {
"id": str(obj.id),
"email": email,
"name": serialized_user["name"] if serialized_user else email,
"name": name,
"user": attrs["user"],
"orgRole": obj.role,
"pending": obj.is_pending,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ def get_attrs(
serializer=UserSerializeType.DETAILED,
)
}

# Filter out emails from the serialized user data
for user_data in users_by_id.values():
user_data.pop("emails", None)

for item in item_list:
result.setdefault(item, {})["serializedUser"] = users_by_id.get(str(item.user_id), {})
return result
Expand Down
16 changes: 16 additions & 0 deletions tests/sentry/api/endpoints/test_organization_member_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,22 @@ def test_lists_team_roles(self):
role_ids = [role["id"] for role in response.data["teamRoleList"]]
assert role_ids == ["contributor", "admin"]

def test_does_not_include_secondary_emails(self):
# Create a user with multiple email addresses
user = self.create_user("[email protected]", username="multi_email_user")
self.create_useremail(user, "[email protected]")
self.create_useremail(user, "[email protected]")

# Add user to organization
member = self.create_member(organization=self.organization, user=user, role="member")

response = self.get_success_response(self.organization.slug, member.id)

# Check that only primary email is present and no other email addresses are exposed
assert response.data["email"] == "[email protected]"
assert "emails" not in response.data["user"]
assert "emails" not in response.data.get("serializedUser", {})


class UpdateOrganizationMemberTest(OrganizationMemberTestBase, HybridCloudTestMixin):
method = "put"
Expand Down
20 changes: 20 additions & 0 deletions tests/sentry/api/endpoints/test_organization_member_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,26 @@ def test_cannot_invite_retired_role_with_flag(self):
== "The role 'admin' is deprecated and may no longer be assigned."
)

def test_does_not_include_secondary_emails(self):
# Create a user with multiple email addresses
user3 = self.create_user("[email protected]", username="multi_email_user")
self.create_useremail(user3, "[email protected]")
self.create_useremail(user3, "[email protected]")

# Add user to organization
self.create_member(organization=self.organization, user=user3)

response = self.get_success_response(self.organization.slug)

# Find the member in the response
member_data = next(m for m in response.data if m["email"] == "[email protected]")

# Check that only primary email is present and no other email addresses are exposed
assert member_data["email"] == "[email protected]"
assert "emails" not in member_data["user"]
assert all("email" not in team for team in member_data.get("teams", []))
assert all("email" not in role for role in member_data.get("teamRoles", []))


class OrganizationMemberPermissionRoleTest(OrganizationMemberListTestBase, HybridCloudTestMixin):
method = "post"
Expand Down
Loading