Skip to content

Commit e170e2d

Browse files
committed
feat: repair multiple permissions docs for a shared drive on documents get(permissions check)
1 parent fd87add commit e170e2d

2 files changed

Lines changed: 66 additions & 5 deletions

File tree

model/permission/permissions.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,14 @@ func getShareInteractPermissions(db prefixer.Prefixer, sharingID string) ([]Perm
281281
}
282282
err := couchdb.FindDocs(db, consts.Permissions, &req, &res)
283283
if err != nil {
284-
return nil, err
284+
// With a cluster of couchdb, we can have a race condition where we
285+
// query an index before it has been updated for a doc that has just
286+
// been created. Keep the same fallback as getFromSource.
287+
time.Sleep(1 * time.Second)
288+
err = couchdb.FindDocs(db, consts.Permissions, &req, &res)
289+
if err != nil {
290+
return nil, err
291+
}
285292
}
286293
return res, nil
287294
}
@@ -335,7 +342,11 @@ func shareInteractRepairState(perms []Permission, sharingID string) (*Permission
335342
canonicalID := ShareInteractPermissionID(sharingID)
336343
var canonical *Permission
337344
usable := 0
345+
hasDuplicate := false
338346
for i := range perms {
347+
if perms[i].ID() != canonicalID {
348+
hasDuplicate = true
349+
}
339350
if perms[i].Expired() {
340351
continue
341352
}
@@ -347,7 +358,7 @@ func shareInteractRepairState(perms []Permission, sharingID string) (*Permission
347358
if usable == 0 {
348359
return nil, false, ErrExpiredToken
349360
}
350-
if usable == 1 && canonical != nil {
361+
if usable == 1 && canonical != nil && !hasDuplicate {
351362
return canonical, false, nil
352363
}
353364
return nil, true, nil
@@ -370,15 +381,14 @@ func repairShareInteractPermissions(db prefixer.Prefixer, sharingID string, perm
370381
hasCanonical = true
371382
// If the canonical doc is expired, update that doc ID but rebuild its content from non-expired docs.
372383
merged.SetRev(p.Rev())
384+
} else {
385+
duplicates = append(duplicates, p)
373386
}
374387
if p.Expired() {
375388
continue
376389
}
377390

378391
hasUsablePermission = true
379-
if p.ID() != canonicalID {
380-
duplicates = append(duplicates, p)
381-
}
382392

383393
if len(merged.Permissions) == 0 && len(p.Permissions) > 0 {
384394
merged.Permissions = p.Clone().(*Permission).Permissions

model/permission/permissions_test.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,57 @@ func TestGetForShareInteractRepairsDuplicateDocs(t *testing.T) {
494494
require.Len(t, all, 1)
495495
}
496496

497+
func TestGetForShareInteractRepairsExpiredDuplicateDocs(t *testing.T) {
498+
if testing.Short() {
499+
t.Skip("an instance is required for this test: test skipped due to the use of --short flag")
500+
}
501+
502+
config.UseTestFile(t)
503+
require.NoError(t, couchdb.ResetDB(testPrefix, consts.Permissions))
504+
505+
const sharingID = "sharing-expired-duplicate-interact-permissions"
506+
rules := Set{{
507+
Title: "Shared drive",
508+
Type: consts.Files,
509+
Values: []string{"shared-drive-root"},
510+
Verbs: ALL,
511+
}}
512+
expiredAt := time.Now().Add(-time.Hour).Format(time.RFC3339)
513+
514+
err := couchdb.CreateNamedDoc(testPrefix, &Permission{
515+
PID: ShareInteractPermissionID(sharingID),
516+
Type: TypeShareInteract,
517+
Permissions: rules,
518+
Codes: map[string]string{
519+
"alice@example.test": "alice-token",
520+
},
521+
SourceID: consts.Sharings + "/" + sharingID,
522+
})
523+
require.NoError(t, err)
524+
err = couchdb.CreateDoc(testPrefix, &Permission{
525+
Type: TypeShareInteract,
526+
Permissions: rules,
527+
Codes: map[string]string{
528+
"expired@example.test": "expired-token",
529+
},
530+
ExpiresAt: expiredAt,
531+
SourceID: consts.Sharings + "/" + sharingID,
532+
})
533+
require.NoError(t, err)
534+
535+
repaired, err := GetForShareInteract(testPrefix, sharingID)
536+
require.NoError(t, err)
537+
require.Equal(t, ShareInteractPermissionID(sharingID), repaired.ID())
538+
require.Equal(t, map[string]string{
539+
"alice@example.test": "alice-token",
540+
}, repaired.Codes)
541+
542+
all, err := getShareInteractPermissions(testPrefix, sharingID)
543+
require.NoError(t, err)
544+
require.Len(t, all, 1)
545+
require.Equal(t, ShareInteractPermissionID(sharingID), all[0].ID())
546+
}
547+
497548
func TestCreateShareInteractSetUsesCanonicalDoc(t *testing.T) {
498549
if testing.Short() {
499550
t.Skip("an instance is required for this test: test skipped due to the use of --short flag")

0 commit comments

Comments
 (0)