feat(org member invite): OrganizationMemberInviteDetails PUT endpoint #88409

mifu67 · 2025-04-01T05:28:21Z

Implement the PUT method on the OrganizationMemberInviteDetails endpoint. The method supports the following functionality:

Allow members to reinvite users that they invited
Allow org managers/owners to approve invites
Allow org managers/owners to edit invitation details

src/sentry/api/serializers/rest_framework/organizationmemberinvite.py

+        try:
+            invited_member.validate_invitation(approving_user, allowed_roles)
+        except UnableToAcceptMemberInvitationException as err:
+            raise serializers.ValidationError(str(err))


To fix the problem, we should avoid exposing the detailed exception message to the end user. Instead, we should log the detailed error message on the server and return a generic error message to the user. This can be achieved by modifying the exception handling in the validate_approve method to log the exception and raise a generic ValidationError.

codecov · 2025-04-01T16:58:19Z

Codecov Report

Attention: Patch coverage is 94.55128% with 17 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...dpoints/test_organization_member_invite_details.py	92.82%	14 Missing ⚠️
...pi/endpoints/organization_member_invite/details.py	96.92%	2 Missing ⚠️
src/sentry/models/organizationmemberinvite.py	97.05%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           master   #88409    +/-   ##
========================================
  Coverage   87.72%   87.73%            
========================================
  Files       10003    10004     +1     
  Lines      566676   567124   +448     
  Branches    22265    22265            
========================================
+ Hits       497108   497538   +430     
- Misses      69150    69168    +18     
  Partials      418      418

mifu67 · 2025-04-01T17:05:53Z

src/sentry/api/endpoints/organization_member_invite/details.py

+
+        if result.get("orgRole"):
+            invited_member.set_org_role(result["orgRole"])
+        if result.get("teams"):


The original OrganizationMemberDetails endpoint allows you to set team roles for invited members. Do we want to continue to allow this behavior? We don't set team roles when inviting members, so users would be required to make an additional PUT request for an invited member.

probably a good idea to try to replicate existing behavior where possible

cathteng

might be helpful to diagram out all the possible flows within this PUT 😵‍💫

cathteng · 2025-04-01T21:19:02Z

src/sentry/api/endpoints/organization_member_invite/details.py

+                return Response({"detail": ERR_INSUFFICIENT_SCOPE}, status=400)
+        if invited_member.token_expired:
+            return Response({"detail": ERR_EXPIRED}, status=400)
+        invited_member.send_invite_email()


what happened to sending SSO linked emails?

cathteng · 2025-04-01T22:06:25Z

src/sentry/api/endpoints/organization_member_invite/details.py

+
+        if result.get("orgRole"):
+            invited_member.set_org_role(result["orgRole"])
+        if result.get("teams"):


probably a good idea to try to replicate existing behavior where possible

cathteng · 2025-04-01T22:06:49Z

src/sentry/api/endpoints/organization_member_invite/details.py

+            if request.data["approve"] is False:
+                return Response({"detail": ERR_WRONG_METHOD}, status=400)


can this be in the validator?

cathteng · 2025-04-01T22:10:59Z

src/sentry/api/endpoints/organization_member_invite/details.py

+            if not is_invite_from_user:
+                return Response({"detail": ERR_MEMBER_INVITE}, status=403)
+
+        if result.get("reinvite"):


it seems like we ignore the other fields if reinvite is true. should this be moved to the top of the method so we don't attempt to validate the other fields? although it also doesn't seem like an implausible situation to use the API to update AND reinvite, even if it's not surfaced in the UI

cathteng · 2025-04-01T22:21:38Z

src/sentry/api/endpoints/organization_member_invite/details.py

+        is_member = not request.access.has_scope("member:admin") and (
+            request.access.has_scope("member:invite")
+        )
+        members_can_invite = not organization.flags.disable_member_invite
+        # Members can only resend invites
+        is_reinvite_request_only = (
+            set(result.keys()).issubset({"reinvite", "regenerate"})
+            and "approve" not in request.data
+        )
+
+        # Members can only resend invites that they sent
+        is_invite_from_user = invited_member.inviter_id == request.user.id
+
+        if is_member:
+            if not (members_can_invite and is_reinvite_request_only):
+                # this check blocks members from doing anything but reinviting
+                raise PermissionDenied
+            if not is_invite_from_user:
+                return Response({"detail": ERR_MEMBER_INVITE}, status=403)


