From 86d31e99d59a263f0445084e4936860a074fc747 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Mon, 18 Jul 2016 23:56:27 -0700 Subject: [PATCH] api: use checkAuth now at PutBucket, DeleteBucket handlers. (#2225) Additionally add a unit test for isReqAuthenticated function. --- auth-handler.go | 4 +- auth-handler_test.go | 93 ++++++++++++++++++++++++++++++++++++++++++++ bucket-handlers.go | 44 +++++++-------------- 3 files changed, 110 insertions(+), 31 deletions(-) create mode 100644 auth-handler_test.go diff --git a/auth-handler.go b/auth-handler.go index 9d330b8dc..cf1d0638a 100644 --- a/auth-handler.go +++ b/auth-handler.go @@ -1,5 +1,5 @@ /* - * Minio Cloud Storage, (C) 2015 Minio, Inc. + * Minio Cloud Storage, (C) 2015, 2016 Minio, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -153,7 +153,7 @@ func isReqAuthenticated(r *http.Request) (s3Error APIErrorCode) { // is returned for unhandled auth type. Once the auth type is indentified // request headers and body are used to calculate the signature validating // the client signature present in request. -func checkAuth(w http.ResponseWriter, r *http.Request) APIErrorCode { +func checkAuth(r *http.Request) APIErrorCode { authType := getRequestAuthType(r) if authType != authTypePresigned && authType != authTypeSigned { // For all unhandled auth types return error AccessDenied. diff --git a/auth-handler_test.go b/auth-handler_test.go new file mode 100644 index 000000000..5484168c6 --- /dev/null +++ b/auth-handler_test.go @@ -0,0 +1,93 @@ +/* + * Minio Cloud Storage, (C) 2016 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 main + +import ( + "bytes" + "io" + "io/ioutil" + "net/http" + "testing" +) + +// Provides a fully populated http request instance, fails otherwise. +func mustNewRequest(method string, urlStr string, contentLength int64, body io.ReadSeeker, t *testing.T) *http.Request { + req, err := newTestRequest(method, urlStr, contentLength, body) + if err != nil { + t.Fatalf("Unable to initialize new http request %s", err) + } + return req +} + +// This is similar to mustNewRequest but additionally the request +// is signed with AWS Signature V4, fails if not able to do so. +func mustNewSignedRequest(method string, urlStr string, contentLength int64, body io.ReadSeeker, t *testing.T) *http.Request { + req := mustNewRequest(method, urlStr, contentLength, body, t) + cred := serverConfig.GetCredential() + if err := signRequest(req, cred.AccessKeyID, cred.SecretAccessKey); err != nil { + t.Fatalf("Unable to inititalized new signed http request %s", err) + } + return req +} + +// Tests is requested authenticated function, tests replies for s3 errors. +func TestIsReqAuthenticated(t *testing.T) { + savedServerConfig := serverConfig + defer func() { + serverConfig = savedServerConfig + }() + serverConfig = nil + + // Test initialized config file. + path, err := ioutil.TempDir("", "minio-") + if err != nil { + t.Fatalf("Unable to create a temporary directory, %s", err) + } + defer removeAll(path) + + // Inititalize a new config. + setGlobalConfigPath(path) + if err := initConfig(); err != nil { + t.Fatalf("unable initialize config file, %s", err) + } + serverConfig.SetCredential(credential{"myuser", "mypassword"}) + + // List of test cases for validating http request authentication. + testCases := []struct { + req *http.Request + s3Error APIErrorCode + }{ + // When request is nil, internal error is returned. + {nil, ErrInternalError}, + // When request is unsigned, access denied is returned. + {mustNewRequest("GET", "http://localhost:9000", 0, nil, t), ErrAccessDenied}, + // When request is properly signed, but has bad Content-MD5 header. + {mustNewSignedRequest("PUT", "http://localhost:9000", 5, bytes.NewReader([]byte("hello")), t), ErrBadDigest}, + // When request is properly signed, error is none. + {mustNewSignedRequest("GET", "http://localhost:9000", 0, nil, t), ErrNone}, + } + + // Validates all testcases. + for _, testCase := range testCases { + if testCase.s3Error == ErrBadDigest { + testCase.req.Header.Set("Content-Md5", "garbage") + } + if s3Error := isReqAuthenticated(testCase.req); s3Error != testCase.s3Error { + t.Fatalf("Unexpected s3error returned wanted %d, got %d", testCase.s3Error, s3Error) + } + } +} diff --git a/bucket-handlers.go b/bucket-handlers.go index 9e122a441..504101915 100644 --- a/bucket-handlers.go +++ b/bucket-handlers.go @@ -199,7 +199,7 @@ func (api objectAPIHandlers) ListMultipartUploadsHandler(w http.ResponseWriter, // owned by the authenticated sender of the request. func (api objectAPIHandlers) ListBucketsHandler(w http.ResponseWriter, r *http.Request) { // List buckets does not support bucket policies, no need to enforce it. - if s3Error := checkAuth(w, r); s3Error != ErrNone { + if s3Error := checkAuth(r); s3Error != ErrNone { writeErrorResponse(w, r, s3Error, r.URL.Path) return } @@ -306,27 +306,19 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter, // ---------- // This implementation of the PUT operation creates a new bucket for authenticated request func (api objectAPIHandlers) PutBucketHandler(w http.ResponseWriter, r *http.Request) { + // PutBucket does not support policies, use checkAuth to validate signature. + if s3Error := checkAuth(r); s3Error != ErrNone { + writeErrorResponse(w, r, s3Error, r.URL.Path) + return + } + vars := mux.Vars(r) bucket := vars["bucket"] - // Set http request for signature. - switch getRequestAuthType(r) { - default: - // For all unknown auth types return error. - writeErrorResponse(w, r, ErrAccessDenied, r.URL.Path) - return - case authTypePresigned, authTypeSigned: - if s3Error := isReqAuthenticated(r); s3Error != ErrNone { - writeErrorResponse(w, r, s3Error, r.URL.Path) - return - } - } - // Validate if incoming location constraint is valid, reject // requests which do not follow valid region requirements. - errCode := isValidLocationConstraint(r) - if errCode != ErrNone { - writeErrorResponse(w, r, errCode, r.URL.Path) + if s3Error := isValidLocationConstraint(r); s3Error != ErrNone { + writeErrorResponse(w, r, s3Error, r.URL.Path) return } // Make bucket. @@ -463,21 +455,15 @@ func (api objectAPIHandlers) HeadBucketHandler(w http.ResponseWriter, r *http.Re // DeleteBucketHandler - Delete bucket func (api objectAPIHandlers) DeleteBucketHandler(w http.ResponseWriter, r *http.Request) { + // DeleteBucket does not support bucket policies, use checkAuth to validate signature. + if s3Error := checkAuth(r); s3Error != ErrNone { + writeErrorResponse(w, r, s3Error, r.URL.Path) + return + } + vars := mux.Vars(r) bucket := vars["bucket"] - switch getRequestAuthType(r) { - default: - // For all unknown auth types return error. - writeErrorResponse(w, r, ErrAccessDenied, r.URL.Path) - return - case authTypePresigned, authTypeSigned: - if s3Error := isReqAuthenticated(r); s3Error != ErrNone { - writeErrorResponse(w, r, s3Error, r.URL.Path) - return - } - } - if err := api.ObjectAPI.DeleteBucket(bucket); err != nil { errorIf(err, "Unable to delete a bucket.") writeErrorResponse(w, r, toAPIErrorCode(err), r.URL.Path)