feat: repair multiple permissions docs for a shared drive on documents get(permissions check)

Author: shepilov

model/permission/permissions.go

@@ -3,20 +3,23 @@
 package permission
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"net/http"
 	"strings"
 	"time"
 
 	build "github.com/cozy/cozy-stack/pkg/config"
+	stackconfig "github.com/cozy/cozy-stack/pkg/config/config"
 	"github.com/cozy/cozy-stack/pkg/consts"
 	"github.com/cozy/cozy-stack/pkg/couchdb"
 	"github.com/cozy/cozy-stack/pkg/couchdb/mango"
 	"github.com/cozy/cozy-stack/pkg/crypto"
 	"github.com/cozy/cozy-stack/pkg/logger"
 	"github.com/cozy/cozy-stack/pkg/metadata"
 	"github.com/cozy/cozy-stack/pkg/prefixer"
+	"github.com/cozy/cozy-stack/pkg/utils"
 	"github.com/labstack/echo/v4"
 )
 
@@ -243,9 +246,17 @@ func GetForSharePreview(db prefixer.Prefixer, sharingID string) (*Permission, er
 }
 
 // GetForShareInteract retrieves the Permission doc for a given sharing to
-// read/write a note
+// read/write a note. It may repair legacy duplicate share-interact permission
+// docs as part of the read by creating/updating the canonical permission doc
+// and deleting duplicate legacy docs.
 func GetForShareInteract(db prefixer.Prefixer, sharingID string) (*Permission, error) {
-	return getFromSource(db, TypeShareInteract, consts.Sharings, sharingID)
+	mu := stackconfig.Lock().ReadWrite(db, shareInteractLockName(sharingID))
+	if err := mu.Lock(); err != nil {
+		return nil, err
+	}
+	defer mu.Unlock()
+
+	return getOrRepairShareInteractPermissions(db, sharingID)
 }
 
 // ShareInteractPermissionID returns the canonical permission document ID for a
@@ -254,6 +265,10 @@ func ShareInteractPermissionID(sharingID string) string {
 	return TypeShareInteract + "-" + sharingID
 }
 
