Skip to content

Commit 77cebc1

Browse files
Copilotbootjp
andauthored
fix: short-circuit resolveAuth for no-creds/signed requests; fix backward compat test; translate design doc
Agent-Logs-Url: https://github.com/bootjp/elastickv/sessions/7abd0211-57ab-4b16-9583-7f4c77e060cd Co-authored-by: bootjp <1306365+bootjp@users.noreply.github.com>
1 parent 024d4e6 commit 77cebc1

3 files changed

Lines changed: 117 additions & 96 deletions

File tree

adapter/s3.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,10 @@ func (s *S3Server) handle(w http.ResponseWriter, r *http.Request) {
358358
}
359359

360360
func (s *S3Server) resolveAuth(r *http.Request, bucket string) *s3AuthError {
361+
// When no credentials are configured, all requests are permitted without auth.
362+
if len(s.staticCreds) == 0 {
363+
return nil
364+
}
361365
// Write methods always require authentication.
362366
if r.Method != http.MethodGet && r.Method != http.MethodHead {
363367
return s.authorizeRequest(r)
@@ -370,6 +374,11 @@ func (s *S3Server) resolveAuth(r *http.Request, bucket string) *s3AuthError {
370374
if r.URL.Query().Has("acl") {
371375
return s.authorizeRequest(r)
372376
}
377+
// Signed requests (Authorization header or X-Amz-Algorithm presign param) are
378+
// already authenticated; skip the public-bucket metadata read on the hot path.
379+
if r.Header.Get("Authorization") != "" || r.URL.Query().Get("X-Amz-Algorithm") != "" {
380+
return s.authorizeRequest(r)
381+
}
373382
// Check if the bucket is public-read and the request is read-only.
374383
if isReadOnlyS3Request(r) {
375384
readTS := s.readTS()

adapter/s3_test.go

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/bootjp/elastickv/internal/s3keys"
2424
"github.com/bootjp/elastickv/kv"
2525
"github.com/bootjp/elastickv/store"
26+
json "github.com/goccy/go-json"
2627
"github.com/hashicorp/raft"
2728
"github.com/stretchr/testify/require"
2829
)
@@ -1890,30 +1891,40 @@ func TestS3Server_BackwardCompatibility_NoBucketAclFieldIsPrivate(t *testing.T)
18901891
t.Parallel()
18911892

18921893
st := store.NewMVCCStore()
1893-
// No credentials → all requests pass auth, simulating no-auth setup.
1894-
server := NewS3Server(nil, "", st, newLocalAdapterCoordinator(st), nil)
1895-
1896-
// Create bucket without ACL header (pre-existing bucket pattern).
1897-
rec := httptest.NewRecorder()
1898-
req := newS3TestRequest(http.MethodPut, "/legacy-bucket", nil)
1899-
server.handle(rec, req)
1900-
require.Equal(t, http.StatusOK, rec.Code)
1901-
1902-
// Upload object.
1903-
rec = httptest.NewRecorder()
1904-
req = newS3TestRequest(http.MethodPut, "/legacy-bucket/file.txt", strings.NewReader("data"))
1905-
server.handle(rec, req)
1906-
require.Equal(t, http.StatusOK, rec.Code)
1894+
coord := newLocalAdapterCoordinator(st)
1895+
1896+
// Write a legacy bucket meta JSON directly into the store without the "acl"
1897+
// field, simulating a bucket created before the ACL feature was introduced.
1898+
type legacyBucketMeta struct {
1899+
BucketName string `json:"bucket_name"`
1900+
Generation uint64 `json:"generation"`
1901+
CreatedAtHLC uint64 `json:"created_at_hlc"`
1902+
Owner string `json:"owner,omitempty"`
1903+
Region string `json:"region,omitempty"`
1904+
// intentionally omits Acl to replicate the pre-ACL schema
1905+
}
1906+
legacyMeta := legacyBucketMeta{
1907+
BucketName: "legacy-bucket",
1908+
Generation: 1,
1909+
CreatedAtHLC: 1,
1910+
}
1911+
legacyJSON, err := json.Marshal(legacyMeta)
1912+
require.NoError(t, err)
1913+
commitTS := coord.Clock().Next()
1914+
err = st.ApplyMutations(context.Background(), []*store.KVPairMutation{
1915+
{Op: store.OpTypePut, Key: s3keys.BucketMetaKey("legacy-bucket"), Value: legacyJSON},
1916+
}, commitTS-1, commitTS)
1917+
require.NoError(t, err)
19071918

1908-
// Now create a server WITH credentials to test that legacy bucket is private.
1919+
// Create a server WITH credentials; the legacy bucket has no acl field.
19091920
authServer := NewS3Server(
19101921
nil, "", st, newLocalAdapterCoordinator(st), nil,
19111922
WithS3StaticCredentials(map[string]string{testS3AccessKey: testS3SecretKey}),
19121923
)
19131924

1914-
// Anonymous GET should fail on legacy bucket (treated as private).
1915-
rec = httptest.NewRecorder()
1916-
req = newS3TestRequest(http.MethodGet, "/legacy-bucket/file.txt", nil)
1925+
// Anonymous GET should fail on legacy bucket (empty acl field treated as private).
1926+
rec := httptest.NewRecorder()
1927+
req := newS3TestRequest(http.MethodGet, "/legacy-bucket/file.txt", nil)
19171928
authServer.handle(rec, req)
19181929
require.Equal(t, http.StatusForbidden, rec.Code)
19191930
}

0 commit comments

Comments
 (0)