should we be checking member permissions before validating the incoming data? i would also put this into its own function if possible

cathteng · 2025-04-01T22:32:57Z

tests/sentry/api/endpoints/test_organization_member_invite_details.py

+        "sentry.roles.organization_roles.get",
+        wraps=mock_organization_roles_get_factory(organization_roles.get),
+    )
+    def test_update_teams_invalid__a(self, mock_get):


Suggested change

def test_update_teams_invalid__a(self, mock_get):

def test_update_teams_invalid__team_assignments_change_orgrole(self, mock_get):

?

cathteng · 2025-04-01T22:34:45Z

tests/sentry/api/endpoints/test_organization_member_invite_details.py

+            == "You do not have permission to approve a member invitation with the role owner."
+        )
+
+    @with_feature({"organizations:invite-members": False})


the default is True, should we just remove this FF?

github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 1, 2025

vercel bot deployed to Preview April 1, 2025 05:31 View deployment

Base automatically changed from mifu67/member-invite/omi-details-get to master April 1, 2025 16:04

mifu67 added 3 commits April 1, 2025 09:11

first pass: reinvite, update details, approve invite

a11ced8

add some tests

8d3b266

tests woooo

1a75bd2

mifu67 force-pushed the mifu67/member-invite/omi-details-put branch from b998826 to 1a75bd2 Compare April 1, 2025 16:12

🛠️ apply pre-commit fixes

52d5c82

github-advanced-security bot found potential problems Apr 1, 2025

View reviewed changes

vercel bot had a problem deploying to Preview April 1, 2025 16:18 Failure

flag

e0946c3

mifu67 marked this pull request as ready for review April 1, 2025 16:32

mifu67 requested a review from cathteng April 1, 2025 16:32

vercel bot deployed to Preview April 1, 2025 16:35 View deployment

mifu67 requested a review from leedongwei April 1, 2025 16:58

mifu67 commented Apr 1, 2025

View reviewed changes

mifu67 requested a review from sentaur-athena April 1, 2025 18:43

orgRole, not role

078db69

vercel bot deployed to Preview April 1, 2025 21:54 View deployment

cathteng reviewed Apr 1, 2025

View reviewed changes

mifu67 requested a review from saponifi3d April 1, 2025 22:56

@@ -15,3 +15,5 @@
             from sentry.users.services.user.service import user_service
+            import logging
+            logger = logging.getLogger(__name__)
@@ -166,3 +168,4 @@
                     except UnableToAcceptMemberInvitationException as err:
-                        raise serializers.ValidationError(str(err))
+                        logger.error(f"Unable to accept member invitation: {err}")
+                        raise serializers.ValidationError("An error occurred while processing the invitation.")

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(org member invite): OrganizationMemberInviteDetails PUT endpoint #88409

feat(org member invite): OrganizationMemberInviteDetails PUT endpoint #88409

mifu67 commented Apr 1, 2025 •

edited

Loading

Copilot Autofix

codecov bot commented Apr 1, 2025 •

edited

Loading

mifu67 Apr 1, 2025

cathteng Apr 1, 2025

cathteng left a comment

cathteng Apr 1, 2025

cathteng Apr 1, 2025

cathteng Apr 1, 2025

cathteng Apr 1, 2025

cathteng Apr 1, 2025

cathteng Apr 1, 2025

cathteng Apr 1, 2025

		if request.data["approve"] is False:
		return Response({"detail": ERR_WRONG_METHOD}, status=400)

	def test_update_teams_invalid__a(self, mock_get):
	def test_update_teams_invalid__team_assignments_change_orgrole(self, mock_get):

feat(org member invite): OrganizationMemberInviteDetails PUT endpoint #88409

Are you sure you want to change the base?

feat(org member invite): OrganizationMemberInviteDetails PUT endpoint #88409

Conversation

mifu67 commented Apr 1, 2025 • edited Loading

Copilot Autofix

codecov bot commented Apr 1, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cathteng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mifu67 commented Apr 1, 2025 •

edited

Loading

codecov bot commented Apr 1, 2025 •

edited

Loading