From 4c9edbfdf761a9639cb8e404ebff659f5a653478 Mon Sep 17 00:00:00 2001 From: buua436 Date: Sat, 10 Oct 2026 10:37:43 +0800 Subject: [PATCH] refa: centralize Go resource permissions (#20657) --- internal/admin/service_ee.go | 26 +- internal/admin/user_detail_test.go | 7 + .../component/production_chain_fixes_test.go | 51 ++ .../agent/component/universe_a_wrappers.go | 35 +- internal/agent/tool/retrieval_nlp.go | 18 + internal/agent/tool/retrieval_nlp_test.go | 42 ++ internal/dao/kb.go | 170 +----- internal/dao/kb_test.go | 107 +--- internal/dao/order_by.go | 4 - internal/handler/chat.go | 3 + internal/handler/chunk.go | 18 + internal/handler/common.go | 33 ++ internal/handler/compilation_status_test.go | 23 +- internal/handler/dataset.go | 18 +- internal/handler/dataset_artifact.go | 5 +- internal/handler/dataset_list_test.go | 9 + internal/handler/dataset_search_test.go | 21 + internal/handler/dify_retrieval_handler.go | 11 +- .../handler/dify_retrieval_handler_test.go | 34 +- internal/handler/document.go | 82 +-- internal/handler/document_test.go | 78 ++- internal/handler/file.go | 10 +- internal/handler/file_commit.go | 15 +- internal/handler/ingestion_messages_test.go | 9 + internal/handler/mcp_retrieval.go | 3 - internal/handler/permission_error_test.go | 33 ++ internal/handler/tenant.go | 9 +- internal/permission/database_source.go | 202 +++++++ internal/permission/database_source_test.go | 167 ++++++ internal/permission/policy_ce.go | 521 ++++++++++++++++++ internal/permission/policy_ce_test.go | 213 +++++++ internal/permission/response/errors.go | 78 +++ internal/permission/response/errors_test.go | 74 +++ internal/permission/types.go | 276 ++++++++++ internal/service/chat.go | 14 +- internal/service/chunk/chunk.go | 48 +- .../service/chunk/chunk_permissions_test.go | 3 +- internal/service/chunk/chunk_test.go | 59 +- internal/service/connector.go | 7 +- internal/service/connector_syncer_test.go | 37 +- .../service/dataset/compilation_status.go | 7 +- .../dataset/compilation_status_test.go | 5 +- internal/service/dataset/create_test.go | 92 +++- internal/service/dataset/crud.go | 277 ++++++---- internal/service/dataset/crud_delete_test.go | 4 +- internal/service/dataset/embedding.go | 11 +- internal/service/dataset/ingestion.go | 22 +- internal/service/dataset/list_test.go | 70 +++ internal/service/dataset/metadata.go | 37 +- .../service/dataset/metadata_config_test.go | 7 +- .../dataset/parse_type_required_test.go | 10 +- internal/service/dataset/permission.go | 38 +- internal/service/dataset/search.go | 7 +- internal/service/dataset/setup_test.go | 23 + internal/service/dataset/tags.go | 7 +- .../service/dataset/tags_aggregate_test.go | 9 +- internal/service/dataset/update.go | 72 +-- internal/service/dataset/update_test.go | 146 ++++- internal/service/dataset_permission.go | 18 + internal/service/document/document.go | 1 + .../service/document/document_artifact.go | 15 +- internal/service/document/document_crud.go | 34 +- .../document/document_dataset_update.go | 15 +- internal/service/document/document_ingest.go | 7 +- internal/service/document/document_list.go | 3 +- .../document/document_modify_guard_test.go | 8 +- internal/service/document/document_test.go | 112 ++-- internal/service/document/file2document.go | 12 +- internal/service/file_permission.go | 8 +- internal/service/search.go | 9 +- internal/service/team_permission.go | 32 -- test/testcases/restful_api/test_chunks.py | 4 +- test/testcases/restful_api/test_datasets.py | 61 +- test/testcases/restful_api/test_documents.py | 16 +- test/testcases/restful_api/test_searches.py | 4 +- 75 files changed, 2906 insertions(+), 870 deletions(-) create mode 100644 internal/handler/permission_error_test.go create mode 100644 internal/permission/database_source.go create mode 100644 internal/permission/database_source_test.go create mode 100644 internal/permission/policy_ce.go create mode 100644 internal/permission/policy_ce_test.go create mode 100644 internal/permission/response/errors.go create mode 100644 internal/permission/response/errors_test.go create mode 100644 internal/permission/types.go create mode 100644 internal/service/dataset_permission.go delete mode 100644 internal/service/team_permission.go diff --git a/internal/admin/service_ee.go b/internal/admin/service_ee.go index 45ed04111d..95830ce22d 100644 --- a/internal/admin/service_ee.go +++ b/internal/admin/service_ee.go @@ -21,11 +21,13 @@ import ( "errors" "fmt" "mime/multipart" + "sort" "strings" "ragflow/internal/common" "ragflow/internal/dao" "ragflow/internal/entity" + "ragflow/internal/permission" "gorm.io/gorm" ) @@ -585,14 +587,28 @@ func (s *Service) ListUserDatasets(ctx context.Context, email string) ([]map[str return nil, fmt.Errorf("failed to get user: %w", err) } - tenantIDs, err := s.userTenantDAO.GetTenantIDsByUserID(ctx, dao.DB, user.ID) + scope, err := permission.NewDatabaseChecker(dao.DB).Scope(ctx, permission.Subject{UserID: user.ID}, permission.ScopeQuery{ + Kind: permission.ResourceKindDataset, Operation: permission.OperationRead, + }) if err != nil { - return nil, fmt.Errorf("failed to get user tenants: %w", err) + return nil, fmt.Errorf("failed to resolve user dataset permissions: %w", err) } - datasets, err := s.kbDAO.GetAllByTenantIDs(ctx, dao.DB, tenantIDs, user.ID) - if err != nil { - return nil, fmt.Errorf("failed to get user datasets: %w", err) + var datasets []*entity.Knowledgebase + if scope.Mode == permission.ScopeIDs { + datasets, err = s.kbDAO.GetByIDs(ctx, dao.DB, scope.ResourceIDs) + if err != nil { + return nil, fmt.Errorf("failed to get user datasets: %w", err) + } + sort.SliceStable(datasets, func(i, j int) bool { + if datasets[i].CreateTime == nil { + return datasets[j].CreateTime != nil + } + if datasets[j].CreateTime == nil { + return false + } + return *datasets[i].CreateTime < *datasets[j].CreateTime + }) } result := make([]map[string]interface{}, 0, len(datasets)) diff --git a/internal/admin/user_detail_test.go b/internal/admin/user_detail_test.go index d27fa19cfa..4e58457811 100644 --- a/internal/admin/user_detail_test.go +++ b/internal/admin/user_detail_test.go @@ -78,6 +78,13 @@ func setupAdminUserDetailTest(t *testing.T) *Handler { if err := db.Create(&membership).Error; err != nil { t.Fatalf("create membership: %v", err) } + ownerMemberships := []entity.UserTenant{ + {ID: "membership-owner-user", UserID: user.ID, TenantID: user.ID, Role: "owner", InvitedBy: user.ID, Status: &statusValid}, + {ID: "membership-owner-tenant-2", UserID: "tenant-2", TenantID: "tenant-2", Role: "owner", InvitedBy: "tenant-2", Status: &statusValid}, + } + if err := db.Create(&ownerMemberships).Error; err != nil { + t.Fatalf("create owner memberships: %v", err) + } datasets := []entity.Knowledgebase{ {ID: "dataset-own", TenantID: user.ID, Name: "Own dataset", EmbdID: "embedding", Permission: "me", CreatedBy: user.ID, Status: &statusValid}, diff --git a/internal/agent/component/production_chain_fixes_test.go b/internal/agent/component/production_chain_fixes_test.go index 9be875b30c..eea66c867f 100644 --- a/internal/agent/component/production_chain_fixes_test.go +++ b/internal/agent/component/production_chain_fixes_test.go @@ -385,6 +385,57 @@ func TestRetrieval_ResolveDatasetIDByTenantName(t *testing.T) { } } +func TestRetrieval_ResolveDatasetIDByUserPermission(t *testing.T) { + for _, tc := range []struct { + name string + visibility string + wantID string + }{ + {name: "team-visible", visibility: string(entity.TenantPermissionTeam), wantID: "kb-team"}, + {name: "private-denied", visibility: string(entity.TenantPermissionMe)}, + } { + t.Run(tc.name, func(t *testing.T) { + db, err := gorm.Open(sqlite.Open("file:"+t.Name()+"?mode=memory&cache=shared"), &gorm.Config{}) + if err != nil { + t.Fatalf("failed to open sqlite: %v", err) + } + if err := db.AutoMigrate(&entity.Knowledgebase{}, &entity.UserTenant{}); err != nil { + t.Fatalf("failed to migrate permission tables: %v", err) + } + origDB := dao.DB + dao.DB = db + t.Cleanup(func() { dao.DB = origDB }) + + status := string(entity.StatusValid) + datasetID := "kb-team" + if tc.visibility == string(entity.TenantPermissionMe) { + datasetID = "kb-private" + } + dataset := entity.Knowledgebase{ + ID: datasetID, Name: "named-dataset", TenantID: "tenant-1", EmbdID: "embedding", + Permission: tc.visibility, CreatedBy: "owner", Status: &status, + } + if err := db.Create(&dataset).Error; err != nil { + t.Fatalf("failed to seed dataset: %v", err) + } + memberships := []entity.UserTenant{ + {ID: "owner-membership", UserID: "owner", TenantID: "tenant-1", Role: "owner", InvitedBy: "owner", Status: &status}, + {ID: "member-membership", UserID: "member", TenantID: "tenant-1", Role: "normal", InvitedBy: "owner", Status: &status}, + } + if err := db.Create(&memberships).Error; err != nil { + t.Fatalf("failed to seed memberships: %v", err) + } + + state := runtime.NewCanvasState("run-1", "task-1") + state.Sys["user_id"] = "member" + ctx := runtime.WithState(t.Context(), state) + if got := resolveRetrievalDatasetID(ctx, db, "named-dataset"); got != tc.wantID { + t.Fatalf("resolveRetrievalDatasetID = %q, want %q", got, tc.wantID) + } + }) + } +} + func TestRetrieval_StructuredInputPreservesQueryWhenDatasetIDsAlreadyPresent(t *testing.T) { c, err := newRetrievalComponent(nil) if err != nil { diff --git a/internal/agent/component/universe_a_wrappers.go b/internal/agent/component/universe_a_wrappers.go index 2aec9db2b6..b4ef18cddd 100644 --- a/internal/agent/component/universe_a_wrappers.go +++ b/internal/agent/component/universe_a_wrappers.go @@ -34,7 +34,7 @@ import ( agenttool "ragflow/internal/agent/tool" "ragflow/internal/common" "ragflow/internal/dao" - "ragflow/internal/entity" + "ragflow/internal/permission" "ragflow/internal/storage" "github.com/google/uuid" @@ -460,19 +460,30 @@ func resolveRetrievalDatasetID(ctx context.Context, db *gorm.DB, kbName string) } } if userID, _ := state.Sys["user_id"].(string); userID != "" { - if kbs, lookupErr := dao.NewKnowledgebaseDAO().GetKBByNameAndUserID(ctx, db, kbName, userID); lookupErr == nil && len(kbs) > 0 { - for _, kb := range kbs { - if kb == nil || kb.Status == nil || *kb.Status != string(entity.StatusValid) { - continue - } - common.Debug("agent retrieval component: resolved dataset id by user visibility") - return kb.ID - } - } else if lookupErr != nil { - common.Warn("agent retrieval component: resolve dataset id by name failed", + kbs, lookupErr := dao.NewKnowledgebaseDAO().GetByNameInUserTenants(ctx, db, kbName, userID) + if lookupErr != nil { + common.Warn("agent retrieval component: resolve dataset id by tenant membership failed", zap.Error(lookupErr)) } else { - common.Debug("agent retrieval component: user visibility lookup missed") + refs := make([]permission.ResourceRef, 0, len(kbs)) + for _, kb := range kbs { + if kb != nil { + refs = append(refs, permission.ResourceRef{Kind: permission.ResourceKindDataset, ID: kb.ID}) + } + } + accessible, accessErr := permission.NewDatabaseChecker(db).FilterResources( + ctx, + permission.Subject{UserID: userID}, + refs, + permission.OperationUse, + ) + if accessErr != nil { + common.Warn("agent retrieval component: check dataset use permission failed", + zap.Error(accessErr)) + } else if len(accessible) > 0 { + common.Debug("agent retrieval component: resolved dataset id by permission") + return accessible[0].ID + } } } } else { diff --git a/internal/agent/tool/retrieval_nlp.go b/internal/agent/tool/retrieval_nlp.go index 5af4f491ce..0aba74cde1 100644 --- a/internal/agent/tool/retrieval_nlp.go +++ b/internal/agent/tool/retrieval_nlp.go @@ -73,6 +73,8 @@ import ( "ragflow/internal/engine" "ragflow/internal/entity" modelModule "ragflow/internal/entity/models" + "ragflow/internal/permission" + permissionresponse "ragflow/internal/permission/response" "ragflow/internal/service/nlp" "go.uber.org/zap" @@ -201,6 +203,22 @@ func (a *NLPRetrievalAdapter) Search(ctx context.Context, db *gorm.DB, req Retri if len(datasets.tenantIDs) != 1 { return nil, fmt.Errorf("retrieval: datasets span multiple tenants") } + // Dataset authorization must use the caller identity established by the + // agent run, not req.UserID, which is a configurable per-user memory filter. + userID := "" + if state, stateErr := runtime.GetStateFromContext(ctx); stateErr == nil && state != nil { + userID, _ = state.Sys["user_id"].(string) + } + userID = strings.TrimSpace(userID) + checker := permission.NewDatabaseChecker(db) + for _, datasetID := range datasets.kbIDs { + if err := checker.CheckResource(ctx, permission.Subject{UserID: userID}, permission.ResourceRef{ + Kind: permission.ResourceKindDataset, ID: datasetID, + }, permission.OperationUse); err != nil { + _, permissionErr := permissionresponse.Normalize(err) + return nil, permissionErr + } + } if err := validateEmbeddingModels(ctx, db, datasets.kbs); err != nil { return nil, err } diff --git a/internal/agent/tool/retrieval_nlp_test.go b/internal/agent/tool/retrieval_nlp_test.go index f5c1105751..605f6f81a9 100644 --- a/internal/agent/tool/retrieval_nlp_test.go +++ b/internal/agent/tool/retrieval_nlp_test.go @@ -32,10 +32,13 @@ import ( "math" "testing" + "ragflow/internal/agent/runtime" "ragflow/internal/entity" modelModule "ragflow/internal/entity/models" + "ragflow/internal/permission" "ragflow/internal/service/nlp" + "github.com/glebarez/sqlite" "gorm.io/gorm" ) @@ -412,6 +415,45 @@ func TestNLPRetrievalAdapter_SearchRejectsDatasetsFromMultipleTenants(t *testing } } +func TestNLPRetrievalAdapter_SearchRequiresDatasetUsePermission(t *testing.T) { + db, err := gorm.Open(sqlite.Open("file:"+t.Name()+"?mode=memory&cache=shared"), &gorm.Config{}) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + if err := db.AutoMigrate(&entity.Knowledgebase{}, &entity.UserTenant{}); err != nil { + t.Fatalf("migrate permission tables: %v", err) + } + status := string(entity.StatusValid) + dataset := entity.Knowledgebase{ + ID: "kb-private", TenantID: "tenant-1", Name: "private", EmbdID: "embedding@provider", + Permission: string(entity.TenantPermissionMe), CreatedBy: "owner", Status: &status, + } + if err := db.Create(&dataset).Error; err != nil { + t.Fatalf("create dataset: %v", err) + } + memberships := []entity.UserTenant{ + {ID: "owner-membership", UserID: "owner", TenantID: "tenant-1", Role: string(permission.RoleOwner), InvitedBy: "owner", Status: &status}, + {ID: "member-membership", UserID: "member", TenantID: "tenant-1", Role: string(permission.RoleNormal), InvitedBy: "owner", Status: &status}, + } + if err := db.Create(&memberships).Error; err != nil { + t.Fatalf("create memberships: %v", err) + } + + adapter := &NLPRetrievalAdapter{ + svc: &nlp.RetrievalService{}, + kbDAO: fakeKnowledgebaseLookup{kbs: []*entity.Knowledgebase{&dataset}}, + } + state := runtime.NewCanvasState("run-1", "session-1") + state.Sys["user_id"] = "member" + ctx := runtime.WithState(t.Context(), state) + _, err = adapter.Search(ctx, db, RetrievalRequest{ + Query: "hello", DatasetIDs: []string{dataset.ID}, UserID: "owner", + }) + if !errors.Is(err, permission.ErrPermissionDenied) { + t.Fatalf("Search error = %v, want permission denial for runtime caller despite req.UserID=owner", err) + } +} + func TestNLPRetrievalAdapter_ResolveDatasetsFromDatasetIDs(t *testing.T) { a := &NLPRetrievalAdapter{ kbDAO: fakeKnowledgebaseLookup{ diff --git a/internal/dao/kb.go b/internal/dao/kb.go index 86d5895291..b383d3ae1a 100644 --- a/internal/dao/kb.go +++ b/internal/dao/kb.go @@ -226,9 +226,8 @@ func (dao *KnowledgebaseDAO) Count(ctx context.Context, db *gorm.DB, filters map return count, err } -// GetByTenantIDs retrieves knowledge bases by tenant IDs with pagination -// This matches the Python get_by_tenant_ids method -func (dao *KnowledgebaseDAO) GetByTenantIDs(ctx context.Context, db *gorm.DB, tenantIDs []string, userID string, pageNumber, itemsPerPage int, terms []OrderTerm, keywords, parserID, id, name string, ids []string) ([]*entity.KnowledgebaseListItem, int64, error) { +// ListByResourceIDs retrieves knowledge bases within a permission-filtered ID scope. +func (dao *KnowledgebaseDAO) ListByResourceIDs(ctx context.Context, db *gorm.DB, resourceIDs, ownerIDs []string, pageNumber, itemsPerPage int, terms []OrderTerm, keywords, parserID, name string) ([]*entity.KnowledgebaseListItem, int64, error) { var kbs []*entity.KnowledgebaseListItem var total int64 @@ -242,15 +241,10 @@ func (dao *KnowledgebaseDAO) GetByTenantIDs(ctx context.Context, db *gorm.DB, te knowledgebase.tenant_embd_id, user.nickname, user.avatar as tenant_avatar, knowledgebase.update_time`). Joins("LEFT JOIN user ON knowledgebase.tenant_id = user.id"). - Where("((knowledgebase.tenant_id IN ? AND knowledgebase.permission = ?) OR knowledgebase.tenant_id = ?) AND knowledgebase.status = ?", - tenantIDs, string(entity.TenantPermissionTeam), userID, string(entity.StatusValid)) + Where("knowledgebase.id IN ? AND knowledgebase.status = ?", resourceIDs, string(entity.StatusValid)) - if id != "" { - query = query.Where("knowledgebase.id = ?", id) - } - - if len(ids) > 0 { - query = query.Where("knowledgebase.id IN ?", ids) + if len(ownerIDs) > 0 { + query = query.Where("knowledgebase.tenant_id IN ?", ownerIDs) } if name != "" { @@ -290,34 +284,23 @@ func (dao *KnowledgebaseDAO) GetByTenantIDs(ctx context.Context, db *gorm.DB, te return kbs, total, nil } -// GetOwnerFilter returns owner counts for datasets visible to a user. -func (dao *KnowledgebaseDAO) GetOwnerFilter(ctx context.Context, db *gorm.DB, tenantIDs []string, userID string) ([]*entity.DatasetOwnerFilter, error) { +// GetOwnerFilterByResourceIDs groups permission-filtered datasets by owner tenant. +func (dao *KnowledgebaseDAO) GetOwnerFilterByResourceIDs(ctx context.Context, db *gorm.DB, resourceIDs []string) ([]*entity.DatasetOwnerFilter, error) { owners := make([]*entity.DatasetOwnerFilter, 0) + if len(resourceIDs) == 0 { + return owners, nil + } err := db.WithContext(ctx).Model(&entity.Knowledgebase{}). Select("knowledgebase.tenant_id as id, user.nickname as label, COUNT(knowledgebase.id) as count"). Joins("LEFT JOIN user ON knowledgebase.tenant_id = user.id"). - Where("((knowledgebase.tenant_id IN ? AND knowledgebase.permission = ?) OR knowledgebase.tenant_id = ?) AND knowledgebase.status = ?", - tenantIDs, string(entity.TenantPermissionTeam), userID, string(entity.StatusValid)). + Where("knowledgebase.id IN ? AND knowledgebase.status = ?", resourceIDs, string(entity.StatusValid)). Group("knowledgebase.tenant_id, user.nickname"). Scan(&owners).Error return owners, err } -// GetAllByTenantIDs retrieves all permitted knowledge bases by tenant IDs -// This matches the Python get_all_kb_by_tenant_ids method -func (dao *KnowledgebaseDAO) GetAllByTenantIDs(ctx context.Context, db *gorm.DB, tenantIDs []string, userID string) ([]*entity.Knowledgebase, error) { - var kbs []*entity.Knowledgebase - - err := db.WithContext(ctx).Where( - "((tenant_id IN ? AND permission = ?) OR tenant_id = ?) AND status = ?", - tenantIDs, string(entity.TenantPermissionTeam), userID, string(entity.StatusValid), - ).Order("create_time ASC").Find(&kbs).Error - - return kbs, err -} - // GetDetail retrieves detailed knowledge base information with joined pipeline data // This matches the Python get_detail method func (dao *KnowledgebaseDAO) GetDetail(ctx context.Context, db *gorm.DB, kbID string) (*entity.KnowledgebaseDetail, error) { @@ -345,71 +328,7 @@ func (dao *KnowledgebaseDAO) GetDetail(ctx context.Context, db *gorm.DB, kbID st return &detail, nil } -// Accessible checks if a knowledge base is accessible by a user. -// This matches the Python accessible method: -// 1. KB must exist and be VALID -// 2. If user is the owner tenant, return true -// 3. Non-owners require the explicit "team" permission -// 4. Team members must have a valid user_tenant relationship -func (dao *KnowledgebaseDAO) Accessible(ctx context.Context, db *gorm.DB, datasetID, userID string) bool { - var kb entity.Knowledgebase - err := db.WithContext(ctx).Where("id = ? AND status = ?", datasetID, string(entity.StatusValid)).First(&kb).Error - if err != nil { - return false - } - - // User is the owner tenant itself - if kb.TenantID == userID { - return true - } - - // Fail closed for unknown, missing, or private permissions. - if kb.Permission != string(entity.TenantPermissionTeam) { - return false - } - - var count int64 - err = db.WithContext(ctx).Table("user_tenant"). - Where("tenant_id = ? AND user_id = ? AND status = ?", kb.TenantID, userID, string(entity.StatusValid)). - Count(&count).Error - - if err != nil { - return false - } - return count > 0 -} - -// GetAccessibleIDs returns the subset of ids that are visible to the user: -// team-permission KBs owned by any joined tenant, plus the user's own KBs. -// This matches the Python get_accessible_ids method. -func (dao *KnowledgebaseDAO) GetAccessibleIDs(ctx context.Context, db *gorm.DB, joinedTenantIDs []string, userID string, ids []string) ([]string, error) { - accessibleIDs := make([]string, 0, len(ids)) - err := db.WithContext(ctx).Model(&entity.Knowledgebase{}). - Where("id IN ? AND ((tenant_id IN ? AND permission = ?) OR tenant_id = ?) AND status = ?", - ids, joinedTenantIDs, string(entity.TenantPermissionTeam), userID, string(entity.StatusValid)). - Pluck("id", &accessibleIDs).Error - if err != nil { - return nil, err - } - return accessibleIDs, nil -} - -// Accessible4Deletion checks if a knowledge base can be deleted by a user -// This matches the Python accessible4deletion method -func (dao *KnowledgebaseDAO) Accessible4Deletion(ctx context.Context, db *gorm.DB, kbID, userID string) bool { - var count int64 - err := db.WithContext(ctx).Model(&entity.Knowledgebase{}). - Where("id = ? AND created_by = ? AND status = ?", kbID, userID, string(entity.StatusValid)). - Count(&count).Error - - if err != nil { - return false - } - return count > 0 -} - -// DuplicateName generates a unique name by appending parentheses if name already exists -// This matches the Python duplicate_name function behavior +// DuplicateName generates a unique name by appending parentheses if name already exists. func (dao *KnowledgebaseDAO) DuplicateName(ctx context.Context, db *gorm.DB, name, tenantID string) string { const maxRetries = 1000 @@ -511,72 +430,19 @@ func (dao *KnowledgebaseDAO) GetFieldMap(ctx context.Context, db *gorm.DB, ids [ return conf, nil } -// GetKBByIDAndUserID retrieves a knowledge base by ID and user ID with tenant join -// This matches the Python get_kb_by_id method -func (dao *KnowledgebaseDAO) GetKBByIDAndUserID(ctx context.Context, db *gorm.DB, kbID, userID string) ([]*entity.Knowledgebase, error) { +// GetByNameInUserTenants returns dataset candidates whose tenant the user belongs to. +// Callers must authorize each candidate before using it. +func (dao *KnowledgebaseDAO) GetByNameInUserTenants(ctx context.Context, db *gorm.DB, kbName, userID string) ([]*entity.Knowledgebase, error) { var kbs []*entity.Knowledgebase err := db.WithContext(ctx).Model(&entity.Knowledgebase{}). Joins("JOIN user_tenant ON user_tenant.tenant_id = knowledgebase.tenant_id"). - Where("knowledgebase.id = ? AND user_tenant.user_id = ?", kbID, userID). - Limit(1). + Where("knowledgebase.name = ? AND user_tenant.user_id = ? AND knowledgebase.status = ? AND user_tenant.status = ?", + kbName, userID, string(entity.StatusValid), string(entity.StatusValid)). + Order("knowledgebase.create_time ASC"). Find(&kbs).Error return kbs, err } -// GetKBByNameAndUserID retrieves a knowledge base by name and user ID with tenant join -// This matches the Python get_kb_by_name method -func (dao *KnowledgebaseDAO) GetKBByNameAndUserID(ctx context.Context, db *gorm.DB, kbName, userID string) ([]*entity.Knowledgebase, error) { - var kbs []*entity.Knowledgebase - err := db.WithContext(ctx).Model(&entity.Knowledgebase{}). - Joins("JOIN user_tenant ON user_tenant.tenant_id = knowledgebase.tenant_id"). - Where("knowledgebase.name = ? AND user_tenant.user_id = ?", kbName, userID). - Limit(1). - Find(&kbs).Error - return kbs, err -} - -// GetList retrieves knowledge bases with filtering by ID and name -// This matches the Python get_list method -func (dao *KnowledgebaseDAO) GetList(ctx context.Context, db *gorm.DB, tenantIDs []string, userID string, pageNumber, itemsPerPage int, terms []OrderTerm, id, name string) ([]*entity.Knowledgebase, int64, error) { - var kbs []*entity.Knowledgebase - var total int64 - - query := db.WithContext(ctx).Model(&entity.Knowledgebase{}). - Where("((tenant_id IN ? AND permission = ?) OR tenant_id = ?) AND status = ?", - tenantIDs, string(entity.TenantPermissionTeam), userID, string(entity.StatusValid)) - - if id != "" { - query = query.Where("id = ?", id) - } - if name != "" { - query = query.Where("name = ?", name) - } - - // Route the requested terms through knowledgebaseOrderClause so a user-supplied query - // param can never reach Order() verbatim: the helper validates against - // knowledgebaseOrderableColumns (a closed allowlist) and falls back to - // "create_time" on a miss. - // codeql[go/sql-injection] False positive: knowledgebaseOrderClause - query = query.Order(knowledgebaseOrderClause(terms)) - - if err := query.Count(&total).Error; err != nil { - return nil, 0, err - } - - if pageNumber > 0 && itemsPerPage > 0 { - offset := (pageNumber - 1) * itemsPerPage - if err := query.Offset(offset).Limit(itemsPerPage).Find(&kbs).Error; err != nil { - return nil, 0, err - } - } else { - if err := query.Find(&kbs).Error; err != nil { - return nil, 0, err - } - } - - return kbs, total, nil -} - // DeleteByTenantID deletes all knowledge bases by tenant ID (hard delete) func (dao *KnowledgebaseDAO) DeleteByTenantID(ctx context.Context, db *gorm.DB, tenantID string) (int64, error) { result := db.WithContext(ctx).Unscoped().Where("tenant_id = ?", tenantID).Delete(&entity.Knowledgebase{}) diff --git a/internal/dao/kb_test.go b/internal/dao/kb_test.go index f997b808e7..d6f167a51d 100644 --- a/internal/dao/kb_test.go +++ b/internal/dao/kb_test.go @@ -69,56 +69,6 @@ func stringPtr(s string) *string { return &s } -func createKBAccessFixture(t *testing.T, db *gorm.DB, id, permission, status string) { - t.Helper() - kb := &entity.Knowledgebase{ID: id, TenantID: "owner", CreatedBy: "owner", Name: id, EmbdID: "embd", Permission: permission, Status: stringPtr(status)} - if err := db.Create(kb).Error; err != nil { - t.Fatalf("failed to create KB: %v", err) - } -} - -func createUserTenant(t *testing.T, db *gorm.DB, userID, tenantID, status string) { - t.Helper() - row := &entity.UserTenant{ID: userID + "-" + tenantID, UserID: userID, TenantID: tenantID, Role: "normal", InvitedBy: "owner", Status: stringPtr(status)} - if err := db.Create(row).Error; err != nil { - t.Fatalf("failed to create user_tenant: %v", err) - } -} - -func TestKnowledgebaseDAO_Accessible(t *testing.T) { - db := setupKBTestDB(t) - dao := NewKnowledgebaseDAO() - ctx := t.Context() - - createKBAccessFixture(t, db, "private", string(entity.TenantPermissionMe), string(entity.StatusValid)) - createKBAccessFixture(t, db, "team", string(entity.TenantPermissionTeam), string(entity.StatusValid)) - createKBAccessFixture(t, db, "unknown", "", string(entity.StatusValid)) - createKBAccessFixture(t, db, "invalid", string(entity.TenantPermissionTeam), string(entity.StatusInvalid)) - createUserTenant(t, db, "member", "owner", string(entity.StatusValid)) - createUserTenant(t, db, "revoked", "owner", string(entity.StatusInvalid)) - - tests := []struct { - name, kbID, userID string - want bool - }{ - {"owner private", "private", "owner", true}, - {"member private", "private", "member", false}, - {"member team", "team", "member", true}, - {"revoked member team", "team", "revoked", false}, - {"unrelated team", "team", "other", false}, - {"member unknown permission", "unknown", "member", false}, - {"invalid KB", "invalid", "owner", false}, - {"missing KB", "missing", "owner", false}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := dao.Accessible(ctx, db, tt.kbID, tt.userID); got != tt.want { - t.Fatalf("Accessible(%q, %q) = %v, want %v", tt.kbID, tt.userID, got, tt.want) - } - }) - } -} - func TestKnowledgebaseDAO_DecreaseDocumentNum(t *testing.T) { db := setupKBTestDB(t) pushDB(t, db) @@ -177,19 +127,13 @@ func TestKnowledgebaseDAO_DecreaseDocumentNum_ZeroDecrement(t *testing.T) { } // knowledgebaseQualifiedOrderExpressions are clause fragments rather than -// column names. GetByTenantIDs prefixes the value with "knowledgebase.", so +// column names. ListByResourceIDs prefixes the value with "knowledgebase.", so // these stand in for the `orderby` query parameter that reaches it from the // dataset list request (issue #14268). var knowledgebaseQualifiedOrderExpressions = []string{ "name, (SELECT inner_kb.id FROM knowledgebase AS inner_kb WHERE inner_kb.id = knowledgebase.id)", } -// knowledgebaseOrderExpressions are the unprefixed counterparts used by -// GetList. -var knowledgebaseOrderExpressions = []string{ - "(SELECT inner_kb.name FROM knowledgebase AS inner_kb WHERE inner_kb.id = knowledgebase.id)", -} - // seedOrderableKnowledgebases creates three datasets whose create_time order is // the reverse of their name order, so a test can tell which column the ORDER BY // clause actually used. @@ -219,10 +163,10 @@ func assertKnowledgebaseIDOrder(t *testing.T, got, want []string, orderby string } } -// TestKnowledgebaseDAOGetByTenantIDsOrderByExpressionFallsBack checks that an +// TestKnowledgebaseDAOListByResourceIDsOrderByExpressionFallsBack checks that an // `orderby` value that is not an allowed column name leaves the rows in the // default create_time order. -func TestKnowledgebaseDAOGetByTenantIDsOrderByExpressionFallsBack(t *testing.T) { +func TestKnowledgebaseDAOListByResourceIDsOrderByExpressionFallsBack(t *testing.T) { db := setupKBTestDB(t) if err := db.AutoMigrate(&entity.User{}); err != nil { t.Fatalf("failed to migrate user: %v", err) @@ -234,9 +178,9 @@ func TestKnowledgebaseDAOGetByTenantIDsOrderByExpressionFallsBack(t *testing.T) for _, orderby := range knowledgebaseQualifiedOrderExpressions { t.Run(orderby, func(t *testing.T) { - rows, _, err := d.GetByTenantIDs(ctx, db, []string{"t1"}, "t1", 1, 10, []OrderTerm{{Column: orderby}}, "", "", "", "", nil) + rows, _, err := d.ListByResourceIDs(ctx, db, []string{"kb-1", "kb-2", "kb-3"}, nil, 1, 10, []OrderTerm{{Column: orderby}}, "", "", "") if err != nil { - t.Fatalf("GetByTenantIDs with orderby %q: %v", orderby, err) + t.Fatalf("ListByResourceIDs with orderby %q: %v", orderby, err) } ids := make([]string, 0, len(rows)) for _, row := range rows { @@ -247,32 +191,8 @@ func TestKnowledgebaseDAOGetByTenantIDsOrderByExpressionFallsBack(t *testing.T) } } -// TestKnowledgebaseDAOGetListOrderByExpressionFallsBack is the GetList -// counterpart, which builds the clause without the table prefix. -func TestKnowledgebaseDAOGetListOrderByExpressionFallsBack(t *testing.T) { - db := setupKBTestDB(t) - pushDB(t, db) - seedOrderableKnowledgebases(t, db) - ctx := t.Context() - d := NewKnowledgebaseDAO() - - for _, orderby := range knowledgebaseOrderExpressions { - t.Run(orderby, func(t *testing.T) { - rows, _, err := d.GetList(ctx, db, []string{"t1"}, "t1", 1, 10, []OrderTerm{{Column: orderby}}, "", "") - if err != nil { - t.Fatalf("GetList with orderby %q: %v", orderby, err) - } - ids := make([]string, 0, len(rows)) - for _, row := range rows { - ids = append(ids, row.ID) - } - assertKnowledgebaseIDOrder(t, ids, []string{"kb-1", "kb-2", "kb-3"}, orderby) - }) - } -} - -// TestKnowledgebaseDAOOrderByAllowedColumn checks that the allowlist still -// honors a real column in both directions on both list paths. +// TestKnowledgebaseDAOOrderByAllowedColumn checks that the list query honors a +// real column in both directions. func TestKnowledgebaseDAOOrderByAllowedColumn(t *testing.T) { db := setupKBTestDB(t) if err := db.AutoMigrate(&entity.User{}); err != nil { @@ -283,9 +203,9 @@ func TestKnowledgebaseDAOOrderByAllowedColumn(t *testing.T) { ctx := t.Context() d := NewKnowledgebaseDAO() - rows, _, err := d.GetByTenantIDs(ctx, db, []string{"t1"}, "t1", 1, 10, []OrderTerm{{Column: "name"}}, "", "", "", "", nil) + rows, _, err := d.ListByResourceIDs(ctx, db, []string{"kb-1", "kb-2", "kb-3"}, nil, 1, 10, []OrderTerm{{Column: "name"}}, "", "", "") if err != nil { - t.Fatalf("GetByTenantIDs ascending by name: %v", err) + t.Fatalf("ListByResourceIDs ascending by name: %v", err) } ids := make([]string, 0, len(rows)) for _, row := range rows { @@ -293,13 +213,4 @@ func TestKnowledgebaseDAOOrderByAllowedColumn(t *testing.T) { } assertKnowledgebaseIDOrder(t, ids, []string{"kb-3", "kb-2", "kb-1"}, "name") - listed, _, err := d.GetList(ctx, db, []string{"t1"}, "t1", 1, 10, []OrderTerm{{Column: "name", Desc: true}}, "", "") - if err != nil { - t.Fatalf("GetList descending by name: %v", err) - } - listedIDs := make([]string, 0, len(listed)) - for _, row := range listed { - listedIDs = append(listedIDs, row.ID) - } - assertKnowledgebaseIDOrder(t, listedIDs, []string{"kb-1", "kb-2", "kb-3"}, "name") } diff --git a/internal/dao/order_by.go b/internal/dao/order_by.go index 3cbc13186a..da2ab7b709 100644 --- a/internal/dao/order_by.go +++ b/internal/dao/order_by.go @@ -238,10 +238,6 @@ func fileOrderClause(terms []OrderTerm) string { return orderClause(fileOrderableColumns, terms, defaultOrderColumn) } -func knowledgebaseOrderClause(terms []OrderTerm) string { - return orderClause(knowledgebaseOrderableColumns, terms, defaultOrderColumn) -} - func knowledgebaseQualifiedOrderClause(terms []OrderTerm) string { return qualifiedOrderClause("knowledgebase", knowledgebaseOrderableColumns, terms, defaultOrderColumn) } diff --git a/internal/handler/chat.go b/internal/handler/chat.go index 35c668d05b..b34abc7451 100644 --- a/internal/handler/chat.go +++ b/internal/handler/chat.go @@ -442,6 +442,9 @@ func (h *ChatHandler) updateChatByMethod(c *gin.Context, patch bool) { result, err = h.chatService.UpdateChat(ctx, user.ID, chatID, req) } if err != nil { + if respondPermissionErrorIf(c, err, false) { + return + } if err.Error() == "no authorization" { common.ResponseWithCodeData(c, common.CodeAuthenticationError, false, "no authorization") return diff --git a/internal/handler/chunk.go b/internal/handler/chunk.go index f2e46c0dfb..d148fbfc57 100644 --- a/internal/handler/chunk.go +++ b/internal/handler/chunk.go @@ -169,6 +169,9 @@ func (h *ChunkHandler) Get(c *gin.Context) { ctx := c.Request.Context() resp, err := h.chunkService.Get(ctx, req, user.ID) if err != nil { + if respondPermissionErrorIf(c, err, false) { + return + } common.ResponseWithHttpCodeData(c, http.StatusInternalServerError, 500, nil, err.Error()) return } @@ -257,6 +260,9 @@ func (h *ChunkHandler) ListChunks(c *gin.Context) { ctx := c.Request.Context() resp, err := h.chunkService.List(ctx, &req, user.ID) if err != nil { + if respondPermissionErrorIf(c, err, false) { + return + } common.ResponseWithHttpCodeData(c, http.StatusInternalServerError, common.CodeServerError, nil, err.Error()) return } @@ -389,6 +395,9 @@ func (h *ChunkHandler) List(c *gin.Context) { ctx := c.Request.Context() resp, err := h.chunkService.List(ctx, &req, user.ID) if err != nil { + if respondPermissionErrorIf(c, err, false) { + return + } common.ResponseWithHttpCodeData(c, http.StatusInternalServerError, 500, nil, err.Error()) return } @@ -448,6 +457,9 @@ func (h *ChunkHandler) SwitchChunks(c *gin.Context) { ctx := c.Request.Context() if err = h.chunkService.SwitchChunks(ctx, userID, datasetID, documentID, availableInt, chunkIDs); err != nil { + if respondPermissionErrorIf(c, err, false) { + return + } common.ResponseWithHttpCodeData(c, http.StatusInternalServerError, common.CodeServerError, nil, err.Error()) return } @@ -644,6 +656,9 @@ func (h *ChunkHandler) UpdateChunk(c *gin.Context) { ctx := c.Request.Context() err = h.chunkService.UpdateChunk(ctx, &req, user.ID) if err != nil { + if respondPermissionErrorIf(c, err, false) { + return + } var coded interface { Code() common.ErrorCode } @@ -699,6 +714,9 @@ func (h *ChunkHandler) RemoveChunks(c *gin.Context) { ctx := c.Request.Context() deletedCount, err := h.chunkService.RemoveChunks(ctx, &req, user.ID) if err != nil { + if respondPermissionErrorIf(c, err, false) { + return + } common.ResponseWithHttpCodeData(c, http.StatusInternalServerError, 500, nil, err.Error()) return } diff --git a/internal/handler/common.go b/internal/handler/common.go index 7ddf3c5c5c..20870b86bf 100644 --- a/internal/handler/common.go +++ b/internal/handler/common.go @@ -20,10 +20,43 @@ import ( "ragflow/internal/common" "ragflow/internal/dao" "ragflow/internal/entity" + permissionresponse "ragflow/internal/permission/response" "github.com/gin-gonic/gin" ) +func permissionErrorResult(err error, hideResource bool) (common.ErrorCode, string) { + normalize := permissionresponse.Normalize + if hideResource { + normalize = permissionresponse.NormalizeHidden + } + code, normalizedErr := normalize(err) + return code, normalizedErr.Error() +} + +func respondPermissionError(c *gin.Context, err error, hideResource bool) { + code, message := permissionErrorResult(err, hideResource) + common.ResponseWithCodeData(c, code, nil, message) +} + +func respondPermissionErrorIf(c *gin.Context, err error, hideResource bool) bool { + if !permissionresponse.IsPermissionError(err) { + return false + } + respondPermissionError(c, err, hideResource) + return true +} + +func respondPermissionFailure(c *gin.Context, err error, hideResource bool) { + code, message := permissionErrorResult(err, hideResource) + common.ErrorWithCode(c, code, message) +} + +func respondHTTPPermissionError(c *gin.Context, err error, hideResource bool) { + code, message := permissionErrorResult(err, hideResource) + common.ResponseWithHttpCodeData(c, int(code), code, nil, message) +} + // sortTermsFromQuery reads the `sort` parameter, a comma separated list of // `column:direction` terms such as `name:asc,create_time:desc`. A handler that // rejects an unrecognised `orderby` or `desc` reads this first and skips that diff --git a/internal/handler/compilation_status_test.go b/internal/handler/compilation_status_test.go index 1e13087ee2..1da06ff7c2 100644 --- a/internal/handler/compilation_status_test.go +++ b/internal/handler/compilation_status_test.go @@ -45,6 +45,7 @@ func setupCompilationStatusHandlerDB(t *testing.T) *gorm.DB { if err := db.AutoMigrate( &entity.Knowledgebase{}, &entity.KnowledgeCompileDataset{}, + &entity.UserTenant{}, ); err != nil { t.Fatalf("failed to migrate test schema: %v", err) } @@ -69,6 +70,17 @@ func insertCompilationStatusHandlerKB(t *testing.T, kbID, ownerID string) { if err := dao.DB.Create(kb).Error; err != nil { t.Fatalf("insert kb: %v", err) } + active := string(entity.StatusValid) + if err := dao.DB.Create(&entity.UserTenant{ + ID: ownerID + "-membership", + UserID: ownerID, + TenantID: ownerID, + Role: "owner", + InvitedBy: ownerID, + Status: &active, + }).Error; err != nil { + t.Fatalf("insert tenant owner: %v", err) + } } func newCompilationStatusHandlerRouter() *gin.Engine { @@ -182,8 +194,7 @@ func TestCompilationStatusHandler_FullOutput(t *testing.T) { } // TestCompilationStatusHandler_Unauthorized verifies a user who does not own the -// dataset is rejected with a data error (HTTP 200 + non-zero code, matching the -// handler's ErrorWithCode contract). +// dataset is rejected with the standardized forbidden response. func TestCompilationStatusHandler_Unauthorized(t *testing.T) { db := setupCompilationStatusHandlerDB(t) // KB is owned by user-1; the router sets user to user-1, so this test must @@ -202,10 +213,10 @@ func TestCompilationStatusHandler_Unauthorized(t *testing.T) { } _, body := getCompilationStatus(t, newCompilationStatusHandlerRouter(), "kb-status-forbidden") - if body.Code != int(common.CodeDataError) { - t.Fatalf("code=%d want %d", body.Code, common.CodeDataError) + if body.Code != int(common.CodeForbidden) { + t.Fatalf("code=%d want %d", body.Code, common.CodeForbidden) } - if body.Message != "no authorization" { - t.Fatalf("message=%q want %q", body.Message, "no authorization") + if body.Message != "Permission denied" { + t.Fatalf("message=%q want %q", body.Message, "Permission denied") } } diff --git a/internal/handler/dataset.go b/internal/handler/dataset.go index 679260743a..1ca626c2f9 100644 --- a/internal/handler/dataset.go +++ b/internal/handler/dataset.go @@ -30,6 +30,7 @@ import ( "github.com/gin-gonic/gin" "ragflow/internal/common" + "ragflow/internal/permission" "ragflow/internal/service" dataset "ragflow/internal/service/dataset" ) @@ -265,7 +266,10 @@ func (h *DatasetsHandler) CreateDataset(c *gin.Context) { ctx := c.Request.Context() - result, code, err := h.datasetsService.CreateDataset(ctx, &req, user.ID) + result, code, err := h.datasetsService.CreateDataset(ctx, &req, permission.Subject{ + UserID: user.ID, + TenantID: user.ID, + }) if err != nil { common.ErrorWithCode(c, code, err.Error()) return @@ -706,7 +710,10 @@ func (h *DatasetsHandler) DeleteDatasets(c *gin.Context) { ctx := c.Request.Context() - result, code, err := h.datasetsService.DeleteDatasets(ctx, ids, req.DeleteAll, user.ID) + result, code, err := h.datasetsService.DeleteDatasets(ctx, ids, req.DeleteAll, permission.Subject{ + UserID: user.ID, + TenantID: user.ID, + }) if err != nil { common.ErrorWithCode(c, code, err.Error()) return @@ -1010,8 +1017,8 @@ func (h *DatasetsHandler) ListMetadataFlattened(c *gin.Context) { ctx := c.Request.Context() // Check access for each dataset for _, datasetID := range datasetIDs { - if !h.datasetsService.Accessible(ctx, datasetID, user.ID) { - common.ResponseWithCodeData(c, common.CodeAuthenticationError, nil, "No authorization for dataset: "+datasetID) + if err := h.datasetsService.CheckAccess(ctx, permission.Subject{UserID: user.ID}, datasetID, permission.OperationRead); err != nil { + respondPermissionError(c, err, false) return } } @@ -1137,6 +1144,9 @@ func (h *DatasetsHandler) SearchDatasets(c *gin.Context) { resp, err := searchService.SearchDatasets(ctx, &req, user.ID) if err != nil { + if respondPermissionErrorIf(c, err, false) { + return + } common.ResponseWithCodeData(c, common.CodeDataError, nil, err.Error()) return } diff --git a/internal/handler/dataset_artifact.go b/internal/handler/dataset_artifact.go index 16ea8d1d9a..db31c12997 100644 --- a/internal/handler/dataset_artifact.go +++ b/internal/handler/dataset_artifact.go @@ -22,6 +22,7 @@ import ( "ragflow/internal/common" "ragflow/internal/dao" "ragflow/internal/entity" + "ragflow/internal/permission" "ragflow/internal/service" dataset "ragflow/internal/service/dataset" "ragflow/internal/service/file" @@ -71,8 +72,8 @@ func (h *DatasetArtifactHandler) datasetOwner(c *gin.Context, datasetID string) common.ErrorWithCode(c, common.CodeNotFound, "dataset not found") return nil, "", "" } - if !h.datasetSvc.Accessible(c.Request.Context(), datasetID, user.ID) { - common.ErrorWithCode(c, common.CodeForbidden, "no permission to access this dataset") + if err := h.datasetSvc.CheckAccess(c.Request.Context(), permission.Subject{UserID: user.ID}, datasetID, permission.OperationRead); err != nil { + respondPermissionFailure(c, err, false) return nil, "", "" } return user, kb.TenantID, msg diff --git a/internal/handler/dataset_list_test.go b/internal/handler/dataset_list_test.go index f7a6978f15..c5c413eada 100644 --- a/internal/handler/dataset_list_test.go +++ b/internal/handler/dataset_list_test.go @@ -49,6 +49,15 @@ func insertListDatasetsTestKB(t *testing.T, id, tenantID, name string) { t.Helper() status := string(entity.StatusValid) + if err := dao.DB.Where("user_id = ? AND tenant_id = ?", tenantID, tenantID).FirstOrCreate(&entity.UserTenant{ + ID: "ut-" + tenantID, + UserID: tenantID, + TenantID: tenantID, + Role: "owner", + Status: &status, + }).Error; err != nil { + t.Fatalf("insert tenant owner: %v", err) + } kb := &entity.Knowledgebase{ ID: id, TenantID: tenantID, diff --git a/internal/handler/dataset_search_test.go b/internal/handler/dataset_search_test.go index a7f83c4a6b..6e73ef92ba 100644 --- a/internal/handler/dataset_search_test.go +++ b/internal/handler/dataset_search_test.go @@ -14,6 +14,7 @@ import ( "ragflow/internal/common" "ragflow/internal/entity" + "ragflow/internal/permission" "ragflow/internal/service" ) @@ -276,6 +277,26 @@ func TestDatasetsHandlerSearchDatasetsPropagatesServiceError(t *testing.T) { } } +func TestDatasetsHandlerSearchDatasetsNormalizesPermissionError(t *testing.T) { + gin.SetMode(gin.TestMode) + fake := &fakeSearchDatasetsService{err: permission.ErrPermissionDenied} + h := &DatasetsHandler{searchDatasetsService: fake} + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/api/v1/datasets/search", strings.NewReader(`{"question":"hello","dataset_ids":["ds-1"]}`)) + req.Header.Set("Content-Type", "application/json") + c, _ := gin.CreateTestContext(rec) + c.Request = req + c.Set("user", &entity.User{ID: "user-1"}) + + h.SearchDatasets(c) + + body := decodeSearchResponse(t, rec) + if body["code"] != float64(common.CodeForbidden) || body["message"] != "Permission denied" { + t.Fatalf("response=%v want forbidden permission error", body) + } +} + func decodeSearchResponse(t *testing.T, rec *httptest.ResponseRecorder) map[string]interface{} { t.Helper() if !json.Valid(rec.Body.Bytes()) { diff --git a/internal/handler/dify_retrieval_handler.go b/internal/handler/dify_retrieval_handler.go index e40f8ad1c6..2da98d9249 100644 --- a/internal/handler/dify_retrieval_handler.go +++ b/internal/handler/dify_retrieval_handler.go @@ -29,6 +29,7 @@ import ( "ragflow/internal/engine" "ragflow/internal/entity" modelModule "ragflow/internal/entity/models" + "ragflow/internal/permission" "ragflow/internal/service" "ragflow/internal/service/graph" "ragflow/internal/service/nlp" @@ -43,8 +44,8 @@ import ( // KBServiceIface abstracts KnowledgebaseService for the Dify handler. type KBServiceIface interface { - GetByID(ctx context.Context, kbID string) (*entity.Knowledgebase, error) - Accessible(ctx context.Context, kbID, userID string) bool + GetKnowledgebaseByID(ctx context.Context, kbID string) (*entity.Knowledgebase, error) + CheckAccess(ctx context.Context, subject permission.Subject, datasetID string, operation permission.Operation) error } // ModelCreator supplies typed model instances to Dify retrieval. @@ -205,7 +206,7 @@ func (h *DifyRetrievalHandler) Retrieval(c *gin.Context) { } ctx := c.Request.Context() - kb, err := h.kbSvc.GetByID(ctx, req.KnowledgeID) + kb, err := h.kbSvc.GetKnowledgebaseByID(ctx, req.KnowledgeID) if err != nil { if errors.Is(err, gorm.ErrRecordNotFound) { common.ResponseWithHttpCodeData(c, http.StatusNotFound, common.CodeNotFound, nil, "Knowledge base not found!") @@ -215,8 +216,8 @@ func (h *DifyRetrievalHandler) Retrieval(c *gin.Context) { return } - if !h.kbSvc.Accessible(ctx, req.KnowledgeID, user.ID) { - common.ResponseWithHttpCodeData(c, http.StatusUnauthorized, common.CodeAuthenticationError, nil, "No authorization") + if err := h.kbSvc.CheckAccess(ctx, permission.Subject{UserID: user.ID}, req.KnowledgeID, permission.OperationUse); err != nil { + respondHTTPPermissionError(c, err, false) return } diff --git a/internal/handler/dify_retrieval_handler_test.go b/internal/handler/dify_retrieval_handler_test.go index c5e248a213..5aa4348615 100644 --- a/internal/handler/dify_retrieval_handler_test.go +++ b/internal/handler/dify_retrieval_handler_test.go @@ -32,6 +32,7 @@ import ( "ragflow/internal/engine/types" "ragflow/internal/entity" modelModule "ragflow/internal/entity/models" + "ragflow/internal/permission" "ragflow/internal/service" "ragflow/internal/service/nlp" @@ -42,24 +43,24 @@ import ( type mockKBService struct { KBServiceIface - getByIDFn func(ctx context.Context, kbID string) (*entity.Knowledgebase, error) - accessibleFn func(ctx context.Context, kbID, userID string) bool + getKnowledgebaseByIDFn func(ctx context.Context, kbID string) (*entity.Knowledgebase, error) + checkAccessFn func(ctx context.Context, subject permission.Subject, datasetID string, operation permission.Operation) error } -func (m *mockKBService) GetByID(ctx context.Context, kbID string) (*entity.Knowledgebase, error) { - if m.getByIDFn != nil { - return m.getByIDFn(ctx, kbID) +func (m *mockKBService) GetKnowledgebaseByID(ctx context.Context, kbID string) (*entity.Knowledgebase, error) { + if m.getKnowledgebaseByIDFn != nil { + return m.getKnowledgebaseByIDFn(ctx, kbID) } return &entity.Knowledgebase{ ID: kbID, TenantID: "tenant1", EmbdID: "text-embedding", }, nil } -func (m *mockKBService) Accessible(ctx context.Context, kbID, userID string) bool { - if m.accessibleFn != nil { - return m.accessibleFn(ctx, kbID, userID) +func (m *mockKBService) CheckAccess(ctx context.Context, subject permission.Subject, datasetID string, operation permission.Operation) error { + if m.checkAccessFn != nil { + return m.checkAccessFn(ctx, subject, datasetID, operation) } - return true + return nil } type mockModelFactory struct{} @@ -351,7 +352,7 @@ func TestDifyRetrieval_MissingArgs(t *testing.T) { func TestDifyRetrieval_KBNotFound(t *testing.T) { h, r := setupDifyTest("user1") h.kbSvc = &mockKBService{ - getByIDFn: func(ctx context.Context, kbID string) (*entity.Knowledgebase, error) { + getKnowledgebaseByIDFn: func(ctx context.Context, kbID string) (*entity.Knowledgebase, error) { return nil, gorm.ErrRecordNotFound }, } @@ -380,15 +381,20 @@ func TestDifyRetrieval_NoAuth(t *testing.T) { func TestDifyRetrieval_Unauthorized(t *testing.T) { h, r := setupDifyTest("user1") h.kbSvc = &mockKBService{ - accessibleFn: func(ctx context.Context, kbID, userID string) bool { return false }, + checkAccessFn: func(ctx context.Context, subject permission.Subject, datasetID string, operation permission.Operation) error { + if operation != permission.OperationUse { + t.Errorf("dataset operation = %q, want %q", operation, permission.OperationUse) + } + return permission.ErrPermissionDenied + }, } w := httptest.NewRecorder() body := `{"knowledge_id": "kb1", "query": "test"}` req, _ := http.NewRequest("POST", "/api/v1/dify/retrieval", strings.NewReader(body)) req.Header.Set("Content-Type", "application/json") r.ServeHTTP(w, req) - if w.Code != http.StatusUnauthorized { - t.Errorf("expected 401, got %d", w.Code) + if w.Code != http.StatusForbidden { + t.Errorf("expected 403, got %d", w.Code) } } @@ -442,7 +448,7 @@ func strPtr(s string) *string { return &s } func TestDifyRetrieval_KBDBError(t *testing.T) { h, r := setupDifyTest("user1") h.kbSvc = &mockKBService{ - getByIDFn: func(ctx context.Context, kbID string) (*entity.Knowledgebase, error) { + getKnowledgebaseByIDFn: func(ctx context.Context, kbID string) (*entity.Knowledgebase, error) { return nil, errors.New("connection refused") }, } diff --git a/internal/handler/document.go b/internal/handler/document.go index 3397f0f7fa..2fd5c7006d 100644 --- a/internal/handler/document.go +++ b/internal/handler/document.go @@ -28,6 +28,7 @@ import ( "net/url" "ragflow/internal/common" "ragflow/internal/entity" + "ragflow/internal/permission" "ragflow/internal/utility" "reflect" "strconv" @@ -143,8 +144,8 @@ func (h *DocumentHandler) GetDocumentByID(c *gin.Context) { return } - if !h.datasetService.Accessible(ctx, doc.KbID, user.ID) { - common.ResponseWithHttpCodeData(c, http.StatusForbidden, common.CodeForbidden, nil, "no permission to access this dataset") + if err := h.datasetService.CheckAccess(ctx, permission.Subject{UserID: user.ID}, doc.KbID, permission.OperationRead); err != nil { + respondHTTPPermissionError(c, err, false) return } @@ -378,8 +379,8 @@ func (h *DocumentHandler) UpdateDocument(c *gin.Context) { common.ResponseWithHttpCodeData(c, http.StatusBadRequest, 1, nil, "document not found!") return } - if !h.datasetService.Accessible(ctx, doc.KbID, user.ID) { - common.ResponseWithCodeData(c, common.CodeAuthenticationError, nil, "no authorization") + if err := h.datasetService.CheckAccess(ctx, permission.Subject{UserID: user.ID}, doc.KbID, permission.OperationUpdate); err != nil { + respondPermissionError(c, err, false) return } @@ -433,8 +434,8 @@ func (h *DocumentHandler) DeleteDocument(c *gin.Context) { common.ResponseWithHttpCodeData(c, http.StatusBadRequest, 1, nil, "document not found!") return } - if !h.datasetService.Accessible(ctx, doc.KbID, user.ID) { - common.ResponseWithCodeData(c, common.CodeAuthenticationError, nil, "no authorization") + if err := h.datasetService.CheckAccess(ctx, permission.Subject{UserID: user.ID}, doc.KbID, permission.OperationDelete); err != nil { + respondPermissionError(c, err, false) return } @@ -511,6 +512,9 @@ func (h *DocumentHandler) DeleteDocuments(c *gin.Context) { ctx := c.Request.Context() deleted, err := h.documentService.DeleteDocuments(ctx, ids, req.DeleteAll, datasetID, userID) if err != nil { + if respondPermissionErrorIf(c, err, false) { + return + } common.ResponseWithCodeData(c, common.CodeDataError, nil, err.Error()) return } @@ -606,8 +610,8 @@ func (h *DocumentHandler) ListDocuments(c *gin.Context) { } ctx := c.Request.Context() - if !h.datasetService.Accessible(ctx, datasetID, userID) { - common.ResponseWithCodeData(c, common.CodeDataError, nil, fmt.Sprintf("You don't own the dataset %s.", datasetID)) + if err := h.datasetService.CheckAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationRead); err != nil { + respondPermissionError(c, err, false) return } @@ -1000,8 +1004,8 @@ func (h *DocumentHandler) UploadDocuments(c *gin.Context) { common.ResponseWithCodeData(c, common.CodeDataError, nil, fmt.Sprintf("Can't find the dataset with ID %s!", datasetID)) return } - if !h.datasetService.CheckKBTeamPermission(ctx, kb, tenantID) { - common.ResponseWithCodeData(c, common.CodeAuthenticationError, nil, "no authorization") + if err := service.CheckDatasetAccess(ctx, permission.Subject{UserID: user.ID}, kb.ID, permission.OperationUpdate); err != nil { + respondPermissionError(c, err, false) return } @@ -1194,22 +1198,22 @@ func (h *DocumentHandler) DownloadDocument(c *gin.Context) { return } ctx := c.Request.Context() - // Authorize the caller before serving file bytes. The sibling routes on - // this dataset (PATCH/DELETE documents, chunks, metadata) all gate on - // datasetService.Accessible, and the Python reference - // (document_api.py download) checks KnowledgebaseService.accessible and - // DocumentService.accessible; without this, any logged-in user could - // download any tenant's document by supplying its dataset and document - // ids. Answer exactly like the missing-document case so existence is - // not leaked either. - if !h.datasetService.Accessible(ctx, datasetID, user.ID) { - common.ResponseWithCodeData(c, common.CodeDataError, nil, "document not found") + // Authorize the caller before serving file bytes. Answer exactly like the + // missing-document case so dataset existence is not leaked. + if err := h.datasetService.CheckAccess(ctx, permission.Subject{UserID: user.ID}, datasetID, permission.OperationRead); err != nil { + respondPermissionError(c, err, true) return } res, err := h.documentService.DownloadDocument(ctx, datasetID, docID) if err != nil { - common.ErrorWithCode(c, common.CodeDataError, err.Error()) + if errors.Is(err, document.ErrDocumentNotFound) { + common.ErrorWithCode(c, common.CodeNotFound, "Resource not found") + return + } + common.Error("DownloadDocument failed", err, + zap.String("dataset_id", datasetID), zap.String("document_id", docID), zap.String("user_id", user.ID)) + common.ErrorWithCode(c, common.CodeServerError, "Failed to download document") return } @@ -1332,8 +1336,8 @@ func (h *DocumentHandler) MetadataSummary(c *gin.Context) { } ctx := c.Request.Context() - if !h.datasetService.Accessible(ctx, kbID, user.ID) { - common.ResponseWithHttpCodeData(c, http.StatusForbidden, common.CodeForbidden, nil, "no permission to access this dataset") + if err := h.datasetService.CheckAccess(ctx, permission.Subject{UserID: user.ID}, kbID, permission.OperationRead); err != nil { + respondHTTPPermissionError(c, err, false) return } @@ -1419,8 +1423,8 @@ func (h *DocumentHandler) SetMeta(c *gin.Context) { common.ResponseWithHttpCodeData(c, http.StatusBadRequest, 1, nil, "document not found") return } - if !h.datasetService.Accessible(ctx, doc.KbID, user.ID) { - common.ResponseWithCodeData(c, common.CodeAuthenticationError, nil, "no authorization") + if err := h.datasetService.CheckAccess(ctx, permission.Subject{UserID: user.ID}, doc.KbID, permission.OperationUpdate); err != nil { + respondPermissionError(c, err, false) return } @@ -1514,8 +1518,8 @@ func (h *DocumentHandler) DeleteMeta(c *gin.Context) { common.ResponseWithHttpCodeData(c, http.StatusBadRequest, 1, nil, "document not found") return } - if !h.datasetService.Accessible(ctx, doc.KbID, user.ID) { - common.ResponseWithCodeData(c, common.CodeAuthenticationError, nil, "no authorization") + if err := h.datasetService.CheckAccess(ctx, permission.Subject{UserID: user.ID}, doc.KbID, permission.OperationUpdate); err != nil { + respondPermissionError(c, err, false) return } @@ -1577,8 +1581,8 @@ func (h *DocumentHandler) ListIngestionTasks(c *gin.Context) { var err error ctx := c.Request.Context() if req.DatasetID != nil { - if !h.datasetService.Accessible(ctx, *req.DatasetID, userID) { - common.ResponseWithCodeData(c, common.CodeAuthenticationError, nil, "No authorization to access the dataset.") + if err := h.datasetService.CheckAccess(ctx, permission.Subject{UserID: userID}, *req.DatasetID, permission.OperationRead); err != nil { + respondPermissionError(c, err, false) return } } @@ -1620,8 +1624,8 @@ func (h *DocumentHandler) StartIngestionTask(c *gin.Context) { } ctx := c.Request.Context() - if !h.datasetService.Accessible(ctx, datasetID, userID) { - common.ResponseWithCodeData(c, common.CodeDataError, nil, fmt.Sprintf("You don't own the dataset %s.", datasetID)) + if err := h.datasetService.CheckAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationRun); err != nil { + respondPermissionError(c, err, false) return } @@ -1703,8 +1707,8 @@ func (h *DocumentHandler) ParseDocuments(c *gin.Context) { userID := c.GetString("user_id") ctx := c.Request.Context() - if !h.datasetService.Accessible(ctx, datasetID, userID) { - common.ResponseWithCodeData(c, common.CodeAuthenticationError, nil, "No authorization to access the dataset.") + if err := h.datasetService.CheckAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationRun); err != nil { + respondPermissionError(c, err, false) return } @@ -1745,8 +1749,8 @@ func (h *DocumentHandler) StopParseDocuments(c *gin.Context) { } ctx := c.Request.Context() - if !h.datasetService.Accessible(ctx, datasetID, userID) { - common.ResponseWithCodeData(c, common.CodeDataError, nil, fmt.Sprintf("You don't own the dataset %s.", datasetID)) + if err := h.datasetService.CheckAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationRun); err != nil { + respondPermissionError(c, err, false) return } @@ -1771,8 +1775,8 @@ func (h *DocumentHandler) MetadataSummaryByDataset(c *gin.Context) { return } ctx := c.Request.Context() - if !h.datasetService.Accessible(ctx, datasetID, user.ID) { - common.ErrorWithCode(c, common.CodeServerError, "You don't own the dataset "+datasetID) + if err := h.datasetService.CheckAccess(ctx, permission.Subject{UserID: user.ID}, datasetID, permission.OperationRead); err != nil { + respondPermissionFailure(c, err, false) return } @@ -1952,8 +1956,8 @@ func (h *DocumentHandler) handleBatchUpdateDocumentMetadatas(c *gin.Context) { return } ctx := c.Request.Context() - if !h.datasetService.Accessible(ctx, datasetID, user.ID) { - common.ResponseWithCodeData(c, common.CodeDataError, nil, "You don't own the dataset "+datasetID+".") + if err := h.datasetService.CheckAccess(ctx, permission.Subject{UserID: user.ID}, datasetID, permission.OperationUpdate); err != nil { + respondPermissionError(c, err, false) return } diff --git a/internal/handler/document_test.go b/internal/handler/document_test.go index c873148d16..3eef8ce150 100644 --- a/internal/handler/document_test.go +++ b/internal/handler/document_test.go @@ -123,6 +123,7 @@ func TestDocumentMetadataBatchRejectsNonStringIDs(t *testing.T) { type fakeDocumentService struct { deleted int err error + downloadErr error doc *document.DocumentResponse docErr error updateCalled bool @@ -232,8 +233,11 @@ func (f *fakeDocumentService) GetDocumentPreview(ctx context.Context, userID, do }, nil } func (f *fakeDocumentService) DownloadDocument(ctx context.Context, datasetID, docID string) (*document.DownloadDocumentResp, error) { + if f.downloadErr != nil { + return nil, f.downloadErr + } if docID == "not-found" { - return nil, fmt.Errorf("not found") + return nil, document.ErrDocumentNotFound } return &document.DownloadDocumentResp{ Data: []byte("document data"), @@ -411,6 +415,15 @@ func setupDocumentPermissionDB(t *testing.T, accessible bool) { }).Error; err != nil { t.Fatalf("insert knowledgebase: %v", err) } + if err := db.Create(&entity.UserTenant{ + ID: "ut-owner", + UserID: "owner-user", + TenantID: "tenant-owner", + Role: "owner", + Status: sptr(string(entity.StatusValid)), + }).Error; err != nil { + t.Fatalf("insert tenant owner: %v", err) + } if accessible { if err := db.Create(&entity.UserTenant{ ID: "ut-user-1", @@ -464,7 +477,7 @@ func TestDocumentReadHandlersForbidden(t *testing.T) { if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatal(err) } - if resp.Code != common.CodeForbidden || resp.Message != "no permission to access this dataset" || resp.Data != nil { + if resp.Code != common.CodeForbidden || resp.Message != "Permission denied" || resp.Data != nil { t.Fatalf("unexpected response: %s", w.Body.String()) } if fake.metadataKBID != "" { @@ -495,10 +508,10 @@ func TestSetMetaHandler_NotAccessible(t *testing.T) { if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatal(err) } - if resp["code"] != float64(common.CodeAuthenticationError) { + if resp["code"] != float64(common.CodeForbidden) { t.Fatalf("expected auth error, got %v", resp) } - if resp["message"] != "no authorization" { + if resp["message"] != "Permission denied" { t.Fatalf("unexpected message: %v", resp["message"]) } if fake.setMetaCalled { @@ -563,10 +576,10 @@ func TestDeleteDocumentHandler_NotAccessible(t *testing.T) { if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatal(err) } - if resp["code"] != float64(common.CodeAuthenticationError) { + if resp["code"] != float64(common.CodeForbidden) { t.Fatalf("expected auth error, got %v", resp) } - if resp["message"] != "no authorization" { + if resp["message"] != "Permission denied" { t.Fatalf("unexpected message: %v", resp["message"]) } if fake.deleteCalled { @@ -629,10 +642,10 @@ func TestUpdateDocumentHandler_NotAccessible(t *testing.T) { if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatal(err) } - if resp["code"] != float64(common.CodeAuthenticationError) { + if resp["code"] != float64(common.CodeForbidden) { t.Fatalf("expected auth error, got %v", resp) } - if resp["message"] != "no authorization" { + if resp["message"] != "Permission denied" { t.Fatalf("unexpected message: %v", resp["message"]) } if fake.updateCalled { @@ -763,6 +776,9 @@ func setupUploadHandlerDB(t *testing.T, role string) *gorm.DB { if err := db.Create(&entity.UserTenant{ID: "ut-1", UserID: "user-1", TenantID: "tenant-1", Role: role, Status: sptr(string(entity.StatusValid))}).Error; err != nil { t.Fatalf("insert user_tenant: %v", err) } + if err := db.Create(&entity.UserTenant{ID: "ut-owner", UserID: "owner-1", TenantID: "tenant-1", Role: "owner", Status: sptr(string(entity.StatusValid))}).Error; err != nil { + t.Fatalf("insert tenant owner: %v", err) + } pipelineID := "pipe-1" if err := db.Create(&entity.Knowledgebase{ ID: "123e4567e89b12d3a456426614174000", @@ -1277,8 +1293,7 @@ func TestStopParseDocumentsHandler_BadJSON(t *testing.T) { } } -// setupHandlerAccessDB sets up SQLite in-memory DB for handler tests that need -// datasetService.Accessible to work. +// setupHandlerAccessDB sets up SQLite in-memory DB for handler dataset access tests. func setupHandlerAccessDB(t *testing.T) *gorm.DB { t.Helper() @@ -1303,7 +1318,8 @@ func setupHandlerAccessDB(t *testing.T) *gorm.DB { // Insert tenant db.Create(&entity.Tenant{ID: "tenant-1", LLMID: "llm-1", EmbdID: "embd-1", ASRID: "asr-1"}) // Insert user_tenant mapping - db.Create(&entity.UserTenant{ID: "ut-1", UserID: "user-1", TenantID: "tenant-1", Role: "admin"}) + db.Create(&entity.UserTenant{ID: "ut-owner", UserID: "owner-1", TenantID: "tenant-1", Role: "owner", Status: sptr(string(entity.StatusValid))}) + db.Create(&entity.UserTenant{ID: "ut-1", UserID: "user-1", TenantID: "tenant-1", Role: "normal", Status: sptr(string(entity.StatusValid))}) // Insert knowledgebase db.Create(&entity.Knowledgebase{ ID: "ds-1", TenantID: "tenant-1", Name: "test-kb", EmbdID: "embd-1", @@ -2239,8 +2255,36 @@ func TestDownloadDocument_NotFound(t *testing.T) { } var resp map[string]interface{} json.Unmarshal(w.Body.Bytes(), &resp) - if resp["code"] != float64(common.CodeDataError) { - t.Fatalf("expected code %d, got %v", common.CodeDataError, resp["code"]) + if resp["code"] != float64(common.CodeNotFound) { + t.Fatalf("expected code %d, got %v", common.CodeNotFound, resp["code"]) + } + if resp["message"] != "Resource not found" { + t.Fatalf("unexpected hidden-resource message: %v", resp["message"]) + } +} + +func TestDownloadDocument_ServiceErrorReturnsServerError(t *testing.T) { + h := downloadHandlerWithAccessDB(t) + h.documentService = &fakeDocumentService{downloadErr: errors.New("database connection failed")} + c, w := downloadContextAs("user-1", "ds-1", "doc-1") + + h.DownloadDocument(c) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200 response envelope, got %d", w.Code) + } + var resp map[string]interface{} + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode response: %v", err) + } + if resp["code"] != float64(common.CodeServerError) { + t.Fatalf("expected code %d, got %v", common.CodeServerError, resp["code"]) + } + if resp["message"] != "Failed to download document" { + t.Fatalf("unexpected client-visible error: %v", resp["message"]) + } + if strings.Contains(w.Body.String(), "database connection failed") { + t.Fatal("database error detail leaked to the client") } } @@ -2260,10 +2304,10 @@ func TestDownloadDocument_ForeignUserRejected(t *testing.T) { } var resp map[string]interface{} json.Unmarshal(w.Body.Bytes(), &resp) - if resp["code"] != float64(common.CodeDataError) { - t.Fatalf("expected code %d, got %v", common.CodeDataError, resp["code"]) + if resp["code"] != float64(common.CodeNotFound) { + t.Fatalf("expected code %d, got %v", common.CodeNotFound, resp["code"]) } - if resp["message"] != "document not found" { - t.Fatalf("foreign user must get the same message as a missing document, got %v", resp["message"]) + if resp["message"] != "Resource not found" { + t.Fatalf("unexpected hidden-resource message: %v", resp["message"]) } } diff --git a/internal/handler/file.go b/internal/handler/file.go index 5451a6dd81..a5e2879c86 100644 --- a/internal/handler/file.go +++ b/internal/handler/file.go @@ -21,6 +21,7 @@ import ( "fmt" "net/http" "ragflow/internal/common" + permissionresponse "ragflow/internal/permission/response" "ragflow/internal/storage" "ragflow/internal/utility" "strconv" @@ -28,6 +29,7 @@ import ( "github.com/gin-gonic/gin" + "ragflow/internal/permission" "ragflow/internal/service" "ragflow/internal/service/document" "ragflow/internal/service/file" @@ -598,9 +600,11 @@ func (h *FileHandler) LinkToDatasets(c *gin.Context) { // any other (internal) error is reported as a server error. func linkToDatasetsErrorCode(err error) common.ErrorCode { switch { - case errors.Is(err, document.ErrLinkFileNotFound), - errors.Is(err, document.ErrLinkDatasetNotFound), - errors.Is(err, document.ErrLinkNoAuthorization): + case errors.Is(err, permission.ErrUnauthenticated), errors.Is(err, permission.ErrMembershipNotFound), + errors.Is(err, permission.ErrPermissionDenied), errors.Is(err, permission.ErrResourceNotFound): + code, _ := permissionresponse.Normalize(err) + return code + case errors.Is(err, document.ErrLinkFileNotFound), errors.Is(err, document.ErrLinkDatasetNotFound): return common.CodeDataError default: return common.CodeServerError diff --git a/internal/handler/file_commit.go b/internal/handler/file_commit.go index 28b93f2929..acc5272f9d 100644 --- a/internal/handler/file_commit.go +++ b/internal/handler/file_commit.go @@ -22,6 +22,7 @@ import ( "ragflow/internal/common" "ragflow/internal/dao" "ragflow/internal/entity" + "ragflow/internal/permission" "ragflow/internal/service" "strconv" "strings" @@ -95,10 +96,16 @@ func CommitFolderResolver(h *FileCommitHandler, entityType, urlParam string) gin // Authorize, not just existence: the commit surface exposes artifact // contents and history, so it needs the same access check the dataset // routes apply. - if entityType == "datasets" && !h.kbDAO.Accessible(ctx, dao.DB, id, user.ID) { - common.ResponseWithCodeData(c, common.CodeNotFound, nil, fmt.Sprintf("%s not found", entityType)) - c.Abort() - return + if entityType == "datasets" { + operation := permission.OperationUpdate + if c.Request.Method == "GET" || c.Request.Method == "HEAD" { + operation = permission.OperationRead + } + if err := service.CheckDatasetAccess(ctx, permission.Subject{UserID: user.ID}, id, operation); err != nil { + respondHTTPPermissionError(c, err, true) + c.Abort() + return + } } folderID, err := h.ResolveFolderID(ctx, entityType, id) if err != nil { diff --git a/internal/handler/ingestion_messages_test.go b/internal/handler/ingestion_messages_test.go index 6fe0d9e810..950ac8f51d 100644 --- a/internal/handler/ingestion_messages_test.go +++ b/internal/handler/ingestion_messages_test.go @@ -123,6 +123,15 @@ func setupIngestionMessagesHandlerDB(t *testing.T) *gorm.DB { func insertIngestionMessagesHandlerKB(t *testing.T, db *gorm.DB, id, userID string) { t.Helper() status := string(entity.StatusValid) + if err := db.Create(&entity.UserTenant{ + ID: "ut-owner-" + id, + UserID: userID, + TenantID: userID, + Role: "owner", + Status: &status, + }).Error; err != nil { + t.Fatalf("insert tenant owner: %v", err) + } if err := db.Create(&entity.Knowledgebase{ ID: id, TenantID: userID, diff --git a/internal/handler/mcp_retrieval.go b/internal/handler/mcp_retrieval.go index ce9bd82440..e7c8e49274 100644 --- a/internal/handler/mcp_retrieval.go +++ b/internal/handler/mcp_retrieval.go @@ -122,9 +122,6 @@ func mcpRetrieval(ctx context.Context, ds *dataset.DatasetService, userID string } name, _ := info["name"].(string) datasetNames[id] = name - if !ds.Accessible(ctx, id, userID) { - continue - } // Read only documents returned by retrieval, through the existing listing // and metadata services. This avoids loading entire document collections. for _, chunk := range resp.Chunks { diff --git a/internal/handler/permission_error_test.go b/internal/handler/permission_error_test.go new file mode 100644 index 0000000000..7098d9a53a --- /dev/null +++ b/internal/handler/permission_error_test.go @@ -0,0 +1,33 @@ +package handler + +import ( + "errors" + "testing" + + "ragflow/internal/common" + "ragflow/internal/permission" +) + +func TestPermissionErrorResult(t *testing.T) { + tests := []struct { + name string + err error + hide bool + code common.ErrorCode + message string + }{ + {name: "denial", err: permission.ErrPermissionDenied, code: common.CodeForbidden, message: "Permission denied"}, + {name: "hidden denial", err: permission.ErrPermissionDenied, hide: true, code: common.CodeNotFound, message: "Resource not found"}, + {name: "hidden missing resource", err: permission.ErrResourceNotFound, hide: true, code: common.CodeNotFound, message: "Resource not found"}, + {name: "internal error is not hidden", err: errors.New("database error"), hide: true, code: common.CodeServerError, message: "Permission check failed"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + code, message := permissionErrorResult(test.err, test.hide) + if code != test.code || message != test.message { + t.Fatalf("permissionErrorResult() = (%d, %q), want (%d, %q)", code, message, test.code, test.message) + } + }) + } +} diff --git a/internal/handler/tenant.go b/internal/handler/tenant.go index f6a6ae4775..5767127d5f 100644 --- a/internal/handler/tenant.go +++ b/internal/handler/tenant.go @@ -28,6 +28,7 @@ import ( "github.com/gin-gonic/gin" "ragflow/internal/common" + "ragflow/internal/permission" "ragflow/internal/service" dataset "ragflow/internal/service/dataset" @@ -265,8 +266,8 @@ func (h *TenantHandler) CreateChunkStore(c *gin.Context) { ctx := c.Request.Context() // Check authorization - user must have access to this kb - if !h.datasetService.Accessible(ctx, req.KBID, user.ID) { - common.ResponseWithCodeData(c, common.CodeAuthenticationError, nil, "no authorization") + if err := h.datasetService.CheckAccess(ctx, permission.Subject{UserID: user.ID}, req.KBID, permission.OperationUpdate); err != nil { + respondPermissionError(c, err, false) return } @@ -313,8 +314,8 @@ func (h *TenantHandler) DeleteChunkStore(c *gin.Context) { } // Check authorization - if !h.datasetService.Accessible(ctx, req.KBID, user.ID) { - common.ResponseWithCodeData(c, common.CodeAuthenticationError, nil, "no authorization") + if err := h.datasetService.CheckAccess(ctx, permission.Subject{UserID: user.ID}, req.KBID, permission.OperationUpdate); err != nil { + respondPermissionError(c, err, false) return } diff --git a/internal/permission/database_source.go b/internal/permission/database_source.go new file mode 100644 index 0000000000..153818f4cb --- /dev/null +++ b/internal/permission/database_source.go @@ -0,0 +1,202 @@ +// +// Copyright 2026 The InfiniFlow Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package permission + +import ( + "context" + "errors" + "fmt" + + "ragflow/internal/entity" + + "gorm.io/gorm" + "gorm.io/gorm/clause" +) + +type databaseSource struct { + db *gorm.DB + lockRows bool +} + +// NewDatabaseChecker creates a Checker backed by the application's SQL database. +// It currently loads dataset authorization facts. +func NewDatabaseChecker(db *gorm.DB) *Checker { + return NewChecker(&databaseSource{db: db}) +} + +// NewLockingDatabaseChecker creates a database-backed Checker that locks loaded +// resource and membership rows. Use it only with a short-lived transaction. +func NewLockingDatabaseChecker(tx *gorm.DB) *Checker { + return NewChecker(&databaseSource{db: tx, lockRows: true}) +} + +func (s *databaseSource) GetMembership(ctx context.Context, userID, tenantID string) (*Membership, error) { + if s == nil || s.db == nil { + return nil, ErrSourceUnavailable + } + query := s.db.WithContext(ctx).Where("user_id = ? AND tenant_id = ? AND status = ?", userID, tenantID, string(entity.StatusValid)) + if s.lockRows { + query = query.Clauses(clause.Locking{Strength: "UPDATE"}) + } + var relation entity.UserTenant + if err := query.First(&relation).Error; err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return nil, nil + } + return nil, err + } + return &Membership{ + ID: relation.ID, + UserID: relation.UserID, + TenantID: relation.TenantID, + Role: TenantRole(relation.Role), + Active: relation.Status != nil && *relation.Status == string(entity.StatusValid), + }, nil +} + +func (s *databaseSource) GetResources(ctx context.Context, refs []ResourceRef) ([]Resource, error) { + if s == nil || s.db == nil { + return nil, ErrSourceUnavailable + } + refs = uniqueRefs(refs) + datasetIDs := make([]string, 0, len(refs)) + for _, ref := range refs { + if ref.Kind != ResourceKindDataset { + return nil, fmt.Errorf("%w: unsupported resource kind %q", ErrInvalidPermission, ref.Kind) + } + datasetIDs = append(datasetIDs, ref.ID) + } + datasetsByID, ownerIDsByTenant, err := s.loadDatasets(ctx, datasetIDs) + if err != nil { + return nil, err + } + + resources := make([]Resource, 0, len(refs)) + for _, ref := range refs { + if dataset, ok := datasetsByID[ref.ID]; ok { + resources = append(resources, datasetResource(dataset, ownerIDsByTenant)) + } + } + return resources, nil +} + +func (s *databaseSource) ListResourceRefs(ctx context.Context, subject Subject, kind ResourceKind, parent ResourceRef) ([]ResourceRef, error) { + if s == nil || s.db == nil { + return nil, ErrSourceUnavailable + } + var ids []string + switch kind { + case ResourceKindDataset: + if parent.ID != "" || parent.Kind != "" { + return nil, fmt.Errorf("%w: datasets cannot have a parent scope", ErrInvalidPermission) + } + var tenantIDs []string + if err := s.db.WithContext(ctx).Model(&entity.UserTenant{}). + Distinct("tenant_id"). + Where("user_id = ? AND status = ? AND role IN ?", subject.UserID, string(entity.StatusValid), []string{ + string(RoleOwner), string(RoleAdmin), string(RoleNormal), + }). + Pluck("tenant_id", &tenantIDs).Error; err != nil { + return nil, err + } + if len(tenantIDs) == 0 { + return []ResourceRef{}, nil + } + err := s.db.WithContext(ctx).Model(&entity.Knowledgebase{}). + Where("tenant_id IN ? AND status = ?", tenantIDs, string(entity.StatusValid)). + Pluck("id", &ids).Error + if err != nil { + return nil, err + } + default: + return nil, fmt.Errorf("%w: unsupported scope resource kind %q", ErrInvalidPermission, kind) + } + + refs := make([]ResourceRef, 0, len(ids)) + for _, id := range ids { + refs = append(refs, ResourceRef{Kind: kind, ID: id}) + } + return refs, nil +} + +func (s *databaseSource) loadDatasets(ctx context.Context, ids []string) (map[string]*entity.Knowledgebase, map[string]string, error) { + datasetsByID := make(map[string]*entity.Knowledgebase) + if len(ids) == 0 { + return datasetsByID, nil, nil + } + query := s.db.WithContext(ctx).Model(&entity.Knowledgebase{}).Where("id IN ? AND status = ?", ids, string(entity.StatusValid)) + if s.lockRows { + query = query.Clauses(clause.Locking{Strength: "UPDATE"}) + } + var datasets []*entity.Knowledgebase + if err := query.Find(&datasets).Error; err != nil { + return nil, nil, err + } + tenantIDs := make([]string, 0, len(datasets)) + seenTenantIDs := make(map[string]struct{}, len(datasets)) + for _, dataset := range datasets { + if _, seen := seenTenantIDs[dataset.TenantID]; seen { + continue + } + seenTenantIDs[dataset.TenantID] = struct{}{} + tenantIDs = append(tenantIDs, dataset.TenantID) + } + + ownerQuery := s.db.WithContext(ctx).Model(&entity.UserTenant{}). + Where("tenant_id IN ? AND role = ? AND status = ?", tenantIDs, string(RoleOwner), string(entity.StatusValid)) + if s.lockRows { + ownerQuery = ownerQuery.Clauses(clause.Locking{Strength: "UPDATE"}) + } + var owners []entity.UserTenant + if err := ownerQuery.Find(&owners).Error; err != nil { + return nil, nil, err + } + ownerIDsByTenant := make(map[string]string, len(owners)) + for _, owner := range owners { + if existingOwnerID, exists := ownerIDsByTenant[owner.TenantID]; exists && existingOwnerID != owner.UserID { + return nil, nil, fmt.Errorf("%w: tenant %q has multiple active owners", ErrInvalidPermission, owner.TenantID) + } + ownerIDsByTenant[owner.TenantID] = owner.UserID + } + for _, dataset := range datasets { + ownerUserID := ownerIDsByTenant[dataset.TenantID] + if ownerUserID == "" { + return nil, nil, fmt.Errorf("%w: tenant %q has no active owner", ErrInvalidPermission, dataset.TenantID) + } + datasetsByID[dataset.ID] = dataset + } + return datasetsByID, ownerIDsByTenant, nil +} + +func datasetResource(dataset *entity.Knowledgebase, ownerIDsByTenant map[string]string) Resource { + ownerUserID := ownerIDsByTenant[dataset.TenantID] + visibility := VisibilityPrivate + switch dataset.Permission { + case string(entity.TenantPermissionMe): + visibility = VisibilityPrivate + case string(entity.TenantPermissionTeam): + visibility = VisibilityTenant + } + return Resource{ + Ref: ResourceRef{Kind: ResourceKindDataset, ID: dataset.ID}, + TenantID: dataset.TenantID, + CreatedBy: dataset.CreatedBy, + OwnerUserID: ownerUserID, + Visibility: visibility, + OwnerOperations: []Operation{OperationRead, OperationCreate, OperationUpdate, OperationDelete, OperationShare, OperationRun, OperationUse}, + Active: dataset.Status != nil && *dataset.Status == string(entity.StatusValid), + } +} diff --git a/internal/permission/database_source_test.go b/internal/permission/database_source_test.go new file mode 100644 index 0000000000..1ef26ae8a8 --- /dev/null +++ b/internal/permission/database_source_test.go @@ -0,0 +1,167 @@ +//go:build !enterprise + +package permission + +import ( + "context" + "errors" + "net/url" + "testing" + + "ragflow/internal/entity" + + "github.com/glebarez/sqlite" + "gorm.io/gorm" +) + +func TestDatabaseCheckerAuthorizesDatasetFromPersistedRows(t *testing.T) { + db, err := gorm.Open(sqlite.Open("file:"+url.QueryEscape(t.Name())+"?mode=memory&cache=shared"), &gorm.Config{}) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + if err := db.AutoMigrate(&entity.Knowledgebase{}, &entity.UserTenant{}); err != nil { + t.Fatalf("migrate permission tables: %v", err) + } + + valid := string(entity.StatusValid) + dataset := entity.Knowledgebase{ + ID: "dataset-1", TenantID: "tenant-1", CreatedBy: "creator-1", + Permission: string(entity.TenantPermissionTeam), Status: &valid, + } + ownerMembership := entity.UserTenant{ + ID: "owner-membership", UserID: "tenant-owner", TenantID: dataset.TenantID, + Role: string(RoleOwner), Status: &valid, + } + membership := entity.UserTenant{ + ID: "membership-1", UserID: "member-1", TenantID: dataset.TenantID, + Role: string(RoleNormal), Status: &valid, + } + if err := db.Create(&dataset).Error; err != nil { + t.Fatalf("create dataset: %v", err) + } + if err := db.Create(&membership).Error; err != nil { + t.Fatalf("create membership: %v", err) + } + if err := db.Create(&ownerMembership).Error; err != nil { + t.Fatalf("create owner membership: %v", err) + } + + checker := NewDatabaseChecker(db) + subject := Subject{UserID: membership.UserID} + resources, err := (&databaseSource{db: db}).GetResources(context.Background(), []ResourceRef{{ + Kind: ResourceKindDataset, + ID: dataset.ID, + }}) + if err != nil { + t.Fatalf("load dataset authorization facts: %v", err) + } + if len(resources) != 1 || resources[0].OwnerUserID != ownerMembership.UserID || resources[0].CreatedBy != dataset.CreatedBy { + t.Fatalf("dataset resource = %+v, want tenant owner %q and creator %q", resources, ownerMembership.UserID, dataset.CreatedBy) + } + + ref := ResourceRef{Kind: ResourceKindDataset, ID: dataset.ID} + if err := checker.CheckResource(context.Background(), subject, ref, OperationRead); err != nil { + t.Errorf("CheckResource(%+v) error = %v, want nil", ref, err) + } + adminMembership := entity.UserTenant{ + ID: "admin-membership", UserID: "admin-1", TenantID: dataset.TenantID, + Role: string(RoleAdmin), Status: &valid, + } + if err := db.Create(&adminMembership).Error; err != nil { + t.Fatalf("create admin membership: %v", err) + } + if err := checker.CheckResource(context.Background(), Subject{UserID: adminMembership.UserID}, ref, OperationRead); !errors.Is(err, ErrPermissionDenied) { + t.Errorf("CheckResource(admin member) error = %v, want %v", err, ErrPermissionDenied) + } + + access, err := checker.ResolveAccess(context.Background(), subject, ref) + if err != nil { + t.Fatalf("ResolveAccess(dataset) error = %v", err) + } + if access.TenantID != dataset.TenantID || access.Source != AccessSourceTenant { + t.Fatalf("dataset access = %+v, want tenant access", access) + } +} + +func TestDatabaseCheckerRejectsPrivateDatasetForOtherTenantMember(t *testing.T) { + db, err := gorm.Open(sqlite.Open("file:"+url.QueryEscape(t.Name())+"?mode=memory&cache=shared"), &gorm.Config{}) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + if err := db.AutoMigrate(&entity.Knowledgebase{}, &entity.UserTenant{}); err != nil { + t.Fatalf("migrate permission tables: %v", err) + } + + valid := string(entity.StatusValid) + dataset := entity.Knowledgebase{ + ID: "dataset-private", TenantID: "tenant-1", CreatedBy: "creator-1", + Permission: string(entity.TenantPermissionMe), Status: &valid, + } + membership := entity.UserTenant{ + ID: "membership-1", UserID: "member-1", TenantID: "tenant-1", + Role: string(RoleNormal), Status: &valid, + } + ownerMembership := entity.UserTenant{ + ID: "owner-membership", UserID: "tenant-owner", TenantID: "tenant-1", + Role: string(RoleOwner), Status: &valid, + } + if err := db.Create(&dataset).Error; err != nil { + t.Fatalf("create dataset: %v", err) + } + if err := db.Create(&membership).Error; err != nil { + t.Fatalf("create membership: %v", err) + } + if err := db.Create(&ownerMembership).Error; err != nil { + t.Fatalf("create owner membership: %v", err) + } + + err = NewDatabaseChecker(db).CheckResource(context.Background(), Subject{UserID: membership.UserID}, ResourceRef{ + Kind: ResourceKindDataset, + ID: dataset.ID, + }, OperationRead) + if !errors.Is(err, ErrPermissionDenied) { + t.Fatalf("CheckResource(private dataset) error = %v, want %v", err, ErrPermissionDenied) + } +} + +func TestDatabaseCheckerScopesDatasetsToMembershipAndVisibility(t *testing.T) { + db, err := gorm.Open(sqlite.Open("file:"+url.QueryEscape(t.Name())+"?mode=memory&cache=shared"), &gorm.Config{}) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + if err := db.AutoMigrate(&entity.Knowledgebase{}, &entity.UserTenant{}); err != nil { + t.Fatalf("migrate permission tables: %v", err) + } + + valid := string(entity.StatusValid) + datasets := []entity.Knowledgebase{ + {ID: "team-visible", TenantID: "member-tenant", CreatedBy: "tenant-owner", Permission: string(entity.TenantPermissionTeam), Status: &valid}, + {ID: "private-hidden", TenantID: "member-tenant", CreatedBy: "tenant-owner", Permission: string(entity.TenantPermissionMe), Status: &valid}, + {ID: "outside-tenant", TenantID: "other-tenant", CreatedBy: "other-owner", Permission: string(entity.TenantPermissionTeam), Status: &valid}, + } + for i := range datasets { + if err := db.Create(&datasets[i]).Error; err != nil { + t.Fatalf("create dataset %q: %v", datasets[i].ID, err) + } + } + memberships := []entity.UserTenant{ + {ID: "member", UserID: "member-user", TenantID: "member-tenant", Role: string(RoleNormal), Status: &valid}, + {ID: "owner", UserID: "tenant-owner", TenantID: "member-tenant", Role: string(RoleOwner), Status: &valid}, + {ID: "other-owner", UserID: "other-owner", TenantID: "other-tenant", Role: string(RoleOwner), Status: &valid}, + } + for i := range memberships { + if err := db.Create(&memberships[i]).Error; err != nil { + t.Fatalf("create membership %q: %v", memberships[i].ID, err) + } + } + + scope, err := NewDatabaseChecker(db).Scope(context.Background(), Subject{UserID: "member-user"}, ScopeQuery{ + Kind: ResourceKindDataset, Operation: OperationRead, + }) + if err != nil { + t.Fatalf("Scope(datasets) error = %v", err) + } + if scope.Mode != ScopeIDs || len(scope.ResourceIDs) != 1 || scope.ResourceIDs[0] != "team-visible" { + t.Fatalf("Scope(datasets) = %+v, want only the team-visible dataset", scope) + } +} diff --git a/internal/permission/policy_ce.go b/internal/permission/policy_ce.go new file mode 100644 index 0000000000..201565880e --- /dev/null +++ b/internal/permission/policy_ce.go @@ -0,0 +1,521 @@ +// +// Copyright 2026 The InfiniFlow Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build !enterprise + +package permission + +import ( + "context" + "errors" + "fmt" + "strings" +) + +type communityPolicy struct { + source Source +} + +type membershipResult struct { + membership *Membership + err error +} + +type evaluation struct { + source Source + memberships map[string]membershipResult + resources map[ResourceRef]Resource + resourceErr map[ResourceRef]error +} + +func newPolicy(source Source) policy { + return &communityPolicy{source: source} +} + +func (p *communityPolicy) CheckFeature(ctx context.Context, subject Subject, feature Feature, action Action) error { + if err := validateSubject(subject); err != nil { + return err + } + if strings.TrimSpace(string(feature)) == "" || !validAction(action) { + return ErrInvalidPermission + } + // Community edition has no role-based feature restrictions. + return nil +} + +func (p *communityPolicy) CheckTenant(ctx context.Context, subject Subject, tenantID string, requirement TenantRequirement) error { + return newEvaluation(p.source).checkTenant(ctx, subject, tenantID, requirement) +} + +func (p *communityPolicy) CheckResource(ctx context.Context, subject Subject, ref ResourceRef, operation Operation) error { + if !validOperation(operation) { + return ErrInvalidPermission + } + access, err := newEvaluation(p.source).resolveAccess(ctx, subject, ref) + if err != nil { + return err + } + if !accessAllows(access, operation) { + return ErrPermissionDenied + } + return nil +} + +func (p *communityPolicy) CheckDependency(ctx context.Context, subject Subject, entryRef, dependencyRef ResourceRef, entryOperation, dependencyOperation Operation) error { + if !validOperation(entryOperation) || !validOperation(dependencyOperation) { + return ErrInvalidPermission + } + evaluation := newEvaluation(p.source) + return evaluation.checkDependency(ctx, subject, entryRef, dependencyRef, entryOperation, dependencyOperation) +} + +func (p *communityPolicy) ResolveAccess(ctx context.Context, subject Subject, ref ResourceRef) (Access, error) { + return newEvaluation(p.source).resolveAccess(ctx, subject, ref) +} + +func (p *communityPolicy) Scope(ctx context.Context, subject Subject, query ScopeQuery) (Scope, error) { + if err := validateSubject(subject); err != nil { + return Scope{}, err + } + if strings.TrimSpace(string(query.Kind)) == "" || !validOperation(query.Operation) { + return Scope{}, ErrInvalidPermission + } + if (query.Parent.ID == "") != (query.Parent.Kind == "") { + return Scope{}, ErrInvalidPermission + } + if query.Entry != nil && (!validResourceRef(*query.Entry) || !validOperation(query.EntryOp)) { + return Scope{}, ErrInvalidPermission + } + + evaluation := newEvaluation(p.source) + candidateRefs, err := p.source.ListResourceRefs(ctx, subject, query.Kind, query.Parent) + if err != nil { + return Scope{}, fmt.Errorf("permission: list resource candidates: %w", err) + } + candidateRefs = uniqueRefs(candidateRefs) + resourcesToLoad := append([]ResourceRef(nil), candidateRefs...) + if query.Parent.ID != "" { + resourcesToLoad = append(resourcesToLoad, query.Parent) + } + if query.Entry != nil { + resourcesToLoad = append(resourcesToLoad, *query.Entry) + } + if err := evaluation.loadResources(ctx, resourcesToLoad); err != nil { + return Scope{}, err + } + + var entryRef ResourceRef + if query.Entry != nil { + entryRef = *query.Entry + } + accessible := make([]ResourceRef, 0, len(candidateRefs)) + for _, ref := range candidateRefs { + var checkErr error + if query.Entry == nil { + checkErr = evaluation.checkResource(ctx, subject, ref, query.Operation) + } else { + checkErr = evaluation.checkDependency(ctx, subject, entryRef, ref, query.EntryOp, query.Operation) + } + if checkErr == nil { + accessible = append(accessible, ref) + continue + } + if errorsIsPermissionDenial(checkErr) || errors.Is(checkErr, ErrResourceNotFound) { + continue + } + return Scope{}, checkErr + } + + // In the community edition the effective tenant comes from the persisted + // resource boundary, not from a caller-selected tenant. Scopes without a + // single parent or entry can contain resources from multiple tenants, so + // their explicit authorized IDs are the boundary and TenantID stays empty. + var tenantID string + if query.Parent.ID != "" { + if parent, parentErr := evaluation.getResource(query.Parent); parentErr == nil { + tenantID = parent.TenantID + } + } + if query.Entry != nil { + if entry, entryErr := evaluation.getResource(*query.Entry); entryErr == nil { + tenantID = entry.TenantID + } + } + scope := Scope{TenantID: tenantID, Kind: query.Kind, Parent: query.Parent, Operation: query.Operation} + if len(accessible) == 0 { + scope.Mode = ScopeNone + return scope, nil + } + scope.Mode = ScopeIDs + scope.ResourceIDs = make([]string, 0, len(accessible)) + for _, ref := range accessible { + scope.ResourceIDs = append(scope.ResourceIDs, ref.ID) + } + return scope, nil +} + +func (p *communityPolicy) FilterResources(ctx context.Context, subject Subject, refs []ResourceRef, operation Operation) ([]ResourceRef, error) { + if err := validateSubject(subject); err != nil { + return nil, err + } + if !validOperation(operation) { + return nil, ErrInvalidPermission + } + evaluation := newEvaluation(p.source) + refs = uniqueRefs(refs) + if err := evaluation.loadResources(ctx, refs); err != nil { + return nil, err + } + accessible := make([]ResourceRef, 0, len(refs)) + for _, ref := range refs { + err := evaluation.checkResource(ctx, subject, ref, operation) + if err == nil { + accessible = append(accessible, ref) + continue + } + if errorsIsPermissionDenial(err) || errors.Is(err, ErrResourceNotFound) { + continue + } + return nil, err + } + return accessible, nil +} + +func newEvaluation(source Source) *evaluation { + return &evaluation{ + source: source, + memberships: make(map[string]membershipResult), + resources: make(map[ResourceRef]Resource), + resourceErr: make(map[ResourceRef]error), + } +} + +func (e *evaluation) checkTenant(ctx context.Context, subject Subject, tenantID string, requirement TenantRequirement) error { + if err := validateSubject(subject); err != nil { + return err + } + if strings.TrimSpace(tenantID) == "" || !validTenantRequirement(requirement) { + return ErrInvalidPermission + } + membership, err := e.getMembership(ctx, subject.UserID, tenantID) + if err != nil { + return err + } + if !membershipSatisfies(membership, requirement) { + return ErrMembershipNotFound + } + return nil +} + +func (e *evaluation) checkResource(ctx context.Context, subject Subject, ref ResourceRef, operation Operation) error { + if !validOperation(operation) { + return ErrInvalidPermission + } + access, err := e.resolveAccess(ctx, subject, ref) + if err != nil { + return err + } + if !accessAllows(access, operation) { + return ErrPermissionDenied + } + return nil +} + +func (e *evaluation) checkDependency(ctx context.Context, subject Subject, entryRef, dependencyRef ResourceRef, entryOperation, dependencyOperation Operation) error { + if err := validateSubject(subject); err != nil { + return err + } + if !validOperation(entryOperation) || !validOperation(dependencyOperation) { + return ErrInvalidPermission + } + if err := e.loadResources(ctx, []ResourceRef{entryRef, dependencyRef}); err != nil { + return err + } + entry, err := e.getResource(entryRef) + if err != nil { + return err + } + entryAccess, err := e.resolveResource(ctx, subject, entry) + if err != nil { + return err + } + if !accessAllows(entryAccess, entryOperation) { + return ErrPermissionDenied + } + dependency, err := e.getResource(dependencyRef) + if err != nil { + return err + } + userAccess, err := e.resolveResource(ctx, subject, dependency) + if err != nil { + return err + } + if !accessAllows(userAccess, dependencyOperation) { + return ErrPermissionDenied + } + ownerSubject := Subject{UserID: entry.OwnerUserID, TenantID: entry.TenantID} + ownerAccess, err := e.resolveResource(ctx, ownerSubject, dependency) + if err != nil { + if errorsIsPermissionDenial(err) { + return ErrPermissionDenied + } + return err + } + if !accessAllows(ownerAccess, dependencyOperation) { + return ErrPermissionDenied + } + return nil +} + +func (e *evaluation) resolveAccess(ctx context.Context, subject Subject, ref ResourceRef) (Access, error) { + if err := validateSubject(subject); err != nil { + return Access{}, err + } + if err := e.loadResources(ctx, []ResourceRef{ref}); err != nil { + return Access{}, err + } + resource, err := e.getResource(ref) + if err != nil { + return Access{}, err + } + return e.resolveResource(ctx, subject, resource) +} + +func (e *evaluation) resolveResource(ctx context.Context, subject Subject, resource Resource) (Access, error) { + if resource.TenantID == "" || resource.OwnerUserID == "" || !validResourceRef(resource.Ref) { + return Access{}, fmt.Errorf("%w: incomplete resource authorization facts", ErrInvalidPermission) + } + if !resource.Active { + return Access{}, ErrResourceNotFound + } + if !validOperations(resource.OwnerOperations) { + return Access{}, fmt.Errorf("%w: invalid owner operations", ErrInvalidPermission) + } + switch resource.Visibility { + case VisibilityPrivate, VisibilityTenant, VisibilityShared: + default: + return Access{}, fmt.Errorf("%w: unknown resource visibility %q", ErrInvalidPermission, resource.Visibility) + } + if subject.UserID == resource.OwnerUserID { + return resourceAccess(resource, AccessSourceOwner), nil + } + + switch resource.Visibility { + case VisibilityPrivate: + return Access{}, ErrPermissionDenied + case VisibilityTenant: + if err := e.checkTenant(ctx, subject, resource.TenantID, TenantNormalMember); err != nil { + if errors.Is(err, ErrMembershipNotFound) { + return Access{}, ErrPermissionDenied + } + return Access{}, err + } + return resourceAccess(resource, AccessSourceTenant), nil + case VisibilityShared: + if contains(resource.SharedWithUserIDs, subject.UserID) { + return resourceAccess(resource, AccessSourceShared), nil + } + for _, tenantID := range resource.SharedWithTenantIDs { + if err := e.checkTenant(ctx, subject, tenantID, TenantNormalMember); err != nil { + if errors.Is(err, ErrMembershipNotFound) { + continue + } + return Access{}, err + } + return resourceAccess(resource, AccessSourceShared), nil + } + return Access{}, ErrPermissionDenied + default: + return Access{}, fmt.Errorf("%w: unknown resource visibility %q", ErrInvalidPermission, resource.Visibility) + } +} + +func (e *evaluation) loadResources(ctx context.Context, refs []ResourceRef) error { + if e.source == nil { + return ErrSourceUnavailable + } + var missing []ResourceRef + for _, ref := range uniqueRefs(refs) { + if _, ok := e.resources[ref]; ok { + continue + } + if _, ok := e.resourceErr[ref]; ok { + continue + } + if !validResourceRef(ref) { + return ErrInvalidPermission + } + missing = append(missing, ref) + } + if len(missing) == 0 { + return nil + } + resources, err := e.source.GetResources(ctx, missing) + if err != nil { + return fmt.Errorf("permission: load resources: %w", err) + } + for _, ref := range missing { + e.resourceErr[ref] = ErrResourceNotFound + } + for _, resource := range resources { + if _, requested := e.resourceErr[resource.Ref]; requested { + e.resources[resource.Ref] = resource + delete(e.resourceErr, resource.Ref) + } + } + return nil +} + +func (e *evaluation) getResource(ref ResourceRef) (Resource, error) { + if !validResourceRef(ref) { + return Resource{}, ErrInvalidPermission + } + if resource, ok := e.resources[ref]; ok { + return resource, nil + } + if err, ok := e.resourceErr[ref]; ok { + return Resource{}, err + } + return Resource{}, ErrResourceNotFound +} + +func (e *evaluation) getMembership(ctx context.Context, userID, tenantID string) (*Membership, error) { + if e.source == nil { + return nil, ErrSourceUnavailable + } + key := userID + "\x00" + tenantID + if result, ok := e.memberships[key]; ok { + return result.membership, result.err + } + membership, err := e.source.GetMembership(ctx, userID, tenantID) + if err != nil { + err = fmt.Errorf("permission: load tenant membership: %w", err) + } else if membership == nil || !membership.Active || membership.UserID != userID || membership.TenantID != tenantID || membership.Role == RoleInvite { + membership = nil + err = ErrMembershipNotFound + } + e.memberships[key] = membershipResult{membership: membership, err: err} + return membership, err +} + +func errorsIsPermissionDenial(err error) bool { + return errors.Is(err, ErrPermissionDenied) || errors.Is(err, ErrMembershipNotFound) +} + +func validateSubject(subject Subject) error { + if strings.TrimSpace(subject.UserID) == "" { + return ErrUnauthenticated + } + return nil +} + +func validResourceRef(ref ResourceRef) bool { + return strings.TrimSpace(string(ref.Kind)) != "" && strings.TrimSpace(ref.ID) != "" +} + +func validAction(action Action) bool { + switch action { + case ActionEnable, ActionRead, ActionWrite, ActionShare: + return true + default: + return false + } +} + +func validOperation(operation Operation) bool { + switch operation { + case OperationRead, OperationCreate, OperationUpdate, OperationDelete, OperationShare, OperationRun, OperationUse: + return true + default: + return false + } +} + +func validTenantRequirement(requirement TenantRequirement) bool { + switch requirement { + case TenantMember, TenantNormalMember, TenantAdmin, TenantOwner: + return true + default: + return false + } +} + +func membershipSatisfies(membership *Membership, requirement TenantRequirement) bool { + if membership == nil || !membership.Active || membership.Role == RoleInvite { + return false + } + switch requirement { + case TenantMember: + return membership.Role == RoleOwner || membership.Role == RoleAdmin || membership.Role == RoleNormal + case TenantNormalMember: + return membership.Role == RoleNormal + case TenantAdmin: + return membership.Role == RoleOwner || membership.Role == RoleAdmin + case TenantOwner: + return membership.Role == RoleOwner + default: + return false + } +} + +func validOperations(operations []Operation) bool { + for _, operation := range operations { + if !validOperation(operation) { + return false + } + } + return true +} + +func resourceAccess(resource Resource, source AccessSource) Access { + return Access{ + Resource: resource.Ref, + TenantID: resource.TenantID, + Source: source, + Operations: append([]Operation(nil), resource.OwnerOperations...), + } +} + +func accessAllows(access Access, operation Operation) bool { + if !access.Allows(operation) { + return false + } + return operation != OperationShare || access.Source == AccessSourceOwner +} + +func contains(values []string, target string) bool { + for _, value := range values { + if value == target { + return true + } + } + return false +} + +func uniqueRefs(refs []ResourceRef) []ResourceRef { + seen := make(map[ResourceRef]struct{}, len(refs)) + unique := make([]ResourceRef, 0, len(refs)) + for _, ref := range refs { + if !validResourceRef(ref) { + continue + } + if _, ok := seen[ref]; ok { + continue + } + seen[ref] = struct{}{} + unique = append(unique, ref) + } + return unique +} diff --git a/internal/permission/policy_ce_test.go b/internal/permission/policy_ce_test.go new file mode 100644 index 0000000000..4e9344d2a8 --- /dev/null +++ b/internal/permission/policy_ce_test.go @@ -0,0 +1,213 @@ +//go:build !enterprise + +package permission + +import ( + "context" + "errors" + "testing" +) + +type permissionTestSource struct { + memberships map[string]*Membership + resources map[ResourceRef]Resource + candidates []ResourceRef +} + +func (s *permissionTestSource) GetMembership(_ context.Context, userID, tenantID string) (*Membership, error) { + return s.memberships[userID+"\x00"+tenantID], nil +} + +func (s *permissionTestSource) GetResources(_ context.Context, refs []ResourceRef) ([]Resource, error) { + resources := make([]Resource, 0, len(refs)) + for _, ref := range refs { + if resource, ok := s.resources[ref]; ok { + resources = append(resources, resource) + } + } + return resources, nil +} + +func (s *permissionTestSource) ListResourceRefs(_ context.Context, _ Subject, kind ResourceKind, parent ResourceRef) ([]ResourceRef, error) { + refs := make([]ResourceRef, 0, len(s.candidates)) + for _, ref := range s.candidates { + if ref.Kind == kind { + refs = append(refs, ref) + } + } + return refs, nil +} + +func TestCommunityPolicyResolvesResourceTenant(t *testing.T) { + resourceRef := ResourceRef{Kind: "dataset", ID: "dataset-1"} + source := &permissionTestSource{ + memberships: map[string]*Membership{ + "user-1\x00tenant-resource": { + UserID: "user-1", TenantID: "tenant-resource", Role: RoleNormal, Active: true, + }, + }, + resources: map[ResourceRef]Resource{ + resourceRef: { + Ref: resourceRef, TenantID: "tenant-resource", OwnerUserID: "tenant-owner", + Visibility: VisibilityTenant, OwnerOperations: []Operation{OperationRead}, Active: true, + }, + }, + } + + access, err := NewChecker(source).ResolveAccess(context.Background(), Subject{UserID: "user-1"}, resourceRef) + if err != nil { + t.Fatalf("ResolveAccess() error = %v", err) + } + if access.TenantID != "tenant-resource" { + t.Fatalf("ResolveAccess() tenant = %q, want resource tenant", access.TenantID) + } + if access.Source != AccessSourceTenant || !access.Allows(OperationRead) { + t.Fatalf("ResolveAccess() = %#v, want tenant read access", access) + } +} + +func TestCommunityPolicyUsesResourceTenantInsteadOfSubjectTenant(t *testing.T) { + resourceRef := ResourceRef{Kind: "dataset", ID: "dataset-1"} + source := &permissionTestSource{ + memberships: map[string]*Membership{ + "user-1\x00tenant-resource": { + UserID: "user-1", TenantID: "tenant-resource", Role: RoleNormal, Active: true, + }, + }, + resources: map[ResourceRef]Resource{ + resourceRef: { + Ref: resourceRef, TenantID: "tenant-resource", OwnerUserID: "tenant-owner", + Visibility: VisibilityTenant, OwnerOperations: []Operation{OperationRead}, Active: true, + }, + }, + } + + access, err := NewChecker(source).ResolveAccess(context.Background(), Subject{UserID: "user-1", TenantID: "another-tenant"}, resourceRef) + if err != nil { + t.Fatalf("ResolveAccess() error = %v", err) + } + if access.TenantID != "tenant-resource" { + t.Fatalf("ResolveAccess() tenant = %q, want resource tenant", access.TenantID) + } + + _, err = NewChecker(source).ResolveAccess(context.Background(), Subject{UserID: "unrelated-user", TenantID: "tenant-resource"}, resourceRef) + if !errors.Is(err, ErrPermissionDenied) { + t.Fatalf("ResolveAccess() error = %v, want %v for a non-member", err, ErrPermissionDenied) + } +} + +func TestCommunityPolicySharedResourceNeedsNoSubjectTenant(t *testing.T) { + resourceRef := ResourceRef{Kind: "agent", ID: "agent-1"} + source := &permissionTestSource{ + memberships: map[string]*Membership{ + "user-1\x00shared-tenant": { + UserID: "user-1", TenantID: "shared-tenant", Role: RoleNormal, Active: true, + }, + }, + resources: map[ResourceRef]Resource{ + resourceRef: { + Ref: resourceRef, TenantID: "owner-tenant", OwnerUserID: "owner-user", + Visibility: VisibilityShared, SharedWithTenantIDs: []string{"shared-tenant"}, + OwnerOperations: []Operation{OperationRun}, Active: true, + }, + }, + } + + access, err := NewChecker(source).ResolveAccess(context.Background(), Subject{UserID: "user-1"}, resourceRef) + if err != nil { + t.Fatalf("ResolveAccess() error = %v", err) + } + if access.Source != AccessSourceShared || !access.Allows(OperationRun) { + t.Fatalf("ResolveAccess() = %#v, want shared run access", access) + } + if err := NewChecker(source).CheckResource(context.Background(), Subject{UserID: "user-1"}, resourceRef, OperationShare); !errors.Is(err, ErrPermissionDenied) { + t.Fatalf("CheckResource(share) error = %v, want %v for a shared resource", err, ErrPermissionDenied) + } +} + +func TestCommunityPolicyChecksDependencyForBothUserAndEntryOwner(t *testing.T) { + entryRef := ResourceRef{Kind: "agent", ID: "agent-1"} + dependencyRef := ResourceRef{Kind: "dataset", ID: "dataset-1"} + source := &permissionTestSource{ + resources: map[ResourceRef]Resource{ + entryRef: { + Ref: entryRef, TenantID: "entry-tenant", OwnerUserID: "entry-owner", + Visibility: VisibilityShared, SharedWithUserIDs: []string{"caller"}, + OwnerOperations: []Operation{OperationRun}, Active: true, + }, + dependencyRef: { + Ref: dependencyRef, TenantID: "dataset-tenant", OwnerUserID: "dataset-owner", + Visibility: VisibilityShared, SharedWithUserIDs: []string{"caller"}, + OwnerOperations: []Operation{OperationRead}, Active: true, + }, + }, + } + + err := NewChecker(source).CheckDependency( + context.Background(), Subject{UserID: "caller"}, entryRef, dependencyRef, OperationRun, OperationRead, + ) + if !errors.Is(err, ErrPermissionDenied) { + t.Fatalf("CheckDependency() error = %v, want %v", err, ErrPermissionDenied) + } + + dependency := source.resources[dependencyRef] + dependency.SharedWithUserIDs = []string{"caller", "entry-owner"} + source.resources[dependencyRef] = dependency + err = NewChecker(source).CheckDependency( + context.Background(), Subject{UserID: "caller"}, entryRef, dependencyRef, OperationRun, OperationRead, + ) + if err != nil { + t.Fatalf("CheckDependency() error = %v, want nil when both caller and entry owner can read dependency", err) + } +} + +func TestCommunityPolicyFeatureDoesNotRequireTenantContext(t *testing.T) { + checker := NewChecker(&permissionTestSource{}) + if err := checker.CheckFeature(context.Background(), Subject{UserID: "user-1"}, "dataset", ActionRead); err != nil { + t.Fatalf("CheckFeature() error = %v", err) + } +} + +func TestCommunityPolicyScopeUsesParentResourceTenant(t *testing.T) { + parentRef := ResourceRef{Kind: "dataset", ID: "dataset-1"} + childRef := ResourceRef{Kind: "document", ID: "document-1"} + source := &permissionTestSource{ + memberships: map[string]*Membership{ + "user-1\x00dataset-tenant": { + UserID: "user-1", TenantID: "dataset-tenant", Role: RoleNormal, Active: true, + }, + }, + resources: map[ResourceRef]Resource{ + parentRef: { + Ref: parentRef, TenantID: "dataset-tenant", OwnerUserID: "dataset-owner", + Visibility: VisibilityTenant, OwnerOperations: []Operation{OperationRead}, Active: true, + }, + childRef: { + Ref: childRef, TenantID: "dataset-tenant", OwnerUserID: "dataset-owner", + Visibility: VisibilityTenant, OwnerOperations: []Operation{OperationRead}, Active: true, + }, + }, + candidates: []ResourceRef{childRef}, + } + + scope, err := NewChecker(source).Scope(context.Background(), Subject{UserID: "user-1"}, ScopeQuery{ + Kind: "document", Parent: parentRef, Operation: OperationRead, + }) + if err != nil { + t.Fatalf("Scope() error = %v", err) + } + if scope.TenantID != "dataset-tenant" { + t.Fatalf("Scope() tenant = %q, want parent tenant", scope.TenantID) + } + if scope.Mode != ScopeIDs || len(scope.ResourceIDs) != 1 || scope.ResourceIDs[0] != childRef.ID { + t.Fatalf("Scope() = %#v, want the accessible child ID", scope) + } +} + +func TestCommunityPolicyRequiresUserIdentity(t *testing.T) { + resourceRef := ResourceRef{Kind: "dataset", ID: "dataset-1"} + _, err := NewChecker(&permissionTestSource{}).ResolveAccess(context.Background(), Subject{}, resourceRef) + if !errors.Is(err, ErrUnauthenticated) { + t.Fatalf("ResolveAccess() error = %v, want %v", err, ErrUnauthenticated) + } +} diff --git a/internal/permission/response/errors.go b/internal/permission/response/errors.go new file mode 100644 index 0000000000..66a5001e88 --- /dev/null +++ b/internal/permission/response/errors.go @@ -0,0 +1,78 @@ +// +// Copyright 2026 The InfiniFlow Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package permissionresponse + +import ( + "errors" + + "ragflow/internal/common" + "ragflow/internal/permission" +) + +// Normalize maps permission decisions to stable API codes and safe messages +// while retaining the original error for errors.Is/errors.As. +func Normalize(err error) (common.ErrorCode, error) { + return normalize(err, false) +} + +// NormalizeHidden hides whether a resource exists when the caller is not +// allowed to access it. +func NormalizeHidden(err error) (common.ErrorCode, error) { + return normalize(err, true) +} + +// IsPermissionError reports whether err carries a typed permission decision. +func IsPermissionError(err error) bool { + return errors.Is(err, permission.ErrUnauthenticated) || + errors.Is(err, permission.ErrResourceNotFound) || + errors.Is(err, permission.ErrMembershipNotFound) || + errors.Is(err, permission.ErrPermissionDenied) +} + +func normalize(err error, hideResource bool) (common.ErrorCode, error) { + if err == nil { + return common.CodeSuccess, nil + } + + code := common.CodeServerError + message := "Permission check failed" + switch { + case errors.Is(err, permission.ErrUnauthenticated): + code = common.CodeUnauthorized + message = "Authentication required" + case errors.Is(err, permission.ErrResourceNotFound): + code = common.CodeNotFound + message = "Resource not found" + case errors.Is(err, permission.ErrPermissionDenied), errors.Is(err, permission.ErrMembershipNotFound): + code = common.CodeForbidden + message = "Permission denied" + } + if hideResource && (code == common.CodeForbidden || code == common.CodeNotFound) { + code = common.CodeNotFound + message = "Resource not found" + } + + return code, &normalizedError{cause: err, message: message} +} + +type normalizedError struct { + cause error + message string +} + +func (e *normalizedError) Error() string { return e.message } + +func (e *normalizedError) Unwrap() error { return e.cause } diff --git a/internal/permission/response/errors_test.go b/internal/permission/response/errors_test.go new file mode 100644 index 0000000000..164c8f5fb6 --- /dev/null +++ b/internal/permission/response/errors_test.go @@ -0,0 +1,74 @@ +package permissionresponse + +import ( + "errors" + "fmt" + "testing" + + "ragflow/internal/common" + "ragflow/internal/permission" +) + +func TestNormalize(t *testing.T) { + tests := []struct { + name string + err error + code common.ErrorCode + message string + }{ + {name: "unauthenticated", err: permission.ErrUnauthenticated, code: common.CodeUnauthorized, message: "Authentication required"}, + {name: "missing resource", err: permission.ErrResourceNotFound, code: common.CodeNotFound, message: "Resource not found"}, + {name: "denied", err: permission.ErrPermissionDenied, code: common.CodeForbidden, message: "Permission denied"}, + {name: "missing membership", err: permission.ErrMembershipNotFound, code: common.CodeForbidden, message: "Permission denied"}, + {name: "internal permission failure", err: errors.New("database credentials must not leak"), code: common.CodeServerError, message: "Permission check failed"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + code, normalizedErr := Normalize(test.err) + if code != test.code { + t.Fatalf("code = %d, want %d", code, test.code) + } + if normalizedErr == nil || normalizedErr.Error() != test.message { + t.Fatalf("error = %v, want message %q", normalizedErr, test.message) + } + if !errors.Is(normalizedErr, test.err) { + t.Fatalf("normalized error %v does not wrap %v", normalizedErr, test.err) + } + }) + } +} + +func TestNormalizeHidden(t *testing.T) { + tests := []struct { + name string + err error + code common.ErrorCode + message string + }{ + {name: "denied resource", err: permission.ErrPermissionDenied, code: common.CodeNotFound, message: "Resource not found"}, + {name: "missing resource", err: permission.ErrResourceNotFound, code: common.CodeNotFound, message: "Resource not found"}, + {name: "internal failure remains visible", err: errors.New("db error"), code: common.CodeServerError, message: "Permission check failed"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + code, normalizedErr := NormalizeHidden(test.err) + if code != test.code || normalizedErr.Error() != test.message { + t.Fatalf("NormalizeHidden() = (%d, %q), want (%d, %q)", code, normalizedErr, test.code, test.message) + } + if !errors.Is(normalizedErr, test.err) { + t.Fatalf("normalized error %v does not wrap %v", normalizedErr, test.err) + } + }) + } +} + +func TestIsPermissionError(t *testing.T) { + if !IsPermissionError(fmt.Errorf("wrapped: %w", permission.ErrPermissionDenied)) { + t.Fatal("IsPermissionError() = false for wrapped permission denial") + } + if IsPermissionError(errors.New("ordinary failure")) { + t.Fatal("IsPermissionError() = true for ordinary failure") + } +} diff --git a/internal/permission/types.go b/internal/permission/types.go new file mode 100644 index 0000000000..9cd673c421 --- /dev/null +++ b/internal/permission/types.go @@ -0,0 +1,276 @@ +// +// Copyright 2026 The InfiniFlow Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package permission provides shared permission contracts and edition policies. +package permission + +import ( + "context" + "errors" +) + +var ( + ErrUnauthenticated = errors.New("permission: user is required") + ErrInvalidSubject = errors.New("permission: tenant is required") + ErrResourceNotFound = errors.New("permission: resource not found") + ErrMembershipNotFound = errors.New("permission: active tenant membership not found") + ErrPermissionDenied = errors.New("permission: access denied") + ErrInvalidPermission = errors.New("permission: invalid permission request") + ErrSourceUnavailable = errors.New("permission: source is not configured") +) + +// Subject identifies the authenticated user and, when the policy requires it, +// the active tenant context. Community-edition resource checks derive the +// effective tenant from the persisted resource, so TenantID may be empty. +type Subject struct { + UserID string + TenantID string +} + +// ResourceKind identifies a resource family without coupling this package to business entities. +type ResourceKind string + +const ( + ResourceKindDataset ResourceKind = "dataset" +) + +// ResourceRef identifies a persisted resource. +type ResourceRef struct { + Kind ResourceKind + ID string +} + +// Feature identifies a feature whose availability is edition-dependent. +type Feature string + +// Action identifies a feature-level action. +type Action string + +const ( + ActionEnable Action = "enable" + ActionRead Action = "read" + ActionWrite Action = "write" + ActionShare Action = "share" +) + +// Operation identifies an operation on a concrete resource. +type Operation string + +const ( + OperationRead Operation = "read" + OperationCreate Operation = "create" + OperationUpdate Operation = "update" + OperationDelete Operation = "delete" + OperationShare Operation = "share" + OperationRun Operation = "run" + OperationUse Operation = "use" +) + +// TenantRequirement describes the tenant role required by a check. +type TenantRequirement string + +const ( + TenantMember TenantRequirement = "member" + TenantNormalMember TenantRequirement = "normal_member" + TenantAdmin TenantRequirement = "admin" + TenantOwner TenantRequirement = "owner" +) + +// TenantRole is the normalized role in a user-tenant membership. +type TenantRole string + +const ( + RoleOwner TenantRole = "owner" + RoleAdmin TenantRole = "admin" + RoleNormal TenantRole = "normal" + RoleInvite TenantRole = "invite" +) + +// Membership is the normalized active membership fact returned by Source. +type Membership struct { + ID string + UserID string + TenantID string + Role TenantRole + Active bool +} + +// Visibility describes how a resource is exposed in the community edition. +type Visibility string + +const ( + VisibilityPrivate Visibility = "private" + VisibilityTenant Visibility = "tenant" + VisibilityShared Visibility = "shared" +) + +// Resource contains normalized authorization and attribution facts loaded from +// the persisted business resource. TenantID, CreatedBy, and OwnerUserID are +// independent facts: the resource tenant, creator, and permission owner need +// not be the same identity. +type Resource struct { + Ref ResourceRef + TenantID string + CreatedBy string + OwnerUserID string + Visibility Visibility + OwnerOperations []Operation + SharedWithUserIDs []string + SharedWithTenantIDs []string + Active bool +} + +// AccessSource identifies the rule that granted access. +type AccessSource string + +const ( + AccessSourceOwner AccessSource = "owner" + AccessSourceTenant AccessSource = "tenant" + AccessSourceShared AccessSource = "shared" +) + +// Access describes the operations a subject can perform on a resource. +type Access struct { + Resource ResourceRef + TenantID string + Source AccessSource + Operations []Operation +} + +func (a Access) Allows(operation Operation) bool { + for _, allowed := range a.Operations { + if allowed == operation { + return true + } + } + return false +} + +// ScopeMode distinguishes an empty scope from an explicit resource-ID scope. +type ScopeMode string + +const ( + ScopeNone ScopeMode = "none" + ScopeIDs ScopeMode = "ids" +) + +// Scope is a permission-filtered candidate set for list and retrieval queries. +type Scope struct { + Mode ScopeMode + TenantID string + Kind ResourceKind + Parent ResourceRef + Operation Operation + ResourceIDs []string +} + +// ScopeQuery describes the candidate resource set and optional execution entry. +type ScopeQuery struct { + Kind ResourceKind + Parent ResourceRef + Operation Operation + Entry *ResourceRef + EntryOp Operation +} + +// Source loads normalized permission facts. GetResources must return the +// requested resources with their tenant, creator, permission owner, active +// state, visibility, owner operations, and share recipients. ListResourceRefs +// returns all candidate IDs within the query boundary, including candidates +// that policy will reject; Checker authorizes them before returning a scope. +type Source interface { + GetMembership(ctx context.Context, userID, tenantID string) (*Membership, error) + GetResources(ctx context.Context, refs []ResourceRef) ([]Resource, error) + ListResourceRefs(ctx context.Context, subject Subject, kind ResourceKind, parent ResourceRef) ([]ResourceRef, error) +} + +type policy interface { + CheckFeature(context.Context, Subject, Feature, Action) error + CheckTenant(context.Context, Subject, string, TenantRequirement) error + CheckResource(context.Context, Subject, ResourceRef, Operation) error + CheckDependency(context.Context, Subject, ResourceRef, ResourceRef, Operation, Operation) error + ResolveAccess(context.Context, Subject, ResourceRef) (Access, error) + Scope(context.Context, Subject, ScopeQuery) (Scope, error) + FilterResources(context.Context, Subject, []ResourceRef, Operation) ([]ResourceRef, error) +} + +// Checker exposes edition-independent permission methods. +type Checker struct { + policy policy +} + +// NewChecker creates a checker backed by normalized membership and resource facts. +func NewChecker(source Source) *Checker { + if source == nil { + return &Checker{} + } + return &Checker{policy: newPolicy(source)} +} + +// CheckFeature checks feature access under the active edition's policy. +func (c *Checker) CheckFeature(ctx context.Context, subject Subject, feature Feature, action Action) error { + if c == nil || c.policy == nil { + return ErrSourceUnavailable + } + return c.policy.CheckFeature(ctx, subject, feature, action) +} + +// CheckTenant checks the user's membership and role in tenantID. +func (c *Checker) CheckTenant(ctx context.Context, subject Subject, tenantID string, requirement TenantRequirement) error { + if c == nil || c.policy == nil { + return ErrSourceUnavailable + } + return c.policy.CheckTenant(ctx, subject, tenantID, requirement) +} + +// CheckResource checks a direct operation on a resource. +func (c *Checker) CheckResource(ctx context.Context, subject Subject, resource ResourceRef, operation Operation) error { + if c == nil || c.policy == nil { + return ErrSourceUnavailable + } + return c.policy.CheckResource(ctx, subject, resource, operation) +} + +// CheckDependency checks entry access and the user's and entry owner's access to a dependency. +func (c *Checker) CheckDependency(ctx context.Context, subject Subject, entry, dependency ResourceRef, entryOperation, dependencyOperation Operation) error { + if c == nil || c.policy == nil { + return ErrSourceUnavailable + } + return c.policy.CheckDependency(ctx, subject, entry, dependency, entryOperation, dependencyOperation) +} + +// ResolveAccess resolves the effective operations and grant source for a resource. +func (c *Checker) ResolveAccess(ctx context.Context, subject Subject, resource ResourceRef) (Access, error) { + if c == nil || c.policy == nil { + return Access{}, ErrSourceUnavailable + } + return c.policy.ResolveAccess(ctx, subject, resource) +} + +// Scope returns the explicitly authorized IDs for a resource query. +func (c *Checker) Scope(ctx context.Context, subject Subject, query ScopeQuery) (Scope, error) { + if c == nil || c.policy == nil { + return Scope{}, ErrSourceUnavailable + } + return c.policy.Scope(ctx, subject, query) +} + +// FilterResources returns only candidate resources that allow operation. +func (c *Checker) FilterResources(ctx context.Context, subject Subject, resources []ResourceRef, operation Operation) ([]ResourceRef, error) { + if c == nil || c.policy == nil { + return nil, ErrSourceUnavailable + } + return c.policy.FilterResources(ctx, subject, resources, operation) +} diff --git a/internal/service/chat.go b/internal/service/chat.go index 37f4e86861..0044374353 100644 --- a/internal/service/chat.go +++ b/internal/service/chat.go @@ -23,6 +23,8 @@ import ( "fmt" "ragflow/internal/common" "ragflow/internal/entity" + "ragflow/internal/permission" + permissionresponse "ragflow/internal/permission/response" "ragflow/internal/utility" "strings" "unicode/utf8" @@ -381,7 +383,7 @@ func validateCreateChatName(value interface{}) (string, error) { return name, nil } -func (s *ChatService) validateCreateDatasetIDs(ctx context.Context, value interface{}, tenantID string) ([]string, error) { +func (s *ChatService) validateCreateDatasetIDs(ctx context.Context, value interface{}, userID string) ([]string, error) { if value == nil { return []string{}, nil } @@ -401,8 +403,9 @@ func (s *ChatService) validateCreateDatasetIDs(ctx context.Context, value interf } for _, datasetID := range normalizedIDs { - if !s.kbDAO.Accessible(ctx, dao.DB, datasetID, tenantID) { - return nil, fmt.Errorf("you don't own the dataset %s", datasetID) + if err := CheckDatasetAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationUse); err != nil { + _, permissionErr := permissionresponse.Normalize(err) + return nil, permissionErr } kb, err := s.kbDAO.GetByID(ctx, dao.DB, datasetID) if err != nil { @@ -1055,8 +1058,9 @@ func (s *ChatService) validateRESTDatasetIDs(ctx context.Context, value interfac continue } datasetID := fmt.Sprint(item) - if !s.kbDAO.Accessible(ctx, dao.DB, datasetID, userID) { - return nil, fmt.Errorf("you don't own the dataset %s", datasetID) + if err := CheckDatasetAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationUse); err != nil { + _, permissionErr := permissionresponse.Normalize(err) + return nil, permissionErr } kb, err := s.kbDAO.GetByID(ctx, dao.DB, datasetID) if err != nil || kb == nil { diff --git a/internal/service/chunk/chunk.go b/internal/service/chunk/chunk.go index c50afa2df8..0e45473114 100644 --- a/internal/service/chunk/chunk.go +++ b/internal/service/chunk/chunk.go @@ -29,6 +29,8 @@ import ( "ragflow/internal/common" "ragflow/internal/entity" "ragflow/internal/entity/models" + "ragflow/internal/permission" + permissionresponse "ragflow/internal/permission/response" "ragflow/internal/service" "strings" "sync" @@ -83,7 +85,7 @@ type ChunkService struct { ingestionTaskDAO *dao.IngestionTaskDAO searchService *service.SearchService - accessibleFunc func(string, string) bool + checkDatasetAccessFunc func(string, string) error getKnowledgebaseByIDFunc func(string) (*entity.Knowledgebase, error) getDocumentsByIDsFunc func([]string) ([]*entity.Document, error) // startParseDocumentsFunc overrides the DSL start-parse flow. Production @@ -517,7 +519,7 @@ func (s *ChunkService) Get(ctx context.Context, req *service.GetChunkRequest, us return nil, fmt.Errorf("chunk_id is required") } - kb, err := s.getAccessibleKnowledgebase(ctx, req.DatasetID, userID) + kb, err := s.getAccessibleKnowledgebase(ctx, req.DatasetID, userID, permission.OperationRead) if err != nil { return nil, err } @@ -590,8 +592,9 @@ func (s *ChunkService) cancelAllTasksOfDoc(ctx context.Context, doc *entity.Docu } func (s *ChunkService) StopParsing(ctx context.Context, userID, datasetID string, req service.StopParsingRequest) (*service.StopParsingResponse, common.ErrorCode, error) { - if !s.kbDAO.Accessible(ctx, dao.DB, datasetID, userID) { - return nil, common.CodeDataError, fmt.Errorf("you don't own the dataset %s", datasetID) + if err := service.CheckDatasetAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationRun); err != nil { + code, permissionErr := permissionresponse.Normalize(err) + return nil, code, permissionErr } if len(req.DocumentIDs) == 0 { @@ -666,11 +669,11 @@ func checkDuplicateIDs(documentIDs []string, idTypes string) ([]string, []string return uniqueDocIDs, duplicateMessages } -func (s *ChunkService) accessible(ctx context.Context, datasetID, userID string) bool { - if s.accessibleFunc != nil { - return s.accessibleFunc(datasetID, userID) +func (s *ChunkService) checkDatasetAccess(ctx context.Context, datasetID, userID string, operation permission.Operation) error { + if s.checkDatasetAccessFunc != nil { + return s.checkDatasetAccessFunc(datasetID, userID) } - return s.kbDAO.Accessible(ctx, dao.DB, datasetID, userID) + return service.CheckDatasetAccess(ctx, permission.Subject{UserID: userID}, datasetID, operation) } func (s *ChunkService) getKnowledgebaseByID(ctx context.Context, datasetID string) (*entity.Knowledgebase, error) { @@ -680,9 +683,9 @@ func (s *ChunkService) getKnowledgebaseByID(ctx context.Context, datasetID strin return s.kbDAO.GetByID(ctx, dao.DB, datasetID) } -func (s *ChunkService) getAccessibleKnowledgebase(ctx context.Context, datasetID, userID string) (*entity.Knowledgebase, error) { - if !s.accessible(ctx, datasetID, userID) { - return nil, fmt.Errorf("user does not have access to this dataset") +func (s *ChunkService) getAccessibleKnowledgebase(ctx context.Context, datasetID, userID string, operation permission.Operation) (*entity.Knowledgebase, error) { + if err := s.checkDatasetAccess(ctx, datasetID, userID, operation); err != nil { + return nil, err } kb, err := s.getKnowledgebaseByID(ctx, datasetID) if err != nil || kb == nil { @@ -699,8 +702,9 @@ func (s *ChunkService) getDocumentsByIDs(ctx context.Context, docIDs []string) ( } func (s *ChunkService) Parse(ctx context.Context, userID, datasetID string, req *service.ParseFileRequest) (map[string]interface{}, common.ErrorCode, error) { - if !s.accessible(ctx, datasetID, userID) { - return nil, common.CodeOperatingError, fmt.Errorf("you don't own the dataset %s", datasetID) + if err := s.checkDatasetAccess(ctx, datasetID, userID, permission.OperationRun); err != nil { + code, permissionErr := permissionresponse.Normalize(err) + return nil, code, permissionErr } if req == nil || len(req.DocumentIDs) == 0 { return nil, common.CodeDataError, fmt.Errorf("`document_ids` is required") @@ -786,7 +790,7 @@ func (s *ChunkService) List(ctx context.Context, req *service.ListChunksRequest, return nil, fmt.Errorf("document not found") } - kb, err := s.getAccessibleKnowledgebase(ctx, doc.KbID, userID) + kb, err := s.getAccessibleKnowledgebase(ctx, doc.KbID, userID, permission.OperationRead) if err != nil { return nil, err } @@ -1002,7 +1006,7 @@ func (s *ChunkService) SwitchChunks(ctx context.Context, userID, datasetID, docu return fmt.Errorf("req is null") } - kb, err := s.getAccessibleKnowledgebase(ctx, datasetID, userID) + kb, err := s.getAccessibleKnowledgebase(ctx, datasetID, userID, permission.OperationUpdate) if err != nil { return err } @@ -1044,7 +1048,7 @@ func (s *ChunkService) UpdateChunk(ctx context.Context, req *service.UpdateChunk return fmt.Errorf("chunk_id is required") } - kb, err := s.getAccessibleKnowledgebase(ctx, req.DatasetID, userID) + kb, err := s.getAccessibleKnowledgebase(ctx, req.DatasetID, userID, permission.OperationUpdate) if err != nil { return err } @@ -1226,7 +1230,7 @@ func (s *ChunkService) RemoveChunks(ctx context.Context, req *service.RemoveChun return 0, fmt.Errorf("document not found") } - kb, err := s.getAccessibleKnowledgebase(ctx, doc.KbID, userID) + kb, err := s.getAccessibleKnowledgebase(ctx, doc.KbID, userID, permission.OperationUpdate) if err != nil { return 0, err } @@ -1276,8 +1280,9 @@ func (s *ChunkService) AddChunk(ctx context.Context, req *service.AddChunkReques if req == nil { return nil, addChunkError{code: common.CodeDataError, message: "invalid request payload"} } - if !s.accessible(ctx, req.DatasetID, userID) { - return nil, addChunkError{code: common.CodeDataError, message: fmt.Sprintf("You don't own the dataset %s.", req.DatasetID)} + if err := s.checkDatasetAccess(ctx, req.DatasetID, userID, permission.OperationUpdate); err != nil { + code, permissionErr := permissionresponse.Normalize(err) + return nil, addChunkError{code: code, message: permissionErr.Error(), cause: permissionErr} } kb, err := s.getKnowledgebaseByID(ctx, req.DatasetID) @@ -1448,6 +1453,7 @@ func (s *ChunkService) markWikiDirty(ctx context.Context, tenantID, datasetID, d type addChunkError struct { code common.ErrorCode message string + cause error } type updateChunkError struct { @@ -1467,6 +1473,10 @@ func (e addChunkError) Error() string { return e.message } +func (e addChunkError) Unwrap() error { + return e.cause +} + func (e addChunkError) Code() common.ErrorCode { return e.code } diff --git a/internal/service/chunk/chunk_permissions_test.go b/internal/service/chunk/chunk_permissions_test.go index 76e089eef9..a62f6e0ca6 100644 --- a/internal/service/chunk/chunk_permissions_test.go +++ b/internal/service/chunk/chunk_permissions_test.go @@ -5,6 +5,7 @@ import ( "ragflow/internal/dao" "ragflow/internal/entity" + permissionresponse "ragflow/internal/permission/response" "ragflow/internal/service" ) @@ -79,7 +80,7 @@ func TestChunkManagementDatasetPermissions(t *testing.T) { if err != nil { t.Fatalf("authorized operation failed: %v", err) } - } else if err == nil || err.Error() != "user does not have access to this dataset" { + } else if !permissionresponse.IsPermissionError(err) { t.Fatalf("unauthorized operation error = %v, want access denied", err) } }) diff --git a/internal/service/chunk/chunk_test.go b/internal/service/chunk/chunk_test.go index 8db3287eca..7be2bd87f5 100644 --- a/internal/service/chunk/chunk_test.go +++ b/internal/service/chunk/chunk_test.go @@ -14,6 +14,7 @@ import ( "ragflow/internal/engine/types" "ragflow/internal/entity" "ragflow/internal/entity/models" + "ragflow/internal/permission" "ragflow/internal/service" "ragflow/internal/service/document" "ragflow/internal/storage" @@ -208,23 +209,23 @@ func TestParsePrevalidatesDocumentsBeforeMutating(t *testing.T) { func TestParseRejectsInaccessibleDataset(t *testing.T) { svc := newParseTestService(t) - svc.accessibleFunc = func(string, string) bool { return false } + svc.checkDatasetAccessFunc = func(string, string) error { return permission.ErrPermissionDenied } ctx := t.Context() _, code, err := svc.Parse(ctx, "user-1", "kb-1", &service.ParseFileRequest{DocumentIDs: []string{"doc-1"}}) if err == nil { t.Fatal("expected parse to fail") } - if code != common.CodeOperatingError { - t.Fatalf("expected CodeOperatingError, got %v", code) + if code != common.CodeForbidden { + t.Fatalf("expected CodeForbidden, got %v", code) } - if !strings.Contains(err.Error(), "don't own the dataset") { + if err.Error() != "Permission denied" { t.Fatalf("unexpected error: %v", err) } } func TestParseRequiresDocumentIDs(t *testing.T) { svc := newParseTestService(t) - svc.accessibleFunc = func(string, string) bool { return true } + svc.checkDatasetAccessFunc = func(string, string) error { return nil } ctx := t.Context() for _, req := range []*service.ParseFileRequest{nil, {DocumentIDs: nil}, {DocumentIDs: []string{}}} { _, code, err := svc.Parse(ctx, "user-1", "kb-1", req) @@ -242,7 +243,7 @@ func TestParseRequiresDocumentIDs(t *testing.T) { func TestParseReturnsDataErrorWhenDatasetMissing(t *testing.T) { svc := newParseTestService(t) - svc.accessibleFunc = func(string, string) bool { return true } + svc.checkDatasetAccessFunc = func(string, string) error { return nil } svc.getKnowledgebaseByIDFunc = func(string) (*entity.Knowledgebase, error) { return nil, nil } ctx := t.Context() _, code, err := svc.Parse(ctx, "user-1", "kb-1", &service.ParseFileRequest{DocumentIDs: []string{"doc-1"}}) @@ -260,7 +261,7 @@ func TestParseReturnsDataErrorWhenDatasetMissing(t *testing.T) { func TestParseReturnsServerErrorWhenDocumentsQueryFails(t *testing.T) { svc := newParseTestService(t) queryErr := errors.New("documents query failed") - svc.accessibleFunc = func(string, string) bool { return true } + svc.checkDatasetAccessFunc = func(string, string) error { return nil } svc.getKnowledgebaseByIDFunc = func(string) (*entity.Knowledgebase, error) { return &entity.Knowledgebase{ID: "kb-1", TenantID: "user-1"}, nil } @@ -1070,8 +1071,11 @@ func TestAddChunkSuccess(t *testing.T) { docEngine: engine, kbDAO: dao.NewKnowledgebaseDAO(), documentDAO: dao.NewDocumentDAO(), - accessibleFunc: func(datasetIDArg, userIDArg string) bool { - return datasetIDArg == datasetID && userIDArg == userID + checkDatasetAccessFunc: func(datasetIDArg, userIDArg string) error { + if datasetIDArg == datasetID && userIDArg == userID { + return nil + } + return permission.ErrPermissionDenied }, getKnowledgebaseByIDFunc: func(id string) (*entity.Knowledgebase, error) { return &entity.Knowledgebase{ID: id, TenantID: userID, EmbdID: "embed-1"}, nil @@ -1166,8 +1170,11 @@ func TestAddChunkPersistsTagKwdOnlyWhenProvided(t *testing.T) { docEngine: engine, kbDAO: dao.NewKnowledgebaseDAO(), documentDAO: dao.NewDocumentDAO(), - accessibleFunc: func(datasetIDArg, userIDArg string) bool { - return datasetIDArg == datasetID && userIDArg == userID + checkDatasetAccessFunc: func(datasetIDArg, userIDArg string) error { + if datasetIDArg != datasetID || userIDArg != userID { + return permission.ErrPermissionDenied + } + return nil }, getKnowledgebaseByIDFunc: func(id string) (*entity.Knowledgebase, error) { return &entity.Knowledgebase{ID: id, TenantID: userID, EmbdID: "embed-1"}, nil @@ -1291,7 +1298,7 @@ func TestAddChunkValidationErrors(t *testing.T) { name: "empty content", req: &service.AddChunkRequest{DatasetID: "kb-1", DocumentID: "doc-1", Content: " "}, setup: func(svc *ChunkService) { - svc.accessibleFunc = func(string, string) bool { return true } + svc.checkDatasetAccessFunc = func(string, string) error { return nil } svc.getKnowledgebaseByIDFunc = func(string) (*entity.Knowledgebase, error) { return &entity.Knowledgebase{ID: "kb-1", TenantID: "user-1", EmbdID: "embed-1"}, nil } @@ -1323,10 +1330,10 @@ func TestAddChunkImageAndTagFeatureValidation(t *testing.T) { storeCalls := 0 svc := &ChunkService{ - docEngine: &addChunkTestEngine{}, - kbDAO: dao.NewKnowledgebaseDAO(), - documentDAO: dao.NewDocumentDAO(), - accessibleFunc: func(string, string) bool { return true }, + docEngine: &addChunkTestEngine{}, + kbDAO: dao.NewKnowledgebaseDAO(), + documentDAO: dao.NewDocumentDAO(), + checkDatasetAccessFunc: func(string, string) error { return nil }, getKnowledgebaseByIDFunc: func(id string) (*entity.Knowledgebase, error) { return &entity.Knowledgebase{ID: id, TenantID: userID, EmbdID: "embed-1"}, nil }, @@ -1407,10 +1414,10 @@ func TestAddChunkIncrementsStatsAfterInsert(t *testing.T) { var incrementCalls int engine := &addChunkTestEngine{} svc := &ChunkService{ - docEngine: engine, - kbDAO: dao.NewKnowledgebaseDAO(), - documentDAO: dao.NewDocumentDAO(), - accessibleFunc: func(string, string) bool { return true }, + docEngine: engine, + kbDAO: dao.NewKnowledgebaseDAO(), + documentDAO: dao.NewDocumentDAO(), + checkDatasetAccessFunc: func(string, string) error { return nil }, getKnowledgebaseByIDFunc: func(id string) (*entity.Knowledgebase, error) { return &entity.Knowledgebase{ID: id, TenantID: userID, EmbdID: "embed-1"}, nil }, @@ -1852,6 +1859,14 @@ func insertChunkTestUserTenant(t *testing.T, userID, tenantID string) { func insertChunkTestKB(t *testing.T, id, tenantID string) { t.Helper() + var owner entity.UserTenant + err := dao.DB.Where("tenant_id = ? AND role = ? AND status = ?", tenantID, "owner", "1").First(&owner).Error + if errors.Is(err, gorm.ErrRecordNotFound) { + insertChunkTestUserTenant(t, tenantID, tenantID) + } else if err != nil { + t.Fatalf("find tenant owner: %v", err) + } + status := string(entity.StatusValid) kb := &entity.Knowledgebase{ ID: id, @@ -2463,7 +2478,7 @@ func TestChunkServiceParse_RejectsBatchWithRunningIngestionTask(t *testing.T) { insertChunkTestIngestionTask(t, "task-2", "user-1", "doc-2", "kb-1", common.RUNNING) svc := newParseTestService(t) - svc.accessibleFunc = func(string, string) bool { return true } + svc.checkDatasetAccessFunc = func(string, string) error { return nil } ctx := t.Context() _, _, err := svc.Parse(ctx, "user-1", "kb-1", &service.ParseFileRequest{DocumentIDs: []string{"doc-1", "doc-2"}}) if err == nil { @@ -2479,7 +2494,7 @@ func TestChunkServiceParse_CallsStartParseDocumentsWithRerunWithDelete(t *testin insertChunkTestDoc(t, "doc-1", "kb-1") svc := newParseTestService(t) - svc.accessibleFunc = func(string, string) bool { return true } + svc.checkDatasetAccessFunc = func(string, string) error { return nil } var calledOpts document.StartParseOptions var calledDocID string svc.startParseDocumentsFunc = func(ctx context.Context, doc *entity.Document, kb *entity.Knowledgebase, userID string, opts document.StartParseOptions) error { diff --git a/internal/service/connector.go b/internal/service/connector.go index f0a4d6472b..c5232f9a70 100644 --- a/internal/service/connector.go +++ b/internal/service/connector.go @@ -41,6 +41,8 @@ import ( "ragflow/internal/dao" "ragflow/internal/engine" "ragflow/internal/entity" + "ragflow/internal/permission" + permissionresponse "ragflow/internal/permission/response" ) const ( @@ -1088,10 +1090,11 @@ func (s *ConnectorService) RebuildConnector(ctx context.Context, connectorID, us // The caller-supplied kb is targeted by delete + re-sync below, so it must // be accessible to the caller and the connector must be bound to it. - if !s.knowledgebaseDAO.Accessible(ctx, dao.DB, kbID, userID) { + if err := CheckDatasetAccess(ctx, permission.Subject{UserID: userID}, kbID, permission.OperationRun); err != nil { common.Warn("rebuild denied: kb not accessible", zap.String("connector_id", connectorID), zap.String("kb_id", kbID), zap.String("user_id", userID)) - return false, common.CodeAuthenticationError, ErrConnectorNoAuth + code, permissionErr := permissionresponse.Normalize(err) + return false, code, permissionErr } bound, err := s.connectorDAO.Connector2KBExists(ctx, dao.DB, connectorID, kbID) if err != nil { diff --git a/internal/service/connector_syncer_test.go b/internal/service/connector_syncer_test.go index d1540c5e3e..b6adb91051 100644 --- a/internal/service/connector_syncer_test.go +++ b/internal/service/connector_syncer_test.go @@ -8,6 +8,7 @@ import ( "ragflow/internal/common" "ragflow/internal/dao" "ragflow/internal/entity" + "ragflow/internal/permission" syncerconnector "ragflow/internal/syncer/connector" ) @@ -216,14 +217,22 @@ func TestRebuildConnectorDeletesOldSyncCheckpointsBeforePublishing(t *testing.T) t.Fatalf("insert connector: %v", err) } if err := db.Create(&entity.Knowledgebase{ - ID: "kb-1", - TenantID: "user-1", - Name: "kb-1", - CreatedBy: "user-1", - EmbdID: "embd", + ID: "kb-1", + TenantID: "user-1", + Name: "kb-1", + CreatedBy: "user-1", + EmbdID: "embd", + Permission: string(entity.TenantPermissionMe), + Status: sptr(string(entity.StatusValid)), }).Error; err != nil { t.Fatalf("insert kb: %v", err) } + if err := db.Create(&entity.UserTenant{ + ID: "ut-owner", UserID: "user-1", TenantID: "user-1", + Role: "owner", Status: sptr(string(entity.StatusValid)), + }).Error; err != nil { + t.Fatalf("insert tenant owner: %v", err) + } if err := db.Create(&entity.Connector2Kb{ID: "conn-1-kb-1", ConnectorID: "conn-1", KbID: "kb-1", AutoParse: "1"}).Error; err != nil { t.Fatalf("insert connector mapping: %v", err) } @@ -286,13 +295,19 @@ func TestRebuildConnectorRejectsCrossTenantAndUnboundKB(t *testing.T) { t.Fatalf("insert connector: %v", err) } // kb-1 is owned by the caller but is never bound to conn-1. - if err := db.Create(&entity.Knowledgebase{ID: "kb-1", TenantID: "user-1", Name: "kb-1", CreatedBy: "user-1", EmbdID: "embd"}).Error; err != nil { + if err := db.Create(&entity.Knowledgebase{ID: "kb-1", TenantID: "user-1", Name: "kb-1", CreatedBy: "user-1", EmbdID: "embd", Permission: string(entity.TenantPermissionMe), Status: sptr(string(entity.StatusValid))}).Error; err != nil { t.Fatalf("insert kb-1: %v", err) } // kb-foreign belongs to another tenant. - if err := db.Create(&entity.Knowledgebase{ID: "kb-foreign", TenantID: "user-2", Name: "kb-foreign", CreatedBy: "user-2", EmbdID: "embd"}).Error; err != nil { + if err := db.Create(&entity.Knowledgebase{ID: "kb-foreign", TenantID: "user-2", Name: "kb-foreign", CreatedBy: "user-2", EmbdID: "embd", Permission: string(entity.TenantPermissionMe), Status: sptr(string(entity.StatusValid))}).Error; err != nil { t.Fatalf("insert kb-foreign: %v", err) } + if err := db.Create([]entity.UserTenant{ + {ID: "ut-owner-1", UserID: "user-1", TenantID: "user-1", Role: "owner", Status: sptr(string(entity.StatusValid))}, + {ID: "ut-owner-2", UserID: "user-2", TenantID: "user-2", Role: "owner", Status: sptr(string(entity.StatusValid))}, + }).Error; err != nil { + t.Fatalf("insert tenant owners: %v", err) + } svc := NewConnectorService() @@ -308,11 +323,11 @@ func TestRebuildConnectorRejectsCrossTenantAndUnboundKB(t *testing.T) { // The caller cannot access a cross-tenant kb at all: denied up front. ok, code, err = svc.RebuildConnector(t.Context(), "conn-1", "user-1", "kb-foreign") - if err == nil || !errors.Is(err, ErrConnectorNoAuth) { - t.Fatalf("err = %v, want ErrConnectorNoAuth", err) + if err == nil || !errors.Is(err, permission.ErrPermissionDenied) { + t.Fatalf("err = %v, want permission denial", err) } - if ok || code != common.CodeAuthenticationError { - t.Fatalf("ok/code = %v/%v, want false/authentication error", ok, code) + if ok || code != common.CodeForbidden { + t.Fatalf("ok/code = %v/%v, want false/forbidden", ok, code) } } diff --git a/internal/service/dataset/compilation_status.go b/internal/service/dataset/compilation_status.go index 9224bde74d..3ebde2aed3 100644 --- a/internal/service/dataset/compilation_status.go +++ b/internal/service/dataset/compilation_status.go @@ -12,6 +12,8 @@ import ( "ragflow/internal/common" "ragflow/internal/dao" "ragflow/internal/entity" + "ragflow/internal/permission" + permissionresponse "ragflow/internal/permission/response" ) // CompilationStatus is the dataset-level knowledge-compile lifecycle state @@ -41,8 +43,9 @@ func (d *DatasetService) GetDatasetCompilationStatus(ctx context.Context, userID if datasetID == "" { return CompilationStatus{}, common.CodeDataError, errors.New("dataset_id is required") } - if !d.kbDAO.Accessible(ctx, dao.DB, datasetID, userID) { - return CompilationStatus{}, common.CodeDataError, errors.New("no authorization") + if err := d.CheckAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationRead); err != nil { + code, permissionErr := permissionresponse.Normalize(err) + return CompilationStatus{}, code, permissionErr } requestedKind := strings.ToLower(strings.TrimSpace(kind)) normalizedKind := normalizeCompilationKind(requestedKind) diff --git a/internal/service/dataset/compilation_status_test.go b/internal/service/dataset/compilation_status_test.go index 7120d5e3ca..ad52da55fb 100644 --- a/internal/service/dataset/compilation_status_test.go +++ b/internal/service/dataset/compilation_status_test.go @@ -80,6 +80,7 @@ func insertCompilationOwnerKB(t *testing.T, kbID, userID string) { if err := dao.DB.Create(kb).Error; err != nil { t.Fatalf("insert kb: %v", err) } + ensureDatasetTestMembership(t, userID, userID, "owner") } func testCompilationStatusService() *DatasetService { @@ -189,8 +190,8 @@ func TestGetDatasetCompilationStatus_Unauthorized(t *testing.T) { if err == nil { t.Fatalf("expected authorization error, got nil (status=%+v)", st) } - if code != common.CodeDataError { - t.Fatalf("code=%d want %d", code, common.CodeDataError) + if code != common.CodeForbidden { + t.Fatalf("code=%d want %d", code, common.CodeForbidden) } } diff --git a/internal/service/dataset/create_test.go b/internal/service/dataset/create_test.go index cf2e14b5cc..f29b26c009 100644 --- a/internal/service/dataset/create_test.go +++ b/internal/service/dataset/create_test.go @@ -9,9 +9,14 @@ import ( "ragflow/internal/common" "ragflow/internal/dao" "ragflow/internal/entity" + "ragflow/internal/permission" "ragflow/internal/service" ) +func testDatasetSubject(id string) permission.Subject { + return permission.Subject{UserID: id, TenantID: id} +} + func insertCreateDatasetTenant(t *testing.T, tenantID string) { t.Helper() var existing entity.Tenant @@ -28,6 +33,7 @@ func insertCreateDatasetTenant(t *testing.T, tenantID string) { t.Fatalf("insert test tenant: %v", err) } } + ensureDatasetTestMembership(t, tenantID, tenantID, "owner") } func testDatasetCreateService(t *testing.T) *DatasetService { @@ -54,7 +60,7 @@ func TestCreateDataset_NoComponentParams(t *testing.T) { Name: "ds-no-cp", ParserID: &chunkMethod, ParseType: &parseType, - }, "tenant-1") + }, testDatasetSubject("tenant-1")) if err != nil { t.Fatalf("CreateDataset failed: %v", err) } @@ -82,7 +88,7 @@ func TestCreateDataset_DefaultsParentChildConfig(t *testing.T) { ParserID: sptr("general"), ParseType: iptr(1), ParserConfig: map[string]interface{}{}, - }, "tenant-1") + }, testDatasetSubject("tenant-1")) if err != nil || code != common.CodeSuccess { t.Fatalf("CreateDataset err=%v code=%d", err, code) } @@ -117,6 +123,20 @@ func findChunkerNodeInTest(t *testing.T, config map[string]interface{}) map[stri return nil } +func TestCreateDataset_RequiresParseTypeWhenParserIDIsProvided(t *testing.T) { + db := setupServiceTestDB(t) + pushServiceDB(t, db) + insertCreateDatasetTenant(t, "tenant-1") + + parserID := "qa" + _, code, err := testDatasetCreateService(t).CreateDataset(t.Context(), &service.CreateDatasetRequest{ + Name: "ds-parser-only", + ParserID: &parserID, + }, testDatasetSubject("tenant-1")) + if err == nil || code != common.CodeDataError || err.Error() != "parse_type is required" { + t.Fatalf("CreateDataset = (code=%d, err=%v), want data error for missing parse_type", code, err) + } +} func TestCreateDataset_ComponentParamsPopulated(t *testing.T) { db := setupServiceTestDB(t) pushServiceDB(t, db) @@ -130,7 +150,7 @@ func TestCreateDataset_ComponentParamsPopulated(t *testing.T) { ParserID: &chunkMethod, ParserConfig: map[string]interface{}{}, ParseType: &parseType, - }, "tenant-1") + }, testDatasetSubject("tenant-1")) if err != nil { t.Fatalf("CreateDataset failed: %v", err) } @@ -172,7 +192,7 @@ func TestCreateDataset_ParentChildConfigReachesGeneralChunker(t *testing.T) { }, }, }, - }, "tenant-1") + }, testDatasetSubject("tenant-1")) if err != nil || code != common.CodeSuccess { t.Fatalf("CreateDataset err=%v code=%d", err, code) } @@ -205,7 +225,7 @@ func TestCreateDataset_PreservesChunkerComponentOverrides(t *testing.T) { "chunk_token_size": float64(256), }, }, - }, "tenant-1") + }, testDatasetSubject("tenant-1")) if err != nil || code != common.CodeSuccess { t.Fatalf("CreateDataset err=%v code=%d", err, code) } @@ -235,7 +255,7 @@ func TestCreateDataset_ParseTypeBuiltinHasNoPipelineID(t *testing.T) { Name: "ds-parse-builtin", ParserID: &chunkMethod, ParseType: &parseTypeBuiltin, - }, "tenant-1") + }, testDatasetSubject("tenant-1")) if err != nil { t.Fatalf("CreateDataset failed: %v", err) } @@ -263,7 +283,7 @@ func TestCreateDataset_ParseTypePipelineIgnoresParserID(t *testing.T) { Name: "ds-parse-pipeline", PipelineID: &pipelineID, ParseType: &parseTypePipeline, - }, "tenant-1") + }, testDatasetSubject("tenant-1")) if err != nil { t.Fatalf("CreateDataset failed: %v", err) } @@ -282,7 +302,7 @@ func TestCreateDataset_ValidatesName(t *testing.T) { insertCreateDatasetTenant(t, "tenant-1") ctx := t.Context() - _, code, err := testDatasetCreateService(t).CreateDataset(ctx, &service.CreateDatasetRequest{Name: " "}, "tenant-1") + _, code, err := testDatasetCreateService(t).CreateDataset(ctx, &service.CreateDatasetRequest{Name: " "}, testDatasetSubject("tenant-1")) if err == nil { t.Fatal("expected name validation error") } @@ -316,7 +336,7 @@ func TestCreateDataset_DedupesDuplicateName(t *testing.T) { Name: "Existing", ParserID: sptr("general"), ParseType: iptr(1), - }, "tenant-1") + }, testDatasetSubject("tenant-1")) if err != nil { t.Fatalf("expected success, got %v", err) } @@ -328,6 +348,48 @@ func TestCreateDataset_DedupesDuplicateName(t *testing.T) { } } +func TestCreateDataset_UsesActorAsCreatorAndTenantAsOwner(t *testing.T) { + db := setupServiceTestDB(t) + pushServiceDB(t, db) + insertCreateDatasetTenant(t, "tenant-1") + ensureDatasetTestMembership(t, "member-1", "tenant-1", "normal") + + parserID := "general" + parseType := 1 + result, code, err := testDatasetCreateService(t).CreateDataset(t.Context(), &service.CreateDatasetRequest{ + Name: "created-by-member", + ParserID: &parserID, + ParseType: &parseType, + }, permission.Subject{UserID: "member-1", TenantID: "tenant-1"}) + if err != nil || code != common.CodeSuccess { + t.Fatalf("CreateDataset err=%v code=%d", err, code) + } + + var dataset entity.Knowledgebase + if err := db.Where("id = ?", result["id"]).Take(&dataset).Error; err != nil { + t.Fatalf("load created dataset: %v", err) + } + if dataset.TenantID != "tenant-1" { + t.Fatalf("tenant_id = %q, want tenant-1", dataset.TenantID) + } + if dataset.CreatedBy != "member-1" { + t.Fatalf("created_by = %q, want member-1", dataset.CreatedBy) + } +} + +func TestCreateDatasetRejectsUserWithoutTenantMembership(t *testing.T) { + db := setupServiceTestDB(t) + pushServiceDB(t, db) + insertCreateDatasetTenant(t, "tenant-1") + + _, code, err := testDatasetCreateService(t).CreateDataset(t.Context(), &service.CreateDatasetRequest{ + Name: "unauthorized-dataset", + }, permission.Subject{UserID: "outsider", TenantID: "tenant-1"}) + if err == nil || code != common.CodeForbidden || err.Error() != "Permission denied" { + t.Fatalf("CreateDataset = (code=%d, err=%v), want forbidden for missing tenant membership", code, err) + } +} + func TestCreateDataset_RejectsInvalidEmbeddingModel(t *testing.T) { cases := []struct { name string @@ -355,7 +417,7 @@ func TestCreateDataset_RejectsInvalidEmbeddingModel(t *testing.T) { ParserID: sptr("general"), ParseType: iptr(1), EmbeddingModel: &tc.embeddingModel, - }, "tenant-1") + }, testDatasetSubject("tenant-1")) if err == nil { t.Fatal("expected embedding model validation error") } @@ -381,7 +443,7 @@ func TestCreateDataset_SetsExplicitLanguage(t *testing.T) { Language: &language, ParserID: sptr("general"), ParseType: iptr(1), - }, "tenant-1") + }, testDatasetSubject("tenant-1")) if err != nil { t.Fatalf("CreateDataset failed: %v", err) } @@ -405,7 +467,7 @@ func TestCreateDataset_OmittedLanguageKeepsDefault(t *testing.T) { Name: "ds-no-language", ParserID: sptr("general"), ParseType: iptr(1), - }, "tenant-1") + }, testDatasetSubject("tenant-1")) if err != nil { t.Fatalf("CreateDataset failed: %v", err) } @@ -437,7 +499,7 @@ func TestCreateDataset_RejectsBlankLanguage(t *testing.T) { Language: &language, ParserID: sptr("general"), ParseType: iptr(1), - }, "tenant-1") + }, testDatasetSubject("tenant-1")) if err == nil { t.Fatal("expected language validation error") } @@ -467,7 +529,7 @@ func TestCreateDataset_LanguageLimitCountsCharacters(t *testing.T) { Language: &atLimit, ParserID: sptr("general"), ParseType: iptr(1), - }, "tenant-1") + }, testDatasetSubject("tenant-1")) if err != nil { t.Fatalf("CreateDataset failed for a %d-character language: %v", datasetLanguageLimit, err) } @@ -483,7 +545,7 @@ func TestCreateDataset_LanguageLimitCountsCharacters(t *testing.T) { Language: &overLimit, ParserID: sptr("general"), ParseType: iptr(1), - }, "tenant-1") + }, testDatasetSubject("tenant-1")) if err == nil { t.Fatal("expected language length validation error") } diff --git a/internal/service/dataset/crud.go b/internal/service/dataset/crud.go index 5273481471..a894b35fac 100644 --- a/internal/service/dataset/crud.go +++ b/internal/service/dataset/crud.go @@ -11,6 +11,8 @@ import ( "ragflow/internal/dao" "ragflow/internal/entity" pipelinepkg "ragflow/internal/ingestion/pipeline" + "ragflow/internal/permission" + permissionresponse "ragflow/internal/permission/response" "ragflow/internal/service" "ragflow/internal/storage" "ragflow/internal/utility" @@ -19,7 +21,12 @@ import ( "gorm.io/gorm" ) -func (d *DatasetService) CreateDataset(ctx context.Context, req *service.CreateDatasetRequest, tenantID string) (map[string]interface{}, common.ErrorCode, error) { +func (d *DatasetService) CreateDataset(ctx context.Context, req *service.CreateDatasetRequest, subject permission.Subject) (map[string]interface{}, common.ErrorCode, error) { + tenantID := strings.TrimSpace(subject.TenantID) + userID := strings.TrimSpace(subject.UserID) + if tenantID == "" || userID == "" { + return nil, common.CodeUnauthorized, errors.New("user id and tenant id are required") + } if !common.IsValidString(req.Name) { return nil, common.CodeDataError, errors.New("dataset name must be string") } @@ -36,6 +43,10 @@ func (d *DatasetService) CreateDataset(ctx context.Context, req *service.CreateD if err != nil || tenant == nil { return nil, common.CodeDataError, errors.New("tenant not found") } + if err := permission.NewDatabaseChecker(dao.DB).CheckTenant(ctx, subject, tenantID, permission.TenantMember); err != nil { + code, permissionErr := permissionresponse.Normalize(err) + return nil, code, permissionErr + } // parse_type is required on dataset creation: it explicitly selects BuiltIn // (1) or Pipeline (2) mode. This replaces the previous silent default to @@ -101,7 +112,7 @@ func (d *DatasetService) CreateDataset(ctx context.Context, req *service.CreateD } if pipelineID != nil && strings.TrimSpace(*pipelineID) != "" { - if ok, err := canvasAccessibleForUser(ctx, tenantID, strings.TrimSpace(*pipelineID)); err != nil { + if ok, err := canvasAccessibleForUser(ctx, userID, strings.TrimSpace(*pipelineID)); err != nil { return nil, common.CodeServerError, err } else if !ok { return nil, common.CodeDataError, errors.New("canvas is not accessible") @@ -171,7 +182,7 @@ func (d *DatasetService) CreateDataset(ctx context.Context, req *service.CreateD embdID := tenant.EmbdID tenantEmbdID := ptrStringValue(tenant.TenantEmbdID) if embeddingModel != "" { - ok, message := d.verifyEmbeddingAvailability(ctx, embeddingModel, tenantID) + ok, message := d.verifyEmbeddingAvailability(ctx, embeddingModel, service.ModelAccess{UserID: userID, TenantID: tenantID}) if !ok { return nil, common.CodeDataError, errors.New(message) } @@ -179,7 +190,7 @@ func (d *DatasetService) CreateDataset(ctx context.Context, req *service.CreateD tenantEmbdID = "" } if embdID != "" && tenantEmbdID == "" { - target, err := service.NewModelFactory().ResolveInfo(ctx, service.ModelAccess{TenantID: tenantID}, entity.ModelTypeEmbedding, embdID) + target, err := service.NewModelFactory().ResolveInfo(ctx, service.ModelAccess{UserID: userID, TenantID: tenantID}, entity.ModelTypeEmbedding, embdID) if err == nil { tenantEmbdID = target.ID } else { @@ -205,7 +216,7 @@ func (d *DatasetService) CreateDataset(ctx context.Context, req *service.CreateD ID: kbID, Name: name, TenantID: tenantID, - CreatedBy: tenantID, + CreatedBy: userID, ParserID: parserID, PipelineID: pipelineID, ParserConfig: entity.JSONMap(parserConfigMap), @@ -253,17 +264,20 @@ func (d *DatasetService) GetDataset(ctx context.Context, datasetID, userID strin // or malformed id simply fails the permission check. normalizedID, err := normalizeDatasetID(datasetID) if err != nil { - return nil, common.CodeDataError, fmt.Errorf("user '%s' lacks permission for dataset '%s'", userID, datasetID) + code, permissionErr := permissionresponse.NormalizeHidden(permission.ErrResourceNotFound) + return nil, code, permissionErr } datasetID = normalizedID - if !d.kbDAO.Accessible(ctx, dao.DB, datasetID, userID) { - return nil, common.CodeDataError, fmt.Errorf("user '%s' lacks permission for dataset '%s'", userID, datasetID) + if err := d.CheckAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationRead); err != nil { + code, permissionErr := permissionresponse.NormalizeHidden(err) + return nil, code, permissionErr } kb, err := d.kbDAO.GetByID(ctx, dao.DB, datasetID) if err != nil || kb == nil { - return nil, common.CodeDataError, errors.New("invalid Dataset ID") + code, permissionErr := permissionresponse.NormalizeHidden(permission.ErrResourceNotFound) + return nil, code, permissionErr } data := datasetToMap(kb) @@ -283,7 +297,13 @@ func (d *DatasetService) GetDataset(ctx context.Context, datasetID, userID strin return data, common.CodeSuccess, nil } -func (d *DatasetService) DeleteDatasets(ctx context.Context, ids []string, deleteAll bool, tenantID string) (map[string]interface{}, common.ErrorCode, error) { +func (d *DatasetService) DeleteDatasets(ctx context.Context, ids []string, deleteAll bool, subject permission.Subject) (map[string]interface{}, common.ErrorCode, error) { + subject.UserID = strings.TrimSpace(subject.UserID) + subject.TenantID = strings.TrimSpace(subject.TenantID) + if subject.UserID == "" { + return nil, common.CodeUnauthorized, errors.New("user id is required") + } + normalizedIDs := make([]string, 0, len(ids)) seenIDs := make(map[string]struct{}, len(ids)) for _, id := range ids { @@ -298,12 +318,20 @@ func (d *DatasetService) DeleteDatasets(ctx context.Context, ids []string, delet normalizedIDs = append(normalizedIDs, normalizedID) } - // If no explicit ids and deleteAll is set, resolve all datasets for this tenant. + checker := permission.NewDatabaseChecker(dao.DB) + // Delete-all is tenant-scoped and remains an owner-only operation. if len(normalizedIDs) == 0 { if !deleteAll { return map[string]interface{}{"deleted": []string{}}, common.CodeSuccess, nil } - kbs, err := d.kbDAO.Query(ctx, dao.DB, map[string]interface{}{"tenant_id": tenantID}) + if subject.TenantID == "" { + return nil, common.CodeUnauthorized, errors.New("tenant id is required to delete all datasets") + } + if err := checker.CheckTenant(ctx, subject, subject.TenantID, permission.TenantOwner); err != nil { + code, permissionErr := permissionresponse.Normalize(err) + return nil, code, permissionErr + } + kbs, err := d.kbDAO.Query(ctx, dao.DB, map[string]interface{}{"tenant_id": subject.TenantID}) if err != nil { return nil, common.CodeServerError, errors.New("database operation failed") } @@ -312,11 +340,28 @@ func (d *DatasetService) DeleteDatasets(ctx context.Context, ids []string, delet } } - // Validate ownership: collect KBs that exist and belong to this tenant. + refs := make([]permission.ResourceRef, 0, len(normalizedIDs)) + for _, id := range normalizedIDs { + refs = append(refs, permission.ResourceRef{Kind: permission.ResourceKindDataset, ID: id}) + } + accessible, err := checker.FilterResources(ctx, subject, refs, permission.OperationDelete) + if err != nil { + code, permissionErr := permissionresponse.Normalize(err) + return nil, code, permissionErr + } + accessibleIDs := make(map[string]struct{}, len(accessible)) + for _, ref := range accessible { + accessibleIDs[ref.ID] = struct{}{} + } + kbs := make([]*entity.Knowledgebase, 0, len(normalizedIDs)) unauthorizedIDs := make([]string, 0) for _, id := range normalizedIDs { - kb, err := d.kbDAO.GetByIDAndTenantID(ctx, dao.DB, id, tenantID) + if _, ok := accessibleIDs[id]; !ok { + unauthorizedIDs = append(unauthorizedIDs, id) + continue + } + kb, err := d.kbDAO.GetByID(ctx, dao.DB, id) if err != nil || kb == nil { unauthorizedIDs = append(unauthorizedIDs, id) continue @@ -324,13 +369,13 @@ func (d *DatasetService) DeleteDatasets(ctx context.Context, ids []string, delet kbs = append(kbs, kb) } if len(unauthorizedIDs) > 0 { - return nil, common.CodeDataError, - fmt.Errorf("user '%s' lacks permission for datasets: '%s'", tenantID, strings.Join(unauthorizedIDs, ", ")) + _, permissionErr := permissionresponse.Normalize(permission.ErrPermissionDenied) + return nil, common.CodeForbidden, permissionErr } successCount := 0 for _, kb := range kbs { - if err := d.deleteDataset(ctx, tenantID, kb); err != nil { + if err := d.deleteDataset(ctx, kb); err != nil { common.Warn("deleteDataset failed", zap.String("dataset", datasetNameAndID(kb)), zap.String("kb_id", kb.ID), zap.Error(err)) return nil, common.CodeServerError, err } @@ -343,7 +388,7 @@ func (d *DatasetService) DeleteDatasets(ctx context.Context, ids []string, delet }, common.CodeSuccess, nil } -func (d *DatasetService) deleteDataset(ctx context.Context, tenantID string, kb *entity.Knowledgebase) error { +func (d *DatasetService) deleteDataset(ctx context.Context, kb *entity.Knowledgebase) error { storageImpl := storage.GetStorageFactory().GetStorage() if storageImpl == nil { return fmt.Errorf("storage not initialized") @@ -427,7 +472,7 @@ func (d *DatasetService) deleteDataset(ctx context.Context, tenantID string, kb // Delete the KB folder file record. if err := tx.Unscoped(). Where("source_type = ? AND type = ? AND name = ? AND tenant_id = ?", - string(entity.FileSourceKnowledgebase), "folder", kb.Name, tenantID). + string(entity.FileSourceKnowledgebase), "folder", kb.Name, kb.TenantID). Delete(&entity.File{}).Error; err != nil { return fmt.Errorf("delete dataset error for %s", kb.ID) } @@ -468,26 +513,9 @@ func (d *DatasetService) ListDatasets(ctx context.Context, id, name string, page return nil, 0, common.CodeArgumentError, err } id = normalizedID - - kbs, err := d.kbDAO.GetKBByIDAndUserID(ctx, dao.DB, id, userID) - if err != nil { - return nil, 0, common.CodeServerError, errors.New("database operation failed") - } - if len(kbs) == 0 { - return nil, 0, common.CodeDataError, fmt.Errorf("user '%s' lacks permission for dataset '%s'", userID, id) - } } name = strings.TrimSpace(name) - if name != "" { - kbs, err := d.kbDAO.GetKBByNameAndUserID(ctx, dao.DB, name, userID) - if err != nil { - return nil, 0, common.CodeServerError, errors.New("database operation failed") - } - if len(kbs) == 0 { - return nil, 0, common.CodeDataError, fmt.Errorf("user '%s' lacks permission for dataset '%s'", userID, name) - } - } if page <= 0 { page = 1 @@ -501,90 +529,92 @@ func (d *DatasetService) ListDatasets(ctx context.Context, id, name string, page keywords = strings.TrimSpace(keywords) parserID = strings.TrimSpace(parserID) - tenantIDs := make([]string, 0, len(ownerIDs)) + filteredOwnerIDs := make([]string, 0, len(ownerIDs)) + seenOwnerIDs := make(map[string]struct{}, len(ownerIDs)) for _, ownerID := range ownerIDs { ownerID = strings.TrimSpace(ownerID) - if ownerID != "" { - tenantIDs = append(tenantIDs, ownerID) + if ownerID == "" { + continue } - } - queryUserID := userID - var joinedTenantIDs []string - if len(tenantIDs) > 0 { - joinedTenants, err := d.tenantDAO.GetJoinedTenantsByUserID(ctx, dao.DB, userID) - if err != nil { - return nil, 0, common.CodeServerError, errors.New("database operation failed") - } - allowedTenantIDs := map[string]struct{}{userID: {}} - for _, joinedTenant := range joinedTenants { - if joinedTenant == nil || joinedTenant.TenantID == "" { - continue - } - allowedTenantIDs[joinedTenant.TenantID] = struct{}{} - joinedTenantIDs = append(joinedTenantIDs, joinedTenant.TenantID) - } - filteredTenantIDs := tenantIDs[:0] - queryUserID = "" - for _, tenantID := range tenantIDs { - if _, ok := allowedTenantIDs[tenantID]; !ok { - continue - } - filteredTenantIDs = append(filteredTenantIDs, tenantID) - if tenantID == userID { - queryUserID = userID - } - } - tenantIDs = filteredTenantIDs - } else { - joinedTenants, err := d.tenantDAO.GetJoinedTenantsByUserID(ctx, dao.DB, userID) - if err != nil { - return nil, 0, common.CodeServerError, errors.New("database operation failed") - } - for _, joinedTenant := range joinedTenants { - if joinedTenant == nil || joinedTenant.TenantID == "" { - continue - } - tenantIDs = append(tenantIDs, joinedTenant.TenantID) - joinedTenantIDs = append(joinedTenantIDs, joinedTenant.TenantID) + if _, exists := seenOwnerIDs[ownerID]; exists { + continue } + seenOwnerIDs[ownerID] = struct{}{} + filteredOwnerIDs = append(filteredOwnerIDs, ownerID) } - // Mirror Python: ids are checked for accessibility against the joined - // tenants (not the owner-filtered tenant list) before filtering. - if len(ids) > 0 { - accessibleIDs, err := d.kbDAO.GetAccessibleIDs(ctx, dao.DB, joinedTenantIDs, userID, ids) - if err != nil { - return nil, 0, common.CodeServerError, errors.New("database operation failed") - } - accessible := make(map[string]struct{}, len(accessibleIDs)) - for _, accessibleID := range accessibleIDs { - accessible[accessibleID] = struct{}{} - } - filteredIDs := make([]string, 0, len(ids)) - deniedIDs := make([]string, 0, len(ids)) - for _, datasetID := range ids { - if _, ok := accessible[datasetID]; ok { - filteredIDs = append(filteredIDs, datasetID) - } else { - deniedIDs = append(deniedIDs, datasetID) + var kbs []*entity.KnowledgebaseListItem + var total int64 + var deniedIDs []string + err := dao.DB.Transaction(func(tx *gorm.DB) error { + checker := permission.NewDatabaseChecker(tx) + subject := permission.Subject{UserID: userID} + var resourceIDs []string + + switch { + case id != "": + checkErr := checker.CheckResource(ctx, subject, permission.ResourceRef{Kind: permission.ResourceKindDataset, ID: id}, permission.OperationRead) + if checkErr != nil { + return checkErr + } + resourceIDs = []string{id} + case len(ids) > 0: + refs := make([]permission.ResourceRef, 0, len(ids)) + for _, datasetID := range ids { + refs = append(refs, permission.ResourceRef{Kind: permission.ResourceKindDataset, ID: datasetID}) + } + accessible, checkErr := checker.FilterResources(ctx, subject, refs, permission.OperationRead) + if checkErr != nil { + return checkErr + } + accessibleIDs := make(map[string]struct{}, len(accessible)) + for _, ref := range accessible { + accessibleIDs[ref.ID] = struct{}{} + } + resourceIDs = make([]string, 0, len(accessibleIDs)) + for _, datasetID := range ids { + if _, ok := accessibleIDs[datasetID]; ok { + resourceIDs = append(resourceIDs, datasetID) + } else { + deniedIDs = append(deniedIDs, datasetID) + } + } + default: + scope, checkErr := checker.Scope(ctx, subject, permission.ScopeQuery{ + Kind: permission.ResourceKindDataset, Operation: permission.OperationRead, + }) + if checkErr != nil { + return checkErr + } + if scope.Mode == permission.ScopeIDs { + resourceIDs = scope.ResourceIDs } } - if len(deniedIDs) > 0 { - common.Warn("User lacks permission for datasets", - zap.String("user_id", userID), - zap.Strings("dataset_ids", deniedIDs), - ) - } - ids = filteredIDs - if len(ids) == 0 { - return []map[string]interface{}{}, 0, common.CodeSuccess, nil - } - } - kbs, total, err := d.kbDAO.GetByTenantIDs(ctx, dao.DB, tenantIDs, queryUserID, page, pageSize, terms, keywords, parserID, id, name, ids) + if len(resourceIDs) == 0 { + return nil + } + listed, count, err := d.kbDAO.ListByResourceIDs(ctx, tx, resourceIDs, filteredOwnerIDs, page, pageSize, terms, keywords, parserID, name) + if err != nil { + return err + } + kbs, total = listed, count + return nil + }) if err != nil { + if errors.Is(err, permission.ErrUnauthenticated) || errors.Is(err, permission.ErrPermissionDenied) || + errors.Is(err, permission.ErrMembershipNotFound) || errors.Is(err, permission.ErrResourceNotFound) { + code, permissionErr := permissionresponse.Normalize(err) + return nil, 0, code, permissionErr + } return nil, 0, common.CodeServerError, errors.New("database operation failed") } + if len(deniedIDs) > 0 { + common.Warn("User lacks permission for datasets", + zap.String("user_id", userID), + zap.Strings("dataset_ids", common.Deduplicate(deniedIDs)), + ) + } data := make([]map[string]interface{}, 0, len(kbs)) modelNameCache := make(map[string]string) @@ -607,21 +637,26 @@ func (d *DatasetService) ListDatasets(ctx context.Context, id, name string, page } func (d *DatasetService) ListDatasetFilters(ctx context.Context, userID string) (map[string]interface{}, common.ErrorCode, error) { - joinedTenants, err := d.tenantDAO.GetJoinedTenantsByUserID(ctx, dao.DB, userID) - if err != nil { - return nil, common.CodeServerError, errors.New("database operation failed") - } - - tenantIDs := make([]string, 0, len(joinedTenants)) - for _, joinedTenant := range joinedTenants { - if joinedTenant == nil || joinedTenant.TenantID == "" { - continue + owners := make([]*entity.DatasetOwnerFilter, 0) + err := dao.DB.Transaction(func(tx *gorm.DB) error { + scope, err := permission.NewDatabaseChecker(tx).Scope(ctx, permission.Subject{UserID: userID}, permission.ScopeQuery{ + Kind: permission.ResourceKindDataset, Operation: permission.OperationRead, + }) + if err != nil { + return err } - tenantIDs = append(tenantIDs, joinedTenant.TenantID) - } - - owners, err := d.kbDAO.GetOwnerFilter(ctx, dao.DB, tenantIDs, userID) + if scope.Mode == permission.ScopeNone { + return nil + } + owners, err = d.kbDAO.GetOwnerFilterByResourceIDs(ctx, tx, scope.ResourceIDs) + return err + }) if err != nil { + if errors.Is(err, permission.ErrUnauthenticated) || errors.Is(err, permission.ErrPermissionDenied) || + errors.Is(err, permission.ErrMembershipNotFound) || errors.Is(err, permission.ErrResourceNotFound) { + code, permissionErr := permissionresponse.Normalize(err) + return nil, code, permissionErr + } return nil, common.CodeServerError, errors.New("database operation failed") } diff --git a/internal/service/dataset/crud_delete_test.go b/internal/service/dataset/crud_delete_test.go index a2924f227a..015d91e37e 100644 --- a/internal/service/dataset/crud_delete_test.go +++ b/internal/service/dataset/crud_delete_test.go @@ -57,7 +57,7 @@ func TestDeleteDatasetRemovesTemporaryFiles(t *testing.T) { t.Fatal(err) } service := testDatasetUpdateService(t) - if err := service.deleteDataset(t.Context(), "tenant-1", kb); err != nil { + if err := service.deleteDataset(t.Context(), kb); err != nil { t.Fatalf("delete dataset with temporary file: %v", err) } if store.ObjExist(t.Context(), "kb-1", "temporary") { @@ -96,7 +96,7 @@ func TestDeleteDatasetContinuesWhenStorageIsMissing(t *testing.T) { if err != nil { t.Fatal(err) } - if err := testDatasetUpdateService(t).deleteDataset(t.Context(), "tenant-1", kb); err != nil { + if err := testDatasetUpdateService(t).deleteDataset(t.Context(), kb); err != nil { t.Fatalf("delete dataset with missing storage: %v", err) } var count int64 diff --git a/internal/service/dataset/embedding.go b/internal/service/dataset/embedding.go index 396067863e..18f0609cc1 100644 --- a/internal/service/dataset/embedding.go +++ b/internal/service/dataset/embedding.go @@ -11,6 +11,8 @@ import ( "ragflow/internal/common" "ragflow/internal/dao" "ragflow/internal/entity" + "ragflow/internal/permission" + permissionresponse "ragflow/internal/permission/response" "ragflow/internal/service" enginetypes "ragflow/internal/engine/types" @@ -39,8 +41,9 @@ func (d *DatasetService) CheckEmbedding(ctx context.Context, userID, datasetID s if datasetID == "" { return nil, common.CodeDataError, errors.New(`lack of "Dataset ID"`) } - if !d.kbDAO.Accessible(ctx, dao.DB, datasetID, userID) { - return nil, common.CodeDataError, errors.New("no authorization") + if err := service.CheckDatasetAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationUse); err != nil { + code, permissionErr := permissionresponse.Normalize(err) + return nil, code, permissionErr } kb, err := d.kbDAO.GetByID(ctx, dao.DB, datasetID) @@ -278,8 +281,8 @@ func (d *DatasetService) sampleRandomChunksWithVectors(ctx context.Context, tena return samples, nil } -func (d *DatasetService) verifyEmbeddingAvailability(ctx context.Context, embdID string, tenantID string) (bool, string) { - _, err := service.NewModelFactory().ResolveInfo(ctx, service.ModelAccess{TenantID: tenantID}, entity.ModelTypeEmbedding, embdID) +func (d *DatasetService) verifyEmbeddingAvailability(ctx context.Context, embdID string, access service.ModelAccess) (bool, string) { + _, err := service.NewModelFactory().ResolveInfo(ctx, access, entity.ModelTypeEmbedding, embdID) if err != nil { return false, err.Error() } diff --git a/internal/service/dataset/ingestion.go b/internal/service/dataset/ingestion.go index 16e97ce661..9b18e9a738 100644 --- a/internal/service/dataset/ingestion.go +++ b/internal/service/dataset/ingestion.go @@ -8,6 +8,8 @@ import ( "ragflow/internal/common" "ragflow/internal/dao" "ragflow/internal/entity" + "ragflow/internal/permission" + permissionresponse "ragflow/internal/permission/response" "ragflow/internal/service" syncerconnector "ragflow/internal/syncer/connector" @@ -46,8 +48,9 @@ func (d *DatasetService) GetIngestionSummary(ctx context.Context, datasetID, use if datasetID == "" { return nil, common.CodeDataError, errors.New(`lack of "Dataset ID"`) } - if !d.kbDAO.Accessible(ctx, dao.DB, datasetID, userID) { - return nil, common.CodeDataError, errors.New("no authorization") + if err := d.CheckAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationRead); err != nil { + code, permissionErr := permissionresponse.Normalize(err) + return nil, code, permissionErr } kb, err := d.kbDAO.GetByID(ctx, dao.DB, datasetID) @@ -168,8 +171,9 @@ func (d *DatasetService) ListIngestionMessages(ctx context.Context, datasetID, u if limit < 0 || limit > maxIngestionMessagesLimit { return nil, common.CodeArgumentError, fmt.Errorf("limit must be between 1 and %d", maxIngestionMessagesLimit) } - if !d.kbDAO.Accessible(ctx, dao.DB, datasetID, userID) { - return nil, common.CodeDataError, errors.New("no authorization") + if err := d.CheckAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationRead); err != nil { + code, permissionErr := permissionresponse.Normalize(err) + return nil, code, permissionErr } run, err := d.pipelineLogDAO.GetByIDAndKBID(ctx, dao.DB, logID, datasetID) @@ -228,8 +232,9 @@ func (d *DatasetService) ListIngestionLogs(ctx context.Context, datasetID, userI if datasetID == "" { return nil, common.CodeDataError, errors.New(`lack of "Dataset ID"`) } - if !d.kbDAO.Accessible(ctx, dao.DB, datasetID, userID) { - return nil, common.CodeDataError, errors.New("no authorization") + if err := d.CheckAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationRead); err != nil { + code, permissionErr := permissionresponse.Normalize(err) + return nil, code, permissionErr } if page <= 0 { @@ -289,8 +294,9 @@ func (d *DatasetService) GetIngestionLog(ctx context.Context, datasetID, userID, if logID == "" { return nil, common.CodeDataError, errors.New(`lack of "Log ID"`) } - if !d.kbDAO.Accessible(ctx, dao.DB, datasetID, userID) { - return nil, common.CodeDataError, errors.New("no authorization") + if err := d.CheckAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationRead); err != nil { + code, permissionErr := permissionresponse.Normalize(err) + return nil, code, permissionErr } log, err := d.pipelineLogDAO.GetByIDAndKBID(ctx, dao.DB, logID, datasetID) diff --git a/internal/service/dataset/list_test.go b/internal/service/dataset/list_test.go index 00e78ce726..869de48dd5 100644 --- a/internal/service/dataset/list_test.go +++ b/internal/service/dataset/list_test.go @@ -1,6 +1,7 @@ package dataset import ( + "encoding/json" "math" "testing" @@ -80,6 +81,75 @@ func TestDatasetServiceListDatasetsIDsAccessibleViaTeamTenant(t *testing.T) { } } +func TestDatasetServiceListDatasetsAndFiltersUseAuthorizedScope(t *testing.T) { + db := setupDatasetUpdateTestDB(t) + pushServiceDB(t, db) + insertDatasetUpdateKB(t, "kb-own", "user-1", "Own") + insertDatasetUpdateKB(t, "kb-team", "owner-1", "Team") + insertDatasetUpdateKB(t, "kb-private", "owner-1", "Private") + insertDatasetUpdateTeamMember(t, "user-1", "owner-1") + if err := dao.DB.Model(&entity.Knowledgebase{}). + Where("id = ?", "kb-team"). + Update("permission", string(entity.TenantPermissionTeam)).Error; err != nil { + t.Fatalf("share team dataset: %v", err) + } + + ctx := t.Context() + data, total, code, err := testDatasetListService(t).ListDatasets(ctx, + "", "", 1, 30, []dao.OrderTerm{{Column: "create_time", Desc: true}}, + "", []string{"owner-1"}, "", "user-1", nil, + ) + if err != nil || code != common.CodeSuccess { + t.Fatalf("ListDatasets = (%#v, %d, %v), want success", data, code, err) + } + if total != 1 || len(data) != 1 || data[0]["id"] != "kb-team" { + t.Fatalf("ListDatasets returned %#v (total=%d), want only the shared dataset", data, total) + } + + filters, code, err := testDatasetListService(t).ListDatasetFilters(ctx, "user-1") + if err != nil || code != common.CodeSuccess { + t.Fatalf("ListDatasetFilters = (%#v, %v, %v), want success", filters, code, err) + } + if filters["total"] != int64(2) { + t.Fatalf("ListDatasetFilters total = %#v, want 2 visible datasets", filters["total"]) + } + owners, ok := filters["filter"].(map[string]interface{})["owner"].([]*entity.DatasetOwnerFilter) + if !ok || len(owners) != 2 { + t.Fatalf("ListDatasetFilters owners = %#v, want own and shared tenant groups", filters["filter"]) + } +} + +func TestDatasetServiceListDatasetFiltersReturnsEmptyOwnerList(t *testing.T) { + db := setupDatasetUpdateTestDB(t) + pushServiceDB(t, db) + + filters, code, err := testDatasetListService(t).ListDatasetFilters(t.Context(), "user-without-datasets") + if err != nil || code != common.CodeSuccess { + t.Fatalf("ListDatasetFilters = (%#v, %v, %v), want success", filters, code, err) + } + + owners, ok := filters["filter"].(map[string]interface{})["owner"].([]*entity.DatasetOwnerFilter) + if !ok || owners == nil || len(owners) != 0 { + t.Fatalf("ListDatasetFilters owners = %#v, want a non-nil empty slice", filters["filter"]) + } + + encoded, err := json.Marshal(filters) + if err != nil { + t.Fatalf("marshal filters: %v", err) + } + var response map[string]interface{} + if err := json.Unmarshal(encoded, &response); err != nil { + t.Fatalf("unmarshal filters: %v", err) + } + ownerJSON := response["filter"].(map[string]interface{})["owner"] + if ownerJSON == nil { + t.Fatal(`serialized owner is null; want []`) + } + if ownerList, ok := ownerJSON.([]interface{}); !ok || len(ownerList) != 0 { + t.Fatalf("serialized owner = %#v, want []", ownerJSON) + } +} + func TestDatasetServiceListDatasetsRejectsIDAndIDsTogether(t *testing.T) { db := setupDatasetUpdateTestDB(t) pushServiceDB(t, db) diff --git a/internal/service/dataset/metadata.go b/internal/service/dataset/metadata.go index b7854e7247..e6d2e9bbf9 100644 --- a/internal/service/dataset/metadata.go +++ b/internal/service/dataset/metadata.go @@ -9,6 +9,8 @@ import ( "ragflow/internal/common" "ragflow/internal/dao" "ragflow/internal/entity" + "ragflow/internal/permission" + permissionresponse "ragflow/internal/permission/response" "ragflow/internal/service" ) @@ -16,8 +18,9 @@ import ( func (d *DatasetService) UpdateDocumentMetadataConfig(ctx context.Context, userID, datasetID, documentID string, req map[string]interface{}) (*entity.Document, common.ErrorCode, error) { userID = strings.TrimSpace(userID) datasetID = strings.TrimSpace(datasetID) - if !d.Accessible(ctx, datasetID, userID) { - return nil, common.CodeDataError, errors.New("you don't own the dataset") + if err := service.CheckDatasetAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationUpdate); err != nil { + code, permissionErr := permissionresponse.Normalize(err) + return nil, code, permissionErr } doc, err := d.documentDAO.GetByDocumentIDAndDatasetID(ctx, dao.DB, documentID, datasetID) @@ -67,22 +70,25 @@ func (d *DatasetService) UpdateDocumentMetadataConfig(ctx context.Context, userI } // GetMetadataConfig gets the auto-metadata configuration for a dataset. -func (d *DatasetService) GetMetadataConfig(ctx context.Context, datasetID, tenantID string) (map[string]interface{}, common.ErrorCode, error) { +func (d *DatasetService) GetMetadataConfig(ctx context.Context, datasetID, userID string) (map[string]interface{}, common.ErrorCode, error) { datasetID = strings.TrimSpace(datasetID) - tenantID = strings.TrimSpace(tenantID) - if !d.Accessible(ctx, datasetID, tenantID) { - return nil, common.CodeDataError, fmt.Errorf("user '%s' lacks permission for dataset '%s'", tenantID, datasetID) + userID = strings.TrimSpace(userID) + if err := d.CheckAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationRead); err != nil { + code, permissionErr := permissionresponse.Normalize(err) + return nil, code, permissionErr } kb, err := d.kbDAO.GetByID(ctx, dao.DB, datasetID) if err != nil { if dao.IsNotFoundErr(err) { - return nil, common.CodeDataError, errors.New("dataset not found") + code, permissionErr := permissionresponse.Normalize(permission.ErrResourceNotFound) + return nil, code, permissionErr } return nil, common.CodeServerError, errors.New("database operation failed") } if kb == nil { - return nil, common.CodeDataError, fmt.Errorf("user '%s' lacks permission for dataset '%s'", tenantID, datasetID) + code, permissionErr := permissionresponse.Normalize(permission.ErrResourceNotFound) + return nil, code, permissionErr } _, enabled, metadata, builtInMetadata := modularMetadataConfig(kb.ParserConfig) @@ -95,23 +101,26 @@ func (d *DatasetService) GetMetadataConfig(ctx context.Context, datasetID, tenan } // UpdateMetadataConfig updates the auto-metadata configuration for a dataset. -func (d *DatasetService) UpdateMetadataConfig(ctx context.Context, datasetID, tenantID string, req *service.MetadataConfigRequest) (map[string]interface{}, common.ErrorCode, error) { +func (d *DatasetService) UpdateMetadataConfig(ctx context.Context, datasetID, userID string, req *service.MetadataConfigRequest) (map[string]interface{}, common.ErrorCode, error) { datasetID = strings.TrimSpace(datasetID) - tenantID = strings.TrimSpace(tenantID) + userID = strings.TrimSpace(userID) - if !d.Accessible(ctx, datasetID, tenantID) { - return nil, common.CodeDataError, fmt.Errorf("user '%s' lacks permission for dataset '%s'", tenantID, datasetID) + if err := service.CheckDatasetAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationUpdate); err != nil { + code, permissionErr := permissionresponse.Normalize(err) + return nil, code, permissionErr } kb, err := d.kbDAO.GetByID(ctx, dao.DB, datasetID) if err != nil { if dao.IsNotFoundErr(err) { - return nil, common.CodeDataError, errors.New("dataset not found") + code, permissionErr := permissionresponse.Normalize(permission.ErrResourceNotFound) + return nil, code, permissionErr } return nil, common.CodeServerError, errors.New("database operation failed") } if kb == nil { - return nil, common.CodeDataError, fmt.Errorf("user '%s' lacks permission for dataset '%s'", tenantID, datasetID) + code, permissionErr := permissionresponse.Normalize(permission.ErrResourceNotFound) + return nil, code, permissionErr } if req == nil { diff --git a/internal/service/dataset/metadata_config_test.go b/internal/service/dataset/metadata_config_test.go index afb97bfae6..f186bca133 100644 --- a/internal/service/dataset/metadata_config_test.go +++ b/internal/service/dataset/metadata_config_test.go @@ -65,6 +65,7 @@ func insertDatasetMetadataConfigKB(t *testing.T, datasetID, tenantID string) { if err := dao.DB.Create(kb).Error; err != nil { t.Fatalf("insert test kb: %v", err) } + ensureDatasetTestMembership(t, tenantID, tenantID, "owner") } func insertDatasetMetadataConfigTeamMember(t *testing.T, userID, tenantID string) { @@ -203,10 +204,10 @@ func TestDatasetServiceUpdateDocumentMetadataConfigRejectsNonOwner(t *testing.T) if err == nil { t.Fatal("expected ownership error") } - if code != common.CodeDataError { - t.Fatalf("expected data error code, got %d", code) + if code != common.CodeForbidden { + t.Fatalf("expected forbidden code, got %d", code) } - if err.Error() != "you don't own the dataset" { + if err.Error() != "Permission denied" { t.Fatalf("unexpected error: %v", err) } } diff --git a/internal/service/dataset/parse_type_required_test.go b/internal/service/dataset/parse_type_required_test.go index d64b689ab4..d0545de845 100644 --- a/internal/service/dataset/parse_type_required_test.go +++ b/internal/service/dataset/parse_type_required_test.go @@ -20,7 +20,7 @@ func TestCreateDataset_RequiresParseType(t *testing.T) { // defaulted to BuiltIn; now this must be rejected. _, code, err := testDatasetCreateService(t).CreateDataset(t.Context(), &service.CreateDatasetRequest{ Name: "ds-no-parse-type", - }, "tenant-1") + }, testDatasetSubject("tenant-1")) if err == nil { t.Fatal("expected parse_type required error") } @@ -41,7 +41,7 @@ func TestCreateDataset_BuiltinRequiresParserID(t *testing.T) { _, code, err := testDatasetCreateService(t).CreateDataset(t.Context(), &service.CreateDatasetRequest{ Name: "ds-builtin-no-parser", ParseType: &parseType, - }, "tenant-1") + }, testDatasetSubject("tenant-1")) if err == nil { t.Fatal("expected parser_id required error for BuiltIn mode") } @@ -62,7 +62,7 @@ func TestCreateDataset_PipelineRequiresPipelineID(t *testing.T) { _, code, err := testDatasetCreateService(t).CreateDataset(t.Context(), &service.CreateDatasetRequest{ Name: "ds-pipeline-no-id", ParseType: &parseType, - }, "tenant-1") + }, testDatasetSubject("tenant-1")) if err == nil { t.Fatal("expected pipeline_id required error for Pipeline mode") } @@ -83,7 +83,7 @@ func TestCreateDataset_RejectsInvalidParseType(t *testing.T) { _, code, err := testDatasetCreateService(t).CreateDataset(t.Context(), &service.CreateDatasetRequest{ Name: "ds-bad-parse-type", ParseType: &parseType, - }, "tenant-1") + }, testDatasetSubject("tenant-1")) if err == nil { t.Fatal("expected invalid parse_type error") } @@ -106,7 +106,7 @@ func TestCreateDataset_BuiltinSucceedsWithParseType(t *testing.T) { Name: "ds-builtin-ok", ParseType: &parseType, ParserID: &parserID, - }, "tenant-1") + }, testDatasetSubject("tenant-1")) if err != nil { t.Fatalf("CreateDataset failed: %v", err) } diff --git a/internal/service/dataset/permission.go b/internal/service/dataset/permission.go index ec5cce6d97..48ab0e9774 100644 --- a/internal/service/dataset/permission.go +++ b/internal/service/dataset/permission.go @@ -3,20 +3,17 @@ package dataset import ( "context" "errors" - "ragflow/internal/dao" "strings" + "ragflow/internal/dao" "ragflow/internal/entity" + "ragflow/internal/permission" + "ragflow/internal/service" ) -// Accessible checks if a user has access to a dataset. -func (d *DatasetService) Accessible(ctx context.Context, kbID, userID string) bool { - return d.kbDAO.Accessible(ctx, dao.DB, kbID, userID) -} - -// GetByID retrieves a knowledge base by ID. -func (d *DatasetService) GetByID(ctx context.Context, kbID string) (*entity.Knowledgebase, error) { - return d.kbDAO.GetByID(ctx, dao.DB, kbID) +// CheckAccess authorizes an operation on a dataset under the active edition's policy. +func (d *DatasetService) CheckAccess(ctx context.Context, subject permission.Subject, datasetID string, operation permission.Operation) error { + return service.CheckDatasetAccess(ctx, subject, datasetID, operation) } // GetKnowledgebaseByID resolves a dataset entity without applying permission @@ -33,29 +30,6 @@ func (d *DatasetService) GetKnowledgebaseByID(ctx context.Context, datasetID str return d.kbDAO.GetByID(ctx, dao.DB, normalizedID) } -// CheckKBTeamPermission checks if a user has team-level permission for the KB. -func (d *DatasetService) CheckKBTeamPermission(ctx context.Context, kb *entity.Knowledgebase, userID string) bool { - if kb == nil { - return false - } - if kb.TenantID == userID { - return true - } - if kb.Permission != string(entity.TenantPermissionTeam) { - return false - } - joinedTenants, err := d.tenantDAO.GetJoinedTenantsByUserID(ctx, dao.DB, userID) - if err != nil { - return false - } - for _, jt := range joinedTenants { - if jt != nil && jt.TenantID == kb.TenantID { - return true - } - } - return false -} - // GetFieldMap returns the field map for the given knowledge base IDs. func (d *DatasetService) GetFieldMap(ctx context.Context, ids []string) (map[string]interface{}, error) { return d.kbDAO.GetFieldMap(ctx, dao.DB, ids) diff --git a/internal/service/dataset/search.go b/internal/service/dataset/search.go index 255ce32ecc..ef2b55f7fb 100644 --- a/internal/service/dataset/search.go +++ b/internal/service/dataset/search.go @@ -10,6 +10,8 @@ import ( "ragflow/internal/common" "ragflow/internal/entity" modelModule "ragflow/internal/entity/models" + "ragflow/internal/permission" + permissionresponse "ragflow/internal/permission/response" "ragflow/internal/service" "ragflow/internal/service/nlp" ) @@ -112,9 +114,10 @@ func (d *DatasetService) SearchDatasets(ctx context.Context, req *service.Search var kbRecords []*entity.Knowledgebase seenTenants := make(map[string]bool) for _, datasetID := range datasetIDs { - if !d.kbDAO.Accessible(ctx, dao.DB, datasetID, userID) { + if err := service.CheckDatasetAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationUse); err != nil { common.Warn("SearchDatasets access denied", zap.String("datasetID", datasetID), zap.String("userID", userID)) - return nil, fmt.Errorf("only owner of dataset %s is authorized for this operation", datasetID) + _, permissionErr := permissionresponse.Normalize(err) + return nil, permissionErr } kb, err := d.kbDAO.GetByID(ctx, dao.DB, datasetID) diff --git a/internal/service/dataset/setup_test.go b/internal/service/dataset/setup_test.go index b5a5da9d39..abd6f82f86 100644 --- a/internal/service/dataset/setup_test.go +++ b/internal/service/dataset/setup_test.go @@ -60,4 +60,27 @@ func pushServiceDB(t *testing.T, testDB *gorm.DB) { t.Cleanup(func() { dao.DB = oldDB }) } +func ensureDatasetTestMembership(t *testing.T, userID, tenantID, role string) { + t.Helper() + var count int64 + if err := dao.DB.Model(&entity.UserTenant{}). + Where("user_id = ? AND tenant_id = ? AND status = ?", userID, tenantID, string(entity.StatusValid)). + Count(&count).Error; err != nil { + t.Fatalf("count test tenant memberships: %v", err) + } + if count > 0 { + return + } + if err := dao.DB.Create(&entity.UserTenant{ + ID: userID + "-" + tenantID, + UserID: userID, + TenantID: tenantID, + Role: role, + InvitedBy: tenantID, + Status: sptr(string(entity.StatusValid)), + }).Error; err != nil { + t.Fatalf("insert test tenant membership: %v", err) + } +} + func sptr(s string) *string { return &s } diff --git a/internal/service/dataset/tags.go b/internal/service/dataset/tags.go index b1b8cb6a68..a61f89daa2 100644 --- a/internal/service/dataset/tags.go +++ b/internal/service/dataset/tags.go @@ -10,6 +10,8 @@ import ( "ragflow/internal/common" "ragflow/internal/dao" "ragflow/internal/ingestion/component" + "ragflow/internal/permission" + permissionresponse "ragflow/internal/permission/response" ) func (d *DatasetService) AggregateTags(ctx context.Context, datasetIDs []string, userID string) ([]map[string]interface{}, common.ErrorCode, error) { @@ -66,8 +68,9 @@ func (d *DatasetService) AggregateTags(ctx context.Context, datasetIDs []string, } authorized := make([]authorizedDataset, 0, len(orderedIDs)) for _, datasetID := range orderedIDs { - if !d.kbDAO.Accessible(ctx, dao.DB, datasetID, userID) { - return nil, common.CodeDataError, fmt.Errorf("No authorization for dataset '%s'", datasetID) + if err := d.CheckAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationRead); err != nil { + code, permissionErr := permissionresponse.Normalize(err) + return nil, code, permissionErr } kb, err := d.kbDAO.GetByID(ctx, dao.DB, datasetID) if err != nil { diff --git a/internal/service/dataset/tags_aggregate_test.go b/internal/service/dataset/tags_aggregate_test.go index 2a640ce348..00925e4fd0 100644 --- a/internal/service/dataset/tags_aggregate_test.go +++ b/internal/service/dataset/tags_aggregate_test.go @@ -65,6 +65,7 @@ func insertAggregateTagsKB(t *testing.T, datasetID, tenantID, permission, tagFil if err := dao.DB.Create(kb).Error; err != nil { t.Fatalf("insert test kb: %v", err) } + ensureDatasetTestMembership(t, tenantID, tenantID, "owner") } func insertAggregateTagsMembership(t *testing.T, tenantID, userID string) { @@ -73,7 +74,7 @@ func insertAggregateTagsMembership(t *testing.T, tenantID, userID string) { ID: tenantID + "-" + userID, UserID: userID, TenantID: tenantID, - Role: "member", + Role: "normal", InvitedBy: tenantID, Status: sptr(string(entity.StatusValid)), } @@ -222,10 +223,10 @@ func TestDatasetServiceAggregateTagsRejectsUnauthorizedDataset(t *testing.T) { if err == nil { t.Fatal("expected authorization error") } - if code != common.CodeDataError { - t.Fatalf("code=%d want=%d", code, common.CodeDataError) + if code != common.CodeForbidden { + t.Fatalf("code=%d want=%d", code, common.CodeForbidden) } - if err.Error() != "No authorization for dataset '"+kbID+"'" { + if err.Error() != "Permission denied" { t.Fatalf("unexpected error: %v", err) } } diff --git a/internal/service/dataset/update.go b/internal/service/dataset/update.go index 1a3c925ffd..c284bcd810 100644 --- a/internal/service/dataset/update.go +++ b/internal/service/dataset/update.go @@ -13,6 +13,8 @@ import ( "ragflow/internal/engine/types" "ragflow/internal/entity" pipelinepkg "ragflow/internal/ingestion/pipeline" + "ragflow/internal/permission" + permissionresponse "ragflow/internal/permission/response" "ragflow/internal/service" "go.uber.org/zap" @@ -28,14 +30,13 @@ type datasetPagerankUpdate struct { datasetID string } -func (d *DatasetService) UpdateDataset(ctx context.Context, datasetID, tenantID string, req service.UpdateDatasetRequest) (map[string]interface{}, common.ErrorCode, error) { +func (d *DatasetService) UpdateDataset(ctx context.Context, datasetID, userID string, req service.UpdateDatasetRequest) (map[string]interface{}, common.ErrorCode, error) { datasetID = strings.TrimSpace(datasetID) - tenantID = strings.TrimSpace(tenantID) + userID = strings.TrimSpace(userID) if _, err := d.kbDAO.GetByID(ctx, dao.DB, datasetID); err != nil { if dao.IsNotFoundErr(err) { - // Match Python: nonexistent and not-owned datasets share the - // "lacks permission" error so existence is not revealed (IDOR). - return nil, common.CodeDataError, fmt.Errorf("user '%s' lacks permission for dataset '%s'", tenantID, datasetID) + code, permissionErr := permissionresponse.NormalizeHidden(permission.ErrResourceNotFound) + return nil, code, permissionErr } return nil, common.CodeServerError, errors.New("database operation failed") } @@ -144,7 +145,7 @@ func (d *DatasetService) UpdateDataset(ctx context.Context, datasetID, tenantID common.Warn("dropping unscoped (flat) parser_config keys; keys must be component-scoped (contain ':')", zap.Strings("keys", dropped), zap.String("dataset_id", datasetID), - zap.String("tenant_id", tenantID), + zap.String("user_id", userID), ) } if err != nil { @@ -179,26 +180,38 @@ func (d *DatasetService) UpdateDataset(ctx context.Context, datasetID, tenantID var scheduledTaskIDs []string var pagerankUpdate *datasetPagerankUpdate err = dao.DB.Transaction(func(tx *gorm.DB) error { - lockedKB, code, authErr := d.lockAccessibleDatasetForUpdate(tx, datasetID, tenantID) + lockedKB, code, authErr := d.lockAccessibleDatasetForUpdate(ctx, tx, datasetID, userID) if authErr != nil { txCode = code return authErr } - if req.Permission != nil && lockedKB.TenantID != tenantID { - txCode = common.CodeDataError - return errors.New("only dataset owner can change permission") - } - updates := make(map[string]interface{}, len(simpleUpdates)+6) for key, value := range simpleUpdates { updates[key] = value } + if permissionValue, ok := updates["permission"].(string); ok { + if permissionValue != lockedKB.Permission { + accessErr := permission.NewLockingDatabaseChecker(tx).CheckResource( + ctx, + permission.Subject{UserID: userID}, + permission.ResourceRef{Kind: permission.ResourceKindDataset, ID: datasetID}, + permission.OperationShare, + ) + if accessErr != nil { + txCode, accessErr = permissionresponse.Normalize(accessErr) + return accessErr + } + } + if permissionValue == lockedKB.Permission { + delete(updates, "permission") + } + } if nameValue, ok := updates["name"].(string); ok { available, nameErr := common.NameAvailable(lockedKB.Name, nameValue, func(candidate string) (bool, error) { var existing entity.Knowledgebase - lookupErr := tx.Where("LOWER(name) = LOWER(?) AND tenant_id = ? AND status = ?", candidate, tenantID, string(entity.StatusValid)).First(&existing).Error + lookupErr := tx.Where("LOWER(name) = LOWER(?) AND tenant_id = ? AND status = ?", candidate, lockedKB.TenantID, string(entity.StatusValid)).First(&existing).Error if lookupErr != nil { if dao.IsNotFoundErr(lookupErr) { return false, nil @@ -231,13 +244,14 @@ func (d *DatasetService) UpdateDataset(ctx context.Context, datasetID, tenantID } else { tenantEmbdID = "" } - ok, message := d.verifyEmbeddingAvailability(ctx, effectiveEmbdID, tenantID) + modelAccess := service.ModelAccess{UserID: userID, TenantID: lockedKB.TenantID} + ok, message := d.verifyEmbeddingAvailability(ctx, effectiveEmbdID, modelAccess) if !ok { txCode = common.CodeDataError return errors.New(message) } if effectiveEmbdID != "" && tenantEmbdID == "" { - target, err := service.NewModelFactory().ResolveInfo(ctx, service.ModelAccess{TenantID: tenantID}, entity.ModelTypeEmbedding, effectiveEmbdID) + target, err := service.NewModelFactory().ResolveInfo(ctx, modelAccess, entity.ModelTypeEmbedding, effectiveEmbdID) if err == nil { tenantEmbdID = target.ID } @@ -426,34 +440,28 @@ func publishDatasetSyncerTasks(taskIDs []string) { } } -func (d *DatasetService) lockAccessibleDatasetForUpdate(tx *gorm.DB, datasetID, userID string) (*entity.Knowledgebase, common.ErrorCode, error) { +func (d *DatasetService) lockAccessibleDatasetForUpdate(ctx context.Context, tx *gorm.DB, datasetID, userID string) (*entity.Knowledgebase, common.ErrorCode, error) { var kb entity.Knowledgebase err := tx.Clauses(clause.Locking{Strength: "UPDATE"}). Where("id = ? AND status = ?", datasetID, string(entity.StatusValid)). First(&kb).Error if err != nil { if dao.IsNotFoundErr(err) { - return nil, common.CodeDataError, errors.New("dataset not found") + code, permissionErr := permissionresponse.NormalizeHidden(permission.ErrResourceNotFound) + return nil, code, permissionErr } return nil, common.CodeServerError, errors.New("database operation failed") } - if kb.TenantID == userID { - return &kb, common.CodeSuccess, nil - } - if kb.Permission != string(entity.TenantPermissionTeam) { - return nil, common.CodeDataError, fmt.Errorf("user '%s' lacks permission for dataset '%s'", userID, datasetID) - } - - var relation entity.UserTenant - err = tx.Clauses(clause.Locking{Strength: "UPDATE"}). - Where("tenant_id = ? AND user_id = ? AND status = ?", kb.TenantID, userID, "1"). - First(&relation).Error + err = permission.NewLockingDatabaseChecker(tx).CheckResource( + ctx, + permission.Subject{UserID: userID}, + permission.ResourceRef{Kind: permission.ResourceKindDataset, ID: datasetID}, + permission.OperationUpdate, + ) if err != nil { - if dao.IsNotFoundErr(err) { - return nil, common.CodeDataError, fmt.Errorf("user '%s' lacks permission for dataset '%s'", userID, datasetID) - } - return nil, common.CodeServerError, errors.New("database operation failed") + code, permissionErr := permissionresponse.NormalizeHidden(err) + return nil, code, permissionErr } return &kb, common.CodeSuccess, nil diff --git a/internal/service/dataset/update_test.go b/internal/service/dataset/update_test.go index 8bd5e11a2d..eb108e07b1 100644 --- a/internal/service/dataset/update_test.go +++ b/internal/service/dataset/update_test.go @@ -25,6 +25,7 @@ import ( "ragflow/internal/common" "ragflow/internal/dao" "ragflow/internal/entity" + "ragflow/internal/permission" "ragflow/internal/service" "gorm.io/gorm" @@ -440,13 +441,10 @@ func TestDatasetServiceUpdateDatasetRejectsMissingDataset(t *testing.T) { if err == nil { t.Fatal("expected missing dataset error") } - if code != common.CodeDataError { - t.Fatalf("expected data error code, got %d", code) + if code != common.CodeNotFound { + t.Fatalf("expected not found code, got %d", code) } - // Nonexistent and not-owned datasets share the "lacks permission" - // error so existence is not revealed (IDOR), matching Python. - expected := "user 'tenant-1' lacks permission for dataset 'missing-kb'" - if err.Error() != expected { + if err.Error() != "Resource not found" { t.Fatalf("unexpected error: %v", err) } } @@ -462,10 +460,10 @@ func TestDatasetServiceUpdateDatasetRejectsNonOwner(t *testing.T) { if err == nil { t.Fatal("expected permission error") } - if code != common.CodeDataError { - t.Fatalf("expected data error code, got %d", code) + if code != common.CodeNotFound { + t.Fatalf("expected not found code, got %d", code) } - if !strings.Contains(err.Error(), "lacks permission") { + if err.Error() != "Resource not found" { t.Fatalf("unexpected error: %v", err) } } @@ -489,10 +487,10 @@ func TestDatasetServiceUpdateDatasetRejectsTeamMemberPermissionChange(t *testing if err == nil { t.Fatal("expected permission change error") } - if code != common.CodeDataError { - t.Fatalf("expected data error code, got %d", code) + if code != common.CodeForbidden { + t.Fatalf("expected forbidden code, got %d", code) } - if err.Error() != "only dataset owner can change permission" { + if err.Error() != "Permission denied" { t.Fatalf("unexpected error: %v", err) } @@ -505,6 +503,81 @@ func TestDatasetServiceUpdateDatasetRejectsTeamMemberPermissionChange(t *testing } } +func TestDatasetServiceUpdateDatasetAllowsTeamMemberToResubmitUnchangedPermission(t *testing.T) { + db := setupDatasetUpdateTestDB(t) + pushServiceDB(t, db) + insertDatasetUpdateKB(t, "kb-1", "owner-1", "Original") + if err := dao.DB.Model(&entity.Knowledgebase{}). + Where("id = ?", "kb-1"). + Update("permission", string(entity.TenantPermissionTeam)).Error; err != nil { + t.Fatalf("update kb permission: %v", err) + } + insertDatasetUpdateTeamMember(t, "user-1", "owner-1") + + name := "Renamed by team member" + permission := string(entity.TenantPermissionTeam) + result, code, err := testDatasetUpdateService(t).UpdateDataset(t.Context(), "kb-1", "user-1", service.UpdateDatasetRequest{ + Name: &name, + Permission: &permission, + }) + if err != nil { + t.Fatalf("expected unchanged permission to be accepted, got: %v", err) + } + if code != common.CodeSuccess { + t.Fatalf("expected success code, got %d", code) + } + if result["name"] != name { + t.Fatalf("expected updated name %q, got %#v", name, result["name"]) + } + + persisted, err := dao.NewKnowledgebaseDAO().GetByID(t.Context(), db, "kb-1") + if err != nil { + t.Fatalf("get dataset: %v", err) + } + if persisted.Name != name { + t.Fatalf("expected persisted name %q, got %q", name, persisted.Name) + } + if persisted.Permission != permission { + t.Fatalf("expected permission to remain %q, got %q", permission, persisted.Permission) + } +} + +func TestDatasetServiceUpdateDatasetResolvesEmbeddingInDatasetTenant(t *testing.T) { + db := setupDatasetUpdateTestDB(t) + pushServiceDB(t, db) + insertDatasetUpdateKB(t, "kb-1", "tenant-1", "Original") + if err := dao.DB.Model(&entity.Knowledgebase{}). + Where("id = ?", "kb-1"). + Update("permission", string(entity.TenantPermissionTeam)).Error; err != nil { + t.Fatalf("update kb permission: %v", err) + } + insertDatasetUpdateTeamMember(t, "user-1", "tenant-1") + insertDatasetUpdateModelProvider(t, "provider-1", "tenant-1", "ZHIPU-AI") + insertDatasetUpdateModelInstance(t, "instance-1", "provider-1", "test") + embeddingModelID := "aabbccdd11223344aabbccdd11223344" + insertDatasetUpdateTenantModel(t, embeddingModelID, "provider-1", "instance-1", "embedding-2", int(entity.ModelTypeEmbedding)) + + permission := string(entity.TenantPermissionTeam) + _, code, err := testDatasetUpdateService(t).UpdateDataset(t.Context(), "kb-1", "user-1", service.UpdateDatasetRequest{ + Permission: &permission, + EmbeddingModel: &embeddingModelID, + }) + if err != nil { + t.Fatalf("expected shared dataset embedding model to resolve in its owning tenant, got: %v", err) + } + if code != common.CodeSuccess { + t.Fatalf("expected success code, got %d", code) + } + + persisted, err := dao.NewKnowledgebaseDAO().GetByID(t.Context(), db, "kb-1") + if err != nil { + t.Fatalf("get dataset: %v", err) + } + if persisted.TenantEmbdID == nil || *persisted.TenantEmbdID != embeddingModelID { + t.Fatalf("expected tenant_embd_id %q, got %#v", embeddingModelID, persisted.TenantEmbdID) + } +} + func TestDatasetServiceUpdateDatasetValidatesName(t *testing.T) { db := setupDatasetUpdateTestDB(t) pushServiceDB(t, db) @@ -868,14 +941,14 @@ func TestDatasetServiceDeleteDatasetsRejectsUnauthorizedID(t *testing.T) { ctx := t.Context() svc := NewDatasetService() normalizedID := "11111111111141118111111111111111" - _, code, err := svc.DeleteDatasets(ctx, []string{normalizedID}, false, "tenant-2") + _, code, err := svc.DeleteDatasets(ctx, []string{normalizedID}, false, permission.Subject{UserID: "tenant-2"}) if err == nil { t.Fatal("expected unauthorized error") } - if code != common.CodeDataError { - t.Fatalf("expected data error code, got %d", code) + if code != common.CodeForbidden { + t.Fatalf("expected forbidden code, got %d", code) } - if !strings.Contains(err.Error(), "lacks permission") { + if err.Error() != "Permission denied" { t.Fatalf("unexpected error: %v", err) } } @@ -886,14 +959,34 @@ func TestDatasetServiceDeleteDatasetsRejectsAllUnauthorized(t *testing.T) { ctx := t.Context() svc := NewDatasetService() - _, code, err := svc.DeleteDatasets(ctx, []string{"d94a8dc02c9711f0930f7fbc369eab6d"}, false, "tenant-1") + _, code, err := svc.DeleteDatasets(ctx, []string{"d94a8dc02c9711f0930f7fbc369eab6d"}, false, testDatasetSubject("tenant-1")) if err == nil { t.Fatal("expected unauthorized error") } - if code != common.CodeDataError { - t.Fatalf("expected data error code, got %d", code) + if code != common.CodeForbidden { + t.Fatalf("expected forbidden code, got %d", code) } - if !strings.Contains(err.Error(), "lacks permission") { + if err.Error() != "Permission denied" { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestDatasetServiceDeleteAllDatasetsRequiresTenantOwner(t *testing.T) { + db := setupDatasetUpdateTestDB(t) + pushServiceDB(t, db) + insertDatasetUpdateKB(t, "kb-1", "tenant-1", "Shared dataset") + insertDatasetUpdateTeamMember(t, "member-1", "tenant-1") + + _, code, err := NewDatasetService().DeleteDatasets( + t.Context(), nil, true, permission.Subject{UserID: "member-1", TenantID: "tenant-1"}, + ) + if err == nil { + t.Fatal("expected delete-all to be denied for a non-owner tenant member") + } + if code != common.CodeForbidden { + t.Fatalf("expected forbidden code, got %d", code) + } + if err.Error() != "Permission denied" { t.Fatalf("unexpected error: %v", err) } } @@ -949,6 +1042,7 @@ func insertDatasetUpdateKB(t *testing.T, id, tenantID, name string) { if err := dao.DB.Create(kb).Error; err != nil { t.Fatalf("insert test kb: %v", err) } + ensureDatasetTestMembership(t, tenantID, tenantID, "owner") } func insertDatasetUpdateCanvas(t *testing.T, id, userID string) { @@ -965,16 +1059,7 @@ func insertDatasetUpdateCanvas(t *testing.T, id, userID string) { func insertDatasetUpdateTeamMember(t *testing.T, userID, tenantID string) { t.Helper() - if err := dao.DB.Create(&entity.UserTenant{ - ID: userID + "-" + tenantID, - UserID: userID, - TenantID: tenantID, - Role: "normal", - InvitedBy: tenantID, - Status: sptr("1"), - }).Error; err != nil { - t.Fatalf("insert user tenant: %v", err) - } + ensureDatasetTestMembership(t, userID, tenantID, "normal") } func insertDatasetUpdateConnector(t *testing.T, id, tenantID string) { @@ -1101,6 +1186,7 @@ func insertDatasetUpdateCanvasKB(t *testing.T, id, tenantID, name, pipelineID st if err := dao.DB.Create(kb).Error; err != nil { t.Fatalf("insert test canvas kb: %v", err) } + ensureDatasetTestMembership(t, tenantID, tenantID, "owner") } func TestUpdateDataset_StripsUnknownParam_Builtin(t *testing.T) { diff --git a/internal/service/dataset_permission.go b/internal/service/dataset_permission.go new file mode 100644 index 0000000000..b1f3c9edcd --- /dev/null +++ b/internal/service/dataset_permission.go @@ -0,0 +1,18 @@ +package service + +import ( + "context" + + "ragflow/internal/dao" + "ragflow/internal/permission" +) + +// CheckDatasetAccess checks a dataset operation using the active edition's permission policy. +func CheckDatasetAccess(ctx context.Context, subject permission.Subject, datasetID string, operation permission.Operation) error { + return permission.NewDatabaseChecker(dao.DB).CheckResource( + ctx, + subject, + permission.ResourceRef{Kind: permission.ResourceKindDataset, ID: datasetID}, + operation, + ) +} diff --git a/internal/service/document/document.go b/internal/service/document/document.go index a416890053..6be6e8b96d 100644 --- a/internal/service/document/document.go +++ b/internal/service/document/document.go @@ -170,6 +170,7 @@ type UpdateDatasetDocumentResponse struct { } var ( + ErrDocumentNotFound = errors.New("document not found") ErrArtifactInvalidFilename = errors.New("invalid filename") ErrArtifactInvalidFileType = errors.New("invalid file type") ErrArtifactNotFound = errors.New("artifact not found") diff --git a/internal/service/document/document_artifact.go b/internal/service/document/document_artifact.go index dc20234421..9e12286690 100644 --- a/internal/service/document/document_artifact.go +++ b/internal/service/document/document_artifact.go @@ -12,6 +12,7 @@ import ( "ragflow/internal/dao" enginetypes "ragflow/internal/engine/types" "ragflow/internal/parser/parser" + "ragflow/internal/permission" "ragflow/internal/service" "ragflow/internal/storage" "ragflow/internal/utility" @@ -40,7 +41,7 @@ func (s *DocumentService) GetDocumentImage(ctx context.Context, userID, imageID // composite image ID belongs to an indexed chunk of an accessible document. func (s *DocumentService) GetDocumentImageForDocument(ctx context.Context, userID, docID, imageID string) ([]byte, error) { doc, err := s.documentDAO.GetByID(ctx, dao.DB, docID) - if err != nil || doc == nil || !s.kbDAO.Accessible(ctx, dao.DB, doc.KbID, userID) { + if err != nil || doc == nil || service.CheckDatasetAccess(ctx, permission.Subject{UserID: userID}, doc.KbID, permission.OperationRead) != nil { return nil, ErrDocumentImageNotFound } bucket, objectKey, ok := strings.Cut(imageID, "-") @@ -68,7 +69,7 @@ func (s *DocumentService) GetDocumentImageForDocument(ctx context.Context, userI // document metadata. func (s *DocumentService) GetDocumentThumbnail(ctx context.Context, userID, docID string) ([]byte, error) { doc, err := s.documentDAO.GetByID(ctx, dao.DB, docID) - if err != nil || doc == nil || !s.kbDAO.Accessible(ctx, dao.DB, doc.KbID, userID) || doc.Thumbnail == nil || *doc.Thumbnail == "" || strings.HasPrefix(*doc.Thumbnail, imgBase64Prefix) { + if err != nil || doc == nil || service.CheckDatasetAccess(ctx, permission.Subject{UserID: userID}, doc.KbID, permission.OperationRead) != nil || doc.Thumbnail == nil || *doc.Thumbnail == "" || strings.HasPrefix(*doc.Thumbnail, imgBase64Prefix) { return nil, ErrDocumentImageNotFound } storageImpl := storage.GetStorageFactory().GetStorage() @@ -279,13 +280,9 @@ func (s *DocumentService) GetDocumentPreview(ctx context.Context, userID, docID return nil, ErrPreviewDocumentNotFound } - // Reuse KnowledgebaseDAO.Accessible — the exact rule the chunk list on - // the same page uses — so the two panels can never disagree: the owning - // tenant always, and tenant members only when the dataset's permission - // is TEAM. A denial stays indistinguishable from a missing document so - // an unauthorized caller cannot probe document IDs (mirrors Python - // DocumentService.accessible in the preview path). - if !s.kbDAO.Accessible(ctx, dao.DB, doc.KbID, userID) { + // Keep access denial indistinguishable from a missing document so callers + // cannot probe document IDs. + if service.CheckDatasetAccess(ctx, permission.Subject{UserID: userID}, doc.KbID, permission.OperationRead) != nil { return nil, ErrPreviewDocumentNotFound } diff --git a/internal/service/document/document_crud.go b/internal/service/document/document_crud.go index 5b05deb8e1..411494a8a8 100644 --- a/internal/service/document/document_crud.go +++ b/internal/service/document/document_crud.go @@ -10,28 +10,15 @@ import ( "ragflow/internal/dao" "ragflow/internal/entity" "ragflow/internal/ingestion/knowledge_compile" + "ragflow/internal/permission" + permissionresponse "ragflow/internal/permission/response" + "ragflow/internal/service" "ragflow/internal/storage" "gorm.io/gorm" "gorm.io/gorm/clause" ) -// Accessible reports whether docID belongs to a knowledge base -// reachable by userID. Used to gate actions on a document the caller -// has access to. Returns false on any lookup failure or empty inputs -// so callers can treat a denial as a 404-equivalent and avoid leaking -// whether the document exists at all. -func (s *DocumentService) Accessible(ctx context.Context, docID, userID string) bool { - if docID == "" || userID == "" { - return false - } - doc, err := s.documentDAO.GetByID(ctx, dao.DB, docID) - if err != nil || doc == nil { - return false - } - return s.kbDAO.Accessible(ctx, dao.DB, doc.KbID, userID) -} - func (s *DocumentService) GetDocumentStorageAddress(ctx context.Context, doc *entity.Document) (string, string, error) { if doc == nil { return "", "", fmt.Errorf("document is nil") @@ -70,8 +57,14 @@ func (s *DocumentService) DownloadDocument(ctx context.Context, datasetID, docID return nil, fmt.Errorf("specify document_id please") } doc, err := s.documentDAO.GetByID(ctx, dao.DB, docID) - if err != nil || doc.KbID != datasetID { - return nil, fmt.Errorf("document not found") + if err != nil { + if dao.IsNotFoundErr(err) { + return nil, ErrDocumentNotFound + } + return nil, fmt.Errorf("failed to get document %s: %w", docID, err) + } + if doc == nil || doc.KbID != datasetID { + return nil, ErrDocumentNotFound } bucket, name, err := s.GetDocumentStorageAddress(ctx, doc) if err != nil { @@ -227,8 +220,9 @@ func (s *DocumentService) DeleteDocument(ctx context.Context, id string) error { // Returns the number of successfully deleted documents. func (s *DocumentService) DeleteDocuments(ctx context.Context, ids []string, deleteAll bool, datasetID, userID string) (int, error) { // 1. Check dataset is accessible by the user - if !s.kbDAO.Accessible(ctx, dao.DB, datasetID, userID) { - return 0, fmt.Errorf("You don't own the dataset %s.", datasetID) + if err := service.CheckDatasetAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationDelete); err != nil { + _, permissionErr := permissionresponse.Normalize(err) + return 0, permissionErr } // 2. Resolve document IDs diff --git a/internal/service/document/document_dataset_update.go b/internal/service/document/document_dataset_update.go index 28518b9cf7..167aedd7f1 100644 --- a/internal/service/document/document_dataset_update.go +++ b/internal/service/document/document_dataset_update.go @@ -14,13 +14,19 @@ import ( "ragflow/internal/dao" "ragflow/internal/entity" pipelinepkg "ragflow/internal/ingestion/pipeline" + "ragflow/internal/permission" + permissionresponse "ragflow/internal/permission/response" "ragflow/internal/tokenizer" "go.uber.org/zap" ) func (s *DocumentService) BatchUpdateDocumentStatus(ctx context.Context, userID, datasetID, status string, documentIDs []string) (map[string]interface{}, common.ErrorCode, error) { - kb, err := s.kbDAO.GetByIDAndTenantID(ctx, dao.DB, datasetID, userID) + if err := service.CheckDatasetAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationUpdate); err != nil { + code, permissionErr := permissionresponse.Normalize(err) + return nil, code, permissionErr + } + kb, err := s.kbDAO.GetByID(ctx, dao.DB, datasetID) if err != nil { return nil, common.CodeDataError, fmt.Errorf("you don't own the dataset") } @@ -108,8 +114,11 @@ func (s *DocumentService) BatchUpdateDocumentStatus(ctx context.Context, userID, } func (s *DocumentService) UpdateDatasetDocument(ctx context.Context, userID, datasetID, documentID string, req *UpdateDatasetDocumentRequest, present map[string]bool) (*UpdateDatasetDocumentResponse, common.ErrorCode, error) { - tenantID := userID - kb, err := s.kbDAO.GetByIDAndTenantID(ctx, dao.DB, datasetID, tenantID) + if err := service.CheckDatasetAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationUpdate); err != nil { + code, permissionErr := permissionresponse.Normalize(err) + return nil, code, permissionErr + } + kb, err := s.kbDAO.GetByID(ctx, dao.DB, datasetID) if err != nil { if dao.IsNotFoundErr(err) { return nil, common.CodeDataError, errors.New("you don't own the dataset") diff --git a/internal/service/document/document_ingest.go b/internal/service/document/document_ingest.go index 157fd1e283..02835fedf1 100644 --- a/internal/service/document/document_ingest.go +++ b/internal/service/document/document_ingest.go @@ -9,6 +9,8 @@ import ( "ragflow/internal/common" "ragflow/internal/entity" + "ragflow/internal/permission" + permissionresponse "ragflow/internal/permission/response" ) func (s *DocumentService) ListIngestionTasks(ctx context.Context, userID string, datasetID *string, page, pageSize int) ([]*entity.IngestionTask, error) { @@ -64,8 +66,9 @@ func (s *DocumentService) Ingest(ctx context.Context, userID string, req *Ingest if err != nil { return common.CodeDataError, fmt.Errorf("dataset not found") } - if !s.kbDAO.Accessible(ctx, dao.DB, kb.ID, userID) { - return common.CodeAuthenticationError, fmt.Errorf("no authorization") + if err := service.CheckDatasetAccess(ctx, permission.Subject{UserID: userID}, kb.ID, permission.OperationRun); err != nil { + code, permissionErr := permissionresponse.Normalize(err) + return code, permissionErr } validated = append(validated, validatedDoc{doc, kb}) validatedIDs = append(validatedIDs, docID) diff --git a/internal/service/document/document_list.go b/internal/service/document/document_list.go index 78054b91ba..91af98504a 100644 --- a/internal/service/document/document_list.go +++ b/internal/service/document/document_list.go @@ -9,6 +9,7 @@ import ( "ragflow/internal/common" "ragflow/internal/dao" "ragflow/internal/entity" + "ragflow/internal/permission" "ragflow/internal/service" ) @@ -77,7 +78,7 @@ func (s *DocumentService) GetThumbnails(ctx context.Context, userID string, docI result := make(map[string]string, len(documents)) for _, document := range documents { - if document == nil || !s.kbDAO.Accessible(ctx, dao.DB, document.KbID, userID) { + if document == nil || service.CheckDatasetAccess(ctx, permission.Subject{UserID: userID}, document.KbID, permission.OperationRead) != nil { continue } diff --git a/internal/service/document/document_modify_guard_test.go b/internal/service/document/document_modify_guard_test.go index 915ce85e55..95aa509676 100644 --- a/internal/service/document/document_modify_guard_test.go +++ b/internal/service/document/document_modify_guard_test.go @@ -92,7 +92,7 @@ func updateDatasetDocumentRejected(t *testing.T, status string, req *UpdateDatas svc := testDocumentService(t) ctx := t.Context() - _, code, err := svc.UpdateDatasetDocument(ctx, "tenant-1", "kb-1", "doc-1", req, present) + _, code, err := svc.UpdateDatasetDocument(ctx, "user-1", "kb-1", "doc-1", req, present) if err == nil { t.Fatalf("expected task-state rejection, got nil error (code=%v)", code) } @@ -118,7 +118,7 @@ func updateDatasetDocumentAllowed(t *testing.T, status string, req *UpdateDatase svc := testDocumentService(t) ctx := t.Context() - _, code, err := svc.UpdateDatasetDocument(ctx, "tenant-1", "kb-1", "doc-1", req, present) + _, code, err := svc.UpdateDatasetDocument(ctx, "user-1", "kb-1", "doc-1", req, present) if err != nil && strings.Contains(err.Error(), "cannot be modified") { t.Fatalf("editable doc was wrongly rejected: %v", err) } @@ -201,7 +201,7 @@ func TestUpdateDatasetDocumentRunningEmptyPresentNotRejected(t *testing.T) { svc := testDocumentService(t) ctx := t.Context() - _, code, err := svc.UpdateDatasetDocument(ctx, "tenant-1", "kb-1", "doc-1", + _, code, err := svc.UpdateDatasetDocument(ctx, "user-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{}, map[string]bool{}) if err != nil { t.Fatalf("empty PATCH on RUNNING doc should not be rejected: code=%v err=%v", code, err) @@ -231,7 +231,7 @@ func TestUpdateDatasetDocumentAllowsDoneRename(t *testing.T) { name := "renamed.txt" svc := testDocumentService(t) ctx := t.Context() - _, code, err := svc.UpdateDatasetDocument(ctx, "tenant-1", "kb-1", "doc-1", + _, code, err := svc.UpdateDatasetDocument(ctx, "user-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{Name: &name}, map[string]bool{"name": true}) if err != nil && strings.Contains(err.Error(), "cannot be modified") { diff --git a/internal/service/document/document_test.go b/internal/service/document/document_test.go index 0895aad093..726a27b1a2 100644 --- a/internal/service/document/document_test.go +++ b/internal/service/document/document_test.go @@ -600,6 +600,7 @@ func sptr(s string) *string { return &s } func insertTestKB(t *testing.T, id, tenantID string, docNum, tokenNum, chunkNum int64) { t.Helper() + ensureTestTenantOwner(t, "user-1", tenantID) kb := &entity.Knowledgebase{ ID: id, TenantID: tenantID, @@ -617,6 +618,30 @@ func insertTestKB(t *testing.T, id, tenantID string, docNum, tokenNum, chunkNum } } +func ensureTestTenantOwner(t *testing.T, userID, tenantID string) { + t.Helper() + var owner entity.UserTenant + err := dao.DB.Where("tenant_id = ? AND role = ? AND status = ?", tenantID, "owner", string(entity.StatusValid)).First(&owner).Error + if err == nil { + return + } + if !dao.IsNotFoundErr(err) { + t.Fatalf("find test tenant owner: %v", err) + } + status := string(entity.StatusValid) + owner = entity.UserTenant{ + ID: userID + "_" + tenantID, + UserID: userID, + TenantID: tenantID, + Role: "owner", + InvitedBy: userID, + Status: &status, + } + if err := dao.DB.Create(&owner).Error; err != nil { + t.Fatalf("insert test tenant owner: %v", err) + } +} + func assertKBDocNum(t *testing.T, kbID string, want int64) { t.Helper() var got int64 @@ -1251,7 +1276,7 @@ func insertUserTenantForAccessCheck(t *testing.T, userID, tenantID string) { ID: userID + "_" + tenantID, UserID: userID, TenantID: tenantID, - Role: "admin", + Role: "owner", } if err = dao.DB.Create(ut).Error; err != nil { t.Fatalf("insert test user_tenant: %v", err) @@ -2048,7 +2073,7 @@ func TestGetDocumentPreview_DocumentNotFound(t *testing.T) { svc := testDocumentService(t) ctx := t.Context() - _, err := svc.GetDocumentPreview(ctx, "tenant-1", "nonexistent") + _, err := svc.GetDocumentPreview(ctx, "user-1", "nonexistent") if !errors.Is(err, ErrPreviewDocumentNotFound) { t.Errorf("expected ErrPreviewDocumentNotFound, got %v", err) } @@ -2098,7 +2123,7 @@ func TestGetDocumentPreview_AccessControl(t *testing.T) { ctx := t.Context() // Dataset owner reads the original file. - p, err := svc.GetDocumentPreview(ctx, "tenant-1", "doc-prev") + p, err := svc.GetDocumentPreview(ctx, "user-1", "doc-prev") if err != nil { t.Fatalf("owner preview: %v", err) } @@ -2118,7 +2143,7 @@ func TestGetDocumentPreview_AccessControl(t *testing.T) { } // An unrelated user gets the same answer as for a missing document. - _, err = svc.GetDocumentPreview(ctx, "tenant-2", "doc-prev") + _, err = svc.GetDocumentPreview(ctx, "user-3", "doc-prev") if !errors.Is(err, ErrPreviewDocumentNotFound) { t.Fatalf("stranger preview: expected ErrPreviewDocumentNotFound, got %v", err) } @@ -2137,7 +2162,7 @@ func TestGetDocumentPreview_AccessControl(t *testing.T) { if _, err = svc.GetDocumentPreview(ctx, "user-2", "doc-prev-me"); !errors.Is(err, ErrPreviewDocumentNotFound) { t.Fatalf("team member on private dataset: expected ErrPreviewDocumentNotFound, got %v", err) } - p, err = svc.GetDocumentPreview(ctx, "tenant-1", "doc-prev-me") + p, err = svc.GetDocumentPreview(ctx, "user-1", "doc-prev-me") if err != nil { t.Fatalf("owner preview private dataset: %v", err) } @@ -2154,7 +2179,7 @@ func TestGetDocumentPreview_EmptyObject(t *testing.T) { insertTestPreviewDoc(t, db, mockStorage, "doc-prev-empty", "kb-prev-empty", "") svc := testDocumentService(t) - _, err := svc.GetDocumentPreview(t.Context(), "tenant-1", "doc-prev-empty") + _, err := svc.GetDocumentPreview(t.Context(), "user-1", "doc-prev-empty") if !errors.Is(err, ErrPreviewFileEmpty) { t.Fatalf("expected ErrPreviewFileEmpty, got %v", err) } @@ -2177,7 +2202,7 @@ func TestGetDocumentPreview_MissingObjectSurfacesStorageError(t *testing.T) { } svc := testDocumentService(t) - _, err := svc.GetDocumentPreview(t.Context(), "tenant-1", "doc-prev-miss") + _, err := svc.GetDocumentPreview(t.Context(), "user-1", "doc-prev-miss") if err == nil { t.Fatal("expected storage read error") } @@ -2212,6 +2237,26 @@ func TestDownloadDocument_WrongDataset(t *testing.T) { } } +func TestDownloadDocument_DatabaseErrorIsNotNotFound(t *testing.T) { + db := setupServiceTestDB(t) + pushServiceDB(t, db) + if err := db.Migrator().DropTable(&entity.Document{}); err != nil { + t.Fatalf("drop document table: %v", err) + } + + svc := testDocumentService(t) + _, err := svc.DownloadDocument(t.Context(), "kb-1", "doc-1") + if err == nil { + t.Fatal("expected database error") + } + if errors.Is(err, ErrDocumentNotFound) { + t.Fatalf("database error was masked as document not found: %v", err) + } + if !strings.Contains(err.Error(), "failed to get document doc-1") { + t.Fatalf("unexpected database error: %v", err) + } +} + func TestUpdateDatasetDocumentRejectsNonOwner(t *testing.T) { db := setupServiceTestDB(t) pushServiceDB(t, db) @@ -2220,14 +2265,14 @@ func TestUpdateDatasetDocumentRejectsNonOwner(t *testing.T) { svc := testDocumentService(t) ctx := t.Context() - _, code, err := svc.UpdateDatasetDocument(ctx, "tenant-2", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{}, map[string]bool{}) + _, code, err := svc.UpdateDatasetDocument(ctx, "user-2", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{}, map[string]bool{}) if err == nil { t.Fatal("expected ownership error") } - if code != common.CodeDataError { - t.Fatalf("code = %v, want %v", code, common.CodeDataError) + if code != common.CodeForbidden { + t.Fatalf("code = %v, want %v", code, common.CodeForbidden) } - if err.Error() != "you don't own the dataset" { + if err.Error() != "Permission denied" { t.Fatalf("err = %q", err.Error()) } } @@ -2241,7 +2286,7 @@ func TestUpdateDatasetDocumentRejectsCounterMutation(t *testing.T) { chunkCount := int64(6) svc := testDocumentService(t) ctx := t.Context() - _, code, err := svc.UpdateDatasetDocument(ctx, "tenant-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ + _, code, err := svc.UpdateDatasetDocument(ctx, "user-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ ChunkCount: &chunkCount, }, map[string]bool{"chunk_count": true}) if err == nil { @@ -2279,7 +2324,7 @@ func TestUpdateDatasetDocumentRejectsZeroImmutableFields(t *testing.T) { t.Fatalf("prepare document: %v", err) } - _, code, err := testDocumentService(t).UpdateDatasetDocument(t.Context(), "tenant-1", "kb-1", "doc-1", &tt.request, tt.present) + _, code, err := testDocumentService(t).UpdateDatasetDocument(t.Context(), "user-1", "kb-1", "doc-1", &tt.request, tt.present) if err == nil { t.Fatalf("expected %s mutation error", tt.name) } @@ -2399,7 +2444,7 @@ func TestUpdateDatasetDocumentRejectsUnsupportedParserIDForVisualDoc(t *testing. parseType := 1 svc := testDocumentService(t) ctx := t.Context() - _, code, err := svc.UpdateDatasetDocument(ctx, "tenant-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ + _, code, err := svc.UpdateDatasetDocument(ctx, "user-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ ParserID: &parserID, ParseType: &parseType, }, map[string]bool{"parser_id": true, "parse_type": true}) @@ -2425,7 +2470,7 @@ func TestUpdateDatasetDocumentRenameUpdatesDocumentAndFile(t *testing.T) { newName := "new.pdf" svc := testDocumentService(t) ctx := t.Context() - resp, code, err := svc.UpdateDatasetDocument(ctx, "tenant-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ + resp, code, err := svc.UpdateDatasetDocument(ctx, "user-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ Name: &newName, }, map[string]bool{"name": true}) if err != nil { @@ -2455,7 +2500,7 @@ func TestUpdateDatasetDocumentParserIDResetsForReparse(t *testing.T) { chunkMethod := "manual" svc := testDocumentService(t) ctx := t.Context() - resp, code, err := svc.UpdateDatasetDocument(ctx, "tenant-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ + resp, code, err := svc.UpdateDatasetDocument(ctx, "user-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ ParserID: &chunkMethod, ParseType: &parseType, }, map[string]bool{"parser_id": true, "parse_type": true}) @@ -3036,7 +3081,7 @@ func TestUpdateDatasetDocumentPropagatesMetadataDeleteFailure(t *testing.T) { svc.docEngine = engine svc.metadataSvc = service.NewMetadataServiceForTest(nil, nil) ctx := t.Context() - _, code, err := svc.UpdateDatasetDocument(ctx, "tenant-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ + _, code, err := svc.UpdateDatasetDocument(ctx, "user-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ MetaFields: map[string]any{"new": "value"}, }, map[string]bool{"meta_fields": true}) if err == nil { @@ -3375,7 +3420,7 @@ func TestUpdateDatasetDocumentPipelineIDTakesPrecedenceOverParserID(t *testing.T parseType := 2 svc := testDocumentService(t) ctx := t.Context() - resp, code, err := svc.UpdateDatasetDocument(ctx, "tenant-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ + resp, code, err := svc.UpdateDatasetDocument(ctx, "user-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ PipelineID: &pipelineID, ParseType: &parseType, }, map[string]bool{"pipeline_id": true, "parse_type": true}) @@ -3410,7 +3455,7 @@ func TestUpdateDatasetDocumentParseTypeBuiltin(t *testing.T) { pipelineIDEmpty := "" svc := testDocumentService(t) ctx := t.Context() - resp, code, err := svc.UpdateDatasetDocument(ctx, "tenant-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ + resp, code, err := svc.UpdateDatasetDocument(ctx, "user-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ ParseType: &parseType, ParserID: &parserID, PipelineID: &pipelineIDEmpty, @@ -3445,7 +3490,7 @@ func TestUpdateDatasetDocumentParseTypePipeline(t *testing.T) { pipelineID := "1234567890abcdef1234567890abcdef" svc := testDocumentService(t) ctx := t.Context() - resp, code, err := svc.UpdateDatasetDocument(ctx, "tenant-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ + resp, code, err := svc.UpdateDatasetDocument(ctx, "user-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ ParseType: &parseType, PipelineID: &pipelineID, }, map[string]bool{"parser_id": false, "pipeline_id": true, "parse_type": true}) @@ -3475,7 +3520,7 @@ func TestUpdateDatasetDocumentParseTypePipelineRejectsParserID(t *testing.T) { pipelineID := "1234567890abcdef1234567890abcdef" svc := testDocumentService(t) ctx := t.Context() - _, code, err := svc.UpdateDatasetDocument(ctx, "tenant-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ + _, code, err := svc.UpdateDatasetDocument(ctx, "user-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ ParseType: &parseType, ParserID: &parserID, PipelineID: &pipelineID, @@ -3506,7 +3551,7 @@ func TestUpdateDatasetDocumentParentChildConfigSurvivesDSLFailure(t *testing.T) parseType := 2 pipelineID := "1234567890abcdef1234567890abcdef" - resp, code, err := testDocumentService(t).UpdateDatasetDocument(t.Context(), "tenant-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ + resp, code, err := testDocumentService(t).UpdateDatasetDocument(t.Context(), "user-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ ParseType: &parseType, PipelineID: &pipelineID, ParserConfig: map[string]any{ @@ -3543,7 +3588,7 @@ func TestUpdateDatasetDocumentRejectsInvalidPages(t *testing.T) { parserID := "manual" svc := testDocumentService(t) ctx := t.Context() - _, code, err := svc.UpdateDatasetDocument(ctx, "tenant-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ + _, code, err := svc.UpdateDatasetDocument(ctx, "user-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ ParseType: &parseType, ParserID: &parserID, ParserConfig: map[string]any{ @@ -3568,7 +3613,7 @@ func TestUpdateDatasetDocumentParentChildConfigReachesGeneralChunker(t *testing. insertTestKB(t, "kb-1", "tenant-1", 1, 10, 5) insertNamedTestDoc(t, "doc-1", "kb-1", "doc.txt", 10, 5) - resp, code, err := testDocumentService(t).UpdateDatasetDocument(t.Context(), "tenant-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ + resp, code, err := testDocumentService(t).UpdateDatasetDocument(t.Context(), "user-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ ParserConfig: map[string]any{ "GeneralChunker:SixApplesFall": map[string]any{ "parent_child": map[string]any{ @@ -3606,7 +3651,7 @@ func TestUpdateDatasetDocumentEnabledUpdatesStatus(t *testing.T) { enabled := 0 svc := testDocumentService(t) ctx := t.Context() - resp, code, err := svc.UpdateDatasetDocument(ctx, "tenant-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ + resp, code, err := svc.UpdateDatasetDocument(ctx, "user-1", "kb-1", "doc-1", &UpdateDatasetDocumentRequest{ Enabled: &enabled, }, map[string]bool{"enabled": true}) if err != nil { @@ -3826,17 +3871,15 @@ func TestGetThumbnails_AlignsWithPythonFormatting(t *testing.T) { insertTestKB(t, "kb-1", "tenant-1", 0, 0, 0) insertTestKB(t, "kb-2", "tenant-1", 0, 0, 0) insertTestKB(t, "kb-other", "tenant-other", 0, 0, 0) + if err := db.Where("tenant_id = ? AND role = ?", "tenant-other", "owner").Delete(&entity.UserTenant{}).Error; err != nil { + t.Fatalf("remove fixture owner: %v", err) + } if err := db.Create(&entity.UserTenant{ - ID: "user-1_tenant-1", - UserID: "user-1", - TenantID: "tenant-1", - Role: "owner", - InvitedBy: "user-1", - Status: sptr("1"), + ID: "other-owner_tenant-other", UserID: "other-owner", TenantID: "tenant-other", + Role: "owner", InvitedBy: "other-owner", Status: sptr(string(entity.StatusValid)), }).Error; err != nil { - t.Fatalf("seed user tenant: %v", err) + t.Fatalf("seed other tenant owner: %v", err) } - base64Thumb := "data:image/png;base64,AAAA" fileThumb := "thumb.png" otherThumb := "secret.png" @@ -3938,6 +3981,7 @@ func TestGetDocumentImageForDocumentAllowsIndexedSharedBucketImage(t *testing.T) kbID := strings.Repeat("b", 32) status := string(entity.StatusValid) + ensureTestTenantOwner(t, "user-1", "user-1") if err := db.Create(&entity.Knowledgebase{ID: kbID, TenantID: "user-1", Name: "kb", EmbdID: "embd", Status: &status}).Error; err != nil { t.Fatal(err) } @@ -3991,7 +4035,7 @@ func TestGetDocumentThumbnailAuthorizesBeforeStorage(t *testing.T) { if store.getCalls != 0 { t.Fatalf("storage Get called %d times after authorization denial", store.getCalls) } - got, err := svc.GetDocumentThumbnail(ctx, "owner-1", "doc-thumb") + got, err := svc.GetDocumentThumbnail(ctx, "user-1", "doc-thumb") if err != nil || !bytes.Equal(got, image) { t.Fatalf("authorized GetDocumentThumbnail() = %q, %v", got, err) } diff --git a/internal/service/document/file2document.go b/internal/service/document/file2document.go index 0b2e97f640..c5bfbd2ef5 100644 --- a/internal/service/document/file2document.go +++ b/internal/service/document/file2document.go @@ -28,6 +28,8 @@ import ( "ragflow/internal/common" "ragflow/internal/dao" "ragflow/internal/entity" + "ragflow/internal/permission" + permissionresponse "ragflow/internal/permission/response" "ragflow/internal/utility" ) @@ -40,8 +42,6 @@ var ( ErrLinkFileNotFound = errors.New("File not found!") // ErrLinkDatasetNotFound mirrors Python "Can't find this dataset!". ErrLinkDatasetNotFound = errors.New("Can't find this dataset!") - // ErrLinkNoAuthorization mirrors Python "no authorization". - ErrLinkNoAuthorization = errors.New("no authorization") // ErrLinkInternal is a generic, safe-to-expose internal failure. ErrLinkInternal = errors.New("Internal server error.") ) @@ -133,14 +133,16 @@ func (s *File2DocumentService) LinkToDatasets(ctx context.Context, userID string return ErrLinkFileNotFound } if !service.CheckFileTeamPermission(ctx, s.fileDAO, file, userID) { - return ErrLinkNoAuthorization + _, permissionErr := permissionresponse.Normalize(permission.ErrPermissionDenied) + return permissionErr } } // ── 5. Validate KB permissions ──────────────────────────────────────────── for _, kb := range kbMap { - if !service.HasKBTeamPermission(ctx, kb, userID, dao.NewTenantDAO()) { - return ErrLinkNoAuthorization + if err := service.CheckDatasetAccess(ctx, permission.Subject{UserID: userID}, kb.ID, permission.OperationUpdate); err != nil { + _, permissionErr := permissionresponse.Normalize(err) + return permissionErr } } diff --git a/internal/service/file_permission.go b/internal/service/file_permission.go index 72194d50f3..5d5fe9257a 100644 --- a/internal/service/file_permission.go +++ b/internal/service/file_permission.go @@ -4,6 +4,7 @@ import ( "context" "ragflow/internal/dao" "ragflow/internal/entity" + "ragflow/internal/permission" ) // CheckFileTeamPermission reports whether userID may access the given file: either @@ -21,13 +22,8 @@ func CheckFileTeamPermission(ctx context.Context, fileDAO *dao.FileDAO, file *en return false } - kbDAO := dao.NewKnowledgebaseDAO() for _, datasetID := range datasetIDs { - kb, err := kbDAO.GetByID(ctx, dao.DB, datasetID) - if err != nil || kb == nil { - continue - } - if HasKBTeamPermission(ctx, kb, userID, dao.NewTenantDAO()) { + if CheckDatasetAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationRead) == nil { return true } } diff --git a/internal/service/search.go b/internal/service/search.go index fc28675c56..a9d4ff6406 100644 --- a/internal/service/search.go +++ b/internal/service/search.go @@ -24,6 +24,8 @@ import ( "ragflow/internal/common" "ragflow/internal/dao" "ragflow/internal/entity" + "ragflow/internal/permission" + permissionresponse "ragflow/internal/permission/response" "ragflow/internal/utility" ) @@ -389,10 +391,9 @@ func (s *SearchService) PrepareCompletion(ctx context.Context, userID, searchID } for _, datasetID := range datasetIDs { - accessible = s.datasetDAO.Accessible(ctx, dao.DB, datasetID, userID) - if !accessible { - // Mirror Python's search completion endpoint message. - return nil, common.CodeDataError, fmt.Errorf("You don't own the dataset %s", datasetID) + if err := CheckDatasetAccess(ctx, permission.Subject{UserID: userID}, datasetID, permission.OperationUse); err != nil { + code, permissionErr := permissionresponse.Normalize(err) + return nil, code, permissionErr } } diff --git a/internal/service/team_permission.go b/internal/service/team_permission.go deleted file mode 100644 index 6cc770b843..0000000000 --- a/internal/service/team_permission.go +++ /dev/null @@ -1,32 +0,0 @@ -package service - -import ( - "context" - "ragflow/internal/dao" - "ragflow/internal/entity" -) - -// HasKBTeamPermission mirrors Python check_kb_team_permission: -// direct owner access is always allowed; otherwise the KB must be team-shared -// and the caller must be a joined normal member of the owner tenant. -func HasKBTeamPermission(ctx context.Context, kb *entity.Knowledgebase, userID string, tenantDAO *dao.TenantDAO) bool { - if kb == nil { - return false - } - if kb.TenantID == userID { - return true - } - if kb.Permission != string(entity.TenantPermissionTeam) { - return false - } - joinedTenants, err := tenantDAO.GetJoinedTenantsByUserID(ctx, dao.DB, userID) - if err != nil { - return false - } - for _, tenant := range joinedTenants { - if tenant.TenantID == kb.TenantID { - return true - } - } - return false -} diff --git a/test/testcases/restful_api/test_chunks.py b/test/testcases/restful_api/test_chunks.py index eabb439609..0b9ba1301d 100644 --- a/test/testcases/restful_api/test_chunks.py +++ b/test/testcases/restful_api/test_chunks.py @@ -271,8 +271,8 @@ def test_chunk_add_invalid_dataset_and_document_contract(rest_client, create_doc ) assert invalid_dataset_res.status_code == 200 invalid_dataset_payload = invalid_dataset_res.json() - assert invalid_dataset_payload["code"] == 102, invalid_dataset_payload - assert invalid_dataset_payload["message"] == f"You don't own the dataset {INVALID_ID_32}.", invalid_dataset_payload + assert invalid_dataset_payload["code"] == 404, invalid_dataset_payload + assert invalid_dataset_payload["message"] == "Resource not found", invalid_dataset_payload invalid_document_res = rest_client.post( f"/datasets/{dataset_id}/documents/{INVALID_ID_32}/chunks", diff --git a/test/testcases/restful_api/test_datasets.py b/test/testcases/restful_api/test_datasets.py index 7c669faeda..2fa8ae42ce 100644 --- a/test/testcases/restful_api/test_datasets.py +++ b/test/testcases/restful_api/test_datasets.py @@ -560,14 +560,14 @@ def test_dataset_update_identifier_validation_contract(rest_client): not_uuid1_res = rest_client.put(f"/datasets/{uuid.uuid4().hex}", json=payload) assert not_uuid1_res.status_code == 200 not_uuid1_payload = not_uuid1_res.json() - assert not_uuid1_payload["code"] == 102, not_uuid1_payload - assert "lacks permission for dataset" in not_uuid1_payload["message"], not_uuid1_payload + assert not_uuid1_payload["code"] == 404, not_uuid1_payload + assert not_uuid1_payload["message"] == "Resource not found", not_uuid1_payload wrong_uuid_res = rest_client.put("/datasets/d94a8dc02c9711f0930f7fbc369eab6d", json=payload) assert wrong_uuid_res.status_code == 200 wrong_uuid_payload = wrong_uuid_res.json() - assert wrong_uuid_payload["code"] == 102, wrong_uuid_payload - assert "lacks permission for dataset" in wrong_uuid_payload["message"], wrong_uuid_payload + assert wrong_uuid_payload["code"] == 404, wrong_uuid_payload + assert wrong_uuid_payload["message"] == "Resource not found", wrong_uuid_payload @pytest.mark.p2 @@ -1312,14 +1312,14 @@ def test_dataset_delete_contract_matrix(rest_client, clear_datasets): id_not_uuid1_res = rest_client.delete("/datasets", json={"ids": [uuid.uuid4().hex]}) assert id_not_uuid1_res.status_code == 200 id_not_uuid1_payload = id_not_uuid1_res.json() - assert id_not_uuid1_payload["code"] == 102, id_not_uuid1_payload - assert "lacks permission for dataset" in id_not_uuid1_payload["message"], id_not_uuid1_payload + assert id_not_uuid1_payload["code"] == 403, id_not_uuid1_payload + assert id_not_uuid1_payload["message"] == "Permission denied", id_not_uuid1_payload id_wrong_uuid_res = rest_client.delete("/datasets", json={"ids": ["d94a8dc02c9711f0930f7fbc369eab6d"]}) assert id_wrong_uuid_res.status_code == 200 id_wrong_uuid_payload = id_wrong_uuid_res.json() - assert id_wrong_uuid_payload["code"] == 102, id_wrong_uuid_payload - assert "lacks permission for dataset" in id_wrong_uuid_payload["message"], id_wrong_uuid_payload + assert id_wrong_uuid_payload["code"] == 403, id_wrong_uuid_payload + assert id_wrong_uuid_payload["message"] == "Permission denied", id_wrong_uuid_payload list_res = rest_client.get("/datasets") assert list_res.status_code == 200 @@ -1335,8 +1335,8 @@ def test_dataset_delete_contract_matrix(rest_client, clear_datasets): partial_invalid_res = rest_client.delete("/datasets", json={"ids": invalid_ids}) assert partial_invalid_res.status_code == 200 partial_invalid_payload = partial_invalid_res.json() - assert partial_invalid_payload["code"] == 102, partial_invalid_payload - assert "lacks permission for dataset" in partial_invalid_payload["message"], partial_invalid_payload + assert partial_invalid_payload["code"] == 403, partial_invalid_payload + assert partial_invalid_payload["message"] == "Permission denied", partial_invalid_payload duplicate_ids_res = rest_client.delete("/datasets", json={"ids": remaining_ids + remaining_ids}) assert duplicate_ids_res.status_code == 200 @@ -1353,8 +1353,8 @@ def test_dataset_delete_contract_matrix(rest_client, clear_datasets): second_delete_res = rest_client.delete("/datasets", json=repeated_delete_payload) assert second_delete_res.status_code == 200 second_delete_payload = second_delete_res.json() - assert second_delete_payload["code"] == 102, second_delete_payload - assert "lacks permission for dataset" in second_delete_payload["message"], second_delete_payload + assert second_delete_payload["code"] == 403, second_delete_payload + assert second_delete_payload["message"] == "Permission denied", second_delete_payload unsupported_field_res = rest_client.delete("/datasets", json={"unknown_field": "unknown_field"}) assert unsupported_field_res.status_code == 200 @@ -1558,8 +1558,8 @@ def test_dataset_list_query_contract_matrix(rest_client, clear_datasets): name_wrong_res = rest_client.get("/datasets", params={"name": "wrong name"}) assert name_wrong_res.status_code == 200 name_wrong_payload = name_wrong_res.json() - assert name_wrong_payload["code"] == 102, name_wrong_payload - assert "lacks permission for dataset" in name_wrong_payload["message"], name_wrong_payload + assert name_wrong_payload["code"] == 0, name_wrong_payload + assert name_wrong_payload["data"] == [], name_wrong_payload name_empty_res = rest_client.get("/datasets", params={"name": ""}) assert name_empty_res.status_code == 200 @@ -1589,14 +1589,14 @@ def test_dataset_list_query_contract_matrix(rest_client, clear_datasets): id_not_uuid1_res = rest_client.get("/datasets", params={"id": uuid.uuid4().hex}) assert id_not_uuid1_res.status_code == 200 id_not_uuid1_payload = id_not_uuid1_res.json() - assert id_not_uuid1_payload["code"] == 102, id_not_uuid1_payload - assert "lacks permission for dataset" in id_not_uuid1_payload["message"], id_not_uuid1_payload + assert id_not_uuid1_payload["code"] == 404, id_not_uuid1_payload + assert id_not_uuid1_payload["message"] == "Resource not found", id_not_uuid1_payload id_wrong_uuid_res = rest_client.get("/datasets", params={"id": "d94a8dc02c9711f0930f7fbc369eab6d"}) assert id_wrong_uuid_res.status_code == 200 id_wrong_uuid_payload = id_wrong_uuid_res.json() - assert id_wrong_uuid_payload["code"] == 102, id_wrong_uuid_payload - assert "lacks permission for dataset" in id_wrong_uuid_payload["message"], id_wrong_uuid_payload + assert id_wrong_uuid_payload["code"] == 404, id_wrong_uuid_payload + assert id_wrong_uuid_payload["message"] == "Resource not found", id_wrong_uuid_payload id_empty_res = rest_client.get("/datasets", params={"id": ""}) assert id_empty_res.status_code == 200 @@ -1622,12 +1622,17 @@ def test_dataset_list_query_contract_matrix(rest_client, clear_datasets): assert name_id_mismatch_payload["code"] == 0, name_id_mismatch_payload assert len(name_id_mismatch_payload["data"]) == 0, name_id_mismatch_payload - for dataset_id, name in ((dataset_ids[0], "wrong_name"), (uuid.uuid1().hex, "dataset_0")): - name_id_wrong_res = rest_client.get("/datasets", params={"id": dataset_id, "name": name}) - assert name_id_wrong_res.status_code == 200 - name_id_wrong_payload = name_id_wrong_res.json() - assert name_id_wrong_payload["code"] == 102, name_id_wrong_payload - assert "lacks permission for dataset" in name_id_wrong_payload["message"], name_id_wrong_payload + name_id_wrong_res = rest_client.get("/datasets", params={"id": dataset_ids[0], "name": "wrong_name"}) + assert name_id_wrong_res.status_code == 200 + name_id_wrong_payload = name_id_wrong_res.json() + assert name_id_wrong_payload["code"] == 0, name_id_wrong_payload + assert name_id_wrong_payload["data"] == [], name_id_wrong_payload + + missing_id_name_res = rest_client.get("/datasets", params={"id": uuid.uuid1().hex, "name": "dataset_0"}) + assert missing_id_name_res.status_code == 200 + missing_id_name_payload = missing_id_name_res.json() + assert missing_id_name_payload["code"] == 404, missing_id_name_payload + assert missing_id_name_payload["message"] == "Resource not found", missing_id_name_payload unsupported_field_res = rest_client.get("/datasets", params={"unknown_field": "unknown_field"}) assert unsupported_field_res.status_code == 200 @@ -1709,8 +1714,8 @@ def test_dataset_metadata_config_get_and_update_contract(rest_client, create_dat invalid_dataset_res = rest_client.get("/datasets/invalid_dataset_id/metadata/config") assert invalid_dataset_res.status_code == 200 invalid_dataset_payload = invalid_dataset_res.json() - assert invalid_dataset_payload["code"] == 102, invalid_dataset_payload - assert "lacks permission for dataset 'invalid_dataset_id'" in invalid_dataset_payload["message"], invalid_dataset_payload + assert invalid_dataset_payload["code"] == 404, invalid_dataset_payload + assert invalid_dataset_payload["message"] == "Resource not found", invalid_dataset_payload update_payload = { "metadata": [ @@ -1758,8 +1763,8 @@ def test_dataset_metadata_config_get_and_update_contract(rest_client, create_dat ) assert invalid_update_dataset_res.status_code == 200 invalid_update_dataset_payload = invalid_update_dataset_res.json() - assert invalid_update_dataset_payload["code"] == 102, invalid_update_dataset_payload - assert "lacks permission for dataset 'invalid_dataset_id'" in invalid_update_dataset_payload["message"], invalid_update_dataset_payload + assert invalid_update_dataset_payload["code"] == 404, invalid_update_dataset_payload + assert invalid_update_dataset_payload["message"] == "Resource not found", invalid_update_dataset_payload def test_dataset_metadata_summary_contract(rest_client, create_dataset, tmp_path): diff --git a/test/testcases/restful_api/test_documents.py b/test/testcases/restful_api/test_documents.py index 9d87fbbb3e..1737e3dc83 100644 --- a/test/testcases/restful_api/test_documents.py +++ b/test/testcases/restful_api/test_documents.py @@ -584,8 +584,8 @@ def test_documents_update_invalid_dataset_and_document_contract(rest_client, cre ) assert invalid_dataset_res.status_code == 200 invalid_dataset_body = invalid_dataset_res.json() - assert invalid_dataset_body["code"] == 102, invalid_dataset_body - assert "you don't own the dataset" in invalid_dataset_body["message"], invalid_dataset_body + assert invalid_dataset_body["code"] == 404, invalid_dataset_body + assert invalid_dataset_body["message"] == "Resource not found", invalid_dataset_body invalid_document_res = rest_client.patch( f"/datasets/{dataset_id}/documents/{INVALID_ID_32}", @@ -1007,8 +1007,8 @@ def test_document_metadata_config_contract(rest_client, create_document): ) assert invalid_dataset_res.status_code == 200 invalid_dataset_payload = invalid_dataset_res.json() - assert invalid_dataset_payload["code"] == 102, invalid_dataset_payload - assert invalid_dataset_payload["message"] == "you don't own the dataset", invalid_dataset_payload + assert invalid_dataset_payload["code"] == 404, invalid_dataset_payload + assert invalid_dataset_payload["message"] == "Resource not found", invalid_dataset_payload invalid_document_res = rest_client.put( f"/datasets/{dataset_id}/documents/{INVALID_ID_32}/metadata/config", @@ -1487,15 +1487,15 @@ def test_documents_download_requires_auth_and_invalid_id_contract(rest_client, c invalid_doc_res = _download_document_to_file(rest_client, dataset_id, "invalid_document_id", invalid_doc_path) assert invalid_doc_res.status_code == 200 invalid_doc_payload = invalid_doc_res.json() - assert invalid_doc_payload["code"] == 102, invalid_doc_payload - assert invalid_doc_payload["message"] == "document not found", invalid_doc_payload + assert invalid_doc_payload["code"] == 404, invalid_doc_payload + assert invalid_doc_payload["message"] == "Resource not found", invalid_doc_payload invalid_dataset_path = tmp_path / "invalid_dataset_download.txt" invalid_dataset_res = _download_document_to_file(rest_client, "invalid_dataset_id", document_id, invalid_dataset_path) assert invalid_dataset_res.status_code == 200 invalid_dataset_payload = invalid_dataset_res.json() - assert invalid_dataset_payload["code"] == 102, invalid_dataset_payload - assert invalid_dataset_payload["message"] == "document not found", invalid_dataset_payload + assert invalid_dataset_payload["code"] == 404, invalid_dataset_payload + assert invalid_dataset_payload["message"] == "Resource not found", invalid_dataset_payload @pytest.mark.p3 diff --git a/test/testcases/restful_api/test_searches.py b/test/testcases/restful_api/test_searches.py index 4310c1352e..a61c527306 100644 --- a/test/testcases/restful_api/test_searches.py +++ b/test/testcases/restful_api/test_searches.py @@ -146,5 +146,5 @@ def test_search_completion_sse_shape_when_kb_ids_provided(rest_client, search_re ) assert res.status_code == 200 payload = res.json() - assert payload["code"] == 102, payload - assert "You don't own the dataset nonexistent_dataset" in payload["message"], payload + assert payload["code"] == 404, payload + assert payload["message"] == "Resource not found", payload