+func shareInteractLockName(sharingID string) string {
+	return "permissions/share-interact/" + sharingID
+}
+
 func getShareInteractPermissions(db prefixer.Prefixer, sharingID string) ([]Permission, error) {
 	var res []Permission
 	req := couchdb.FindRequest{
@@ -271,33 +286,44 @@ func getShareInteractPermissions(db prefixer.Prefixer, sharingID string) ([]Perm
 	return res, nil
 }
 
-// RepairShareInteractPermissions merges duplicate share-interact permission
-// documents for a sharing into the canonical document and deletes the legacy
-// duplicates.
-func RepairShareInteractPermissions(db prefixer.Prefixer, sharingID string) (*Permission, error) {
-	var lastErr error
-	for attempt := 0; attempt < 5; attempt++ {
-		perm, retry, err := repairShareInteractPermissions(db, sharingID)
+func getOrRepairShareInteractPermissions(db prefixer.Prefixer, sharingID string) (*Permission, error) {
+	return utils.RetryWithBackoffValue(context.Background(), shareInteractRetryOptions(), func() (*Permission, error) {
+		perms, err := getShareInteractPermissions(db, sharingID)
+		if err != nil {
+			return nil, err
+		}
+		if canonical, needsRepair, err := shareInteractRepairState(perms, sharingID); err != nil || !needsRepair {
+			return canonical, err
+		}
+
+		perm, retry, err := repairShareInteractPermissions(db, sharingID, perms)
 		if err == nil && !retry {
 			return perm, nil
 		}
-		lastErr = err
-	}
-	if lastErr != nil {
-		return nil, lastErr
-	}
-	return nil, &couchdb.Error{
-		StatusCode: http.StatusConflict,
-		Name:       "conflict",
-		Reason:     "could not repair share-interact permissions after retries",
-	}
+		if retry && err == nil {
+			return nil, &couchdb.Error{
+				StatusCode: http.StatusConflict,
+				Name:       "conflict",
+				Reason:     "could not repair share-interact permissions after retries",
+			}
+		}
+		return nil, err
+	})
 }
 
-func repairShareInteractPermissions(db prefixer.Prefixer, sharingID string) (*Permission, bool, error) {
-	perms, err := getShareInteractPermissions(db, sharingID)
-	if err != nil {
-		return nil, false, err
+func shareInteractRetryOptions() utils.RetryOptions {
+	return utils.RetryOptions{
+		Attempts:     5,
+		Delay:        10 * time.Millisecond,
+		MaxDelay:     100 * time.Millisecond,
+		JitterFactor: 0.25,
+		ShouldRetry: func(err error) bool {
+			return couchdb.IsConflictError(err) || couchdb.IsFileExists(err)
+		},
 	}
+}
+
+func shareInteractRepairState(perms []Permission, sharingID string) (*Permission, bool, error) {
 	if len(perms) == 0 {
 		return nil, false, &couchdb.Error{
 			StatusCode: http.StatusNotFound,
@@ -306,6 +332,28 @@ func repairShareInteractPermissions(db prefixer.Prefixer, sharingID string) (*Pe
 		}
 	}
 
+	canonicalID := ShareInteractPermissionID(sharingID)
+	var canonical *Permission
+	usable := 0
+	for i := range perms {
+		if perms[i].Expired() {
+			continue
+		}
+		usable++
+		if perms[i].ID() == canonicalID {
+			canonical = &perms[i]
+		}
+	}
+	if usable == 0 {
+		return nil, false, ErrExpiredToken
+	}
+	if usable == 1 && canonical != nil {
+		return canonical, false, nil
+	}
+	return nil, true, nil
+}
+
+func repairShareInteractPermissions(db prefixer.Prefixer, sharingID string, perms []Permission) (*Permission, bool, error) {
 	canonicalID := ShareInteractPermissionID(sharingID)
 	merged := &Permission{
 		PID:      canonicalID,
@@ -318,15 +366,17 @@ func repairShareInteractPermissions(db prefixer.Prefixer, sharingID string) (*Pe
 	duplicates := make([]*Permission, 0, len(perms))
 	for i := range perms {
 		p := &perms[i]
+		if p.ID() == canonicalID {
+			hasCanonical = true
+			// If the canonical doc is expired, update that doc ID but rebuild its content from non-expired docs.
+			merged.SetRev(p.Rev())
+		}
 		if p.Expired() {
 			continue
 		}
 
 		hasUsablePermission = true
-		if p.ID() == canonicalID {
-			hasCanonical = true
-			merged.SetRev(p.Rev())
-		} else {
+		if p.ID() != canonicalID {
 			duplicates = append(duplicates, p)
 		}
 
@@ -743,21 +793,75 @@ func CreateSharePreviewSet(db prefixer.Prefixer, sharingID string, codes, shortc
 	return doc, nil
 }
 
-// CreateShareInteractSet creates a Permission doc for reading/writing a note
-// inside a sharing
+// CreateShareInteractSet creates or updates the Permission doc for reading and
+// writing a note inside a sharing. When subdoc.Permissions is not empty, it
+// replaces the existing permission rules with that full set.
 func CreateShareInteractSet(db prefixer.Prefixer, sharingID string, codes map[string]string, subdoc Permission) (*Permission, error) {
 	doc := &Permission{
+		PID:         ShareInteractPermissionID(sharingID),
 		Type:        TypeShareInteract,
 		Permissions: subdoc.Permissions,
 		Codes:       codes,
 		SourceID:    consts.Sharings + "/" + sharingID,
 		Metadata:    subdoc.Metadata,
 	}
-	err := couchdb.CreateDoc(db, doc)
-	if err != nil {
+
+	if doc.Codes == nil {
+		doc.Codes = make(map[string]string)
+	}
+
+	mu := stackconfig.Lock().ReadWrite(db, shareInteractLockName(sharingID))
+	if err := mu.Lock(); err != nil {
 		return nil, err
 	}
-	return doc, nil
+	defer mu.Unlock()
+
+	return utils.RetryWithBackoffValue(context.Background(), shareInteractRetryOptions(), func() (*Permission, error) {
+		existing, err := getOrRepairShareInteractPermissions(db, sharingID)
+		if err != nil && !couchdb.IsNotFoundError(err) {
+			return nil, err
+		}
+		if err == nil {
+			merged := existing.Clone().(*Permission)
+			merged.Type = TypeShareInteract
+			merged.SourceID = consts.Sharings + "/" + sharingID
+			merged.SetID(ShareInteractPermissionID(sharingID))
+			if len(subdoc.Permissions) > 0 {
+				// Callers pass the complete interact rule set, so replace instead of merging rules.
+				merged.Permissions = subdoc.Permissions
+			}
+			if merged.Metadata == nil && subdoc.Metadata != nil {
+				merged.Metadata = subdoc.Metadata.Clone()
+			}
+			if merged.Metadata != nil {
+				merged.Metadata.ChangeUpdatedAt()
+			}
+			if merged.Codes == nil {
+				merged.Codes = make(map[string]string)
+			}
+			for key, code := range codes {
+				if key == "" {
+					continue
+				}
+				if existingCode, ok := merged.Codes[key]; ok && existingCode != code {
+					logger.WithDomain(db.DomainName()).WithNamespace("permissions").
+						Warnf("keeping existing share-interact code for %s in sharing %s", key, sharingID)
+					continue
+				}
+				merged.Codes[key] = code
+			}
+			if err := couchdb.UpdateDoc(db, merged); err != nil {
+				return nil, err
+			}
+			return merged, nil
+		}
+
+		err = couchdb.CreateNamedDoc(db, doc)
+		if err == nil {
+			return doc, nil
+		}
+		return nil, err
+	})
 }
 
 // ForceWebapp creates or updates a Permission doc for a given webapp

model/permission/permissions_test.go

@@ -5,10 +5,12 @@ import (
 	"encoding/json"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/cozy/cozy-stack/pkg/config/config"
 	"github.com/cozy/cozy-stack/pkg/consts"
 	"github.com/cozy/cozy-stack/pkg/couchdb"
+	"github.com/cozy/cozy-stack/pkg/metadata"
 	"github.com/cozy/cozy-stack/pkg/prefixer"
 	"github.com/labstack/echo/v4"
 	"github.com/stretchr/testify/assert"
@@ -430,7 +432,7 @@ func TestShareSetPermissions(t *testing.T) {
 	assert.Error(t, err)
 }
 
-func TestRepairShareInteractPermissionsMergesDuplicateDocs(t *testing.T) {
+func TestGetForShareInteractRepairsDuplicateDocs(t *testing.T) {
 	if testing.Short() {
 		t.Skip("an instance is required for this test: test skipped due to the use of --short flag")
 	}
@@ -448,16 +450,26 @@ func TestRepairShareInteractPermissionsMergesDuplicateDocs(t *testing.T) {
 		}},
 	}
 
-	_, err := CreateShareInteractSet(testPrefix, sharingID, map[string]string{
-		"alice@example.test": "alice-token",
-	}, perms)
+	err := couchdb.CreateDoc(testPrefix, &Permission{
+		Type:        TypeShareInteract,
+		Permissions: perms.Permissions,
+		Codes: map[string]string{
+			"alice@example.test": "alice-token",
+		},
+		SourceID: consts.Sharings + "/" + sharingID,
+	})
 	require.NoError(t, err)
-	_, err = CreateShareInteractSet(testPrefix, sharingID, map[string]string{
-		"bob@example.test": "bob-token",
-	}, perms)
+	err = couchdb.CreateDoc(testPrefix, &Permission{
+		Type:        TypeShareInteract,
+		Permissions: perms.Permissions,
+		Codes: map[string]string{
+			"bob@example.test": "bob-token",
+		},
+		SourceID: consts.Sharings + "/" + sharingID,
+	})
 	require.NoError(t, err)
 
-	repaired, err := RepairShareInteractPermissions(testPrefix, sharingID)
+	repaired, err := GetForShareInteract(testPrefix, sharingID)
 	require.NoError(t, err)
 	require.Equal(t, ShareInteractPermissionID(sharingID), repaired.ID())
 	require.Equal(t, map[string]string{
@@ -470,7 +482,7 @@ func TestRepairShareInteractPermissionsMergesDuplicateDocs(t *testing.T) {
 	require.Len(t, all, 1)
 	require.Equal(t, ShareInteractPermissionID(sharingID), all[0].ID())
 
-	repaired, err = RepairShareInteractPermissions(testPrefix, sharingID)
+	repaired, err = GetForShareInteract(testPrefix, sharingID)
 	require.NoError(t, err)
 	require.Equal(t, map[string]string{
 		"alice@example.test": "alice-token",
@@ -482,6 +494,52 @@ func TestRepairShareInteractPermissionsMergesDuplicateDocs(t *testing.T) {
 	require.Len(t, all, 1)
 }
 
+func TestCreateShareInteractSetUsesCanonicalDoc(t *testing.T) {
+	if testing.Short() {
+		t.Skip("an instance is required for this test: test skipped due to the use of --short flag")
+	}
+
+	config.UseTestFile(t)
+	require.NoError(t, couchdb.ResetDB(testPrefix, consts.Permissions))
+
+	const sharingID = "sharing-canonical-interact-permissions"
+	md := metadata.New()
+	md.UpdatedAt = time.Now().Add(-time.Hour)
+	perms := Permission{
+		Permissions: Set{{
+			Title:  "Shared drive",
+			Type:   consts.Files,
+			Values: []string{"shared-drive-root"},
+			Verbs:  ALL,
+		}},
+		Metadata: md,
+	}
+
+	first, err := CreateShareInteractSet(testPrefix, sharingID, map[string]string{
+		"alice@example.test": "alice-token",
+	}, perms)
+	require.NoError(t, err)
+	require.Equal(t, ShareInteractPermissionID(sharingID), first.ID())
+
+	second, err := CreateShareInteractSet(testPrefix, sharingID, map[string]string{
+		"bob@example.test": "bob-token",
+	}, perms)
+	require.NoError(t, err)
+	require.Equal(t, ShareInteractPermissionID(sharingID), second.ID())
+	require.Equal(t, map[string]string{
+		"alice@example.test": "alice-token",
+		"bob@example.test":   "bob-token",
+	}, second.Codes)
+	require.NotNil(t, first.Metadata)
+	require.NotNil(t, second.Metadata)
+	require.True(t, second.Metadata.UpdatedAt.After(first.Metadata.UpdatedAt))
+
+	all, err := getShareInteractPermissions(testPrefix, sharingID)
+	require.NoError(t, err)
+	require.Len(t, all, 1)
+	require.Equal(t, ShareInteractPermissionID(sharingID), all[0].ID())
+}
+
 func TestCreateShareSetBlocklist(t *testing.T) {
 	s := Set{Rule{Type: "io.cozy.notifications"}}
 	subdoc := Permission{