nit

backend/ee/onyx/server/saml.py

@@ -36,6 +36,9 @@ from onyx.utils.logger import setup_logger
 logger = setup_logger()
 router = APIRouter(prefix="/auth/saml")
 
+# Define non-authenticated user roles that should be re-created during SAML login
+NON_AUTHENTICATED_ROLES = {UserRole.SLACK_USER, UserRole.EXT_PERM_USER}
+
 
 async def upsert_saml_user(email: str) -> User:
     logger.debug(f"Attempting to upsert SAML user with email: {email}")
@@ -51,7 +54,7 @@ async def upsert_saml_user(email: str) -> User:
                 try:
                     user = await user_manager.get_by_email(email)
                     # If user has a non-authenticated role, treat as non-existent
-                    if not user.role.is_web_login:
+                    if user.role in NON_AUTHENTICATED_ROLES:
                         raise exceptions.UserNotExists()
                     return user
                 except exceptions.UserNotExists:

backend/onyx/auth/schemas.py

@@ -26,7 +26,6 @@ class UserRole(str, Enum):
     SLACK_USER = "slack_user"
     EXT_PERM_USER = "ext_perm_user"
 
-    @property
     def is_web_login(self) -> bool:
         return self not in [
             UserRole.SLACK_USER,

backend/onyx/auth/users.py

@@ -319,7 +319,7 @@ class UserManager(UUIDIDMixin, BaseUserManager[User, uuid.UUID]):
             except exceptions.UserAlreadyExists:
                 user = await self.get_by_email(user_create.email)
                 # Handle case where user has used product outside of web and is now creating an account through web
-                if not user.role.is_web_login and user_create.role.is_web_login:
+                if not user.role.is_web_login() and user_create.role.is_web_login():
                     user_update = UserUpdateWithRole(
                         password=user_create.password,
                         is_verified=user_create.is_verified,
@@ -490,7 +490,7 @@ class UserManager(UUIDIDMixin, BaseUserManager[User, uuid.UUID]):
                 )
 
             # Handle case where user has used product outside of web and is now creating an account through web
-            if not user.role.is_web_login:
+            if not user.role.is_web_login():
                 await self.user_db.update(
                     user,
                     {
@@ -629,7 +629,7 @@ class UserManager(UUIDIDMixin, BaseUserManager[User, uuid.UUID]):
                 self.password_helper.hash(credentials.password)
                 return None
 
-            if not user.role.is_web_login:
+            if not user.role.is_web_login():
                 raise BasicAuthenticationError(
                     detail="NO_WEB_LOGIN_AND_HAS_NO_PASSWORD",
                 )

backend/onyx/background/celery/tasks/connector_deletion/tasks.py

@@ -25,14 +25,8 @@ from onyx.configs.constants import OnyxRedisLocks
 from onyx.configs.constants import OnyxRedisSignals
 from onyx.db.connector import fetch_connector_by_id
 from onyx.db.connector_credential_pair import add_deletion_failure_message
-from onyx.db.connector_credential_pair import (
-    delete_connector_credential_pair__no_commit,
-)
 from onyx.db.connector_credential_pair import get_connector_credential_pair_from_id
 from onyx.db.connector_credential_pair import get_connector_credential_pairs
-from onyx.db.document import (
-    delete_all_documents_by_connector_credential_pair__no_commit,
-)
 from onyx.db.document import get_document_ids_for_connector_credential_pair
 from onyx.db.document_set import delete_document_set_cc_pair_relationship__no_commit
 from onyx.db.engine import get_session_with_current_tenant
@@ -449,27 +443,15 @@ def monitor_connector_deletion_taskset(
             connector_id_to_delete = cc_pair.connector_id
             credential_id_to_delete = cc_pair.credential_id
 
-            # Explicitly delete document by connector credential pair records before deleting the connector
-            # This is needed because connector_id is a primary key in that table and cascading deletes won't work
-            delete_all_documents_by_connector_credential_pair__no_commit(
-                db_session=db_session,
-                connector_id=connector_id_to_delete,
-                credential_id=credential_id_to_delete,
-            )
+            # No need to explicitly delete DocumentByConnectorCredentialPair records anymore
+            # as we have proper cascade relationships set up in the models
 
-            # Flush to ensure document deletion happens before connector deletion
+            # Flush to ensure all operations happen in sequence
             db_session.flush()
 
-            # Expire the cc_pair to ensure SQLAlchemy doesn't try to manage its state
-            # related to the deleted DocumentByConnectorCredentialPair during commit
-            db_session.expire(cc_pair)
+            # Delete the cc-pair directly
+            db_session.delete(cc_pair)
 
-            # finally, delete the cc-pair
-            delete_connector_credential_pair__no_commit(
-                db_session=db_session,
-                connector_id=connector_id_to_delete,
-                credential_id=credential_id_to_delete,
-            )
             # if there are no credentials left, delete the connector
             connector = fetch_connector_by_id(
                 db_session=db_session,

backend/onyx/db/document.py

@@ -555,28 +555,6 @@ def delete_documents_by_connector_credential_pair__no_commit(
     db_session.execute(stmt)
 
 
-def delete_all_documents_by_connector_credential_pair__no_commit(
-    db_session: Session,
-    connector_id: int,
-    credential_id: int,
-) -> None:
-    """Deletes all document by connector credential pair entries for a specific connector and credential.
-    This is primarily used during connector deletion to ensure all references are removed
-    before deleting the connector itself. This is crucial because connector_id is part of the
-    primary key in DocumentByConnectorCredentialPair, and attempting to delete the Connector
-    would otherwise try to set the foreign key to NULL, which fails for primary keys.
-
-    NOTE: Does not commit the transaction, this must be done by the caller.
-    """
-    stmt = delete(DocumentByConnectorCredentialPair).where(
-        and_(
-            DocumentByConnectorCredentialPair.connector_id == connector_id,
-            DocumentByConnectorCredentialPair.credential_id == credential_id,
-        )
-    )
-    db_session.execute(stmt)
-
-
 def delete_documents__no_commit(db_session: Session, document_ids: list[str]) -> None:
     db_session.execute(delete(DbDocument).where(DbDocument.id.in_(document_ids)))
 

backend/onyx/db/models.py

@@ -694,7 +694,11 @@ class Connector(Base):
     )
     documents_by_connector: Mapped[
         list["DocumentByConnectorCredentialPair"]
-    ] = relationship("DocumentByConnectorCredentialPair", back_populates="connector")
+    ] = relationship(
+        "DocumentByConnectorCredentialPair",
+        back_populates="connector",
+        cascade="all, delete-orphan",
+    )
 
     # synchronize this validation logic with RefreshFrequencySchema etc on front end
     # until we have a centralized validation schema
@@ -748,7 +752,11 @@ class Credential(Base):
     )
     documents_by_credential: Mapped[
         list["DocumentByConnectorCredentialPair"]
-    ] = relationship("DocumentByConnectorCredentialPair", back_populates="credential")
+    ] = relationship(
+        "DocumentByConnectorCredentialPair",
+        back_populates="credential",
+        cascade="all, delete-orphan",
+    )
 
     user: Mapped[User | None] = relationship("User", back_populates="credentials")