Merge pull request #140 from uc-cdis/feat/update-user-username

(PXP-8992): PATCH /user/{username}

.secrets.baseline

@@ -3,7 +3,7 @@
     "files": "go.sum",
     "lines": null
   },
-  "generated_at": "2021-11-09T20:08:18Z",
+  "generated_at": "2021-11-29T14:09:22Z",
   "plugins_used": [
     {
       "name": "AWSKeyDetector"
@@ -62,7 +62,7 @@
       {
         "hashed_secret": "f9fdc64928c96c7ad56bf7da557f70345d83a6ed",
         "is_verified": false,
-        "line_number": 1615,
+        "line_number": 1643,
         "type": "Base64 High Entropy String"
       }
     ],

arborist/server.go

@@ -133,6 +133,7 @@ func (server *Server) MakeRouter(out io.Writer) http.Handler {
 	router.Handle("/user", http.HandlerFunc(server.handleUserList)).Methods("GET")
 	router.Handle("/user", http.HandlerFunc(server.parseJSON(server.handleUserCreate))).Methods("POST")
 	router.Handle("/user/{username}", http.HandlerFunc(server.handleUserRead)).Methods("GET")
+	router.Handle("/user/{username}", http.HandlerFunc(server.parseJSON(server.handleUserUpdate))).Methods("PATCH")
 	router.Handle("/user/{username}", http.HandlerFunc(server.handleUserDelete)).Methods("DELETE")
 	router.Handle("/user/{username}/policy", http.HandlerFunc(server.parseJSON(server.handleUserGrantPolicy))).Methods("POST")
 	router.Handle("/user/{username}/bulk/policy", http.HandlerFunc(server.parseJSON(server.handleBulkUserGrantPolicy))).Methods("POST") // NEW bulk grant policy
@@ -660,8 +661,8 @@ func (server *Server) handlePolicyList(w http.ResponseWriter, r *http.Request) {
 		// generate expanded policies with role details
 		for _, policy := range policies {
 			expandedPolicy := ExpandedPolicy{
-				Name: policy.Name,
-				Description: policy.Description,
+				Name:          policy.Name,
+				Description:   policy.Description,
 				ResourcePaths: policy.ResourcePaths,
 			}
 			roles := []Role{}
@@ -1183,6 +1184,38 @@ func (server *Server) handleUserRead(w http.ResponseWriter, r *http.Request) {
 	_ = jsonResponseFrom(user, http.StatusOK).write(w, r)
 }
 
+func (server *Server) handleUserUpdate(w http.ResponseWriter, r *http.Request, body []byte) {
+	name := mux.Vars(r)["username"]
+	user := User{Name: name}
+
+	userWithScalars := &UserWithScalars{}
+	err := json.Unmarshal(body, userWithScalars)
+	if err != nil {
+		msg := fmt.Sprintf("could not unmarshal body: %s", err.Error())
+		errResponse := newErrorResponse(msg, 400, nil)
+		errResponse.log.write(server.logger)
+		_ = errResponse.write(w, r)
+		return
+	}
+
+	if userWithScalars.Name == nil && userWithScalars.Email == nil {
+		msg := `body must contain at least one valid field. possible valid fields are "name" and "email"`
+		errResponse := newErrorResponse(msg, 400, nil)
+		errResponse.log.write(server.logger)
+		_ = errResponse.write(w, r)
+		return
+	}
+
+	errResponse := user.updateInDb(server.db, userWithScalars.Name, userWithScalars.Email)
+	if errResponse != nil {
+		errResponse.log.write(server.logger)
+		_ = errResponse.write(w, r)
+		return
+	}
+	server.logger.Info("updated user %s", user.Name)
+	_ = jsonResponseFrom(nil, http.StatusNoContent).write(w, r)
+}
+
 func (server *Server) handleUserDelete(w http.ResponseWriter, r *http.Request) {
 	name := mux.Vars(r)["username"]
 	user := User{Name: name}

arborist/server_test.go

@@ -283,6 +283,34 @@ func TestServer(t *testing.T) {
 		}
 	}
 
+	updateUserBytes := func(t *testing.T, username string, body []byte, expectedHTTPCode int) {
+		w := httptest.NewRecorder()
+		url := fmt.Sprintf("/user/%s", username)
+		req := newRequest("PATCH", url, bytes.NewBuffer(body))
+		handler.ServeHTTP(w, req)
+		assert.Equalf(t, expectedHTTPCode, w.Code, "Wanted http response: %v \t Got: %v", expectedHTTPCode, w.Code)
+	}
+
+	assertUsernameAndEmail := func(t *testing.T, expectedUsername string, expectedUserEmail string) {
+		w := httptest.NewRecorder()
+		url := fmt.Sprintf("/user/%s", expectedUsername)
+		req := newRequest("GET", url, nil)
+		handler.ServeHTTP(w, req)
+		if w.Code != http.StatusOK {
+			httpError(t, w, "couldn't read user")
+		}
+		result := struct {
+			Name  string `json:"name"`
+			Email string `json:"email"`
+		}{}
+		err = json.Unmarshal(w.Body.Bytes(), &result)
+		if err != nil {
+			httpError(t, w, "couldn't read response from user read")
+		}
+		assert.Equalf(t, expectedUsername, result.Name, "Wanted username: %v \t Got: %v", expectedUsername, result.Name)
+		assert.Equalf(t, expectedUserEmail, result.Email, "Wanted email: %v \t Got: %v", expectedUserEmail, result.Email)
+	}
+
 	createResourceBytes := func(t *testing.T, body []byte) {
 		w := httptest.NewRecorder()
 		req := newRequest("POST", "/resource", bytes.NewBuffer(body))
@@ -1688,6 +1716,86 @@ func TestServer(t *testing.T) {
 			assert.ElementsMatchf(t, expectedGroups, result.Groups, "Wanted groups: %v \t Got: %v", expectedGroups, result.Groups)
 		})
 
+		t.Run("Update", func(t *testing.T) {
+			originalUsername := "johnsmith"
+			originalUserEmail := "johnsmith@domain.tld"
+			createUserBytes(t, []byte(fmt.Sprintf(`{"name": "%s", "email": "%s"}`, originalUsername, originalUserEmail)))
+
+			t.Run("OnlyName", func(t *testing.T) {
+				newUsername := "jsmith"
+				updateUserBytes(
+					t,
+					originalUsername,
+					[]byte(fmt.Sprintf(`{"name": "%s"}`, newUsername)),
+					http.StatusNoContent,
+				)
+				assertUsernameAndEmail(t, newUsername, originalUserEmail)
+			})
+
+			originalUsername = "jsmith"
+			t.Run("OnlyEmail", func(t *testing.T) {
+				newUserEmail := "jsmith@domain.tld"
+				updateUserBytes(
+					t,
+					originalUsername,
+					[]byte(fmt.Sprintf(`{"email": "%s"}`, newUserEmail)),
+					http.StatusNoContent,
+				)
+				assertUsernameAndEmail(t, originalUsername, newUserEmail)
+			})
+
+			t.Run("BothNameAndEmail", func(t *testing.T) {
+				newUsername := "janesmith"
+				newUserEmail := "janesmith@domain.tld"
+				updateUserBytes(
+					t,
+					originalUsername,
+					[]byte(fmt.Sprintf(`{"name": "%s", "email": "%s"}`, newUsername, newUserEmail)),
+					http.StatusNoContent,
+				)
+				assertUsernameAndEmail(t, newUsername, newUserEmail)
+			})
+
+			originalUsername = "janesmith"
+			t.Run("NeitherNameNorEmail", func(t *testing.T) {
+				updateUserBytes(
+					t,
+					originalUsername,
+					[]byte("{}"),
+					http.StatusBadRequest,
+				)
+			})
+
+			t.Run("InvalidName", func(t *testing.T) {
+				updateUserBytes(
+					t,
+					originalUsername,
+					[]byte(`{"name": 42}`),
+					http.StatusBadRequest,
+				)
+			})
+
+			t.Run("NonExistentUsername", func(t *testing.T) {
+				updateUserBytes(
+					t,
+					"nonexistentuser",
+					[]byte(`{"name": "existentuser"}`),
+					http.StatusNotFound,
+				)
+			})
+
+			t.Run("ConflictingName", func(t *testing.T) {
+				otherUsername := "joesmith"
+				createUserBytes(t, []byte(fmt.Sprintf(`{"name": "%s"}`, otherUsername)))
+				updateUserBytes(
+					t,
+					originalUsername,
+					[]byte(fmt.Sprintf(`{"name": "%s"}`, otherUsername)),
+					http.StatusConflict,
+				)
+			})
+		})
+
 		// do some preliminary setup so we have a policy to work with
 		createResourceBytes(t, resourceBody)
 		createRoleBytes(t, roleBody)

arborist/user.go

@@ -22,6 +22,11 @@ type User struct {
 	Policies []PolicyBinding `json:"policies"`
 }
 
+type UserWithScalars struct {
+	Name  *string `json:"name,omitempty"`
+	Email *string `json:"email,omitempty"`
+}
+
 func (user *User) UnmarshalJSON(data []byte) error {
 	fields := make(map[string]interface{})
 	err := json.Unmarshal(data, &fields)
@@ -210,6 +215,33 @@ func (user *User) createInDb(db *sqlx.DB) *ErrorResponse {
 	return nil
 }
 
+func (user *User) updateInDb(db *sqlx.DB, name *string, email *string) *ErrorResponse {
+	stmt := `
+		UPDATE usr
+		SET
+			name = COALESCE($1, name),
+			email = COALESCE($2, email)
+		WHERE
+			name = $3
+	`
+	result, err := db.Exec(stmt, name, email, user.Name)
+	if err != nil {
+		// this should only fail because the target name was not unique
+		msg := fmt.Sprintf(`failed to update name to "%s": user with this name already exists`, *name)
+		return newErrorResponse(msg, 409, &err)
+	}
+
+	rowsAffected, _ := result.RowsAffected()
+	if rowsAffected == 0 {
+		msg := fmt.Sprintf(
+			"failed to update user: user does not exist: %s",
+			user.Name,
+		)
+		return newErrorResponse(msg, 404, nil)
+	}
+	return nil
+}
+
 func (user *User) deleteInDb(db *sqlx.DB) *ErrorResponse {
 	stmt := "DELETE FROM usr WHERE name = $1"
 	_, err := db.Exec(stmt, user.Name)

docs/openapi.yaml

@@ -845,6 +845,25 @@ paths:
                 $ref: '#/components/schemas/User'
         404:
           description: user not found
+    patch:
+      tags:
+        - user
+      description: >-
+        Update either the name, email or both for a specific user.
+      requestBody:
+        content:
+          application/json:
+            schema:
+              $ref: '#/components/schemas/UserWithScalars'
+      responses:
+        204:
+          description: user successfully updated
+        400:
+          description: invalid input
+        404:
+          description: user not found
+        409:
+          description: user with target name already exists
     delete:
       tags:
         - user
@@ -1599,6 +1618,15 @@ components:
               expires_at:
                 type: string
                 example: '2019-08-12T12:34:56Z'
+    UserWithScalars:
+      type: object
+      properties:
+        name:
+          type: string
+          example: 'username'
+        email:
+          type: string
+          example: 'user@example.net'
     UsersList:
       type: object
       properties: