diff --git a/internal/authz/authorizer.go b/internal/authz/authorizer.go index 8dba3bd2..e78a0d15 100644 --- a/internal/authz/authorizer.go +++ b/internal/authz/authorizer.go @@ -7,6 +7,7 @@ import ( "time" "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" "github.com/jackc/pgx/v5/pgxpool" "chat-agent-system/internal/docduty" @@ -227,12 +228,61 @@ func (a *Authorizer) ListProjectMembers(ctx context.Context, projectID int64) ([ return out, rows.Err() } +// GetProjectMember 读取单个成员（含已退出）。无记录或禁用时返回 nil, nil。 +func (a *Authorizer) GetProjectMember(ctx context.Context, projectID, userID int64) (*ProjectMemberRecord, error) { + if a == nil || !a.enabled() || projectID <= 0 || userID <= 0 { + return nil, nil + } + var rec ProjectMemberRecord + var joined time.Time + err := a.pool.QueryRow(ctx, ` + SELECT pm.id, pm.project_id, pm.user_id, + COALESCE(NULLIF(u.username, ''), u.account_id, '') AS username, + COALESCE(u.name, '') AS name, + pm.project_role, COALESCE(pm.document_duty, ''), pm.status, pm.joined_at + FROM project_members pm + JOIN users u ON u.id = pm.user_id + WHERE pm.project_id = $1 AND pm.user_id = $2 + `, projectID, userID).Scan( + &rec.ID, &rec.ProjectID, &rec.UserID, + &rec.Username, &rec.Name, + &rec.ProjectRole, &rec.DocumentDuty, &rec.Status, &joined, + ) + if errors.Is(err, pgx.ErrNoRows) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("get project member: %w", err) + } + rec.JoinedAt = joined.Format("2006-01-02 15:04:05") + return &rec, nil +} + // UpsertProjectMember 新增或更新成员访问角色（禁止把 author/reviewer/approver 写入 project_role）。 func (a *Authorizer) UpsertProjectMember(ctx context.Context, projectID, userID int64, projectRole string) error { return a.UpsertProjectMemberSpec(ctx, projectID, userID, MemberUpsert{ProjectRole: projectRole}) } +func isUniqueViolation(err error) bool { + var pgErr *pgconn.PgError + return errors.As(err, &pgErr) && pgErr.Code == "23505" +} + +func isRetryableDutyConflict(err error) bool { + var pgErr *pgconn.PgError + if !errors.As(err, &pgErr) { + return false + } + switch pgErr.Code { + case "23505", "40P01", "40001": + return true + default: + return false + } +} + // UpsertProjectMemberSpec 写入访问角色，并可选独占 document_duty。 +// 并发抢同一职责时重试，收敛为 last-writer，并依赖部分唯一索引保证无双 reviewer。 func (a *Authorizer) UpsertProjectMemberSpec(ctx context.Context, projectID, userID int64, spec MemberUpsert) error { if projectID <= 0 || userID <= 0 { return fmt.Errorf("authz: invalid project or user id") @@ -251,12 +301,30 @@ func (a *Authorizer) UpsertProjectMemberSpec(ctx context.Context, projectID, use if !a.enabled() { return nil } + const maxAttempts = 4 + var last error + for attempt := 0; attempt < maxAttempts; attempt++ { + last = a.upsertProjectMemberSpecOnce(ctx, projectID, userID, spec, duty) + if last == nil || !isRetryableDutyConflict(last) { + return last + } + } + return last +} + +func (a *Authorizer) upsertProjectMemberSpecOnce(ctx context.Context, projectID, userID int64, spec MemberUpsert, duty string) error { tx, err := a.pool.Begin(ctx) if err != nil { return err } defer func() { _ = tx.Rollback(ctx) }() + if spec.DocumentDuty != nil { + if _, err := tx.Exec(ctx, `SELECT pg_advisory_xact_lock($1)`, projectID); err != nil { + return err + } + } + if spec.DocumentDuty != nil && duty != docduty.DutyNone { if _, err := tx.Exec(ctx, ` UPDATE project_members diff --git a/internal/authz/service_test.go b/internal/authz/service_test.go index 30dfb7ab..3f753d37 100644 --- a/internal/authz/service_test.go +++ b/internal/authz/service_test.go @@ -42,3 +42,11 @@ func TestUpsertProjectMember_RejectsDocRolesWithoutDB(t *testing.T) { t.Fatal("document_duty=viewer must be rejected") } } + +func TestGetProjectMember_DisabledWithoutPool(t *testing.T) { + a := authz.NewAuthorizer(nil) + rec, err := a.GetProjectMember(context.Background(), 1, 2) + if err != nil || rec != nil { + t.Fatalf("disabled authorizer must not invent a member, got (%v, %v)", rec, err) + } +} diff --git a/internal/server/document_revision_duty_write_test.go b/internal/server/document_revision_duty_write_test.go index 9297c1c9..c678ebc2 100644 --- a/internal/server/document_revision_duty_write_test.go +++ b/internal/server/document_revision_duty_write_test.go @@ -2,6 +2,7 @@ package server import ( "context" + "errors" "testing" "chat-agent-system/internal/auth" @@ -11,6 +12,9 @@ import ( func TestAuthorizeDocRevision_ReviewerAndNonMemberCannotWrite(t *testing.T) { srv := newTestServer(t) + // 隔离写权门闩：跳过项目成员库检查，只验证 AuthorID。禁止依赖共享库或默认拒绝。 + srv.cfg.AllowInMemory = true + srv.authorizer = authz.NewAuthorizer(nil) ctx := context.Background() const ( authorID int64 = 170 @@ -29,20 +33,17 @@ func TestAuthorizeDocRevision_ReviewerAndNonMemberCannotWrite(t *testing.T) { } author := &auth.UserAccount{ID: authorID, Name: "编制人A"} - if err := srv.authorizeDocRevision(ctx, author, ref, true); err != nil && err != authz.ErrForbidden { - // 无 project_members 时可能因 authorizer 策略拒绝项目访问；编制人写权仍由 AuthorID 门闩覆盖。 - if err != docrevision.ErrForbidden { - t.Fatalf("author write unexpected err: %v", err) - } + if err := srv.authorizeDocRevision(ctx, author, ref, true); err != nil { + t.Fatalf("author write must succeed, got %v", err) } reviewer := &auth.UserAccount{ID: reviewerID, Name: "审核人B"} - if err := srv.authorizeDocRevision(ctx, reviewer, ref, true); err == nil { - t.Fatal("reviewer B must not write controlled body") + if err := srv.authorizeDocRevision(ctx, reviewer, ref, true); !errors.Is(err, docrevision.ErrForbidden) { + t.Fatalf("reviewer write want docrevision.ErrForbidden, got %v", err) } stranger := &auth.UserAccount{ID: strangerID, Name: "局外人C"} - if err := srv.authorizeDocRevision(ctx, stranger, ref, true); err == nil { - t.Fatal("non-member C must not write controlled body") + if err := srv.authorizeDocRevision(ctx, stranger, ref, true); !errors.Is(err, docrevision.ErrForbidden) { + t.Fatalf("outsider write want docrevision.ErrForbidden, got %v", err) } } diff --git a/internal/server/project_members_handlers.go b/internal/server/project_members_handlers.go index 472784b1..cfcc9666 100644 --- a/internal/server/project_members_handlers.go +++ b/internal/server/project_members_handlers.go @@ -1,14 +1,45 @@ package server import ( + "context" "encoding/json" + "errors" + "log" "net/http" "strconv" "chat-agent-system/internal/auth" "chat-agent-system/internal/authz" + "chat-agent-system/internal/docduty" ) +var errProjectRoleRequired = errors.New("project_role required when member does not exist") + +func resolveUpdateProjectRole(requested string, existing *authz.ProjectMemberRecord) (string, error) { + if requested != "" { + return requested, nil + } + if existing != nil && existing.ProjectRole != "" { + return existing.ProjectRole, nil + } + return "", errProjectRoleRequired +} + +func (s *Server) syncProjectDocumentDuties(ctx context.Context, projectID int64) error { + if s == nil || s.svc == nil || projectID <= 0 { + return nil + } + pool := s.svc.Pool() + if pool == nil { + return nil + } + _, _, err := docduty.HealProjectDocuments(ctx, pool, projectID) + if err != nil { + log.Printf("[docduty] sync project %d after member change failed: %v", projectID, err) + } + return err +} + type projectMemberRequest struct { UserID int64 `json:"user_id"` ProjectRole string `json:"project_role"` @@ -89,6 +120,10 @@ func (s *Server) handleAddProjectMember(w http.ResponseWriter, r *http.Request, http.Error(w, err.Error(), http.StatusBadRequest) return } + if err := s.syncProjectDocumentDuties(r.Context(), projectID); err != nil { + http.Error(w, `{"error":"ERR_DUTY_SYNC","message":"成员已保存，但受控文档职责同步失败"}`, http.StatusInternalServerError) + return + } writeJSONResponse(w, http.StatusCreated, map[string]bool{"success": true}) } @@ -102,9 +137,23 @@ func (s *Server) handleUpdateProjectMember(w http.ResponseWriter, r *http.Reques return } if req.ProjectRole != "" || req.DocumentDuty != nil { - role := req.ProjectRole - if role == "" { - role = authz.RoleMember + var existing *authz.ProjectMemberRecord + if req.ProjectRole == "" { + if s.authorizer == nil { + http.Error(w, `{"error":"ERR_INVALID_PARAM","message":"project_role required with document_duty"}`, http.StatusBadRequest) + return + } + rec, err := s.authorizer.GetProjectMember(r.Context(), projectID, req.UserID) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + existing = rec + } + role, err := resolveUpdateProjectRole(req.ProjectRole, existing) + if err != nil { + http.Error(w, `{"error":"ERR_INVALID_PARAM","message":"project_role required when member does not exist"}`, http.StatusBadRequest) + return } if err := s.authorizer.UpsertProjectMemberSpec(r.Context(), projectID, req.UserID, authz.MemberUpsert{ ProjectRole: role, @@ -120,6 +169,12 @@ func (s *Server) handleUpdateProjectMember(w http.ResponseWriter, r *http.Reques return } } + if req.ProjectRole != "" || req.DocumentDuty != nil || req.Status != "" { + if err := s.syncProjectDocumentDuties(r.Context(), projectID); err != nil { + http.Error(w, `{"error":"ERR_DUTY_SYNC","message":"成员已保存，但受控文档职责同步失败"}`, http.StatusInternalServerError) + return + } + } writeJSONResponse(w, http.StatusOK, map[string]bool{"success": true}) } diff --git a/internal/server/project_members_role_test.go b/internal/server/project_members_role_test.go new file mode 100644 index 00000000..fab42769 --- /dev/null +++ b/internal/server/project_members_role_test.go @@ -0,0 +1,37 @@ +package server + +import ( + "errors" + "testing" + + "chat-agent-system/internal/authz" +) + +func TestResolveUpdateProjectRole_KeepsExistingOwner(t *testing.T) { + role, err := resolveUpdateProjectRole("", &authz.ProjectMemberRecord{ProjectRole: authz.RoleOwner}) + if err != nil { + t.Fatal(err) + } + if role != authz.RoleOwner { + t.Fatalf("omitted project_role must keep owner, got %q", role) + } +} + +func TestResolveUpdateProjectRole_RejectsMissingMember(t *testing.T) { + if _, err := resolveUpdateProjectRole("", nil); !errors.Is(err, errProjectRoleRequired) { + t.Fatalf("want errProjectRoleRequired, got %v", err) + } + if _, err := resolveUpdateProjectRole("", &authz.ProjectMemberRecord{}); !errors.Is(err, errProjectRoleRequired) { + t.Fatalf("empty existing role must be rejected, got %v", err) + } +} + +func TestResolveUpdateProjectRole_ExplicitRoleWins(t *testing.T) { + role, err := resolveUpdateProjectRole(authz.RoleViewer, &authz.ProjectMemberRecord{ProjectRole: authz.RoleOwner}) + if err != nil { + t.Fatal(err) + } + if role != authz.RoleViewer { + t.Fatalf("explicit role must win, got %q", role) + } +}