[management] Add user approval (#4411)

This PR adds user approval functionality to the management system, allowing administrators to manually approve new users joining via domain matching. When enabled, users are blocked with pending approval status until explicitly approved by an admin.

Adds UserApprovalRequired setting to control manual user approval requirement
Introduces user approval and rejection endpoints with corresponding business logic
Prevents pending approval users from adding peers or logging in

Author: mlsmaycon

management/server/account.go

@@ -1136,7 +1136,18 @@ func (am *DefaultAccountManager) addNewPrivateAccount(ctx context.Context, domai
 func (am *DefaultAccountManager) addNewUserToDomainAccount(ctx context.Context, domainAccountID string, userAuth nbcontext.UserAuth) (string, error) {
 	newUser := types.NewRegularUser(userAuth.UserId)
 	newUser.AccountID = domainAccountID
-	err := am.Store.SaveUser(ctx, newUser)
+
+	settings, err := am.Store.GetAccountSettings(ctx, store.LockingStrengthNone, domainAccountID)
+	if err != nil {
+		return "", err
+	}
+
+	if settings != nil && settings.Extra != nil && settings.Extra.UserApprovalRequired {
+		newUser.Blocked = true
+		newUser.PendingApproval = true
+	}
+
+	err = am.Store.SaveUser(ctx, newUser)
 	if err != nil {
 		return "", err
 	}
@@ -1146,7 +1157,11 @@ func (am *DefaultAccountManager) addNewUserToDomainAccount(ctx context.Context,
 		return "", err
 	}
 
-	am.StoreEvent(ctx, userAuth.UserId, userAuth.UserId, domainAccountID, activity.UserJoined, nil)
+	if newUser.PendingApproval {
+		am.StoreEvent(ctx, userAuth.UserId, userAuth.UserId, domainAccountID, activity.UserJoined, map[string]any{"pending_approval": true})
+	} else {
+		am.StoreEvent(ctx, userAuth.UserId, userAuth.UserId, domainAccountID, activity.UserJoined, nil)
+	}
 
 	return domainAccountID, nil
 }
@@ -1795,6 +1810,9 @@ func newAccountWithId(ctx context.Context, accountID, userID, domain string, dis
 			PeerInactivityExpirationEnabled: false,
 			PeerInactivityExpiration:        types.DefaultPeerInactivityExpiration,
 			RoutingPeerDNSResolutionEnabled: true,
+			Extra: &types.ExtraSettings{
+				UserApprovalRequired: true,
+			},
 		},
 		Onboarding: types.AccountOnboarding{
 			OnboardingFlowPending: true,
@@ -1901,6 +1919,9 @@ func (am *DefaultAccountManager) GetOrCreateAccountByPrivateDomain(ctx context.C
 				PeerInactivityExpirationEnabled: false,
 				PeerInactivityExpiration:        types.DefaultPeerInactivityExpiration,
 				RoutingPeerDNSResolutionEnabled: true,
+				Extra: &types.ExtraSettings{
+					UserApprovalRequired: true,
+				},
 			},
 		}
 

management/server/account/manager.go

@@ -32,6 +32,8 @@ type Manager interface {
 	DeleteUser(ctx context.Context, accountID, initiatorUserID string, targetUserID string) error
 	DeleteRegularUsers(ctx context.Context, accountID, initiatorUserID string, targetUserIDs []string, userInfos map[string]*types.UserInfo) error
 	InviteUser(ctx context.Context, accountID string, initiatorUserID string, targetUserID string) error
+	ApproveUser(ctx context.Context, accountID, initiatorUserID, targetUserID string) (*types.UserInfo, error)
+	RejectUser(ctx context.Context, accountID, initiatorUserID, targetUserID string) error
 	ListSetupKeys(ctx context.Context, accountID, userID string) ([]*types.SetupKey, error)
 	SaveUser(ctx context.Context, accountID, initiatorUserID string, update *types.User) (*types.UserInfo, error)
 	SaveOrAddUser(ctx context.Context, accountID, initiatorUserID string, update *types.User, addIfNotExists bool) (*types.UserInfo, error)

management/server/account_test.go

@@ -3606,3 +3606,93 @@ func TestDefaultAccountManager_UpdatePeerIP(t *testing.T) {
 		require.Error(t, err, "should fail with invalid peer ID")
 	})
 }
+
+func TestAddNewUserToDomainAccountWithApproval(t *testing.T) {
+	manager, err := createManager(t)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Create a domain-based account with user approval enabled
+	existingAccountID := "existing-account"
+	account := newAccountWithId(context.Background(), existingAccountID, "owner-user", "example.com", false)
+	account.Settings.Extra = &types.ExtraSettings{
+		UserApprovalRequired: true,
+	}
+	err = manager.Store.SaveAccount(context.Background(), account)
+	require.NoError(t, err)
+
+	// Set the account as domain primary account
+	account.IsDomainPrimaryAccount = true
+	account.DomainCategory = types.PrivateCategory
+	err = manager.Store.SaveAccount(context.Background(), account)
+	require.NoError(t, err)
+
+	// Test adding new user to existing account with approval required
+	newUserID := "new-user-id"
+	userAuth := nbcontext.UserAuth{
+		UserId:         newUserID,
+		Domain:         "example.com",
+		DomainCategory: types.PrivateCategory,
+	}
+
+	acc, err := manager.Store.GetAccount(context.Background(), existingAccountID)
+	require.NoError(t, err)
+	require.True(t, acc.IsDomainPrimaryAccount, "Account should be primary for the domain")
+	require.Equal(t, "example.com", acc.Domain, "Account domain should match")
+
+	returnedAccountID, err := manager.getAccountIDWithAuthorizationClaims(context.Background(), userAuth)
+	require.NoError(t, err)
+	require.Equal(t, existingAccountID, returnedAccountID)
+
+	// Verify user was created with pending approval
+	user, err := manager.Store.GetUserByUserID(context.Background(), store.LockingStrengthNone, newUserID)
+	require.NoError(t, err)
+	assert.True(t, user.Blocked, "User should be blocked when approval is required")
+	assert.True(t, user.PendingApproval, "User should be pending approval")
+	assert.Equal(t, existingAccountID, user.AccountID)
+}
+
+func TestAddNewUserToDomainAccountWithoutApproval(t *testing.T) {
+	manager, err := createManager(t)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Create a domain-based account without user approval
+	ownerUserAuth := nbcontext.UserAuth{
+		UserId:         "owner-user",
+		Domain:         "example.com",
+		DomainCategory: types.PrivateCategory,
+	}
+	existingAccountID, err := manager.getAccountIDWithAuthorizationClaims(context.Background(), ownerUserAuth)
+	require.NoError(t, err)
+
+	// Modify the account to disable user approval
+	account, err := manager.Store.GetAccount(context.Background(), existingAccountID)
+	require.NoError(t, err)
+	account.Settings.Extra = &types.ExtraSettings{
+		UserApprovalRequired: false,
+	}
+	err = manager.Store.SaveAccount(context.Background(), account)
+	require.NoError(t, err)
+
+	// Test adding new user to existing account without approval required
+	newUserID := "new-user-id"
+	userAuth := nbcontext.UserAuth{
+		UserId:         newUserID,
+		Domain:         "example.com",
+		DomainCategory: types.PrivateCategory,
+	}
+
+	returnedAccountID, err := manager.getAccountIDWithAuthorizationClaims(context.Background(), userAuth)
+	require.NoError(t, err)
+	require.Equal(t, existingAccountID, returnedAccountID)
+
+	// Verify user was created without pending approval
+	user, err := manager.Store.GetUserByUserID(context.Background(), store.LockingStrengthNone, newUserID)
+	require.NoError(t, err)
+	assert.False(t, user.Blocked, "User should not be blocked when approval is not required")
+	assert.False(t, user.PendingApproval, "User should not be pending approval")
+	assert.Equal(t, existingAccountID, user.AccountID)
+}

management/server/activity/codes.go

@@ -177,6 +177,8 @@ const (
 
 	AccountNetworkRangeUpdated Activity = 87
 	PeerIPUpdated              Activity = 88
+	UserApproved               Activity = 89
+	UserRejected               Activity = 90
 
 	AccountDeleted Activity = 99999
 )
@@ -284,6 +286,8 @@ var activityMap = map[Activity]Code{
 	AccountNetworkRangeUpdated: {"Account network range updated", "account.network.range.update"},
 
 	PeerIPUpdated: {"Peer IP updated", "peer.ip.update"},
+	UserApproved:  {"User approved", "user.approve"},
+	UserRejected:  {"User rejected", "user.reject"},
 }
 
 // StringCode returns a string code of the activity

management/server/http/handlers/accounts/accounts_handler.go

@@ -11,11 +11,11 @@ import (
 
 	"github.com/netbirdio/netbird/management/server/account"
 	nbcontext "github.com/netbirdio/netbird/management/server/context"
+	"github.com/netbirdio/netbird/management/server/settings"
+	"github.com/netbirdio/netbird/management/server/types"
 	"github.com/netbirdio/netbird/shared/management/http/api"
 	"github.com/netbirdio/netbird/shared/management/http/util"
-	"github.com/netbirdio/netbird/management/server/settings"
 	"github.com/netbirdio/netbird/shared/management/status"
-	"github.com/netbirdio/netbird/management/server/types"
 )
 
 const (
@@ -198,6 +198,7 @@ func (h *handler) updateAccount(w http.ResponseWriter, r *http.Request) {
 	if req.Settings.Extra != nil {
 		settings.Extra = &types.ExtraSettings{
 			PeerApprovalEnabled:      req.Settings.Extra.PeerApprovalEnabled,
+			UserApprovalRequired:     req.Settings.Extra.UserApprovalRequired,
 			FlowEnabled:              req.Settings.Extra.NetworkTrafficLogsEnabled,
 			FlowGroups:               req.Settings.Extra.NetworkTrafficLogsGroups,
 			FlowPacketCounterEnabled: req.Settings.Extra.NetworkTrafficPacketCounterEnabled,
@@ -327,6 +328,7 @@ func toAccountResponse(accountID string, settings *types.Settings, meta *types.A
 	if settings.Extra != nil {
 		apiSettings.Extra = &api.AccountExtraSettings{
 			PeerApprovalEnabled:                settings.Extra.PeerApprovalEnabled,
+			UserApprovalRequired:               settings.Extra.UserApprovalRequired,
 			NetworkTrafficLogsEnabled:          settings.Extra.FlowEnabled,
 			NetworkTrafficLogsGroups:           settings.Extra.FlowGroups,
 			NetworkTrafficPacketCounterEnabled: settings.Extra.FlowPacketCounterEnabled,

management/server/http/handlers/accounts/accounts_handler_test.go

@@ -15,11 +15,11 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	nbcontext "github.com/netbirdio/netbird/management/server/context"
-	"github.com/netbirdio/netbird/shared/management/http/api"
 	"github.com/netbirdio/netbird/management/server/mock_server"
 	"github.com/netbirdio/netbird/management/server/settings"
-	"github.com/netbirdio/netbird/shared/management/status"
 	"github.com/netbirdio/netbird/management/server/types"
+	"github.com/netbirdio/netbird/shared/management/http/api"
+	"github.com/netbirdio/netbird/shared/management/status"
 )
 
 func initAccountsTestData(t *testing.T, account *types.Account) *handler {

management/server/http/handlers/users/users_handler.go

@@ -9,11 +9,11 @@ import (
 	log "github.com/sirupsen/logrus"
 
 	"github.com/netbirdio/netbird/management/server/account"
+	"github.com/netbirdio/netbird/management/server/types"
+	"github.com/netbirdio/netbird/management/server/users"
 	"github.com/netbirdio/netbird/shared/management/http/api"
 	"github.com/netbirdio/netbird/shared/management/http/util"
 	"github.com/netbirdio/netbird/shared/management/status"
-	"github.com/netbirdio/netbird/management/server/types"
-	"github.com/netbirdio/netbird/management/server/users"
 
 	nbcontext "github.com/netbirdio/netbird/management/server/context"
 )
@@ -31,6 +31,8 @@ func AddEndpoints(accountManager account.Manager, router *mux.Router) {
 	router.HandleFunc("/users/{userId}", userHandler.deleteUser).Methods("DELETE", "OPTIONS")
 	router.HandleFunc("/users", userHandler.createUser).Methods("POST", "OPTIONS")
 	router.HandleFunc("/users/{userId}/invite", userHandler.inviteUser).Methods("POST", "OPTIONS")
+	router.HandleFunc("/users/{userId}/approve", userHandler.approveUser).Methods("POST", "OPTIONS")
+	router.HandleFunc("/users/{userId}/reject", userHandler.rejectUser).Methods("DELETE", "OPTIONS")
 	addUsersTokensEndpoint(accountManager, router)
 }
 
@@ -323,17 +325,76 @@ func toUserResponse(user *types.UserInfo, currenUserID string) *api.User {
 	}
 
 	isCurrent := user.ID == currenUserID
+
 	return &api.User{
-		Id:            user.ID,
-		Name:          user.Name,
-		Email:         user.Email,
-		Role:          user.Role,
-		AutoGroups:    autoGroups,
-		Status:        userStatus,
-		IsCurrent:     &isCurrent,
-		IsServiceUser: &user.IsServiceUser,
-		IsBlocked:     user.IsBlocked,
-		LastLogin:     &user.LastLogin,
-		Issued:        &user.Issued,
+		Id:              user.ID,
+		Name:            user.Name,
+		Email:           user.Email,
+		Role:            user.Role,
+		AutoGroups:      autoGroups,
+		Status:          userStatus,
+		IsCurrent:       &isCurrent,
+		IsServiceUser:   &user.IsServiceUser,
+		IsBlocked:       user.IsBlocked,
+		LastLogin:       &user.LastLogin,
+		Issued:          &user.Issued,
+		PendingApproval: user.PendingApproval,
+	}
+}
+
+// approveUser is a POST request to approve a user that is pending approval
+func (h *handler) approveUser(w http.ResponseWriter, r *http.Request) {
+	if r.Method != http.MethodPost {
+		util.WriteErrorResponse("wrong HTTP method", http.StatusMethodNotAllowed, w)
+		return
+	}
+
+	vars := mux.Vars(r)
+	targetUserID := vars["userId"]
+	if len(targetUserID) == 0 {
+		util.WriteErrorResponse("invalid user ID", http.StatusBadRequest, w)
+		return
+	}
+
+	userAuth, err := nbcontext.GetUserAuthFromContext(r.Context())
+	if err != nil {
+		util.WriteError(r.Context(), err, w)
+		return
+	}
+	user, err := h.accountManager.ApproveUser(r.Context(), userAuth.AccountId, userAuth.UserId, targetUserID)
+	if err != nil {
+		util.WriteError(r.Context(), err, w)
+		return
+	}
+
+	userResponse := toUserResponse(user, userAuth.UserId)
+	util.WriteJSONObject(r.Context(), w, userResponse)
+}
+
+// rejectUser is a DELETE request to reject a user that is pending approval
+func (h *handler) rejectUser(w http.ResponseWriter, r *http.Request) {
+	if r.Method != http.MethodDelete {
+		util.WriteErrorResponse("wrong HTTP method", http.StatusMethodNotAllowed, w)
+		return
+	}
+
+	vars := mux.Vars(r)
+	targetUserID := vars["userId"]
+	if len(targetUserID) == 0 {
+		util.WriteErrorResponse("invalid user ID", http.StatusBadRequest, w)
+		return
+	}
+
+	userAuth, err := nbcontext.GetUserAuthFromContext(r.Context())
+	if err != nil {
+		util.WriteError(r.Context(), err, w)
+		return
 	}
+	err = h.accountManager.RejectUser(r.Context(), userAuth.AccountId, userAuth.UserId, targetUserID)
+	if err != nil {
+		util.WriteError(r.Context(), err, w)
+		return
+	}
+
+	util.WriteJSONObject(r.Context(), w, util.EmptyObject{})
 }