Allow updates(first_name, last_name, disable, password) to built-in u…

…sers

Signed-off-by: Yuva Shankar <[email protected]>

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,32 +58,74 @@ 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/<username>.
 // params:
 //  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

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)
+	}
+
+}

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) {

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.

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
 	}