From 749a94f6c94fc29f844fbb3be3cb79b4bbcc36f5 Mon Sep 17 00:00:00 2001 From: Bala FA Date: Tue, 12 Jul 2016 10:27:40 +0530 Subject: [PATCH] tests: Add tests for signature-jwt code (#2169) Fixes #1989 --- signature-jwt.go | 64 ++++++++--- signature-jwt_test.go | 253 ++++++++++++++++++++++++++++++++++++++++++ web-handlers.go | 50 ++++++--- 3 files changed, 335 insertions(+), 32 deletions(-) create mode 100644 signature-jwt_test.go diff --git a/signature-jwt.go b/signature-jwt.go index 932174465..fc4ecce41 100644 --- a/signature-jwt.go +++ b/signature-jwt.go @@ -17,6 +17,7 @@ package main import ( + "fmt" "strings" "time" @@ -36,34 +37,63 @@ const ( tokenExpires time.Duration = 10 ) -// initJWT - initialize. -func initJWT() *JWT { - jwt := &JWT{} +// newJWT - returns new JWT object. +func newJWT() (*JWT, error) { + if serverConfig == nil { + return nil, fmt.Errorf("server not initialzed") + } // Save access, secret keys. - jwt.credential = serverConfig.GetCredential() + cred := serverConfig.GetCredential() + if !isValidAccessKey.MatchString(cred.AccessKeyID) { + return nil, fmt.Errorf("Invalid access key") + } + if !isValidSecretKey.MatchString(cred.SecretAccessKey) { + return nil, fmt.Errorf("Invalid secret key") + } - // Return. - return jwt + return &JWT{cred}, nil } -// GenerateToken - generates a new Json Web Token based on the incoming user id. -func (jwt *JWT) GenerateToken(userName string) (string, error) { +// GenerateToken - generates a new Json Web Token based on the incoming access key. +func (jwt *JWT) GenerateToken(accessKey string) (string, error) { + // Trim spaces. + accessKey = strings.TrimSpace(accessKey) + + if !isValidAccessKey.MatchString(accessKey) { + return "", fmt.Errorf("Invalid access key") + } + token := jwtgo.New(jwtgo.SigningMethodHS512) // Token expires in 10hrs. token.Claims["exp"] = time.Now().Add(time.Hour * tokenExpires).Unix() token.Claims["iat"] = time.Now().Unix() - token.Claims["sub"] = userName + token.Claims["sub"] = accessKey return token.SignedString([]byte(jwt.SecretAccessKey)) } -// Authenticate - authenticates incoming username and password. -func (jwt *JWT) Authenticate(userName, password string) bool { - userName = strings.TrimSpace(userName) - password = strings.TrimSpace(password) - if userName != jwt.AccessKeyID { - return false +// Authenticate - authenticates incoming access key and secret key. +func (jwt *JWT) Authenticate(accessKey, secretKey string) error { + // Trim spaces. + accessKey = strings.TrimSpace(accessKey) + + if !isValidAccessKey.MatchString(accessKey) { + return fmt.Errorf("Invalid access key") } - hashedPassword, _ := bcrypt.GenerateFromPassword([]byte(jwt.SecretAccessKey), bcrypt.DefaultCost) - return bcrypt.CompareHashAndPassword(hashedPassword, []byte(password)) == nil + if !isValidSecretKey.MatchString(secretKey) { + return fmt.Errorf("Invalid secret key") + } + + if accessKey != jwt.AccessKeyID { + return fmt.Errorf("Access key does not match") + } + + hashedSecretKey, _ := bcrypt.GenerateFromPassword([]byte(jwt.SecretAccessKey), bcrypt.DefaultCost) + + if bcrypt.CompareHashAndPassword(hashedSecretKey, []byte(secretKey)) != nil { + return fmt.Errorf("Authentication failed") + } + + // Success. + return nil } diff --git a/signature-jwt_test.go b/signature-jwt_test.go new file mode 100644 index 000000000..14a3b7413 --- /dev/null +++ b/signature-jwt_test.go @@ -0,0 +1,253 @@ +/* + * 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 ( + "fmt" + "io/ioutil" + "os" + "path" + "testing" +) + +// Tests newJWT() +func TestNewJWT(t *testing.T) { + savedServerConfig := serverConfig + defer func() { + serverConfig = savedServerConfig + }() + serverConfig = nil + + // Test non-existent config directory. + path1, err := ioutil.TempDir("", "minio-") + if err != nil { + t.Fatalf("Unable to create a temporary directory, %s", err) + } + defer removeAll(path1) + + // Test empty config directory. + path2, err := ioutil.TempDir("", "minio-") + if err != nil { + t.Fatalf("Unable to create a temporary directory, %s", err) + } + defer removeAll(path2) + + // Test empty config file. + path3, err := ioutil.TempDir("", "minio-") + if err != nil { + t.Fatalf("Unable to create a temporary directory, %s", err) + } + defer removeAll(path3) + + if err = ioutil.WriteFile(path.Join(path3, "config.json"), []byte{}, os.ModePerm); err != nil { + t.Fatalf("unable to create config file, %s", err) + } + + // Test initialized config file. + path4, err := ioutil.TempDir("", "minio-") + if err != nil { + t.Fatalf("Unable to create a temporary directory, %s", err) + } + defer removeAll(path4) + + // Define test cases. + testCases := []struct { + dirPath string + init bool + cred *credential + expectedErr error + }{ + // Test non-existent config directory. + {path.Join(path1, "non-existent-dir"), false, nil, fmt.Errorf("server not initialzed")}, + // Test empty config directory. + {path2, false, nil, fmt.Errorf("server not initialzed")}, + // Test empty config file. + {path3, false, nil, fmt.Errorf("server not initialzed")}, + // Test initialized config file. + {path4, true, nil, nil}, + // Test to read already created config file. + {path4, false, nil, nil}, + // Access key is too small. + {path4, false, &credential{"user", "pass"}, fmt.Errorf("Invalid access key")}, + // Access key is too long. + {path4, false, &credential{"user12345678901234567", "pass"}, fmt.Errorf("Invalid access key")}, + // Access key contains unsupported characters. + {path4, false, &credential{"!@#$%^&*()", "pass"}, fmt.Errorf("Invalid access key")}, + // Secret key is too small. + {path4, false, &credential{"myuser", "pass"}, fmt.Errorf("Invalid secret key")}, + // Secret key is too long. + {path4, false, &credential{"myuser", "pass1234567890123456789012345678901234567"}, fmt.Errorf("Invalid secret key")}, + // Valid access/secret keys. + {path4, false, &credential{"myuser", "mypassword"}, nil}, + } + + // Run tests. + for _, testCase := range testCases { + setGlobalConfigPath(testCase.dirPath) + if testCase.init { + if err := initConfig(); err != nil { + t.Fatalf("unable initialize config file, %s", err) + } + } + + if testCase.cred != nil { + serverConfig.SetCredential(*testCase.cred) + } + + _, err := newJWT() + + if testCase.expectedErr != nil { + if err == nil { + t.Fatalf("%+v: expected: %s, got: ", testCase, testCase.expectedErr) + } + + if testCase.expectedErr.Error() != err.Error() { + t.Fatalf("%+v: expected: %s, got: %s", testCase, testCase.expectedErr, err) + } + } else if err != nil { + t.Fatalf("%+v: expected: , got: %s", testCase, err) + } + } +} + +// Tests JWT.GenerateToken() +func TestGenerateToken(t *testing.T) { + savedServerConfig := serverConfig + defer func() { + serverConfig = savedServerConfig + }() + serverConfig = nil + + // Setup. + testPath, err := ioutil.TempDir("", "minio-") + if err != nil { + t.Fatalf("Unable to create a temporary directory, %s", err) + } + defer removeAll(testPath) + + setGlobalConfigPath(testPath) + if err = initConfig(); err != nil { + t.Fatalf("unable initialize config file, %s", err) + } + + jwt, err := newJWT() + if err != nil { + t.Fatalf("unable get new JWT, %s", err) + } + + // Define test cases. + testCases := []struct { + accessKey string + expectedErr error + }{ + // Access key is too small. + {"user", fmt.Errorf("Invalid access key")}, + // Access key is too long. + {"user12345678901234567", fmt.Errorf("Invalid access key")}, + // Access key contains unsupported characters. + {"!@#$%^&*()", fmt.Errorf("Invalid access key")}, + // Valid access key. + {"myuser", nil}, + // Valid access key with leading/trailing spaces. + {" myuser ", nil}, + } + + // Run tests. + for _, testCase := range testCases { + _, err := jwt.GenerateToken(testCase.accessKey) + if testCase.expectedErr != nil { + if err == nil { + t.Fatalf("%+v: expected: %s, got: ", testCase, testCase.expectedErr) + } + + if testCase.expectedErr.Error() != err.Error() { + t.Fatalf("%+v: expected: %s, got: %s", testCase, testCase.expectedErr, err) + } + } else if err != nil { + t.Fatalf("%+v: expected: , got: %s", testCase, err) + } + } +} + +// Tests JWT.Authenticate() +func TestAuthenticate(t *testing.T) { + savedServerConfig := serverConfig + defer func() { + serverConfig = savedServerConfig + }() + serverConfig = nil + + // Setup. + testPath, err := ioutil.TempDir("", "minio-") + if err != nil { + t.Fatalf("Unable to create a temporary directory, %s", err) + } + defer removeAll(testPath) + + setGlobalConfigPath(testPath) + if err = initConfig(); err != nil { + t.Fatalf("unable initialize config file, %s", err) + } + + jwt, err := newJWT() + if err != nil { + t.Fatalf("unable get new JWT, %s", err) + } + + // Define test cases. + testCases := []struct { + accessKey string + secretKey string + expectedErr error + }{ + // Access key too small. + {"user", "pass", fmt.Errorf("Invalid access key")}, + // Access key too long. + {"user12345678901234567", "pass", fmt.Errorf("Invalid access key")}, + // Access key contains unsuppported characters. + {"!@#$%^&*()", "pass", fmt.Errorf("Invalid access key")}, + // Secret key too small. + {"myuser", "pass", fmt.Errorf("Invalid secret key")}, + // Secret key too long. + {"myuser", "pass1234567890123456789012345678901234567", fmt.Errorf("Invalid secret key")}, + // Authentication error. + {"myuser", "mypassword", fmt.Errorf("Access key does not match")}, + // Authentication error. + {serverConfig.GetCredential().AccessKeyID, "mypassword", fmt.Errorf("Authentication failed")}, + // Success. + {serverConfig.GetCredential().AccessKeyID, serverConfig.GetCredential().SecretAccessKey, nil}, + // Success when access key contains leading/trailing spaces. + {" " + serverConfig.GetCredential().AccessKeyID + " ", serverConfig.GetCredential().SecretAccessKey, nil}, + } + + // Run tests. + for _, testCase := range testCases { + err := jwt.Authenticate(testCase.accessKey, testCase.secretKey) + + if testCase.expectedErr != nil { + if err == nil { + t.Fatalf("%+v: expected: %s, got: ", testCase, testCase.expectedErr) + } + + if testCase.expectedErr.Error() != err.Error() { + t.Fatalf("%+v: expected: %s, got: %s", testCase, testCase.expectedErr, err) + } + } else if err != nil { + t.Fatalf("%+v: expected: , got: %s", testCase, err) + } + } +} diff --git a/web-handlers.go b/web-handlers.go index fe574476e..d5e6ad893 100644 --- a/web-handlers.go +++ b/web-handlers.go @@ -36,7 +36,12 @@ import ( // isJWTReqAuthenticated validates if any incoming request to be a // valid JWT authenticated request. func isJWTReqAuthenticated(req *http.Request) bool { - jwt := initJWT() + jwt, err := newJWT() + if err != nil { + errorIf(err, "unable to initialize a new JWT") + return false + } + token, e := jwtgo.ParseFromRequest(req, func(token *jwtgo.Token) (interface{}, error) { if _, ok := token.Method.(*jwtgo.SigningMethodHMAC); !ok { return nil, fmt.Errorf("Unexpected signing method: %v", token.Header["alg"]) @@ -44,6 +49,7 @@ func isJWTReqAuthenticated(req *http.Request) bool { return []byte(jwt.SecretAccessKey), nil }) if e != nil { + errorIf(e, "token parsing failed") return false } return token.Valid @@ -255,17 +261,22 @@ type LoginRep struct { // Login - user login handler. func (web *webAPIHandlers) Login(r *http.Request, args *LoginArgs, reply *LoginRep) error { - jwt := initJWT() - if jwt.Authenticate(args.Username, args.Password) { - token, err := jwt.GenerateToken(args.Username) - if err != nil { - return &json2.Error{Message: err.Error()} - } - reply.Token = token - reply.UIVersion = miniobrowser.UIVersion - return nil + jwt, err := newJWT() + if err != nil { + return &json2.Error{Message: err.Error()} + } + + if err = jwt.Authenticate(args.Username, args.Password); err != nil { + return &json2.Error{Message: err.Error()} + } + + token, err := jwt.GenerateToken(args.Username) + if err != nil { + return &json2.Error{Message: err.Error()} } - return &json2.Error{Message: "Invalid credentials"} + reply.Token = token + reply.UIVersion = miniobrowser.UIVersion + return nil } // GenerateAuthReply - reply for GenerateAuth @@ -315,9 +326,13 @@ func (web *webAPIHandlers) SetAuth(r *http.Request, args *SetAuthArgs, reply *Se return &json2.Error{Message: err.Error()} } - jwt := initJWT() - if !jwt.Authenticate(args.AccessKey, args.SecretKey) { - return &json2.Error{Message: "Invalid credentials"} + jwt, err := newJWT() + if err != nil { + return &json2.Error{Message: err.Error()} + } + + if err = jwt.Authenticate(args.AccessKey, args.SecretKey); err != nil { + return &json2.Error{Message: err.Error()} } token, err := jwt.GenerateToken(args.AccessKey) if err != nil { @@ -369,7 +384,12 @@ func (web *webAPIHandlers) Download(w http.ResponseWriter, r *http.Request) { object := vars["object"] token := r.URL.Query().Get("token") - jwt := initJWT() + jwt, err := newJWT() + if err != nil { + errorIf(err, "error in getting new JWT") + return + } + jwttoken, e := jwtgo.Parse(token, func(token *jwtgo.Token) (interface{}, error) { if _, ok := token.Method.(*jwtgo.SigningMethodHMAC); !ok { return nil, fmt.Errorf("Unexpected signing method: %v", token.Header["alg"])