[management] permission manager validate account access

This commit is contained in:
Pedro Costa
2025-03-05 16:55:44 +00:00
parent 9325fb7990
commit f9f47b0ad8
25 changed files with 267 additions and 155 deletions

View File

@@ -29,6 +29,7 @@ import (
"github.com/netbirdio/netbird/management/server/integrations/integrated_validator"
"github.com/netbirdio/netbird/management/server/integrations/port_forwarding"
nbpeer "github.com/netbirdio/netbird/management/server/peer"
"github.com/netbirdio/netbird/management/server/permissions"
"github.com/netbirdio/netbird/management/server/posture"
"github.com/netbirdio/netbird/management/server/settings"
"github.com/netbirdio/netbird/management/server/status"
@@ -89,6 +90,8 @@ type DefaultAccountManager struct {
integratedPeerValidator integrated_validator.IntegratedValidator
metrics telemetry.AppMetrics
permissionsManager permissions.Manager
}
// getJWTGroupsChanges calculates the changes needed to sync a user's JWT groups.
@@ -156,6 +159,7 @@ func BuildManager(
metrics telemetry.AppMetrics,
proxyController port_forwarding.Controller,
settingsManager settings.Manager,
permissionsManager permissions.Manager,
) (*DefaultAccountManager, error) {
start := time.Now()
defer func() {
@@ -180,6 +184,7 @@ func BuildManager(
requestBuffer: NewAccountRequestBuffer(ctx, store),
proxyController: proxyController,
settingsManager: settingsManager,
permissionsManager: permissionsManager,
}
accountsCounter, err := store.GetAccountsCounter(ctx)
if err != nil {
@@ -508,9 +513,10 @@ func (am *DefaultAccountManager) DeleteAccount(ctx context.Context, accountID, u
return err
}
if !user.HasAdminPower() {
return status.Errorf(status.PermissionDenied, "user is not allowed to delete account")
}
// @note not necessary, below it explicitly checks for Owner role
// if !user.HasAdminPower() {
// return status.Errorf(status.PermissionDenied, "user is not allowed to delete account")
// }
if user.Role != types.UserRoleOwner {
return status.Errorf(status.PermissionDenied, "user is not allowed to delete account. Only account owner can delete account")
@@ -1027,8 +1033,8 @@ func (am *DefaultAccountManager) GetAccountByID(ctx context.Context, accountID s
return nil, err
}
if user.AccountID != accountID {
return nil, status.Errorf(status.PermissionDenied, "the user has no permission to access account data")
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
return am.Store.GetAccount(ctx, accountID)
@@ -1061,6 +1067,7 @@ func (am *DefaultAccountManager) GetAccountIDFromUserAuth(ctx context.Context, u
return accountID, user.Id, nil
}
// @note, this can remain cause above we explicitly early return if auth id for a child account
if user.AccountID != accountID {
return "", "", status.Errorf(status.PermissionDenied, "user %s is not part of the account %s", userAuth.UserId, accountID)
}
@@ -1521,7 +1528,11 @@ func (am *DefaultAccountManager) GetAccountSettings(ctx context.Context, account
return nil, err
}
if user.AccountID != accountID || (!user.HasAdminPower() && !user.IsServiceUser) {
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
if !user.HasAdminPower() && !user.IsServiceUser {
return nil, status.Errorf(status.PermissionDenied, "the user has no permission to access account data")
}

View File

@@ -17,6 +17,7 @@ import (
nbAccount "github.com/netbirdio/netbird/management/server/account"
"github.com/netbirdio/netbird/management/server/integrations/port_forwarding"
"github.com/netbirdio/netbird/management/server/permissions"
"github.com/netbirdio/netbird/management/server/settings"
"github.com/netbirdio/netbird/management/server/util"
@@ -2815,6 +2816,8 @@ func createManager(t TB) (*DefaultAccountManager, error) {
return nil, err
}
permissionsManagerMock := permissions.NewManagerMock()
ctrl := gomock.NewController(t)
t.Cleanup(ctrl.Finish)
@@ -2828,7 +2831,7 @@ func createManager(t TB) (*DefaultAccountManager, error) {
Return(false, nil).
AnyTimes()
manager, err := BuildManager(context.Background(), store, NewPeersUpdateManager(nil), nil, "", "netbird.cloud", eventStore, nil, false, MocIntegratedValidator{}, metrics, port_forwarding.NewControllerMock(), settingsMockManager)
manager, err := BuildManager(context.Background(), store, NewPeersUpdateManager(nil), nil, "", "netbird.cloud", eventStore, nil, false, MocIntegratedValidator{}, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManagerMock)
if err != nil {
return nil, err
}

View File

@@ -67,8 +67,8 @@ func (am *DefaultAccountManager) GetDNSSettings(ctx context.Context, accountID s
return nil, err
}
if user.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
if user.IsRegularUser() {
@@ -89,8 +89,8 @@ func (am *DefaultAccountManager) SaveDNSSettings(ctx context.Context, accountID
return err
}
if user.AccountID != accountID {
return status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return err
}
if !user.HasAdminPower() {

View File

@@ -13,6 +13,7 @@ import (
nbdns "github.com/netbirdio/netbird/dns"
"github.com/netbirdio/netbird/management/server/integrations/port_forwarding"
"github.com/netbirdio/netbird/management/server/permissions"
"github.com/netbirdio/netbird/management/server/settings"
"github.com/netbirdio/netbird/management/server/store"
"github.com/netbirdio/netbird/management/server/telemetry"
@@ -210,13 +211,14 @@ func createDNSManager(t *testing.T) (*DefaultAccountManager, error) {
metrics, err := telemetry.NewDefaultAppMetrics(context.Background())
require.NoError(t, err)
permissionsManagerMock := permissions.NewManagerMock()
ctrl := gomock.NewController(t)
t.Cleanup(ctrl.Finish)
settingsMockManager := settings.NewMockManager(ctrl)
return BuildManager(context.Background(), store, NewPeersUpdateManager(nil), nil, "", "netbird.test", eventStore, nil, false, MocIntegratedValidator{}, metrics, port_forwarding.NewControllerMock(), settingsMockManager)
return BuildManager(context.Background(), store, NewPeersUpdateManager(nil), nil, "", "netbird.test", eventStore, nil, false, MocIntegratedValidator{}, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManagerMock)
}
func createDNSStore(t *testing.T) (store.Store, error) {

View File

@@ -35,8 +35,8 @@ func (am *DefaultAccountManager) CheckGroupPermissions(ctx context.Context, acco
return err
}
if user.AccountID != accountID {
return status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return err
}
if user.IsRegularUser() {
@@ -83,8 +83,8 @@ func (am *DefaultAccountManager) SaveGroups(ctx context.Context, accountID, user
return err
}
if user.AccountID != accountID {
return status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return err
}
if user.IsRegularUser() {
@@ -215,8 +215,8 @@ func (am *DefaultAccountManager) DeleteGroups(ctx context.Context, accountID, us
return err
}
if user.AccountID != accountID {
return status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return err
}
if user.IsRegularUser() {

View File

@@ -16,19 +16,18 @@ import (
"github.com/golang-jwt/jwt"
"github.com/netbirdio/management-integrations/integrations"
"github.com/netbirdio/netbird/management/server/account"
"github.com/netbirdio/netbird/management/server/settings"
"github.com/netbirdio/netbird/management/server/users"
"github.com/stretchr/testify/assert"
"golang.zx2c4.com/wireguard/wgctrl/wgtypes"
"github.com/netbirdio/management-integrations/integrations"
"github.com/netbirdio/netbird/management/server/peers"
"github.com/netbirdio/netbird/management/server/permissions"
"github.com/netbirdio/netbird/management/server/settings"
"github.com/netbirdio/netbird/management/server/users"
"github.com/netbirdio/netbird/management/server"
"github.com/netbirdio/netbird/management/server/account"
"github.com/netbirdio/netbird/management/server/activity"
"github.com/netbirdio/netbird/management/server/auth"
nbcontext "github.com/netbirdio/netbird/management/server/context"
@@ -125,7 +124,8 @@ func BuildApiBlackBoxWithDBState(t TB, sqlFile string, expectedPeerUpdate *serve
proxyController := integrations.NewController(store)
userManager := users.NewManager(store)
settingsManager := settings.NewManager(store, userManager, integrations.NewManager(&activity.InMemoryEventStore{}))
am, err := server.BuildManager(context.Background(), store, peersUpdateManager, nil, "", "", &activity.InMemoryEventStore{}, geoMock, false, validatorMock, metrics, proxyController, settingsManager)
permissionsManagerMock := permissions.NewManagerMock()
am, err := server.BuildManager(context.Background(), store, peersUpdateManager, nil, "", "", &activity.InMemoryEventStore{}, geoMock, false, validatorMock, metrics, proxyController, settingsManager, permissionsManagerMock)
if err != nil {
t.Fatalf("Failed to create manager: %v", err)
}
@@ -143,7 +143,6 @@ func BuildApiBlackBoxWithDBState(t TB, sqlFile string, expectedPeerUpdate *serve
resourcesManagerMock := resources.NewManagerMock()
routersManagerMock := routers.NewManagerMock()
groupsManagerMock := groups.NewManagerMock()
permissionsManagerMock := permissions.NewManagerMock()
peersManager := peers.NewManager(store, permissionsManagerMock)
apiHandler, err := nbhttp.NewAPIHandler(context.Background(), am, networksManagerMock, resourcesManagerMock, routersManagerMock, groupsManagerMock, geoMock, authManagerMock, metrics, validatorMock, proxyController, permissionsManagerMock, peersManager, settingsManager)

View File

@@ -25,6 +25,7 @@ import (
mgmtProto "github.com/netbirdio/netbird/management/proto"
"github.com/netbirdio/netbird/management/server/activity"
"github.com/netbirdio/netbird/management/server/integrations/port_forwarding"
"github.com/netbirdio/netbird/management/server/permissions"
"github.com/netbirdio/netbird/management/server/settings"
"github.com/netbirdio/netbird/management/server/store"
"github.com/netbirdio/netbird/management/server/telemetry"
@@ -431,6 +432,8 @@ func startManagementForTest(t *testing.T, testFile string, config *Config) (*grp
metrics, err := telemetry.NewDefaultAppMetrics(context.Background())
require.NoError(t, err)
permissionsManagerMock := permissions.NewManagerMock()
ctrl := gomock.NewController(t)
t.Cleanup(ctrl.Finish)
settingsMockManager := settings.NewMockManager(ctrl)
@@ -441,7 +444,7 @@ func startManagementForTest(t *testing.T, testFile string, config *Config) (*grp
Return(&types.Settings{}, nil)
accountManager, err := BuildManager(ctx, store, peersUpdateManager, nil, "", "netbird.selfhosted",
eventStore, nil, false, MocIntegratedValidator{}, metrics, port_forwarding.NewControllerMock(), settingsMockManager)
eventStore, nil, false, MocIntegratedValidator{}, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManagerMock)
if err != nil {
cleanup()

View File

@@ -24,6 +24,7 @@ import (
"github.com/netbirdio/netbird/management/server"
"github.com/netbirdio/netbird/management/server/activity"
"github.com/netbirdio/netbird/management/server/integrations/port_forwarding"
"github.com/netbirdio/netbird/management/server/permissions"
"github.com/netbirdio/netbird/management/server/settings"
"github.com/netbirdio/netbird/management/server/store"
"github.com/netbirdio/netbird/management/server/telemetry"
@@ -194,6 +195,7 @@ func startServer(
Return(&types.Settings{}, nil).
AnyTimes()
permissionsManagerMock := permissions.NewManagerMock()
accountManager, err := server.BuildManager(
context.Background(),
str,
@@ -208,6 +210,7 @@ func startServer(
metrics,
port_forwarding.NewControllerMock(),
settingsMockManager,
permissionsManagerMock,
)
if err != nil {
t.Fatalf("failed creating an account manager: %v", err)

View File

@@ -25,8 +25,8 @@ func (am *DefaultAccountManager) GetNameServerGroup(ctx context.Context, account
return nil, err
}
if user.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
if user.IsRegularUser() {
@@ -46,8 +46,8 @@ func (am *DefaultAccountManager) CreateNameServerGroup(ctx context.Context, acco
return nil, err
}
if user.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
newNSGroup := &nbdns.NameServerGroup{
@@ -108,8 +108,8 @@ func (am *DefaultAccountManager) SaveNameServerGroup(ctx context.Context, accoun
return err
}
if user.AccountID != accountID {
return status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return err
}
var updateAccountPeers bool
@@ -159,8 +159,8 @@ func (am *DefaultAccountManager) DeleteNameServerGroup(ctx context.Context, acco
return err
}
if user.AccountID != accountID {
return status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return err
}
var nsGroup *nbdns.NameServerGroup
@@ -203,8 +203,8 @@ func (am *DefaultAccountManager) ListNameServerGroups(ctx context.Context, accou
return nil, err
}
if user.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
if user.IsRegularUser() {

View File

@@ -14,6 +14,7 @@ import (
"github.com/netbirdio/netbird/management/server/activity"
"github.com/netbirdio/netbird/management/server/integrations/port_forwarding"
nbpeer "github.com/netbirdio/netbird/management/server/peer"
"github.com/netbirdio/netbird/management/server/permissions"
"github.com/netbirdio/netbird/management/server/settings"
"github.com/netbirdio/netbird/management/server/store"
"github.com/netbirdio/netbird/management/server/telemetry"
@@ -774,11 +775,12 @@ func createNSManager(t *testing.T) (*DefaultAccountManager, error) {
metrics, err := telemetry.NewDefaultAppMetrics(context.Background())
require.NoError(t, err)
permissionsManagerMock := permissions.NewManagerMock()
ctrl := gomock.NewController(t)
t.Cleanup(ctrl.Finish)
settingsMockManager := settings.NewMockManager(ctrl)
return BuildManager(context.Background(), store, NewPeersUpdateManager(nil), nil, "", "netbird.selfhosted", eventStore, nil, false, MocIntegratedValidator{}, metrics, port_forwarding.NewControllerMock(), settingsMockManager)
return BuildManager(context.Background(), store, NewPeersUpdateManager(nil), nil, "", "netbird.selfhosted", eventStore, nil, false, MocIntegratedValidator{}, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManagerMock)
}
func createNSStore(t *testing.T) (store.Store, error) {

View File

@@ -37,8 +37,8 @@ func (am *DefaultAccountManager) GetPeers(ctx context.Context, accountID, userID
return nil, err
}
if user.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
settings, err := am.Store.GetAccountSettings(ctx, store.LockingStrengthShare, accountID)
@@ -188,8 +188,8 @@ func (am *DefaultAccountManager) UpdatePeer(ctx context.Context, accountID, user
return nil, err
}
if user.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
var peer *nbpeer.Peer
@@ -321,8 +321,8 @@ func (am *DefaultAccountManager) DeletePeer(ctx context.Context, accountID, peer
return err
}
if user.AccountID != accountID {
return status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return err
}
}
@@ -1099,8 +1099,8 @@ func (am *DefaultAccountManager) GetPeer(ctx context.Context, accountID, peerID,
return nil, err
}
if user.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
settings, err := am.Store.GetAccountSettings(ctx, store.LockingStrengthShare, accountID)

View File

@@ -20,9 +20,10 @@ import (
"github.com/stretchr/testify/require"
"golang.zx2c4.com/wireguard/wgctrl/wgtypes"
"github.com/netbirdio/netbird/management/server/integrations/port_forwarding"
"github.com/netbirdio/netbird/management/server/permissions"
"github.com/netbirdio/netbird/management/server/settings"
"github.com/netbirdio/netbird/management/server/integrations/port_forwarding"
"github.com/netbirdio/netbird/management/server/util"
resourceTypes "github.com/netbirdio/netbird/management/server/networks/resources/types"
@@ -1217,7 +1218,8 @@ func Test_RegisterPeerByUser(t *testing.T) {
t.Cleanup(ctrl.Finish)
settingsMockManager := settings.NewMockManager(ctrl)
am, err := BuildManager(context.Background(), s, NewPeersUpdateManager(nil), nil, "", "netbird.cloud", eventStore, nil, false, MocIntegratedValidator{}, metrics, port_forwarding.NewControllerMock(), settingsMockManager)
permissionsManagerMock := permissions.NewManagerMock()
am, err := BuildManager(context.Background(), s, NewPeersUpdateManager(nil), nil, "", "netbird.cloud", eventStore, nil, false, MocIntegratedValidator{}, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManagerMock)
assert.NoError(t, err)
existingAccountID := "bf1c8084-ba50-4ce7-9439-34653001fc3b"
@@ -1285,7 +1287,8 @@ func Test_RegisterPeerBySetupKey(t *testing.T) {
t.Cleanup(ctrl.Finish)
settingsMockManager := settings.NewMockManager(ctrl)
am, err := BuildManager(context.Background(), s, NewPeersUpdateManager(nil), nil, "", "netbird.cloud", eventStore, nil, false, MocIntegratedValidator{}, metrics, port_forwarding.NewControllerMock(), settingsMockManager)
permissionsManagerMock := permissions.NewManagerMock()
am, err := BuildManager(context.Background(), s, NewPeersUpdateManager(nil), nil, "", "netbird.cloud", eventStore, nil, false, MocIntegratedValidator{}, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManagerMock)
assert.NoError(t, err)
existingAccountID := "bf1c8084-ba50-4ce7-9439-34653001fc3b"
@@ -1356,7 +1359,8 @@ func Test_RegisterPeerRollbackOnFailure(t *testing.T) {
t.Cleanup(ctrl.Finish)
settingsMockManager := settings.NewMockManager(ctrl)
am, err := BuildManager(context.Background(), s, NewPeersUpdateManager(nil), nil, "", "netbird.cloud", eventStore, nil, false, MocIntegratedValidator{}, metrics, port_forwarding.NewControllerMock(), settingsMockManager)
permissionsManagerMock := permissions.NewManagerMock()
am, err := BuildManager(context.Background(), s, NewPeersUpdateManager(nil), nil, "", "netbird.cloud", eventStore, nil, false, MocIntegratedValidator{}, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManagerMock)
assert.NoError(t, err)
existingAccountID := "bf1c8084-ba50-4ce7-9439-34653001fc3b"

View File

@@ -7,6 +7,7 @@ import (
"github.com/netbirdio/netbird/management/server/activity"
"github.com/netbirdio/netbird/management/server/settings"
"github.com/netbirdio/netbird/management/server/status"
"github.com/netbirdio/netbird/management/server/types"
"github.com/netbirdio/netbird/management/server/users"
)
@@ -28,6 +29,7 @@ const (
type Manager interface {
ValidateUserPermissions(ctx context.Context, accountID, userID string, module Module, operation Operation) (bool, error)
ValidateAccountAccess(ctx context.Context, accountID string, user *types.User) error
}
type managerImpl struct {
@@ -52,11 +54,11 @@ func (m *managerImpl) ValidateUserPermissions(ctx context.Context, accountID, us
}
if user == nil {
return false, errors.New("user not found")
return false, status.NewUserNotFoundError(userID)
}
if user.AccountID != accountID {
return false, errors.New("user does not belong to account")
if err := m.ValidateAccountAccess(ctx, accountID, user); err != nil {
return false, err
}
switch user.Role {
@@ -91,6 +93,13 @@ func (m *managerImpl) validateRegularUserPermissions(ctx context.Context, accoun
return false, nil
}
func (m *managerImpl) ValidateAccountAccess(ctx context.Context, accountID string, user *types.User) error {
if user.AccountID != accountID {
return status.NewUserNotPartOfAccountError()
}
return nil
}
func NewManagerMock() Manager {
return &managerMock{}
}
@@ -101,3 +110,11 @@ func (m *managerMock) ValidateUserPermissions(ctx context.Context, accountID, us
}
return false, nil
}
func (m *managerMock) ValidateAccountAccess(ctx context.Context, accountID string, user *types.User) error {
// @note managers explicitly checked this, so should the mock
if user.AccountID != accountID {
return status.NewUserNotPartOfAccountError()
}
return nil
}

View File

@@ -22,8 +22,8 @@ func (am *DefaultAccountManager) GetPolicy(ctx context.Context, accountID, polic
return nil, err
}
if user.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
if user.IsRegularUser() {
@@ -43,8 +43,8 @@ func (am *DefaultAccountManager) SavePolicy(ctx context.Context, accountID, user
return nil, err
}
if user.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
if user.IsRegularUser() {
@@ -100,8 +100,8 @@ func (am *DefaultAccountManager) DeletePolicy(ctx context.Context, accountID, po
return err
}
if user.AccountID != accountID {
return status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return err
}
if user.IsRegularUser() {
@@ -148,8 +148,8 @@ func (am *DefaultAccountManager) ListPolicies(ctx context.Context, accountID, us
return nil, err
}
if user.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
if user.IsRegularUser() {

View File

@@ -22,8 +22,8 @@ func (am *DefaultAccountManager) GetPostureChecks(ctx context.Context, accountID
return nil, err
}
if user.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
if !user.HasAdminPower() {
@@ -43,8 +43,8 @@ func (am *DefaultAccountManager) SavePostureChecks(ctx context.Context, accountI
return nil, err
}
if user.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
if !user.HasAdminPower() {
@@ -99,8 +99,8 @@ func (am *DefaultAccountManager) DeletePostureChecks(ctx context.Context, accoun
return err
}
if user.AccountID != accountID {
return status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return err
}
if !user.HasAdminPower() {
@@ -141,8 +141,8 @@ func (am *DefaultAccountManager) ListPostureChecks(ctx context.Context, accountI
return nil, err
}
if user.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
if !user.HasAdminPower() {

View File

@@ -25,7 +25,11 @@ func (am *DefaultAccountManager) GetRoute(ctx context.Context, accountID string,
return nil, err
}
if !user.IsAdminOrServiceUser() || user.AccountID != accountID {
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
if !user.IsAdminOrServiceUser() {
return nil, status.Errorf(status.PermissionDenied, "only users with admin power can view Network Routes")
}
@@ -342,7 +346,11 @@ func (am *DefaultAccountManager) ListRoutes(ctx context.Context, accountID, user
return nil, err
}
if !user.IsAdminOrServiceUser() || user.AccountID != accountID {
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
if !user.IsAdminOrServiceUser() {
return nil, status.Errorf(status.PermissionDenied, "only users with admin power can view Network Routes")
}

View File

@@ -21,6 +21,7 @@ import (
routerTypes "github.com/netbirdio/netbird/management/server/networks/routers/types"
networkTypes "github.com/netbirdio/netbird/management/server/networks/types"
nbpeer "github.com/netbirdio/netbird/management/server/peer"
"github.com/netbirdio/netbird/management/server/permissions"
"github.com/netbirdio/netbird/management/server/settings"
"github.com/netbirdio/netbird/management/server/store"
"github.com/netbirdio/netbird/management/server/telemetry"
@@ -1259,6 +1260,7 @@ func createRouterManager(t *testing.T) (*DefaultAccountManager, error) {
metrics, err := telemetry.NewDefaultAppMetrics(context.Background())
require.NoError(t, err)
permissionsManagerMock := permissions.NewManagerMock()
ctrl := gomock.NewController(t)
t.Cleanup(ctrl.Finish)
@@ -1281,7 +1283,7 @@ func createRouterManager(t *testing.T) (*DefaultAccountManager, error) {
AnyTimes().
Return(&types.ExtraSettings{}, nil)
return BuildManager(context.Background(), store, NewPeersUpdateManager(nil), nil, "", "netbird.selfhosted", eventStore, nil, false, MocIntegratedValidator{}, metrics, port_forwarding.NewControllerMock(), settingsMockManager)
return BuildManager(context.Background(), store, NewPeersUpdateManager(nil), nil, "", "netbird.selfhosted", eventStore, nil, false, MocIntegratedValidator{}, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManagerMock)
}
func createRouterStore(t *testing.T) (store.Store, error) {

View File

@@ -61,8 +61,8 @@ func (am *DefaultAccountManager) CreateSetupKey(ctx context.Context, accountID s
return nil, err
}
if user.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
if user.IsRegularUser() {
@@ -118,8 +118,8 @@ func (am *DefaultAccountManager) SaveSetupKey(ctx context.Context, accountID str
return nil, err
}
if user.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
if user.IsRegularUser() {
@@ -180,8 +180,8 @@ func (am *DefaultAccountManager) ListSetupKeys(ctx context.Context, accountID, u
return nil, err
}
if user.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
if user.IsRegularUser() {
@@ -198,8 +198,8 @@ func (am *DefaultAccountManager) GetSetupKey(ctx context.Context, accountID, use
return nil, err
}
if user.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return nil, err
}
if user.IsRegularUser() {
@@ -226,8 +226,8 @@ func (am *DefaultAccountManager) DeleteSetupKey(ctx context.Context, accountID,
return err
}
if user.AccountID != accountID {
return status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user); err != nil {
return err
}
if user.IsRegularUser() {

View File

@@ -30,8 +30,8 @@ func (am *DefaultAccountManager) createServiceUser(ctx context.Context, accountI
return nil, err
}
if initiatorUser.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, initiatorUser); err != nil {
return nil, err
}
if !initiatorUser.HasAdminPower() {
@@ -93,8 +93,8 @@ func (am *DefaultAccountManager) inviteNewUser(ctx context.Context, accountID, u
return nil, err
}
if initiatorUser.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, initiatorUser); err != nil {
return nil, err
}
inviterID := userID
@@ -228,8 +228,8 @@ func (am *DefaultAccountManager) DeleteUser(ctx context.Context, accountID, init
return err
}
if initiatorUser.AccountID != accountID {
return status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, initiatorUser); err != nil {
return err
}
if !initiatorUser.HasAdminPower() {
@@ -290,8 +290,8 @@ func (am *DefaultAccountManager) InviteUser(ctx context.Context, accountID strin
return err
}
if initiatorUser.AccountID != accountID {
return status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, initiatorUser); err != nil {
return err
}
// check if the user is already registered with this ID
@@ -338,8 +338,8 @@ func (am *DefaultAccountManager) CreatePAT(ctx context.Context, accountID string
return nil, err
}
if initiatorUser.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, initiatorUser); err != nil {
return nil, err
}
targetUser, err := am.Store.GetUserByUserID(ctx, store.LockingStrengthShare, targetUserID)
@@ -347,6 +347,7 @@ func (am *DefaultAccountManager) CreatePAT(ctx context.Context, accountID string
return nil, err
}
// @todo how to handle this case, PAT can only be created own user?
if initiatorUserID != targetUserID && !(initiatorUser.HasAdminPower() && targetUser.IsServiceUser) {
return nil, status.NewAdminPermissionError()
}
@@ -376,10 +377,11 @@ func (am *DefaultAccountManager) DeletePAT(ctx context.Context, accountID string
return err
}
if initiatorUser.AccountID != accountID {
return status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, initiatorUser); err != nil {
return err
}
// @todo how to handle this case, PAT can only be deleted by own user?
if initiatorUserID != targetUserID && initiatorUser.IsRegularUser() {
return status.NewAdminPermissionError()
}
@@ -411,10 +413,11 @@ func (am *DefaultAccountManager) GetPAT(ctx context.Context, accountID string, i
return nil, err
}
if initiatorUser.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, initiatorUser); err != nil {
return nil, err
}
// @todo how to handle this case, PAT can only be got by own user?
if initiatorUserID != targetUserID && initiatorUser.IsRegularUser() {
return nil, status.NewAdminPermissionError()
}
@@ -429,8 +432,8 @@ func (am *DefaultAccountManager) GetAllPATs(ctx context.Context, accountID strin
return nil, err
}
if initiatorUser.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, initiatorUser); err != nil {
return nil, err
}
if initiatorUserID != targetUserID && initiatorUser.IsRegularUser() {
@@ -476,8 +479,8 @@ func (am *DefaultAccountManager) SaveOrAddUsers(ctx context.Context, accountID,
return nil, err
}
if initiatorUser.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, initiatorUser); err != nil {
return nil, err
}
if !initiatorUser.HasAdminPower() || initiatorUser.IsBlocked() {
@@ -705,6 +708,7 @@ func (am *DefaultAccountManager) getUserInfo(ctx context.Context, user *types.Us
// validateUserUpdate validates the update operation for a user.
func validateUserUpdate(groupsMap map[string]*types.Group, initiatorUser, oldUser, update *types.User) error {
// @todo double check these
if initiatorUser.HasAdminPower() && initiatorUser.Id == update.Id && oldUser.Blocked != update.Blocked {
return status.Errorf(status.PermissionDenied, "admins can't block or unblock themselves")
}
@@ -790,8 +794,8 @@ func (am *DefaultAccountManager) GetUsersFromAccount(ctx context.Context, accoun
return nil, err
}
if initiatorUser.AccountID != accountID {
return nil, status.NewUserNotPartOfAccountError()
if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, initiatorUser); err != nil {
return nil, err
}
return am.BuildUserInfosForAccount(ctx, accountID, initiatorUserID, accountUsers)
@@ -967,6 +971,8 @@ func (am *DefaultAccountManager) DeleteRegularUsers(ctx context.Context, account
return err
}
// @todo maybe add ValidateAccountPermission?
if !initiatorUser.HasAdminPower() {
return status.NewAdminPermissionError()
}

View File

@@ -13,6 +13,7 @@ import (
nbcache "github.com/netbirdio/netbird/management/server/cache"
nbcontext "github.com/netbirdio/netbird/management/server/context"
"github.com/netbirdio/netbird/management/server/permissions"
"github.com/netbirdio/netbird/management/server/util"
nbpeer "github.com/netbirdio/netbird/management/server/peer"
@@ -59,9 +60,11 @@ func TestUser_CreatePAT_ForSameUser(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: s,
eventStore: &activity.InMemoryEventStore{},
Store: s,
eventStore: &activity.InMemoryEventStore{},
permissionsManager: permissionsMananagerMock,
}
pat, err := am.CreatePAT(context.Background(), mockAccountID, mockUserID, mockUserID, mockTokenName, mockExpiresIn)
@@ -107,9 +110,11 @@ func TestUser_CreatePAT_ForDifferentUser(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
Store: store,
eventStore: &activity.InMemoryEventStore{},
permissionsManager: permissionsMananagerMock,
}
_, err = am.CreatePAT(context.Background(), mockAccountID, mockUserID, mockTargetUserId, mockTokenName, mockExpiresIn)
@@ -133,9 +138,11 @@ func TestUser_CreatePAT_ForServiceUser(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
Store: store,
eventStore: &activity.InMemoryEventStore{},
permissionsManager: permissionsMananagerMock,
}
pat, err := am.CreatePAT(context.Background(), mockAccountID, mockUserID, mockTargetUserId, mockTokenName, mockExpiresIn)
@@ -160,9 +167,11 @@ func TestUser_CreatePAT_WithWrongExpiration(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
Store: store,
eventStore: &activity.InMemoryEventStore{},
permissionsManager: permissionsMananagerMock,
}
_, err = am.CreatePAT(context.Background(), mockAccountID, mockUserID, mockUserID, mockTokenName, mockWrongExpiresIn)
@@ -183,9 +192,11 @@ func TestUser_CreatePAT_WithEmptyName(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
Store: store,
eventStore: &activity.InMemoryEventStore{},
permissionsManager: permissionsMananagerMock,
}
_, err = am.CreatePAT(context.Background(), mockAccountID, mockUserID, mockUserID, mockEmptyTokenName, mockExpiresIn)
@@ -214,9 +225,11 @@ func TestUser_DeletePAT(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
Store: store,
eventStore: &activity.InMemoryEventStore{},
permissionsManager: permissionsMananagerMock,
}
err = am.DeletePAT(context.Background(), mockAccountID, mockUserID, mockUserID, mockTokenID1)
@@ -255,9 +268,11 @@ func TestUser_GetPAT(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
Store: store,
eventStore: &activity.InMemoryEventStore{},
permissionsManager: permissionsMananagerMock,
}
pat, err := am.GetPAT(context.Background(), mockAccountID, mockUserID, mockUserID, mockTokenID1)
@@ -296,9 +311,11 @@ func TestUser_GetAllPATs(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
Store: store,
eventStore: &activity.InMemoryEventStore{},
permissionsManager: permissionsMananagerMock,
}
pats, err := am.GetAllPATs(context.Background(), mockAccountID, mockUserID, mockUserID)
@@ -390,9 +407,11 @@ func TestUser_CreateServiceUser(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
Store: store,
eventStore: &activity.InMemoryEventStore{},
permissionsManager: permissionsMananagerMock,
}
user, err := am.createServiceUser(context.Background(), mockAccountID, mockUserID, mockRole, mockServiceUserName, false, []string{"group1", "group2"})
@@ -435,9 +454,11 @@ func TestUser_CreateUser_ServiceUser(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
Store: store,
eventStore: &activity.InMemoryEventStore{},
permissionsManager: permissionsMananagerMock,
}
user, err := am.CreateUser(context.Background(), mockAccountID, mockUserID, &types.UserInfo{
@@ -481,9 +502,11 @@ func TestUser_CreateUser_RegularUser(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
Store: store,
eventStore: &activity.InMemoryEventStore{},
permissionsManager: permissionsMananagerMock,
}
_, err = am.CreateUser(context.Background(), mockAccountID, mockUserID, &types.UserInfo{
@@ -510,10 +533,12 @@ func TestUser_InviteNewUser(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
cacheLoading: map[string]chan struct{}{},
Store: store,
eventStore: &activity.InMemoryEventStore{},
cacheLoading: map[string]chan struct{}{},
permissionsManager: permissionsMananagerMock,
}
cs, err := nbcache.NewStore(context.Background(), nbcache.DefaultIDPCacheExpirationMax, nbcache.DefaultIDPCacheCleanupInterval)
@@ -616,9 +641,11 @@ func TestUser_DeleteUser_ServiceUser(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
Store: store,
eventStore: &activity.InMemoryEventStore{},
permissionsManager: permissionsMananagerMock,
}
err = am.DeleteUser(context.Background(), mockAccountID, mockUserID, mockServiceUserID)
@@ -652,9 +679,11 @@ func TestUser_DeleteUser_SelfDelete(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
Store: store,
eventStore: &activity.InMemoryEventStore{},
permissionsManager: permissionsMananagerMock,
}
err = am.DeleteUser(context.Background(), mockAccountID, mockUserID, mockUserID)
@@ -704,10 +733,12 @@ func TestUser_DeleteUser_regularUser(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
integratedPeerValidator: MocIntegratedValidator{},
permissionsManager: permissionsMananagerMock,
}
testCases := []struct {
@@ -812,10 +843,12 @@ func TestUser_DeleteUser_RegularUsers(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
integratedPeerValidator: MocIntegratedValidator{},
permissionsManager: permissionsMananagerMock,
}
testCases := []struct {
@@ -921,9 +954,11 @@ func TestDefaultAccountManager_GetUser(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
Store: store,
eventStore: &activity.InMemoryEventStore{},
permissionsManager: permissionsMananagerMock,
}
claims := nbcontext.UserAuth{
@@ -957,9 +992,11 @@ func TestDefaultAccountManager_ListUsers(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
Store: store,
eventStore: &activity.InMemoryEventStore{},
permissionsManager: permissionsMananagerMock,
}
users, err := am.ListUsers(context.Background(), mockAccountID)
@@ -1044,9 +1081,11 @@ func TestDefaultAccountManager_ListUsers_DashboardPermissions(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
Store: store,
eventStore: &activity.InMemoryEventStore{},
permissionsManager: permissionsMananagerMock,
}
users, err := am.ListUsers(context.Background(), mockAccountID)
@@ -1087,11 +1126,13 @@ func TestDefaultAccountManager_ExternalCache(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
idpManager: &idp.GoogleWorkspaceManager{}, // empty manager
cacheLoading: map[string]chan struct{}{},
Store: store,
eventStore: &activity.InMemoryEventStore{},
idpManager: &idp.GoogleWorkspaceManager{}, // empty manager
cacheLoading: map[string]chan struct{}{},
permissionsManager: permissionsMananagerMock,
}
cacheStore, err := nbcache.NewStore(context.Background(), nbcache.DefaultIDPCacheExpirationMax, nbcache.DefaultIDPCacheCleanupInterval)
@@ -1148,9 +1189,11 @@ func TestUser_GetUsersFromAccount_ForAdmin(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
Store: store,
eventStore: &activity.InMemoryEventStore{},
permissionsManager: permissionsMananagerMock,
}
users, err := am.GetUsersFromAccount(context.Background(), mockAccountID, mockUserID)
@@ -1180,9 +1223,11 @@ func TestUser_GetUsersFromAccount_ForUser(t *testing.T) {
t.Fatalf("Error when saving account: %s", err)
}
permissionsMananagerMock := permissions.NewManagerMock()
am := DefaultAccountManager{
Store: store,
eventStore: &activity.InMemoryEventStore{},
Store: store,
eventStore: &activity.InMemoryEventStore{},
permissionsManager: permissionsMananagerMock,
}
users, err := am.GetUsersFromAccount(context.Background(), mockAccountID, mockServiceUserID)