Add account deletion endpoint (#1331)

Adding support to account owners to delete an account

This will remove all users from local, and if --user-delete-from-idp is set it will remove from the remote IDP

management/server/account.go

@@ -64,6 +64,7 @@ type AccountManager interface {
 	GetAccountByUserOrAccountID(userID, accountID, domain string) (*Account, error)
 	GetAccountFromToken(claims jwtclaims.AuthorizationClaims) (*Account, *User, error)
 	GetAccountFromPAT(pat string) (*Account, *User, *PersonalAccessToken, error)
+	DeleteAccount(accountID, userID string) error
 	MarkPATUsed(tokenID string) error
 	GetUser(claims jwtclaims.AuthorizationClaims) (*User, error)
 	ListUsers(accountID string) ([]*User, error)
@@ -1004,6 +1005,57 @@ func (am *DefaultAccountManager) warmupIDPCache() error {
 	return nil
 }
 
+// DeleteAccount deletes an account and all its users from local store and from the remote IDP if the requester is an admin and account owner
+func (am *DefaultAccountManager) DeleteAccount(accountID, userID string) error {
+	unlock := am.Store.AcquireAccountLock(accountID)
+	defer unlock()
+	account, err := am.Store.GetAccount(accountID)
+	if err != nil {
+		return err
+	}
+
+	user, err := account.FindUser(userID)
+	if err != nil {
+		return err
+	}
+
+	if !user.IsAdmin() {
+		return status.Errorf(status.PermissionDenied, "user is not allowed to delete account")
+	}
+
+	if user.Id != account.CreatedBy {
+		return status.Errorf(status.PermissionDenied, "user is not allowed to delete account. Only account owner can delete account")
+	}
+	for _, otherUser := range account.Users {
+		if otherUser.IsServiceUser {
+			continue
+		}
+
+		if otherUser.Id == userID {
+			continue
+		}
+
+		deleteUserErr := am.deleteRegularUser(account, userID, otherUser.Id)
+		if deleteUserErr != nil {
+			return deleteUserErr
+		}
+	}
+
+	err = am.deleteRegularUser(account, userID, userID)
+	if err != nil {
+		log.Errorf("failed deleting user %s. error: %s", userID, err)
+		return err
+	}
+
+	err = am.Store.DeleteAccount(account)
+	if err != nil {
+		log.Errorf("failed deleting account %s. error: %s", accountID, err)
+		return err
+	}
+	log.Debugf("account %s deleted", accountID)
+	return nil
+}
+
 // GetAccountByUserOrAccountID looks for an account by user or accountID, if no account is provided and
 // userID doesn't have an account associated with it, one account is created
 func (am *DefaultAccountManager) GetAccountByUserOrAccountID(userID, accountID, domain string) (*Account, error) {

management/server/account_test.go

@@ -746,6 +746,31 @@ func TestAccountManager_GetAccount(t *testing.T) {
 	}
 }
 
+func TestAccountManager_DeleteAccount(t *testing.T) {
+	manager, err := createManager(t)
+	if err != nil {
+		t.Fatal(err)
+		return
+	}
+
+	expectedId := "test_account"
+	userId := "account_creator"
+	account, err := createAccount(manager, expectedId, userId, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = manager.DeleteAccount(account.Id, userId)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	getAccount, err := manager.Store.GetAccount(account.Id)
+	if err == nil {
+		t.Fatal(fmt.Errorf("expected to get an error when trying to get deleted account, got %v", getAccount))
+	}
+}
+
 func TestAccountManager_AddPeer(t *testing.T) {
 	manager, err := createManager(t)
 	if err != nil {

management/server/file_store.go

@@ -351,6 +351,41 @@ func (s *FileStore) SaveAccount(account *Account) error {
 	return s.persist(s.storeFile)
 }
 
+func (s *FileStore) DeleteAccount(account *Account) error {
+	s.mux.Lock()
+	defer s.mux.Unlock()
+
+	if account.Id == "" {
+		return status.Errorf(status.InvalidArgument, "account id should not be empty")
+	}
+
+	for keyID := range account.SetupKeys {
+		delete(s.SetupKeyID2AccountID, strings.ToUpper(keyID))
+	}
+
+	// enforce peer to account index and delete peer to route indexes for rebuild
+	for _, peer := range account.Peers {
+		delete(s.PeerKeyID2AccountID, peer.Key)
+		delete(s.PeerID2AccountID, peer.ID)
+	}
+
+	for _, user := range account.Users {
+		for _, pat := range user.PATs {
+			delete(s.TokenID2UserID, pat.ID)
+			delete(s.HashedPAT2TokenID, pat.HashedToken)
+		}
+		delete(s.UserID2AccountID, user.Id)
+	}
+
+	if account.DomainCategory == PrivateCategory && account.IsDomainPrimaryAccount {
+		delete(s.PrivateDomain2AccountID, account.Domain)
+	}
+
+	delete(s.Accounts, account.Id)
+
+	return s.persist(s.storeFile)
+}
+
 // DeleteHashedPAT2TokenIDIndex removes an entry from the indexing map HashedPAT2TokenID
 func (s *FileStore) DeleteHashedPAT2TokenIDIndex(hashedToken string) error {
 	s.mux.Lock()

management/server/file_store_test.go

@@ -121,6 +121,60 @@ func TestSaveAccount(t *testing.T) {
 	}
 }
 
+func TestDeleteAccount(t *testing.T) {
+	storeDir := t.TempDir()
+	storeFile := filepath.Join(storeDir, "store.json")
+	err := util.CopyFileContents("testdata/store.json", storeFile)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	store, err := NewFileStore(storeDir, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var account *Account
+	for _, a := range store.Accounts {
+		account = a
+		break
+	}
+
+	require.NotNil(t, account, "failed to restore a FileStore file and get at least one account")
+
+	err = store.DeleteAccount(account)
+	require.NoError(t, err, "failed to delete account, error: %v", err)
+
+	_, ok := store.Accounts[account.Id]
+	require.False(t, ok, "failed to delete account")
+
+	for id := range account.Users {
+		_, ok := store.UserID2AccountID[id]
+		assert.False(t, ok, "failed to delete UserID2AccountID index")
+		for _, pat := range account.Users[id].PATs {
+			_, ok := store.HashedPAT2TokenID[pat.HashedToken]
+			assert.False(t, ok, "failed to delete HashedPAT2TokenID index")
+			_, ok = store.TokenID2UserID[pat.ID]
+			assert.False(t, ok, "failed to delete TokenID2UserID index")
+		}
+	}
+
+	for _, p := range account.Peers {
+		_, ok := store.PeerKeyID2AccountID[p.Key]
+		assert.False(t, ok, "failed to delete PeerKeyID2AccountID index")
+		_, ok = store.PeerID2AccountID[p.ID]
+		assert.False(t, ok, "failed to delete PeerID2AccountID index")
+	}
+
+	for id := range account.SetupKeys {
+		_, ok := store.SetupKeyID2AccountID[id]
+		assert.False(t, ok, "failed to delete SetupKeyID2AccountID index")
+	}
+
+	_, ok = store.PrivateDomain2AccountID[account.Domain]
+	assert.False(t, ok, "failed to delete PrivateDomain2AccountID index")
+
+}
+
 func TestStore(t *testing.T) {
 	store := newStore(t)
 

management/server/http/accounts_handler.go

@@ -98,6 +98,30 @@ func (h *AccountsHandler) UpdateAccount(w http.ResponseWriter, r *http.Request)
 	util.WriteJSONObject(w, &resp)
 }
 
+// DeleteAccount is a HTTP DELETE handler to delete an account
+func (h *AccountsHandler) DeleteAccount(w http.ResponseWriter, r *http.Request) {
+	if r.Method != http.MethodDelete {
+		util.WriteErrorResponse("wrong HTTP method", http.StatusMethodNotAllowed, w)
+		return
+	}
+
+	claims := h.claimsExtractor.FromRequestContext(r)
+	vars := mux.Vars(r)
+	targetAccountID := vars["accountId"]
+	if len(targetAccountID) == 0 {
+		util.WriteError(status.Errorf(status.InvalidArgument, "invalid account ID"), w)
+		return
+	}
+
+	err := h.accountManager.DeleteAccount(targetAccountID, claims.UserId)
+	if err != nil {
+		util.WriteError(err, w)
+		return
+	}
+
+	util.WriteJSONObject(w, emptyObject{})
+}
+
 func toAccountResponse(account *server.Account) *api.Account {
 	return &api.Account{
 		Id: account.Id,

management/server/http/api/openapi.yml

@@ -1074,6 +1074,32 @@ paths:
         '500':
           "$ref": "#/components/responses/internal_error"
   /api/accounts/{accountId}:
+    delete:
+      summary: Delete an Account
+      description: Deletes an account and all its resources. Only administrators and account owners can delete accounts.
+      tags: [ Accounts ]
+      security:
+        - BearerAuth: [ ]
+        - TokenAuth: [ ]
+      parameters:
+        - in: path
+          name: accountId
+          required: true
+          schema:
+            type: string
+          description: The unique identifier of an account
+      responses:
+        '200':
+          description: Delete account status code
+          content: { }
+        '400':
+          "$ref": "#/components/responses/bad_request"
+        '401':
+          "$ref": "#/components/responses/requires_authentication"
+        '403':
+          "$ref": "#/components/responses/forbidden"
+        '500':
+          "$ref": "#/components/responses/internal_error"
     put:
       summary: Update an Account
       description: Update information about an account

management/server/http/handler.go

@@ -7,6 +7,7 @@ import (
 	"github.com/rs/cors"
 
 	"github.com/netbirdio/management-integrations/integrations"
+
 	s "github.com/netbirdio/netbird/management/server"
 	"github.com/netbirdio/netbird/management/server/http/middleware"
 	"github.com/netbirdio/netbird/management/server/jwtclaims"
@@ -105,6 +106,7 @@ func APIHandler(accountManager s.AccountManager, jwtValidator jwtclaims.JWTValid
 func (apiHandler *apiHandler) addAccountsEndpoint() {
 	accountsHandler := NewAccountsHandler(apiHandler.AccountManager, apiHandler.AuthCfg)
 	apiHandler.Router.HandleFunc("/accounts/{accountId}", accountsHandler.UpdateAccount).Methods("PUT", "OPTIONS")
+	apiHandler.Router.HandleFunc("/accounts/{accountId}", accountsHandler.DeleteAccount).Methods("DELETE", "OPTIONS")
 	apiHandler.Router.HandleFunc("/accounts", accountsHandler.GetAllAccounts).Methods("GET", "OPTIONS")
 }
 

management/server/http/users_handler.go

@@ -94,7 +94,7 @@ func (h *UsersHandler) UpdateUser(w http.ResponseWriter, r *http.Request) {
 	util.WriteJSONObject(w, toUserResponse(newUser, claims.UserId))
 }
 
-// DeleteUser is a DELETE request to delete a user (only works for service users right now)
+// DeleteUser is a DELETE request to delete a user
 func (h *UsersHandler) DeleteUser(w http.ResponseWriter, r *http.Request) {
 	if r.Method != http.MethodDelete {
 		util.WriteErrorResponse("wrong HTTP method", http.StatusMethodNotAllowed, w)

management/server/mock_server/account_mock.go

@@ -68,6 +68,7 @@ type MockAccountManager struct {
 	ListNameServerGroupsFunc        func(accountID string) ([]*nbdns.NameServerGroup, error)
 	CreateUserFunc                  func(accountID, userID string, key *server.UserInfo) (*server.UserInfo, error)
 	GetAccountFromTokenFunc         func(claims jwtclaims.AuthorizationClaims) (*server.Account, *server.User, error)
+	DeleteAccountFunc               func(accountID, userID string) error
 	GetDNSDomainFunc                func() string
 	StoreEventFunc                  func(initiatorID, targetID, accountID string, activityID activity.Activity, meta map[string]any)
 	GetEventsFunc                   func(accountID, userID string) ([]*activity.Event, error)
@@ -157,6 +158,14 @@ func (am *MockAccountManager) GetAccountFromPAT(pat string) (*server.Account, *s
 	return nil, nil, nil, status.Errorf(codes.Unimplemented, "method GetAccountFromPAT is not implemented")
 }
 
+// DeleteAccount mock implementation of DeleteAccount from server.AccountManager interface
+func (am *MockAccountManager) DeleteAccount(accountID, userID string) error {
+	if am.DeleteAccountFunc != nil {
+		return am.DeleteAccountFunc(accountID, userID)
+	}
+	return status.Errorf(codes.Unimplemented, "method DeleteAccount is not implemented")
+}
+
 // MarkPATUsed mock implementation of MarkPATUsed from server.AccountManager interface
 func (am *MockAccountManager) MarkPATUsed(pat string) error {
 	if am.MarkPATUsedFunc != nil {

management/server/sqlite_store.go

@@ -7,15 +7,16 @@ import (
 	"sync"
 	"time"
 
-	nbdns "github.com/netbirdio/netbird/dns"
-	"github.com/netbirdio/netbird/management/server/status"
-	"github.com/netbirdio/netbird/management/server/telemetry"
-	"github.com/netbirdio/netbird/route"
 	log "github.com/sirupsen/logrus"
 	"gorm.io/driver/sqlite"
 	"gorm.io/gorm"
 	"gorm.io/gorm/clause"
 	"gorm.io/gorm/logger"
+
+	nbdns "github.com/netbirdio/netbird/dns"
+	"github.com/netbirdio/netbird/management/server/status"
+	"github.com/netbirdio/netbird/management/server/telemetry"
+	"github.com/netbirdio/netbird/route"
 )
 
 // SqliteStore represents an account storage backed by a Sqlite DB persisted to disk
@@ -202,6 +203,37 @@ func (s *SqliteStore) SaveAccount(account *Account) error {
 	return err
 }
 
+func (s *SqliteStore) DeleteAccount(account *Account) error {
+	start := time.Now()
+
+	err := s.db.Transaction(func(tx *gorm.DB) error {
+		result := tx.Select(clause.Associations).Delete(account.Policies, "account_id = ?", account.Id)
+		if result.Error != nil {
+			return result.Error
+		}
+
+		result = tx.Select(clause.Associations).Delete(account.UsersG, "account_id = ?", account.Id)
+		if result.Error != nil {
+			return result.Error
+		}
+
+		result = tx.Select(clause.Associations).Delete(account)
+		if result.Error != nil {
+			return result.Error
+		}
+
+		return nil
+	})
+
+	took := time.Since(start)
+	if s.metrics != nil {
+		s.metrics.StoreMetrics().CountPersistenceDuration(took)
+	}
+	log.Debugf("took %d ms to delete an account to the SQLite", took.Milliseconds())
+
+	return err
+}
+
 func (s *SqliteStore) SaveInstallationID(ID string) error {
 	installation := installation{InstallationIDValue: ID}
 	installation.ID = uint(s.installationPK)
@@ -336,7 +368,7 @@ func (s *SqliteStore) GetAccount(accountID string) (*Account, error) {
 		var rules []*PolicyRule
 		err := s.db.Model(&PolicyRule{}).Find(&rules, "policy_id = ?", policy.ID).Error
 		if err != nil {
-			return nil, status.Errorf(status.NotFound, "account not found")
+			return nil, status.Errorf(status.NotFound, "rule not found")
 		}
 		account.Policies[i].Rules = rules
 	}

management/server/sqlite_store_test.go

@@ -9,9 +9,10 @@ import (
 	"time"
 
 	"github.com/google/uuid"
-	"github.com/netbirdio/netbird/util"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/netbirdio/netbird/util"
 )
 
 func TestSqlite_NewStore(t *testing.T) {
@@ -98,6 +99,80 @@ func TestSqlite_SaveAccount(t *testing.T) {
 	}
 }
 
+func TestSqlite_DeleteAccount(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("The SQLite store is not properly supported by Windows yet")
+	}
+
+	store := newSqliteStore(t)
+
+	testUserID := "testuser"
+	user := NewAdminUser(testUserID)
+	user.PATs = map[string]*PersonalAccessToken{"testtoken": {
+		ID:   "testtoken",
+		Name: "test token",
+	}}
+
+	account := newAccountWithId("account_id", testUserID, "")
+	setupKey := GenerateDefaultSetupKey()
+	account.SetupKeys[setupKey.Key] = setupKey
+	account.Peers["testpeer"] = &Peer{
+		Key:      "peerkey",
+		SetupKey: "peerkeysetupkey",
+		IP:       net.IP{127, 0, 0, 1},
+		Meta:     PeerSystemMeta{},
+		Name:     "peer name",
+		Status:   &PeerStatus{Connected: true, LastSeen: time.Now().UTC()},
+	}
+	account.Users[testUserID] = user
+
+	err := store.SaveAccount(account)
+	require.NoError(t, err)
+
+	if len(store.GetAllAccounts()) != 1 {
+		t.Errorf("expecting 1 Accounts to be stored after SaveAccount()")
+	}
+
+	err = store.DeleteAccount(account)
+	require.NoError(t, err)
+
+	if len(store.GetAllAccounts()) != 0 {
+		t.Errorf("expecting 0 Accounts to be stored after DeleteAccount()")
+	}
+
+	_, err = store.GetAccountByPeerPubKey("peerkey")
+	require.Error(t, err, "expecting error after removing DeleteAccount when getting account by peer public key")
+
+	_, err = store.GetAccountByUser("testuser")
+	require.Error(t, err, "expecting error after removing DeleteAccount when getting account by user")
+
+	_, err = store.GetAccountByPeerID("testpeer")
+	require.Error(t, err, "expecting error after removing DeleteAccount when getting account by peer id")
+
+	_, err = store.GetAccountBySetupKey(setupKey.Key)
+	require.Error(t, err, "expecting error after removing DeleteAccount when getting account by setup key")
+
+	_, err = store.GetAccount(account.Id)
+	require.Error(t, err, "expecting error after removing DeleteAccount when getting account by id")
+
+	for _, policy := range account.Policies {
+		var rules []*PolicyRule
+		err = store.db.Model(&PolicyRule{}).Find(&rules, "policy_id = ?", policy.ID).Error
+		require.NoError(t, err, "expecting no error after removing DeleteAccount when searching for policy rules")
+		require.Len(t, rules, 0, "expecting no policy rules to be found after removing DeleteAccount")
+
+	}
+
+	for _, accountUser := range account.Users {
+		var pats []*PersonalAccessToken
+		err = store.db.Model(&PersonalAccessToken{}).Find(&pats, "user_id = ?", accountUser.Id).Error
+		require.NoError(t, err, "expecting no error after removing DeleteAccount when searching for personal access token")
+		require.Len(t, pats, 0, "expecting no personal access token to be found after removing DeleteAccount")
+
+	}
+
+}
+
 func TestSqlite_SavePeerStatus(t *testing.T) {
 	if runtime.GOOS == "windows" {
 		t.Skip("The SQLite store is not properly supported by Windows yet")

management/server/store.go

@@ -14,6 +14,7 @@ import (
 type Store interface {
 	GetAllAccounts() []*Account
 	GetAccount(accountID string) (*Account, error)
+	DeleteAccount(account *Account) error
 	GetAccountByUser(userID string) (*Account, error)
 	GetAccountByPeerPubKey(peerKey string) (*Account, error)
 	GetAccountByPeerID(peerID string) (*Account, error)