From 8594315c05e0f5a720065a387f7451af7e6cec61 Mon Sep 17 00:00:00 2001 From: Yuva Shankar Date: Tue, 13 Dec 2016 15:41:07 -0800 Subject: [PATCH] Allow updates(first_name, last_name, disable, password) to built-in users Signed-off-by: Yuva Shankar --- db/local.go | 100 ++++++++++------- db/local_test.go | 80 +++++++++---- proxy/handlers.go | 2 - systemtests/basic_test.go | 1 + systemtests/init_test.go | 53 +++++++-- systemtests/local_user_test.go | 199 +++++++++++++++++++++++++++++++++ 6 files changed, 359 insertions(+), 76 deletions(-) create mode 100644 systemtests/local_user_test.go diff --git a/db/local.go b/db/local.go index c495ec9e..b0e81832 100644 --- a/db/local.go +++ b/db/local.go @@ -14,30 +14,6 @@ import ( // This file contains all local user management APIs. // NOTE: Built-in users(admin, ops) cannot be changed/updated. it needs to be consumed in the way its defined in code. -// getLocalUser helper function that retrieves a user entry from `/ccn_proxy/local_users` using username -// params: -// username:string; name of the user to be fetched from store -// return values: -// *types.InternalLocalUser: reference to the internal representation of the local user object -// error: as returned by the consecutive calls or any relevant custom errors -func getLocalUser(username string, stateDrv types.StateDriver) (*types.LocalUser, error) { - rawData, err := stateDrv.Read(GetPath(RootLocalUsers, username)) - if err != nil { - if err == ccnerrors.ErrKeyNotFound { - return nil, err - } - - return nil, fmt.Errorf("Failed to read local user %q data from store: %#v", username, err) - } - - var localUser types.LocalUser - if err := json.Unmarshal(rawData, &localUser); err != nil { - return nil, fmt.Errorf("Failed to unmarshal local user %q info %#v", username, err) - } - - return &localUser, nil -} - // GetLocalUsers returns all defined local users. // return values: // []types.InternalLocalUser: slice of local users @@ -82,7 +58,21 @@ func GetLocalUser(username string) (*types.LocalUser, error) { return nil, err } - return getLocalUser(username, stateDrv) + rawData, err := stateDrv.Read(GetPath(RootLocalUsers, username)) + if err != nil { + if err == ccnerrors.ErrKeyNotFound { + return nil, err + } + + return nil, fmt.Errorf("Failed to read local user %q data from store: %#v", username, err) + } + + var localUser types.LocalUser + if err := json.Unmarshal(rawData, &localUser); err != nil { + return nil, fmt.Errorf("Failed to unmarshal local user %q info %#v", username, err) + } + + return &localUser, nil } // UpdateLocalUser updates an existing entry in /ccn_proxy/local_users/. @@ -90,24 +80,52 @@ func GetLocalUser(username string) (*types.LocalUser, error) { // username: string; of the user that requires update // user: local user object to be updated in the data store // return values: -// error: as returned by AddLocalUser, DeleteLocalUser +// error: as returned by GetStateDriveror, any consecutive function call or relevant custom error func UpdateLocalUser(username string, user *types.LocalUser) error { - err := DeleteLocalUser(username) + stateDrv, err := state.GetStateDriver() + if err != nil { + return err + } + + key := GetPath(RootLocalUsers, username) + + _, err = stateDrv.Read(key) + switch err { case nil: - // if the password is getting updated; clear the existing hash + // generate password hash only if the password is not empty, otherwise use the existing hash if !common.IsEmpty(user.Password) { - user.PasswordHash = []byte{} + user.PasswordHash, err = common.GenPasswordHash(user.Password) + + if err != nil { + log.Debugf("Failed to create password hash for user %q: %#v", user.Username, err) + return err + } + } + + // raw password will never be stored in the store + user.Password = "" + + val, err := json.Marshal(user) + if err != nil { + return fmt.Errorf("Failed to marshal user %#v: %#v", user, err) } - return AddLocalUser(user) - case ccnerrors.ErrKeyNotFound, ccnerrors.ErrIllegalOperation: + if err := stateDrv.Write(key, val); err != nil { + return fmt.Errorf("Failed to write local user info. to data store: %#v", err) + } + + // not to let the user know about password hash + user.PasswordHash = []byte{} + + return nil + case ccnerrors.ErrKeyNotFound: return err default: - // this should never be leaked to the user - log.Debugf("Failed to delete user %q as part of update process %#v", username, err) + log.Debugf("Failed to update user %q: %#v", username, err) return fmt.Errorf("Couldn't update user information: %q", username) } + } // DeleteLocalUser removes a local user from `/ccn_proxy/local_users` @@ -127,9 +145,10 @@ func DeleteLocalUser(username string) error { return err } + key := GetPath(RootLocalUsers, username) + // handles `ErrKeyNotFound` - _, err = getLocalUser(username, stateDrv) - if err != nil { + if _, err := stateDrv.Read(key); err != nil { return err } @@ -159,14 +178,11 @@ func AddLocalUser(user *types.LocalUser) error { case nil: return ccnerrors.ErrKeyExists case ccnerrors.ErrKeyNotFound: - // `AddLocalUser` request from `UpdateLocalUser` carries hash in `user.PasswordHash` - if common.IsEmpty(string(user.PasswordHash)) { - user.PasswordHash, err = common.GenPasswordHash(user.Password) + user.PasswordHash, err = common.GenPasswordHash(user.Password) - if err != nil { - log.Debugf("Failed to create password hash for user %q: %#v", user.Username, err) - return err - } + if err != nil { + log.Debugf("Failed to create password hash for user %q: %#v", user.Username, err) + return err } // raw password will never be stored in the store diff --git a/db/local_test.go b/db/local_test.go index ac655ad9..0ee40dd1 100644 --- a/db/local_test.go +++ b/db/local_test.go @@ -46,7 +46,6 @@ var ( datastore = "" datastorePaths = []string{ GetPath(RootLocalUsers), - GetPath(RootPrincipals), GetPath(RootLdapConfiguration), } ) @@ -164,54 +163,41 @@ func (s *dbSuite) TestUpdateLocalUser(c *C) { // change the username and update newObj := &types.LocalUser{ - Username: user.Username + "_u", + Username: user.Username, PasswordHash: uUser.PasswordHash, Disable: uUser.Disable, - FirstName: uUser.FirstName, - LastName: uUser.LastName, + FirstName: uUser.FirstName + "_u", + LastName: uUser.LastName + "_u", } err = UpdateLocalUser(user.Username, newObj) c.Assert(err, IsNil) newObj.PasswordHash = uUser.PasswordHash - // search the data store for old username - oldUser, err := GetLocalUser(user.Username) - c.Assert(err, Equals, ccnerrors.ErrKeyNotFound) - c.Assert(oldUser, IsNil) - // search the data store for new username - newUser, err := GetLocalUser(user.Username + "_u") + newUser, err := GetLocalUser(user.Username) c.Assert(err, IsNil) c.Assert(newUser, DeepEquals, newObj) } // revert the changes for _, user := range newUsers { - updateTo := user.Username - user.Username = user.Username + "_u" - uUser, err := GetLocalUser(user.Username) c.Assert(err, IsNil) // change the username and update newObj := &types.LocalUser{ - Username: updateTo, + Username: uUser.Username, // same as user.Username PasswordHash: uUser.PasswordHash, Disable: uUser.Disable, - FirstName: uUser.FirstName, - LastName: uUser.LastName, + FirstName: user.FirstName, + LastName: user.LastName, } err = UpdateLocalUser(user.Username, newObj) c.Assert(err, IsNil) newObj.PasswordHash = uUser.PasswordHash - // search the data store for old username - oldUser, err := GetLocalUser(user.Username) - c.Assert(err, Equals, ccnerrors.ErrKeyNotFound) - c.Assert(oldUser, IsNil) - - newUser, err := GetLocalUser(updateTo) + newUser, err := GetLocalUser(user.Username) c.Assert(err, IsNil) c.Assert(newUser, DeepEquals, newObj) } @@ -246,3 +232,53 @@ func (s *dbSuite) TestGetLocalUsers(c *C) { c.Assert(usernames, DeepEquals, allUsers) } + +func (s *dbSuite) TestUpdateBuiltInUsers(c *C) { + s.addBuiltInUsers(c) + + // update all the details except `password` + for _, username := range builtInUsers { + user, err := GetLocalUser(username) + c.Assert(err, IsNil) + + uUser := &types.LocalUser{ + Username: user.Username, + Disable: true, + FirstName: "BuiltIn", + LastName: "User", + PasswordHash: user.PasswordHash, + } + + err = UpdateLocalUser(username, uUser) + c.Assert(err, IsNil) + uUser.PasswordHash = user.PasswordHash + + obtainedUser, err := GetLocalUser(username) + c.Assert(err, IsNil) + c.Assert(obtainedUser, DeepEquals, uUser) + } + + // update password and check hash + for _, username := range builtInUsers { + user, err := GetLocalUser(username) + c.Assert(err, IsNil) + + uUser := &types.LocalUser{ + Username: user.Username, + Password: user.Username + "_U", + } + + err = UpdateLocalUser(username, uUser) + c.Assert(err, IsNil) + + obtainedUser, err := GetLocalUser(username) + c.Assert(err, IsNil) + c.Assert(string(obtainedUser.PasswordHash), Not(Equals), string(user.PasswordHash)) + + // all other attributes got default value + c.Assert(obtainedUser.LastName, Equals, "") + c.Assert(obtainedUser.FirstName, Equals, "") + c.Assert(obtainedUser.Disable, Equals, false) + } + +} diff --git a/proxy/handlers.go b/proxy/handlers.go index 078c688c..85b45bd6 100644 --- a/proxy/handlers.go +++ b/proxy/handlers.go @@ -176,7 +176,6 @@ func adminOnly(handler func(*auth.Token, http.ResponseWriter, *http.Request)) fu // convert token from string to Token type token, err := auth.ParseToken(tokenStr) if err != nil { - log.Infof("From here???") httpStatus := http.StatusInternalServerError httpResponse := []byte(ccnerrors.ErrParsingToken.Error()) processStatusCodes(httpStatus, httpResponse, w) @@ -242,7 +241,6 @@ func deleteLocalUser(token *auth.Token, w http.ResponseWriter, req *http.Request // updateLocalUser updates the existing user with the given details. // it can return various HTTP status codes: // 204 (NoContent; update was successful) -// 400 (BadRequest; invalid role/cannot update built-in user) // 404 (NotFound; user not found) // 500 (internal server error) func updateLocalUser(token *auth.Token, w http.ResponseWriter, req *http.Request) { diff --git a/systemtests/basic_test.go b/systemtests/basic_test.go index 81cd470d..c9587df4 100644 --- a/systemtests/basic_test.go +++ b/systemtests/basic_test.go @@ -47,6 +47,7 @@ func (s *systemtestSuite) TestRequestProxying(c *C) { token := adminToken(c) resp, body := proxyGet(c, token, endpoint) + c.Assert(resp.StatusCode, Equals, 200) c.Assert(data, Equals, string(body)) // check that the Content-Type header was set properly. diff --git a/systemtests/init_test.go b/systemtests/init_test.go index a2de9781..b575f1ee 100644 --- a/systemtests/init_test.go +++ b/systemtests/init_test.go @@ -130,7 +130,7 @@ func login(username, password string) (string, *http.Response, error) { return "", nil, err } - resp, data, err := insecurePostJSONBody("", proxy.LoginPath, loginBytes) + resp, data, err := insecureJSONBody("", proxy.LoginPath, "POST", loginBytes) if err != nil { return "", resp, err } @@ -200,7 +200,6 @@ func proxyGet(c *C, token, path string) (*http.Response, []byte) { resp, err := insecureClient().Do(req) c.Assert(err, IsNil) - c.Assert(resp.StatusCode, Equals, 200) defer resp.Body.Close() @@ -210,26 +209,60 @@ func proxyGet(c *C, token, path string) (*http.Response, []byte) { return resp, data } +// proxyDelete is a convenience function which sends an insecure HTTPS DELETE +// request to the proxy. +func proxyDelete(c *C, token, path string) (*http.Response, []byte) { + url := "https://" + proxyHost + path + + log.Info("GET to ", url) + + req, err := http.NewRequest("DELETE", url, nil) + c.Assert(err, IsNil) + + if len(token) > 0 { + log.Println("Setting X-Auth-token to:", token) + req.Header.Set("X-Auth-Token", token) + } + + resp, err := insecureClient().Do(req) + c.Assert(err, IsNil) + + defer resp.Body.Close() + + data, err := ioutil.ReadAll(resp.Body) + c.Assert(err, IsNil) + + return resp, data +} + +// proxyPatch is a convenience function which sends an insecure HTTPS PATCH +// request with the specified body to the proxy. +func proxyPatch(c *C, token, path string, body []byte) (*http.Response, []byte) { + resp, body, err := insecureJSONBody(token, path, "PATCH", body) + c.Assert(err, IsNil) + + return resp, body +} + // proxyPost is a convenience function which sends an insecure HTTPS POST // request with the specified body to the proxy. func proxyPost(c *C, token, path string, body []byte) (*http.Response, []byte) { - resp, body, err := insecurePostJSONBody(token, path, body) + resp, body, err := insecureJSONBody(token, path, "POST", body) c.Assert(err, IsNil) - c.Assert(resp.StatusCode/100, Equals, 2) // 2xx is okay return resp, body } -// insecurePostBody sends an insecure HTTPS POST request with the specified +// insecureJSONBody sends an insecure HTTPS POST request with the specified // JSON payload as the body. -func insecurePostJSONBody(token, path string, body []byte) (*http.Response, []byte, error) { +func insecureJSONBody(token, path, requestType string, body []byte) (*http.Response, []byte, error) { url := "https://" + proxyHost + path - log.Info("POST to ", url) + log.Info(requestType, " to ", url) - req, err := http.NewRequest("POST", url, bytes.NewBuffer(body)) + req, err := http.NewRequest(requestType, url, bytes.NewBuffer(body)) if err != nil { - log.Debugf("POST request creation failed: %s", err) + log.Debugf("%v request creation failed: %s", requestType, err) return nil, nil, err } @@ -242,7 +275,7 @@ func insecurePostJSONBody(token, path string, body []byte) (*http.Response, []by resp, err := insecureClient().Do(req) if err != nil { - log.Debugf("POST request failed: %s", err) + log.Debugf("%v request failed: %s", requestType, err) return nil, nil, err } diff --git a/systemtests/local_user_test.go b/systemtests/local_user_test.go new file mode 100644 index 00000000..f1be2605 --- /dev/null +++ b/systemtests/local_user_test.go @@ -0,0 +1,199 @@ +package systemtests + +import ( + "github.com/contiv/ccn_proxy/common/types" + "github.com/contiv/ccn_proxy/proxy" + . "gopkg.in/check.v1" +) + +var ( + builtInUsers = []string{types.Admin.String(), types.Ops.String()} + newUsers = []string{"xxx", "yyy", "zzz"} +) + +// TestLocalUserEndpoints tests ccn_proxy's local user endpoints +func (s *systemtestSuite) TestLocalUserEndpoints(c *C) { + + runTest(func(p *proxy.Server, ms *MockServer) { + token := adminToken(c) + + for _, username := range newUsers { + endpoint := "/api/v1/ccn_proxy/local_users" + resp, body := proxyGet(c, token, endpoint) + c.Assert(resp.StatusCode, Equals, 200) + c.Assert(len(body), Not(Equals), 0) + + // add new local_user to the system + data := `{"username":"` + username + `","password":"` + username + `", "disable":false}` + respBody := `{"username":"` + username + `","first_name":"","last_name":"","disable":false}` + s.addLocalUser(c, data, respBody, token) + + // get `username` + endpoint = "/api/v1/ccn_proxy/local_users/" + username + resp, body = proxyGet(c, token, endpoint) + c.Assert(resp.StatusCode, Equals, 200) + c.Assert(string(body), DeepEquals, respBody) + + // try login using `username` + testuserToken := loginAs(c, username, username) + c.Assert(len(testuserToken), Not(Equals), 0) + + // delete `username` + resp, body = proxyDelete(c, token, endpoint) + c.Assert(resp.StatusCode, Equals, 204) + c.Assert(len(body), Equals, 0) + + // get `username` + resp, body = proxyGet(c, token, endpoint) + c.Assert(resp.StatusCode, Equals, 404) + c.Assert(len(body), Equals, 0) + } + + }) +} + +// TestLocalUserUpdateEndpoint tests ccn_proxy's local user update endpoint +func (s *systemtestSuite) TestLocalUserUpdateEndpoint(c *C) { + s.userUpdate(c) + s.builtInUserUpdate(c) +} + +// userUpdateEndpoint tests update on local user +func (s *systemtestSuite) userUpdate(c *C) { + + runTest(func(p *proxy.Server, ms *MockServer) { + token := adminToken(c) + + for _, username := range newUsers { + // add new local_user to the system + data := `{"username":"` + username + `","password":"` + username + `", "disable":false}` + respBody := `{"username":"` + username + `","first_name":"","last_name":"","disable":false}` + s.addLocalUser(c, data, respBody, token) + + // try login using `username` + testuserToken := loginAs(c, username, username) + c.Assert(len(testuserToken), Not(Equals), 0) + + // update `testuser` details + data = `{"first_name":"Temp", "last_name": "User"}` + respBody = `{"username":"` + username + `","first_name":"Temp","last_name":"User","disable":false}` + s.updateLocalUser(c, username, data, respBody, token) + + // try login again using `username` after update + testuserToken = loginAs(c, username, username) + c.Assert(len(testuserToken), Not(Equals), 0) + + // update `username`'s password + data = `{"password":"test"}` + s.updateLocalUser(c, username, data, respBody, token) + + // try login again using old password + testuserToken, resp, err := login(username, username) + c.Assert(err, IsNil) + c.Assert(resp.StatusCode, Equals, 401) + c.Assert(len(testuserToken), Equals, 0) + + // try login again using new password + testuserToken = loginAs(c, username, "test") + c.Assert(len(testuserToken), Not(Equals), 0) + } + }) +} + +// builtInUserUpdate tests built-in user update functionality +func (s *systemtestSuite) builtInUserUpdate(c *C) { + + runTest(func(p *proxy.Server, ms *MockServer) { + token := adminToken(c) + + for _, username := range builtInUsers { + // update user details + data := `{"first_name":"Built-in", "last_name": "User", "disable":false}` + respBody := `{"username":"` + username + `","first_name":"Built-in","last_name":"User","disable":false}` + s.updateLocalUser(c, username, data, respBody, token) + + // login + testuserToken := loginAs(c, username, username) + c.Assert(len(testuserToken), Not(Equals), 0) + + // update password + data = `{"password":"test"}` + s.updateLocalUser(c, username, data, respBody, token) + + // try login again using old password + testuserToken, resp, err := login(username, username) + c.Assert(err, IsNil) + c.Assert(resp.StatusCode, Equals, 401) + c.Assert(len(testuserToken), Equals, 0) + + // try login again using new password + testuserToken = loginAs(c, username, "test") + c.Assert(len(testuserToken), Not(Equals), 0) + + // revert password so that it wont block other tests + data = `{"password":"` + username + `"}` + s.updateLocalUser(c, username, data, respBody, token) + } + }) +} + +// TestLocalUserDeleteEndpoint tests ccn_proxy's local user delete endpoint +func (s *systemtestSuite) TestLocalUserDeleteEndpoint(c *C) { + + runTest(func(p *proxy.Server, ms *MockServer) { + token := adminToken(c) + + // add and delete new users + for _, username := range newUsers { + // add new local_user to the system + data := `{"username":"` + username + `","password":"` + username + `", "disable":false}` + respBody := `{"username":"` + username + `","first_name":"","last_name":"","disable":false}` + s.addLocalUser(c, data, respBody, token) + + endpoint := "/api/v1/ccn_proxy/local_users/" + username + + // delete `username` + resp, body := proxyDelete(c, token, endpoint) + c.Assert(resp.StatusCode, Equals, 204) + c.Assert(len(body), Equals, 0) + + // get `username` + resp, body = proxyGet(c, token, endpoint) + c.Assert(resp.StatusCode, Equals, 404) + c.Assert(len(body), Equals, 0) + } + + // delete built-in users + for _, username := range builtInUsers { + endpoint := "/api/v1/ccn_proxy/local_users/" + username + + // delete `username` + resp, body := proxyDelete(c, token, endpoint) + c.Assert(resp.StatusCode, Equals, 400) + c.Assert(len(body), Not(Equals), 0) + + // get `username` + resp, body = proxyGet(c, token, endpoint) + c.Assert(resp.StatusCode, Equals, 200) + c.Assert(len(body), Not(Equals), 0) + } + }) +} + +// addLocalUser helper function for the tests +func (s *systemtestSuite) addLocalUser(c *C, data, expectedRespBody, token string) { + endpoint := "/api/v1/ccn_proxy/local_users" + + resp, body := proxyPost(c, token, endpoint, []byte(data)) + c.Assert(resp.StatusCode, Equals, 201) + c.Assert(string(body), DeepEquals, expectedRespBody) +} + +// updateLocalUser helper function for the tests +func (s *systemtestSuite) updateLocalUser(c *C, username, data, expectedRespBody, token string) { + endpoint := "/api/v1/ccn_proxy/local_users/" + username + + resp, body := proxyPatch(c, token, endpoint, []byte(data)) + c.Assert(resp.StatusCode, Equals, 200) + c.Assert(string(body), DeepEquals, expectedRespBody) +}