Make curators able to create permission synced connectors (#3126)

* Make curators able to create permission synced connectors

* removed editing permission synced connectors for curators

* updated tests to use access type instead of is_public

* update copy
This commit is contained in:
hagen-danswer 2024-11-13 10:58:23 -08:00 committed by GitHub
parent 60471b6a73
commit a50a3944b3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
25 changed files with 102 additions and 72 deletions

View File

@ -78,6 +78,7 @@ def _add_user_filters(
)
else:
where_clause |= ConnectorCredentialPair.access_type == AccessType.PUBLIC
where_clause |= ConnectorCredentialPair.access_type == AccessType.SYNC
return stmt.where(where_clause)

View File

@ -387,6 +387,7 @@ def associate_credential_to_connector(
user=user,
target_group_ids=metadata.groups,
object_is_public=metadata.access_type == AccessType.PUBLIC,
object_is_perm_sync=metadata.access_type == AccessType.SYNC,
)
try:

View File

@ -81,7 +81,6 @@ from danswer.db.index_attempt import get_latest_index_attempts_by_status
from danswer.db.models import IndexingStatus
from danswer.db.models import SearchSettings
from danswer.db.models import User
from danswer.db.models import UserRole
from danswer.db.search_settings import get_current_search_settings
from danswer.db.search_settings import get_secondary_search_settings
from danswer.file_store.file_store import get_default_file_store
@ -665,7 +664,8 @@ def create_connector_from_model(
db_session=db_session,
user=user,
target_group_ids=connector_data.groups,
object_is_public=connector_data.is_public,
object_is_public=connector_data.access_type == AccessType.PUBLIC,
object_is_perm_sync=connector_data.access_type == AccessType.SYNC,
)
connector_base = connector_data.to_connector_base()
return create_connector(
@ -683,32 +683,31 @@ def create_connector_with_mock_credential(
user: User = Depends(current_curator_or_admin_user),
db_session: Session = Depends(get_session),
) -> StatusResponse:
if user and user.role != UserRole.ADMIN:
if connector_data.is_public:
raise HTTPException(
status_code=401,
detail="User does not have permission to create public credentials",
)
if not connector_data.groups:
raise HTTPException(
status_code=401,
detail="Curators must specify 1+ groups",
)
fetch_ee_implementation_or_noop(
"danswer.db.user_group", "validate_user_creation_permissions", None
)(
db_session=db_session,
user=user,
target_group_ids=connector_data.groups,
object_is_public=connector_data.access_type == AccessType.PUBLIC,
object_is_perm_sync=connector_data.access_type == AccessType.SYNC,
)
try:
_validate_connector_allowed(connector_data.source)
connector_response = create_connector(
db_session=db_session, connector_data=connector_data
db_session=db_session,
connector_data=connector_data,
)
mock_credential = CredentialBase(
credential_json={}, admin_public=True, source=connector_data.source
credential_json={},
admin_public=True,
source=connector_data.source,
)
credential = create_credential(
mock_credential, user=user, db_session=db_session
)
access_type = (
AccessType.PUBLIC if connector_data.is_public else AccessType.PRIVATE
credential_data=mock_credential,
user=user,
db_session=db_session,
)
response = add_credential_to_connector(
@ -716,7 +715,7 @@ def create_connector_with_mock_credential(
user=user,
connector_id=cast(int, connector_response.id), # will aways be an int
credential_id=credential.id,
access_type=access_type,
access_type=connector_data.access_type,
cc_pair_name=connector_data.name,
groups=connector_data.groups,
)
@ -741,7 +740,8 @@ def update_connector_from_model(
db_session=db_session,
user=user,
target_group_ids=connector_data.groups,
object_is_public=connector_data.is_public,
object_is_public=connector_data.access_type == AccessType.PUBLIC,
object_is_perm_sync=connector_data.access_type == AccessType.SYNC,
)
connector_base = connector_data.to_connector_base()
except ValueError as e:

View File

@ -64,11 +64,11 @@ class ConnectorBase(BaseModel):
class ConnectorUpdateRequest(ConnectorBase):
is_public: bool = True
access_type: AccessType
groups: list[int] = Field(default_factory=list)
def to_connector_base(self) -> ConnectorBase:
return ConnectorBase(**self.model_dump(exclude={"is_public", "groups"}))
return ConnectorBase(**self.model_dump(exclude={"access_type", "groups"}))
class ConnectorSnapshot(ConnectorBase):

View File

@ -124,16 +124,21 @@ def _cleanup_document_set__user_group_relationships__no_commit(
def validate_user_creation_permissions(
db_session: Session,
user: User | None,
target_group_ids: list[int] | None,
object_is_public: bool | None,
target_group_ids: list[int] | None = None,
object_is_public: bool | None = None,
object_is_perm_sync: bool | None = None,
) -> None:
"""
All users can create/edit permission synced objects if they don't specify a group
All admin actions are allowed.
Prevents non-admins from creating/editing:
- public objects
- objects with no groups
- objects that belong to a group they don't curate
"""
if object_is_perm_sync and not target_group_ids:
return
if not user or user.role == UserRole.ADMIN:
return

View File

@ -72,7 +72,7 @@ class CCPairManager:
source=source,
input_type=input_type,
connector_specific_config=connector_specific_config,
is_public=(access_type == AccessType.PUBLIC),
access_type=access_type,
groups=groups,
user_performing_action=user_performing_action,
)

View File

@ -4,6 +4,7 @@ from uuid import uuid4
import requests
from danswer.connectors.models import InputType
from danswer.db.enums import AccessType
from danswer.server.documents.models import ConnectorUpdateRequest
from danswer.server.documents.models import DocumentSource
from tests.integration.common_utils.constants import API_SERVER_URL
@ -19,7 +20,7 @@ class ConnectorManager:
source: DocumentSource = DocumentSource.FILE,
input_type: InputType = InputType.LOAD_STATE,
connector_specific_config: dict[str, Any] | None = None,
is_public: bool = True,
access_type: AccessType = AccessType.PUBLIC,
groups: list[int] | None = None,
user_performing_action: DATestUser | None = None,
) -> DATestConnector:
@ -30,7 +31,7 @@ class ConnectorManager:
source=source,
input_type=input_type,
connector_specific_config=connector_specific_config or {},
is_public=is_public,
access_type=access_type,
groups=groups or [],
)
@ -51,7 +52,7 @@ class ConnectorManager:
input_type=input_type,
connector_specific_config=connector_specific_config or {},
groups=groups,
is_public=is_public,
access_type=access_type,
)
@staticmethod

