From cf7254f8cdea348ee1dc4be7f841e2cbb2ee877c Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Thu, 5 Oct 2023 11:05:21 +0300 Subject: [PATCH] [#260] Refactor api/auth/center.go Move the Center interface to middleware package where it's used Signed-off-by: Denis Kirillov --- api/auth/center.go | 61 +++++++++++++--------------------------- api/auth/center_test.go | 2 +- api/auth/presign_test.go | 2 +- api/handler/put_test.go | 4 +-- api/middleware/auth.go | 34 ++++++++++++++++++++-- api/middleware/util.go | 7 ++--- api/router.go | 3 +- api/router_mock_test.go | 5 ++-- cmd/s3-gw/app.go | 3 +- 9 files changed, 61 insertions(+), 60 deletions(-) diff --git a/api/auth/center.go b/api/auth/center.go index c131424..5a2d424 100644 --- a/api/auth/center.go +++ b/api/auth/center.go @@ -5,7 +5,6 @@ import ( "crypto/hmac" "crypto/sha256" "encoding/hex" - "errors" "fmt" "io" "mime/multipart" @@ -17,6 +16,7 @@ import ( v4 "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/auth/signer/v4" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/cache" apiErrors "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors" + "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/middleware" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/creds/accessbox" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/creds/tokens" oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" @@ -31,27 +31,13 @@ var authorizationFieldRegexp = regexp.MustCompile(`AWS4-HMAC-SHA256 Credential=( var postPolicyCredentialRegexp = regexp.MustCompile(`(?P[^/]+)/(?P[^/]+)/(?P[^/]*)/(?P[^/]+)/aws4_request`) type ( - // Center is a user authentication interface. - Center interface { - Authenticate(request *http.Request) (*Box, error) - } - - // Box contains access box and additional info. - Box struct { - AccessBox *accessbox.Box - ClientTime time.Time - AuthHeaders *AuthHeader - } - - center struct { + Center struct { reg *RegexpSubmatcher postReg *RegexpSubmatcher cli tokens.Credentials allowedAccessKeyIDPrefixes []string // empty slice means all access key ids are allowed } - prs int - //nolint:revive AuthHeader struct { AccessKeyID string @@ -97,22 +83,9 @@ var ContentSHA256HeaderStandardValue = map[string]struct{}{ StreamingContentECDSASHA256Trailer: {}, } -// ErrNoAuthorizationHeader is returned for unauthenticated requests. -var ErrNoAuthorizationHeader = errors.New("no authorization header") - -func (p prs) Read(_ []byte) (n int, err error) { - panic("implement me") -} - -func (p prs) Seek(_ int64, _ int) (int64, error) { - panic("implement me") -} - -var _ io.ReadSeeker = prs(0) - // New creates an instance of AuthCenter. -func New(frostFS tokens.FrostFS, key *keys.PrivateKey, prefixes []string, config *cache.Config) Center { - return ¢er{ +func New(frostFS tokens.FrostFS, key *keys.PrivateKey, prefixes []string, config *cache.Config) *Center { + return &Center{ cli: tokens.New(frostFS, key, config), reg: NewRegexpMatcher(authorizationFieldRegexp), postReg: NewRegexpMatcher(postPolicyCredentialRegexp), @@ -120,7 +93,7 @@ func New(frostFS tokens.FrostFS, key *keys.PrivateKey, prefixes []string, config } } -func (c *center) parseAuthHeader(header string) (*AuthHeader, error) { +func (c *Center) parseAuthHeader(header string) (*AuthHeader, error) { submatches := c.reg.GetSubmatches(header) if len(submatches) != authHeaderPartsNum { return nil, apiErrors.GetAPIError(apiErrors.ErrAuthorizationHeaderMalformed) @@ -156,7 +129,7 @@ func IsStandardContentSHA256(key string) bool { return ok } -func (c *center) Authenticate(r *http.Request) (*Box, error) { +func (c *Center) Authenticate(r *http.Request) (*middleware.Box, error) { var ( err error authHdr *AuthHeader @@ -190,7 +163,7 @@ func (c *center) Authenticate(r *http.Request) (*Box, error) { if strings.HasPrefix(r.Header.Get(ContentTypeHdr), "multipart/form-data") { return c.checkFormData(r) } - return nil, ErrNoAuthorizationHeader + return nil, middleware.ErrNoAuthorizationHeader } authHdr, err = c.parseAuthHeader(authHeaderField[0]) if err != nil { @@ -228,9 +201,13 @@ func (c *center) Authenticate(r *http.Request) (*Box, error) { return nil, err } - result := &Box{ - AccessBox: box, - AuthHeaders: authHdr, + result := &middleware.Box{ + AccessBox: box, + AuthHeaders: &middleware.AuthHeader{ + AccessKeyID: authHdr.AccessKeyID, + Region: authHdr.Region, + SignatureV4: authHdr.SignatureV4, + }, } if needClientTime { result.ClientTime = signatureDateTime @@ -253,7 +230,7 @@ func checkFormatHashContentSHA256(hash string) error { return nil } -func (c center) checkAccessKeyID(accessKeyID string) error { +func (c Center) checkAccessKeyID(accessKeyID string) error { if len(c.allowedAccessKeyIDPrefixes) == 0 { return nil } @@ -267,7 +244,7 @@ func (c center) checkAccessKeyID(accessKeyID string) error { return apiErrors.GetAPIError(apiErrors.ErrAccessDenied) } -func (c *center) checkFormData(r *http.Request) (*Box, error) { +func (c *Center) checkFormData(r *http.Request) (*middleware.Box, error) { if err := r.ParseMultipartForm(maxFormSizeMemory); err != nil { return nil, apiErrors.GetAPIError(apiErrors.ErrInvalidArgument) } @@ -278,7 +255,7 @@ func (c *center) checkFormData(r *http.Request) (*Box, error) { policy := MultipartFormValue(r, "policy") if policy == "" { - return nil, ErrNoAuthorizationHeader + return nil, middleware.ErrNoAuthorizationHeader } submatches := c.postReg.GetSubmatches(MultipartFormValue(r, "x-amz-credential")) @@ -309,7 +286,7 @@ func (c *center) checkFormData(r *http.Request) (*Box, error) { return nil, apiErrors.GetAPIError(apiErrors.ErrSignatureDoesNotMatch) } - return &Box{AccessBox: box}, nil + return &middleware.Box{AccessBox: box}, nil } func cloneRequest(r *http.Request, authHeader *AuthHeader) *http.Request { @@ -333,7 +310,7 @@ func cloneRequest(r *http.Request, authHeader *AuthHeader) *http.Request { return otherRequest } -func (c *center) checkSign(authHeader *AuthHeader, box *accessbox.Box, request *http.Request, signatureDateTime time.Time) error { +func (c *Center) checkSign(authHeader *AuthHeader, box *accessbox.Box, request *http.Request, signatureDateTime time.Time) error { awsCreds := credentials.NewStaticCredentials(authHeader.AccessKeyID, box.Gate.SecretKey, "") signer := v4.NewSigner(awsCreds) signer.DisableURIPathEscaping = true diff --git a/api/auth/center_test.go b/api/auth/center_test.go index 3097325..69ef9d4 100644 --- a/api/auth/center_test.go +++ b/api/auth/center_test.go @@ -12,7 +12,7 @@ import ( func TestAuthHeaderParse(t *testing.T) { defaultHeader := "AWS4-HMAC-SHA256 Credential=oid0cid/20210809/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=2811ccb9e242f41426738fb1f" - center := ¢er{ + center := &Center{ reg: NewRegexpMatcher(authorizationFieldRegexp), } diff --git a/api/auth/presign_test.go b/api/auth/presign_test.go index 188456a..36eb97c 100644 --- a/api/auth/presign_test.go +++ b/api/auth/presign_test.go @@ -84,7 +84,7 @@ func TestCheckSign(t *testing.T) { mock := newTokensFrostfsMock() mock.addBox(accessKeyAddr, expBox) - c := ¢er{ + c := &Center{ cli: mock, reg: NewRegexpMatcher(authorizationFieldRegexp), postReg: NewRegexpMatcher(postPolicyCredentialRegexp), diff --git a/api/handler/put_test.go b/api/handler/put_test.go index 6f3a9c0..345242e 100644 --- a/api/handler/put_test.go +++ b/api/handler/put_test.go @@ -17,7 +17,6 @@ import ( "time" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api" - "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/auth" v4 "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/auth/signer/v4" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/data" s3errors "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors" @@ -355,10 +354,9 @@ func getChunkedRequest(ctx context.Context, t *testing.T, bktName, objName strin reqInfo := middleware.NewReqInfo(w, req, middleware.ObjectRequest{Bucket: bktName, Object: objName}) req = req.WithContext(middleware.SetReqInfo(ctx, reqInfo)) req = req.WithContext(middleware.SetClientTime(req.Context(), signTime)) - req = req.WithContext(middleware.SetAuthHeaders(req.Context(), &auth.AuthHeader{ + req = req.WithContext(middleware.SetAuthHeaders(req.Context(), &middleware.AuthHeader{ AccessKeyID: AWSAccessKeyID, SignatureV4: "4f232c4386841ef735655705268965c44a0e4690baa4adea153f7db9fa80a0a9", - Service: "s3", Region: "us-east-1", })) req = req.WithContext(middleware.SetBoxData(req.Context(), &accessbox.Box{ diff --git a/api/middleware/auth.go b/api/middleware/auth.go index 8e079c1..852823b 100644 --- a/api/middleware/auth.go +++ b/api/middleware/auth.go @@ -1,21 +1,49 @@ package middleware import ( + stderrors "errors" "net/http" + "time" - "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/auth" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors" + "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/creds/accessbox" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/logs" "go.uber.org/zap" ) -func Auth(center auth.Center, log *zap.Logger) Func { +type ( + // Box contains access box and additional info. + Box struct { + AccessBox *accessbox.Box + ClientTime time.Time + AuthHeaders *AuthHeader + } + + // Center is a user authentication interface. + Center interface { + // Authenticate validate and authenticate request. + // Must return ErrNoAuthorizationHeader if auth header is missed. + Authenticate(request *http.Request) (*Box, error) + } + + //nolint:revive + AuthHeader struct { + AccessKeyID string + Region string + SignatureV4 string + } +) + +// ErrNoAuthorizationHeader is returned for unauthenticated requests. +var ErrNoAuthorizationHeader = stderrors.New("no authorization header") + +func Auth(center Center, log *zap.Logger) Func { return func(h http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctx := r.Context() box, err := center.Authenticate(r) if err != nil { - if err == auth.ErrNoAuthorizationHeader { + if err == ErrNoAuthorizationHeader { reqLogOrDefault(ctx, log).Debug(logs.CouldntReceiveAccessBoxForGateKeyRandomKeyWillBeUsed) } else { reqLogOrDefault(ctx, log).Error(logs.FailedToPassAuthentication, zap.Error(err)) diff --git a/api/middleware/util.go b/api/middleware/util.go index bb01762..8bdd38a 100644 --- a/api/middleware/util.go +++ b/api/middleware/util.go @@ -5,7 +5,6 @@ import ( "fmt" "time" - "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/auth" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/creds/accessbox" ) @@ -37,8 +36,8 @@ func GetBoxData(ctx context.Context) (*accessbox.Box, error) { } // GetAuthHeaders extracts auth.AuthHeader from context. -func GetAuthHeaders(ctx context.Context) (*auth.AuthHeader, error) { - authHeaders, ok := ctx.Value(authHeadersKey).(*auth.AuthHeader) +func GetAuthHeaders(ctx context.Context) (*AuthHeader, error) { + authHeaders, ok := ctx.Value(authHeadersKey).(*AuthHeader) if !ok { return nil, fmt.Errorf("couldn't get auth headers from context") } @@ -62,7 +61,7 @@ func SetBoxData(ctx context.Context, box *accessbox.Box) context.Context { } // SetAuthHeaders sets auth.AuthHeader in the context. -func SetAuthHeaders(ctx context.Context, header *auth.AuthHeader) context.Context { +func SetAuthHeaders(ctx context.Context, header *AuthHeader) context.Context { return context.WithValue(ctx, authHeadersKey, header) } diff --git a/api/router.go b/api/router.go index 0da2d5f..b48c484 100644 --- a/api/router.go +++ b/api/router.go @@ -5,7 +5,6 @@ import ( "fmt" "net/http" - "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/auth" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/data" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors" s3middleware "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/middleware" @@ -90,7 +89,7 @@ type ( } ) -func AttachChi(api *chi.Mux, domains []string, throttle middleware.ThrottleOpts, h Handler, center auth.Center, log *zap.Logger, appMetrics *metrics.AppMetrics) { +func AttachChi(api *chi.Mux, domains []string, throttle middleware.ThrottleOpts, h Handler, center s3middleware.Center, log *zap.Logger, appMetrics *metrics.AppMetrics) { api.Use( s3middleware.Request(log), middleware.ThrottleWithOpts(throttle), diff --git a/api/router_mock_test.go b/api/router_mock_test.go index 0ba5386..b08eafc 100644 --- a/api/router_mock_test.go +++ b/api/router_mock_test.go @@ -6,7 +6,6 @@ import ( "net/http" "testing" - "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/auth" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/data" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/middleware" "github.com/stretchr/testify/require" @@ -15,8 +14,8 @@ import ( type centerMock struct { } -func (c *centerMock) Authenticate(*http.Request) (*auth.Box, error) { - return &auth.Box{}, nil +func (c *centerMock) Authenticate(*http.Request) (*middleware.Box, error) { + return &middleware.Box{}, nil } type handlerMock struct { diff --git a/cmd/s3-gw/app.go b/cmd/s3-gw/app.go index a8b6f42..fcc71e3 100644 --- a/cmd/s3-gw/app.go +++ b/cmd/s3-gw/app.go @@ -21,6 +21,7 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/cache" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/handler" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer" + s3middleware "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/middleware" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/notifications" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/resolver" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/frostfs" @@ -47,7 +48,7 @@ const awsDefaultNamespace = "http://s3.amazonaws.com/doc/2006-03-01/" type ( // App is the main application structure. App struct { - ctr auth.Center + ctr s3middleware.Center log *zap.Logger cfg *viper.Viper pool *pool.Pool