diff --git a/client/cmd/testutil_test.go b/client/cmd/testutil_test.go index 4bda33e65..c575a0c0c 100644 --- a/client/cmd/testutil_test.go +++ b/client/cmd/testutil_test.go @@ -12,6 +12,7 @@ import ( "google.golang.org/grpc" "github.com/netbirdio/management-integrations/integrations" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/internals/controllers/network_map/controller" "github.com/netbirdio/netbird/management/internals/controllers/network_map/update_channel" @@ -27,7 +28,6 @@ import ( "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/groups" "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" diff --git a/client/internal/engine_test.go b/client/internal/engine_test.go index 012c8ad6e..1ee0ec235 100644 --- a/client/internal/engine_test.go +++ b/client/internal/engine_test.go @@ -25,6 +25,7 @@ import ( "google.golang.org/grpc/keepalive" "github.com/netbirdio/netbird/client/internal/stdnet" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/server/job" "github.com/netbirdio/management-integrations/integrations" @@ -56,7 +57,6 @@ 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" diff --git a/client/server/server_test.go b/client/server/server_test.go index 82079c531..8e050081c 100644 --- a/client/server/server_test.go +++ b/client/server/server_test.go @@ -14,6 +14,7 @@ import ( "go.opentelemetry.io/otel" "github.com/netbirdio/management-integrations/integrations" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/internals/controllers/network_map/controller" "github.com/netbirdio/netbird/management/internals/controllers/network_map/update_channel" @@ -37,7 +38,6 @@ 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" diff --git a/management/internals/modules/peers/manager.go b/management/internals/modules/peers/manager.go index 2f796a5d1..6accb7105 100644 --- a/management/internals/modules/peers/manager.go +++ b/management/internals/modules/peers/manager.go @@ -16,9 +16,6 @@ import ( "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/integrations/integrated_validator" "github.com/netbirdio/netbird/management/server/peer" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/shared/management/status" ) @@ -38,17 +35,15 @@ type Manager interface { type managerImpl struct { store store.Store - permissionsManager permissions.Manager integratedPeerValidator integrated_validator.IntegratedValidator accountManager account.Manager networkMapController network_map.Controller } -func NewManager(store store.Store, permissionsManager permissions.Manager) Manager { +func NewManager(store store.Store) Manager { return &managerImpl{ - store: store, - permissionsManager: permissionsManager, + store: store, } } @@ -65,28 +60,10 @@ func (m *managerImpl) SetAccountManager(accountManager account.Manager) { } func (m *managerImpl) GetPeer(ctx context.Context, accountID, userID, peerID string) (*peer.Peer, error) { - allowed, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Peers, operations.Read) - if err != nil { - return nil, fmt.Errorf("failed to validate user permissions: %w", err) - } - - if !allowed { - return nil, status.NewPermissionDeniedError() - } - return m.store.GetPeerByID(ctx, store.LockingStrengthNone, accountID, peerID) } func (m *managerImpl) GetAllPeers(ctx context.Context, accountID, userID string) ([]*peer.Peer, error) { - allowed, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Peers, operations.Read) - if err != nil { - return nil, fmt.Errorf("failed to validate user permissions: %w", err) - } - - if !allowed { - return m.store.GetUserPeers(ctx, store.LockingStrengthNone, accountID, userID) - } - return m.store.GetAccountPeers(ctx, store.LockingStrengthNone, accountID, "", "") } diff --git a/management/server/permissions/manager.go b/management/internals/modules/permissions/manager.go similarity index 95% rename from management/server/permissions/manager.go rename to management/internals/modules/permissions/manager.go index 914b88324..c7bb549d9 100644 --- a/management/server/permissions/manager.go +++ b/management/internals/modules/permissions/manager.go @@ -8,12 +8,12 @@ import ( log "github.com/sirupsen/logrus" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" + "github.com/netbirdio/netbird/management/internals/modules/permissions/roles" "github.com/netbirdio/netbird/management/server/account" "github.com/netbirdio/netbird/management/server/activity" nbcontext "github.com/netbirdio/netbird/management/server/context" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" - "github.com/netbirdio/netbird/management/server/permissions/roles" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/auth" diff --git a/management/server/permissions/manager_mock.go b/management/internals/modules/permissions/manager_mock.go similarity index 91% rename from management/server/permissions/manager_mock.go rename to management/internals/modules/permissions/manager_mock.go index 82e728fd4..1373a24af 100644 --- a/management/server/permissions/manager_mock.go +++ b/management/internals/modules/permissions/manager_mock.go @@ -5,17 +5,18 @@ package permissions import ( - context "context" - http "net/http" - reflect "reflect" + "context" + "net/http" + "reflect" - gomock "github.com/golang/mock/gomock" - account "github.com/netbirdio/netbird/management/server/account" - modules "github.com/netbirdio/netbird/management/server/permissions/modules" - operations "github.com/netbirdio/netbird/management/server/permissions/operations" - roles "github.com/netbirdio/netbird/management/server/permissions/roles" - types "github.com/netbirdio/netbird/management/server/types" - auth "github.com/netbirdio/netbird/shared/auth" + "github.com/golang/mock/gomock" + + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" + "github.com/netbirdio/netbird/management/internals/modules/permissions/roles" + "github.com/netbirdio/netbird/management/server/account" + "github.com/netbirdio/netbird/management/server/types" + "github.com/netbirdio/netbird/shared/auth" ) // MockManager is a mock of Manager interface. diff --git a/management/server/permissions/modules/module.go b/management/internals/modules/permissions/modules/module.go similarity index 100% rename from management/server/permissions/modules/module.go rename to management/internals/modules/permissions/modules/module.go diff --git a/management/server/permissions/operations/operation.go b/management/internals/modules/permissions/operations/operation.go similarity index 100% rename from management/server/permissions/operations/operation.go rename to management/internals/modules/permissions/operations/operation.go diff --git a/management/server/permissions/roles/admin.go b/management/internals/modules/permissions/roles/admin.go similarity index 74% rename from management/server/permissions/roles/admin.go rename to management/internals/modules/permissions/roles/admin.go index af3a81297..bcfda9e59 100644 --- a/management/server/permissions/roles/admin.go +++ b/management/internals/modules/permissions/roles/admin.go @@ -1,8 +1,8 @@ package roles import ( - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/types" ) diff --git a/management/server/permissions/roles/auditor.go b/management/internals/modules/permissions/roles/auditor.go similarity index 78% rename from management/server/permissions/roles/auditor.go rename to management/internals/modules/permissions/roles/auditor.go index 33d8651f4..7d762e2fa 100644 --- a/management/server/permissions/roles/auditor.go +++ b/management/internals/modules/permissions/roles/auditor.go @@ -1,7 +1,7 @@ package roles import ( - "github.com/netbirdio/netbird/management/server/permissions/operations" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/types" ) diff --git a/management/server/permissions/roles/network_admin.go b/management/internals/modules/permissions/roles/network_admin.go similarity index 93% rename from management/server/permissions/roles/network_admin.go rename to management/internals/modules/permissions/roles/network_admin.go index 8f69d46ad..23afb6181 100644 --- a/management/server/permissions/roles/network_admin.go +++ b/management/internals/modules/permissions/roles/network_admin.go @@ -1,8 +1,8 @@ package roles import ( - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/types" ) diff --git a/management/server/permissions/roles/owner.go b/management/internals/modules/permissions/roles/owner.go similarity index 78% rename from management/server/permissions/roles/owner.go rename to management/internals/modules/permissions/roles/owner.go index 668470e47..d1eb4c70c 100644 --- a/management/server/permissions/roles/owner.go +++ b/management/internals/modules/permissions/roles/owner.go @@ -1,7 +1,7 @@ package roles import ( - "github.com/netbirdio/netbird/management/server/permissions/operations" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/types" ) diff --git a/management/server/permissions/roles/role_permissions.go b/management/internals/modules/permissions/roles/role_permissions.go similarity index 76% rename from management/server/permissions/roles/role_permissions.go rename to management/internals/modules/permissions/roles/role_permissions.go index 754e568f5..296acf04a 100644 --- a/management/server/permissions/roles/role_permissions.go +++ b/management/internals/modules/permissions/roles/role_permissions.go @@ -1,8 +1,8 @@ package roles import ( - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/types" ) diff --git a/management/server/permissions/roles/user.go b/management/internals/modules/permissions/roles/user.go similarity index 78% rename from management/server/permissions/roles/user.go rename to management/internals/modules/permissions/roles/user.go index bb3df0aea..1df61ff6e 100644 --- a/management/server/permissions/roles/user.go +++ b/management/internals/modules/permissions/roles/user.go @@ -1,7 +1,7 @@ package roles import ( - "github.com/netbirdio/netbird/management/server/permissions/operations" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/types" ) diff --git a/management/internals/modules/reverseproxy/accesslogs/manager/api.go b/management/internals/modules/reverseproxy/accesslogs/manager/api.go index 4859bc147..4710fe680 100644 --- a/management/internals/modules/reverseproxy/accesslogs/manager/api.go +++ b/management/internals/modules/reverseproxy/accesslogs/manager/api.go @@ -5,10 +5,10 @@ import ( "github.com/gorilla/mux" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/accesslogs" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" "github.com/netbirdio/netbird/shared/management/http/util" diff --git a/management/internals/modules/reverseproxy/accesslogs/manager/manager.go b/management/internals/modules/reverseproxy/accesslogs/manager/manager.go index e7fba7bed..1d810d412 100644 --- a/management/internals/modules/reverseproxy/accesslogs/manager/manager.go +++ b/management/internals/modules/reverseproxy/accesslogs/manager/manager.go @@ -9,25 +9,19 @@ import ( "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/accesslogs" "github.com/netbirdio/netbird/management/server/geolocation" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" - "github.com/netbirdio/netbird/shared/management/status" ) type managerImpl struct { - store store.Store - permissionsManager permissions.Manager - geo geolocation.Geolocation - cleanupCancel context.CancelFunc + store store.Store + geo geolocation.Geolocation + cleanupCancel context.CancelFunc } -func NewManager(store store.Store, permissionsManager permissions.Manager, geo geolocation.Geolocation) accesslogs.Manager { +func NewManager(store store.Store, geo geolocation.Geolocation) accesslogs.Manager { return &managerImpl{ - store: store, - permissionsManager: permissionsManager, - geo: geo, + store: store, + geo: geo, } } @@ -60,14 +54,6 @@ func (m *managerImpl) SaveAccessLog(ctx context.Context, logEntry *accesslogs.Ac // GetAllAccessLogs retrieves access logs for an account with pagination and filtering func (m *managerImpl) GetAllAccessLogs(ctx context.Context, accountID, userID string, filter *accesslogs.AccessLogFilter) ([]*accesslogs.AccessLogEntry, int64, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Services, operations.Read) - if err != nil { - return nil, 0, status.NewPermissionValidationError(err) - } - if !ok { - return nil, 0, status.NewPermissionDeniedError() - } - if err := m.resolveUserFilters(ctx, accountID, filter); err != nil { log.WithContext(ctx).Warnf("failed to resolve user filters: %v", err) } diff --git a/management/internals/modules/reverseproxy/domain/manager/api.go b/management/internals/modules/reverseproxy/domain/manager/api.go index 63bc79a64..b9592ca18 100644 --- a/management/internals/modules/reverseproxy/domain/manager/api.go +++ b/management/internals/modules/reverseproxy/domain/manager/api.go @@ -6,10 +6,10 @@ import ( "github.com/gorilla/mux" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" "github.com/netbirdio/netbird/shared/management/http/util" diff --git a/management/internals/modules/reverseproxy/domain/manager/manager.go b/management/internals/modules/reverseproxy/domain/manager/manager.go index 12dd051fd..bb59ff1c4 100644 --- a/management/internals/modules/reverseproxy/domain/manager/manager.go +++ b/management/internals/modules/reverseproxy/domain/manager/manager.go @@ -9,11 +9,7 @@ import ( log "github.com/sirupsen/logrus" "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/types" - "github.com/netbirdio/netbird/shared/management/status" ) type store interface { @@ -32,32 +28,22 @@ type proxyManager interface { } type Manager struct { - store store - validator domain.Validator - proxyManager proxyManager - permissionsManager permissions.Manager + store store + validator domain.Validator + proxyManager proxyManager } -func NewManager(store store, proxyMgr proxyManager, permissionsManager permissions.Manager) Manager { +func NewManager(store store, proxyMgr proxyManager) Manager { return Manager{ store: store, proxyManager: proxyMgr, validator: domain.Validator{ Resolver: net.DefaultResolver, }, - permissionsManager: permissionsManager, } } func (m Manager) GetDomains(ctx context.Context, accountID, userID string) ([]*domain.Domain, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Services, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - domains, err := m.store.ListCustomDomains(ctx, accountID) if err != nil { return nil, fmt.Errorf("list custom domains: %w", err) @@ -102,14 +88,6 @@ func (m Manager) GetDomains(ctx context.Context, accountID, userID string) ([]*d } func (m Manager) CreateDomain(ctx context.Context, accountID, userID, domainName, targetCluster string) (*domain.Domain, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Services, operations.Create) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - // Verify the target cluster is in the available clusters allowList, err := m.proxyManager.GetActiveClusterAddresses(ctx) if err != nil { @@ -140,14 +118,6 @@ func (m Manager) CreateDomain(ctx context.Context, accountID, userID, domainName } func (m Manager) DeleteDomain(ctx context.Context, accountID, userID, domainID string) error { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Services, operations.Delete) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !ok { - return status.NewPermissionDeniedError() - } - if err := m.store.DeleteCustomDomain(ctx, accountID, domainID); err != nil { // TODO: check for "no records" type error. Because that is a success condition. return fmt.Errorf("delete domain from store: %w", err) @@ -156,21 +126,6 @@ func (m Manager) DeleteDomain(ctx context.Context, accountID, userID, domainID s } func (m Manager) ValidateDomain(ctx context.Context, accountID, userID, domainID string) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Services, operations.Create) - if err != nil { - log.WithFields(log.Fields{ - "accountID": accountID, - "domainID": domainID, - }).WithError(err).Error("validate domain") - return - } - if !ok { - log.WithFields(log.Fields{ - "accountID": accountID, - "domainID": domainID, - }).WithError(err).Error("validate domain") - } - log.WithFields(log.Fields{ "accountID": accountID, "domainID": domainID, diff --git a/management/internals/modules/reverseproxy/service/manager/api.go b/management/internals/modules/reverseproxy/service/manager/api.go index eb1587110..daa7b5a2a 100644 --- a/management/internals/modules/reverseproxy/service/manager/api.go +++ b/management/internals/modules/reverseproxy/service/manager/api.go @@ -6,14 +6,13 @@ import ( "github.com/gorilla/mux" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/accesslogs" accesslogsmanager "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/accesslogs/manager" domainmanager "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain/manager" rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service" - nbcontext "github.com/netbirdio/netbird/management/server/context" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" "github.com/netbirdio/netbird/shared/management/http/util" @@ -65,6 +64,7 @@ func (h *handler) createService(w http.ResponseWriter, r *http.Request, userAuth } service := new(rpservice.Service) + var err error if err = service.FromAPIRequest(&req, userAuth.AccountId); err != nil { util.WriteError(r.Context(), status.Errorf(status.InvalidArgument, "%s", err.Error()), w) return @@ -115,6 +115,7 @@ func (h *handler) updateService(w http.ResponseWriter, r *http.Request, userAuth service := new(rpservice.Service) service.ID = serviceID + var err error if err = service.FromAPIRequest(&req, userAuth.AccountId); err != nil { util.WriteError(r.Context(), status.Errorf(status.InvalidArgument, "%s", err.Error()), w) return diff --git a/management/internals/modules/reverseproxy/service/manager/manager.go b/management/internals/modules/reverseproxy/service/manager/manager.go index b5e643799..19200d2db 100644 --- a/management/internals/modules/reverseproxy/service/manager/manager.go +++ b/management/internals/modules/reverseproxy/service/manager/manager.go @@ -16,9 +16,6 @@ import ( "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/sessionkey" "github.com/netbirdio/netbird/management/server/account" "github.com/netbirdio/netbird/management/server/activity" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/shared/management/status" ) @@ -32,22 +29,20 @@ type ClusterDeriver interface { } type Manager struct { - store store.Store - accountManager account.Manager - permissionsManager permissions.Manager - proxyController proxy.Controller - clusterDeriver ClusterDeriver - exposeReaper *exposeReaper + store store.Store + accountManager account.Manager + proxyController proxy.Controller + clusterDeriver ClusterDeriver + exposeReaper *exposeReaper } // NewManager creates a new service manager. -func NewManager(store store.Store, accountManager account.Manager, permissionsManager permissions.Manager, proxyController proxy.Controller, clusterDeriver ClusterDeriver) *Manager { +func NewManager(store store.Store, accountManager account.Manager, proxyController proxy.Controller, clusterDeriver ClusterDeriver) *Manager { mgr := &Manager{ - store: store, - accountManager: accountManager, - permissionsManager: permissionsManager, - proxyController: proxyController, - clusterDeriver: clusterDeriver, + store: store, + accountManager: accountManager, + proxyController: proxyController, + clusterDeriver: clusterDeriver, } mgr.exposeReaper = &exposeReaper{manager: mgr} return mgr @@ -59,14 +54,6 @@ func (m *Manager) StartExposeReaper(ctx context.Context) { } func (m *Manager) GetAllServices(ctx context.Context, accountID, userID string) ([]*service.Service, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Services, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - services, err := m.store.GetAccountServices(ctx, store.LockingStrengthNone, accountID) if err != nil { return nil, fmt.Errorf("failed to get services: %w", err) @@ -119,14 +106,6 @@ func (m *Manager) replaceHostByLookup(ctx context.Context, accountID string, s * } func (m *Manager) GetService(ctx context.Context, accountID, userID, serviceID string) (*service.Service, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Services, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - service, err := m.store.GetServiceByID(ctx, store.LockingStrengthNone, accountID, serviceID) if err != nil { return nil, fmt.Errorf("failed to get service: %w", err) @@ -140,14 +119,6 @@ func (m *Manager) GetService(ctx context.Context, accountID, userID, serviceID s } func (m *Manager) CreateService(ctx context.Context, accountID, userID string, s *service.Service) (*service.Service, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Services, operations.Create) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - if err := m.initializeServiceForCreate(ctx, accountID, s); err != nil { return nil, err } @@ -158,7 +129,7 @@ func (m *Manager) CreateService(ctx context.Context, accountID, userID string, s m.accountManager.StoreEvent(ctx, userID, s.ID, accountID, activity.ServiceCreated, s.EventMeta()) - err = m.replaceHostByLookup(ctx, accountID, s) + err := m.replaceHostByLookup(ctx, accountID, s) if err != nil { return nil, fmt.Errorf("failed to replace host by lookup for service %s: %w", s.ID, err) } @@ -278,14 +249,6 @@ func (m *Manager) checkDomainAvailable(ctx context.Context, transaction store.St } func (m *Manager) UpdateService(ctx context.Context, accountID, userID string, service *service.Service) (*service.Service, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Services, operations.Update) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - if err := service.Auth.HashSecrets(); err != nil { return nil, fmt.Errorf("hash secrets: %w", err) } @@ -428,16 +391,8 @@ func validateTargetReferences(ctx context.Context, transaction store.Store, acco } func (m *Manager) DeleteService(ctx context.Context, accountID, userID, serviceID string) error { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Services, operations.Delete) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !ok { - return status.NewPermissionDeniedError() - } - var s *service.Service - err = m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + err := m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error { var err error s, err = transaction.GetServiceByID(ctx, store.LockingStrengthUpdate, accountID, serviceID) if err != nil { @@ -468,16 +423,8 @@ func (m *Manager) DeleteService(ctx context.Context, accountID, userID, serviceI } func (m *Manager) DeleteAllServices(ctx context.Context, accountID, userID string) error { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Services, operations.Delete) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !ok { - return status.NewPermissionDeniedError() - } - var services []*service.Service - err = m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + err := m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error { var err error services, err = transaction.GetAccountServices(ctx, store.LockingStrengthUpdate, accountID) if err != nil { diff --git a/management/internals/modules/reverseproxy/service/manager/manager_test.go b/management/internals/modules/reverseproxy/service/manager/manager_test.go index 196eead22..31c77a05f 100644 --- a/management/internals/modules/reverseproxy/service/manager/manager_test.go +++ b/management/internals/modules/reverseproxy/service/manager/manager_test.go @@ -12,6 +12,9 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/metric/noop" + permissions2 "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy" proxymanager "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy/manager" rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service" @@ -20,9 +23,6 @@ import ( "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/mock_server" nbpeer "github.com/netbirdio/netbird/management/server/peer" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/management/status" @@ -693,8 +693,6 @@ func setupIntegrationTest(t *testing.T) (*Manager, store.Store) { err = testStore.AddPeerToGroup(ctx, testAccountID, testPeerID, testGroupID) require.NoError(t, err) - permsMgr := permissions.NewManager(testStore) - accountMgr := &mock_server.MockAccountManager{ StoreEventFunc: func(_ context.Context, _, _, _ string, _ activity.ActivityDescriber, _ map[string]any) {}, UpdateAccountPeersFunc: func(_ context.Context, _ string) {}, @@ -712,10 +710,9 @@ func setupIntegrationTest(t *testing.T) (*Manager, store.Store) { require.NoError(t, err) mgr := &Manager{ - store: testStore, - accountManager: accountMgr, - permissionsManager: permsMgr, - proxyController: proxyController, + store: testStore, + accountManager: accountMgr, + proxyController: proxyController, clusterDeriver: &testClusterDeriver{ domains: []string{"test.netbird.io"}, }, @@ -1131,7 +1128,6 @@ func TestDeleteService_DeletesTargets(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - mockPerms := permissions.NewMockManager(ctrl) mockAcct := account.NewMockManager(ctrl) tokenStore, err := nbgrpc.NewOneTimeTokenStore(ctx, 1*time.Hour, 10*time.Minute, 100) @@ -1143,10 +1139,9 @@ func TestDeleteService_DeletesTargets(t *testing.T) { require.NoError(t, err) mgr := &Manager{ - store: sqlStore, - permissionsManager: mockPerms, - accountManager: mockAcct, - proxyController: proxyController, + store: sqlStore, + accountManager: mockAcct, + proxyController: proxyController, } service := &rpservice.Service{ @@ -1169,9 +1164,6 @@ func TestDeleteService_DeletesTargets(t *testing.T) { require.NoError(t, err) require.Len(t, retrievedService.Targets, 3, "Service should have 3 targets before deletion") - mockPerms.EXPECT(). - ValidateUserPermissions(ctx, accountID, userID, modules.Services, operations.Delete). - Return(true, nil) mockAcct.EXPECT(). StoreEvent(ctx, userID, service.ID, accountID, activity.ServiceDeleted, gomock.Any()) mockAcct.EXPECT(). diff --git a/management/internals/modules/zones/manager/api.go b/management/internals/modules/zones/manager/api.go index d5a894eb5..0fddb45a9 100644 --- a/management/internals/modules/zones/manager/api.go +++ b/management/internals/modules/zones/manager/api.go @@ -6,10 +6,10 @@ import ( "github.com/gorilla/mux" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/internals/modules/zones" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" "github.com/netbirdio/netbird/shared/management/http/util" diff --git a/management/internals/modules/zones/manager/manager.go b/management/internals/modules/zones/manager/manager.go index 8548dd48c..6b8f5fbb9 100644 --- a/management/internals/modules/zones/manager/manager.go +++ b/management/internals/modules/zones/manager/manager.go @@ -7,62 +7,34 @@ import ( "github.com/netbirdio/netbird/management/internals/modules/zones" "github.com/netbirdio/netbird/management/server/account" "github.com/netbirdio/netbird/management/server/activity" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/shared/management/status" ) type managerImpl struct { - store store.Store - accountManager account.Manager - permissionsManager permissions.Manager - dnsDomain string + store store.Store + accountManager account.Manager + dnsDomain string } -func NewManager(store store.Store, accountManager account.Manager, permissionsManager permissions.Manager, dnsDomain string) zones.Manager { +func NewManager(store store.Store, accountManager account.Manager, dnsDomain string) zones.Manager { return &managerImpl{ - store: store, - accountManager: accountManager, - permissionsManager: permissionsManager, - dnsDomain: dnsDomain, + store: store, + accountManager: accountManager, + dnsDomain: dnsDomain, } } func (m *managerImpl) GetAllZones(ctx context.Context, accountID, userID string) ([]*zones.Zone, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Dns, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - return m.store.GetAccountZones(ctx, store.LockingStrengthNone, accountID) } func (m *managerImpl) GetZone(ctx context.Context, accountID, userID, zoneID string) (*zones.Zone, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Dns, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - return m.store.GetZoneByID(ctx, store.LockingStrengthNone, accountID, zoneID) } func (m *managerImpl) CreateZone(ctx context.Context, accountID, userID string, zone *zones.Zone) (*zones.Zone, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Dns, operations.Create) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - + var err error if err = m.validateZoneDomainConflict(ctx, accountID, zone.Domain); err != nil { return nil, err } @@ -102,14 +74,6 @@ func (m *managerImpl) CreateZone(ctx context.Context, accountID, userID string, } func (m *managerImpl) UpdateZone(ctx context.Context, accountID, userID string, updatedZone *zones.Zone) (*zones.Zone, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Dns, operations.Update) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - zone, err := m.store.GetZoneByID(ctx, store.LockingStrengthUpdate, accountID, updatedZone.ID) if err != nil { return nil, fmt.Errorf("failed to get zone: %w", err) @@ -150,14 +114,6 @@ func (m *managerImpl) UpdateZone(ctx context.Context, accountID, userID string, } func (m *managerImpl) DeleteZone(ctx context.Context, accountID, userID, zoneID string) error { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Dns, operations.Delete) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !ok { - return status.NewPermissionDeniedError() - } - zone, err := m.store.GetZoneByID(ctx, store.LockingStrengthUpdate, accountID, zoneID) if err != nil { return fmt.Errorf("failed to get zone: %w", err) diff --git a/management/internals/modules/zones/manager/manager_test.go b/management/internals/modules/zones/manager/manager_test.go index b45ec7874..df39f8a8d 100644 --- a/management/internals/modules/zones/manager/manager_test.go +++ b/management/internals/modules/zones/manager/manager_test.go @@ -13,9 +13,6 @@ import ( "github.com/netbirdio/netbird/management/internals/modules/zones/records" "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/mock_server" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/management/status" @@ -29,7 +26,7 @@ const ( testDNSDomain = "netbird.selfhosted" ) -func setupTest(t *testing.T) (*managerImpl, store.Store, *mock_server.MockAccountManager, *permissions.MockManager, *gomock.Controller, func()) { +func setupTest(t *testing.T) (*managerImpl, store.Store, *mock_server.MockAccountManager, *gomock.Controller, func()) { t.Helper() ctx := context.Background() @@ -49,23 +46,17 @@ func setupTest(t *testing.T) (*managerImpl, store.Store, *mock_server.MockAccoun ctrl := gomock.NewController(t) mockAccountManager := &mock_server.MockAccountManager{} - mockPermissionsManager := permissions.NewMockManager(ctrl) - manager := &managerImpl{ - store: testStore, - accountManager: mockAccountManager, - permissionsManager: mockPermissionsManager, - dnsDomain: testDNSDomain, - } + manager := NewManager(testStore, mockAccountManager, testDNSDomain).(*managerImpl) - return manager, testStore, mockAccountManager, mockPermissionsManager, ctrl, cleanup + return manager, testStore, mockAccountManager, ctrl, cleanup } func TestManagerImpl_GetAllZones(t *testing.T) { ctx := context.Background() t.Run("success", func(t *testing.T) { - manager, testStore, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, testStore, _, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -77,10 +68,6 @@ func TestManagerImpl_GetAllZones(t *testing.T) { err = testStore.CreateZone(ctx, zone2) require.NoError(t, err) - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Read). - Return(true, nil) - result, err := manager.GetAllZones(ctx, testAccountID, testUserID) require.NoError(t, err) assert.Len(t, result, 2) @@ -88,43 +75,13 @@ func TestManagerImpl_GetAllZones(t *testing.T) { assert.Equal(t, zone2.ID, result[1].ID) }) - t.Run("permission denied", func(t *testing.T) { - manager, _, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) - defer cleanup() - defer ctrl.Finish() - - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Read). - Return(false, nil) - - result, err := manager.GetAllZones(ctx, testAccountID, testUserID) - require.Error(t, err) - assert.Nil(t, result) - s, ok := status.FromError(err) - assert.True(t, ok) - assert.Equal(t, status.PermissionDenied, s.Type()) - }) - - t.Run("permission validation error", func(t *testing.T) { - manager, _, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) - defer cleanup() - defer ctrl.Finish() - - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Read). - Return(false, status.Errorf(status.Internal, "permission check failed")) - - result, err := manager.GetAllZones(ctx, testAccountID, testUserID) - require.Error(t, err) - assert.Nil(t, result) - }) } func TestManagerImpl_GetZone(t *testing.T) { ctx := context.Background() t.Run("success", func(t *testing.T) { - manager, testStore, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, testStore, _, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -132,10 +89,6 @@ func TestManagerImpl_GetZone(t *testing.T) { err := testStore.CreateZone(ctx, zone) require.NoError(t, err) - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Read). - Return(true, nil) - result, err := manager.GetZone(ctx, testAccountID, testUserID, zone.ID) require.NoError(t, err) assert.Equal(t, zone.ID, result.ID) @@ -143,29 +96,13 @@ func TestManagerImpl_GetZone(t *testing.T) { assert.Equal(t, zone.Domain, result.Domain) }) - t.Run("permission denied", func(t *testing.T) { - manager, _, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) - defer cleanup() - defer ctrl.Finish() - - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Read). - Return(false, nil) - - result, err := manager.GetZone(ctx, testAccountID, testUserID, testZoneID) - require.Error(t, err) - assert.Nil(t, result) - s, ok := status.FromError(err) - assert.True(t, ok) - assert.Equal(t, status.PermissionDenied, s.Type()) - }) } func TestManagerImpl_CreateZone(t *testing.T) { ctx := context.Background() t.Run("success", func(t *testing.T) { - manager, _, mockAccountManager, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, _, mockAccountManager, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -177,10 +114,6 @@ func TestManagerImpl_CreateZone(t *testing.T) { DistributionGroups: []string{testGroupID}, } - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Create). - Return(true, nil) - mockAccountManager.StoreEventFunc = func(ctx context.Context, initiatorID, targetID, accountID string, activityID activity.ActivityDescriber, meta map[string]any) { assert.Equal(t, testUserID, initiatorID) assert.Equal(t, testAccountID, accountID) @@ -199,31 +132,8 @@ func TestManagerImpl_CreateZone(t *testing.T) { assert.Equal(t, inputZone.DistributionGroups, result.DistributionGroups) }) - t.Run("permission denied", func(t *testing.T) { - manager, _, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) - defer cleanup() - defer ctrl.Finish() - - inputZone := &zones.Zone{ - Name: "New Zone", - Domain: "new.example.com", - DistributionGroups: []string{testGroupID}, - } - - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Create). - Return(false, nil) - - result, err := manager.CreateZone(ctx, testAccountID, testUserID, inputZone) - require.Error(t, err) - assert.Nil(t, result) - s, ok := status.FromError(err) - assert.True(t, ok) - assert.Equal(t, status.PermissionDenied, s.Type()) - }) - t.Run("invalid group", func(t *testing.T) { - manager, _, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, _, _, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -233,17 +143,13 @@ func TestManagerImpl_CreateZone(t *testing.T) { DistributionGroups: []string{"invalid-group"}, } - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Create). - Return(true, nil) - result, err := manager.CreateZone(ctx, testAccountID, testUserID, inputZone) require.Error(t, err) assert.Nil(t, result) }) t.Run("duplicate domain", func(t *testing.T) { - manager, testStore, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, testStore, _, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -259,10 +165,6 @@ func TestManagerImpl_CreateZone(t *testing.T) { DistributionGroups: []string{testGroupID}, } - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Create). - Return(true, nil) - result, err := manager.CreateZone(ctx, testAccountID, testUserID, inputZone) require.Error(t, err) assert.Nil(t, result) @@ -273,7 +175,7 @@ func TestManagerImpl_CreateZone(t *testing.T) { }) t.Run("peer DNS domain conflict", func(t *testing.T) { - manager, testStore, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, testStore, _, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -291,10 +193,6 @@ func TestManagerImpl_CreateZone(t *testing.T) { DistributionGroups: []string{testGroupID}, } - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Create). - Return(true, nil) - result, err := manager.CreateZone(ctx, testAccountID, testUserID, inputZone) require.Error(t, err) assert.Nil(t, result) @@ -305,7 +203,7 @@ func TestManagerImpl_CreateZone(t *testing.T) { }) t.Run("default DNS domain conflict", func(t *testing.T) { - manager, _, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, _, _, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -317,10 +215,6 @@ func TestManagerImpl_CreateZone(t *testing.T) { DistributionGroups: []string{testGroupID}, } - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Create). - Return(true, nil) - result, err := manager.CreateZone(ctx, testAccountID, testUserID, inputZone) require.Error(t, err) assert.Nil(t, result) @@ -335,7 +229,7 @@ func TestManagerImpl_UpdateZone(t *testing.T) { ctx := context.Background() t.Run("success", func(t *testing.T) { - manager, testStore, mockAccountManager, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, testStore, mockAccountManager, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -352,10 +246,6 @@ func TestManagerImpl_UpdateZone(t *testing.T) { DistributionGroups: []string{testGroupID}, } - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Update). - Return(true, nil) - storeEventCalled := false mockAccountManager.StoreEventFunc = func(ctx context.Context, initiatorID, targetID, accountID string, activityID activity.ActivityDescriber, meta map[string]any) { storeEventCalled = true @@ -375,7 +265,7 @@ func TestManagerImpl_UpdateZone(t *testing.T) { }) t.Run("domain change not allowed", func(t *testing.T) { - manager, testStore, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, testStore, _, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -392,10 +282,6 @@ func TestManagerImpl_UpdateZone(t *testing.T) { DistributionGroups: []string{testGroupID}, } - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Update). - Return(true, nil) - result, err := manager.UpdateZone(ctx, testAccountID, testUserID, updatedZone) require.Error(t, err) assert.Nil(t, result) @@ -405,31 +291,8 @@ func TestManagerImpl_UpdateZone(t *testing.T) { assert.Equal(t, status.InvalidArgument, s.Type()) }) - t.Run("permission denied", func(t *testing.T) { - manager, _, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) - defer cleanup() - defer ctrl.Finish() - - updatedZone := &zones.Zone{ - ID: testZoneID, - Name: "Updated Name", - Domain: "example.com", - } - - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Update). - Return(false, nil) - - result, err := manager.UpdateZone(ctx, testAccountID, testUserID, updatedZone) - require.Error(t, err) - assert.Nil(t, result) - s, ok := status.FromError(err) - assert.True(t, ok) - assert.Equal(t, status.PermissionDenied, s.Type()) - }) - t.Run("zone not found", func(t *testing.T) { - manager, _, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, _, _, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -439,10 +302,6 @@ func TestManagerImpl_UpdateZone(t *testing.T) { Domain: "example.com", } - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Update). - Return(true, nil) - result, err := manager.UpdateZone(ctx, testAccountID, testUserID, updatedZone) require.Error(t, err) assert.Nil(t, result) @@ -453,7 +312,7 @@ func TestManagerImpl_DeleteZone(t *testing.T) { ctx := context.Background() t.Run("success with records", func(t *testing.T) { - manager, testStore, mockAccountManager, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, testStore, mockAccountManager, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -469,10 +328,6 @@ func TestManagerImpl_DeleteZone(t *testing.T) { err = testStore.CreateDNSRecord(ctx, record2) require.NoError(t, err) - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Delete). - Return(true, nil) - storeEventCallCount := 0 mockAccountManager.StoreEventFunc = func(ctx context.Context, initiatorID, targetID, accountID string, activityID activity.ActivityDescriber, meta map[string]any) { storeEventCallCount++ @@ -493,7 +348,7 @@ func TestManagerImpl_DeleteZone(t *testing.T) { }) t.Run("success without records", func(t *testing.T) { - manager, testStore, mockAccountManager, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, testStore, mockAccountManager, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -501,10 +356,6 @@ func TestManagerImpl_DeleteZone(t *testing.T) { err := testStore.CreateZone(ctx, zone) require.NoError(t, err) - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Delete). - Return(true, nil) - storeEventCalled := false mockAccountManager.StoreEventFunc = func(ctx context.Context, initiatorID, targetID, accountID string, activityID activity.ActivityDescriber, meta map[string]any) { storeEventCalled = true @@ -522,31 +373,11 @@ func TestManagerImpl_DeleteZone(t *testing.T) { require.Error(t, err) }) - t.Run("permission denied", func(t *testing.T) { - manager, _, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) - defer cleanup() - defer ctrl.Finish() - - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Delete). - Return(false, nil) - - err := manager.DeleteZone(ctx, testAccountID, testUserID, testZoneID) - require.Error(t, err) - s, ok := status.FromError(err) - assert.True(t, ok) - assert.Equal(t, status.PermissionDenied, s.Type()) - }) - t.Run("zone not found", func(t *testing.T) { - manager, _, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, _, _, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Delete). - Return(true, nil) - err := manager.DeleteZone(ctx, testAccountID, testUserID, "non-existent-zone") require.Error(t, err) }) diff --git a/management/internals/modules/zones/records/manager/api.go b/management/internals/modules/zones/records/manager/api.go index ebf2b3ebd..aa746799b 100644 --- a/management/internals/modules/zones/records/manager/api.go +++ b/management/internals/modules/zones/records/manager/api.go @@ -6,10 +6,10 @@ import ( "github.com/gorilla/mux" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/internals/modules/zones/records" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" "github.com/netbirdio/netbird/shared/management/http/util" diff --git a/management/internals/modules/zones/records/manager/manager.go b/management/internals/modules/zones/records/manager/manager.go index 5374a2ef2..dad8a8a14 100644 --- a/management/internals/modules/zones/records/manager/manager.go +++ b/management/internals/modules/zones/records/manager/manager.go @@ -9,64 +9,36 @@ import ( "github.com/netbirdio/netbird/management/internals/modules/zones/records" "github.com/netbirdio/netbird/management/server/account" "github.com/netbirdio/netbird/management/server/activity" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/shared/management/status" ) type managerImpl struct { - store store.Store - accountManager account.Manager - permissionsManager permissions.Manager + store store.Store + accountManager account.Manager } -func NewManager(store store.Store, accountManager account.Manager, permissionsManager permissions.Manager) records.Manager { +func NewManager(store store.Store, accountManager account.Manager) records.Manager { return &managerImpl{ - store: store, - accountManager: accountManager, - permissionsManager: permissionsManager, + store: store, + accountManager: accountManager, } } func (m *managerImpl) GetAllRecords(ctx context.Context, accountID, userID, zoneID string) ([]*records.Record, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Dns, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - return m.store.GetZoneDNSRecords(ctx, store.LockingStrengthNone, accountID, zoneID) } func (m *managerImpl) GetRecord(ctx context.Context, accountID, userID, zoneID, recordID string) (*records.Record, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Dns, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - return m.store.GetDNSRecordByID(ctx, store.LockingStrengthNone, accountID, zoneID, recordID) } func (m *managerImpl) CreateRecord(ctx context.Context, accountID, userID, zoneID string, record *records.Record) (*records.Record, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Dns, operations.Create) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - var zone *zones.Zone record = records.NewRecord(accountID, zoneID, record.Name, record.Type, record.Content, record.TTL) - err = m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + err := m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + var err error zone, err = transaction.GetZoneByID(ctx, store.LockingStrengthUpdate, accountID, zoneID) if err != nil { return fmt.Errorf("failed to get zone: %w", err) @@ -101,18 +73,11 @@ func (m *managerImpl) CreateRecord(ctx context.Context, accountID, userID, zoneI } func (m *managerImpl) UpdateRecord(ctx context.Context, accountID, userID, zoneID string, updatedRecord *records.Record) (*records.Record, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Dns, operations.Update) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - var zone *zones.Zone var record *records.Record - err = m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + err := m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + var err error zone, err = transaction.GetZoneByID(ctx, store.LockingStrengthUpdate, accountID, zoneID) if err != nil { return fmt.Errorf("failed to get zone: %w", err) @@ -160,18 +125,11 @@ func (m *managerImpl) UpdateRecord(ctx context.Context, accountID, userID, zoneI } func (m *managerImpl) DeleteRecord(ctx context.Context, accountID, userID, zoneID, recordID string) error { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Dns, operations.Delete) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !ok { - return status.NewPermissionDeniedError() - } - var record *records.Record var zone *zones.Zone - err = m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + err := m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + var err error zone, err = transaction.GetZoneByID(ctx, store.LockingStrengthUpdate, accountID, zoneID) if err != nil { return fmt.Errorf("failed to get zone: %w", err) diff --git a/management/internals/modules/zones/records/manager/manager_test.go b/management/internals/modules/zones/records/manager/manager_test.go index 0a962e0f4..4edfaa090 100644 --- a/management/internals/modules/zones/records/manager/manager_test.go +++ b/management/internals/modules/zones/records/manager/manager_test.go @@ -12,9 +12,6 @@ import ( "github.com/netbirdio/netbird/management/internals/modules/zones/records" "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/mock_server" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/management/status" @@ -27,7 +24,7 @@ const ( testGroupID = "test-group-id" ) -func setupTest(t *testing.T) (*managerImpl, store.Store, *zones.Zone, *mock_server.MockAccountManager, *permissions.MockManager, *gomock.Controller, func()) { +func setupTest(t *testing.T) (*managerImpl, store.Store, *zones.Zone, *mock_server.MockAccountManager, *gomock.Controller, func()) { t.Helper() ctx := context.Background() @@ -51,22 +48,17 @@ func setupTest(t *testing.T) (*managerImpl, store.Store, *zones.Zone, *mock_serv ctrl := gomock.NewController(t) mockAccountManager := &mock_server.MockAccountManager{} - mockPermissionsManager := permissions.NewMockManager(ctrl) - manager := &managerImpl{ - store: testStore, - accountManager: mockAccountManager, - permissionsManager: mockPermissionsManager, - } + manager := NewManager(testStore, mockAccountManager).(*managerImpl) - return manager, testStore, zone, mockAccountManager, mockPermissionsManager, ctrl, cleanup + return manager, testStore, zone, mockAccountManager, ctrl, cleanup } func TestManagerImpl_GetAllRecords(t *testing.T) { ctx := context.Background() t.Run("success", func(t *testing.T) { - manager, testStore, zone, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, testStore, zone, _, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -78,10 +70,6 @@ func TestManagerImpl_GetAllRecords(t *testing.T) { err = testStore.CreateDNSRecord(ctx, record2) require.NoError(t, err) - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Read). - Return(true, nil) - result, err := manager.GetAllRecords(ctx, testAccountID, testUserID, zone.ID) require.NoError(t, err) assert.Len(t, result, 2) @@ -89,43 +77,13 @@ func TestManagerImpl_GetAllRecords(t *testing.T) { assert.Equal(t, record2.ID, result[1].ID) }) - t.Run("permission denied", func(t *testing.T) { - manager, _, zone, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) - defer cleanup() - defer ctrl.Finish() - - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Read). - Return(false, nil) - - result, err := manager.GetAllRecords(ctx, testAccountID, testUserID, zone.ID) - require.Error(t, err) - assert.Nil(t, result) - s, ok := status.FromError(err) - assert.True(t, ok) - assert.Equal(t, status.PermissionDenied, s.Type()) - }) - - t.Run("permission validation error", func(t *testing.T) { - manager, _, zone, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) - defer cleanup() - defer ctrl.Finish() - - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Read). - Return(false, status.Errorf(status.Internal, "permission check failed")) - - result, err := manager.GetAllRecords(ctx, testAccountID, testUserID, zone.ID) - require.Error(t, err) - assert.Nil(t, result) - }) } func TestManagerImpl_GetRecord(t *testing.T) { ctx := context.Background() t.Run("success", func(t *testing.T) { - manager, testStore, zone, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, testStore, zone, _, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -133,10 +91,6 @@ func TestManagerImpl_GetRecord(t *testing.T) { err := testStore.CreateDNSRecord(ctx, record) require.NoError(t, err) - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Read). - Return(true, nil) - result, err := manager.GetRecord(ctx, testAccountID, testUserID, zone.ID, record.ID) require.NoError(t, err) assert.Equal(t, record.ID, result.ID) @@ -146,29 +100,13 @@ func TestManagerImpl_GetRecord(t *testing.T) { assert.Equal(t, record.TTL, result.TTL) }) - t.Run("permission denied", func(t *testing.T) { - manager, _, zone, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) - defer cleanup() - defer ctrl.Finish() - - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Read). - Return(false, nil) - - result, err := manager.GetRecord(ctx, testAccountID, testUserID, zone.ID, testRecordID) - require.Error(t, err) - assert.Nil(t, result) - s, ok := status.FromError(err) - assert.True(t, ok) - assert.Equal(t, status.PermissionDenied, s.Type()) - }) } func TestManagerImpl_CreateRecord(t *testing.T) { ctx := context.Background() t.Run("success - A record", func(t *testing.T) { - manager, _, zone, mockAccountManager, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, _, zone, mockAccountManager, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -179,10 +117,6 @@ func TestManagerImpl_CreateRecord(t *testing.T) { TTL: 300, } - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Create). - Return(true, nil) - mockAccountManager.StoreEventFunc = func(ctx context.Context, initiatorID, targetID, accountID string, activityID activity.ActivityDescriber, meta map[string]any) { assert.Equal(t, testUserID, initiatorID) assert.Equal(t, testAccountID, accountID) @@ -202,7 +136,7 @@ func TestManagerImpl_CreateRecord(t *testing.T) { }) t.Run("success - AAAA record", func(t *testing.T) { - manager, _, zone, mockAccountManager, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, _, zone, mockAccountManager, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -213,10 +147,6 @@ func TestManagerImpl_CreateRecord(t *testing.T) { TTL: 600, } - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Create). - Return(true, nil) - mockAccountManager.StoreEventFunc = func(ctx context.Context, initiatorID, targetID, accountID string, activityID activity.ActivityDescriber, meta map[string]any) { assert.Equal(t, testUserID, initiatorID) assert.Equal(t, testAccountID, accountID) @@ -231,7 +161,7 @@ func TestManagerImpl_CreateRecord(t *testing.T) { }) t.Run("success - CNAME record", func(t *testing.T) { - manager, _, zone, mockAccountManager, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, _, zone, mockAccountManager, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -242,10 +172,6 @@ func TestManagerImpl_CreateRecord(t *testing.T) { TTL: 300, } - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Create). - Return(true, nil) - mockAccountManager.StoreEventFunc = func(ctx context.Context, initiatorID, targetID, accountID string, activityID activity.ActivityDescriber, meta map[string]any) { assert.Equal(t, testUserID, initiatorID) assert.Equal(t, testAccountID, accountID) @@ -259,32 +185,8 @@ func TestManagerImpl_CreateRecord(t *testing.T) { assert.Equal(t, inputRecord.Content, result.Content) }) - t.Run("permission denied", func(t *testing.T) { - manager, _, zone, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) - defer cleanup() - defer ctrl.Finish() - - inputRecord := &records.Record{ - Name: "api.example.com", - Type: records.RecordTypeA, - Content: "192.168.1.1", - TTL: 300, - } - - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Create). - Return(false, nil) - - result, err := manager.CreateRecord(ctx, testAccountID, testUserID, zone.ID, inputRecord) - require.Error(t, err) - assert.Nil(t, result) - s, ok := status.FromError(err) - assert.True(t, ok) - assert.Equal(t, status.PermissionDenied, s.Type()) - }) - t.Run("record name not in zone", func(t *testing.T) { - manager, _, zone, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, _, zone, _, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -295,10 +197,6 @@ func TestManagerImpl_CreateRecord(t *testing.T) { TTL: 300, } - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Create). - Return(true, nil) - result, err := manager.CreateRecord(ctx, testAccountID, testUserID, zone.ID, inputRecord) require.Error(t, err) assert.Nil(t, result) @@ -306,7 +204,7 @@ func TestManagerImpl_CreateRecord(t *testing.T) { }) t.Run("duplicate record", func(t *testing.T) { - manager, testStore, zone, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, testStore, zone, _, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -321,10 +219,6 @@ func TestManagerImpl_CreateRecord(t *testing.T) { TTL: 300, } - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Create). - Return(true, nil) - result, err := manager.CreateRecord(ctx, testAccountID, testUserID, zone.ID, inputRecord) require.Error(t, err) assert.Nil(t, result) @@ -332,7 +226,7 @@ func TestManagerImpl_CreateRecord(t *testing.T) { }) t.Run("CNAME conflict with existing A record", func(t *testing.T) { - manager, testStore, zone, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, testStore, zone, _, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -347,10 +241,6 @@ func TestManagerImpl_CreateRecord(t *testing.T) { TTL: 300, } - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Create). - Return(true, nil) - result, err := manager.CreateRecord(ctx, testAccountID, testUserID, zone.ID, inputRecord) require.Error(t, err) assert.Nil(t, result) @@ -362,7 +252,7 @@ func TestManagerImpl_UpdateRecord(t *testing.T) { ctx := context.Background() t.Run("success", func(t *testing.T) { - manager, testStore, zone, mockAccountManager, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, testStore, zone, mockAccountManager, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -378,10 +268,6 @@ func TestManagerImpl_UpdateRecord(t *testing.T) { TTL: 600, // Changed TTL } - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Update). - Return(true, nil) - storeEventCalled := false mockAccountManager.StoreEventFunc = func(ctx context.Context, initiatorID, targetID, accountID string, activityID activity.ActivityDescriber, meta map[string]any) { storeEventCalled = true @@ -400,7 +286,7 @@ func TestManagerImpl_UpdateRecord(t *testing.T) { }) t.Run("update only TTL - no validation", func(t *testing.T) { - manager, testStore, zone, mockAccountManager, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, testStore, zone, mockAccountManager, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -416,10 +302,6 @@ func TestManagerImpl_UpdateRecord(t *testing.T) { TTL: 600, // Only TTL changed } - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Update). - Return(true, nil) - mockAccountManager.StoreEventFunc = func(ctx context.Context, initiatorID, targetID, accountID string, activityID activity.ActivityDescriber, meta map[string]any) { // Event should be stored } @@ -430,33 +312,8 @@ func TestManagerImpl_UpdateRecord(t *testing.T) { assert.Equal(t, 600, result.TTL) }) - t.Run("permission denied", func(t *testing.T) { - manager, _, zone, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) - defer cleanup() - defer ctrl.Finish() - - updatedRecord := &records.Record{ - ID: testRecordID, - Name: "api.example.com", - Type: records.RecordTypeA, - Content: "192.168.1.100", - TTL: 600, - } - - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Update). - Return(false, nil) - - result, err := manager.UpdateRecord(ctx, testAccountID, testUserID, zone.ID, updatedRecord) - require.Error(t, err) - assert.Nil(t, result) - s, ok := status.FromError(err) - assert.True(t, ok) - assert.Equal(t, status.PermissionDenied, s.Type()) - }) - t.Run("record not found", func(t *testing.T) { - manager, _, zone, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, _, zone, _, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -468,17 +325,13 @@ func TestManagerImpl_UpdateRecord(t *testing.T) { TTL: 600, } - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Update). - Return(true, nil) - result, err := manager.UpdateRecord(ctx, testAccountID, testUserID, zone.ID, updatedRecord) require.Error(t, err) assert.Nil(t, result) }) t.Run("update creates duplicate", func(t *testing.T) { - manager, testStore, zone, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, testStore, zone, _, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -498,10 +351,6 @@ func TestManagerImpl_UpdateRecord(t *testing.T) { TTL: 300, } - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Update). - Return(true, nil) - result, err := manager.UpdateRecord(ctx, testAccountID, testUserID, zone.ID, updatedRecord) require.Error(t, err) assert.Nil(t, result) @@ -513,7 +362,7 @@ func TestManagerImpl_DeleteRecord(t *testing.T) { ctx := context.Background() t.Run("success", func(t *testing.T) { - manager, testStore, zone, mockAccountManager, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, testStore, zone, mockAccountManager, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() @@ -521,10 +370,6 @@ func TestManagerImpl_DeleteRecord(t *testing.T) { err := testStore.CreateDNSRecord(ctx, record) require.NoError(t, err) - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Delete). - Return(true, nil) - storeEventCalled := false mockAccountManager.StoreEventFunc = func(ctx context.Context, initiatorID, targetID, accountID string, activityID activity.ActivityDescriber, meta map[string]any) { storeEventCalled = true @@ -542,31 +387,11 @@ func TestManagerImpl_DeleteRecord(t *testing.T) { require.Error(t, err) }) - t.Run("permission denied", func(t *testing.T) { - manager, _, zone, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) - defer cleanup() - defer ctrl.Finish() - - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Delete). - Return(false, nil) - - err := manager.DeleteRecord(ctx, testAccountID, testUserID, zone.ID, testRecordID) - require.Error(t, err) - s, ok := status.FromError(err) - assert.True(t, ok) - assert.Equal(t, status.PermissionDenied, s.Type()) - }) - t.Run("record not found", func(t *testing.T) { - manager, _, zone, _, mockPermissionsManager, ctrl, cleanup := setupTest(t) + manager, _, zone, _, ctrl, cleanup := setupTest(t) defer cleanup() defer ctrl.Finish() - mockPermissionsManager.EXPECT(). - ValidateUserPermissions(ctx, testAccountID, testUserID, modules.Dns, operations.Delete). - Return(true, nil) - err := manager.DeleteRecord(ctx, testAccountID, testUserID, zone.ID, "non-existent-record") require.Error(t, err) }) diff --git a/management/internals/server/boot.go b/management/internals/server/boot.go index 2049f0051..da32f7b54 100644 --- a/management/internals/server/boot.go +++ b/management/internals/server/boot.go @@ -205,7 +205,7 @@ func (s *BaseServer) ProxyTokenStore() *nbgrpc.OneTimeTokenStore { func (s *BaseServer) AccessLogsManager() accesslogs.Manager { return Create(s, func() accesslogs.Manager { - accessLogManager := accesslogsmanager.NewManager(s.Store(), s.PermissionsManager(), s.GeoLocationManager()) + accessLogManager := accesslogsmanager.NewManager(s.Store(), s.GeoLocationManager()) accessLogManager.StartPeriodicCleanup( context.Background(), s.Config.ReverseProxy.AccessLogRetentionDays, diff --git a/management/internals/server/modules.go b/management/internals/server/modules.go index 2383019e2..5b1d62f61 100644 --- a/management/internals/server/modules.go +++ b/management/internals/server/modules.go @@ -8,6 +8,7 @@ import ( "github.com/netbirdio/management-integrations/integrations" "github.com/netbirdio/netbird/management/internals/modules/peers" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain/manager" "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy" proxymanager "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy/manager" @@ -26,7 +27,6 @@ import ( "github.com/netbirdio/netbird/management/server/networks/resources" "github.com/netbirdio/netbird/management/server/networks/routers" - "github.com/netbirdio/netbird/management/server/permissions" "github.com/netbirdio/netbird/management/server/settings" "github.com/netbirdio/netbird/management/server/users" ) @@ -81,13 +81,13 @@ func (s *BaseServer) SettingsManager() settings.Manager { idpConfig.LocalAuthDisabled = s.Config.EmbeddedIdP.LocalAuthDisabled } - return settings.NewManager(s.Store(), s.UsersManager(), extraSettingsManager, s.PermissionsManager(), idpConfig) + return settings.NewManager(s.Store(), s.UsersManager(), extraSettingsManager, idpConfig) }) } func (s *BaseServer) PeersManager() peers.Manager { return Create(s, func() peers.Manager { - manager := peers.NewManager(s.Store(), s.PermissionsManager()) + manager := peers.NewManager(s.Store()) s.AfterInit(func(s *BaseServer) { manager.SetNetworkMapController(s.NetworkMapController()) manager.SetIntegratedPeerValidator(s.IntegratedValidator()) @@ -158,7 +158,7 @@ func (s *BaseServer) OAuthConfigProvider() idp.OAuthConfigProvider { func (s *BaseServer) GroupsManager() groups.Manager { return Create(s, func() groups.Manager { - return groups.NewManager(s.Store(), s.PermissionsManager(), s.AccountManager()) + return groups.NewManager(s.Store(), s.AccountManager()) }) } @@ -182,19 +182,19 @@ func (s *BaseServer) NetworksManager() networks.Manager { func (s *BaseServer) ZonesManager() zones.Manager { return Create(s, func() zones.Manager { - return zonesManager.NewManager(s.Store(), s.AccountManager(), s.PermissionsManager(), s.DNSDomain()) + return zonesManager.NewManager(s.Store(), s.AccountManager(), s.DNSDomain()) }) } func (s *BaseServer) RecordsManager() records.Manager { return Create(s, func() records.Manager { - return recordsManager.NewManager(s.Store(), s.AccountManager(), s.PermissionsManager()) + return recordsManager.NewManager(s.Store(), s.AccountManager()) }) } func (s *BaseServer) ServiceManager() service.Manager { return Create(s, func() service.Manager { - return nbreverseproxy.NewManager(s.Store(), s.AccountManager(), s.PermissionsManager(), s.ServiceProxyController(), s.ReverseProxyDomainManager()) + return nbreverseproxy.NewManager(s.Store(), s.AccountManager(), s.ServiceProxyController(), s.ReverseProxyDomainManager()) }) } @@ -210,7 +210,7 @@ func (s *BaseServer) ProxyManager() proxy.Manager { func (s *BaseServer) ReverseProxyDomainManager() *manager.Manager { return Create(s, func() *manager.Manager { - m := manager.NewManager(s.Store(), s.ProxyManager(), s.PermissionsManager()) + m := manager.NewManager(s.Store(), s.ProxyManager()) return &m }) } diff --git a/management/server/account.go b/management/server/account.go index 01d0eebfa..051753ca4 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -15,6 +15,7 @@ import ( "sync" "time" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service" "github.com/netbirdio/netbird/management/server/job" "github.com/netbirdio/netbird/shared/auth" @@ -39,9 +40,6 @@ 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/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/posture" "github.com/netbirdio/netbird/management/server/settings" "github.com/netbirdio/netbird/management/server/store" @@ -285,22 +283,14 @@ func (am *DefaultAccountManager) GetIdpManager() idp.Manager { // User that performs the update has to belong to the account. // Returns an updated Settings func (am *DefaultAccountManager) UpdateAccountSettings(ctx context.Context, accountID, userID string, newSettings *types.Settings) (*types.Settings, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Settings, operations.Update) - if err != nil { - return nil, fmt.Errorf("failed to validate user permissions: %w", err) - } - - if !allowed { - return nil, status.NewPermissionDeniedError() - } - var oldSettings *types.Settings var updateAccountPeers bool var groupChangesAffectPeers bool var reloadReverseProxy bool - err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + err := am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { var groupsUpdated bool + var err error oldSettings, err = transaction.GetAccountSettings(ctx, store.LockingStrengthUpdate, accountID) if err != nil { @@ -716,15 +706,6 @@ func (am *DefaultAccountManager) DeleteAccount(ctx context.Context, accountID, u return err } - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Accounts, operations.Delete) - if err != nil { - return fmt.Errorf("failed to validate user permissions: %w", err) - } - - if !allowed { - return status.Errorf(status.PermissionDenied, "user is not allowed to delete account. Only account owner can delete account") - } - userInfosMap, err := am.BuildUserInfosForAccount(ctx, accountID, userID, maps.Values(account.Users)) if err != nil { return status.Errorf(status.Internal, "failed to build user infos for account %s: %v", accountID, err) @@ -1283,41 +1264,16 @@ func (am *DefaultAccountManager) GetAccount(ctx context.Context, accountID strin // GetAccountByID returns an account associated with this account ID. func (am *DefaultAccountManager) GetAccountByID(ctx context.Context, accountID string, userID string) (*types.Account, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Accounts, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - return am.Store.GetAccount(ctx, accountID) } // GetAccountMeta returns the account metadata associated with this account ID. func (am *DefaultAccountManager) GetAccountMeta(ctx context.Context, accountID string, userID string) (*types.AccountMeta, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Accounts, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - return am.Store.GetAccountMeta(ctx, store.LockingStrengthNone, accountID) } // GetAccountOnboarding retrieves the onboarding information for a specific account. func (am *DefaultAccountManager) GetAccountOnboarding(ctx context.Context, accountID string, userID string) (*types.AccountOnboarding, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Accounts, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - - if !allowed { - return nil, status.NewPermissionDeniedError() - } - onboarding, err := am.Store.GetAccountOnboarding(ctx, accountID) if err != nil && err.Error() != status.NewAccountOnboardingNotFoundError(accountID).Error() { log.Errorf("failed to get account onboarding for account %s: %v", accountID, err) @@ -1334,15 +1290,6 @@ func (am *DefaultAccountManager) GetAccountOnboarding(ctx context.Context, accou } func (am *DefaultAccountManager) UpdateAccountOnboarding(ctx context.Context, accountID, userID string, newOnboarding *types.AccountOnboarding) (*types.AccountOnboarding, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Settings, operations.Update) - if err != nil { - return nil, fmt.Errorf("failed to validate user permissions: %w", err) - } - - if !allowed { - return nil, status.NewPermissionDeniedError() - } - oldOnboarding, err := am.Store.GetAccountOnboarding(ctx, accountID) if err != nil && err.Error() != status.NewAccountOnboardingNotFoundError(accountID).Error() { return nil, fmt.Errorf("failed to get account onboarding: %w", err) @@ -1401,9 +1348,8 @@ func (am *DefaultAccountManager) GetAccountIDFromUserAuth(ctx context.Context, u return accountID, user.Id, nil } - if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user, false); err != nil { - return "", "", err - } + // Permission checks are now handled by the HTTP middleware via WithPermission wrapper + // User account association is already validated above by GetUserByUserID if !user.IsServiceUser && userAuth.Invited { err = am.redeemInvite(ctx, accountID, user.Id) @@ -1845,13 +1791,6 @@ func (am *DefaultAccountManager) handleUserPeer(ctx context.Context, transaction } func (am *DefaultAccountManager) GetAccountSettings(ctx context.Context, accountID string, userID string) (*types.Settings, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Settings, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } return am.Store.GetAccountSettings(ctx, store.LockingStrengthNone, accountID) } @@ -2193,14 +2132,6 @@ func (am *DefaultAccountManager) validateIPForUpdate(account *types.Account, pee } func (am *DefaultAccountManager) UpdatePeerIP(ctx context.Context, accountID, userID, peerID string, newIP netip.Addr) error { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Peers, operations.Update) - if err != nil { - return fmt.Errorf("validate user permissions: %w", err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - updateNetworkMap, err := am.updatePeerIPInTransaction(ctx, accountID, userID, peerID, newIP) if err != nil { return fmt.Errorf("update peer IP transaction: %w", err) diff --git a/management/server/account_test.go b/management/server/account_test.go index a073d4fca..b2b058f36 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -15,7 +15,6 @@ import ( "time" "github.com/golang/mock/gomock" - "github.com/netbirdio/netbird/shared/management/status" "github.com/prometheus/client_golang/prometheus/push" log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" @@ -23,6 +22,9 @@ import ( "go.opentelemetry.io/otel/metric/noop" "golang.zx2c4.com/wireguard/wgctrl/wgtypes" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/shared/management/status" + nbdns "github.com/netbirdio/netbird/dns" "github.com/netbirdio/netbird/management/internals/controllers/network_map" "github.com/netbirdio/netbird/management/internals/controllers/network_map/controller" @@ -47,7 +49,6 @@ 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/posture" "github.com/netbirdio/netbird/management/server/settings" "github.com/netbirdio/netbird/management/server/store" diff --git a/management/server/dns.go b/management/server/dns.go index baf6debc3..dd3cecb71 100644 --- a/management/server/dns.go +++ b/management/server/dns.go @@ -8,8 +8,6 @@ import ( nbdns "github.com/netbirdio/netbird/dns" "github.com/netbirdio/netbird/management/server/activity" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/management/server/util" @@ -22,14 +20,6 @@ const ( // GetDNSSettings validates a user role and returns the DNS settings for the provided account ID func (am *DefaultAccountManager) GetDNSSettings(ctx context.Context, accountID string, userID string) (*types.DNSSettings, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Dns, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - return am.Store.GetAccountDNSSettings(ctx, store.LockingStrengthNone, accountID) } @@ -39,18 +29,11 @@ func (am *DefaultAccountManager) SaveDNSSettings(ctx context.Context, accountID return status.Errorf(status.InvalidArgument, "the dns settings provided are nil") } - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Dns, operations.Update) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - var updateAccountPeers bool var eventsToStore []func() - err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + err := am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + var err error if err = validateDNSSettings(ctx, transaction, accountID, dnsSettingsToSave); err != nil { return err } diff --git a/management/server/dns_test.go b/management/server/dns_test.go index bd0755d0d..554a7f37c 100644 --- a/management/server/dns_test.go +++ b/management/server/dns_test.go @@ -14,10 +14,10 @@ import ( "github.com/netbirdio/netbird/management/internals/controllers/network_map/update_channel" "github.com/netbirdio/netbird/management/internals/modules/peers" ephemeral_manager "github.com/netbirdio/netbird/management/internals/modules/peers/ephemeral/manager" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/internals/server/config" "github.com/netbirdio/netbird/management/server/integrations/port_forwarding" "github.com/netbirdio/netbird/management/server/job" - "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" diff --git a/management/server/event.go b/management/server/event.go index d26c569ae..96a0ba279 100644 --- a/management/server/event.go +++ b/management/server/event.go @@ -9,11 +9,8 @@ import ( log "github.com/sirupsen/logrus" "github.com/netbirdio/netbird/management/server/activity" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" - "github.com/netbirdio/netbird/shared/management/status" ) func isEnabled() bool { @@ -23,14 +20,6 @@ func isEnabled() bool { // GetEvents returns a list of activity events of an account func (am *DefaultAccountManager) GetEvents(ctx context.Context, accountID, userID string) ([]*activity.Event, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Events, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - events, err := am.eventStore.Get(ctx, accountID, 0, 10000, true) if err != nil { return nil, err diff --git a/management/server/group.go b/management/server/group.go index 326b167cf..06d21de20 100644 --- a/management/server/group.go +++ b/management/server/group.go @@ -12,8 +12,6 @@ import ( nbdns "github.com/netbirdio/netbird/dns" "github.com/netbirdio/netbird/management/server/activity" routerTypes "github.com/netbirdio/netbird/management/server/networks/routers/types" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/management/server/util" @@ -32,13 +30,24 @@ func (e *GroupLinkError) Error() string { // CheckGroupPermissions validates if a user has the necessary permissions to view groups func (am *DefaultAccountManager) CheckGroupPermissions(ctx context.Context, accountID, userID string) error { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Groups, operations.Read) + // Permission checks are now handled by the HTTP middleware via WithPermission wrapper + // This method is called from authenticated/authorized handlers, so we just validate + // that the user exists and is part of the account + user, err := am.Store.GetUserByUserID(ctx, store.LockingStrengthNone, userID) if err != nil { return err } - if !allowed { - return status.NewPermissionDeniedError() + if user == nil { + return status.NewUserNotFoundError(userID) + } + + if user.AccountID != accountID { + return status.NewUserNotPartOfAccountError() + } + + if user.IsBlocked() { + return status.NewUserBlockedError() } return nil @@ -67,18 +76,11 @@ func (am *DefaultAccountManager) GetGroupByName(ctx context.Context, groupName, // CreateGroup object of the peers func (am *DefaultAccountManager) CreateGroup(ctx context.Context, accountID, userID string, newGroup *types.Group) error { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Groups, operations.Create) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - var eventsToStore []func() var updateAccountPeers bool - err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + err := am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + var err error if err = validateNewGroup(ctx, transaction, accountID, newGroup); err != nil { return err } @@ -122,19 +124,11 @@ func (am *DefaultAccountManager) CreateGroup(ctx context.Context, accountID, use // UpdateGroup object of the peers func (am *DefaultAccountManager) UpdateGroup(ctx context.Context, accountID, userID string, newGroup *types.Group) error { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Groups, operations.Update) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - var eventsToStore []func() var updateAccountPeers bool - err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { - if err = validateNewGroup(ctx, transaction, accountID, newGroup); err != nil { + err := am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + if err := validateNewGroup(ctx, transaction, accountID, newGroup); err != nil { return err } @@ -193,33 +187,24 @@ func (am *DefaultAccountManager) UpdateGroup(ctx context.Context, accountID, use // It is the caller's responsibility to ensure proper locking is in place before invoking this method. // This method will not create group peer membership relations. Use AddPeerToGroup or RemovePeerFromGroup methods for that. func (am *DefaultAccountManager) CreateGroups(ctx context.Context, accountID, userID string, groups []*types.Group) error { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Groups, operations.Create) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - var eventsToStore []func() var updateAccountPeers bool var globalErr error groupIDs := make([]string, 0, len(groups)) for _, newGroup := range groups { - err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { - if err = validateNewGroup(ctx, transaction, accountID, newGroup); err != nil { + err := am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + if err := validateNewGroup(ctx, transaction, accountID, newGroup); err != nil { return err } newGroup.AccountID = accountID - if err = transaction.CreateGroup(ctx, newGroup); err != nil { + if err := transaction.CreateGroup(ctx, newGroup); err != nil { return err } - err = transaction.IncrementNetworkSerial(ctx, accountID) - if err != nil { + if err := transaction.IncrementNetworkSerial(ctx, accountID); err != nil { return err } @@ -240,6 +225,7 @@ func (am *DefaultAccountManager) CreateGroups(ctx context.Context, accountID, us } } + var err error updateAccountPeers, err = areGroupChangesAffectPeers(ctx, am.Store, accountID, groupIDs) if err != nil { return err @@ -261,21 +247,14 @@ func (am *DefaultAccountManager) CreateGroups(ctx context.Context, accountID, us // It is the caller's responsibility to ensure proper locking is in place before invoking this method. // This method will not create group peer membership relations. Use AddPeerToGroup or RemovePeerFromGroup methods for that. func (am *DefaultAccountManager) UpdateGroups(ctx context.Context, accountID, userID string, groups []*types.Group) error { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Groups, operations.Update) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - var eventsToStore []func() var updateAccountPeers bool var globalErr error groupIDs := make([]string, 0, len(groups)) for _, newGroup := range groups { - err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + err := am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + var err error if err = validateNewGroup(ctx, transaction, accountID, newGroup); err != nil { return err } @@ -308,6 +287,7 @@ func (am *DefaultAccountManager) UpdateGroups(ctx context.Context, accountID, us } } + var err error updateAccountPeers, err = areGroupChangesAffectPeers(ctx, am.Store, accountID, groupIDs) if err != nil { return err @@ -413,14 +393,6 @@ func (am *DefaultAccountManager) DeleteGroup(ctx context.Context, accountID, use // If an error occurs while deleting a group, the function skips it and continues deleting other groups. // Errors are collected and returned at the end. func (am *DefaultAccountManager) DeleteGroups(ctx context.Context, accountID, userID string, groupIDs []string) error { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Groups, operations.Delete) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - var allErrors error var groupIDsToDelete []string var deletedGroups []*types.Group diff --git a/management/server/group_test.go b/management/server/group_test.go index fa818e532..4197775b6 100644 --- a/management/server/group_test.go +++ b/management/server/group_test.go @@ -19,6 +19,7 @@ import ( "golang.org/x/exp/maps" nbdns "github.com/netbirdio/netbird/dns" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/server/groups" "github.com/netbirdio/netbird/management/server/networks" "github.com/netbirdio/netbird/management/server/networks/resources" @@ -26,7 +27,6 @@ import ( routerTypes "github.com/netbirdio/netbird/management/server/networks/routers/types" networkTypes "github.com/netbirdio/netbird/management/server/networks/types" peer2 "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/types" diff --git a/management/server/groups/manager.go b/management/server/groups/manager.go index d110ab564..40fe03550 100644 --- a/management/server/groups/manager.go +++ b/management/server/groups/manager.go @@ -6,9 +6,6 @@ import ( "github.com/netbirdio/netbird/management/server/account" "github.com/netbirdio/netbird/management/server/activity" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/management/http/api" @@ -25,31 +22,21 @@ type Manager interface { } type managerImpl struct { - store store.Store - permissionsManager permissions.Manager - accountManager account.Manager + store store.Store + accountManager account.Manager } type mockManager struct { } -func NewManager(store store.Store, permissionsManager permissions.Manager, accountManager account.Manager) Manager { +func NewManager(store store.Store, accountManager account.Manager) Manager { return &managerImpl{ - store: store, - permissionsManager: permissionsManager, - accountManager: accountManager, + store: store, + accountManager: accountManager, } } func (m *managerImpl) GetAllGroups(ctx context.Context, accountID, userID string) ([]*types.Group, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Groups, operations.Read) - if err != nil { - return nil, err - } - if !ok { - return nil, err - } - groups, err := m.store.GetAccountGroups(ctx, store.LockingStrengthNone, accountID) if err != nil { return nil, fmt.Errorf("error getting account groups: %w", err) @@ -73,14 +60,6 @@ func (m *managerImpl) GetAllGroupsMap(ctx context.Context, accountID, userID str } func (m *managerImpl) AddResourceToGroup(ctx context.Context, accountID, userID, groupID string, resource *types.Resource) error { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Groups, operations.Update) - if err != nil { - return err - } - if !ok { - return err - } - event, err := m.AddResourceToGroupInTransaction(ctx, m.store, accountID, userID, groupID, resource) if err != nil { return fmt.Errorf("error adding resource to group: %w", err) diff --git a/management/server/http/handler.go b/management/server/http/handler.go index 228656377..5489a550a 100644 --- a/management/server/http/handler.go +++ b/management/server/http/handler.go @@ -13,6 +13,7 @@ import ( "github.com/rs/cors" log "github.com/sirupsen/logrus" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain/manager" "github.com/netbirdio/netbird/management/server/types" @@ -34,10 +35,8 @@ import ( "github.com/netbirdio/netbird/management/server/account" "github.com/netbirdio/netbird/management/server/settings" - "github.com/netbirdio/netbird/management/server/integrations/port_forwarding" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/http/handlers/proxy" + "github.com/netbirdio/netbird/management/server/integrations/port_forwarding" nbpeers "github.com/netbirdio/netbird/management/internals/modules/peers" "github.com/netbirdio/netbird/management/server/auth" diff --git a/management/server/http/handlers/accounts/accounts_handler.go b/management/server/http/handlers/accounts/accounts_handler.go index b41ce8dc9..45ee774c3 100644 --- a/management/server/http/handlers/accounts/accounts_handler.go +++ b/management/server/http/handlers/accounts/accounts_handler.go @@ -12,10 +12,10 @@ import ( goversion "github.com/hashicorp/go-version" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/account" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/settings" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/auth" diff --git a/management/server/http/handlers/dns/dns_settings_handler.go b/management/server/http/handlers/dns/dns_settings_handler.go index c7f99a784..bd1573c6c 100644 --- a/management/server/http/handlers/dns/dns_settings_handler.go +++ b/management/server/http/handlers/dns/dns_settings_handler.go @@ -6,10 +6,10 @@ import ( "github.com/gorilla/mux" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/account" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" diff --git a/management/server/http/handlers/dns/nameservers_handler.go b/management/server/http/handlers/dns/nameservers_handler.go index 9421b8f45..ff2a6ef55 100644 --- a/management/server/http/handlers/dns/nameservers_handler.go +++ b/management/server/http/handlers/dns/nameservers_handler.go @@ -8,10 +8,10 @@ import ( "github.com/gorilla/mux" nbdns "github.com/netbirdio/netbird/dns" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/account" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" "github.com/netbirdio/netbird/shared/management/http/util" diff --git a/management/server/http/handlers/events/events_handler.go b/management/server/http/handlers/events/events_handler.go index add30848b..ee931f069 100644 --- a/management/server/http/handlers/events/events_handler.go +++ b/management/server/http/handlers/events/events_handler.go @@ -6,11 +6,11 @@ import ( "github.com/gorilla/mux" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/account" "github.com/netbirdio/netbird/management/server/activity" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" "github.com/netbirdio/netbird/shared/management/http/util" diff --git a/management/server/http/handlers/groups/groups_handler.go b/management/server/http/handlers/groups/groups_handler.go index 41e1af825..f04294a96 100644 --- a/management/server/http/handlers/groups/groups_handler.go +++ b/management/server/http/handlers/groups/groups_handler.go @@ -7,11 +7,11 @@ import ( "github.com/gorilla/mux" log "github.com/sirupsen/logrus" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/account" nbpeer "github.com/netbirdio/netbird/management/server/peer" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" diff --git a/management/server/http/handlers/idp/idp_handler.go b/management/server/http/handlers/idp/idp_handler.go index 5a192e7df..c478ed1f5 100644 --- a/management/server/http/handlers/idp/idp_handler.go +++ b/management/server/http/handlers/idp/idp_handler.go @@ -6,10 +6,10 @@ import ( "github.com/gorilla/mux" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/account" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" diff --git a/management/server/http/handlers/instance/instance_handler.go b/management/server/http/handlers/instance/instance_handler.go index 77e1efb57..9e22b7796 100644 --- a/management/server/http/handlers/instance/instance_handler.go +++ b/management/server/http/handlers/instance/instance_handler.go @@ -7,10 +7,10 @@ import ( "github.com/gorilla/mux" log "github.com/sirupsen/logrus" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" nbinstance "github.com/netbirdio/netbird/management/server/instance" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" "github.com/netbirdio/netbird/shared/management/http/util" diff --git a/management/server/http/handlers/networks/handler.go b/management/server/http/handlers/networks/handler.go index f8fec0948..ada3102ba 100644 --- a/management/server/http/handlers/networks/handler.go +++ b/management/server/http/handlers/networks/handler.go @@ -9,6 +9,9 @@ import ( "github.com/gorilla/mux" log "github.com/sirupsen/logrus" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/account" "github.com/netbirdio/netbird/management/server/groups" "github.com/netbirdio/netbird/management/server/networks" @@ -16,9 +19,6 @@ import ( "github.com/netbirdio/netbird/management/server/networks/routers" routerTypes "github.com/netbirdio/netbird/management/server/networks/routers/types" "github.com/netbirdio/netbird/management/server/networks/types" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" nbtypes "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" diff --git a/management/server/http/handlers/networks/resources_handler.go b/management/server/http/handlers/networks/resources_handler.go index 4df968d6b..e57bbb420 100644 --- a/management/server/http/handlers/networks/resources_handler.go +++ b/management/server/http/handlers/networks/resources_handler.go @@ -6,12 +6,12 @@ import ( "github.com/gorilla/mux" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/groups" "github.com/netbirdio/netbird/management/server/networks/resources" "github.com/netbirdio/netbird/management/server/networks/resources/types" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" "github.com/netbirdio/netbird/shared/management/http/util" diff --git a/management/server/http/handlers/networks/routers_handler.go b/management/server/http/handlers/networks/routers_handler.go index 0e33d2757..d61981233 100644 --- a/management/server/http/handlers/networks/routers_handler.go +++ b/management/server/http/handlers/networks/routers_handler.go @@ -6,11 +6,11 @@ import ( "github.com/gorilla/mux" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/networks/routers" "github.com/netbirdio/netbird/management/server/networks/routers/types" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" "github.com/netbirdio/netbird/shared/management/http/util" diff --git a/management/server/http/handlers/peers/peers_handler.go b/management/server/http/handlers/peers/peers_handler.go index 566fcc064..06823c0f8 100644 --- a/management/server/http/handlers/peers/peers_handler.go +++ b/management/server/http/handlers/peers/peers_handler.go @@ -11,13 +11,13 @@ import ( "github.com/netbirdio/netbird/dns" "github.com/netbirdio/netbird/management/internals/controllers/network_map" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/account" "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/groups" nbpeer "github.com/netbirdio/netbird/management/server/peer" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" @@ -333,19 +333,16 @@ func (h *Handler) GetAccessiblePeers(w http.ResponseWriter, r *http.Request, use return } - allowed, err := h.permissionsManager.ValidateUserPermissions(r.Context(), userAuth.AccountId, userAuth.UserId, modules.Peers, operations.Read) - if err != nil { - util.WriteError(r.Context(), status.NewPermissionValidationError(err), w) - return - } - account, err := h.accountManager.GetAccountByID(r.Context(), userAuth.AccountId, activity.SystemInitiator) if err != nil { util.WriteError(r.Context(), err, w) return } - if !allowed && !userAuth.IsChild { + // Check if user is an admin/service user through their role + isAdmin := user.Role == types.UserRoleAdmin || user.Role == types.UserRoleOwner + + if !isAdmin && !userAuth.IsChild { if account.Settings.RegularUsersViewBlocked { util.WriteJSONObject(r.Context(), w, []api.AccessiblePeer{}) return diff --git a/management/server/http/handlers/peers/peers_handler_test.go b/management/server/http/handlers/peers/peers_handler_test.go index ae4cbe0af..4d803d436 100644 --- a/management/server/http/handlers/peers/peers_handler_test.go +++ b/management/server/http/handlers/peers/peers_handler_test.go @@ -19,11 +19,11 @@ import ( "golang.org/x/exp/maps" "github.com/netbirdio/netbird/management/internals/controllers/network_map" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" nbcontext "github.com/netbirdio/netbird/management/server/context" nbpeer "github.com/netbirdio/netbird/management/server/peer" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" diff --git a/management/server/http/handlers/policies/geolocation_handler_test.go b/management/server/http/handlers/policies/geolocation_handler_test.go index 8084cac75..696fcf665 100644 --- a/management/server/http/handlers/policies/geolocation_handler_test.go +++ b/management/server/http/handlers/policies/geolocation_handler_test.go @@ -14,12 +14,12 @@ import ( "github.com/gorilla/mux" "github.com/stretchr/testify/assert" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" nbcontext "github.com/netbirdio/netbird/management/server/context" "github.com/netbirdio/netbird/management/server/geolocation" "github.com/netbirdio/netbird/management/server/mock_server" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" diff --git a/management/server/http/handlers/policies/geolocations_handler.go b/management/server/http/handlers/policies/geolocations_handler.go index 793e943a8..dd9ea7c14 100644 --- a/management/server/http/handlers/policies/geolocations_handler.go +++ b/management/server/http/handlers/policies/geolocations_handler.go @@ -6,11 +6,11 @@ import ( "github.com/gorilla/mux" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/account" "github.com/netbirdio/netbird/management/server/geolocation" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" "github.com/netbirdio/netbird/shared/management/http/util" diff --git a/management/server/http/handlers/policies/policies_handler.go b/management/server/http/handlers/policies/policies_handler.go index b382168d9..a156df78e 100644 --- a/management/server/http/handlers/policies/policies_handler.go +++ b/management/server/http/handlers/policies/policies_handler.go @@ -7,11 +7,11 @@ import ( "github.com/gorilla/mux" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/account" "github.com/netbirdio/netbird/management/server/geolocation" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" diff --git a/management/server/http/handlers/policies/posture_checks_handler.go b/management/server/http/handlers/policies/posture_checks_handler.go index 01d919151..c4b86288e 100644 --- a/management/server/http/handlers/policies/posture_checks_handler.go +++ b/management/server/http/handlers/policies/posture_checks_handler.go @@ -6,11 +6,11 @@ import ( "github.com/gorilla/mux" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/account" "github.com/netbirdio/netbird/management/server/geolocation" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/posture" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" diff --git a/management/server/http/handlers/routes/routes_handler.go b/management/server/http/handlers/routes/routes_handler.go index 19eb1f305..a6a8ba210 100644 --- a/management/server/http/handlers/routes/routes_handler.go +++ b/management/server/http/handlers/routes/routes_handler.go @@ -8,10 +8,10 @@ import ( "github.com/gorilla/mux" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/account" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/route" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/domain" diff --git a/management/server/http/handlers/setup_keys/setupkeys_handler.go b/management/server/http/handlers/setup_keys/setupkeys_handler.go index 12506464d..cb3089344 100644 --- a/management/server/http/handlers/setup_keys/setupkeys_handler.go +++ b/management/server/http/handlers/setup_keys/setupkeys_handler.go @@ -8,10 +8,10 @@ import ( "github.com/gorilla/mux" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/account" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" diff --git a/management/server/http/handlers/users/invites_handler.go b/management/server/http/handlers/users/invites_handler.go index 41f2b9211..81585c28e 100644 --- a/management/server/http/handlers/users/invites_handler.go +++ b/management/server/http/handlers/users/invites_handler.go @@ -9,11 +9,11 @@ import ( "github.com/gorilla/mux" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/account" "github.com/netbirdio/netbird/management/server/http/middleware" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" diff --git a/management/server/http/handlers/users/pat_handler.go b/management/server/http/handlers/users/pat_handler.go index 79ee1fe9d..3a978a4d5 100644 --- a/management/server/http/handlers/users/pat_handler.go +++ b/management/server/http/handlers/users/pat_handler.go @@ -6,10 +6,10 @@ import ( "github.com/gorilla/mux" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/account" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/management/http/api" diff --git a/management/server/http/handlers/users/users_handler.go b/management/server/http/handlers/users/users_handler.go index 11480f104..09b506b67 100644 --- a/management/server/http/handlers/users/users_handler.go +++ b/management/server/http/handlers/users/users_handler.go @@ -8,10 +8,10 @@ import ( "github.com/gorilla/mux" log "github.com/sirupsen/logrus" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + "github.com/netbirdio/netbird/management/internals/modules/permissions/operations" "github.com/netbirdio/netbird/management/server/account" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/management/server/users" "github.com/netbirdio/netbird/shared/auth" diff --git a/management/server/http/handlers/users/users_handler_test.go b/management/server/http/handlers/users/users_handler_test.go index ef2f96685..ea9093915 100644 --- a/management/server/http/handlers/users/users_handler_test.go +++ b/management/server/http/handlers/users/users_handler_test.go @@ -15,10 +15,10 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + roles2 "github.com/netbirdio/netbird/management/internals/modules/permissions/roles" nbcontext "github.com/netbirdio/netbird/management/server/context" "github.com/netbirdio/netbird/management/server/mock_server" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/roles" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/management/server/users" "github.com/netbirdio/netbird/shared/auth" @@ -151,7 +151,7 @@ func initUsersTestData() *handler { NonDeletable: false, Issued: "api", }, - Permissions: mergeRolePermissions(roles.Owner), + Permissions: mergeRolePermissions(roles2.Owner), }, nil case "regular-user": return &users.UserInfoWithPermissions{ @@ -165,7 +165,7 @@ func initUsersTestData() *handler { NonDeletable: false, Issued: "api", }, - Permissions: mergeRolePermissions(roles.User), + Permissions: mergeRolePermissions(roles2.User), }, nil case "admin-user": @@ -181,7 +181,7 @@ func initUsersTestData() *handler { LastLogin: time.Time{}, Issued: "api", }, - Permissions: mergeRolePermissions(roles.Admin), + Permissions: mergeRolePermissions(roles2.Admin), }, nil case "restricted-user": return &users.UserInfoWithPermissions{ @@ -196,7 +196,7 @@ func initUsersTestData() *handler { LastLogin: time.Time{}, Issued: "api", }, - Permissions: mergeRolePermissions(roles.User), + Permissions: mergeRolePermissions(roles2.User), Restricted: true, }, nil } @@ -624,7 +624,7 @@ func TestCurrentUser(t *testing.T) { Issued: ptr("api"), LastLogin: ptr(time.Time{}), Permissions: &api.UserPermissions{ - Modules: stringifyPermissionsKeys(mergeRolePermissions(roles.Owner)), + Modules: stringifyPermissionsKeys(mergeRolePermissions(roles2.Owner)), }, }, }, @@ -643,7 +643,7 @@ func TestCurrentUser(t *testing.T) { Issued: ptr("api"), LastLogin: ptr(time.Time{}), Permissions: &api.UserPermissions{ - Modules: stringifyPermissionsKeys(mergeRolePermissions(roles.User)), + Modules: stringifyPermissionsKeys(mergeRolePermissions(roles2.User)), }, }, }, @@ -662,7 +662,7 @@ func TestCurrentUser(t *testing.T) { Issued: ptr("api"), LastLogin: ptr(time.Time{}), Permissions: &api.UserPermissions{ - Modules: stringifyPermissionsKeys(mergeRolePermissions(roles.Admin)), + Modules: stringifyPermissionsKeys(mergeRolePermissions(roles2.Admin)), }, }, }, @@ -682,7 +682,7 @@ func TestCurrentUser(t *testing.T) { LastLogin: ptr(time.Time{}), Permissions: &api.UserPermissions{ IsRestricted: true, - Modules: stringifyPermissionsKeys(mergeRolePermissions(roles.User)), + Modules: stringifyPermissionsKeys(mergeRolePermissions(roles2.User)), }, }, }, @@ -722,8 +722,8 @@ func ptr[T any, PT *T](x T) PT { return &x } -func mergeRolePermissions(role roles.RolePermissions) roles.Permissions { - permissions := roles.Permissions{} +func mergeRolePermissions(role roles2.RolePermissions) roles2.Permissions { + permissions := roles2.Permissions{} for k := range modules.All { if rolePermissions, ok := role.Permissions[k]; ok { @@ -736,7 +736,7 @@ func mergeRolePermissions(role roles.RolePermissions) roles.Permissions { return permissions } -func stringifyPermissionsKeys(permissions roles.Permissions) map[string]map[string]bool { +func stringifyPermissionsKeys(permissions roles2.Permissions) map[string]map[string]bool { modules := make(map[string]map[string]bool) for module, operations := range permissions { modules[string(module)] = make(map[string]bool) diff --git a/management/server/http/testing/testing_tools/channel/channel.go b/management/server/http/testing/testing_tools/channel/channel.go index 1d74f88d5..9c8f5ca03 100644 --- a/management/server/http/testing/testing_tools/channel/channel.go +++ b/management/server/http/testing/testing_tools/channel/channel.go @@ -12,6 +12,7 @@ import ( "go.opentelemetry.io/otel/metric/noop" "github.com/netbirdio/management-integrations/integrations" + "github.com/netbirdio/netbird/management/internals/modules/permissions" accesslogsmanager "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/accesslogs/manager" "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain/manager" proxymanager "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy/manager" @@ -41,7 +42,6 @@ import ( "github.com/netbirdio/netbird/management/server/networks" "github.com/netbirdio/netbird/management/server/networks/resources" "github.com/netbirdio/netbird/management/server/networks/routers" - "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" diff --git a/management/server/identity_provider.go b/management/server/identity_provider.go index 8fd96c238..0533ed7c5 100644 --- a/management/server/identity_provider.go +++ b/management/server/identity_provider.go @@ -17,8 +17,6 @@ import ( "github.com/netbirdio/netbird/idp/dex" "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/idp" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/management/status" ) @@ -88,14 +86,6 @@ func validateIdentityProviderConfig(ctx context.Context, idpConfig *types.Identi // GetIdentityProviders returns all identity providers for an account func (am *DefaultAccountManager) GetIdentityProviders(ctx context.Context, accountID, userID string) ([]*types.IdentityProvider, error) { - ok, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.IdentityProviders, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - embeddedManager, ok := am.idpManager.(*idp.EmbeddedIdPManager) if !ok { log.Warn("identity provider management requires embedded IdP") @@ -117,14 +107,6 @@ func (am *DefaultAccountManager) GetIdentityProviders(ctx context.Context, accou // GetIdentityProvider returns a specific identity provider by ID func (am *DefaultAccountManager) GetIdentityProvider(ctx context.Context, accountID, idpID, userID string) (*types.IdentityProvider, error) { - ok, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.IdentityProviders, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - embeddedManager, ok := am.idpManager.(*idp.EmbeddedIdPManager) if !ok { return nil, status.Errorf(status.Internal, "identity provider management requires embedded IdP") @@ -143,14 +125,6 @@ func (am *DefaultAccountManager) GetIdentityProvider(ctx context.Context, accoun // CreateIdentityProvider creates a new identity provider func (am *DefaultAccountManager) CreateIdentityProvider(ctx context.Context, accountID, userID string, idpConfig *types.IdentityProvider) (*types.IdentityProvider, error) { - ok, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.IdentityProviders, operations.Create) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - if err := validateIdentityProviderConfig(ctx, idpConfig); err != nil { return nil, err } @@ -168,7 +142,7 @@ func (am *DefaultAccountManager) CreateIdentityProvider(ctx context.Context, acc connCfg := identityProviderToConnectorConfig(idpConfig) - _, err = embeddedManager.CreateConnector(ctx, connCfg) + _, err := embeddedManager.CreateConnector(ctx, connCfg) if err != nil { return nil, status.Errorf(status.Internal, "failed to create identity provider: %v", err) } @@ -180,14 +154,6 @@ func (am *DefaultAccountManager) CreateIdentityProvider(ctx context.Context, acc // UpdateIdentityProvider updates an existing identity provider func (am *DefaultAccountManager) UpdateIdentityProvider(ctx context.Context, accountID, idpID, userID string, idpConfig *types.IdentityProvider) (*types.IdentityProvider, error) { - ok, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.IdentityProviders, operations.Update) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - if err := validateIdentityProviderConfig(ctx, idpConfig); err != nil { return nil, err } @@ -213,14 +179,6 @@ func (am *DefaultAccountManager) UpdateIdentityProvider(ctx context.Context, acc // DeleteIdentityProvider deletes an identity provider func (am *DefaultAccountManager) DeleteIdentityProvider(ctx context.Context, accountID, idpID, userID string) error { - ok, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.IdentityProviders, operations.Delete) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !ok { - return status.NewPermissionDeniedError() - } - embeddedManager, ok := am.idpManager.(*idp.EmbeddedIdPManager) if !ok { return status.Errorf(status.Internal, "identity provider management requires embedded IdP") diff --git a/management/server/identity_provider_test.go b/management/server/identity_provider_test.go index 9fce6b9c0..28468fac6 100644 --- a/management/server/identity_provider_test.go +++ b/management/server/identity_provider_test.go @@ -17,12 +17,12 @@ import ( "github.com/netbirdio/netbird/management/internals/controllers/network_map/update_channel" "github.com/netbirdio/netbird/management/internals/modules/peers" ephemeral_manager "github.com/netbirdio/netbird/management/internals/modules/peers/ephemeral/manager" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/internals/server/config" "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/idp" "github.com/netbirdio/netbird/management/server/integrations/port_forwarding" "github.com/netbirdio/netbird/management/server/job" - "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" diff --git a/management/server/management_proto_test.go b/management/server/management_proto_test.go index 090c99877..ffb7392d2 100644 --- a/management/server/management_proto_test.go +++ b/management/server/management_proto_test.go @@ -26,6 +26,7 @@ import ( "github.com/netbirdio/netbird/management/internals/controllers/network_map/update_channel" "github.com/netbirdio/netbird/management/internals/modules/peers" "github.com/netbirdio/netbird/management/internals/modules/peers/ephemeral/manager" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/internals/server/config" nbgrpc "github.com/netbirdio/netbird/management/internals/shared/grpc" "github.com/netbirdio/netbird/management/server/activity" @@ -33,7 +34,6 @@ import ( "github.com/netbirdio/netbird/management/server/integrations/port_forwarding" "github.com/netbirdio/netbird/management/server/job" 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" diff --git a/management/server/management_test.go b/management/server/management_test.go index de02855bf..cd8c98636 100644 --- a/management/server/management_test.go +++ b/management/server/management_test.go @@ -24,6 +24,7 @@ import ( "github.com/netbirdio/netbird/management/internals/controllers/network_map/update_channel" "github.com/netbirdio/netbird/management/internals/modules/peers" ephemeral_manager "github.com/netbirdio/netbird/management/internals/modules/peers/ephemeral/manager" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/internals/server/config" nbgrpc "github.com/netbirdio/netbird/management/internals/shared/grpc" "github.com/netbirdio/netbird/management/server" @@ -31,7 +32,6 @@ import ( "github.com/netbirdio/netbird/management/server/groups" "github.com/netbirdio/netbird/management/server/integrations/port_forwarding" "github.com/netbirdio/netbird/management/server/job" - "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" diff --git a/management/server/nameserver.go b/management/server/nameserver.go index 3d8c78912..3f4d97c32 100644 --- a/management/server/nameserver.go +++ b/management/server/nameserver.go @@ -11,8 +11,6 @@ import ( nbdns "github.com/netbirdio/netbird/dns" "github.com/netbirdio/netbird/management/server/activity" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" nbdomain "github.com/netbirdio/netbird/shared/management/domain" @@ -23,27 +21,11 @@ var errInvalidDomainName = errors.New("invalid domain name") // GetNameServerGroup gets a nameserver group object from account and nameserver group IDs func (am *DefaultAccountManager) GetNameServerGroup(ctx context.Context, accountID, userID, nsGroupID string) (*nbdns.NameServerGroup, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Nameservers, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - return am.Store.GetNameServerGroupByID(ctx, store.LockingStrengthNone, accountID, nsGroupID) } // CreateNameServerGroup creates and saves a new nameserver group func (am *DefaultAccountManager) CreateNameServerGroup(ctx context.Context, accountID string, name, description string, nameServerList []nbdns.NameServer, groups []string, primary bool, domains []string, enabled bool, userID string, searchDomainEnabled bool) (*nbdns.NameServerGroup, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Nameservers, operations.Create) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - newNSGroup := &nbdns.NameServerGroup{ ID: xid.New().String(), AccountID: accountID, @@ -59,7 +41,8 @@ func (am *DefaultAccountManager) CreateNameServerGroup(ctx context.Context, acco var updateAccountPeers bool - err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + err := am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + var err error if err = validateNameServerGroup(ctx, transaction, accountID, newNSGroup); err != nil { return err } @@ -94,17 +77,9 @@ func (am *DefaultAccountManager) SaveNameServerGroup(ctx context.Context, accoun return status.Errorf(status.InvalidArgument, "nameserver group provided is nil") } - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Nameservers, operations.Update) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - var updateAccountPeers bool - err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + err := am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { oldNSGroup, err := transaction.GetNameServerGroupByID(ctx, store.LockingStrengthNone, accountID, nsGroupToSave.ID) if err != nil { return err @@ -141,18 +116,11 @@ func (am *DefaultAccountManager) SaveNameServerGroup(ctx context.Context, accoun // DeleteNameServerGroup deletes nameserver group with nsGroupID func (am *DefaultAccountManager) DeleteNameServerGroup(ctx context.Context, accountID, nsGroupID, userID string) error { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Nameservers, operations.Delete) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - var nsGroup *nbdns.NameServerGroup var updateAccountPeers bool - err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + err := am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + var err error nsGroup, err = transaction.GetNameServerGroupByID(ctx, store.LockingStrengthUpdate, accountID, nsGroupID) if err != nil { return err @@ -184,14 +152,6 @@ func (am *DefaultAccountManager) DeleteNameServerGroup(ctx context.Context, acco // ListNameServerGroups returns a list of nameserver groups from account func (am *DefaultAccountManager) ListNameServerGroups(ctx context.Context, accountID string, userID string) ([]*nbdns.NameServerGroup, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Nameservers, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - return am.Store.GetAccountNameServerGroups(ctx, store.LockingStrengthNone, accountID) } diff --git a/management/server/nameserver_test.go b/management/server/nameserver_test.go index 90b4b9687..9bfbbe8d7 100644 --- a/management/server/nameserver_test.go +++ b/management/server/nameserver_test.go @@ -15,12 +15,12 @@ import ( "github.com/netbirdio/netbird/management/internals/controllers/network_map/update_channel" "github.com/netbirdio/netbird/management/internals/modules/peers" ephemeral_manager "github.com/netbirdio/netbird/management/internals/modules/peers/ephemeral/manager" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/internals/server/config" "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/integrations/port_forwarding" "github.com/netbirdio/netbird/management/server/job" 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" diff --git a/management/server/networks/manager.go b/management/server/networks/manager.go index b6706ca45..b23a0e410 100644 --- a/management/server/networks/manager.go +++ b/management/server/networks/manager.go @@ -6,16 +6,13 @@ import ( "github.com/rs/xid" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/server/account" "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/networks/resources" "github.com/netbirdio/netbird/management/server/networks/routers" "github.com/netbirdio/netbird/management/server/networks/types" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" - "github.com/netbirdio/netbird/shared/management/status" ) type Manager interface { @@ -48,29 +45,13 @@ func NewManager(store store.Store, permissionsManager permissions.Manager, resou } func (m *managerImpl) GetAllNetworks(ctx context.Context, accountID, userID string) ([]*types.Network, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Networks, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - return m.store.GetAccountNetworks(ctx, store.LockingStrengthNone, accountID) } func (m *managerImpl) CreateNetwork(ctx context.Context, userID string, network *types.Network) (*types.Network, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, network.AccountID, userID, modules.Networks, operations.Create) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - network.ID = xid.New().String() - err = m.store.SaveNetwork(ctx, network) + err := m.store.SaveNetwork(ctx, network) if err != nil { return nil, fmt.Errorf("failed to save network: %w", err) } @@ -81,27 +62,11 @@ func (m *managerImpl) CreateNetwork(ctx context.Context, userID string, network } func (m *managerImpl) GetNetwork(ctx context.Context, accountID, userID, networkID string) (*types.Network, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Networks, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - return m.store.GetNetworkByID(ctx, store.LockingStrengthNone, accountID, networkID) } func (m *managerImpl) UpdateNetwork(ctx context.Context, userID string, network *types.Network) (*types.Network, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, network.AccountID, userID, modules.Networks, operations.Update) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - - _, err = m.store.GetNetworkByID(ctx, store.LockingStrengthUpdate, network.AccountID, network.ID) + _, err := m.store.GetNetworkByID(ctx, store.LockingStrengthUpdate, network.AccountID, network.ID) if err != nil { return nil, fmt.Errorf("failed to get network: %w", err) } @@ -112,14 +77,6 @@ func (m *managerImpl) UpdateNetwork(ctx context.Context, userID string, network } func (m *managerImpl) DeleteNetwork(ctx context.Context, accountID, userID, networkID string) error { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Networks, operations.Delete) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !ok { - return status.NewPermissionDeniedError() - } - network, err := m.store.GetNetworkByID(ctx, store.LockingStrengthUpdate, accountID, networkID) if err != nil { return fmt.Errorf("failed to get network: %w", err) diff --git a/management/server/networks/manager_test.go b/management/server/networks/manager_test.go index 6fb19d157..0af59d2a0 100644 --- a/management/server/networks/manager_test.go +++ b/management/server/networks/manager_test.go @@ -6,12 +6,12 @@ import ( "github.com/stretchr/testify/require" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/server/groups" "github.com/netbirdio/netbird/management/server/mock_server" "github.com/netbirdio/netbird/management/server/networks/resources" "github.com/netbirdio/netbird/management/server/networks/routers" "github.com/netbirdio/netbird/management/server/networks/types" - "github.com/netbirdio/netbird/management/server/permissions" "github.com/netbirdio/netbird/management/server/store" ) diff --git a/management/server/networks/resources/manager.go b/management/server/networks/resources/manager.go index 86f9b6579..5a9ea442b 100644 --- a/management/server/networks/resources/manager.go +++ b/management/server/networks/resources/manager.go @@ -7,14 +7,12 @@ import ( log "github.com/sirupsen/logrus" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service" "github.com/netbirdio/netbird/management/server/account" "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/groups" "github.com/netbirdio/netbird/management/server/networks/resources/types" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" nbtypes "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/management/server/util" @@ -54,38 +52,14 @@ func NewManager(store store.Store, permissionsManager permissions.Manager, group } func (m *managerImpl) GetAllResourcesInNetwork(ctx context.Context, accountID, userID, networkID string) ([]*types.NetworkResource, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Networks, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - return m.store.GetNetworkResourcesByNetID(ctx, store.LockingStrengthNone, accountID, networkID) } func (m *managerImpl) GetAllResourcesInAccount(ctx context.Context, accountID, userID string) ([]*types.NetworkResource, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Networks, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - return m.store.GetNetworkResourcesByAccountID(ctx, store.LockingStrengthNone, accountID) } func (m *managerImpl) GetAllResourceIDsInAccount(ctx context.Context, accountID, userID string) (map[string][]string, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Networks, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - resources, err := m.store.GetNetworkResourcesByAccountID(ctx, store.LockingStrengthNone, accountID) if err != nil { return nil, fmt.Errorf("failed to get network resources: %w", err) @@ -100,15 +74,7 @@ func (m *managerImpl) GetAllResourceIDsInAccount(ctx context.Context, accountID, } func (m *managerImpl) CreateResource(ctx context.Context, userID string, resource *types.NetworkResource) (*types.NetworkResource, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, resource.AccountID, userID, modules.Networks, operations.Create) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - - resource, err = types.NewNetworkResource(resource.AccountID, resource.NetworkID, resource.Name, resource.Description, resource.Address, resource.GroupIDs, resource.Enabled) + resource, err := types.NewNetworkResource(resource.AccountID, resource.NetworkID, resource.Name, resource.Description, resource.Address, resource.GroupIDs, resource.Enabled) if err != nil { return nil, fmt.Errorf("failed to create new network resource: %w", err) } @@ -168,14 +134,6 @@ func (m *managerImpl) CreateResource(ctx context.Context, userID string, resourc } func (m *managerImpl) GetResource(ctx context.Context, accountID, userID, networkID, resourceID string) (*types.NetworkResource, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Networks, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - resource, err := m.store.GetNetworkResourceByID(ctx, store.LockingStrengthNone, accountID, resourceID) if err != nil { return nil, fmt.Errorf("failed to get network resource: %w", err) @@ -189,14 +147,6 @@ func (m *managerImpl) GetResource(ctx context.Context, accountID, userID, networ } func (m *managerImpl) UpdateResource(ctx context.Context, userID string, resource *types.NetworkResource) (*types.NetworkResource, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, resource.AccountID, userID, modules.Networks, operations.Update) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - resourceType, domain, prefix, err := types.GetResourceType(resource.Address) if err != nil { return nil, fmt.Errorf("failed to get resource type: %w", err) @@ -314,14 +264,6 @@ func (m *managerImpl) updateResourceGroups(ctx context.Context, transaction stor } func (m *managerImpl) DeleteResource(ctx context.Context, accountID, userID, networkID, resourceID string) error { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Networks, operations.Delete) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !ok { - return status.NewPermissionDeniedError() - } - serviceID, err := m.serviceManager.GetServiceIDByTargetID(ctx, accountID, resourceID) if err != nil { return fmt.Errorf("failed to check if resource is used by service: %w", err) diff --git a/management/server/networks/resources/manager_test.go b/management/server/networks/resources/manager_test.go index c6d8e7bcc..b5e94bf89 100644 --- a/management/server/networks/resources/manager_test.go +++ b/management/server/networks/resources/manager_test.go @@ -7,11 +7,11 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" + "github.com/netbirdio/netbird/management/internals/modules/permissions" reverseproxy "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service" "github.com/netbirdio/netbird/management/server/groups" "github.com/netbirdio/netbird/management/server/mock_server" "github.com/netbirdio/netbird/management/server/networks/resources/types" - "github.com/netbirdio/netbird/management/server/permissions" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/shared/management/status" ) diff --git a/management/server/networks/routers/manager.go b/management/server/networks/routers/manager.go index 82cac424a..9864d7def 100644 --- a/management/server/networks/routers/manager.go +++ b/management/server/networks/routers/manager.go @@ -7,13 +7,11 @@ import ( "github.com/rs/xid" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/server/account" "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/networks/routers/types" networkTypes "github.com/netbirdio/netbird/management/server/networks/types" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/shared/management/status" ) @@ -46,26 +44,10 @@ func NewManager(store store.Store, permissionsManager permissions.Manager, accou } func (m *managerImpl) GetAllRoutersInNetwork(ctx context.Context, accountID, userID, networkID string) ([]*types.NetworkRouter, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Networks, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - return m.store.GetNetworkRoutersByNetID(ctx, store.LockingStrengthNone, accountID, networkID) } func (m *managerImpl) GetAllRoutersInAccount(ctx context.Context, accountID, userID string) (map[string][]*types.NetworkRouter, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Networks, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - routers, err := m.store.GetNetworkRoutersByAccountID(ctx, store.LockingStrengthNone, accountID) if err != nil { return nil, fmt.Errorf("failed to get network routers: %w", err) @@ -80,16 +62,9 @@ func (m *managerImpl) GetAllRoutersInAccount(ctx context.Context, accountID, use } func (m *managerImpl) CreateRouter(ctx context.Context, userID string, router *types.NetworkRouter) (*types.NetworkRouter, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, router.AccountID, userID, modules.Networks, operations.Create) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - var network *networkTypes.Network - err = m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + err := m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + var err error network, err = transaction.GetNetworkByID(ctx, store.LockingStrengthNone, router.AccountID, router.NetworkID) if err != nil { return fmt.Errorf("failed to get network: %w", err) @@ -125,14 +100,6 @@ func (m *managerImpl) CreateRouter(ctx context.Context, userID string, router *t } func (m *managerImpl) GetRouter(ctx context.Context, accountID, userID, networkID, routerID string) (*types.NetworkRouter, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Networks, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - router, err := m.store.GetNetworkRouterByID(ctx, store.LockingStrengthNone, accountID, routerID) if err != nil { return nil, fmt.Errorf("failed to get network router: %w", err) @@ -146,16 +113,9 @@ func (m *managerImpl) GetRouter(ctx context.Context, accountID, userID, networkI } func (m *managerImpl) UpdateRouter(ctx context.Context, userID string, router *types.NetworkRouter) (*types.NetworkRouter, error) { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, router.AccountID, userID, modules.Networks, operations.Update) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - var network *networkTypes.Network - err = m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + err := m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + var err error network, err = transaction.GetNetworkByID(ctx, store.LockingStrengthNone, router.AccountID, router.NetworkID) if err != nil { return fmt.Errorf("failed to get network: %w", err) @@ -189,16 +149,9 @@ func (m *managerImpl) UpdateRouter(ctx context.Context, userID string, router *t } func (m *managerImpl) DeleteRouter(ctx context.Context, accountID, userID, networkID, routerID string) error { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Networks, operations.Delete) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !ok { - return status.NewPermissionDeniedError() - } - var event func() - err = m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + err := m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + var err error event, err = m.DeleteRouterInTransaction(ctx, transaction, accountID, userID, networkID, routerID) if err != nil { return fmt.Errorf("failed to delete network router: %w", err) diff --git a/management/server/networks/routers/manager_test.go b/management/server/networks/routers/manager_test.go index 6be90baa7..ed24ad8f7 100644 --- a/management/server/networks/routers/manager_test.go +++ b/management/server/networks/routers/manager_test.go @@ -6,9 +6,9 @@ import ( "github.com/stretchr/testify/require" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/server/mock_server" "github.com/netbirdio/netbird/management/server/networks/routers/types" - "github.com/netbirdio/netbird/management/server/permissions" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/shared/management/status" ) diff --git a/management/server/peer.go b/management/server/peer.go index 78ecbfcae..3079a7095 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -18,8 +18,6 @@ import ( "github.com/netbirdio/netbird/management/server/geolocation" "github.com/netbirdio/netbird/management/server/idp" routerTypes "github.com/netbirdio/netbird/management/server/networks/routers/types" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/shared/management/domain" "github.com/netbirdio/netbird/management/server/posture" @@ -41,21 +39,11 @@ func (am *DefaultAccountManager) GetPeers(ctx context.Context, accountID, userID return nil, err } - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Peers, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - accountPeers, err := am.Store.GetAccountPeers(ctx, store.LockingStrengthNone, accountID, nameFilter, ipFilter) if err != nil { return nil, err } - // @note if the user has permission to read peers it shows all account peers - if allowed { - return accountPeers, nil - } - settings, err := am.Store.GetAccountSettings(ctx, store.LockingStrengthNone, accountID) if err != nil { return nil, fmt.Errorf("failed to get account settings: %w", err) @@ -198,15 +186,8 @@ func updatePeerStatusAndLocation(ctx context.Context, geo geolocation.Geolocatio // UpdatePeer updates peer. Only Peer.Name, Peer.SSHEnabled, Peer.LoginExpirationEnabled and Peer.InactivityExpirationEnabled can be updated. func (am *DefaultAccountManager) UpdatePeer(ctx context.Context, accountID, userID string, update *nbpeer.Peer) (*nbpeer.Peer, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Peers, operations.Update) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - var peer *nbpeer.Peer + var err error var settings *types.Settings var peerGroupList []string var peerLabelChanged bool @@ -343,14 +324,6 @@ func (am *DefaultAccountManager) UpdatePeer(ctx context.Context, accountID, user } func (am *DefaultAccountManager) CreatePeerJob(ctx context.Context, accountID, peerID, userID string, job *types.Job) error { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.RemoteJobs, operations.Create) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - p, err := am.Store.GetPeerByID(ctx, store.LockingStrengthNone, accountID, peerID) if err != nil { return err @@ -418,15 +391,6 @@ func (am *DefaultAccountManager) CreatePeerJob(ctx context.Context, accountID, p } func (am *DefaultAccountManager) GetAllPeerJobs(ctx context.Context, accountID, userID, peerID string) ([]*types.Job, error) { - // todo: Create permissions for job - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.RemoteJobs, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - peerAccountID, err := am.Store.GetAccountIDByPeerID(ctx, store.LockingStrengthNone, peerID) if err != nil { return nil, err @@ -445,14 +409,6 @@ func (am *DefaultAccountManager) GetAllPeerJobs(ctx context.Context, accountID, } func (am *DefaultAccountManager) GetPeerJobByID(ctx context.Context, accountID, userID, peerID, jobID string) (*types.Job, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.RemoteJobs, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - peerAccountID, err := am.Store.GetAccountIDByPeerID(ctx, store.LockingStrengthNone, peerID) if err != nil { return nil, err @@ -472,14 +428,6 @@ func (am *DefaultAccountManager) GetPeerJobByID(ctx context.Context, accountID, // DeletePeer removes peer from the account by its IP func (am *DefaultAccountManager) DeletePeer(ctx context.Context, accountID, peerID, userID string) error { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Peers, operations.Delete) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - peerAccountID, err := am.Store.GetAccountIDByPeerID(ctx, store.LockingStrengthNone, peerID) if err != nil { return err @@ -610,15 +558,7 @@ func (am *DefaultAccountManager) handleUserAddedPeer(ctx context.Context, accoun return status.Errorf(status.PermissionDenied, "user pending approval cannot add peers") } - if temporary { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Peers, operations.Create) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - } else { + if !temporary { config.AccountID = user.AccountID config.GroupsToAdd = user.AutoGroups } @@ -1235,14 +1175,6 @@ func (am *DefaultAccountManager) GetPeer(ctx context.Context, accountID, peerID, return nil, err } - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Peers, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if allowed { - return peer, nil - } - user, err := am.Store.GetUserByUserID(ctx, store.LockingStrengthNone, userID) if err != nil { return nil, err diff --git a/management/server/peer_test.go b/management/server/peer_test.go index b17757ffd..9810d82a7 100644 --- a/management/server/peer_test.go +++ b/management/server/peer_test.go @@ -30,12 +30,12 @@ import ( "github.com/netbirdio/netbird/management/internals/controllers/network_map/update_channel" "github.com/netbirdio/netbird/management/internals/modules/peers" ephemeral_manager "github.com/netbirdio/netbird/management/internals/modules/peers/ephemeral/manager" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/internals/server/config" "github.com/netbirdio/netbird/management/internals/shared/grpc" "github.com/netbirdio/netbird/management/server/http/testing/testing_tools" "github.com/netbirdio/netbird/management/server/integrations/port_forwarding" "github.com/netbirdio/netbird/management/server/job" - "github.com/netbirdio/netbird/management/server/permissions" "github.com/netbirdio/netbird/management/server/settings" "github.com/netbirdio/netbird/shared/management/status" diff --git a/management/server/policy.go b/management/server/policy.go index 3e84c3d10..5cc641c45 100644 --- a/management/server/policy.go +++ b/management/server/policy.go @@ -6,8 +6,6 @@ import ( "github.com/rs/xid" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" @@ -18,32 +16,13 @@ import ( // GetPolicy from the store func (am *DefaultAccountManager) GetPolicy(ctx context.Context, accountID, policyID, userID string) (*types.Policy, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Policies, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - return am.Store.GetPolicyByID(ctx, store.LockingStrengthNone, accountID, policyID) } // SavePolicy in the store func (am *DefaultAccountManager) SavePolicy(ctx context.Context, accountID, userID string, policy *types.Policy, create bool) (*types.Policy, error) { - operation := operations.Create - if !create { - operation = operations.Update - } - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Policies, operation) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - var isUpdate = policy.ID != "" + var err error var updateAccountPeers bool var action = activity.PolicyAdded @@ -84,16 +63,9 @@ func (am *DefaultAccountManager) SavePolicy(ctx context.Context, accountID, user // DeletePolicy from the store func (am *DefaultAccountManager) DeletePolicy(ctx context.Context, accountID, policyID, userID string) error { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Policies, operations.Delete) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - var policy *types.Policy var updateAccountPeers bool + var err error err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { policy, err = transaction.GetPolicyByID(ctx, store.LockingStrengthUpdate, accountID, policyID) @@ -127,14 +99,6 @@ func (am *DefaultAccountManager) DeletePolicy(ctx context.Context, accountID, po // ListPolicies from the store. func (am *DefaultAccountManager) ListPolicies(ctx context.Context, accountID, userID string) ([]*types.Policy, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Policies, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - return am.Store.GetAccountPolicies(ctx, store.LockingStrengthNone, accountID) } diff --git a/management/server/posture_checks.go b/management/server/posture_checks.go index ba901c771..6220cb322 100644 --- a/management/server/posture_checks.go +++ b/management/server/posture_checks.go @@ -7,40 +7,19 @@ import ( "github.com/rs/xid" "github.com/netbirdio/netbird/management/server/activity" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/posture" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/shared/management/status" ) func (am *DefaultAccountManager) GetPostureChecks(ctx context.Context, accountID, postureChecksID, userID string) (*posture.Checks, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Policies, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - return am.Store.GetPostureChecksByID(ctx, store.LockingStrengthNone, accountID, postureChecksID) } // SavePostureChecks saves a posture check. func (am *DefaultAccountManager) SavePostureChecks(ctx context.Context, accountID, userID string, postureChecks *posture.Checks, create bool) (*posture.Checks, error) { - operation := operations.Create - if !create { - operation = operations.Update - } - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Policies, operation) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - var updateAccountPeers bool + var err error var isUpdate = postureChecks.ID != "" var action = activity.PostureCheckCreated @@ -84,15 +63,8 @@ func (am *DefaultAccountManager) SavePostureChecks(ctx context.Context, accountI // DeletePostureChecks deletes a posture check by ID. func (am *DefaultAccountManager) DeletePostureChecks(ctx context.Context, accountID, postureChecksID, userID string) error { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Routes, operations.Read) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - var postureChecks *posture.Checks + var err error err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { postureChecks, err = transaction.GetPostureChecksByID(ctx, store.LockingStrengthNone, accountID, postureChecksID) @@ -121,14 +93,6 @@ func (am *DefaultAccountManager) DeletePostureChecks(ctx context.Context, accoun // ListPostureChecks returns a list of posture checks. func (am *DefaultAccountManager) ListPostureChecks(ctx context.Context, accountID, userID string) ([]*posture.Checks, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Policies, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - return am.Store.GetAccountPostureChecks(ctx, store.LockingStrengthNone, accountID) } diff --git a/management/server/route.go b/management/server/route.go index 2b4f11d05..64ff4ee91 100644 --- a/management/server/route.go +++ b/management/server/route.go @@ -10,8 +10,6 @@ import ( "github.com/rs/xid" "github.com/netbirdio/netbird/management/server/activity" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/route" @@ -21,14 +19,6 @@ import ( // GetRoute gets a route object from account and route IDs func (am *DefaultAccountManager) GetRoute(ctx context.Context, accountID string, routeID route.ID, userID string) (*route.Route, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Routes, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - return am.Store.GetRouteByID(ctx, store.LockingStrengthNone, accountID, string(routeID)) } @@ -134,20 +124,13 @@ func getRouteDescriptor(prefix netip.Prefix, domains domain.List) string { // CreateRoute creates and saves a new route func (am *DefaultAccountManager) CreateRoute(ctx context.Context, accountID string, prefix netip.Prefix, networkType route.NetworkType, domains domain.List, peerID string, peerGroupIDs []string, description string, netID route.NetID, masquerade bool, metric int, groups, accessControlGroupIDs []string, enabled bool, userID string, keepRoute bool, skipAutoApply bool) (*route.Route, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Routes, operations.Create) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - if len(domains) > 0 && prefix.IsValid() { return nil, status.Errorf(status.InvalidArgument, "domains and network should not be provided at the same time") } var newRoute *route.Route var updateAccountPeers bool + var err error err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { newRoute = &route.Route{ @@ -199,15 +182,8 @@ func (am *DefaultAccountManager) CreateRoute(ctx context.Context, accountID stri // SaveRoute saves route func (am *DefaultAccountManager) SaveRoute(ctx context.Context, accountID, userID string, routeToSave *route.Route) error { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Routes, operations.Update) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - var oldRoute *route.Route + var err error var oldRouteAffectsPeers bool var newRouteAffectsPeers bool @@ -253,16 +229,9 @@ func (am *DefaultAccountManager) SaveRoute(ctx context.Context, accountID, userI // DeleteRoute deletes route with routeID func (am *DefaultAccountManager) DeleteRoute(ctx context.Context, accountID string, routeID route.ID, userID string) error { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Routes, operations.Delete) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - var route *route.Route var updateAccountPeers bool + var err error err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { route, err = transaction.GetRouteByID(ctx, store.LockingStrengthUpdate, accountID, string(routeID)) @@ -296,14 +265,6 @@ func (am *DefaultAccountManager) DeleteRoute(ctx context.Context, accountID stri // ListRoutes returns a list of routes from account func (am *DefaultAccountManager) ListRoutes(ctx context.Context, accountID, userID string) ([]*route.Route, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Routes, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - return am.Store.GetAccountRoutes(ctx, store.LockingStrengthNone, accountID) } diff --git a/management/server/route_test.go b/management/server/route_test.go index d4882eff8..aefdbd5c5 100644 --- a/management/server/route_test.go +++ b/management/server/route_test.go @@ -18,6 +18,7 @@ import ( "github.com/netbirdio/netbird/management/internals/controllers/network_map/update_channel" "github.com/netbirdio/netbird/management/internals/modules/peers" ephemeral_manager "github.com/netbirdio/netbird/management/internals/modules/peers/ephemeral/manager" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/internals/server/config" "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/integrations/port_forwarding" @@ -26,7 +27,6 @@ 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" diff --git a/management/server/settings/manager.go b/management/server/settings/manager.go index 74af0a3ef..ef8b9c95d 100644 --- a/management/server/settings/manager.go +++ b/management/server/settings/manager.go @@ -6,15 +6,10 @@ import ( "context" "fmt" - "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/integrations/extra_settings" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/management/server/users" - "github.com/netbirdio/netbird/shared/management/status" ) type Manager interface { @@ -35,16 +30,14 @@ type managerImpl struct { store store.Store extraSettingsManager extra_settings.Manager userManager users.Manager - permissionsManager permissions.Manager idpConfig IdpConfig } -func NewManager(store store.Store, userManager users.Manager, extraSettingsManager extra_settings.Manager, permissionsManager permissions.Manager, idpConfig IdpConfig) Manager { +func NewManager(store store.Store, userManager users.Manager, extraSettingsManager extra_settings.Manager, idpConfig IdpConfig) Manager { return &managerImpl{ store: store, extraSettingsManager: extraSettingsManager, userManager: userManager, - permissionsManager: permissionsManager, idpConfig: idpConfig, } } @@ -54,16 +47,6 @@ func (m *managerImpl) GetExtraSettingsManager() extra_settings.Manager { } func (m *managerImpl) GetSettings(ctx context.Context, accountID, userID string) (*types.Settings, error) { - if userID != activity.SystemInitiator { - ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Settings, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !ok { - return nil, status.NewPermissionDeniedError() - } - } - extraSettings, err := m.extraSettingsManager.GetExtraSettings(ctx, accountID) if err != nil { return nil, fmt.Errorf("get extra settings: %w", err) diff --git a/management/server/setupkey.go b/management/server/setupkey.go index 8d0509871..bde39cff2 100644 --- a/management/server/setupkey.go +++ b/management/server/setupkey.go @@ -8,8 +8,6 @@ import ( log "github.com/sirupsen/logrus" "github.com/netbirdio/netbird/management/server/activity" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/management/server/util" @@ -56,19 +54,12 @@ type SetupKeyUpdateOperation struct { func (am *DefaultAccountManager) CreateSetupKey(ctx context.Context, accountID string, keyName string, keyType types.SetupKeyType, expiresIn time.Duration, autoGroups []string, usageLimit int, userID string, ephemeral bool, allowExtraDNSLabels bool) (*types.SetupKey, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.SetupKeys, operations.Create) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - var setupKey *types.SetupKey var plainKey string var eventsToStore []func() - err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + err := am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + var err error if err = validateSetupKeyAutoGroups(ctx, transaction, accountID, autoGroups); err != nil { return status.Errorf(status.InvalidArgument, "invalid auto groups: %v", err) } @@ -105,19 +96,12 @@ func (am *DefaultAccountManager) SaveSetupKey(ctx context.Context, accountID str return nil, status.Errorf(status.InvalidArgument, "provided setup key to update is nil") } - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.SetupKeys, operations.Update) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - var oldKey *types.SetupKey var newKey *types.SetupKey var eventsToStore []func() - err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + err := am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + var err error if err = validateSetupKeyAutoGroups(ctx, transaction, accountID, keyToSave.AutoGroups); err != nil { return status.Errorf(status.InvalidArgument, "invalid auto groups: %v", err) } @@ -162,27 +146,11 @@ func (am *DefaultAccountManager) SaveSetupKey(ctx context.Context, accountID str // ListSetupKeys returns a list of all setup keys of the account func (am *DefaultAccountManager) ListSetupKeys(ctx context.Context, accountID, userID string) ([]*types.SetupKey, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.SetupKeys, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - return am.Store.GetAccountSetupKeys(ctx, store.LockingStrengthNone, accountID) } // GetSetupKey looks up a SetupKey by KeyID, returns NotFound error if not found. func (am *DefaultAccountManager) GetSetupKey(ctx context.Context, accountID, userID, keyID string) (*types.SetupKey, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.SetupKeys, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - setupKey, err := am.Store.GetSetupKeyByID(ctx, store.LockingStrengthNone, accountID, keyID) if err != nil { return nil, err @@ -198,17 +166,10 @@ func (am *DefaultAccountManager) GetSetupKey(ctx context.Context, accountID, use // DeleteSetupKey removes the setup key from the account func (am *DefaultAccountManager) DeleteSetupKey(ctx context.Context, accountID, userID, keyID string) error { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.SetupKeys, operations.Delete) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - var deletedSetupKey *types.SetupKey - err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + err := am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + var err error deletedSetupKey, err = transaction.GetSetupKeyByID(ctx, store.LockingStrengthUpdate, accountID, keyID) if err != nil { return err diff --git a/management/server/user.go b/management/server/user.go index 327aec2d0..98c38433a 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -18,8 +18,6 @@ import ( "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/idp" nbpeer "github.com/netbirdio/netbird/management/server/peer" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/management/server/users" @@ -29,14 +27,6 @@ import ( // createServiceUser creates a new service user under the given account. func (am *DefaultAccountManager) createServiceUser(ctx context.Context, accountID string, initiatorUserID string, role types.UserRole, serviceUserName string, nonDeletable bool, autoGroups []string) (*types.UserInfo, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, initiatorUserID, modules.Users, operations.Create) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - if role == types.UserRoleOwner { return nil, status.NewServiceUserRoleInvalidError() } @@ -46,7 +36,7 @@ func (am *DefaultAccountManager) createServiceUser(ctx context.Context, accountI newUser.AccountID = accountID log.WithContext(ctx).Debugf("New User: %v", newUser) - if err = am.Store.SaveUser(ctx, newUser); err != nil { + if err := am.Store.SaveUser(ctx, newUser); err != nil { return nil, err } @@ -84,14 +74,6 @@ func (am *DefaultAccountManager) inviteNewUser(ctx context.Context, accountID, u return nil, err } - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Users, operations.Create) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - initiatorUser, err := am.Store.GetUserByUserID(ctx, store.LockingStrengthNone, userID) if err != nil { return nil, err @@ -305,14 +287,6 @@ func (am *DefaultAccountManager) DeleteUser(ctx context.Context, accountID, init return err } - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, initiatorUserID, modules.Users, operations.Delete) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - targetUser, err := am.Store.GetUserByUserID(ctx, store.LockingStrengthNone, targetUserID) if err != nil { return err @@ -355,14 +329,6 @@ func (am *DefaultAccountManager) InviteUser(ctx context.Context, accountID strin return status.Errorf(status.PreconditionFailed, "IdP manager must be enabled to send user invites") } - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, initiatorUserID, modules.Users, operations.Create) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - // check if the user is already registered with this ID user, err := am.lookupUserInCache(ctx, targetUserID, accountID) if err != nil { @@ -399,14 +365,6 @@ func (am *DefaultAccountManager) CreatePAT(ctx context.Context, accountID string return nil, status.Errorf(status.InvalidArgument, "expiration has to be between 1 and 365") } - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, initiatorUserID, modules.Pats, operations.Create) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - initiatorUser, err := am.Store.GetUserByUserID(ctx, store.LockingStrengthNone, initiatorUserID) if err != nil { return nil, err @@ -439,14 +397,6 @@ func (am *DefaultAccountManager) CreatePAT(ctx context.Context, accountID string // DeletePAT deletes a specific PAT from a user func (am *DefaultAccountManager) DeletePAT(ctx context.Context, accountID string, initiatorUserID string, targetUserID string, tokenID string) error { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, initiatorUserID, modules.Pats, operations.Delete) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - initiatorUser, err := am.Store.GetUserByUserID(ctx, store.LockingStrengthNone, initiatorUserID) if err != nil { return err @@ -478,14 +428,6 @@ func (am *DefaultAccountManager) DeletePAT(ctx context.Context, accountID string // GetPAT returns a specific PAT from a user func (am *DefaultAccountManager) GetPAT(ctx context.Context, accountID string, initiatorUserID string, targetUserID string, tokenID string) (*types.PersonalAccessToken, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, initiatorUserID, modules.Pats, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - initiatorUser, err := am.Store.GetUserByUserID(ctx, store.LockingStrengthNone, initiatorUserID) if err != nil { return nil, err @@ -505,14 +447,6 @@ func (am *DefaultAccountManager) GetPAT(ctx context.Context, accountID string, i // GetAllPATs returns all PATs for a user func (am *DefaultAccountManager) GetAllPATs(ctx context.Context, accountID string, initiatorUserID string, targetUserID string) ([]*types.PersonalAccessToken, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, initiatorUserID, modules.Pats, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - initiatorUser, err := am.Store.GetUserByUserID(ctx, store.LockingStrengthNone, initiatorUserID) if err != nil { return nil, err @@ -558,13 +492,6 @@ func (am *DefaultAccountManager) SaveOrAddUsers(ctx context.Context, accountID, return nil, nil //nolint:nilnil } - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, initiatorUserID, modules.Users, operations.Create) // TODO: split by Create and Update - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } settings, err := am.Store.GetAccountSettings(ctx, store.LockingStrengthNone, accountID) if err != nil { return nil, err @@ -955,12 +882,8 @@ func (am *DefaultAccountManager) GetOrCreateAccountByUser(ctx context.Context, u // GetUsersFromAccount performs a batched request for users from IDP by account ID apply filter on what data to return // based on provided user role. func (am *DefaultAccountManager) GetUsersFromAccount(ctx context.Context, accountID, initiatorUserID string) (map[string]*types.UserInfo, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, initiatorUserID, modules.Users, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - var user *types.User + var err error if initiatorUserID != activity.SystemInitiator { result, err := am.Store.GetUserByUserID(ctx, store.LockingStrengthNone, initiatorUserID) if err != nil { @@ -969,9 +892,16 @@ func (am *DefaultAccountManager) GetUsersFromAccount(ctx context.Context, accoun user = result } + // Permission checks are now handled by the HTTP middleware via WithPermission wrapper + // This internal method is called from authenticated/authorized handlers accountUsers := []*types.User{} + + // Determine if user has full access based on their role + // Service users and regular users have limited visibility + hasFullAccess := user.HasAdminPower() + switch { - case allowed: + case hasFullAccess: start := time.Now() accountUsers, err = am.Store.GetAccountUsers(ctx, store.LockingStrengthNone, accountID) if err != nil { @@ -1172,14 +1102,6 @@ func (am *DefaultAccountManager) deleteUserFromIDP(ctx context.Context, targetUs // If an error occurs while deleting the user, the function skips it and continues deleting other users. // Errors are collected and returned at the end. func (am *DefaultAccountManager) DeleteRegularUsers(ctx context.Context, accountID, initiatorUserID string, targetUserIDs []string, userInfos map[string]*types.UserInfo) error { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, initiatorUserID, modules.Users, operations.Delete) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - initiatorUser, err := am.Store.GetUserByUserID(ctx, store.LockingStrengthNone, initiatorUserID) if err != nil { return err @@ -1370,9 +1292,8 @@ func (am *DefaultAccountManager) GetCurrentUserInfo(ctx context.Context, userAut return nil, status.NewPermissionDeniedError() } - if err := am.permissionsManager.ValidateAccountAccess(ctx, accountID, user, false); err != nil { - return nil, err - } + // Permission checks are now handled by the HTTP middleware via WithPermission wrapper + // User account association is already validated above by GetUserByUserID settings, err := am.Store.GetAccountSettings(ctx, store.LockingStrengthNone, accountID) if err != nil { @@ -1399,14 +1320,6 @@ func (am *DefaultAccountManager) GetCurrentUserInfo(ctx context.Context, userAut // ApproveUser approves a user that is pending approval func (am *DefaultAccountManager) ApproveUser(ctx context.Context, accountID, initiatorUserID, targetUserID string) (*types.UserInfo, error) { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, initiatorUserID, modules.Users, operations.Update) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - user, err := am.Store.GetUserByUserID(ctx, store.LockingStrengthNone, targetUserID) if err != nil { return nil, err @@ -1440,14 +1353,6 @@ func (am *DefaultAccountManager) ApproveUser(ctx context.Context, accountID, ini // RejectUser rejects a user that is pending approval by deleting them func (am *DefaultAccountManager) RejectUser(ctx context.Context, accountID, initiatorUserID, targetUserID string) error { - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, initiatorUserID, modules.Users, operations.Delete) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - user, err := am.Store.GetUserByUserID(ctx, store.LockingStrengthNone, targetUserID) if err != nil { return err @@ -1486,14 +1391,6 @@ func (am *DefaultAccountManager) CreateUserInvite(ctx context.Context, accountID return nil, err } - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, initiatorUserID, modules.Users, operations.Create) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - // Check if user already exists in NetBird DB existingUsers, err := am.Store.GetAccountUsers(ctx, store.LockingStrengthNone, accountID) if err != nil { @@ -1604,14 +1501,6 @@ func (am *DefaultAccountManager) ListUserInvites(ctx context.Context, accountID, return nil, status.Errorf(status.PreconditionFailed, "invite links are only available with embedded identity provider") } - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, initiatorUserID, modules.Users, operations.Read) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - records, err := am.Store.GetAccountUserInvites(ctx, store.LockingStrengthNone, accountID) if err != nil { return nil, err @@ -1718,14 +1607,6 @@ func (am *DefaultAccountManager) RegenerateUserInvite(ctx context.Context, accou return nil, status.Errorf(status.PreconditionFailed, "invite links are only available with embedded identity provider") } - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, initiatorUserID, modules.Users, operations.Update) - if err != nil { - return nil, status.NewPermissionValidationError(err) - } - if !allowed { - return nil, status.NewPermissionDeniedError() - } - // Get existing invite existingInvite, err := am.Store.GetUserInviteByID(ctx, store.LockingStrengthUpdate, accountID, inviteID) if err != nil { @@ -1780,14 +1661,6 @@ func (am *DefaultAccountManager) DeleteUserInvite(ctx context.Context, accountID return status.Errorf(status.PreconditionFailed, "invite links are only available with embedded identity provider") } - allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, initiatorUserID, modules.Users, operations.Delete) - if err != nil { - return status.NewPermissionValidationError(err) - } - if !allowed { - return status.NewPermissionDeniedError() - } - invite, err := am.Store.GetUserInviteByID(ctx, store.LockingStrengthUpdate, accountID, inviteID) if err != nil { return err diff --git a/management/server/user_invite_test.go b/management/server/user_invite_test.go index 6256ed44a..039bf00c0 100644 --- a/management/server/user_invite_test.go +++ b/management/server/user_invite_test.go @@ -9,9 +9,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/idp" - "github.com/netbirdio/netbird/management/server/permissions" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/management/status" diff --git a/management/server/user_test.go b/management/server/user_test.go index 800d2406c..275842997 100644 --- a/management/server/user_test.go +++ b/management/server/user_test.go @@ -13,10 +13,10 @@ import ( "golang.org/x/exp/maps" "github.com/netbirdio/netbird/management/internals/controllers/network_map" + "github.com/netbirdio/netbird/management/internals/modules/permissions" + "github.com/netbirdio/netbird/management/internals/modules/permissions/modules" + roles2 "github.com/netbirdio/netbird/management/internals/modules/permissions/roles" nbcache "github.com/netbirdio/netbird/management/server/cache" - "github.com/netbirdio/netbird/management/server/permissions" - "github.com/netbirdio/netbird/management/server/permissions/modules" - "github.com/netbirdio/netbird/management/server/permissions/roles" "github.com/netbirdio/netbird/management/server/users" "github.com/netbirdio/netbird/management/server/util" "github.com/netbirdio/netbird/shared/auth" @@ -1639,7 +1639,7 @@ func TestDefaultAccountManager_GetCurrentUserInfo(t *testing.T) { Issued: "api", IntegrationReference: integration_reference.IntegrationReference{}, }, - Permissions: mergeRolePermissions(roles.Owner), + Permissions: mergeRolePermissions(roles2.Owner), }, }, { @@ -1658,7 +1658,7 @@ func TestDefaultAccountManager_GetCurrentUserInfo(t *testing.T) { Issued: "api", IntegrationReference: integration_reference.IntegrationReference{}, }, - Permissions: mergeRolePermissions(roles.User), + Permissions: mergeRolePermissions(roles2.User), }, }, { @@ -1677,7 +1677,7 @@ func TestDefaultAccountManager_GetCurrentUserInfo(t *testing.T) { Issued: "api", IntegrationReference: integration_reference.IntegrationReference{}, }, - Permissions: mergeRolePermissions(roles.Admin), + Permissions: mergeRolePermissions(roles2.Admin), }, }, { @@ -1696,7 +1696,7 @@ func TestDefaultAccountManager_GetCurrentUserInfo(t *testing.T) { Issued: "api", IntegrationReference: integration_reference.IntegrationReference{}, }, - Permissions: mergeRolePermissions(roles.User), + Permissions: mergeRolePermissions(roles2.User), Restricted: true, }, }, @@ -1717,7 +1717,7 @@ func TestDefaultAccountManager_GetCurrentUserInfo(t *testing.T) { Issued: "api", IntegrationReference: integration_reference.IntegrationReference{}, }, - Permissions: mergeRolePermissions(roles.User), + Permissions: mergeRolePermissions(roles2.User), Restricted: false, }, }, @@ -1738,7 +1738,7 @@ func TestDefaultAccountManager_GetCurrentUserInfo(t *testing.T) { Issued: "api", IntegrationReference: integration_reference.IntegrationReference{}, }, - Permissions: mergeRolePermissions(roles.Owner), + Permissions: mergeRolePermissions(roles2.Owner), }, }, } @@ -1758,8 +1758,8 @@ func TestDefaultAccountManager_GetCurrentUserInfo(t *testing.T) { } } -func mergeRolePermissions(role roles.RolePermissions) roles.Permissions { - permissions := roles.Permissions{} +func mergeRolePermissions(role roles2.RolePermissions) roles2.Permissions { + permissions := roles2.Permissions{} for k := range modules.All { if rolePermissions, ok := role.Permissions[k]; ok { diff --git a/management/server/users/user.go b/management/server/users/user.go index 2f2788271..e966a0365 100644 --- a/management/server/users/user.go +++ b/management/server/users/user.go @@ -1,7 +1,7 @@ package users import ( - "github.com/netbirdio/netbird/management/server/permissions/roles" + "github.com/netbirdio/netbird/management/internals/modules/permissions/roles" "github.com/netbirdio/netbird/management/server/types" ) diff --git a/shared/management/client/client_test.go b/shared/management/client/client_test.go index a11f863a7..eb52ac46a 100644 --- a/shared/management/client/client_test.go +++ b/shared/management/client/client_test.go @@ -19,6 +19,7 @@ import ( "github.com/netbirdio/management-integrations/integrations" ephemeral_manager "github.com/netbirdio/netbird/management/internals/modules/peers/ephemeral/manager" + "github.com/netbirdio/netbird/management/internals/modules/permissions" "github.com/netbirdio/netbird/management/internals/controllers/network_map/controller" "github.com/netbirdio/netbird/management/internals/controllers/network_map/update_channel" @@ -34,7 +35,6 @@ import ( "github.com/netbirdio/netbird/management/server/groups" "github.com/netbirdio/netbird/management/server/integrations/port_forwarding" "github.com/netbirdio/netbird/management/server/mock_server" - "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"