From 06b26aa45b814ed9cecc016c9737a68f05dc4b5b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Oct 2025 09:48:03 +0000 Subject: [PATCH 1/6] Initial plan From d6f3570adef609ab5fba6775e3998fb32d0108e4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Oct 2025 09:58:22 +0000 Subject: [PATCH 2/6] Add DELETE endpoint for removing organization members Co-authored-by: rishi-raj-jain <46300090+rishi-raj-jain@users.noreply.github.com> --- server/polar/organization/endpoints.py | 43 ++++++++ server/tests/organization/test_endpoints.py | 115 ++++++++++++++++++++ 2 files changed, 158 insertions(+) diff --git a/server/polar/organization/endpoints.py b/server/polar/organization/endpoints.py index 5402f9ba7e..58b7f95caa 100644 --- a/server/polar/organization/endpoints.py +++ b/server/polar/organization/endpoints.py @@ -1,4 +1,5 @@ from typing import cast +from uuid import UUID from fastapi import Depends, Query, Response, status from sqlalchemy.orm import joinedload @@ -344,6 +345,48 @@ async def invite_member( return OrganizationMember.model_validate(user_org) +@router.delete( + "/{id}/members/{user_id}", + status_code=204, + tags=[APITag.private], +) +async def remove_member( + id: OrganizationID, + user_id: UUID, + auth_subject: auth.OrganizationsWrite, + session: AsyncSession = Depends(get_db_session), +) -> None: + """Remove a member from an organization. + + Only non-admin members can be removed. The organization admin cannot be removed. + + Raises: + 404: Organization not found or user is not a member + 403: Cannot remove organization admin + """ + from polar.user_organization.service import ( + CannotRemoveOrganizationAdmin, + OrganizationNotFound, + UserNotMemberOfOrganization, + ) + + organization = await organization_service.get(session, auth_subject, id) + + if organization is None: + raise ResourceNotFound() + + try: + await user_organization_service.remove_member_safe( + session, user_id, organization.id + ) + except OrganizationNotFound: + raise ResourceNotFound() + except UserNotMemberOfOrganization: + raise ResourceNotFound() + except CannotRemoveOrganizationAdmin: + raise NotPermitted("Cannot remove organization admin") + + @router.post( "/{id}/ai-validation", response_model=OrganizationReviewStatus, diff --git a/server/tests/organization/test_endpoints.py b/server/tests/organization/test_endpoints.py index 0881b0fdf2..1e81689114 100644 --- a/server/tests/organization/test_endpoints.py +++ b/server/tests/organization/test_endpoints.py @@ -631,3 +631,118 @@ async def test_valid_account_admin( assert json["is_details_submitted"] assert json["is_charges_enabled"] assert json["is_payouts_enabled"] + + +@pytest.mark.asyncio +class TestRemoveMember: + async def test_anonymous( + self, client: AsyncClient, organization: Organization, user: User + ) -> None: + response = await client.delete( + f"/v1/organizations/{organization.id}/members/{user.id}" + ) + + assert response.status_code == 401 + + @pytest.mark.auth + async def test_not_member_of_org( + self, + client: AsyncClient, + organization: Organization, + user_second: User, + ) -> None: + response = await client.delete( + f"/v1/organizations/{organization.id}/members/{user_second.id}" + ) + + assert response.status_code == 404 + + @pytest.mark.auth + async def test_remove_non_member( + self, + client: AsyncClient, + session: AsyncSession, + organization: Organization, + user_organization: UserOrganization, + user_second: User, + ) -> None: + # user_second is not a member + response = await client.delete( + f"/v1/organizations/{organization.id}/members/{user_second.id}" + ) + + assert response.status_code == 404 + + @pytest.mark.auth + async def test_remove_member_success( + self, + client: AsyncClient, + session: AsyncSession, + organization: Organization, + user_organization: UserOrganization, + user_organization_second: UserOrganization, + ) -> None: + # user_organization is the authenticated user (member of org) + # user_organization_second is another member to be removed + + members_before = await user_organization_service.list_by_org( + session, organization.id + ) + assert len(members_before) == 2 + + response = await client.delete( + f"/v1/organizations/{organization.id}/members/{user_organization_second.user.id}" + ) + + assert response.status_code == 204 + + members_after = await user_organization_service.list_by_org( + session, organization.id + ) + assert len(members_after) == 1 + assert members_after[0].user_id == user_organization.user_id + + @pytest.mark.auth + async def test_cannot_remove_admin( + self, + client: AsyncClient, + session: AsyncSession, + organization: Organization, + organization_account: Account, + user_organization: UserOrganization, + user_organization_second: UserOrganization, + save_fixture: SaveFixture, + ) -> None: + # Make user the admin by linking the account + # The organization_account fixture already has user as admin + # Just need to ensure user_organization exists + + members_before = await user_organization_service.list_by_org( + session, organization.id + ) + + # Try to remove the admin user (the fixture user is the admin through organization_account) + response = await client.delete( + f"/v1/organizations/{organization.id}/members/{user_organization.user.id}" + ) + + assert response.status_code == 403 + + members_after = await user_organization_service.list_by_org( + session, organization.id + ) + # No members should be removed + assert len(members_after) == len(members_before) + + @pytest.mark.auth + async def test_organization_not_found( + self, + client: AsyncClient, + user: User, + ) -> None: + response = await client.delete( + f"/v1/organizations/{uuid.uuid4()}/members/{user.id}" + ) + + assert response.status_code == 404 + From f579a6ce7743533f31da9157eaa28496f9f89ce5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Oct 2025 10:01:54 +0000 Subject: [PATCH 3/6] Add user_id and is_admin fields to OrganizationMember schema Co-authored-by: rishi-raj-jain <46300090+rishi-raj-jain@users.noreply.github.com> --- clients/apps/web/src/hooks/queries/org.ts | 14 ++++++++++++++ server/polar/organization/endpoints.py | 20 +++++++++++++++++++- server/polar/user_organization/schemas.py | 5 +++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/clients/apps/web/src/hooks/queries/org.ts b/clients/apps/web/src/hooks/queries/org.ts index aa3857dfec..e10ca30169 100644 --- a/clients/apps/web/src/hooks/queries/org.ts +++ b/clients/apps/web/src/hooks/queries/org.ts @@ -30,6 +30,20 @@ export const useInviteOrganizationMember = (id: string) => }, }) +export const useRemoveOrganizationMember = (id: string) => + useMutation({ + mutationFn: (userId: string) => { + return api.DELETE('/v1/organizations/{id}/members/{user_id}', { + params: { path: { id, user_id: userId } }, + }) + }, + onSuccess: async (_result, _variables, _ctx) => { + queryClient.invalidateQueries({ + queryKey: ['organizationMembers', id], + }) + }, + }) + export const useListOrganizations = ( params: operations['organizations:list']['parameters']['query'], enabled: boolean = true, diff --git a/server/polar/organization/endpoints.py b/server/polar/organization/endpoints.py index 58b7f95caa..2ab6771e59 100644 --- a/server/polar/organization/endpoints.py +++ b/server/polar/organization/endpoints.py @@ -267,6 +267,8 @@ async def members( session: AsyncReadSession = Depends(get_db_read_session), ) -> ListResource[OrganizationMember]: """List members in an organization.""" + from polar.organization.repository import OrganizationRepository + organization = await organization_service.get(session, auth_subject, id) if organization is None: @@ -274,8 +276,24 @@ async def members( members = await user_organization_service.list_by_org(session, id) + # Get admin user to mark them in the response + admin_user_id = None + if organization.account_id: + org_repo = OrganizationRepository.from_session(session) + admin_user = await org_repo.get_admin_user( + cast(AsyncSession, session), organization + ) + if admin_user: + admin_user_id = admin_user.id + + items = [] + for m in members: + member_dict = OrganizationMember.model_validate(m).model_dump() + member_dict["is_admin"] = m.user_id == admin_user_id + items.append(OrganizationMember.model_validate(member_dict)) + return ListResource( - items=[OrganizationMember.model_validate(m) for m in members], + items=items, pagination=Pagination(total_count=len(members), max_page=1), ) diff --git a/server/polar/user_organization/schemas.py b/server/polar/user_organization/schemas.py index 57f8b40632..633e7cd69d 100644 --- a/server/polar/user_organization/schemas.py +++ b/server/polar/user_organization/schemas.py @@ -1,4 +1,5 @@ from datetime import datetime +from uuid import UUID from pydantic import AliasPath, EmailStr, Field @@ -6,11 +7,15 @@ class OrganizationMember(Schema): + user_id: UUID = Field(description="The user ID of the organization member") created_at: datetime = Field( description="The time the OrganizationMember was creatd." ) email: str = Field(validation_alias=AliasPath("user", "email")) avatar_url: str | None = Field(validation_alias=AliasPath("user", "avatar_url")) + is_admin: bool = Field( + default=False, description="Whether the member is the organization admin" + ) class OrganizationMemberInvite(Schema): From 4b36949d2e57a565ce5a5245e4c2d4096ff0ddc6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Oct 2025 10:04:36 +0000 Subject: [PATCH 4/6] Add member removal UI with confirmation modal Co-authored-by: rishi-raj-jain <46300090+rishi-raj-jain@users.noreply.github.com> --- .../(header)/settings/members/ClientPage.tsx | 125 +++++++++++++++++- 1 file changed, 124 insertions(+), 1 deletion(-) diff --git a/clients/apps/web/src/app/(main)/dashboard/[organization]/(header)/settings/members/ClientPage.tsx b/clients/apps/web/src/app/(main)/dashboard/[organization]/(header)/settings/members/ClientPage.tsx index 65d3cc5ff9..8ede476e34 100644 --- a/clients/apps/web/src/app/(main)/dashboard/[organization]/(header)/settings/members/ClientPage.tsx +++ b/clients/apps/web/src/app/(main)/dashboard/[organization]/(header)/settings/members/ClientPage.tsx @@ -7,8 +7,10 @@ import { useToast } from '@/components/Toast/use-toast' import { useInviteOrganizationMember, useListOrganizationMembers, + useRemoveOrganizationMember, } from '@/hooks/queries/org' import Add from '@mui/icons-material/Add' +import ClearOutlined from '@mui/icons-material/ClearOutlined' import { schemas } from '@polar-sh/client' import Avatar from '@polar-sh/ui/components/atoms/Avatar' import Button from '@polar-sh/ui/components/atoms/Button' @@ -34,6 +36,18 @@ export default function ClientPage({ hide: hideInviteMemberModal, isShown: isInviteMemberModalShown, } = useModal() + const { + show: openRemoveMemberModal, + hide: hideRemoveMemberModal, + isShown: isRemoveMemberModalShown, + } = useModal() + const [memberToRemove, setMemberToRemove] = + useState(null) + + const handleRemoveClick = (member: schemas['OrganizationMember']) => { + setMemberToRemove(member) + openRemoveMemberModal() + } const columns: DataTableColumnDef[] = [ { @@ -47,7 +61,14 @@ export default function ClientPage({ return (
-
{member.email}
+
+ {member.email} + {member.is_admin && ( + + (Admin) + + )} +
) }, @@ -62,6 +83,29 @@ export default function ClientPage({ return }, }, + { + id: 'actions', + cell: ({ row: { original: member } }) => { + // Don't show remove button for admins + if (member.is_admin) { + return null + } + return ( +
+ +
+ ) + }, + }, ] return ( @@ -100,6 +144,19 @@ export default function ClientPage({ isShown={isInviteMemberModalShown} hide={hideInviteMemberModal} /> + + + } + isShown={isRemoveMemberModalShown} + hide={hideRemoveMemberModal} + /> ) } @@ -170,3 +227,69 @@ function InviteMemberModal({ ) } + +function RemoveMemberModal({ + organizationId, + member, + onClose, +}: { + organizationId: string + member: schemas['OrganizationMember'] | null + onClose: () => void +}) { + const { toast } = useToast() + const removeMember = useRemoveOrganizationMember(organizationId) + + const handleRemove = async () => { + if (!member) return + + try { + const result = await removeMember.mutateAsync(member.user_id) + if (result.error) { + toast({ + title: 'Failed to remove member', + description: + result.error.detail || 'Failed to remove member. Please try again.', + variant: 'destructive', + }) + } else { + toast({ + title: 'Member removed', + description: `${member.email} has been removed from the organization`, + }) + onClose() + } + } catch (error) { + toast({ + title: 'Failed to remove member', + description: 'An unexpected error occurred. Please try again.', + variant: 'destructive', + }) + } + } + + if (!member) return null + + return ( +
+

Remove Member

+

+ Are you sure you want to remove {member.email} from + this organization? They will lose access to all organization resources. +

+
+ + +
+
+ ) +} From c755dbc2b16ed3e680b63e4b19bf54c9ebda49c1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Oct 2025 10:06:55 +0000 Subject: [PATCH 5/6] Fix members endpoint to properly serialize is_admin field Co-authored-by: rishi-raj-jain <46300090+rishi-raj-jain@users.noreply.github.com> --- server/polar/organization/endpoints.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/server/polar/organization/endpoints.py b/server/polar/organization/endpoints.py index 2ab6771e59..7cdb904015 100644 --- a/server/polar/organization/endpoints.py +++ b/server/polar/organization/endpoints.py @@ -288,9 +288,14 @@ async def members( items = [] for m in members: - member_dict = OrganizationMember.model_validate(m).model_dump() - member_dict["is_admin"] = m.user_id == admin_user_id - items.append(OrganizationMember.model_validate(member_dict)) + # Create a dict with all the necessary fields + member_data = { + "user_id": m.user_id, + "created_at": m.created_at, + "user": {"email": m.user.email, "avatar_url": m.user.avatar_url}, + "is_admin": m.user_id == admin_user_id, + } + items.append(OrganizationMember.model_validate(member_data)) return ListResource( items=items, From f2d12992eeca7cd61126e8e7d29ef194a339d00e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Oct 2025 08:45:25 +0000 Subject: [PATCH 6/6] Restrict member removal to organization admins only Co-authored-by: rishi-raj-jain <46300090+rishi-raj-jain@users.noreply.github.com> --- .../(header)/settings/members/ClientPage.tsx | 12 ++++- server/polar/organization/endpoints.py | 16 ++++++- server/tests/organization/test_endpoints.py | 44 +++++++++++++------ 3 files changed, 55 insertions(+), 17 deletions(-) diff --git a/clients/apps/web/src/app/(main)/dashboard/[organization]/(header)/settings/members/ClientPage.tsx b/clients/apps/web/src/app/(main)/dashboard/[organization]/(header)/settings/members/ClientPage.tsx index 8ede476e34..5df14c0a19 100644 --- a/clients/apps/web/src/app/(main)/dashboard/[organization]/(header)/settings/members/ClientPage.tsx +++ b/clients/apps/web/src/app/(main)/dashboard/[organization]/(header)/settings/members/ClientPage.tsx @@ -4,6 +4,7 @@ import { DashboardBody } from '@/components/Layout/DashboardLayout' import { Modal } from '@/components/Modal' import { useModal } from '@/components/Modal/useModal' import { useToast } from '@/components/Toast/use-toast' +import { useAuth } from '@/hooks/auth' import { useInviteOrganizationMember, useListOrganizationMembers, @@ -28,6 +29,7 @@ export default function ClientPage({ }: { organization: schemas['Organization'] }) { + const { currentUser } = useAuth() const { data: members, isLoading } = useListOrganizationMembers( organization.id, ) @@ -49,6 +51,12 @@ export default function ClientPage({ openRemoveMemberModal() } + // Check if the current user is the admin + const currentUserMember = members?.items.find( + (m) => currentUser && m.user_id === currentUser.id, + ) + const isCurrentUserAdmin = currentUserMember?.is_admin ?? false + const columns: DataTableColumnDef[] = [ { id: 'member', @@ -86,8 +94,8 @@ export default function ClientPage({ { id: 'actions', cell: ({ row: { original: member } }) => { - // Don't show remove button for admins - if (member.is_admin) { + // Only show remove button if current user is admin and the member is not an admin + if (!isCurrentUserAdmin || member.is_admin) { return null } return ( diff --git a/server/polar/organization/endpoints.py b/server/polar/organization/endpoints.py index 7cdb904015..306ff6dbff 100644 --- a/server/polar/organization/endpoints.py +++ b/server/polar/organization/endpoints.py @@ -381,12 +381,14 @@ async def remove_member( ) -> None: """Remove a member from an organization. - Only non-admin members can be removed. The organization admin cannot be removed. + Only the organization admin can remove members. + Non-admin members cannot be removed by other members. Raises: 404: Organization not found or user is not a member - 403: Cannot remove organization admin + 403: Only admin can remove members, or cannot remove organization admin """ + from polar.organization.repository import OrganizationRepository from polar.user_organization.service import ( CannotRemoveOrganizationAdmin, OrganizationNotFound, @@ -398,6 +400,16 @@ async def remove_member( if organization is None: raise ResourceNotFound() + # Check if the authenticated user is the organization admin + if not is_user(auth_subject): + raise NotPermitted("Only users can remove members") + + org_repo = OrganizationRepository.from_session(session) + admin_user = await org_repo.get_admin_user(session, organization) + + if not admin_user or admin_user.id != auth_subject.subject.id: + raise NotPermitted("Only the organization admin can remove members") + try: await user_organization_service.remove_member_safe( session, user_id, organization.id diff --git a/server/tests/organization/test_endpoints.py b/server/tests/organization/test_endpoints.py index 1e81689114..0989f3519e 100644 --- a/server/tests/organization/test_endpoints.py +++ b/server/tests/organization/test_endpoints.py @@ -645,44 +645,62 @@ async def test_anonymous( assert response.status_code == 401 @pytest.mark.auth - async def test_not_member_of_org( + async def test_not_admin( self, client: AsyncClient, organization: Organization, + user_organization: UserOrganization, user_second: User, ) -> None: + # Authenticated user is a member but not admin response = await client.delete( f"/v1/organizations/{organization.id}/members/{user_second.id}" ) - assert response.status_code == 404 + assert response.status_code == 403 @pytest.mark.auth - async def test_remove_non_member( + async def test_non_admin_cannot_remove_member( self, client: AsyncClient, session: AsyncSession, organization: Organization, user_organization: UserOrganization, - user_second: User, + user_organization_second: UserOrganization, ) -> None: - # user_second is not a member + # user_organization is the authenticated user (member but not admin) + # user_organization_second is another member to be removed + # Should fail because authenticated user is not admin + + members_before = await user_organization_service.list_by_org( + session, organization.id + ) + assert len(members_before) == 2 + response = await client.delete( - f"/v1/organizations/{organization.id}/members/{user_second.id}" + f"/v1/organizations/{organization.id}/members/{user_organization_second.user.id}" ) - assert response.status_code == 404 + assert response.status_code == 403 + + members_after = await user_organization_service.list_by_org( + session, organization.id + ) + # No members should be removed + assert len(members_after) == len(members_before) @pytest.mark.auth - async def test_remove_member_success( + async def test_admin_can_remove_member( self, client: AsyncClient, session: AsyncSession, organization: Organization, + organization_account: Account, user_organization: UserOrganization, user_organization_second: UserOrganization, + save_fixture: SaveFixture, ) -> None: - # user_organization is the authenticated user (member of org) + # user_organization is the authenticated user (admin through organization_account) # user_organization_second is another member to be removed members_before = await user_organization_service.list_by_org( @@ -713,15 +731,14 @@ async def test_cannot_remove_admin( user_organization_second: UserOrganization, save_fixture: SaveFixture, ) -> None: - # Make user the admin by linking the account - # The organization_account fixture already has user as admin - # Just need to ensure user_organization exists + # user_organization is the authenticated user and admin + # Try to remove themselves (the admin) members_before = await user_organization_service.list_by_org( session, organization.id ) - # Try to remove the admin user (the fixture user is the admin through organization_account) + # Try to remove the admin user (themselves) response = await client.delete( f"/v1/organizations/{organization.id}/members/{user_organization.user.id}" ) @@ -746,3 +763,4 @@ async def test_organization_not_found( assert response.status_code == 404 +