View File

@ -55,7 +55,7 @@ class DATestConnector(BaseModel):
input_type: InputType
connector_specific_config: dict[str, Any]
groups: list[int] | None = None
is_public: bool | None = None
access_type: AccessType | None = None
class SimpleTestDocument(BaseModel):

View File

@ -67,7 +67,7 @@ def test_slack_permission_sync(
"workspace": "onyx-test-workspace",
"channels": [public_channel["name"], private_channel["name"]],
},
is_public=True,
access_type=AccessType.SYNC,
groups=[],
user_performing_action=admin_user,
)

View File

@ -64,7 +64,7 @@ def test_slack_prune(
"workspace": "onyx-test-workspace",
"channels": [public_channel["name"], private_channel["name"]],
},
is_public=True,
access_type=AccessType.PUBLIC,
groups=[],
user_performing_action=admin_user,
)

View File

@ -26,7 +26,7 @@ def test_tenant_creation(reset_multitenant: None) -> None:
test_connector = ConnectorManager.create(
name="admin_test_connector",
source=DocumentSource.FILE,
is_public=False,
access_type=AccessType.PRIVATE,
user_performing_action=test_user,
)

View File

@ -50,12 +50,11 @@ def test_cc_pair_permissions(reset: None) -> None:
user_groups_to_check=[user_group_1], user_performing_action=admin_user
)
# Create a credentials that the curator is and is not curator of
connector_1 = ConnectorManager.create(
name="curator_owned_connector",
name="admin_owned_connector",
source=DocumentSource.CONFLUENCE,
groups=[user_group_1.id],
is_public=False,
access_type=AccessType.PRIVATE,
user_performing_action=admin_user,
)
# currently we dont enforce permissions at the connector level
@ -67,6 +66,7 @@ def test_cc_pair_permissions(reset: None) -> None:
# is_public=False,
# user_performing_action=admin_user,
# )
# Create a credentials that the curator is and is not curator of
credential_1 = CredentialManager.create(
name="curator_owned_credential",
source=DocumentSource.CONFLUENCE,

View File

@ -5,6 +5,7 @@ the permissions of the curator manipulating connectors.
import pytest
from requests.exceptions import HTTPError
from danswer.db.enums import AccessType
from danswer.server.documents.models import DocumentSource
from tests.integration.common_utils.managers.connector import ConnectorManager
from tests.integration.common_utils.managers.user import DATestUser
@ -57,7 +58,7 @@ def test_connector_permissions(reset: None) -> None:
name="invalid_connector_1",
source=DocumentSource.CONFLUENCE,
groups=[user_group_1.id],
is_public=True,
access_type=AccessType.PUBLIC,
user_performing_action=curator,
)
@ -68,7 +69,7 @@ def test_connector_permissions(reset: None) -> None:
name="invalid_connector_2",
source=DocumentSource.CONFLUENCE,
groups=[user_group_1.id, user_group_2.id],
is_public=False,
access_type=AccessType.PRIVATE,
user_performing_action=curator,
)
@ -80,7 +81,7 @@ def test_connector_permissions(reset: None) -> None:
name="valid_connector",
source=DocumentSource.CONFLUENCE,
groups=[user_group_1.id],
is_public=False,
access_type=AccessType.PRIVATE,
user_performing_action=curator,
)
assert valid_connector.id is not None
@ -121,7 +122,7 @@ def test_connector_permissions(reset: None) -> None:
name="invalid_connector_3",
source=DocumentSource.CONFLUENCE,
groups=[user_group_2.id],
is_public=False,
access_type=AccessType.PRIVATE,
user_performing_action=curator,
)
@ -131,6 +132,6 @@ def test_connector_permissions(reset: None) -> None:
name="invalid_connector_4",
source=DocumentSource.CONFLUENCE,
groups=[user_group_1.id],
is_public=True,
access_type=AccessType.PUBLIC,
user_performing_action=curator,
)

