From 9f7e13fc87b037c4d3071351bb58d010f1aca67e Mon Sep 17 00:00:00 2001
From: Bethuel Mmbaga <bethuelmbaga12@gmail.com>
Date: Tue, 7 Nov 2023 17:02:51 +0300
Subject: [PATCH] Enable deletion of integration resources (#1294)

* Enforce admin service user role for integration group deletion

Added a check to prevent non-admin service users from deleting integration groups.

* Restrict deletion of integration user to admin service user only

* Refactor user and group deletion tests
---
 management/server/group.go      | 10 ++++++++--
 management/server/group_test.go | 17 ++++++++++++++---
 management/server/user.go       |  5 +++--
 management/server/user_test.go  |  2 +-
 4 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/management/server/group.go b/management/server/group.go
index d626c353806..16f19147fba 100644
--- a/management/server/group.go
+++ b/management/server/group.go
@@ -163,9 +163,15 @@ func (am *DefaultAccountManager) DeleteGroup(accountId, userId, groupID string)
 		return nil
 	}
 
-	// check integration link
+	// disable a deleting integration group if the initiator is not an admin service user
 	if g.Issued == GroupIssuedIntegration {
-		return &GroupLinkError{GroupIssuedIntegration, g.IntegrationReference.String()}
+		executingUser := account.Users[userId]
+		if executingUser == nil {
+			return status.Errorf(status.NotFound, "user not found")
+		}
+		if executingUser.Role != UserRoleAdmin || !executingUser.IsServiceUser {
+			return status.Errorf(status.PermissionDenied, "only admins service user can delete integration group")
+		}
 	}
 
 	// check route links
diff --git a/management/server/group_test.go b/management/server/group_test.go
index 5db0ca9004f..97a1b75f5ed 100644
--- a/management/server/group_test.go
+++ b/management/server/group_test.go
@@ -1,9 +1,11 @@
 package server
 
 import (
+	"errors"
 	"testing"
 
 	nbdns "github.com/netbirdio/netbird/dns"
+	"github.com/netbirdio/netbird/management/server/status"
 	"github.com/netbirdio/netbird/route"
 )
 
@@ -55,19 +57,28 @@ func TestDefaultAccountManager_DeleteGroup(t *testing.T) {
 		{
 			"integration",
 			"grp-for-integration",
-			"integration",
+			"only admins service user can delete integration group",
 		},
 	}
 
 	for _, testCase := range testCases {
 		t.Run(testCase.name, func(t *testing.T) {
-			err = am.DeleteGroup(account.Id, "", testCase.groupID)
+			err = am.DeleteGroup(account.Id, groupAdminUserID, testCase.groupID)
 			if err == nil {
 				t.Errorf("delete %s group successfully", testCase.groupID)
 				return
 			}
 
-			gErr, ok := err.(*GroupLinkError)
+			var sErr *status.Error
+			if errors.As(err, &sErr) {
+				if sErr.Message != testCase.expectedReason {
+					t.Errorf("invalid error case: %s, expected: %s", sErr.Message, testCase.expectedReason)
+				}
+				return
+			}
+
+			var gErr *GroupLinkError
+			ok := errors.As(err, &gErr)
 			if !ok {
 				t.Error("invalid error type")
 				return
diff --git a/management/server/user.go b/management/server/user.go
index 22edd2c2cef..71d3cd7f4de 100644
--- a/management/server/user.go
+++ b/management/server/user.go
@@ -387,8 +387,9 @@ func (am *DefaultAccountManager) DeleteUser(accountID, initiatorUserID string, t
 		return status.Errorf(status.NotFound, "target user not found")
 	}
 
-	if targetUser.Issued == UserIssuedIntegration {
-		return status.Errorf(status.PermissionDenied, "only integration can delete this user")
+	// disable deleting integration user if the initiator is not admin service user
+	if targetUser.Issued == UserIssuedIntegration && !executingUser.IsServiceUser {
+		return status.Errorf(status.PermissionDenied, "only admin service user can delete this user")
 	}
 
 	// handle service user first and exit, no need to fetch extra data from IDP, etc
diff --git a/management/server/user_test.go b/management/server/user_test.go
index f1b997186ee..40e7f9a2d27 100644
--- a/management/server/user_test.go
+++ b/management/server/user_test.go
@@ -508,7 +508,7 @@ func TestUser_DeleteUser_regularUser(t *testing.T) {
 			name:             "Delete integration regular user permission denied ",
 			userID:           "user4",
 			assertErrFunc:    assert.Error,
-			assertErrMessage: "only integration can delete this user",
+			assertErrMessage: "only admin service user can delete this user",
 		},
 	}