diff --git a/cmd/admin-rpc-server_test.go b/cmd/admin-rpc-server_test.go index 2dea7caa0..36c46398d 100644 --- a/cmd/admin-rpc-server_test.go +++ b/cmd/admin-rpc-server_test.go @@ -33,16 +33,19 @@ func testAdminCmd(cmd cmdType, t *testing.T) { } defer os.RemoveAll(rootPath) - adminServer := adminCmd{} creds := serverConfig.GetCredential() + token, err := authenticateNode(creds.AccessKey, creds.SecretKey) + if err != nil { + t.Fatal(err) + } + + adminServer := adminCmd{} args := LoginRPCArgs{ - Username: creds.AccessKey, - Password: creds.SecretKey, + AuthToken: token, Version: Version, RequestTime: UTCNow(), } - reply := LoginRPCReply{} - err = adminServer.Login(&args, &reply) + err = adminServer.Login(&args, &LoginRPCReply{}) if err != nil { t.Fatalf("Failed to login to admin server - %v", err) } @@ -52,7 +55,7 @@ func testAdminCmd(cmd cmdType, t *testing.T) { <-globalServiceSignalCh }() - ga := AuthRPCArgs{AuthToken: reply.AuthToken} + ga := AuthRPCArgs{AuthToken: token} genReply := AuthRPCReply{} switch cmd { case restartCmd: @@ -91,21 +94,25 @@ func TestReInitDisks(t *testing.T) { // Setup admin rpc server for an XL backend. globalIsXL = true adminServer := adminCmd{} + creds := serverConfig.GetCredential() + token, err := authenticateNode(creds.AccessKey, creds.SecretKey) + if err != nil { + t.Fatal(err) + } + args := LoginRPCArgs{ - Username: creds.AccessKey, - Password: creds.SecretKey, + AuthToken: token, Version: Version, RequestTime: UTCNow(), } - reply := LoginRPCReply{} - err = adminServer.Login(&args, &reply) + err = adminServer.Login(&args, &LoginRPCReply{}) if err != nil { t.Fatalf("Failed to login to admin server - %v", err) } authArgs := AuthRPCArgs{ - AuthToken: reply.AuthToken, + AuthToken: token, } authReply := AuthRPCReply{} @@ -114,12 +121,15 @@ func TestReInitDisks(t *testing.T) { t.Errorf("Expected to pass, but failed with %v", err) } + token, err = authenticateNode(creds.AccessKey, creds.SecretKey) + if err != nil { + t.Fatal(err) + } // Negative test case with admin rpc server setup for FS. globalIsXL = false fsAdminServer := adminCmd{} fsArgs := LoginRPCArgs{ - Username: creds.AccessKey, - Password: creds.SecretKey, + AuthToken: token, Version: Version, RequestTime: UTCNow(), } @@ -130,7 +140,7 @@ func TestReInitDisks(t *testing.T) { } authArgs = AuthRPCArgs{ - AuthToken: fsReply.AuthToken, + AuthToken: token, } authReply = AuthRPCReply{} // Attempt ReInitDisks service on a FS backend. @@ -154,9 +164,14 @@ func TestGetConfig(t *testing.T) { adminServer := adminCmd{} creds := serverConfig.GetCredential() + + token, err := authenticateNode(creds.AccessKey, creds.SecretKey) + if err != nil { + t.Fatal(err) + } + args := LoginRPCArgs{ - Username: creds.AccessKey, - Password: creds.SecretKey, + AuthToken: token, Version: Version, RequestTime: UTCNow(), } @@ -167,7 +182,7 @@ func TestGetConfig(t *testing.T) { } authArgs := AuthRPCArgs{ - AuthToken: reply.AuthToken, + AuthToken: token, } configReply := ConfigReply{} @@ -198,9 +213,12 @@ func TestWriteAndCommitConfig(t *testing.T) { adminServer := adminCmd{} creds := serverConfig.GetCredential() + token, err := authenticateNode(creds.AccessKey, creds.SecretKey) + if err != nil { + t.Fatal(err) + } args := LoginRPCArgs{ - Username: creds.AccessKey, - Password: creds.SecretKey, + AuthToken: token, Version: Version, RequestTime: UTCNow(), } @@ -215,7 +233,7 @@ func TestWriteAndCommitConfig(t *testing.T) { tmpFileName := mustGetUUID() wArgs := WriteConfigArgs{ AuthRPCArgs: AuthRPCArgs{ - AuthToken: reply.AuthToken, + AuthToken: token, }, TmpFileName: tmpFileName, Buf: buf, @@ -232,7 +250,7 @@ func TestWriteAndCommitConfig(t *testing.T) { cArgs := CommitConfigArgs{ AuthRPCArgs: AuthRPCArgs{ - AuthToken: reply.AuthToken, + AuthToken: token, }, FileName: tmpFileName, } diff --git a/cmd/auth-rpc-client.go b/cmd/auth-rpc-client.go index f9e667662..523019c77 100644 --- a/cmd/auth-rpc-client.go +++ b/cmd/auth-rpc-client.go @@ -99,21 +99,22 @@ func (authClient *AuthRPCClient) Login() (err error) { // Attempt to login if not logged in already. if authClient.authToken == "" { - // Login to authenticate and acquire a new auth token. + authClient.authToken, err = authenticateNode(authClient.config.accessKey, authClient.config.secretKey) + if err != nil { + return err + } + // Login to authenticate your token. var ( loginMethod = authClient.config.serviceName + loginMethodName loginArgs = LoginRPCArgs{ - Username: authClient.config.accessKey, - Password: authClient.config.secretKey, + AuthToken: authClient.authToken, Version: Version, RequestTime: UTCNow(), } - loginReply = LoginRPCReply{} ) - if err = authClient.rpcClient.Call(loginMethod, &loginArgs, &loginReply); err != nil { + if err = authClient.rpcClient.Call(loginMethod, &loginArgs, &LoginRPCReply{}); err != nil { return err } - authClient.authToken = loginReply.AuthToken } return nil } diff --git a/cmd/auth-rpc-server.go b/cmd/auth-rpc-server.go index 05a62b826..86e6a3cac 100644 --- a/cmd/auth-rpc-server.go +++ b/cmd/auth-rpc-server.go @@ -20,8 +20,7 @@ package cmd const loginMethodName = ".Login" // AuthRPCServer RPC server authenticates using JWT. -type AuthRPCServer struct { -} +type AuthRPCServer struct{} // Login - Handles JWT based RPC login. func (b AuthRPCServer) Login(args *LoginRPCArgs, reply *LoginRPCReply) error { @@ -30,14 +29,10 @@ func (b AuthRPCServer) Login(args *LoginRPCArgs, reply *LoginRPCReply) error { return err } - // Authenticate using JWT. - token, err := authenticateNode(args.Username, args.Password) - if err != nil { - return err + // Return an error if token is not valid. + if !isAuthTokenValid(args.AuthToken) { + return errAuthentication } - // Return the token. - reply.AuthToken = token - return nil } diff --git a/cmd/auth-rpc-server_test.go b/cmd/auth-rpc-server_test.go index 21355fcfc..fbd853952 100644 --- a/cmd/auth-rpc-server_test.go +++ b/cmd/auth-rpc-server_test.go @@ -29,6 +29,10 @@ func TestLogin(t *testing.T) { } defer os.RemoveAll(rootPath) creds := serverConfig.GetCredential() + token, err := authenticateNode(creds.AccessKey, creds.SecretKey) + if err != nil { + t.Fatal(err) + } ls := AuthRPCServer{} testCases := []struct { args LoginRPCArgs @@ -38,9 +42,8 @@ func TestLogin(t *testing.T) { // Valid case. { args: LoginRPCArgs{ - Username: creds.AccessKey, - Password: creds.SecretKey, - Version: Version, + AuthToken: token, + Version: Version, }, skewTime: 0, expectedErr: nil, @@ -48,9 +51,8 @@ func TestLogin(t *testing.T) { // Valid username, password and request time, not version. { args: LoginRPCArgs{ - Username: creds.AccessKey, - Password: creds.SecretKey, - Version: "INVALID-" + Version, + AuthToken: token, + Version: "INVALID-" + Version, }, skewTime: 0, expectedErr: errServerVersionMismatch, @@ -58,49 +60,17 @@ func TestLogin(t *testing.T) { // Valid username, password and version, not request time { args: LoginRPCArgs{ - Username: creds.AccessKey, - Password: creds.SecretKey, - Version: Version, + AuthToken: token, + Version: Version, }, skewTime: 20 * time.Minute, expectedErr: errServerTimeMismatch, }, - // Invalid username length + // Invalid token, fails with authentication error { args: LoginRPCArgs{ - Username: "aaa", - Password: "minio123", - Version: Version, - }, - skewTime: 0, - expectedErr: errInvalidAccessKeyLength, - }, - // Invalid password length - { - args: LoginRPCArgs{ - Username: "minio", - Password: "aaa", - Version: Version, - }, - skewTime: 0, - expectedErr: errInvalidSecretKeyLength, - }, - // Invalid username - { - args: LoginRPCArgs{ - Username: "aaaaa", - Password: creds.SecretKey, - Version: Version, - }, - skewTime: 0, - expectedErr: errInvalidAccessKeyID, - }, - // Invalid password - { - args: LoginRPCArgs{ - Username: creds.AccessKey, - Password: "aaaaaaaa", - Version: Version, + AuthToken: "", + Version: Version, }, skewTime: 0, expectedErr: errAuthentication, @@ -108,7 +78,7 @@ func TestLogin(t *testing.T) { } for i, test := range testCases { reply := LoginRPCReply{} - test.args.RequestTime = time.Now().Add(test.skewTime).UTC() + test.args.RequestTime = UTCNow().Add(test.skewTime) err := ls.Login(&test.args, &reply) if err != test.expectedErr { t.Errorf("Test %d: Expected error %v but received %v", diff --git a/cmd/browser-peer-rpc.go b/cmd/browser-peer-rpc.go index b985d54ae..6ae72900c 100644 --- a/cmd/browser-peer-rpc.go +++ b/cmd/browser-peer-rpc.go @@ -23,26 +23,6 @@ import ( "time" ) -// Login handler implements JWT login token generator, which upon login request -// along with username and password is generated. -func (br *browserPeerAPIHandlers) Login(args *LoginRPCArgs, reply *LoginRPCReply) error { - // Validate LoginRPCArgs - if err := args.IsValid(); err != nil { - return err - } - - // Authenticate using JWT. - token, err := authenticateWeb(args.Username, args.Password) - if err != nil { - return err - } - - // Return the token. - reply.AuthToken = token - - return nil -} - // SetAuthPeerArgs - Arguments collection for SetAuth RPC call type SetAuthPeerArgs struct { // For Auth diff --git a/cmd/browser-peer-rpc_test.go b/cmd/browser-peer-rpc_test.go index 16e0aae30..4f3a4b122 100644 --- a/cmd/browser-peer-rpc_test.go +++ b/cmd/browser-peer-rpc_test.go @@ -91,9 +91,12 @@ func (s *TestRPCBrowserPeerSuite) testBrowserPeerRPC(t *testing.T) { // Validate for failure in login handler with previous credentials. rclient = newRPCClient(s.testAuthConf.serverAddr, s.testAuthConf.serviceEndpoint, false) defer rclient.Close() + token, err := authenticateNode(creds.AccessKey, creds.SecretKey) + if err != nil { + t.Fatal(err) + } rargs := &LoginRPCArgs{ - Username: creds.AccessKey, - Password: creds.SecretKey, + AuthToken: token, Version: Version, RequestTime: UTCNow(), } @@ -105,20 +108,18 @@ func (s *TestRPCBrowserPeerSuite) testBrowserPeerRPC(t *testing.T) { } } + token, err = authenticateNode(creds.AccessKey, creds.SecretKey) + if err != nil { + t.Fatal(err) + } // Validate for success in loing handled with valid credetnails. rargs = &LoginRPCArgs{ - Username: creds.AccessKey, - Password: creds.SecretKey, + AuthToken: token, Version: Version, RequestTime: UTCNow(), } rreply = &LoginRPCReply{} - err = rclient.Call("BrowserPeer"+loginMethodName, rargs, rreply) - if err != nil { + if err = rclient.Call("BrowserPeer"+loginMethodName, rargs, rreply); err != nil { t.Fatal(err) } - // Validate all the replied fields after successful login. - if rreply.AuthToken == "" { - t.Fatalf("Generated token cannot be empty %s", errInvalidToken) - } } diff --git a/cmd/browser-rpc-router.go b/cmd/browser-rpc-router.go index fdf88c856..f79488413 100644 --- a/cmd/browser-rpc-router.go +++ b/cmd/browser-rpc-router.go @@ -35,7 +35,7 @@ type browserPeerAPIHandlers struct { // Register RPC router func registerBrowserPeerRPCRouter(mux *router.Router) error { - bpHandlers := &browserPeerAPIHandlers{} + bpHandlers := &browserPeerAPIHandlers{AuthRPCServer{}} bpRPCServer := newRPCServer() err := bpRPCServer.RegisterName("BrowserPeer", bpHandlers) diff --git a/cmd/jwt.go b/cmd/jwt.go index d1f39ac42..980c62551 100644 --- a/cmd/jwt.go +++ b/cmd/jwt.go @@ -63,10 +63,10 @@ func authenticateJWT(accessKey, secretKey string, expiry time.Duration) (string, } utcNow := UTCNow() - token := jwtgo.NewWithClaims(jwtgo.SigningMethodHS512, jwtgo.MapClaims{ - "exp": utcNow.Add(expiry).Unix(), - "iat": utcNow.Unix(), - "sub": accessKey, + token := jwtgo.NewWithClaims(jwtgo.SigningMethodHS512, jwtgo.StandardClaims{ + ExpiresAt: utcNow.Add(expiry).Unix(), + IssuedAt: utcNow.Unix(), + Subject: accessKey, }) return token.SignedString([]byte(serverCred.SecretKey)) @@ -93,13 +93,17 @@ func keyFuncCallback(jwtToken *jwtgo.Token) (interface{}, error) { } func isAuthTokenValid(tokenString string) bool { - jwtToken, err := jwtgo.Parse(tokenString, keyFuncCallback) + var claims jwtgo.StandardClaims + jwtToken, err := jwtgo.ParseWithClaims(tokenString, &claims, keyFuncCallback) if err != nil { errorIf(err, "Unable to parse JWT token string") return false } - - return jwtToken.Valid + if err = claims.Valid(); err != nil { + errorIf(err, "Invalid claims in JWT token string") + return false + } + return jwtToken.Valid && claims.Subject == serverConfig.GetCredential().AccessKey } func isHTTPRequestValid(req *http.Request) bool { @@ -110,14 +114,20 @@ func isHTTPRequestValid(req *http.Request) bool { // Returns nil if the request is authenticated. errNoAuthToken if token missing. // Returns errAuthentication for all other errors. func webRequestAuthenticate(req *http.Request) error { - jwtToken, err := jwtreq.ParseFromRequest(req, jwtreq.AuthorizationHeaderExtractor, keyFuncCallback) + var claims jwtgo.StandardClaims + jwtToken, err := jwtreq.ParseFromRequestWithClaims(req, jwtreq.AuthorizationHeaderExtractor, &claims, keyFuncCallback) if err != nil { if err == jwtreq.ErrNoTokenInRequest { return errNoAuthToken } return errAuthentication } - + if err = claims.Valid(); err != nil { + return err + } + if claims.Subject != serverConfig.GetCredential().AccessKey { + return errInvalidAccessKeyID + } if !jwtToken.Valid { return errAuthentication } diff --git a/cmd/jwt_test.go b/cmd/jwt_test.go index 9f075ae28..f7b2efe10 100644 --- a/cmd/jwt_test.go +++ b/cmd/jwt_test.go @@ -17,6 +17,7 @@ package cmd import ( + "net/http" "os" "testing" ) @@ -88,6 +89,58 @@ func TestAuthenticateURL(t *testing.T) { testAuthenticate("url", t) } +// Tests web request authenticator. +func TestWebRequestAuthenticate(t *testing.T) { + testPath, err := newTestConfig(globalMinioDefaultRegion) + if err != nil { + t.Fatalf("unable initialize config file, %s", err) + } + defer os.RemoveAll(testPath) + + creds := serverConfig.GetCredential() + token, err := getTokenString(creds.AccessKey, creds.SecretKey) + if err != nil { + t.Fatalf("unable get token %s", err) + } + testCases := []struct { + req *http.Request + expectedErr error + }{ + // Set valid authorization header. + { + req: &http.Request{ + Header: http.Header{ + "Authorization": []string{token}, + }, + }, + expectedErr: nil, + }, + // No authorization header. + { + req: &http.Request{ + Header: http.Header{}, + }, + expectedErr: errNoAuthToken, + }, + // Invalid authorization token. + { + req: &http.Request{ + Header: http.Header{ + "Authorization": []string{"invalid-token"}, + }, + }, + expectedErr: errAuthentication, + }, + } + + for i, testCase := range testCases { + gotErr := webRequestAuthenticate(testCase.req) + if testCase.expectedErr != gotErr { + t.Errorf("Test %d, expected err %s, got %s", i+1, testCase.expectedErr, gotErr) + } + } +} + func BenchmarkAuthenticateNode(b *testing.B) { testPath, err := newTestConfig(globalMinioDefaultRegion) if err != nil { diff --git a/cmd/lock-rpc-server_test.go b/cmd/lock-rpc-server_test.go index 8ae66ff02..31ff8c42b 100644 --- a/cmd/lock-rpc-server_test.go +++ b/cmd/lock-rpc-server_test.go @@ -58,9 +58,12 @@ func createLockTestServer(t *testing.T) (string, *lockServer, string) { }, } creds := serverConfig.GetCredential() + token, err := authenticateNode(creds.AccessKey, creds.SecretKey) + if err != nil { + t.Fatal(err) + } loginArgs := LoginRPCArgs{ - Username: creds.AccessKey, - Password: creds.SecretKey, + AuthToken: token, Version: Version, RequestTime: UTCNow(), } @@ -69,8 +72,6 @@ func createLockTestServer(t *testing.T) (string, *lockServer, string) { if err != nil { t.Fatalf("Failed to login to lock server - %v", err) } - token := loginReply.AuthToken - return testPath, locker, token } diff --git a/cmd/notifier-config.go b/cmd/notifier-config.go index 044033274..e46c16eb8 100644 --- a/cmd/notifier-config.go +++ b/cmd/notifier-config.go @@ -235,10 +235,7 @@ func (n *notifier) Validate() error { if err := n.MySQL.Validate(); err != nil { return err } - if err := n.MQTT.Validate(); err != nil { - return err - } - return nil + return n.MQTT.Validate() } func (n *notifier) SetAMQPByID(accountID string, amqpn amqpNotify) { diff --git a/cmd/rpc-common.go b/cmd/rpc-common.go index 9761787e3..5f5e56e95 100644 --- a/cmd/rpc-common.go +++ b/cmd/rpc-common.go @@ -60,8 +60,7 @@ type AuthRPCReply struct{} // LoginRPCArgs - login username and password for RPC. type LoginRPCArgs struct { - Username string - Password string + AuthToken string Version string RequestTime time.Time } @@ -80,11 +79,8 @@ func (args LoginRPCArgs) IsValid() error { return nil } -// LoginRPCReply - login reply provides generated token to be used -// with subsequent requests. -type LoginRPCReply struct { - AuthToken string -} +// LoginRPCReply - login reply is a dummy struct perhaps for future use. +type LoginRPCReply struct{} // LockArgs represents arguments for any authenticated lock RPC call. type LockArgs struct { diff --git a/cmd/web-handlers_test.go b/cmd/web-handlers_test.go index 571f4cf60..b2ddc8698 100644 --- a/cmd/web-handlers_test.go +++ b/cmd/web-handlers_test.go @@ -34,6 +34,7 @@ import ( "strings" "testing" + jwtgo "github.com/dgrijalva/jwt-go" humanize "github.com/dustin/go-humanize" "github.com/minio/minio-go/pkg/policy" "github.com/minio/minio-go/pkg/set" @@ -667,6 +668,16 @@ func TestWebCreateURLToken(t *testing.T) { ExecObjectLayerTest(t, testCreateURLToken) } +func getTokenString(accessKey, secretKey string) (string, error) { + utcNow := UTCNow() + token := jwtgo.NewWithClaims(jwtgo.SigningMethodHS512, jwtgo.StandardClaims{ + ExpiresAt: utcNow.Add(defaultJWTExpiry).Unix(), + IssuedAt: utcNow.Unix(), + Subject: accessKey, + }) + return token.SignedString([]byte(secretKey)) +} + func testCreateURLToken(obj ObjectLayer, instanceType string, t TestErrHandler) { apiRouter := initTestWebRPCEndPoint(obj) credentials := serverConfig.GetCredential() @@ -700,6 +711,21 @@ func testCreateURLToken(obj ObjectLayer, instanceType string, t TestErrHandler) if !isAuthTokenValid(tokenReply.Token) { t.Fatalf("token is not valid") } + + // Token is invalid. + if isAuthTokenValid("") { + t.Fatalf("token shouldn't be valid, but it is") + } + + token, err := getTokenString("invalid-access", credentials.SecretKey) + if err != nil { + t.Fatal(err) + } + + // Token has invalid access key. + if isAuthTokenValid(token) { + t.Fatalf("token shouldn't be valid, but it is") + } } // Wrapper for calling Upload Handler diff --git a/pkg/quick/quick.go b/pkg/quick/quick.go index e010c8f9a..a8a730a83 100644 --- a/pkg/quick/quick.go +++ b/pkg/quick/quick.go @@ -92,12 +92,7 @@ func (d config) Save(filename string) error { func (d config) Load(filename string) error { d.lock.Lock() defer d.lock.Unlock() - - if err := loadFileConfig(filename, d.data); err != nil { - return err - } - - return nil + return loadFileConfig(filename, d.data) } // Data - grab internal data map for reading