tests: Add tests for signature-jwt code (#2169)

Fixes #1989

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
 }

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: <nil>", 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: <nil>, 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: <nil>", 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: <nil>, 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: <nil>", 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: <nil>, got: %s", testCase, err)
+		}
+	}
+}

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"])