From ffd7b7059cba29ac4c24d424bab943ad9eb40c62 Mon Sep 17 00:00:00 2001 From: Krishnan Parthasarathi Date: Thu, 11 Jul 2019 14:37:13 -0700 Subject: [PATCH] Pass on web-handler arguments properly to log entries (#7894) --- cmd/web-handler-context.go | 244 ++++++++++++++++++++++++++++++++ cmd/web-handler-context_test.go | 111 +++++++++++++++ cmd/web-handlers.go | 52 ++----- 3 files changed, 370 insertions(+), 37 deletions(-) create mode 100644 cmd/web-handler-context.go create mode 100644 cmd/web-handler-context_test.go diff --git a/cmd/web-handler-context.go b/cmd/web-handler-context.go new file mode 100644 index 000000000..c7d260897 --- /dev/null +++ b/cmd/web-handler-context.go @@ -0,0 +1,244 @@ +/* + * MinIO Cloud Storage, (C) 2019 MinIO, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package cmd + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + + "github.com/minio/minio/cmd/logger" + "github.com/minio/minio/pkg/handlers" +) + +const ( + kmBucket = "BucketName" + kmObject = "ObjectName" + kmObjects = "Objects" + kmPrefix = "Prefix" + kmMarker = "Marker" + kmUsername = "UserName" + kmHostname = "HostName" + kmPolicy = "Policy" +) + +// KeyValueMap extends builtin map to support setting and getting +// select fields like BucketName, ObjectName, Prefix, etc. +type KeyValueMap map[string]string + +// Bucket returns the BucketName +func (km KeyValueMap) Bucket() string { + return km[kmBucket] +} + +// Object returns the ObjectName +func (km KeyValueMap) Object() string { + return km[kmObject] +} + +// Prefix returns the Prefix +func (km KeyValueMap) Prefix() string { + return km[kmPrefix] +} + +// Username returns the Username +func (km KeyValueMap) Username() string { + return km[kmUsername] +} + +// Hostname returns the Hostname +func (km KeyValueMap) Hostname() string { + return km[kmHostname] +} + +// Policy returns the Policy +func (km KeyValueMap) Policy() string { + return km[kmPolicy] +} + +// Objects returns the Objects +func (km KeyValueMap) Objects() []string { + var objects []string + _ = json.Unmarshal([]byte(km[kmObjects]), &objects) + return objects +} + +// SetBucket sets the given bucket to the KeyValueMap +func (km *KeyValueMap) SetBucket(bucket string) { + (*km)[kmBucket] = bucket +} + +// SetPrefix sets the given prefix to the KeyValueMap +func (km *KeyValueMap) SetPrefix(prefix string) { + (*km)[kmPrefix] = prefix +} + +// SetObject sets the given object to the KeyValueMap +func (km *KeyValueMap) SetObject(object string) { + (*km)[kmObject] = object +} + +// SetMarker sets the given marker to the KeyValueMap +func (km *KeyValueMap) SetMarker(marker string) { + (*km)[kmMarker] = marker +} + +// SetPolicy sets the given policy to the KeyValueMap +func (km *KeyValueMap) SetPolicy(policy string) { + (*km)[kmPolicy] = policy +} + +// SetExpiry sets the expiry to the KeyValueMap +func (km *KeyValueMap) SetExpiry(expiry int64) { + (*km)[kmPolicy] = fmt.Sprintf("%d", expiry) +} + +// SetObjects sets the list of objects to the KeyValueMap +func (km *KeyValueMap) SetObjects(objects []string) { + objsVal, err := json.Marshal(objects) + if err != nil { + // NB this can only happen when we can't marshal a Go + // slice to its json representation. + objsVal = []byte("[]") + } + (*km)[kmObjects] = string(objsVal) +} + +// SetUsername sets the username to the KeyValueMap +func (km *KeyValueMap) SetUsername(username string) { + (*km)[kmUsername] = username +} + +// SetHostname sets the hostname to the KeyValueMap +func (km *KeyValueMap) SetHostname(hostname string) { + (*km)[kmHostname] = hostname +} + +// ToKeyValuer interface wraps ToKeyValue method that allows types to +// marshal their values as a map of structure member names to their +// values, as strings +type ToKeyValuer interface { + ToKeyValue() KeyValueMap +} + +// ToKeyValue implementation for WebGenericArgs +func (args *WebGenericArgs) ToKeyValue() KeyValueMap { + return KeyValueMap{} +} + +// ToKeyValue implementation for MakeBucketArgs +func (args *MakeBucketArgs) ToKeyValue() KeyValueMap { + km := KeyValueMap{} + km.SetBucket(args.BucketName) + return km +} + +// ToKeyValue implementation for RemoveBucketArgs +func (args *RemoveBucketArgs) ToKeyValue() KeyValueMap { + km := KeyValueMap{} + km.SetBucket(args.BucketName) + return km +} + +// ToKeyValue implementation for ListObjectsArgs +func (args *ListObjectsArgs) ToKeyValue() KeyValueMap { + km := KeyValueMap{} + km.SetBucket(args.BucketName) + km.SetPrefix(args.Prefix) + km.SetMarker(args.Marker) + return km +} + +// ToKeyValue implementation for RemoveObjectArgs +func (args *RemoveObjectArgs) ToKeyValue() KeyValueMap { + km := KeyValueMap{} + km.SetBucket(args.BucketName) + km.SetObjects(args.Objects) + return km +} + +// ToKeyValue implementation for LoginArgs +func (args *LoginArgs) ToKeyValue() KeyValueMap { + km := KeyValueMap{} + km.SetUsername(args.Username) + return km +} + +// ToKeyValue implementation for GetBucketPolicyArgs +func (args *GetBucketPolicyArgs) ToKeyValue() KeyValueMap { + km := KeyValueMap{} + km.SetBucket(args.BucketName) + km.SetPrefix(args.Prefix) + return km +} + +// ToKeyValue implementation for ListAllBucketPoliciesArgs +func (args *ListAllBucketPoliciesArgs) ToKeyValue() KeyValueMap { + km := KeyValueMap{} + km.SetBucket(args.BucketName) + return km +} + +// ToKeyValue implementation for SetBucketPolicyWebArgs +func (args *SetBucketPolicyWebArgs) ToKeyValue() KeyValueMap { + km := KeyValueMap{} + km.SetBucket(args.BucketName) + km.SetPrefix(args.Prefix) + km.SetPolicy(args.Policy) + return km +} + +// ToKeyValue implementation for SetAuthArgs +// SetAuthArgs doesn't implement the ToKeyValue interface that will be +// used by logger subsystem down the line, to avoid leaking +// credentials to an external log target +func (args *SetAuthArgs) ToKeyValue() KeyValueMap { + return KeyValueMap{} +} + +// ToKeyValue implementation for PresignedGetArgs +func (args *PresignedGetArgs) ToKeyValue() KeyValueMap { + km := KeyValueMap{} + km.SetHostname(args.HostName) + km.SetBucket(args.BucketName) + km.SetObject(args.ObjectName) + km.SetExpiry(args.Expiry) + return km +} + +// newWebContext creates a context with ReqInfo values from the given +// http request and api name. +func newWebContext(r *http.Request, args ToKeyValuer, api string) context.Context { + argsMap := args.ToKeyValue() + bucket := argsMap.Bucket() + object := argsMap.Object() + prefix := argsMap.Prefix() + + if prefix != "" { + object = prefix + } + reqInfo := &logger.ReqInfo{ + DeploymentID: globalDeploymentID, + RemoteHost: handlers.GetSourceIP(r), + UserAgent: r.UserAgent(), + API: api, + BucketName: bucket, + ObjectName: object, + } + return logger.SetReqInfo(context.Background(), reqInfo) +} diff --git a/cmd/web-handler-context_test.go b/cmd/web-handler-context_test.go new file mode 100644 index 000000000..807f96d04 --- /dev/null +++ b/cmd/web-handler-context_test.go @@ -0,0 +1,111 @@ +/* + * MinIO Cloud Storage, (C) 2019 MinIO, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package cmd + +import ( + "bytes" + "net/http" + "testing" + + "github.com/minio/minio/cmd/logger" +) + +func TestKeyValueMap(t *testing.T) { + bucket := "bucket" + object := "object" + prefix := "prefix" + username := "username" + policy := "policy" + host := "min.io" + objects := []string{object, object} + + km := KeyValueMap{} + km.SetBucket(bucket) + km.SetPrefix(prefix) + km.SetUsername(username) + km.SetHostname(host) + km.SetObject(object) + km.SetObjects(objects) + km.SetPolicy(policy) + + if got := km.Bucket(); got != bucket { + t.Errorf("Expected %s but got %s", bucket, got) + } + + if got := km.Object(); got != object { + t.Errorf("Expected %s but got %s", object, got) + } + + areEqualObjects := func(as, bs []string) bool { + if len(as) != len(bs) { + return false + } + + for i, a := range as { + b := bs[i] + if a != b { + return false + } + } + return true + } + + if got := km.Objects(); !areEqualObjects(got, objects) { + t.Errorf("Expected %s but got %s", objects, got) + } + + if got := km.Policy(); got != policy { + t.Errorf("Expected %s but got %s", policy, got) + } + + if got := km.Prefix(); got != prefix { + t.Errorf("Expected %s but got %s", prefix, got) + } + + if got := km.Username(); got != username { + t.Errorf("Expected %s but got %s", username, got) + } + + if got := km.Hostname(); got != host { + t.Errorf("Expected %s but got %s", host, got) + } +} + +func TestNewWebContext(t *testing.T) { + api := "Test API" + args := ListObjectsArgs{ + BucketName: "bucket", + Prefix: "prefix", + Marker: "marker", + } + + req, err := http.NewRequest(http.MethodPost, "http://min.io", bytes.NewReader([]byte("nothing"))) + if err != nil { + t.Fatal("Unexpected failure while creating a test request") + } + + ctx := newWebContext(req, &args, api) + reqInfo := logger.GetReqInfo(ctx) + + if reqInfo.API != api { + t.Errorf("Expected %s got %s", api, reqInfo.API) + } + + if reqInfo.BucketName != args.BucketName { + t.Errorf("Expected %s got %s", args.BucketName, reqInfo.BucketName) + } +} diff --git a/cmd/web-handlers.go b/cmd/web-handlers.go index afd589403..8121e79ae 100644 --- a/cmd/web-handlers.go +++ b/cmd/web-handlers.go @@ -74,31 +74,9 @@ type ServerInfoRep struct { UIVersion string `json:"uiVersion"` } -// newWebContext creates a context with ReqInfo values from the given -// http request and api name. -func newWebContext(r *http.Request, api string) context.Context { - vars := mux.Vars(r) - bucket := vars["bucket"] - object := vars["object"] - prefix := vars["prefix"] - - if prefix != "" { - object = prefix - } - reqInfo := &logger.ReqInfo{ - DeploymentID: globalDeploymentID, - RemoteHost: handlers.GetSourceIP(r), - UserAgent: r.UserAgent(), - API: api, - BucketName: bucket, - ObjectName: object, - } - return logger.SetReqInfo(context.Background(), reqInfo) -} - // ServerInfo - get server info. func (web *webAPIHandlers) ServerInfo(r *http.Request, args *WebGenericArgs, reply *ServerInfoRep) error { - ctx := newWebContext(r, "webServerInfo") + ctx := newWebContext(r, args, "webServerInfo") _, owner, authErr := webRequestAuthenticate(r) if authErr != nil { return toJSONError(ctx, authErr) @@ -148,7 +126,7 @@ type StorageInfoRep struct { // StorageInfo - web call to gather storage usage statistics. func (web *webAPIHandlers) StorageInfo(r *http.Request, args *WebGenericArgs, reply *StorageInfoRep) error { - ctx := newWebContext(r, "webStorageInfo") + ctx := newWebContext(r, args, "webStorageInfo") objectAPI := web.ObjectAPI() if objectAPI == nil { return toJSONError(ctx, errServerNotInitialized) @@ -169,7 +147,7 @@ type MakeBucketArgs struct { // MakeBucket - creates a new bucket. func (web *webAPIHandlers) MakeBucket(r *http.Request, args *MakeBucketArgs, reply *WebGenericRep) error { - ctx := newWebContext(r, "webMakeBucket") + ctx := newWebContext(r, args, "webMakeBucket") objectAPI := web.ObjectAPI() if objectAPI == nil { return toJSONError(ctx, errServerNotInitialized) @@ -230,7 +208,7 @@ type RemoveBucketArgs struct { // DeleteBucket - removes a bucket, must be empty. func (web *webAPIHandlers) DeleteBucket(r *http.Request, args *RemoveBucketArgs, reply *WebGenericRep) error { - ctx := newWebContext(r, "webDeleteBucket") + ctx := newWebContext(r, args, "webDeleteBucket") objectAPI := web.ObjectAPI() if objectAPI == nil { return toJSONError(ctx, errServerNotInitialized) @@ -318,7 +296,7 @@ type WebBucketInfo struct { // ListBuckets - list buckets api. func (web *webAPIHandlers) ListBuckets(r *http.Request, args *WebGenericArgs, reply *ListBucketsRep) error { - ctx := newWebContext(r, "webListBuckets") + ctx := newWebContext(r, args, "webListBuckets") objectAPI := web.ObjectAPI() if objectAPI == nil { return toJSONError(ctx, errServerNotInitialized) @@ -421,7 +399,7 @@ type WebObjectInfo struct { // ListObjects - list objects api. func (web *webAPIHandlers) ListObjects(r *http.Request, args *ListObjectsArgs, reply *ListObjectsRep) error { - ctx := newWebContext(r, "webListObjects") + ctx := newWebContext(r, args, "webListObjects") reply.UIVersion = browser.UIVersion objectAPI := web.ObjectAPI() if objectAPI == nil { @@ -616,7 +594,7 @@ type RemoveObjectArgs struct { // RemoveObject - removes an object, or all the objects at a given prefix. func (web *webAPIHandlers) RemoveObject(r *http.Request, args *RemoveObjectArgs, reply *WebGenericRep) error { - ctx := newWebContext(r, "webRemoveObject") + ctx := newWebContext(r, args, "webRemoveObject") objectAPI := web.ObjectAPI() if objectAPI == nil { return toJSONError(ctx, errServerNotInitialized) @@ -776,7 +754,7 @@ type LoginRep struct { // Login - user login handler. func (web *webAPIHandlers) Login(r *http.Request, args *LoginArgs, reply *LoginRep) error { - ctx := newWebContext(r, "webLogin") + ctx := newWebContext(r, args, "webLogin") token, err := authenticateWeb(args.Username, args.Password) if err != nil { return toJSONError(ctx, err) @@ -795,7 +773,7 @@ type GenerateAuthReply struct { } func (web webAPIHandlers) GenerateAuth(r *http.Request, args *WebGenericArgs, reply *GenerateAuthReply) error { - ctx := newWebContext(r, "webGenerateAuth") + ctx := newWebContext(r, args, "webGenerateAuth") _, owner, authErr := webRequestAuthenticate(r) if authErr != nil { return toJSONError(ctx, authErr) @@ -830,7 +808,7 @@ type SetAuthReply struct { // SetAuth - Set accessKey and secretKey credentials. func (web *webAPIHandlers) SetAuth(r *http.Request, args *SetAuthArgs, reply *SetAuthReply) error { - ctx := newWebContext(r, "webSetAuth") + ctx := newWebContext(r, args, "webSetAuth") claims, owner, authErr := webRequestAuthenticate(r) if authErr != nil { return toJSONError(ctx, authErr) @@ -919,7 +897,7 @@ type URLTokenReply struct { // CreateURLToken creates a URL token (short-lived) for GET requests. func (web *webAPIHandlers) CreateURLToken(r *http.Request, args *WebGenericArgs, reply *URLTokenReply) error { - ctx := newWebContext(r, "webCreateURLToken") + ctx := newWebContext(r, args, "webCreateURLToken") claims, owner, authErr := webRequestAuthenticate(r) if authErr != nil { return toJSONError(ctx, authErr) @@ -1503,7 +1481,7 @@ type GetBucketPolicyRep struct { // GetBucketPolicy - get bucket policy for the requested prefix. func (web *webAPIHandlers) GetBucketPolicy(r *http.Request, args *GetBucketPolicyArgs, reply *GetBucketPolicyRep) error { - ctx := newWebContext(r, "webGetBucketPolicy") + ctx := newWebContext(r, args, "webGetBucketPolicy") objectAPI := web.ObjectAPI() if objectAPI == nil { return toJSONError(ctx, errServerNotInitialized) @@ -1599,7 +1577,7 @@ type ListAllBucketPoliciesRep struct { // ListAllBucketPolicies - get all bucket policy. func (web *webAPIHandlers) ListAllBucketPolicies(r *http.Request, args *ListAllBucketPoliciesArgs, reply *ListAllBucketPoliciesRep) error { - ctx := newWebContext(r, "WebListAllBucketPolicies") + ctx := newWebContext(r, args, "WebListAllBucketPolicies") objectAPI := web.ObjectAPI() if objectAPI == nil { return toJSONError(ctx, errServerNotInitialized) @@ -1680,7 +1658,7 @@ type SetBucketPolicyWebArgs struct { // SetBucketPolicy - set bucket policy. func (web *webAPIHandlers) SetBucketPolicy(r *http.Request, args *SetBucketPolicyWebArgs, reply *WebGenericRep) error { - ctx := newWebContext(r, "webSetBucketPolicy") + ctx := newWebContext(r, args, "webSetBucketPolicy") objectAPI := web.ObjectAPI() reply.UIVersion = browser.UIVersion @@ -1832,7 +1810,7 @@ type PresignedGetRep struct { // PresignedGET - returns presigned-Get url. func (web *webAPIHandlers) PresignedGet(r *http.Request, args *PresignedGetArgs, reply *PresignedGetRep) error { - ctx := newWebContext(r, "webPresignedGet") + ctx := newWebContext(r, args, "webPresignedGet") claims, owner, authErr := webRequestAuthenticate(r) if authErr != nil { return toJSONError(ctx, authErr)