From 12a7a15daa9e6f9b7a54323563835cb1a51bac67 Mon Sep 17 00:00:00 2001 From: Krishna Srinivas Date: Thu, 12 Jan 2017 02:56:42 +0530 Subject: [PATCH] browser: Allow anonymous browsing of readable buckets. (#3515) --- cmd/bucket-handlers.go | 16 ++++ cmd/jwt.go | 19 ++++- cmd/web-handlers.go | 58 +++++++++---- cmd/web-handlers_test.go | 180 ++++++++++++++++++++++++++++++--------- 4 files changed, 212 insertions(+), 61 deletions(-) diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index 8f845a046..c0ebb652f 100644 --- a/cmd/bucket-handlers.go +++ b/cmd/bucket-handlers.go @@ -23,6 +23,7 @@ import ( "io" "net/http" "net/url" + "path" "strings" "sync" @@ -71,6 +72,21 @@ func enforceBucketPolicy(bucket string, action string, reqURL *url.URL) (s3Error return ErrNone } +// Check if the action is allowed on the bucket/prefix. +func isBucketActionAllowed(action, bucket, prefix string) bool { + policy := globalBucketPolicies.GetBucketPolicy(bucket) + if policy == nil { + return false + } + resource := bucketARNPrefix + path.Join(bucket, prefix) + var conditionKeyMap map[string]set.StringSet + // Validate action, resource and conditions with current policy statements. + if !bucketPolicyEvalStatements(action, resource, conditionKeyMap, policy.Statements) { + return false + } + return true +} + // GetBucketLocationHandler - GET Bucket location. // ------------------------- // This operation returns bucket location. diff --git a/cmd/jwt.go b/cmd/jwt.go index c87e15ffa..549617cab 100644 --- a/cmd/jwt.go +++ b/cmd/jwt.go @@ -43,6 +43,7 @@ var errInvalidSecretKeyLength = errors.New("Invalid secret key, secret key shoul var errInvalidAccessKeyID = errors.New("The access key ID you provided does not exist in our records") var errAuthentication = errors.New("Authentication failed, check your access credentials") +var errNoAuthToken = errors.New("JWT token missing") func authenticateJWT(accessKey, secretKey string, expiry time.Duration) (string, error) { // Trim spaces. @@ -106,11 +107,23 @@ func isAuthTokenValid(tokenString string) bool { } func isHTTPRequestValid(req *http.Request) bool { + return webReqestAuthenticate(req) == nil +} + +// Check if the request is authenticated. +// Returns nil if the request is authenticated. errNoAuthToken if token missing. +// Returns errAuthentication for all other errors. +func webReqestAuthenticate(req *http.Request) error { jwtToken, err := jwtreq.ParseFromRequest(req, jwtreq.AuthorizationHeaderExtractor, keyFuncCallback) if err != nil { - errorIf(err, "Unable to parse JWT token string") - return false + if err == jwtreq.ErrNoTokenInRequest { + return errNoAuthToken + } + return errAuthentication } - return jwtToken.Valid + if !jwtToken.Valid { + return errAuthentication + } + return nil } diff --git a/cmd/web-handlers.go b/cmd/web-handlers.go index d358020b4..d4cb4a498 100644 --- a/cmd/web-handlers.go +++ b/cmd/web-handlers.go @@ -151,21 +151,23 @@ func (web *webAPIHandlers) ListBuckets(r *http.Request, args *WebGenericArgs, re if objectAPI == nil { return toJSONError(errServerNotInitialized) } - if !isHTTPRequestValid(r) { - return toJSONError(errAuthentication) + authErr := webReqestAuthenticate(r) + if authErr != nil { + return toJSONError(authErr) } buckets, err := objectAPI.ListBuckets() if err != nil { return toJSONError(err) } for _, bucket := range buckets { - // List all buckets which are not private. - if bucket.Name != path.Base(reservedBucket) { - reply.Buckets = append(reply.Buckets, WebBucketInfo{ - Name: bucket.Name, - CreationDate: bucket.Created, - }) + if bucket.Name == path.Base(reservedBucket) { + continue } + + reply.Buckets = append(reply.Buckets, WebBucketInfo{ + Name: bucket.Name, + CreationDate: bucket.Created, + }) } reply.UIVersion = miniobrowser.UIVersion return nil @@ -180,6 +182,7 @@ type ListObjectsArgs struct { // ListObjectsRep - list objects response. type ListObjectsRep struct { Objects []WebObjectInfo `json:"objects"` + Writable bool `json:"writable"` // Used by client to show "upload file" button. UIVersion string `json:"uiVersion"` } @@ -197,12 +200,30 @@ type WebObjectInfo struct { // ListObjects - list objects api. func (web *webAPIHandlers) ListObjects(r *http.Request, args *ListObjectsArgs, reply *ListObjectsRep) error { + reply.UIVersion = miniobrowser.UIVersion objectAPI := web.ObjectAPI() if objectAPI == nil { return toJSONError(errServerNotInitialized) } - if !isHTTPRequestValid(r) { - return toJSONError(errAuthentication) + prefix := args.Prefix + "test" // To test if GetObject/PutObject with the specified prefix is allowed. + readable := isBucketActionAllowed("s3:GetObject", args.BucketName, prefix) + writable := isBucketActionAllowed("s3:PutObject", args.BucketName, prefix) + authErr := webReqestAuthenticate(r) + switch { + case authErr == errAuthentication: + return toJSONError(authErr) + case authErr == nil: + break + case readable && writable: + reply.Writable = true + break + case readable: + break + case writable: + reply.Writable = true + return nil + default: + return errAuthentication } marker := "" for { @@ -228,7 +249,6 @@ func (web *webAPIHandlers) ListObjects(r *http.Request, args *ListObjectsArgs, r break } } - reply.UIVersion = miniobrowser.UIVersion return nil } @@ -420,14 +440,20 @@ func (web *webAPIHandlers) Upload(w http.ResponseWriter, r *http.Request) { return } - if !isHTTPRequestValid(r) { - writeWebErrorResponse(w, errAuthentication) - return - } vars := mux.Vars(r) bucket := vars["bucket"] object := vars["object"] + authErr := webReqestAuthenticate(r) + if authErr == errAuthentication { + writeWebErrorResponse(w, errAuthentication) + return + } + if authErr != nil && !isBucketActionAllowed("s3:PutObject", bucket, object) { + writeWebErrorResponse(w, errAuthentication) + return + } + // Extract incoming metadata if any. metadata := extractMetadataFromHeader(r.Header) @@ -467,7 +493,7 @@ func (web *webAPIHandlers) Download(w http.ResponseWriter, r *http.Request) { object := vars["object"] token := r.URL.Query().Get("token") - if !isAuthTokenValid(token) { + if !isAuthTokenValid(token) && !isBucketActionAllowed("s3:GetObject", bucket, object) { writeWebErrorResponse(w, errAuthentication) return } diff --git a/cmd/web-handlers_test.go b/cmd/web-handlers_test.go index e793d202c..5031cbee6 100644 --- a/cmd/web-handlers_test.go +++ b/cmd/web-handlers_test.go @@ -19,6 +19,7 @@ package cmd import ( "bytes" "errors" + "fmt" "io/ioutil" "net/http" "net/http/httptest" @@ -422,30 +423,62 @@ func testListObjectsWebHandler(obj ObjectLayer, instanceType string, t TestErrHa t.Fatalf("Was not able to upload an object, %v", err) } - listObjectsRequest := ListObjectsArgs{BucketName: bucketName, Prefix: ""} - listObjectsReply := &ListObjectsRep{} - req, err := newTestWebRPCRequest("Web.ListObjects", authorization, listObjectsRequest) - if err != nil { - t.Fatalf("Failed to create HTTP request: %v", err) + test := func(token string) (error, *ListObjectsRep) { + listObjectsRequest := ListObjectsArgs{BucketName: bucketName, Prefix: ""} + listObjectsReply := &ListObjectsRep{} + var req *http.Request + req, err = newTestWebRPCRequest("Web.ListObjects", token, listObjectsRequest) + if err != nil { + t.Fatalf("Failed to create HTTP request: %v", err) + } + apiRouter.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + return fmt.Errorf("Expected the response status to be 200, but instead found `%d`", rec.Code), listObjectsReply + } + err = getTestWebRPCResponse(rec, &listObjectsReply) + if err != nil { + return err, listObjectsReply + } + return nil, listObjectsReply } - apiRouter.ServeHTTP(rec, req) - if rec.Code != http.StatusOK { - t.Fatalf("Expected the response status to be 200, but instead found `%d`", rec.Code) + verifyReply := func(reply *ListObjectsRep) { + if len(reply.Objects) == 0 { + t.Fatalf("Cannot find the object") + } + if reply.Objects[0].Key != objectName { + t.Fatalf("Found another object other than already created by PutObject") + } + if reply.Objects[0].Size != int64(objectSize) { + t.Fatalf("Found a object with the same name but with a different size") + } } - err = getTestWebRPCResponse(rec, &listObjectsReply) + + // Authenticated ListObjects should succeed. + err, reply := test(authorization) if err != nil { - t.Fatalf("Failed, %v", err) - } - if len(listObjectsReply.Objects) == 0 { - t.Fatalf("Cannot find the object") + t.Fatal(err) } - if listObjectsReply.Objects[0].Key != objectName { - t.Fatalf("Found another object other than already created by PutObject") + verifyReply(reply) + + // Unauthenticated ListObjects should fail. + err, reply = test("") + if err == nil { + t.Fatalf("Expected error `%s`", err) } - if listObjectsReply.Objects[0].Size != int64(objectSize) { - t.Fatalf("Found a object with the same name but with a different size") + + policy := bucketPolicy{ + Version: "1.0", + Statements: []policyStatement{getReadOnlyObjectStatement(bucketName, "")}, } + globalBucketPolicies.SetBucketPolicy(bucketName, policyChange{false, &policy}) + + // Unauthenticated ListObjects with READ bucket policy should succeed. + err, reply = test("") + if err != nil { + t.Fatal(err) + } + verifyReply(reply) } // Wrapper for calling RemoveObject Web Handler @@ -695,8 +728,8 @@ func testUploadWebHandler(obj ObjectLayer, instanceType string, t TestErrHandler defer removeAll(rootPath) credentials := serverConfig.GetCredential() + content := []byte("temporary file's content") - rec := httptest.NewRecorder() authorization, err := getWebRPCToken(apiRouter, credentials.AccessKey, credentials.SecretKey) if err != nil { t.Fatal("Cannot authenticate") @@ -704,6 +737,26 @@ func testUploadWebHandler(obj ObjectLayer, instanceType string, t TestErrHandler objectName := "test.file" bucketName := getRandomBucketName() + + test := func(token string) int { + rec := httptest.NewRecorder() + var req *http.Request + req, err = http.NewRequest("PUT", "/minio/upload/"+bucketName+"/"+objectName, nil) + if err != nil { + t.Fatalf("Cannot create upload request, %v", err) + } + + req.Header.Set("Content-Length", strconv.Itoa(len(content))) + req.Header.Set("x-amz-date", "20160814T114029Z") + req.Header.Set("Accept", "*/*") + req.Body = ioutil.NopCloser(bytes.NewReader(content)) + + if token != "" { + req.Header.Set("Authorization", "Bearer "+authorization) + } + apiRouter.ServeHTTP(rec, req) + return rec.Code + } // Create bucket. err = obj.MakeBucket(bucketName) if err != nil { @@ -711,22 +764,10 @@ func testUploadWebHandler(obj ObjectLayer, instanceType string, t TestErrHandler t.Fatalf("%s : %s", instanceType, err) } - content := []byte("temporary file's content") - - req, err := http.NewRequest("PUT", "/minio/upload/"+bucketName+"/"+objectName, nil) - req.Header.Set("Authorization", "Bearer "+authorization) - req.Header.Set("Content-Length", strconv.Itoa(len(content))) - req.Header.Set("x-amz-date", "20160814T114029Z") - req.Header.Set("Accept", "*/*") - req.Body = ioutil.NopCloser(bytes.NewReader(content)) - - if err != nil { - t.Fatalf("Cannot create upload request, %v", err) - } - - apiRouter.ServeHTTP(rec, req) - if rec.Code != http.StatusOK { - t.Fatalf("Expected the response status to be 200, but instead found `%d`", rec.Code) + // Authenticated upload should succeed. + code := test(authorization) + if code != http.StatusOK { + t.Fatalf("Expected the response status to be 200, but instead found `%d`", code) } var byteBuffer bytes.Buffer @@ -738,6 +779,25 @@ func testUploadWebHandler(obj ObjectLayer, instanceType string, t TestErrHandler if bytes.Compare(byteBuffer.Bytes(), content) != 0 { t.Fatalf("The upload file is different from the download file") } + + // Unauthenticated upload should fail. + code = test("") + if code != http.StatusForbidden { + t.Fatalf("Expected the response status to be 403, but instead found `%d`", code) + } + + policy := bucketPolicy{ + Version: "1.0", + Statements: []policyStatement{getWriteOnlyObjectStatement(bucketName, "")}, + } + + globalBucketPolicies.SetBucketPolicy(bucketName, policyChange{false, &policy}) + + // Unauthenticated upload with WRITE policy should succeed. + code = test("") + if code != http.StatusOK { + t.Fatalf("Expected the response status to be 200, but instead found `%d`", code) + } } // Wrapper for calling Upload Handler @@ -760,7 +820,6 @@ func testDownloadWebHandler(obj ObjectLayer, instanceType string, t TestErrHandl credentials := serverConfig.GetCredential() - rec := httptest.NewRecorder() authorization, err := getWebRPCToken(apiRouter, credentials.AccessKey, credentials.SecretKey) if err != nil { t.Fatal("Cannot authenticate") @@ -768,6 +827,24 @@ func testDownloadWebHandler(obj ObjectLayer, instanceType string, t TestErrHandl objectName := "test.file" bucketName := getRandomBucketName() + + test := func(token string) (int, []byte) { + rec := httptest.NewRecorder() + path := "/minio/download/" + bucketName + "/" + objectName + "?token=" + if token != "" { + path = path + token + } + var req *http.Request + req, err = http.NewRequest("GET", path, nil) + + if err != nil { + t.Fatalf("Cannot create upload request, %v", err) + } + + apiRouter.ServeHTTP(rec, req) + return rec.Code, rec.Body.Bytes() + } + // Create bucket. err = obj.MakeBucket(bucketName) if err != nil { @@ -781,18 +858,37 @@ func testDownloadWebHandler(obj ObjectLayer, instanceType string, t TestErrHandl t.Fatalf("Was not able to upload an object, %v", err) } - req, err := http.NewRequest("GET", "/minio/download/"+bucketName+"/"+objectName+"?token="+authorization, nil) + // Authenticated download should succeed. + code, bodyContent := test(authorization) - if err != nil { - t.Fatalf("Cannot create upload request, %v", err) + if code != http.StatusOK { + t.Fatalf("Expected the response status to be 200, but instead found `%d`", code) } - apiRouter.ServeHTTP(rec, req) - if rec.Code != http.StatusOK { - t.Fatalf("Expected the response status to be 200, but instead found `%d`", rec.Code) + if bytes.Compare(bodyContent, content) != 0 { + t.Fatalf("The downloaded file is corrupted") + } + + // Unauthenticated download should fail. + code, bodyContent = test("") + if code != http.StatusForbidden { + t.Fatalf("Expected the response status to be 403, but instead found `%d`", code) + } + + policy := bucketPolicy{ + Version: "1.0", + Statements: []policyStatement{getReadOnlyObjectStatement(bucketName, "")}, + } + + globalBucketPolicies.SetBucketPolicy(bucketName, policyChange{false, &policy}) + + // Unauthenticated download with READ policy should succeed. + code, bodyContent = test("") + if code != http.StatusOK { + t.Fatalf("Expected the response status to be 200, but instead found `%d`", code) } - if bytes.Compare(rec.Body.Bytes(), content) != 0 { + if bytes.Compare(bodyContent, content) != 0 { t.Fatalf("The downloaded file is corrupted") } }