Pass on web-handler arguments properly to log entries (#7894)

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)
+}

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)
+	}
+}

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)