fix user edit bug, allow to delete users from the database (#40)

assets/tpl/admin_edit_user.html

@@ -76,6 +76,11 @@
 
             <button type="submit" class="btn btn-primary">Save</button>
             <a href="/admin/users/" class="btn btn-secondary">Cancel</a>
+            {{if eq $.Session.IsAdmin true}}
+            {{if eq .User.Source "db"}}
+            <a href="/admin/users/delete?pkey={{.User.Email}}" data-toggle="confirmation" data-title="Really delete user and associated peers?" title="Delete user and associated peers" class="btn btn-danger float-right">Delete</a>
+            {{end}}
+            {{end}}
         </form>
     </div>
     {{template "prt_footer.html" .}}

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)

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

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

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

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

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"`