View File

@ -51,7 +51,7 @@ def test_whole_curator_flow(reset: None) -> None:
test_connector = ConnectorManager.create(
name="curator_test_connector",
source=DocumentSource.FILE,
is_public=False,
access_type=AccessType.PRIVATE,
groups=[user_group_1.id],
user_performing_action=curator,
)
@ -130,7 +130,7 @@ def test_global_curator_flow(reset: None) -> None:
test_connector = ConnectorManager.create(
name="curator_test_connector",
source=DocumentSource.FILE,
is_public=False,
access_type=AccessType.PRIVATE,
groups=[user_group_1.id],
user_performing_action=global_curator,
)

View File

@ -214,9 +214,11 @@ function Main({ ccPairId }: { ccPairId: number }) {
</div>
{!ccPair.is_editable_for_current_user && (
<div className="text-sm mt-2 text-neutral-500 italic">
{ccPair.is_public
{ccPair.access_type === "public"
? "Public connectors are not editable by curators."
: "This connector belongs to groups where you don't have curator permissions, so it's not editable."}
: ccPair.access_type === "sync"
? "Sync connectors are not editable by curators unless the curator is also the owner."
: "This connector belongs to groups where you don't have curator permissions, so it's not editable."}
</div>
)}

View File

@ -4,6 +4,7 @@ import {
DeletionAttemptSnapshot,
IndexAttemptSnapshot,
ValidStatuses,
AccessType,
} from "@/lib/types";
export enum ConnectorCredentialPairStatus {
@ -22,7 +23,7 @@ export interface CCPairFullInfo {
number_of_index_attempts: number;
last_index_attempt_status: ValidStatuses | null;
latest_deletion_attempt: DeletionAttemptSnapshot | null;
is_public: boolean;
access_type: AccessType;
is_editable_for_current_user: boolean;
deletion_failure_message: string | null;
indexing: boolean;

View File

@ -54,8 +54,7 @@ const BASE_CONNECTOR_URL = "/api/manage/admin/connector";
export async function submitConnector<T>(
connector: ConnectorBase<T>,
connectorId?: number,
fakeCredential?: boolean,
isPublicCcpair?: boolean // exclusively for mock credentials, when also need to specify ccpair details
fakeCredential?: boolean
): Promise<{ message: string; isSuccess: boolean; response?: Connector<T> }> {
const isUpdate = connectorId !== undefined;
if (!connector.connector_specific_config) {
@ -71,7 +70,7 @@ export async function submitConnector<T>(
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify({ ...connector, is_public: isPublicCcpair }),
body: JSON.stringify({ ...connector }),
}
);
if (response.ok) {
@ -268,7 +267,7 @@ export default function AddConnector({
advancedConfiguration.refreshFreq,
advancedConfiguration.pruneFreq,
advancedConfiguration.indexingStart,
values.access_type == "public",
values.access_type,
groups,
name
);
@ -285,7 +284,7 @@ export default function AddConnector({
setPopup,
setSelectedFiles,
name,
access_type == "public",
access_type,
groups
);
if (response) {
@ -300,15 +299,14 @@ export default function AddConnector({
input_type: isLoadState(connector) ? "load_state" : "poll", // single case
name: name,
source: connector,
is_public: access_type == "public",
access_type: access_type,
refresh_freq: advancedConfiguration.refreshFreq || null,
prune_freq: advancedConfiguration.pruneFreq || null,
indexing_start: advancedConfiguration.indexingStart || null,
groups: groups,
},
undefined,
credentialActivated ? false : true,
access_type == "public"
credentialActivated ? false : true
);
// If no credential
if (!credentialActivated) {

View File

@ -164,7 +164,7 @@ const DynamicConnectionForm: FC<DynamicConnectionFormProps> = ({
)}
<AccessTypeForm connector={connector} />
<AccessTypeGroupSelector />
<AccessTypeGroupSelector connector={connector} />
{config.advanced_values.length > 0 && (
<>

View File

@ -2,13 +2,14 @@ import { PopupSpec } from "@/components/admin/connectors/Popup";
import { createConnector, runConnector } from "@/lib/connector";
import { createCredential, linkCredential } from "@/lib/credential";
import { FileConfig } from "@/lib/connectors/connectors";
import { AccessType } from "@/lib/types";
export const submitFiles = async (
selectedFiles: File[],
setPopup: (popup: PopupSpec) => void,
setSelectedFiles: (files: File[]) => void,
name: string,
isPublic: boolean,
access_type: string,
groups?: number[]
) => {
const formData = new FormData();
@ -42,7 +43,7 @@ export const submitFiles = async (
refresh_freq: null,
prune_freq: null,
indexing_start: null,
is_public: isPublic,
access_type: access_type,
groups: groups,
});
if (connectorErrorMsg || !connector) {
@ -61,7 +62,7 @@ export const submitFiles = async (
credential_json: {},
admin_public: true,
source: "file",
curator_public: isPublic,
curator_public: true,
groups: groups,
name,
});
@ -80,7 +81,7 @@ export const submitFiles = async (
connector.id,
credentialId,
name,
isPublic ? "public" : "private",
access_type as AccessType,
groups
);
if (!credentialResponse.ok) {

View File

@ -10,7 +10,7 @@ export const submitGoogleSite = async (
refreshFreq: number,
pruneFreq: number,
indexingStart: Date,
is_public: boolean,
access_type: string,
groups: number[],
name?: string
) => {
@ -44,7 +44,7 @@ export const submitGoogleSite = async (
base_url: base_url,
zip_path: filePaths[0],
},
is_public: is_public,
access_type: access_type,
refresh_freq: refreshFreq,
prune_freq: pruneFreq,
indexing_start: indexingStart,

View File

@ -394,6 +394,7 @@ export function CCPairIndexingStatusTable({
indexing_start: new Date("2023-07-01T12:00:00Z"),
id: 1,
credential_ids: [],
access_type: "public",
time_created: "2023-07-01T12:00:00Z",
time_updated: "2023-07-01T12:00:00Z",
},

View File

@ -62,7 +62,7 @@ export function AccessTypeForm({
});
}
if (isAutoSyncSupported && isAdmin && isPaidEnterpriseEnabled) {
if (isAutoSyncSupported && isPaidEnterpriseEnabled) {
options.push({
name: "Auto Sync Permissions",
value: "sync",
@ -73,7 +73,7 @@ export function AccessTypeForm({
return (
<>
{isPaidEnterpriseEnabled && isAdmin && (
{isPaidEnterpriseEnabled && (isAdmin || isAutoSyncSupported) && (
<>
<div>
<label className="text-text-950 font-medium">Document Access</label>

View File

@ -6,9 +6,20 @@ import { Separator } from "@/components/ui/separator";
import { FiUsers } from "react-icons/fi";
import { UserGroup, UserRole } from "@/lib/types";
import { useUserGroups } from "@/lib/hooks";
import { AccessType } from "@/lib/types";
import {
AccessType,
ValidAutoSyncSources,
ConfigurableSources,
validAutoSyncSources,
} from "@/lib/types";
import { useUser } from "@/components/user/UserProvider";
function isValidAutoSyncSource(
value: ConfigurableSources
): value is ValidAutoSyncSources {
return validAutoSyncSources.includes(value as ValidAutoSyncSources);
}
// This should be included for all forms that require groups / public access
// to be set, and access to this / permissioning should be handled within this component itself.
@ -17,11 +28,16 @@ export type AccessTypeGroupSelectorFormType = {
groups: number[];
};
export function AccessTypeGroupSelector({}: {}) {
export function AccessTypeGroupSelector({
connector,
}: {
connector: ConfigurableSources;
}) {
const { data: userGroups, isLoading: userGroupsIsLoading } = useUserGroups();
const { isAdmin, user, isLoadingUser, isCurator } = useUser();
const isPaidEnterpriseFeaturesEnabled = usePaidEnterpriseFeaturesEnabled();
const [shouldHideContent, setShouldHideContent] = useState(false);
const isAutoSyncSupported = isValidAutoSyncSource(connector);
const [access_type, meta, access_type_helpers] =
useField<AccessType>("access_type");
@ -34,13 +50,18 @@ export function AccessTypeGroupSelector({}: {}) {
access_type_helpers.setValue("public");
return;
}
if (!isUserAdmin) {
if (!isUserAdmin && !isAutoSyncSupported) {
access_type_helpers.setValue("private");
}
if (userGroups.length === 1 && !isUserAdmin) {
if (
access_type.value === "private" &&
userGroups.length === 1 &&
!isUserAdmin
) {
groups_helpers.setValue([userGroups[0].id]);
setShouldHideContent(true);
} else if (access_type.value !== "private") {
// If the access type is public or sync, empty the groups selection
groups_helpers.setValue([]);
setShouldHideContent(false);
} else {

View File

@ -1,6 +1,5 @@
import React, { useState } from "react";
import { Button } from "@/components/ui/button";
import { Card } from "@/components/ui/card";
import { ValidSources } from "@/lib/types";
import { FaAccusoft } from "react-icons/fa";
import { submitCredential } from "@/components/admin/connectors/CredentialForm";
@ -15,8 +14,6 @@ import {
credentialTemplates,
getDisplayNameForCredentialKey,
} from "@/lib/connectors/credentials";
import { getCurrentUser } from "@/lib/user";
import { User, UserRole } from "@/lib/types";
import { PlusCircleIcon } from "../../icons/icons";
import { GmailMain } from "@/app/admin/connectors/[connector]/pages/gmail/GmailPage";
import { ActionType, dictionaryType } from "../types";

View File

@ -1032,7 +1032,7 @@ export interface ConnectorBase<T> {
refresh_freq: number | null;
prune_freq: number | null;
indexing_start: Date | null;
is_public?: boolean;
access_type: string;
groups?: number[];
}