From 293ab78b4cc505c3ab8170a4b2b4b29c04ad74f9 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Wed, 11 Nov 2026 13:39:19 +0800 Subject: [PATCH] fix(auth): authorize tagging replicas without rejecting replication lag Share the remote tag read and authorization path between GET and mutations. Authorize every present replica, fail closed on unreadable targets, and skip missing replicas unless all targets are absent. Gate remote reads on possible caller access and preserve retryable local read failures for DELETE tagging. Signed-off-by: Feng Ruohang --- cmd/auth-handler-unread-tags.go | 18 +++ cmd/auth-handler-unread-tags_test.go | 8 +- cmd/bucket-replication.go | 175 ++++++++++++--------------- cmd/object-handlers.go | 39 +++++- cmd/tagging-proxy-authz_test.go | 63 ++++++++-- 5 files changed, 190 insertions(+), 113 deletions(-) diff --git a/cmd/auth-handler-unread-tags.go b/cmd/auth-handler-unread-tags.go index a2db1bc8b..b3b95f894 100644 --- a/cmd/auth-handler-unread-tags.go +++ b/cmd/auth-handler-unread-tags.go @@ -50,6 +50,24 @@ func authorizeRequestWithUnreadTags(ctx context.Context, r *http.Request, action if s3Err == ErrNone || toAPIError(ctx, readErr).HTTPStatusCode < http.StatusInternalServerError { return s3Err } + return authorizeRequestWithTagCandidates(ctx, r, action, s3Err) +} + +// authorizeRequestWithPotentialTags performs the authorization needed before +// a remote replica can be read. An empty tag set is enough to reject a caller +// with no possible access, but a caller whose Allow depends on existing tags +// must be admitted so the remote copy can supply the authoritative tags. +// The returned decision is only a gate; the actual tags must be authorized +// again after they are read. +func authorizeRequestWithPotentialTags(ctx context.Context, r *http.Request, action policy.Action) APIErrorCode { + s3Err := authorizeRequest(ctx, r, action) + if s3Err == ErrNone { + return ErrNone + } + return authorizeRequestWithTagCandidates(ctx, r, action, s3Err) +} + +func authorizeRequestWithTagCandidates(ctx context.Context, r *http.Request, action policy.Action, s3Err APIErrorCode) APIErrorCode { for _, existingTags := range policyExistingObjectTags(ctx, r, action) { if authorizeRequestWithExistingTags(ctx, r, action, existingTags) == ErrNone { return ErrNone diff --git a/cmd/auth-handler-unread-tags_test.go b/cmd/auth-handler-unread-tags_test.go index aa3f1e822..2a4a4af4d 100644 --- a/cmd/auth-handler-unread-tags_test.go +++ b/cmd/auth-handler-unread-tags_test.go @@ -87,7 +87,7 @@ func TestExistingObjectTagAuthReadQuorumLost(t *testing.T) { } statement := func(effect, resource, conditions string) string { - st := fmt.Sprintf(`{"Effect": %q, "Action": ["s3:GetObject", "s3:PutObjectTagging"], "Resource": ["arn:aws:s3:::%s/%s"]`, + st := fmt.Sprintf(`{"Effect": %q, "Action": ["s3:GetObject", "s3:GetObjectTagging", "s3:PutObjectTagging", "s3:DeleteObjectTagging"], "Resource": ["arn:aws:s3:::%s/%s"]`, effect, bucket, resource) if conditions != "" { st += `, "Condition": ` + conditions @@ -263,6 +263,8 @@ func TestExistingObjectTagAuthReadQuorumLost(t *testing.T) { taggingURL := makeTestTargetURL("", bucket, object, url.Values{"tagging": []string{""}}) taggingBody := []byte(`kv`) putTagging := request{"PutObjectTagging", http.MethodPut, taggingURL, nil, taggingBody} + getTagging := request{"GetObjectTagging", http.MethodGet, taggingURL, nil, nil} + deleteTagging := request{"DeleteObjectTagging", http.MethodDelete, taggingURL, nil, nil} check := func(stage string, creds auth.Credentials, name string, req request, want int) { t.Helper() var rec *httptest.ResponseRecorder @@ -302,6 +304,7 @@ func TestExistingObjectTagAuthReadQuorumLost(t *testing.T) { var anonymous auth.Credentials for _, u := range users { check("healthy", u.creds, u.name, get, u.healthy) + check("healthy", u.creds, u.name, getTagging, u.healthy) } check("healthy", anonymous, "anonymous", get, http.StatusOK) // The tags were read and allow the request: the range error is the answer. @@ -312,12 +315,13 @@ func TestExistingObjectTagAuthReadQuorumLost(t *testing.T) { missingURL := getGetObjectURL("", bucket, missing) check("missing key", users[0].creds, users[0].name, request{"GET", http.MethodGet, missingURL, nil, nil}, http.StatusForbidden) check("missing key", users[0].creds, users[0].name, request{"HEAD", http.MethodHead, missingURL, nil, nil}, http.StatusForbidden) + check("missing key", users[0].creds, users[0].name, request{"GetObjectTagging", http.MethodGet, missingURL + "?tagging", nil, nil}, http.StatusForbidden) defer objectReadFault(z, bucket, object, 10)() if _, err := z.GetObjectInfo(ctx, bucket, object, ObjectOptions{}); !isErrReadQuorum(err) { t.Fatalf("GetObjectInfo with 10 of 16 drives failing: %v, want InsufficientReadQuorum", err) } - for _, req := range []request{get, head, putTagging} { + for _, req := range []request{get, head, getTagging, putTagging, deleteTagging} { for _, u := range users { check("read quorum lost", u.creds, u.name, req, u.quorumLost) } diff --git a/cmd/bucket-replication.go b/cmd/bucket-replication.go index 81099f504..fb942d2c0 100644 --- a/cmd/bucket-replication.go +++ b/cmd/bucket-replication.go @@ -2773,67 +2773,107 @@ func scheduleReplication(ctx context.Context, oi ObjectInfo, o ObjectLayer, dsc } } -// proxyTaggingToRepTarget proxies tagging requests to remote targets for -// active-active replicated setups -func proxyTaggingToRepTarget(ctx context.Context, bucket, object string, tags *tags.Tags, opts ObjectOptions, proxyTargets *madmin.BucketTargets, authorize func(string, error) APIErrorCode) (proxy proxyResult, authErr APIErrorCode) { - if opts.ProxyRequest || (opts.ProxyHeaderSet && !opts.ProxyRequest) { - return proxy, ErrNone - } - // Keep the exact clients whose addressed versions we read. A second - // discovery could include a newly available, unauthorized destination. - var targets []*TargetClient +// taggingProxyTarget keeps the exact client and tags that were authorized. +// A second discovery must not introduce an unauthorized destination. +type taggingProxyTarget struct { + client *TargetClient + tags *tags.Tags +} + +// readTaggingProxyTargets authorizes all present replicas before returning any +// tags or allowing any mutation. Missing replicas may simply lag replication; +// unreadable replicas must still fail closed. +func readTaggingProxyTargets(ctx context.Context, bucket, object string, opts ObjectOptions, proxyTargets *madmin.BucketTargets, authorize func(string, error) APIErrorCode) ([]taggingProxyTarget, proxyResult, APIErrorCode) { + var targets []taggingProxyTarget for _, target := range proxyTargets.Targets { tgt := globalBucketTargetSys.GetRemoteTargetClient(bucket, target.Arn) if tgt == nil || globalBucketTargetSys.isOffline(tgt.EndpointURL()) || tgt.disableProxy { continue } - targets = append(targets, tgt) + targets = append(targets, taggingProxyTarget{client: tgt}) } if len(targets) == 0 { err := RemoteTargetConnectionErr{Bucket: bucket, Err: errors.New("no available remote replication target")} - return proxyResult{Err: err}, authorize("", err) + return nil, proxyResult{Err: err}, authorize("", err) } var wg sync.WaitGroup errs := make([]error, len(targets)) - existingTags := make([]string, len(targets)) - for i, tgt := range targets { + for i := range targets { wg.Go(func() { - remoteTags, err := tgt.GetObjectTagging(ctx, tgt.Bucket, object, minio.GetObjectTaggingOptions{ + tgt := &targets[i] + tgt.tags, errs[i] = tgt.client.GetObjectTagging(ctx, tgt.client.Bucket, object, minio.GetObjectTaggingOptions{ VersionID: opts.VersionID, Internal: minio.AdvancedObjectTaggingOptions{ReplicationProxyRequest: "true"}, }) - errs[i] = err - if remoteTags != nil { - existingTags[i] = remoteTags.String() - } }) } wg.Wait() - // Authorize every contacted target before any mutation. A tag-based deny - // on one copy must not be hidden by a more permissive copy on another. - var readErr error - for i, err := range errs { - if code := authorize(existingTags[i], err); code != ErrNone { - return proxy, code + + var firstNotFound, readErr error + for _, err := range errs { + switch { + case err == nil: + case isRemoteTaggingNotFound(err): + if firstNotFound == nil { + firstNotFound = err + } + default: + if code := authorize("", err); code != ErrNone { + return nil, proxyResult{}, code + } + if readErr == nil { + readErr = err + } } - if err != nil && !isRemoteTaggingNotFound(err) && readErr == nil { - readErr = err + } + present := make([]taggingProxyTarget, 0, len(targets)) + for i, tgt := range targets { + if errs[i] != nil { + continue } + if tgt.tags == nil { + tgt.tags, _ = tags.MapToObjectTags(nil) + } + if code := authorize(tgt.tags.String(), nil); code != ErrNone { + return nil, proxyResult{}, code + } + present = append(present, tgt) } if readErr != nil { - return proxyResult{Err: readErr}, ErrNone + return nil, proxyResult{Err: readErr}, ErrNone + } + if len(present) == 0 { + // With no object anywhere, authorize the empty tag set once before + // disclosing its absence. No write RPC can help a missing replica. + return nil, proxyResult{Err: firstNotFound}, authorize("", firstNotFound) + } + return present, proxyResult{}, ErrNone +} + +// proxyTaggingToRepTarget proxies tagging requests to remote targets for +// active-active replicated setups. +func proxyTaggingToRepTarget(ctx context.Context, bucket, object string, tags *tags.Tags, opts ObjectOptions, proxyTargets *madmin.BucketTargets, authorize func(string, error) APIErrorCode) (proxy proxyResult, authErr APIErrorCode) { + if opts.ProxyRequest || (opts.ProxyHeaderSet && !opts.ProxyRequest) { + return proxy, ErrNone + } + targets, proxy, authErr := readTaggingProxyTargets(ctx, bucket, object, opts, proxyTargets, authorize) + if authErr != ErrNone || proxy.Err != nil { + return proxy, authErr } - for i, tgt := range targets { + var wg sync.WaitGroup + writeErrs := make([]error, len(targets)) + for i, target := range targets { wg.Go(func() { + tgt := target.client if tags != nil { - errs[i] = tgt.PutObjectTagging(ctx, tgt.Bucket, object, tags, minio.PutObjectTaggingOptions{ + writeErrs[i] = tgt.PutObjectTagging(ctx, tgt.Bucket, object, tags, minio.PutObjectTaggingOptions{ VersionID: opts.VersionID, Internal: minio.AdvancedObjectTaggingOptions{ReplicationProxyRequest: "true"}, }) } else { - errs[i] = tgt.RemoveObjectTagging(ctx, tgt.Bucket, object, minio.RemoveObjectTaggingOptions{ + writeErrs[i] = tgt.RemoveObjectTagging(ctx, tgt.Bucket, object, minio.RemoveObjectTaggingOptions{ VersionID: opts.VersionID, Internal: minio.AdvancedObjectTaggingOptions{ReplicationProxyRequest: "true"}, }) @@ -2841,9 +2881,8 @@ func proxyTaggingToRepTarget(ctx context.Context, bucket, object string, tags *t }) } wg.Wait() - // Preserve partial success, counting only targets actually contacted. var lastErr error - for _, err := range errs { + for _, err := range writeErrs { if err == nil { return proxy, ErrNone } @@ -2853,72 +2892,16 @@ func proxyTaggingToRepTarget(ctx context.Context, bucket, object string, tags *t } // proxyGetTaggingToRepTarget proxies get tagging requests to remote targets for -// active-active replicated setups -func proxyGetTaggingToRepTarget(ctx context.Context, bucket, object string, opts ObjectOptions, proxyTargets *madmin.BucketTargets) (tgs *tags.Tags, proxy proxyResult) { - // this option is set when active-active replication is in place between site A -> B, - // and request hits site B that does not have the object yet. - if opts.ProxyRequest || (opts.ProxyHeaderSet && !opts.ProxyRequest) { // true only when site B sets MinIOSourceProxyRequest header - return nil, proxy +// active-active replicated setups. +func proxyGetTaggingToRepTarget(ctx context.Context, bucket, object string, opts ObjectOptions, proxyTargets *madmin.BucketTargets, authorize func(string, error) APIErrorCode) (tgs *tags.Tags, proxy proxyResult, authErr APIErrorCode) { + if opts.ProxyRequest || (opts.ProxyHeaderSet && !opts.ProxyRequest) { + return nil, proxy, ErrNone } - var wg sync.WaitGroup - errs := make([]error, len(proxyTargets.Targets)) - tagSlc := make([]map[string]string, len(proxyTargets.Targets)) - contacted := 0 - for idx, t := range proxyTargets.Targets { - tgt := globalBucketTargetSys.GetRemoteTargetClient(bucket, t.Arn) - if tgt == nil || globalBucketTargetSys.isOffline(tgt.EndpointURL()) { - continue - } - // if proxying explicitly disabled on remote target - if tgt.disableProxy { - continue - } - contacted++ - idx := idx - wg.Add(1) - go func(idx int, tgt *TargetClient) { - defer wg.Done() - gopts := minio.GetObjectTaggingOptions{ - VersionID: opts.VersionID, - Internal: minio.AdvancedObjectTaggingOptions{ - ReplicationProxyRequest: "true", - }, - } - remoteTags, err := tgt.GetObjectTagging(ctx, tgt.Bucket, object, gopts) - if err != nil { - errs[idx] = err - } else if remoteTags != nil { - tagSlc[idx] = remoteTags.ToMap() - } - }(idx, tgt) + targets, proxy, authErr := readTaggingProxyTargets(ctx, bucket, object, opts, proxyTargets, authorize) + if authErr != ErrNone || proxy.Err != nil { + return nil, proxy, authErr } - wg.Wait() - if contacted == 0 { - return nil, proxyResult{Err: RemoteTargetConnectionErr{ - Bucket: bucket, - Err: errors.New("no available remote replication target"), - }} - } - var firstNotFound error - for idx, err := range errs { - errCode := minio.ToErrorResponse(err).Code - if err != nil { - if errCode != "NoSuchKey" && errCode != "NoSuchVersion" { - return nil, proxyResult{Err: err} - } - if firstNotFound == nil { - firstNotFound = err - } - continue - } - if tagSlc[idx] != nil && tgs == nil { - tgs, _ = tags.MapToObjectTags(tagSlc[idx]) - } - } - if tgs == nil { - proxy.Err = firstNotFound - } - return tgs, proxy + return targets[0].tags, proxy, ErrNone } func scheduleReplicationDelete(ctx context.Context, dv DeletedObjectReplicationInfo, o ObjectLayer) { diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index 13c122f62..e848a5ce4 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -3726,9 +3726,22 @@ func (api objectAPIHandlers) GetObjectTaggingHandler(w http.ResponseWriter, r *h if isErrObjectNotFound(err) || isErrVersionNotFound(err) { proxytgts := getProxyTargets(ctx, bucket, object, opts) if !proxytgts.Empty() { + if s3Error := authorizeRequestWithPotentialTags(ctx, r, policy.GetObjectTaggingAction); s3Error != ErrNone { + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL) + return + } globalReplicationStats.Load().incProxy(bucket, getObjectTaggingAPI, false) // proxy to replication target if site replication is in place. - tags, gerr := proxyGetTaggingToRepTarget(ctx, bucket, object, opts, proxytgts) + tags, gerr, s3Error := proxyGetTaggingToRepTarget(ctx, bucket, object, opts, proxytgts, func(existingTags string, readErr error) APIErrorCode { + if readErr != nil && !isRemoteTaggingNotFound(readErr) { + return authorizeRequestWithUnreadTags(ctx, r, policy.GetObjectTaggingAction, readErr) + } + return authorizeRequestWithExistingTags(ctx, r, policy.GetObjectTaggingAction, existingTags) + }) + if s3Error != ErrNone { + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL) + return + } if gerr.Err != nil || tags == nil { globalReplicationStats.Load().incProxy(bucket, getObjectTaggingAPI, true) writeErrorResponse(ctx, w, toAPIError(ctx, gerr.Err), r.URL) @@ -3737,10 +3750,18 @@ func (api objectAPIHandlers) GetObjectTaggingHandler(w http.ResponseWriter, r *h ot = tags w.Header()[xhttp.MinIOTaggingProxied] = []string{"true"} // indicate that the request was proxied. } else { + if s3Error := authorizeRequestWithExistingTags(ctx, r, policy.GetObjectTaggingAction, ""); s3Error != ErrNone { + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL) + return + } writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL) return } } else { + if s3Error := authorizeRequestWithUnreadTags(ctx, r, policy.GetObjectTaggingAction, err); s3Error != ErrNone { + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL) + return + } writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL) return } @@ -3841,6 +3862,10 @@ func (api objectAPIHandlers) PutObjectTaggingHandler(w http.ResponseWriter, r *h if isErrObjectNotFound(err) || isErrVersionNotFound(err) { proxytgts := getProxyTargets(ctx, bucket, object, opts) if !proxytgts.Empty() { + if s3Error := authorizeRequestWithPotentialTags(ctx, r, policy.PutObjectTaggingAction); s3Error != ErrNone { + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL) + return + } perr, s3Error := proxyTaggingToRepTarget(ctx, bucket, object, tags, opts, proxytgts, func(existingTags string, readErr error) APIErrorCode { if readErr != nil && !isRemoteTaggingNotFound(readErr) { return authorizeRequestWithUnreadTags(ctx, r, policy.PutObjectTaggingAction, readErr) @@ -3969,6 +3994,10 @@ func (api objectAPIHandlers) DeleteObjectTaggingHandler(w http.ResponseWriter, r if isErrObjectNotFound(err) || isErrVersionNotFound(err) { proxytgts := getProxyTargets(ctx, bucket, object, opts) if !proxytgts.Empty() { + if s3Error := authorizeRequestWithPotentialTags(ctx, r, policy.DeleteObjectTaggingAction); s3Error != ErrNone { + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL) + return + } perr, s3Error := proxyTaggingToRepTarget(ctx, bucket, object, nil, opts, proxytgts, func(existingTags string, readErr error) APIErrorCode { if readErr != nil && !isRemoteTaggingNotFound(readErr) { return authorizeRequestWithUnreadTags(ctx, r, policy.DeleteObjectTaggingAction, readErr) @@ -4001,7 +4030,13 @@ func (api objectAPIHandlers) DeleteObjectTaggingHandler(w http.ResponseWriter, r return } } - if s3Error := authorizeRequestWithExistingTags(ctx, r, policy.DeleteObjectTaggingAction, ""); s3Error != ErrNone { + var s3Error APIErrorCode + if isErrObjectNotFound(err) || isErrVersionNotFound(err) { + s3Error = authorizeRequestWithExistingTags(ctx, r, policy.DeleteObjectTaggingAction, "") + } else { + s3Error = authorizeRequestWithUnreadTags(ctx, r, policy.DeleteObjectTaggingAction, err) + } + if s3Error != ErrNone { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL) return } diff --git a/cmd/tagging-proxy-authz_test.go b/cmd/tagging-proxy-authz_test.go index 0e0f0eb77..e50fc8343 100644 --- a/cmd/tagging-proxy-authz_test.go +++ b/cmd/tagging-proxy-authz_test.go @@ -33,11 +33,18 @@ import ( func TestAPITaggingProxyAuthorization(t *testing.T) { ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{t: t, objAPITest: func(obj ObjectLayer, backend, bucket string, router http.Handler, root auth.Credentials, t *testing.T) { conditional := newDeleteAuthzPolicyUser(t, backend, bucket, fmt.Sprintf(`[ - {"Effect":"Allow","Action":["s3:PutObjectTagging","s3:DeleteObjectTagging"],"Resource":["arn:aws:s3:::%s/*"]}, - {"Effect":"Deny","Action":["s3:PutObjectTagging","s3:DeleteObjectTagging"],"Resource":["arn:aws:s3:::%s/*"],"Condition":{"StringEquals":{"s3:ExistingObjectTag/secret":"true"}}} + {"Effect":"Allow","Action":["s3:GetObjectTagging","s3:PutObjectTagging","s3:DeleteObjectTagging"],"Resource":["arn:aws:s3:::%s/*"]}, + {"Effect":"Deny","Action":["s3:GetObjectTagging","s3:PutObjectTagging","s3:DeleteObjectTagging"],"Resource":["arn:aws:s3:::%s/*"],"Condition":{"StringEquals":{"s3:ExistingObjectTag/secret":"true"}}} ]`, bucket, bucket)) + tagAllow := newDeleteAuthzPolicyUser(t, backend, bucket, fmt.Sprintf(`[ + {"Effect":"Allow","Action":["s3:GetObjectTagging","s3:PutObjectTagging","s3:DeleteObjectTagging"],"Resource":["arn:aws:s3:::%s/*"],"Condition":{"StringEquals":{"s3:ExistingObjectTag/secret":"false"}}} + ]`, bucket)) + denied := newDeleteAuthzPolicyUser(t, backend, bucket, fmt.Sprintf(`[ + {"Effect":"Deny","Action":["s3:*"],"Resource":["arn:aws:s3:::%s/*"]} + ]`, bucket)) + zero := iamAdminUser(t.Context(), t, globalIAMSys, "tagging-no-policy-"+mustGetUUID(), "") versionID := mustGetUUID() - for _, method := range []string{http.MethodPut, http.MethodDelete} { + for _, method := range []string{http.MethodGet, http.MethodPut, http.MethodDelete} { for _, tc := range []struct { name string tags []string @@ -45,17 +52,32 @@ func TestAPITaggingProxyAuthorization(t *testing.T) { readStatus, writeStatus, wantStatus int wantWrites int32 disabledFirst bool + firstReadStatus int + noReads bool }{ {name: "deny-second-target", tags: []string{"false", "true"}, cred: conditional, wantStatus: http.StatusForbidden}, {name: "deny-first-target", tags: []string{"true", "false"}, cred: conditional, wantStatus: http.StatusForbidden}, {name: "all-authorized", tags: []string{"false", "false"}, cred: conditional, wantWrites: 2}, - {name: "anonymous", tags: []string{"false"}, wantStatus: http.StatusForbidden}, - {name: "invalid-credentials", tags: []string{"false"}, cred: auth.Credentials{AccessKey: root.AccessKey, SecretKey: "wrong-secret-key"}, wantStatus: http.StatusForbidden}, + {name: "anonymous", tags: []string{"false"}, wantStatus: http.StatusForbidden, noReads: true}, + {name: "invalid-credentials", tags: []string{"false"}, cred: auth.Credentials{AccessKey: root.AccessKey, SecretKey: "wrong-secret-key"}, wantStatus: http.StatusForbidden, noReads: true}, + {name: "zero-permissions", tags: []string{"false"}, cred: zero, wantStatus: http.StatusForbidden, noReads: true}, + {name: "zero-permissions-missing", tags: []string{"false"}, cred: zero, readStatus: http.StatusNotFound, wantStatus: http.StatusForbidden, noReads: true}, + {name: "unconditional-deny", tags: []string{"false"}, cred: denied, wantStatus: http.StatusForbidden, noReads: true}, {name: "unreadable", tags: []string{"false"}, cred: root, readStatus: http.StatusServiceUnavailable, wantStatus: http.StatusServiceUnavailable}, - {name: "missing-version", tags: []string{"false"}, cred: root, readStatus: http.StatusNotFound, writeStatus: http.StatusNotFound, wantStatus: http.StatusNotFound, wantWrites: 1}, + {name: "missing-version", tags: []string{"false", "false"}, cred: root, readStatus: http.StatusNotFound, wantStatus: http.StatusNotFound}, + {name: "tag-allow-missing-version", tags: []string{"false"}, cred: tagAllow, readStatus: http.StatusNotFound, wantStatus: http.StatusForbidden}, + {name: "tag-allow-lagging-first", tags: []string{"false", "false"}, cred: tagAllow, firstReadStatus: http.StatusNotFound, wantWrites: 1}, + {name: "tag-allow-lagging-second", tags: []string{"false", "false"}, cred: tagAllow, firstReadStatus: http.StatusOK, readStatus: http.StatusNotFound, wantWrites: 1}, + {name: "tag-allow-present-untagged", tags: []string{"false", ""}, cred: tagAllow, wantStatus: http.StatusForbidden}, + {name: "tag-allow-unreadable-first", tags: []string{"false", "false"}, cred: tagAllow, firstReadStatus: http.StatusServiceUnavailable, wantStatus: http.StatusServiceUnavailable}, + {name: "tag-allow-unreadable-second", tags: []string{"false", "false"}, cred: tagAllow, firstReadStatus: http.StatusOK, readStatus: http.StatusServiceUnavailable, wantStatus: http.StatusServiceUnavailable}, {name: "disabled-target", tags: []string{"true", "false"}, cred: conditional, disabledFirst: true, wantWrites: 1}, + {name: "no-available-target", tags: []string{"false"}, cred: tagAllow, disabledFirst: true, wantStatus: http.StatusServiceUnavailable, noReads: true}, {name: "skipped-target-cannot-mask-failure", tags: []string{"false", "false"}, cred: root, disabledFirst: true, writeStatus: http.StatusServiceUnavailable, wantStatus: http.StatusServiceUnavailable, wantWrites: 1}, } { + if method == http.MethodGet && tc.writeStatus != 0 { + continue // This case only exercises a remote mutation failure. + } t.Run(backend+"/"+method+"/"+tc.name, func(t *testing.T) { var reads, writes atomic.Int32 globalBucketTargetSys.Lock() @@ -63,6 +85,10 @@ func TestAPITaggingProxyAuthorization(t *testing.T) { globalBucketTargetSys.Unlock() cfg := replication.Config{} for i, value := range tc.tags { + readStatus := tc.readStatus + if i == 0 && tc.firstReadStatus != 0 { + readStatus = tc.firstReadStatus + } target := newILMTestTarget(t) registerILMTestTarget(t, obj, bucket, target, func(w http.ResponseWriter, r *http.Request) { if r.URL.Query().Get("versionId") != versionID || !r.URL.Query().Has("tagging") || r.Header.Get(xhttp.MinIOSourceProxyRequest) != "true" { @@ -71,12 +97,15 @@ func TestAPITaggingProxyAuthorization(t *testing.T) { status := tc.writeStatus if r.Method == http.MethodGet { reads.Add(1) - status = tc.readStatus + status = readStatus } else { writes.Add(1) + if readStatus == http.StatusNotFound { + t.Error("tagging write reached a missing replica") + } } w.Header().Set("Content-Type", "application/xml") - if status != 0 { + if status != 0 && status != http.StatusOK { w.WriteHeader(status) code := "ServiceUnavailable" if status == http.StatusNotFound { @@ -87,7 +116,11 @@ func TestAPITaggingProxyAuthorization(t *testing.T) { } switch r.Method { case http.MethodGet: - fmt.Fprintf(w, "secret%s", value) + if value == "" { + fmt.Fprint(w, "") + } else { + fmt.Fprintf(w, "secret%s", value) + } case http.MethodDelete: w.WriteHeader(http.StatusNoContent) } @@ -130,11 +163,15 @@ func TestAPITaggingProxyAuthorization(t *testing.T) { wantStatus = http.StatusNoContent } } - if rec.Code != wantStatus || writes.Load() != tc.wantWrites { - t.Errorf("status=%d writes=%d, want status=%d writes=%d: %s", rec.Code, writes.Load(), wantStatus, tc.wantWrites, rec.Body.String()) + wantWrites := tc.wantWrites + if method == http.MethodGet { + wantWrites = 0 } - if tc.name == "invalid-credentials" && reads.Load() != 0 { - t.Errorf("invalid credentials reached %d remote reads", reads.Load()) + if rec.Code != wantStatus || writes.Load() != wantWrites { + t.Errorf("status=%d writes=%d, want status=%d writes=%d: %s", rec.Code, writes.Load(), wantStatus, wantWrites, rec.Body.String()) + } + if tc.noReads && reads.Load() != 0 { + t.Errorf("unauthorized request reached %d remote reads", reads.Load()) } }) }