From a3fab39cf02067bede3b84ef98ddf91af8321679 Mon Sep 17 00:00:00 2001 From: mattgarvin1 Date: Tue, 6 Oct 2020 14:14:17 -0500 Subject: [PATCH 01/15] create role update endpoint; add create role logic --- arborist/server.go | 50 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/arborist/server.go b/arborist/server.go index d486d62f..a6ff8304 100644 --- a/arborist/server.go +++ b/arborist/server.go @@ -121,6 +121,10 @@ func (server *Server) MakeRouter(out io.Writer) http.Handler { router.Handle("/role", http.HandlerFunc(server.handleRoleList)).Methods("GET") router.Handle("/role", http.HandlerFunc(server.parseJSON(server.handleRoleCreate))).Methods("POST") router.Handle("/role/{roleID}", http.HandlerFunc(server.handleRoleRead)).Methods("GET") + + // new "update role" endpoint + router.Handle("/role/{roleID}", http.HandlerFunc(server.handleRoleUpdate)).Methods("PUT") + router.Handle("/role/{roleID}", http.HandlerFunc(server.handleRoleDelete)).Methods("DELETE") router.Handle("/user", http.HandlerFunc(server.handleUserList)).Methods("GET") @@ -937,6 +941,52 @@ func (server *Server) handleRoleRead(w http.ResponseWriter, r *http.Request) { _ = jsonResponseFrom(role, http.StatusOK).write(w, r) } +// new method for action "update role" +// updates existing role, +// or creates it if it doesn't already exist +// +// http response codes reference: https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/PUT +func (server *Server) handleRoleUpdate(w http.ResponseWriter, r *http.Request) { + + // extract role name + name := mux.Vars(r)["roleID"] + + // query db for this role + roleFromQuery, err := roleWithName(server.db, name) + if err != nil { + msg := fmt.Sprintf("role query failed: %s", err.Error()) + errResponse := newErrorResponse(msg, 500, nil) + errResponse.log.write(server.logger) + _ = errResponse.write(w, r) + return + } + + // if it doesn't exist in the db, then create the role + if roleFromQuery == nil { + role := roleFromQuery.standardize() + errResponse := role.createInDb(server.db) + if errResponse != nil { + errResponse.log.write(server.logger) + _ = errResponse.write(w, r) + return + } + server.logger.Info("created role %s", role.Name) + created := struct { + Created *Role `json:"created"` + }{ + Created: &role, + } + _ = jsonResponseFrom(created, 201).write(w, r) + return + } + + // HERE - if it DOES exist in the db - update it + + // # from handleRoleRead() + // role := roleFromQuery.standardize() + // _ = jsonResponseFrom(role, http.StatusOK).write(w, r) +} + func (server *Server) handleRoleDelete(w http.ResponseWriter, r *http.Request) { name := mux.Vars(r)["roleID"] role := &Role{Name: name} From 9a10736ba77701a2fc21236ac5ad7ce9a1077f19 Mon Sep 17 00:00:00 2001 From: mattgarvin1 Date: Tue, 6 Oct 2020 14:51:16 -0500 Subject: [PATCH 02/15] fix handler function name --- arborist/server.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arborist/server.go b/arborist/server.go index a6ff8304..2f8366a6 100644 --- a/arborist/server.go +++ b/arborist/server.go @@ -123,7 +123,7 @@ func (server *Server) MakeRouter(out io.Writer) http.Handler { router.Handle("/role/{roleID}", http.HandlerFunc(server.handleRoleRead)).Methods("GET") // new "update role" endpoint - router.Handle("/role/{roleID}", http.HandlerFunc(server.handleRoleUpdate)).Methods("PUT") + router.Handle("/role/{roleID}", http.HandlerFunc(server.handleRoleOverwrite)).Methods("PUT") router.Handle("/role/{roleID}", http.HandlerFunc(server.handleRoleDelete)).Methods("DELETE") @@ -946,7 +946,7 @@ func (server *Server) handleRoleRead(w http.ResponseWriter, r *http.Request) { // or creates it if it doesn't already exist // // http response codes reference: https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/PUT -func (server *Server) handleRoleUpdate(w http.ResponseWriter, r *http.Request) { +func (server *Server) handleRoleOverwrite(w http.ResponseWriter, r *http.Request) { // extract role name name := mux.Vars(r)["roleID"] From f849130ffc0d56b18f7044bc24edc37bf952a04a Mon Sep 17 00:00:00 2001 From: mattgarvin1 Date: Tue, 6 Oct 2020 14:57:26 -0500 Subject: [PATCH 03/15] fix fn signature --- arborist/server.go | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/arborist/server.go b/arborist/server.go index 2f8366a6..f93bca5f 100644 --- a/arborist/server.go +++ b/arborist/server.go @@ -122,8 +122,8 @@ func (server *Server) MakeRouter(out io.Writer) http.Handler { router.Handle("/role", http.HandlerFunc(server.parseJSON(server.handleRoleCreate))).Methods("POST") router.Handle("/role/{roleID}", http.HandlerFunc(server.handleRoleRead)).Methods("GET") - // new "update role" endpoint - router.Handle("/role/{roleID}", http.HandlerFunc(server.handleRoleOverwrite)).Methods("PUT") + // new "overwrite role" endpoint + router.Handle("/role/{roleID}", http.HandlerFunc(server.parseJSON(server.handleRoleOverwrite))).Methods("POST") router.Handle("/role/{roleID}", http.HandlerFunc(server.handleRoleDelete)).Methods("DELETE") @@ -941,12 +941,12 @@ func (server *Server) handleRoleRead(w http.ResponseWriter, r *http.Request) { _ = jsonResponseFrom(role, http.StatusOK).write(w, r) } -// new method for action "update role" +// new method for action "overwrite role" // updates existing role, // or creates it if it doesn't already exist // // http response codes reference: https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/PUT -func (server *Server) handleRoleOverwrite(w http.ResponseWriter, r *http.Request) { +func (server *Server) handleRoleOverwrite(w http.ResponseWriter, r *http.Request, body []byte) { // extract role name name := mux.Vars(r)["roleID"] @@ -963,7 +963,15 @@ func (server *Server) handleRoleOverwrite(w http.ResponseWriter, r *http.Request // if it doesn't exist in the db, then create the role if roleFromQuery == nil { - role := roleFromQuery.standardize() + role := &Role{} + err := json.Unmarshal(body, role) + if err != nil { + msg := fmt.Sprintf("could not parse role from JSON: %s", err.Error()) + server.logger.Info("tried to create role but input was invalid: %s", msg) + response := newErrorResponse(msg, 400, nil) + _ = response.write(w, r) + return + } errResponse := role.createInDb(server.db) if errResponse != nil { errResponse.log.write(server.logger) @@ -974,7 +982,7 @@ func (server *Server) handleRoleOverwrite(w http.ResponseWriter, r *http.Request created := struct { Created *Role `json:"created"` }{ - Created: &role, + Created: role, } _ = jsonResponseFrom(created, 201).write(w, r) return From d4267a1194c8fcd27fcde3f9410082a51fe2a435 Mon Sep 17 00:00:00 2001 From: mattgarvin1 Date: Tue, 6 Oct 2020 15:00:18 -0500 Subject: [PATCH 04/15] refactor --- arborist/server.go | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/arborist/server.go b/arborist/server.go index f93bca5f..0aae837f 100644 --- a/arborist/server.go +++ b/arborist/server.go @@ -942,16 +942,12 @@ func (server *Server) handleRoleRead(w http.ResponseWriter, r *http.Request) { } // new method for action "overwrite role" -// updates existing role, +// overwrites existing role, // or creates it if it doesn't already exist // // http response codes reference: https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/PUT func (server *Server) handleRoleOverwrite(w http.ResponseWriter, r *http.Request, body []byte) { - - // extract role name name := mux.Vars(r)["roleID"] - - // query db for this role roleFromQuery, err := roleWithName(server.db, name) if err != nil { msg := fmt.Sprintf("role query failed: %s", err.Error()) @@ -961,7 +957,6 @@ func (server *Server) handleRoleOverwrite(w http.ResponseWriter, r *http.Request return } - // if it doesn't exist in the db, then create the role if roleFromQuery == nil { role := &Role{} err := json.Unmarshal(body, role) @@ -988,11 +983,8 @@ func (server *Server) handleRoleOverwrite(w http.ResponseWriter, r *http.Request return } - // HERE - if it DOES exist in the db - update it + // HERE - overwrite existing role - // # from handleRoleRead() - // role := roleFromQuery.standardize() - // _ = jsonResponseFrom(role, http.StatusOK).write(w, r) } func (server *Server) handleRoleDelete(w http.ResponseWriter, r *http.Request) { From 3ef1b5153f44384d25c1c47f392b1e32c566d103 Mon Sep 17 00:00:00 2001 From: mattgarvin1 Date: Tue, 6 Oct 2020 15:48:07 -0500 Subject: [PATCH 05/15] create empty overwrite in db role method --- arborist/role.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arborist/role.go b/arborist/role.go index 2ed7317d..cb8f2ff5 100644 --- a/arborist/role.go +++ b/arborist/role.go @@ -210,6 +210,10 @@ func (role *Role) createInDb(db *sqlx.DB) *ErrorResponse { return nil } +func (role *Role) overwriteInDb(db *sqlx.DB) *ErrorResponse { + return nil +} + func (role *Role) deleteInDb(db *sqlx.DB) *ErrorResponse { stmt := "DELETE FROM role WHERE name = $1" _, err := db.Exec(stmt, role.Name) From faaf0ffe9fd93659965a6a0b134d038edc14df61 Mon Sep 17 00:00:00 2001 From: mattgarvin1 Date: Tue, 6 Oct 2020 16:12:21 -0500 Subject: [PATCH 06/15] upsert role --- arborist/role.go | 69 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/arborist/role.go b/arborist/role.go index cb8f2ff5..dbb7b98c 100644 --- a/arborist/role.go +++ b/arborist/role.go @@ -211,6 +211,75 @@ func (role *Role) createInDb(db *sqlx.DB) *ErrorResponse { } func (role *Role) overwriteInDb(db *sqlx.DB) *ErrorResponse { + errResponse := role.validate() + if errResponse != nil { + return errResponse + } + + tx, err := db.Beginx() + if err != nil { + msg := fmt.Sprintf("couldn't open database transaction: %s", err.Error()) + return newErrorResponse(msg, 500, &err) + } + + var roleID int + stmt := ` + INSERT INTO role(name, description) + VALUES ($1, $2) + ON CONFLICT(name) DO UPDATE + SET description = $2 + RETURNING id + ` + row := tx.QueryRowx(stmt, role.Name, role.Description) + err = row.Scan(&roleID) + if err != nil { + // handle err + // not sure what the possible errors are here + } + + // good through here // + + // create permissions as necessary + // permissions are unique per combination of role_id + name + permissionTable := "permission(role_id, name, service, method, constraints, description)" + stmt = multiInsertStmt(permissionTable, len(role.Permissions)) + stmt += " ON CONFLICT DO NOTHING" + permissionRows := []interface{}{} + for _, permission := range role.Permissions { + constraints, err := json.Marshal(permission.Constraints) + if err != nil { + _ = tx.Rollback() + msg := fmt.Sprintf( + "couldn't write constraints for permission %s: %s", + permission.Name, + err.Error(), + ) + return newErrorResponse(msg, 500, &err) + } + row := []interface{}{ + roleID, + permission.Name, + permission.Action.Service, + permission.Action.Method, + constraints, + permission.Description, + } + permissionRows = append(permissionRows, row...) + } + _, err = tx.Exec(stmt, permissionRows...) + if err != nil { + _ = tx.Rollback() + msg := fmt.Sprintf("couldn't create permissions: %s", err.Error()) + return newErrorResponse(msg, 500, &err) + } + + err = tx.Commit() + if err != nil { + _ = tx.Rollback() + msg := fmt.Sprintf("couldn't commit database transaction: %s", err.Error()) + return newErrorResponse(msg, 500, &err) + } + return nil } From a9e6d7aeeab9e63d396942c544cd69e8b1cee9b8 Mon Sep 17 00:00:00 2001 From: mattgarvin1 Date: Tue, 6 Oct 2020 16:37:16 -0500 Subject: [PATCH 07/15] upsert permissions --- arborist/role.go | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/arborist/role.go b/arborist/role.go index dbb7b98c..7c0f4384 100644 --- a/arborist/role.go +++ b/arborist/role.go @@ -233,18 +233,23 @@ func (role *Role) overwriteInDb(db *sqlx.DB) *ErrorResponse { row := tx.QueryRowx(stmt, role.Name, role.Description) err = row.Scan(&roleID) if err != nil { - // handle err - // not sure what the possible errors are here + _ = tx.Rollback() + msg := fmt.Sprintf("couldn't overwrite role: %s", err.Error()) + return newErrorResponse(msg, 500, &err) } - // good through here // - - // create permissions as necessary + // upsert permissions // permissions are unique per combination of role_id + name - permissionTable := "permission(role_id, name, service, method, constraints, description)" - stmt = multiInsertStmt(permissionTable, len(role.Permissions)) - stmt += " ON CONFLICT DO NOTHING" - permissionRows := []interface{}{} + stmt = ` + INSERT INTO permission(role_id, name, service, method, constraints, description) + VALUES ($1, $2, $3, $4, $5, $6) + ON CONFLICT(role_id, name) DO UPDATE + SET + service = $3 + method = $4 + constraints = $5 + description = $6 + ` for _, permission := range role.Permissions { constraints, err := json.Marshal(permission.Constraints) if err != nil { @@ -256,21 +261,20 @@ func (role *Role) overwriteInDb(db *sqlx.DB) *ErrorResponse { ) return newErrorResponse(msg, 500, &err) } - row := []interface{}{ + _, err = tx.Exec( + stmt, roleID, permission.Name, permission.Action.Service, permission.Action.Method, constraints, permission.Description, + ) + if err != nil { + _ = tx.Rollback() + msg := fmt.Sprintf("couldn't overwrite permissions: %s", err.Error()) + return newErrorResponse(msg, 500, &err) } - permissionRows = append(permissionRows, row...) - } - _, err = tx.Exec(stmt, permissionRows...) - if err != nil { - _ = tx.Rollback() - msg := fmt.Sprintf("couldn't create permissions: %s", err.Error()) - return newErrorResponse(msg, 500, &err) } err = tx.Commit() From 110a06b35e8828dfedd3f59efc27cd7269896a0e Mon Sep 17 00:00:00 2001 From: mattgarvin1 Date: Tue, 6 Oct 2020 16:44:27 -0500 Subject: [PATCH 08/15] add commas --- arborist/role.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arborist/role.go b/arborist/role.go index 7c0f4384..ef3a1052 100644 --- a/arborist/role.go +++ b/arborist/role.go @@ -245,9 +245,9 @@ func (role *Role) overwriteInDb(db *sqlx.DB) *ErrorResponse { VALUES ($1, $2, $3, $4, $5, $6) ON CONFLICT(role_id, name) DO UPDATE SET - service = $3 - method = $4 - constraints = $5 + service = $3, + method = $4, + constraints = $5, description = $6 ` for _, permission := range role.Permissions { From 8314a46cc92b5ae6e6ed63a6d8fd416d65a30742 Mon Sep 17 00:00:00 2001 From: mattgarvin1 Date: Tue, 6 Oct 2020 16:54:56 -0500 Subject: [PATCH 09/15] add role overwritedb to role overwrite handler --- arborist/server.go | 44 ++++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/arborist/server.go b/arborist/server.go index 0aae837f..160a783b 100644 --- a/arborist/server.go +++ b/arborist/server.go @@ -942,10 +942,6 @@ func (server *Server) handleRoleRead(w http.ResponseWriter, r *http.Request) { } // new method for action "overwrite role" -// overwrites existing role, -// or creates it if it doesn't already exist -// -// http response codes reference: https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/PUT func (server *Server) handleRoleOverwrite(w http.ResponseWriter, r *http.Request, body []byte) { name := mux.Vars(r)["roleID"] roleFromQuery, err := roleWithName(server.db, name) @@ -957,17 +953,21 @@ func (server *Server) handleRoleOverwrite(w http.ResponseWriter, r *http.Request return } + role := &Role{} + err = json.Unmarshal(body, role) + if err != nil { + msg := fmt.Sprintf("could not parse role from JSON: %s", err.Error()) + server.logger.Info("tried to create role but input was invalid: %s", msg) + response := newErrorResponse(msg, 400, nil) + _ = response.write(w, r) + return + } + + var errResponse *ErrorResponse + + // role doesn't exist in db - create it if roleFromQuery == nil { - role := &Role{} - err := json.Unmarshal(body, role) - if err != nil { - msg := fmt.Sprintf("could not parse role from JSON: %s", err.Error()) - server.logger.Info("tried to create role but input was invalid: %s", msg) - response := newErrorResponse(msg, 400, nil) - _ = response.write(w, r) - return - } - errResponse := role.createInDb(server.db) + errResponse = role.createInDb(server.db) if errResponse != nil { errResponse.log.write(server.logger) _ = errResponse.write(w, r) @@ -983,8 +983,20 @@ func (server *Server) handleRoleOverwrite(w http.ResponseWriter, r *http.Request return } - // HERE - overwrite existing role - + // role exists in db - overwrite it + errResponse = role.overwriteInDb(server.db) + if errResponse != nil { + errResponse.log.write(server.logger) + _ = errResponse.write(w, r) + return + } + server.logger.Info("updated role %s", role.Name) + updated := struct { + Updated *Role `json:"updated"` + }{ + Updated: role, + } + _ = jsonResponseFrom(updated, 200).write(w, r) } func (server *Server) handleRoleDelete(w http.ResponseWriter, r *http.Request) { From c96f978b1b4e3fb53c34424bbeb03a074401a743 Mon Sep 17 00:00:00 2001 From: mattgarvin1 Date: Tue, 6 Oct 2020 16:56:53 -0500 Subject: [PATCH 10/15] fix overwrite http method to PUT not POST --- arborist/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arborist/server.go b/arborist/server.go index 160a783b..aabdd5cd 100644 --- a/arborist/server.go +++ b/arborist/server.go @@ -123,7 +123,7 @@ func (server *Server) MakeRouter(out io.Writer) http.Handler { router.Handle("/role/{roleID}", http.HandlerFunc(server.handleRoleRead)).Methods("GET") // new "overwrite role" endpoint - router.Handle("/role/{roleID}", http.HandlerFunc(server.parseJSON(server.handleRoleOverwrite))).Methods("POST") + router.Handle("/role/{roleID}", http.HandlerFunc(server.parseJSON(server.handleRoleOverwrite))).Methods("PUT") router.Handle("/role/{roleID}", http.HandlerFunc(server.handleRoleDelete)).Methods("DELETE") From 02ce6b3a20951a80d00b3d0b6aceaa9192e4ad7b Mon Sep 17 00:00:00 2001 From: mattgarvin1 Date: Mon, 12 Oct 2020 17:29:58 -0500 Subject: [PATCH 11/15] annotate spot for new test --- arborist/server_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arborist/server_test.go b/arborist/server_test.go index cd756b87..20dd4f4e 100644 --- a/arborist/server_test.go +++ b/arborist/server_test.go @@ -1165,6 +1165,9 @@ func TestServer(t *testing.T) { assert.Equal(t, "foo", result.Name, msg) }) + // HERE - write test for new PUT /role/{role_name} + // t.Run("Overwrite", func(t *testing.T) {...}) + t.Run("List", func(t *testing.T) { w := httptest.NewRecorder() req := newRequest("GET", "/role", nil) From b41fa5dca836ebe08ded95a4e63b47edf96b1818 Mon Sep 17 00:00:00 2001 From: mattgarvin1 Date: Mon, 12 Oct 2020 22:37:11 -0500 Subject: [PATCH 12/15] write two tests for new PUT endpoint --- arborist/server_test.go | 47 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/arborist/server_test.go b/arborist/server_test.go index 20dd4f4e..3a25c048 100644 --- a/arborist/server_test.go +++ b/arborist/server_test.go @@ -1121,6 +1121,29 @@ func TestServer(t *testing.T) { httpError(t, w, "couldn't read response from role creation") } + t.Run("OverwriteCreate", func(t *testing.T) { + w := httptest.NewRecorder() + body := []byte(`{ + "id": "thisNewRole", + "permissions": [ + {"id": "thisNewID", "action": {"service": "test-overwrite", "method": "bar"}} + ] + }`) + req := newRequest("PUT", "/role/thisNewRole", bytes.NewBuffer(body)) + handler.ServeHTTP(w, req) + if w.Code != http.StatusCreated { + httpError(t, w, "couldn't create role") + } + // make one-off struct to read the response into + result := struct { + _ interface{} `json:"created"` + }{} + err = json.Unmarshal(w.Body.Bytes(), &result) + if err != nil { + httpError(t, w, "couldn't read response from role creation") + } + }) + t.Run("AlreadyExists", func(t *testing.T) { w := httptest.NewRecorder() body := []byte(`{ @@ -1165,8 +1188,28 @@ func TestServer(t *testing.T) { assert.Equal(t, "foo", result.Name, msg) }) - // HERE - write test for new PUT /role/{role_name} - // t.Run("Overwrite", func(t *testing.T) {...}) + t.Run("Overwrite", func(t *testing.T) { + w := httptest.NewRecorder() + body := []byte(`{ + "id": "foo", + "permissions": [ + {"id": "foo", "action": {"service": "*", "method": "bar"}} + ] + }`) + req := newRequest("PUT", "/role/foo", bytes.NewBuffer(body)) + handler.ServeHTTP(w, req) + if w.Code != http.StatusOK { + httpError(t, w, "couldn't update role") + } + // make one-off struct to read the response into + result := struct { + _ interface{} `json:"updated"` + }{} + err = json.Unmarshal(w.Body.Bytes(), &result) + if err != nil { + httpError(t, w, "couldn't read response from role overwrite") + } + }) t.Run("List", func(t *testing.T) { w := httptest.NewRecorder() From d6281d3d4dbf372fd72e30fb9b9290f084ad2766 Mon Sep 17 00:00:00 2001 From: mattgarvin1 Date: Mon, 12 Oct 2020 22:41:53 -0500 Subject: [PATCH 13/15] cleanup for PR --- arborist/server.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/arborist/server.go b/arborist/server.go index aabdd5cd..2ea23471 100644 --- a/arborist/server.go +++ b/arborist/server.go @@ -121,10 +121,7 @@ func (server *Server) MakeRouter(out io.Writer) http.Handler { router.Handle("/role", http.HandlerFunc(server.handleRoleList)).Methods("GET") router.Handle("/role", http.HandlerFunc(server.parseJSON(server.handleRoleCreate))).Methods("POST") router.Handle("/role/{roleID}", http.HandlerFunc(server.handleRoleRead)).Methods("GET") - - // new "overwrite role" endpoint router.Handle("/role/{roleID}", http.HandlerFunc(server.parseJSON(server.handleRoleOverwrite))).Methods("PUT") - router.Handle("/role/{roleID}", http.HandlerFunc(server.handleRoleDelete)).Methods("DELETE") router.Handle("/user", http.HandlerFunc(server.handleUserList)).Methods("GET") @@ -941,7 +938,6 @@ func (server *Server) handleRoleRead(w http.ResponseWriter, r *http.Request) { _ = jsonResponseFrom(role, http.StatusOK).write(w, r) } -// new method for action "overwrite role" func (server *Server) handleRoleOverwrite(w http.ResponseWriter, r *http.Request, body []byte) { name := mux.Vars(r)["roleID"] roleFromQuery, err := roleWithName(server.db, name) @@ -964,8 +960,6 @@ func (server *Server) handleRoleOverwrite(w http.ResponseWriter, r *http.Request } var errResponse *ErrorResponse - - // role doesn't exist in db - create it if roleFromQuery == nil { errResponse = role.createInDb(server.db) if errResponse != nil { @@ -983,7 +977,6 @@ func (server *Server) handleRoleOverwrite(w http.ResponseWriter, r *http.Request return } - // role exists in db - overwrite it errResponse = role.overwriteInDb(server.db) if errResponse != nil { errResponse.log.write(server.logger) From ccbb229e2a4cbc54b1d14f9c61c2bbd21051a2b2 Mon Sep 17 00:00:00 2001 From: mattgarvin1 Date: Mon, 12 Oct 2020 22:48:32 -0500 Subject: [PATCH 14/15] fix list roles test to expect additional role from new overwrite test --- arborist/server_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arborist/server_test.go b/arborist/server_test.go index 3a25c048..af9b409e 100644 --- a/arborist/server_test.go +++ b/arborist/server_test.go @@ -1226,7 +1226,7 @@ func TestServer(t *testing.T) { httpError(t, w, "couldn't read response from roles list") } msg := fmt.Sprintf("got response body: %s", w.Body.String()) - assert.Equal(t, 1, len(result.Roles), msg) + assert.Equal(t, 2, len(result.Roles), msg) }) t.Run("Delete", func(t *testing.T) { From 662d8ef5dca2082207257fa4331c137528900247 Mon Sep 17 00:00:00 2001 From: mattgarvin1 Date: Mon, 19 Oct 2020 14:49:24 -0500 Subject: [PATCH 15/15] add validation to check URL roleID vs JSON roleID --- arborist/server.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/arborist/server.go b/arborist/server.go index 2ea23471..5bc25c0d 100644 --- a/arborist/server.go +++ b/arborist/server.go @@ -939,7 +939,25 @@ func (server *Server) handleRoleRead(w http.ResponseWriter, r *http.Request) { } func (server *Server) handleRoleOverwrite(w http.ResponseWriter, r *http.Request, body []byte) { + role := &Role{} + err := json.Unmarshal(body, role) + if err != nil { + msg := fmt.Sprintf("could not parse role from JSON: %s", err.Error()) + server.logger.Info("tried to overwrite role but input was invalid: %s", msg) + response := newErrorResponse(msg, 400, nil) + _ = response.write(w, r) + return + } + name := mux.Vars(r)["roleID"] + if name != role.Name { + msg := fmt.Sprintf("roleID '%s' from URL did not match roleID '%s' from JSON", name, role.Name) + server.logger.Info("tried to overwrite role but input was invalid: %s", msg) + response := newErrorResponse(msg, 400, nil) + _ = response.write(w, r) + return + } + roleFromQuery, err := roleWithName(server.db, name) if err != nil { msg := fmt.Sprintf("role query failed: %s", err.Error()) @@ -949,16 +967,6 @@ func (server *Server) handleRoleOverwrite(w http.ResponseWriter, r *http.Request return } - role := &Role{} - err = json.Unmarshal(body, role) - if err != nil { - msg := fmt.Sprintf("could not parse role from JSON: %s", err.Error()) - server.logger.Info("tried to create role but input was invalid: %s", msg) - response := newErrorResponse(msg, 400, nil) - _ = response.write(w, r) - return - } - var errResponse *ErrorResponse if roleFromQuery == nil { errResponse = role.createInDb(server.db)