fix(api): keep owner role and sync docs after duty upsert
Omit project_role no longer defaults to member. Exclusive duty writes take an advisory lock and retry conflicts so last-writer wins without dual reviewers. Author write tests require err==nil. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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})
|
||||
}
|
||||
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user