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
This commit is contained in:
Bethuel Mmbaga 2023-11-07 17:02:51 +03:00 committed by GitHub
parent 8be6e92563
commit 9f7e13fc87
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 26 additions and 8 deletions

View File

@ -163,9 +163,15 @@ func (am *DefaultAccountManager) DeleteGroup(accountId, userId, groupID string)
return nil return nil
} }
// check integration link // disable a deleting integration group if the initiator is not an admin service user
if g.Issued == GroupIssuedIntegration { 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 // check route links

View File

@ -1,9 +1,11 @@
package server package server
import ( import (
"errors"
"testing" "testing"
nbdns "github.com/netbirdio/netbird/dns" nbdns "github.com/netbirdio/netbird/dns"
"github.com/netbirdio/netbird/management/server/status"
"github.com/netbirdio/netbird/route" "github.com/netbirdio/netbird/route"
) )
@ -55,19 +57,28 @@ func TestDefaultAccountManager_DeleteGroup(t *testing.T) {
{ {
"integration", "integration",
"grp-for-integration", "grp-for-integration",
"integration", "only admins service user can delete integration group",
}, },
} }
for _, testCase := range testCases { for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) { 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 { if err == nil {
t.Errorf("delete %s group successfully", testCase.groupID) t.Errorf("delete %s group successfully", testCase.groupID)
return 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 { if !ok {
t.Error("invalid error type") t.Error("invalid error type")
return return

View File

@ -387,8 +387,9 @@ func (am *DefaultAccountManager) DeleteUser(accountID, initiatorUserID string, t
return status.Errorf(status.NotFound, "target user not found") return status.Errorf(status.NotFound, "target user not found")
} }
if targetUser.Issued == UserIssuedIntegration { // disable deleting integration user if the initiator is not admin service user
return status.Errorf(status.PermissionDenied, "only integration can delete this 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 // handle service user first and exit, no need to fetch extra data from IDP, etc

View File

@ -508,7 +508,7 @@ func TestUser_DeleteUser_regularUser(t *testing.T) {
name: "Delete integration regular user permission denied ", name: "Delete integration regular user permission denied ",
userID: "user4", userID: "user4",
assertErrFunc: assert.Error, assertErrFunc: assert.Error,
assertErrMessage: "only integration can delete this user", assertErrMessage: "only admin service user can delete this user",
}, },
} }