Skip to content

Commit b7a9302

Browse files
authored
fix: avoid races during interactive drive acceptance (#4720)
### What happend: - `alice@twake.app` created the shared drive invitation, at that point, the recipient's Cozy URL(bob@twake.app) was not yet registered on the owner side, so the system only sent the discovery email link - because there was no recipient-side sharing request yet, trusted-domain auto-accept job had nothing to accept yet - `bob@twake.app` clicked the discovery link from the email - the discovery flow created the recipient-side sharing request with `PUT /sharings/:id` on the recipient Cozy, as soon as that request arrived, the recipient recognized the sender as trusted (`linagora.com`) and enqueued drive auto-accept. - the browser then continued into the normal manual acceptance route: `GET /auth/authorize/sharing?...`. - both paths then touched the same sharing on the recipient side - the sharing was actually accepted, and the owner side received a successful `/answer`, but the browser/manual path hit a stale CouchDB revision conflict and returned a `409` page. ### Changes - mark discovery/authorize acceptance as interactive(without auto-acceptance flow) - make `SendAnswer` idempotent on conflicts
2 parents 208560e + 3314179 commit b7a9302

File tree

5 files changed

+639
-270
lines changed

5 files changed

+639
-270
lines changed

model/sharing/oauth.go

Lines changed: 81 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ import (
2828
"github.com/labstack/echo/v4"
2929
)
3030

31+
const InteractiveSharingQueryParam = "interactive"
32+
3133
// CreateSharingRequest sends information about the sharing to the recipient's cozy
3234
func (m *Member) CreateSharingRequest(inst *instance.Instance, s *Sharing, c *Credentials, u *url.URL) error {
3335
if len(c.XorKey) == 0 {
@@ -233,6 +235,16 @@ func countFilesInDirectory(inst *instance.Instance, dir *vfs.DirDoc) (int, error
233235

234236
// RegisterCozyURL saves a new Cozy URL for a member
235237
func (s *Sharing) RegisterCozyURL(inst *instance.Instance, m *Member, cozyURL string) error {
238+
return s.registerCozyURL(inst, m, cozyURL, false)
239+
}
240+
241+
// RegisterCozyURLInteractive saves a Cozy URL discovered during an interactive
242+
// acceptance flow and marks the recipient request so auto-accept is skipped.
243+
func (s *Sharing) RegisterCozyURLInteractive(inst *instance.Instance, m *Member, cozyURL string) error {
244+
return s.registerCozyURL(inst, m, cozyURL, true)
245+
}
246+
247+
func (s *Sharing) registerCozyURL(inst *instance.Instance, m *Member, cozyURL string, interactive bool) error {
236248
if !s.Owner {
237249
return ErrInvalidSharing
238250
}
@@ -256,6 +268,11 @@ func (s *Sharing) RegisterCozyURL(inst *instance.Instance, m *Member, cozyURL st
256268
u.RawQuery = ""
257269
u.Fragment = ""
258270
m.Instance = u.String()
271+
if interactive {
272+
q := u.Query()
273+
q.Set(InteractiveSharingQueryParam, "true")
274+
u.RawQuery = q.Encode()
275+
}
259276

260277
creds := s.FindCredentials(m)
261278
if creds == nil {
@@ -383,6 +400,44 @@ func CreateAccessToken(inst *instance.Instance, cli *oauth.Client, sharingID str
383400
}, nil
384401
}
385402

403+
// updateSharingWithConflictRetry persists the sharing and resolves stale-revision
404+
// conflicts by reconciling the desired changes onto the latest revision.
405+
// reconcile can mutate latest for the next retry and should return true when the
406+
// latest revision already contains a usable final state and no write is needed.
407+
func updateSharingWithConflictRetry(
408+
inst *instance.Instance,
409+
s *Sharing,
410+
retries int,
411+
reconcile func(latest *Sharing) bool,
412+
) error {
413+
log := inst.Logger().WithNamespace("sharing")
414+
current := s
415+
for attempt := 0; ; attempt++ {
416+
err := couchdb.UpdateDoc(inst, current)
417+
if err == nil {
418+
*s = *current
419+
return nil
420+
}
421+
if !couchdb.IsConflictError(err) {
422+
return err
423+
}
424+
425+
latest, findErr := FindSharing(inst, s.SID)
426+
if findErr != nil {
427+
return err
428+
}
429+
if reconcile(latest) {
430+
*s = *latest
431+
return nil
432+
}
433+
if attempt >= retries {
434+
return err
435+
}
436+
log.Debugf("Conflict while updating sharing %s, retrying with latest revision (%d/%d)", s.SID, attempt+1, retries)
437+
current = latest
438+
}
439+
}
440+
386441
// SendAnswer says to the sharer's Cozy that the sharing has been accepted, and
387442
// materialize that by an exchange of credentials.
388443
func (s *Sharing) SendAnswer(inst *instance.Instance, state string) error {
@@ -510,7 +565,19 @@ func (s *Sharing) SendAnswer(inst *instance.Instance, state string) error {
510565
}
511566
}
512567

513-
return couchdb.UpdateDoc(inst, s)
568+
return updateSharingWithConflictRetry(inst, s, 1, func(latest *Sharing) bool {
569+
if latest.Active {
570+
return true
571+
}
572+
latest.Members = s.Members
573+
latest.Credentials = s.Credentials
574+
latest.Active = s.Active
575+
latest.Initial = s.Initial
576+
if s.ShortcutID != "" {
577+
latest.ShortcutID = s.ShortcutID
578+
}
579+
return false
580+
})
514581
}
515582

516583
// ProcessAnswer takes somes credentials and update the sharing with those.
@@ -562,22 +629,20 @@ func (s *Sharing) ProcessAnswer(inst *instance.Instance, creds *APICredentials)
562629
}
563630

564631
s.Active = true
565-
if err := couchdb.UpdateDoc(inst, s); err != nil {
566-
if !couchdb.IsConflictError(err) {
567-
return nil, err
568-
}
569-
// A conflict can occur when several users accept a sharing at
570-
// the same time, and we should just retry in that case
571-
s2, err2 := FindSharing(inst, s.SID)
572-
if err2 != nil {
573-
return nil, err
574-
}
575-
s2.Members[i+1] = s.Members[i+1]
576-
s2.Credentials[i] = s.Credentials[i]
577-
if err2 := couchdb.UpdateDoc(inst, s2); err2 != nil {
578-
return nil, err
632+
if err := updateSharingWithConflictRetry(inst, s, 1, func(latest *Sharing) bool {
633+
if latest.Members[i+1].Status == MemberStatusReady &&
634+
latest.Credentials[i].Client != nil &&
635+
latest.Credentials[i].AccessToken != nil {
636+
ac.Credentials.Client = latest.Credentials[i].Client
637+
ac.Credentials.AccessToken = latest.Credentials[i].AccessToken
638+
return true
579639
}
580-
s = s2
640+
latest.Members[i+1] = s.Members[i+1]
641+
latest.Credentials[i] = s.Credentials[i]
642+
latest.Active = s.Active
643+
return false
644+
}); err != nil {
645+
return nil, err
581646
}
582647
if creds.Bitwarden != nil {
583648
if err := s.SaveBitwarden(inst, &s.Members[i+1], creds.Bitwarden); err != nil {

web/oidc/oidc.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,7 @@ func acceptSharing(c echo.Context, ownerInst *instance.Instance, conf *Config, t
489489
return renderError(c, ownerInst, http.StatusBadRequest, "Sorry, you cannot accept your own sharing invitation.")
490490
}
491491
memberURL := memberInst.PageURL("/", nil)
492-
if err = s.RegisterCozyURL(ownerInst, member, memberURL); err != nil {
492+
if err = s.RegisterCozyURLInteractive(ownerInst, member, memberURL); err != nil {
493493
return renderError(c, ownerInst, http.StatusInternalServerError, "Sorry, the sharing invitation could not be processed.")
494494
}
495495
sharing.PersistInstanceURL(ownerInst, member.Email, member.Instance)

0 commit comments

Comments
 (0)