From d8680974a4bb8240ef9e19a7e27298039084fb02 Mon Sep 17 00:00:00 2001 From: Zheng_Zhi_Qiang Date: Thu, 27 Jun 2024 22:29:46 +0800 Subject: [PATCH 1/2] fix: enable frontend to not include role in req for delete --- backend/src/server/middleware.go | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/backend/src/server/middleware.go b/backend/src/server/middleware.go index a31c2eb0..f2364ebd 100644 --- a/backend/src/server/middleware.go +++ b/backend/src/server/middleware.go @@ -303,17 +303,19 @@ func validateMembershipChange(postgresClient *sql.DB, next http.Handler, logger return } - err = models.ValidateRole(targetMembership.Role) - if err != nil { - logger.Error("unable to add/update membership as role is invalid") - encode(w, r, http.StatusBadRequest, newHandlerError(err, http.StatusBadRequest)) - return - } + if r.Method != http.MethodDelete { + err = models.ValidateRole(targetMembership.Role) + if err != nil { + logger.Error("unable to add/update membership as role is invalid") + encode(w, r, http.StatusBadRequest, newHandlerError(err, http.StatusBadRequest)) + return + } - if targetMembership.Role == models.Owner { - logger.Error("unable to grant/delete ownership") - encode(w, r, http.StatusForbidden, newHandlerError(ErrUnableModifyOwnership, http.StatusForbidden)) - return + if targetMembership.Role == models.Owner { + logger.Error("unable to grant/delete ownership") + encode(w, r, http.StatusForbidden, newHandlerError(ErrUnableModifyOwnership, http.StatusForbidden)) + return + } } targetExistingMembership, err := database.NewMembership(postgresClient).GetMembershipByUserAndOrgId(targetMembership.UserId, targetMembership.OrgId) From 9d8ed3a8681ed036c35e6c705df8245c6949bbc5 Mon Sep 17 00:00:00 2001 From: Zheng_Zhi_Qiang Date: Thu, 27 Jun 2024 23:23:20 +0800 Subject: [PATCH 2/2] refactor: separated validateMembership middleware to validate the request and the target role granted --- backend/src/server/handlers.go | 6 ++--- backend/src/server/middleware.go | 44 ++++++++++++++++++++------------ 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/backend/src/server/handlers.go b/backend/src/server/handlers.go index ba694a3b..b8db52f1 100644 --- a/backend/src/server/handlers.go +++ b/backend/src/server/handlers.go @@ -85,9 +85,9 @@ func (s *ServerHandler) registerRoutes(r *mux.Router) { // Membership r.Handle("/api/membership", isAuthenticated(handleGetMembershipsForUser(s.logger, s.psqlClient), s.logger)).Methods("GET") - r.Handle("/api/membership", isAuthenticated(getOrgIdFromRequestBody(validateMembershipChange(s.psqlClient, isOrgAdmin(s.psqlClient, handleCreateMembership(s.logger, s.psqlClient), s.logger), s.logger), s.logger), s.logger)).Methods("POST").Headers("Content-Type", "application/json") - r.Handle("/api/membership", isAuthenticated(getOrgIdFromRequestBody(validateMembershipChange(s.psqlClient, isOrgAdmin(s.psqlClient, handleUpdateMembership(s.logger, s.psqlClient), s.logger), s.logger), s.logger), s.logger)).Methods("PATCH").Headers("Content-Type", "application/json") - r.Handle("/api/membership", isAuthenticated(getOrgIdFromRequestBody(validateMembershipChange(s.psqlClient, isOrgAdmin(s.psqlClient, handleDeleteMembership(s.logger, s.psqlClient), s.logger), s.logger), s.logger), s.logger)).Methods("DELETE").Headers("Content-Type", "application/json") + r.Handle("/api/membership", isAuthenticated(getOrgIdFromRequestBody(validateMembershipChangeRequest(s.psqlClient, isOrgAdmin(s.psqlClient, validateTargetMembershipRoleGranted(handleCreateMembership(s.logger, s.psqlClient), s.logger), s.logger), s.logger), s.logger), s.logger)).Methods("POST").Headers("Content-Type", "application/json") + r.Handle("/api/membership", isAuthenticated(getOrgIdFromRequestBody(validateMembershipChangeRequest(s.psqlClient, isOrgAdmin(s.psqlClient, validateTargetMembershipRoleGranted(handleUpdateMembership(s.logger, s.psqlClient), s.logger), s.logger), s.logger), s.logger), s.logger)).Methods("PATCH").Headers("Content-Type", "application/json") + r.Handle("/api/membership", isAuthenticated(getOrgIdFromRequestBody(validateMembershipChangeRequest(s.psqlClient, isOrgAdmin(s.psqlClient, handleDeleteMembership(s.logger, s.psqlClient), s.logger), s.logger), s.logger), s.logger)).Methods("DELETE").Headers("Content-Type", "application/json") r.Handle("/api/membership/ownership_transfer", isAuthenticated(getOrgIdFromRequestBody(isOrgOwner(s.psqlClient, handleOwnershipTransfer(s.logger, s.psqlClient), s.logger), s.logger), s.logger)).Methods("POST").Headers("Content-Type", "application/json") } diff --git a/backend/src/server/middleware.go b/backend/src/server/middleware.go index f2364ebd..f228d439 100644 --- a/backend/src/server/middleware.go +++ b/backend/src/server/middleware.go @@ -280,7 +280,7 @@ func getOrgIdUsingSrId(mongoClient *mongo.Client, next http.Handler, logger logg }) } -func validateMembershipChange(postgresClient *sql.DB, next http.Handler, logger logger.ServerLogger) http.Handler { +func validateMembershipChangeRequest(postgresClient *sql.DB, next http.Handler, logger logger.ServerLogger) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { org_id := r.Context().Value(util.OrgContextKey{}).(int) requestorMembership, err := getMembership(org_id, postgresClient, r) @@ -303,21 +303,6 @@ func validateMembershipChange(postgresClient *sql.DB, next http.Handler, logger return } - if r.Method != http.MethodDelete { - err = models.ValidateRole(targetMembership.Role) - if err != nil { - logger.Error("unable to add/update membership as role is invalid") - encode(w, r, http.StatusBadRequest, newHandlerError(err, http.StatusBadRequest)) - return - } - - if targetMembership.Role == models.Owner { - logger.Error("unable to grant/delete ownership") - encode(w, r, http.StatusForbidden, newHandlerError(ErrUnableModifyOwnership, http.StatusForbidden)) - return - } - } - targetExistingMembership, err := database.NewMembership(postgresClient).GetMembershipByUserAndOrgId(targetMembership.UserId, targetMembership.OrgId) if err != nil && !errors.Is(err, sql.ErrNoRows) { logger.Error(fmt.Sprintf("unable to verify subject role: %s", err)) @@ -330,6 +315,33 @@ func validateMembershipChange(postgresClient *sql.DB, next http.Handler, logger encode(w, r, http.StatusForbidden, newHandlerError(ErrUnauthorised, http.StatusForbidden)) return } + + next.ServeHTTP(w, r) + }) +} + +func validateTargetMembershipRoleGranted(next http.Handler, logger logger.ServerLogger) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + targetMembership, err := decode[models.MembershipModel](r) + if err != nil { + logger.Error(fmt.Sprintf("failed to parse json request body: %s", err)) + encode(w, r, http.StatusBadRequest, newHandlerError(ErrJsonParseError, http.StatusBadRequest)) + return + } + + err = models.ValidateRole(targetMembership.Role) + if err != nil { + logger.Error("unable to add/update membership as role is invalid") + encode(w, r, http.StatusBadRequest, newHandlerError(err, http.StatusBadRequest)) + return + } + + if targetMembership.Role == models.Owner { + logger.Error("unable to grant ownership") + encode(w, r, http.StatusForbidden, newHandlerError(ErrUnableModifyOwnership, http.StatusForbidden)) + return + } + next.ServeHTTP(w, r) }) }