Skip to content

Commit e91f004

Browse files
committed
fix: avoid replaying delegated sharing conflicts
1 parent de62987 commit e91f004

2 files changed

Lines changed: 37 additions & 1 deletion

File tree

model/sharing/member.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ func (s *Sharing) SendDelegated(inst *instance.Instance, api *APIDelegateAddCont
390390
}
391391
res, err := request.Req(opts)
392392
preRefreshRes, preRefreshErr := res, err
393-
if res != nil && res.StatusCode/100 == 4 {
393+
if shouldRetryDelegatedRequest(res) {
394394
res, err = RefreshToken(inst, res, err, s, &s.Members[0], c, opts, body)
395395
}
396396
if err != nil {
@@ -449,6 +449,20 @@ func (s *Sharing) SendDelegated(inst *instance.Instance, api *APIDelegateAddCont
449449
return s.SendInvitationsToMembers(inst, api.members, states)
450450
}
451451

452+
// shouldRetryDelegatedRequest restricts replay to authentication and instance
453+
// relocation responses. Retrying other client errors can duplicate invitations.
454+
func shouldRetryDelegatedRequest(res *http.Response) bool {
455+
if res == nil {
456+
return false
457+
}
458+
switch res.StatusCode {
459+
case http.StatusUnauthorized, http.StatusForbidden, http.StatusGone:
460+
return true
461+
default:
462+
return false
463+
}
464+
}
465+
452466
func preserveDelegatedResponseError(
453467
res *http.Response,
454468
err error,

model/sharing/member_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,28 @@ import (
2020
"github.com/stretchr/testify/require"
2121
)
2222

23+
func TestShouldRetryDelegatedRequest(t *testing.T) {
24+
tests := []struct {
25+
name string
26+
response *http.Response
27+
expected bool
28+
}{
29+
{name: "missing response"},
30+
{name: "unauthorized", response: &http.Response{StatusCode: http.StatusUnauthorized}, expected: true},
31+
{name: "forbidden", response: &http.Response{StatusCode: http.StatusForbidden}, expected: true},
32+
{name: "moved", response: &http.Response{StatusCode: http.StatusGone}, expected: true},
33+
{name: "bad request", response: &http.Response{StatusCode: http.StatusBadRequest}},
34+
{name: "conflict", response: &http.Response{StatusCode: http.StatusConflict}},
35+
{name: "not found", response: &http.Response{StatusCode: http.StatusNotFound}},
36+
}
37+
38+
for _, tt := range tests {
39+
t.Run(tt.name, func(t *testing.T) {
40+
require.Equal(t, tt.expected, shouldRetryDelegatedRequest(tt.response))
41+
})
42+
}
43+
}
44+
2345
func TestPreserveDelegatedResponseError(t *testing.T) {
2446
t.Run("prefers the response returned after token refresh", func(t *testing.T) {
2547
preRefreshRes := &http.Response{StatusCode: http.StatusUnauthorized}

0 commit comments

Comments
 (0)