Skip to content
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

Proper user deletion / organization leaving #3460

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
2 changes: 2 additions & 0 deletions backend/ee/onyx/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
basic_router as enterprise_settings_router,
)
from ee.onyx.server.manage.standard_answer import router as standard_answer_router
from ee.onyx.server.manage.users import router as manage_users_router
from ee.onyx.server.middleware.tenant_tracking import add_tenant_id_middleware
from ee.onyx.server.oauth import router as oauth_router
from ee.onyx.server.query_and_chat.chat_backend import (
Expand Down Expand Up @@ -111,6 +112,7 @@ def get_application() -> FastAPI:
elif AUTH_TYPE == AuthType.SAML:
include_router_with_global_prefix_prepended(application, saml_router)

include_router_with_global_prefix_prepended(application, manage_users_router)
# RBAC / group access control
include_router_with_global_prefix_prepended(application, user_group_router)
# Analytics endpoints
Expand Down
34 changes: 34 additions & 0 deletions backend/ee/onyx/server/manage/users.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
from fastapi import APIRouter
from fastapi import Depends
from fastapi import HTTPException
from sqlalchemy.orm import Session

from ee.onyx.server.tenants.user_mapping import remove_users_from_tenant
from onyx.auth.users import current_admin_user
from onyx.db.engine import get_current_tenant_id
from onyx.db.engine import get_session
from onyx.db.models import User
from onyx.db.users import delete_user_from_db
from onyx.db.users import get_user_by_email
from onyx.server.manage.models import UserByEmail
from onyx.utils.logger import setup_logger

logger = setup_logger()
router = APIRouter()


@router.post("/manage/admin/leave-organization")
async def leave_organization(
user_email: UserByEmail,
_: User | None = Depends(current_admin_user),
db_session: Session = Depends(get_session),
tenant_id: str = Depends(get_current_tenant_id),
) -> None:
user_to_delete = get_user_by_email(
email=user_email.user_email, db_session=db_session
)
if not user_to_delete:
raise HTTPException(status_code=404, detail="User not found")

delete_user_from_db(user_to_delete, db_session)
remove_users_from_tenant([user_to_delete.email], tenant_id)
1 change: 1 addition & 0 deletions backend/ee/onyx/server/tenants/provisioning.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ async def rollback_tenant_provisioning(tenant_id: str) -> None:
try:
# Drop the tenant's schema to rollback provisioning
drop_schema(tenant_id)

# Remove tenant mapping
with Session(get_sqlalchemy_engine()) as db_session:
db_session.query(UserTenantMapping).filter(
Expand Down
47 changes: 47 additions & 0 deletions backend/onyx/db/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,15 @@
from sqlalchemy import select
from sqlalchemy.orm import Session

from onyx.auth.invited_users import get_invited_users
from onyx.auth.invited_users import write_invited_users
from onyx.auth.schemas import UserRole
from onyx.db.models import DocumentSet__User
from onyx.db.models import Persona__User
from onyx.db.models import SamlAccount
from onyx.db.models import User
from onyx.db.models import User__UserGroup
from onyx.utils.variable_functionality import fetch_ee_implementation_or_noop


def validate_user_role_update(requested_role: UserRole, current_role: UserRole) -> None:
Expand Down Expand Up @@ -185,3 +192,43 @@ def batch_add_ext_perm_user_if_not_exists(
db_session.commit()

return found_users + new_users


def delete_user_from_db(
user_to_delete: User,
db_session: Session,
) -> None:
for oauth_account in user_to_delete.oauth_accounts:
db_session.delete(oauth_account)

fetch_ee_implementation_or_noop(
"onyx.db.external_perm",
"delete_user__ext_group_for_user__no_commit",
)(
db_session=db_session,
user_id=user_to_delete.id,
)
db_session.query(SamlAccount).filter(
SamlAccount.user_id == user_to_delete.id
).delete()
db_session.query(DocumentSet__User).filter(
DocumentSet__User.user_id == user_to_delete.id
).delete()
db_session.query(Persona__User).filter(
Persona__User.user_id == user_to_delete.id
).delete()
db_session.query(User__UserGroup).filter(
User__UserGroup.user_id == user_to_delete.id
).delete()
db_session.delete(user_to_delete)
db_session.commit()

# NOTE: edge case may exist with race conditions
# with this `invited user` scheme generally.
user_emails = get_invited_users()
remaining_users = [
remaining_user_email
for remaining_user_email in user_emails
if remaining_user_email != user_to_delete.email
]
write_invited_users(remaining_users)
44 changes: 3 additions & 41 deletions backend/onyx/server/manage/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,8 @@
from onyx.db.engine import CURRENT_TENANT_ID_CONTEXTVAR
from onyx.db.engine import get_session
from onyx.db.models import AccessToken
from onyx.db.models import DocumentSet__User
from onyx.db.models import Persona__User
from onyx.db.models import SamlAccount
from onyx.db.models import User
from onyx.db.models import User__UserGroup
from onyx.db.users import delete_user_from_db
from onyx.db.users import get_user_by_email
from onyx.db.users import list_users
from onyx.db.users import validate_user_role_update
Expand Down Expand Up @@ -370,45 +367,10 @@ async def delete_user(
db_session.expunge(user_to_delete)

try:
for oauth_account in user_to_delete.oauth_accounts:
db_session.delete(oauth_account)

fetch_ee_implementation_or_noop(
"onyx.db.external_perm",
"delete_user__ext_group_for_user__no_commit",
)(
db_session=db_session,
user_id=user_to_delete.id,
)
db_session.query(SamlAccount).filter(
SamlAccount.user_id == user_to_delete.id
).delete()
db_session.query(DocumentSet__User).filter(
DocumentSet__User.user_id == user_to_delete.id
).delete()
db_session.query(Persona__User).filter(
Persona__User.user_id == user_to_delete.id
).delete()
db_session.query(User__UserGroup).filter(
User__UserGroup.user_id == user_to_delete.id
).delete()
db_session.delete(user_to_delete)
db_session.commit()

# NOTE: edge case may exist with race conditions
# with this `invited user` scheme generally.
user_emails = get_invited_users()
remaining_users = [
user for user in user_emails if user != user_email.user_email
]
write_invited_users(remaining_users)

delete_user_from_db(user_to_delete, db_session)
logger.info(f"Deleted user {user_to_delete.email}")
except Exception as e:
import traceback

full_traceback = traceback.format_exc()
logger.error(f"Full stack trace:\n{full_traceback}")
except Exception as e:
db_session.rollback()
logger.error(f"Error deleting user {user_to_delete.email}: {str(e)}")
raise HTTPException(status_code=500, detail="Error deleting user")
Expand Down
33 changes: 25 additions & 8 deletions web/src/components/admin/users/SignedUpUserTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ import { TableHeader } from "@/components/ui/table";
import { UserRoleDropdown } from "./buttons/UserRoleDropdown";
import { DeleteUserButton } from "./buttons/DeleteUserButton";
import { DeactivaterButton } from "./buttons/DeactivaterButton";
import { useUser } from "@/components/user/UserProvider";
import { LeaveOrganizationButton } from "./buttons/LeaveOrganizationButton";
import { NEXT_PUBLIC_CLOUD_ENABLED } from "@/lib/constants";

interface Props {
users: Array<User>;
Expand All @@ -28,6 +31,8 @@ const SignedUpUserTable = ({
onPageChange,
mutate,
}: Props & PageSelectorProps) => {
const { user: currentUser } = useUser();

if (!users.length) return null;

const handlePopup = (message: string, type: "success" | "error") => {
Expand Down Expand Up @@ -81,18 +86,30 @@ const SignedUpUserTable = ({
</TableCell>
<TableCell>
<div className="flex justify-end gap-x-2">
<DeactivaterButton
user={user}
deactivate={user.status === UserStatus.live}
setPopup={setPopup}
mutate={mutate}
/>
{user.status == UserStatus.deactivated && (
<DeleteUserButton
{NEXT_PUBLIC_CLOUD_ENABLED &&
user.id === currentUser?.id ? (
<LeaveOrganizationButton
user={user}
setPopup={setPopup}
mutate={mutate}
/>
) : (
<>
<DeactivaterButton
user={user}
deactivate={user.status === UserStatus.live}
setPopup={setPopup}
mutate={mutate}
/>

{user.status == UserStatus.deactivated && (
<DeleteUserButton
user={user}
setPopup={setPopup}
mutate={mutate}
/>
)}
</>
)}
</div>
</TableCell>
Expand Down
30 changes: 1 addition & 29 deletions web/src/components/admin/users/buttons/DeactivaterButton.tsx
Original file line number Diff line number Diff line change
@@ -1,36 +1,8 @@
import {
type User,
UserStatus,
UserRole,
USER_ROLE_LABELS,
INVALID_ROLE_HOVER_TEXT,
} from "@/lib/types";
import { type PageSelectorProps } from "@/components/PageSelector";
import { HidableSection } from "@/app/admin/assistants/HidableSection";
import { type User } from "@/lib/types";
import { PopupSpec } from "@/components/admin/connectors/Popup";
import userMutationFetcher from "@/lib/admin/users/userMutationFetcher";
import useSWRMutation from "swr/mutation";
import {
Table,
TableHead,
TableRow,
TableBody,
TableCell,
} from "@/components/ui/table";

import {
Select,
SelectContent,
SelectItem,
SelectTrigger,
SelectValue,
} from "@/components/ui/select";
import { Button } from "@/components/ui/button";
import { GenericConfirmModal } from "@/components/modals/GenericConfirmModal";
import { useState } from "react";
import { usePaidEnterpriseFeaturesEnabled } from "@/components/settings/usePaidEnterpriseFeaturesEnabled";
import { DeleteEntityModal } from "@/components/modals/DeleteEntityModal";
import { TableHeader } from "@/components/ui/table";

export const DeactivaterButton = ({
user,
Expand Down
66 changes: 66 additions & 0 deletions web/src/components/admin/users/buttons/LeaveOrganizationButton.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import { type User } from "@/lib/types";
import { PopupSpec } from "@/components/admin/connectors/Popup";
import userMutationFetcher from "@/lib/admin/users/userMutationFetcher";
import useSWRMutation from "swr/mutation";
import { Button } from "@/components/ui/button";
import { useState } from "react";
import { DeleteEntityModal } from "@/components/modals/DeleteEntityModal";
export const LeaveOrganizationButton = ({
user,
setPopup,
mutate,
}: {
user: User;
setPopup: (spec: PopupSpec) => void;
mutate: () => void;
}) => {
const { trigger, isMutating } = useSWRMutation(
"/api/manage/admin/leave-organization",
userMutationFetcher,
{
onSuccess: () => {
mutate();
setPopup({
message: "Successfully left the organization!",
type: "success",
});
},
onError: (errorMsg) =>
setPopup({
message: `Unable to leave organization - ${errorMsg}`,
type: "error",
}),
}
);

const [showLeaveModal, setShowLeaveModal] = useState(false);

const handleLeaveOrganization = () => {
trigger({ user_email: user.email, method: "POST" });
};

return (
<>
{showLeaveModal && (
<DeleteEntityModal
deleteButtonText="Leave"
entityType="organization"
entityName="your organization"
onClose={() => setShowLeaveModal(false)}
onSubmit={handleLeaveOrganization}
additionalDetails="You will lose access to all organization data and resources."
/>
)}

<Button
className="w-min"
onClick={() => setShowLeaveModal(true)}
disabled={isMutating}
size="sm"
variant="destructive"
>
Leave Organization
</Button>
</>
);
};
11 changes: 8 additions & 3 deletions web/src/components/modals/DeleteEntityModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,34 @@ export const DeleteEntityModal = ({
entityType,
entityName,
additionalDetails,
deleteButtonText,
}: {
entityType: string;
entityName: string;
onClose: () => void;
onSubmit: () => void;
additionalDetails?: string;
deleteButtonText?: string;
}) => {
return (
<Modal onOutsideClick={onClose}>
<>
<div className="flex mb-4">
<h2 className="my-auto text-2xl font-bold">Delete {entityType}?</h2>
<h2 className="my-auto text-2xl font-bold">
{deleteButtonText || `Delete`} {entityType}
</h2>
</div>
<p className="mb-4">
Click below to confirm that you want to delete <b>{entityName}</b>
Click below to confirm that you want to {deleteButtonText || "delete"}{" "}
<b>{entityName}</b>
</p>
{additionalDetails && <p className="mb-4">{additionalDetails}</p>}
<div className="flex">
<div className="mx-auto">
<BasicClickable onClick={onSubmit}>
<div className="flex mx-2">
<FiTrash className="my-auto mr-2" />
Delete
{deleteButtonText || "Delete"}
</div>
</BasicClickable>
</div>
Expand Down
Loading