From 83271b5d347d088e1fc27dceb121874834544933 Mon Sep 17 00:00:00 2001 From: Christoph Haas Date: Tue, 15 Mar 2022 23:34:55 +0100 Subject: [PATCH] fix user edit bug, allow to delete users from the database (#40) --- assets/tpl/admin_edit_user.html | 5 ++++ internal/server/handlers_user.go | 22 +++++++++++++- internal/server/ldapsync.go | 10 +++---- internal/server/routes.go | 1 + internal/server/server_helper.go | 51 +++++++++++++++++++++----------- internal/users/manager.go | 9 ++++-- internal/users/user.go | 2 +- 7 files changed, 73 insertions(+), 27 deletions(-) diff --git a/assets/tpl/admin_edit_user.html b/assets/tpl/admin_edit_user.html index 9891f32..1218ca5 100644 --- a/assets/tpl/admin_edit_user.html +++ b/assets/tpl/admin_edit_user.html @@ -76,6 +76,11 @@ Cancel + {{if eq $.Session.IsAdmin true}} + {{if eq .User.Source "db"}} + Delete + {{end}} + {{end}} {{template "prt_footer.html" .}} diff --git a/internal/server/handlers_user.go b/internal/server/handlers_user.go index c0816f5..4660105 100644 --- a/internal/server/handlers_user.go +++ b/internal/server/handlers_user.go @@ -83,6 +83,26 @@ func (s *Server) GetAdminUsersEdit(c *gin.Context) { }) } +func (s *Server) GetAdminUsersDelete(c *gin.Context) { + user := s.users.GetUserUnscoped(c.Query("pkey")) + if user == nil { + SetFlashMessage(c, "invalid user", "danger") + c.Redirect(http.StatusSeeOther, "/admin/users/") + return + } + + urlEncodedKey := url.QueryEscape(c.Query("pkey")) + + if err := s.HardDeleteUser(*user); err != nil { + SetFlashMessage(c, "failed to delete user: "+err.Error(), "danger") + c.Redirect(http.StatusSeeOther, "/admin/users/edit?pkey="+urlEncodedKey+"&formerr=delete") + return + } + + SetFlashMessage(c, "user deleted successfully", "success") + c.Redirect(http.StatusSeeOther, "/admin/users/") +} + func (s *Server) PostAdminUsersEdit(c *gin.Context) { currentUser := s.users.GetUserUnscoped(c.Query("pkey")) if currentUser == nil { @@ -113,7 +133,7 @@ func (s *Server) PostAdminUsersEdit(c *gin.Context) { } else { formUser.DeletedAt = gorm.DeletedAt{} } - formUser.IsAdmin = c.PostForm("isadmin") == "true" + formUser.IsAdmin = c.PostForm("isadmin") != "" if err := s.UpdateUser(formUser); err != nil { _ = s.updateFormInSession(c, formUser) diff --git a/internal/server/ldapsync.go b/internal/server/ldapsync.go index eb9fc5c..55429d1 100644 --- a/internal/server/ldapsync.go +++ b/internal/server/ldapsync.go @@ -44,14 +44,14 @@ func (s *Server) SyncLdapWithUserDatabase() { logrus.Info("ldap user synchronization stopped") } -func (s Server)userIsInAdminGroup(ldapData *ldap.RawLdapData) bool { +func (s Server) userIsInAdminGroup(ldapData *ldap.RawLdapData) bool { if s.config.LDAP.AdminLdapGroup_ == nil { - return false + return false } for _, group := range ldapData.RawAttributes[s.config.LDAP.GroupMemberAttribute] { - var dn,_ = gldap.ParseDN(string(group)) + var dn, _ = gldap.ParseDN(string(group)) if s.config.LDAP.AdminLdapGroup_.Equal(dn) { - return true + return true } } return false @@ -114,7 +114,7 @@ func (s *Server) disableMissingLdapUsers(ldapUsers []ldap.RawLdapData) { } } - if err := s.users.DeleteUser(&activeUsers[i]); err != nil { + if err := s.users.DeleteUser(&activeUsers[i], true); err != nil { logrus.Errorf("failed to delete deactivated user %s in database: %v", activeUsers[i].Email, err) } } diff --git a/internal/server/routes.go b/internal/server/routes.go index 5d10776..3922cad 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -64,6 +64,7 @@ func SetupRoutes(s *Server) { admin.GET("/users/create", s.GetAdminUsersCreate) admin.POST("/users/create", s.PostAdminUsersCreate) admin.GET("/users/edit", s.GetAdminUsersEdit) + admin.GET("/users/delete", s.GetAdminUsersDelete) admin.POST("/users/edit", s.PostAdminUsersEdit) // User routes diff --git a/internal/server/server_helper.go b/internal/server/server_helper.go index 0ffd50c..f366396 100644 --- a/internal/server/server_helper.go +++ b/internal/server/server_helper.go @@ -112,7 +112,7 @@ func (s *Server) CreatePeer(device string, peer wireguard.Peer) error { peer.PresharedKey = psk.String() } - if peer.PrivateKey == "" && peer.PublicKey == "" && dev.Type == wireguard.DeviceTypeServer { // if private key is empty create a new one + if peer.PrivateKey == "" && peer.PublicKey == "" && dev.Type == wireguard.DeviceTypeServer { // if private key is empty create a new one key, err := wgtypes.GeneratePrivateKey() if err != nil { @@ -253,10 +253,6 @@ func (s *Server) CreateUser(user users.User, device string) error { // UpdateUser updates the user in the database. If the user is marked as deleted, it will get remove from the database. // Also, if the user is re-enabled, all it's linked WireGuard peers will be activated again. func (s *Server) UpdateUser(user users.User) error { - if user.DeletedAt.Valid { - return s.DeleteUser(user) - } - currentUser := s.users.GetUserUnscoped(user.Email) // Hash user password (if set) @@ -275,7 +271,12 @@ func (s *Server) UpdateUser(user users.User) error { return errors.WithMessage(err, "failed to update user in manager") } - // If user was deleted (disabled), reactivate it's peers + // Set to deleted (disabled) if user's deletedAt date is not empty + if user.DeletedAt.Valid { + return s.DeleteUser(user) + } + + // Otherwise, if user was deleted (disabled), reactivate it's peers if currentUser.DeletedAt.Valid { for _, peer := range s.peers.GetPeersByMail(user.Email) { now := time.Now() @@ -289,24 +290,38 @@ func (s *Server) UpdateUser(user users.User) error { return nil } -// DeleteUser removes the user from the database. +// DeleteUser soft-deletes the user from the database (disable the user). // Also, if the user has linked WireGuard peers, they will be deactivated. func (s *Server) DeleteUser(user users.User) error { - currentUser := s.users.GetUserUnscoped(user.Email) - // Update in database - if err := s.users.DeleteUser(&user); err != nil { + if err := s.users.DeleteUser(&user, true); err != nil { + return errors.WithMessage(err, "failed to disable user in manager") + } + + // Disable users peers + for _, peer := range s.peers.GetPeersByMail(user.Email) { + now := time.Now() + peer.DeactivatedAt = &now + if err := s.UpdatePeer(peer, now); err != nil { + logrus.Errorf("failed to update deactivated peer %s for %s: %v", peer.PublicKey, user.Email, err) + } + } + + return nil +} + +// HardDeleteUser removes the user from the database. +// Also, if the user has linked WireGuard peers, they will be deleted. +func (s *Server) HardDeleteUser(user users.User) error { + // Update in database + if err := s.users.DeleteUser(&user, false); err != nil { return errors.WithMessage(err, "failed to delete user in manager") } - // If user was active, disable it's peers - if !currentUser.DeletedAt.Valid { - for _, peer := range s.peers.GetPeersByMail(user.Email) { - now := time.Now() - peer.DeactivatedAt = &now - if err := s.UpdatePeer(peer, now); err != nil { - logrus.Errorf("failed to update deactivated peer %s for %s: %v", peer.PublicKey, user.Email, err) - } + // remove all linked peers + for _, peer := range s.peers.GetPeersByMail(user.Email) { + if err := s.DeletePeer(peer); err != nil { + logrus.Errorf("failed to delete peer %s for %s: %v", peer.PublicKey, user.Email, err) } } diff --git a/internal/users/manager.go b/internal/users/manager.go index 53fb5d4..5a9c489 100644 --- a/internal/users/manager.go +++ b/internal/users/manager.go @@ -161,9 +161,14 @@ func (m Manager) UpdateUser(user *User) error { return nil } -func (m Manager) DeleteUser(user *User) error { +func (m Manager) DeleteUser(user *User, soft bool) error { user.Email = strings.ToLower(user.Email) - res := m.db.Delete(user) + var res *gorm.DB + if soft { + res = m.db.Delete(user) + } else { + res = m.db.Unscoped().Delete(user) + } if res.Error != nil { return errors.Wrapf(res.Error, "failed to update user %s", user.Email) } diff --git a/internal/users/user.go b/internal/users/user.go index 24397c7..2a94fcf 100644 --- a/internal/users/user.go +++ b/internal/users/user.go @@ -29,7 +29,7 @@ type User struct { // required fields Email string `gorm:"primaryKey" form:"email" binding:"required,email"` Source UserSource - IsAdmin bool + IsAdmin bool `form:"isadmin"` // optional fields Firstname string `form:"firstname" binding:"required"`