merge conflict and onDenied handler

This commit is contained in:
pascal
2026-09-21 16:44:39 +02:00
parent b4f31f65d2
commit 3fe19eb86d
31 changed files with 252 additions and 257 deletions
@@ -21,12 +21,13 @@ import (
"github.com/netbirdio/netbird/shared/management/status" "github.com/netbirdio/netbird/shared/management/status"
) )
// AuthErrorHandler is called when an auth error occurs during permission validation. // PermissionDeniedHandler is called when the user's role does not grant the requested operation.
// If it returns true, the error is considered handled and the default error response is skipped. // It is not called for validation failures such as blocked users or foreign accounts.
type AuthErrorHandler func(w http.ResponseWriter, r *http.Request, userAuth *auth.UserAuth, err error) bool // If it returns true, the request is considered handled and the default 403 response is skipped.
type PermissionDeniedHandler func(w http.ResponseWriter, r *http.Request, userAuth *auth.UserAuth) bool
type Manager interface { type Manager interface {
WithPermission(module modules.Module, operation operations.Operation, handlerFunc func(w http.ResponseWriter, r *http.Request, auth *auth.UserAuth), authErrHandler ...AuthErrorHandler) http.HandlerFunc WithPermission(module modules.Module, operation operations.Operation, handlerFunc func(w http.ResponseWriter, r *http.Request, auth *auth.UserAuth), onDenied ...PermissionDeniedHandler) http.HandlerFunc
ValidateUserPermissions(ctx context.Context, accountID, userID string, module modules.Module, operation operations.Operation) (bool, context.Context, error) ValidateUserPermissions(ctx context.Context, accountID, userID string, module modules.Module, operation operations.Operation) (bool, context.Context, error)
ValidateRoleModuleAccess(ctx context.Context, accountID string, role roles.RolePermissions, module modules.Module, operation operations.Operation) bool ValidateRoleModuleAccess(ctx context.Context, accountID string, role roles.RolePermissions, module modules.Module, operation operations.Operation) bool
ValidateAccountAccess(ctx context.Context, accountID string, user *types.User, allowOwnerAndAdmin bool) (context.Context, error) ValidateAccountAccess(ctx context.Context, accountID string, user *types.User, allowOwnerAndAdmin bool) (context.Context, error)
@@ -45,18 +46,30 @@ func NewManager(store store.Store) Manager {
} }
} }
// WithPermission wraps an HTTP handler with permission checking logic.
// An optional AuthErrorHandler can be provided to intercept auth errors before the default response is written.
// The wrapped handler receives a request whose context is enriched by the permission validation.
func (m *managerImpl) WithPermission( func (m *managerImpl) WithPermission(
module modules.Module, module modules.Module,
operation operations.Operation, operation operations.Operation,
handlerFunc func(w http.ResponseWriter, r *http.Request, auth *auth.UserAuth), handlerFunc func(w http.ResponseWriter, r *http.Request, auth *auth.UserAuth),
authErrHandler ...AuthErrorHandler, onDenied ...PermissionDeniedHandler,
) http.HandlerFunc { ) http.HandlerFunc {
var onAuthErr AuthErrorHandler return WithPermission(m, module, operation, handlerFunc, onDenied...)
if len(authErrHandler) > 0 { }
onAuthErr = authErrHandler[0]
// WithPermission wraps an HTTP handler with permission checking performed by the given manager.
// Implementations embedding another Manager must route their own WithPermission through this
// function so that their ValidateUserPermissions override is the one consulted.
// An optional PermissionDeniedHandler can serve a reduced, self-scoped response when the role denies the operation.
// The wrapped handler receives a request whose context is enriched by the permission validation.
func WithPermission(
m Manager,
module modules.Module,
operation operations.Operation,
handlerFunc func(w http.ResponseWriter, r *http.Request, auth *auth.UserAuth),
onDenied ...PermissionDeniedHandler,
) http.HandlerFunc {
var deniedHandler PermissionDeniedHandler
if len(onDenied) > 0 {
deniedHandler = onDenied[0]
} }
return func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) {
@@ -68,23 +81,19 @@ func (m *managerImpl) WithPermission(
} }
allowed, ctx, err := m.ValidateUserPermissions(r.Context(), userAuth.AccountId, userAuth.UserId, module, operation) allowed, ctx, err := m.ValidateUserPermissions(r.Context(), userAuth.AccountId, userAuth.UserId, module, operation)
enriched := r.WithContext(ctx)
if err != nil { if err != nil {
if onAuthErr != nil && onAuthErr(w, enriched, &userAuth, err) {
return
}
log.WithContext(ctx).Errorf("failed to validate permissions for user %s on account %s: %v", userAuth.UserId, userAuth.AccountId, err) log.WithContext(ctx).Errorf("failed to validate permissions for user %s on account %s: %v", userAuth.UserId, userAuth.AccountId, err)
util.WriteError(ctx, status.NewPermissionValidationError(err), w) util.WriteError(ctx, status.NewPermissionValidationError(err), w)
return return
} }
enriched := r.WithContext(ctx)
if !allowed { if !allowed {
permErr := status.NewPermissionDeniedError() if deniedHandler != nil && deniedHandler(w, enriched, &userAuth) {
if onAuthErr != nil && onAuthErr(w, enriched, &userAuth, permErr) {
return return
} }
log.WithContext(ctx).Tracef("user %s on account %s is not allowed to %s in %s", userAuth.UserId, userAuth.AccountId, operation, module) log.WithContext(ctx).Tracef("user %s on account %s is not allowed to %s in %s", userAuth.UserId, userAuth.AccountId, operation, module)
util.WriteError(ctx, permErr, w) util.WriteError(ctx, status.NewPermissionDeniedError(), w)
return return
} }
@@ -120,10 +120,10 @@ func (mr *MockManagerMockRecorder) ValidateUserPermissions(ctx, accountID, userI
} }
// WithPermission mocks base method. // WithPermission mocks base method.
func (m *MockManager) WithPermission(module modules.Module, operation operations.Operation, handlerFunc func(http.ResponseWriter, *http.Request, *auth.UserAuth), authErrHandler ...AuthErrorHandler) http.HandlerFunc { func (m *MockManager) WithPermission(module modules.Module, operation operations.Operation, handlerFunc func(http.ResponseWriter, *http.Request, *auth.UserAuth), onDenied ...PermissionDeniedHandler) http.HandlerFunc {
m.ctrl.T.Helper() m.ctrl.T.Helper()
varargs := []any{module, operation, handlerFunc} varargs := []any{module, operation, handlerFunc}
for _, a := range authErrHandler { for _, a := range onDenied {
varargs = append(varargs, a) varargs = append(varargs, a)
} }
ret := m.ctrl.Call(m, "WithPermission", varargs...) ret := m.ctrl.Call(m, "WithPermission", varargs...)
@@ -132,8 +132,8 @@ func (m *MockManager) WithPermission(module modules.Module, operation operations
} }
// WithPermission indicates an expected call of WithPermission. // WithPermission indicates an expected call of WithPermission.
func (mr *MockManagerMockRecorder) WithPermission(module, operation, handlerFunc any, authErrHandler ...any) *gomock.Call { func (mr *MockManagerMockRecorder) WithPermission(module, operation, handlerFunc any, onDenied ...any) *gomock.Call {
mr.mock.ctrl.T.Helper() mr.mock.ctrl.T.Helper()
varargs := append([]any{module, operation, handlerFunc}, authErrHandler...) varargs := append([]any{module, operation, handlerFunc}, onDenied...)
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WithPermission", reflect.TypeOf((*MockManager)(nil).WithPermission), varargs...) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WithPermission", reflect.TypeOf((*MockManager)(nil).WithPermission), varargs...)
} }
@@ -16,7 +16,6 @@ type Manager interface {
CreateService(ctx context.Context, accountID, userID string, service *Service) (*Service, error) CreateService(ctx context.Context, accountID, userID string, service *Service) (*Service, error)
UpdateService(ctx context.Context, accountID, userID string, service *Service) (*Service, error) UpdateService(ctx context.Context, accountID, userID string, service *Service) (*Service, error)
DeleteService(ctx context.Context, accountID, userID, serviceID string) error DeleteService(ctx context.Context, accountID, userID, serviceID string) error
DeleteAllServices(ctx context.Context, accountID, userID string) error
SetCertificateIssuedAt(ctx context.Context, accountID, serviceID string) error SetCertificateIssuedAt(ctx context.Context, accountID, serviceID string) error
SetStatus(ctx context.Context, accountID, serviceID string, status Status) error SetStatus(ctx context.Context, accountID, serviceID string, status Status) error
ReloadAllServicesForAccount(ctx context.Context, accountID string) error ReloadAllServicesForAccount(ctx context.Context, accountID string) error
@@ -85,20 +85,6 @@ func (mr *MockManagerMockRecorder) DeleteAccountCluster(ctx, accountID, userID,
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAccountCluster", reflect.TypeOf((*MockManager)(nil).DeleteAccountCluster), ctx, accountID, userID, clusterAddress) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAccountCluster", reflect.TypeOf((*MockManager)(nil).DeleteAccountCluster), ctx, accountID, userID, clusterAddress)
} }
// DeleteAllServices mocks base method.
func (m *MockManager) DeleteAllServices(ctx context.Context, accountID, userID string) error {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "DeleteAllServices", ctx, accountID, userID)
ret0, _ := ret[0].(error)
return ret0
}
// DeleteAllServices indicates an expected call of DeleteAllServices.
func (mr *MockManagerMockRecorder) DeleteAllServices(ctx, accountID, userID any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAllServices", reflect.TypeOf((*MockManager)(nil).DeleteAllServices), ctx, accountID, userID)
}
// DeleteService mocks base method. // DeleteService mocks base method.
func (m *MockManager) DeleteService(ctx context.Context, accountID, userID, serviceID string) error { func (m *MockManager) DeleteService(ctx context.Context, accountID, userID, serviceID string) error {
m.ctrl.T.Helper() m.ctrl.T.Helper()
@@ -20,15 +20,13 @@ import (
) )
type handler struct { type handler struct {
manager rpservice.Manager manager rpservice.Manager
permissionsManager permissions.Manager
} }
// RegisterEndpoints registers all service HTTP endpoints. // RegisterEndpoints registers all service HTTP endpoints.
func RegisterEndpoints(manager rpservice.Manager, domainManager domainmanager.Manager, accessLogsManager accesslogs.Manager, permissionsManager permissions.Manager, router *mux.Router) { func RegisterEndpoints(manager rpservice.Manager, domainManager domainmanager.Manager, accessLogsManager accesslogs.Manager, permissionsManager permissions.Manager, router *mux.Router) {
h := &handler{ h := &handler{
manager: manager, manager: manager,
permissionsManager: permissionsManager,
} }
domainRouter := router.PathPrefix("/reverse-proxies").Subrouter() domainRouter := router.PathPrefix("/reverse-proxies").Subrouter()
@@ -842,43 +842,6 @@ func (m *Manager) DeleteService(ctx context.Context, accountID, userID, serviceI
return nil return nil
} }
func (m *Manager) DeleteAllServices(ctx context.Context, accountID, userID string) error {
var services []*service.Service
err := m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
var err error
services, err = transaction.GetAccountServices(ctx, store.LockingStrengthUpdate, accountID)
if err != nil {
return err
}
for _, svc := range services {
if err = transaction.DeleteServiceTargets(ctx, accountID, svc.ID); err != nil {
return fmt.Errorf("failed to delete service targets: %w", err)
}
if err = transaction.DeleteService(ctx, accountID, svc.ID); err != nil {
return fmt.Errorf("failed to delete service: %w", err)
}
}
return nil
})
if err != nil {
return err
}
oidcCfg := m.proxyController.GetOIDCValidationConfig()
for _, svc := range services {
m.accountManager.StoreEvent(ctx, userID, svc.ID, accountID, activity.ServiceDeleted, svc.EventMeta())
m.proxyController.SendServiceUpdateToCluster(ctx, accountID, svc.ToProtoMapping(service.Delete, "", oidcCfg), svc.ProxyCluster)
}
m.accountManager.UpdateAccountPeers(ctx, accountID, types.UpdateReason{Resource: types.UpdateResourceService, Operation: types.UpdateOperationDelete})
return nil
}
// SetCertificateIssuedAt sets the certificate issued timestamp to the current time. // SetCertificateIssuedAt sets the certificate issued timestamp to the current time.
// Call this when receiving a gRPC notification that the certificate was issued. // Call this when receiving a gRPC notification that the certificate was issued.
func (m *Manager) SetCertificateIssuedAt(ctx context.Context, accountID, serviceID string) error { func (m *Manager) SetCertificateIssuedAt(ctx context.Context, accountID, serviceID string) error {
@@ -1052,30 +1052,6 @@ func TestDeleteService_DeletesEphemeralExpose(t *testing.T) {
assert.NoError(t, err, "new expose should succeed after API delete") assert.NoError(t, err, "new expose should succeed after API delete")
} }
func TestDeleteAllServices_DeletesEphemeralExposes(t *testing.T) {
ctx := context.Background()
mgr, _ := setupIntegrationTest(t)
for i := range 3 {
_, err := mgr.CreateServiceFromPeer(ctx, testAccountID, testPeerID, &rpservice.ExposeServiceRequest{
Port: uint16(8080 + i),
Mode: "http",
})
require.NoError(t, err)
}
count, err := mgr.store.CountEphemeralServicesByPeer(ctx, store.LockingStrengthNone, testAccountID, testPeerID)
require.NoError(t, err)
assert.Equal(t, int64(3), count, "all ephemeral services should exist")
err = mgr.DeleteAllServices(ctx, testAccountID, testUserID)
require.NoError(t, err)
count, err = mgr.store.CountEphemeralServicesByPeer(ctx, store.LockingStrengthNone, testAccountID, testPeerID)
require.NoError(t, err)
assert.Equal(t, int64(0), count, "all ephemeral services should be deleted after DeleteAllServices")
}
func TestRenewServiceFromPeer(t *testing.T) { func TestRenewServiceFromPeer(t *testing.T) {
ctx := context.Background() ctx := context.Background()
@@ -24,10 +24,6 @@ type mockReverseProxyManager struct {
err error err error
} }
func (m *mockReverseProxyManager) DeleteAllServices(ctx context.Context, accountID, userID string) error {
return nil
}
func (m *mockReverseProxyManager) GetAccountServices(ctx context.Context, accountID string) ([]*service.Service, error) { func (m *mockReverseProxyManager) GetAccountServices(ctx context.Context, accountID string) ([]*service.Service, error) {
if m.err != nil { if m.err != nil {
return nil, m.err return nil, m.err
@@ -506,10 +506,6 @@ func (m *testValidateSessionServiceManager) DeleteService(_ context.Context, _,
return nil return nil
} }
func (m *testValidateSessionServiceManager) DeleteAllServices(_ context.Context, _, _ string) error {
return nil
}
func (m *testValidateSessionServiceManager) SetCertificateIssuedAt(_ context.Context, _, _ string) error { func (m *testValidateSessionServiceManager) SetCertificateIssuedAt(_ context.Context, _, _ string) error {
return nil return nil
} }
+1 -1
View File
@@ -1932,7 +1932,7 @@ func (am *DefaultAccountManager) onPeersInvalidated(ctx context.Context, account
peers := []*nbpeer.Peer{} peers := []*nbpeer.Peer{}
log.WithContext(ctx).Debugf("invalidating peers %v for account %s", peerIDs, accountID) log.WithContext(ctx).Debugf("invalidating peers %v for account %s", peerIDs, accountID)
for _, peerID := range peerIDs { for _, peerID := range peerIDs {
peer, err := am.GetPeer(ctx, accountID, peerID, activity.SystemInitiator) peer, err := am.Store.GetPeerByID(ctx, store.LockingStrengthNone, accountID, peerID)
if err != nil { if err != nil {
log.WithContext(ctx).Errorf("failed to get invalidated peer %s for account %s: %v", peerID, accountID, err) log.WithContext(ctx).Errorf("failed to get invalidated peer %s for account %s: %v", peerID, accountID, err)
continue continue
+1 -1
View File
@@ -129,7 +129,7 @@ func NewAPIHandler(ctx context.Context, router *mux.Router, accountManager accou
agentnetworkhandlers.RegisterEndpoints(agentNetworkManager, router) agentnetworkhandlers.RegisterEndpoints(agentNetworkManager, router)
} }
instance.AddEndpoints(instanceManager, accountManager, router) instance.AddEndpoints(instanceManager, accountManager, router)
instance.AddVersionEndpoint(instanceManager, router, permissionsManager) instance.AddVersionEndpoint(instanceManager, router)
if serviceManager != nil && reverseProxyDomainManager != nil { if serviceManager != nil && reverseProxyDomainManager != nil {
reverseproxymanager.RegisterEndpoints(serviceManager, *reverseProxyDomainManager, reverseProxyAccessLogsManager, permissionsManager, router) reverseproxymanager.RegisterEndpoints(serviceManager, *reverseProxyDomainManager, reverseProxyAccessLogsManager, permissionsManager, router)
} }
@@ -8,8 +8,6 @@ import (
log "github.com/sirupsen/logrus" log "github.com/sirupsen/logrus"
"github.com/netbirdio/netbird/management/internals/modules/permissions" "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/account"
nbinstance "github.com/netbirdio/netbird/management/server/instance" nbinstance "github.com/netbirdio/netbird/management/server/instance"
"github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/auth"
@@ -36,12 +34,12 @@ func AddEndpoints(instanceManager nbinstance.Manager, accountManager account.Man
} }
// AddVersionEndpoint registers the authenticated version endpoint. // AddVersionEndpoint registers the authenticated version endpoint.
func AddVersionEndpoint(instanceManager nbinstance.Manager, router *mux.Router, permissionsManager permissions.Manager) { func AddVersionEndpoint(instanceManager nbinstance.Manager, router *mux.Router) {
h := &handler{ h := &handler{
instanceManager: instanceManager, instanceManager: instanceManager,
} }
router.HandleFunc("/instance/version", permissionsManager.WithPermission(modules.Settings, operations.Read, h.getVersionInfo)).Methods("GET", "OPTIONS") router.HandleFunc("/instance/version", permissions.WrapHandler(h.getVersionInfo)).Methods("GET", "OPTIONS")
} }
// getInstanceStatus returns the instance status including whether setup is required. // getInstanceStatus returns the instance status including whether setup is required.
@@ -15,10 +15,8 @@ import (
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"go.uber.org/mock/gomock" "go.uber.org/mock/gomock"
"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/account"
nbcontext "github.com/netbirdio/netbird/management/server/context"
"github.com/netbirdio/netbird/management/server/idp" "github.com/netbirdio/netbird/management/server/idp"
nbinstance "github.com/netbirdio/netbird/management/server/instance" nbinstance "github.com/netbirdio/netbird/management/server/instance"
"github.com/netbirdio/netbird/management/server/mock_server" "github.com/netbirdio/netbird/management/server/mock_server"
@@ -550,17 +548,11 @@ func TestSetup_PAT_CreatePATFails_Rollback(t *testing.T) {
func TestGetVersionInfo_Success(t *testing.T) { func TestGetVersionInfo_Success(t *testing.T) {
manager := &mockInstanceManager{} manager := &mockInstanceManager{}
ctrl := gomock.NewController(t)
permissionsManager := permissions.NewMockManager(ctrl)
permissionsManager.EXPECT().WithPermission(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(module modules.Module, operation operations.Operation, handler func(w http.ResponseWriter, r *http.Request, userAuth *auth.UserAuth), authErrHandler ...permissions.AuthErrorHandler) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
handler(w, r, &auth.UserAuth{})
}
}).AnyTimes()
router := mux.NewRouter() router := mux.NewRouter()
AddVersionEndpoint(manager, router, permissionsManager) AddVersionEndpoint(manager, router)
req := httptest.NewRequest(http.MethodGet, "/instance/version", nil) req := httptest.NewRequest(http.MethodGet, "/instance/version", nil)
req = nbcontext.SetUserAuthInRequest(req, auth.UserAuth{})
rec := httptest.NewRecorder() rec := httptest.NewRecorder()
router.ServeHTTP(rec, req) router.ServeHTTP(rec, req)
@@ -585,17 +577,11 @@ func TestGetVersionInfo_Error(t *testing.T) {
return nil, errors.New("failed to fetch versions") return nil, errors.New("failed to fetch versions")
}, },
} }
ctrl := gomock.NewController(t)
permissionsManager := permissions.NewMockManager(ctrl)
permissionsManager.EXPECT().WithPermission(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(module modules.Module, operation operations.Operation, handler func(w http.ResponseWriter, r *http.Request, userAuth *auth.UserAuth), authErrHandler ...permissions.AuthErrorHandler) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
handler(w, r, &auth.UserAuth{})
}
}).AnyTimes()
router := mux.NewRouter() router := mux.NewRouter()
AddVersionEndpoint(manager, router, permissionsManager) AddVersionEndpoint(manager, router)
req := httptest.NewRequest(http.MethodGet, "/instance/version", nil) req := httptest.NewRequest(http.MethodGet, "/instance/version", nil)
req = nbcontext.SetUserAuthInRequest(req, auth.UserAuth{})
rec := httptest.NewRecorder() rec := httptest.NewRecorder()
router.ServeHTTP(rec, req) router.ServeHTTP(rec, req)
@@ -34,11 +34,11 @@ type Handler struct {
func AddEndpoints(accountManager account.Manager, router *mux.Router, networkMapController network_map.Controller, permissionsManager permissions.Manager) { func AddEndpoints(accountManager account.Manager, router *mux.Router, networkMapController network_map.Controller, permissionsManager permissions.Manager) {
peersHandler := NewHandler(accountManager, networkMapController, permissionsManager) peersHandler := NewHandler(accountManager, networkMapController, permissionsManager)
router.HandleFunc("/peers", permissionsManager.WithPermission(modules.Peers, operations.Read, peersHandler.GetAllPeers)).Methods("GET", "OPTIONS") router.HandleFunc("/peers", permissionsManager.WithPermission(modules.Peers, operations.Read, peersHandler.GetAllPeers, peersHandler.getOwnPeers)).Methods("GET", "OPTIONS")
router.HandleFunc("/peers/{peerId}", permissionsManager.WithPermission(modules.Peers, operations.Read, peersHandler.GetPeer)).Methods("GET", "OPTIONS") router.HandleFunc("/peers/{peerId}", permissionsManager.WithPermission(modules.Peers, operations.Read, peersHandler.GetPeer, peersHandler.getOwnPeer)).Methods("GET", "OPTIONS")
router.HandleFunc("/peers/{peerId}", permissionsManager.WithPermission(modules.Peers, operations.Update, peersHandler.UpdatePeer)).Methods("PUT", "OPTIONS") router.HandleFunc("/peers/{peerId}", permissionsManager.WithPermission(modules.Peers, operations.Update, peersHandler.UpdatePeer)).Methods("PUT", "OPTIONS")
router.HandleFunc("/peers/{peerId}", permissionsManager.WithPermission(modules.Peers, operations.Delete, peersHandler.DeletePeer)).Methods("DELETE", "OPTIONS") router.HandleFunc("/peers/{peerId}", permissionsManager.WithPermission(modules.Peers, operations.Delete, peersHandler.DeletePeer)).Methods("DELETE", "OPTIONS")
router.HandleFunc("/peers/{peerId}/accessible-peers", permissionsManager.WithPermission(modules.Peers, operations.Read, peersHandler.GetAccessiblePeers)).Methods("GET", "OPTIONS") router.HandleFunc("/peers/{peerId}/accessible-peers", permissionsManager.WithPermission(modules.Peers, operations.Read, peersHandler.GetAccessiblePeers, peersHandler.getOwnAccessiblePeers)).Methods("GET", "OPTIONS")
router.HandleFunc("/peers/{peerId}/temporary-access", permissionsManager.WithPermission(modules.Peers, operations.Create, peersHandler.CreateTemporaryAccess)).Methods("POST", "OPTIONS") router.HandleFunc("/peers/{peerId}/temporary-access", permissionsManager.WithPermission(modules.Peers, operations.Create, peersHandler.CreateTemporaryAccess)).Methods("POST", "OPTIONS")
router.HandleFunc("/peers/{peerId}/jobs", permissionsManager.WithPermission(modules.RemoteJobs, operations.Read, peersHandler.ListJobs)).Methods("GET", "OPTIONS") router.HandleFunc("/peers/{peerId}/jobs", permissionsManager.WithPermission(modules.RemoteJobs, operations.Read, peersHandler.ListJobs)).Methods("GET", "OPTIONS")
router.HandleFunc("/peers/{peerId}/jobs", permissionsManager.WithPermission(modules.RemoteJobs, operations.Create, peersHandler.CreateJob)).Methods("POST", "OPTIONS") router.HandleFunc("/peers/{peerId}/jobs", permissionsManager.WithPermission(modules.RemoteJobs, operations.Create, peersHandler.CreateJob)).Methods("POST", "OPTIONS")
@@ -128,19 +128,46 @@ func (h *Handler) GetJob(w http.ResponseWriter, r *http.Request, userAuth *auth.
// GetPeer handles GET request for a single peer // GetPeer handles GET request for a single peer
func (h *Handler) GetPeer(w http.ResponseWriter, r *http.Request, userAuth *auth.UserAuth) { func (h *Handler) GetPeer(w http.ResponseWriter, r *http.Request, userAuth *auth.UserAuth) {
vars := mux.Vars(r) peer, ok := h.peerFromRequest(w, r, userAuth)
peerID := vars["peerId"] if !ok {
return
}
h.writePeer(w, r, userAuth, peer)
}
func (h *Handler) getOwnPeer(w http.ResponseWriter, r *http.Request, userAuth *auth.UserAuth) bool {
peer, ok := h.peerFromRequest(w, r, userAuth)
if !ok {
return true
}
if peer.UserID != userAuth.UserId {
util.WriteError(r.Context(), status.Errorf(status.NotFound, "peer not found"), w)
return true
}
h.writePeer(w, r, userAuth, peer)
return true
}
func (h *Handler) peerFromRequest(w http.ResponseWriter, r *http.Request, userAuth *auth.UserAuth) (*nbpeer.Peer, bool) {
peerID := mux.Vars(r)["peerId"]
if len(peerID) == 0 { if len(peerID) == 0 {
util.WriteError(r.Context(), status.Errorf(status.InvalidArgument, "invalid peer ID"), w) util.WriteError(r.Context(), status.Errorf(status.InvalidArgument, "invalid peer ID"), w)
return return nil, false
} }
peer, err := h.accountManager.GetPeer(r.Context(), userAuth.AccountId, peerID, userAuth.UserId) peer, err := h.accountManager.GetPeer(r.Context(), userAuth.AccountId, peerID, userAuth.UserId)
if err != nil { if err != nil {
util.WriteError(r.Context(), err, w) util.WriteError(r.Context(), err, w)
return return nil, false
} }
return peer, true
}
func (h *Handler) writePeer(w http.ResponseWriter, r *http.Request, userAuth *auth.UserAuth, peer *nbpeer.Peer) {
if peer.ProxyMeta.Embedded { if peer.ProxyMeta.Embedded {
util.WriteError(r.Context(), status.Errorf(status.InvalidArgument, "not allowed to read peer"), w) util.WriteError(r.Context(), status.Errorf(status.InvalidArgument, "not allowed to read peer"), w)
return return
@@ -154,7 +181,7 @@ func (h *Handler) GetPeer(w http.ResponseWriter, r *http.Request, userAuth *auth
dnsDomain := h.networkMapController.GetDNSDomain(settings) dnsDomain := h.networkMapController.GetDNSDomain(settings)
grps, _ := h.accountManager.GetPeerGroups(r.Context(), userAuth.AccountId, peerID) grps, _ := h.accountManager.GetPeerGroups(r.Context(), userAuth.AccountId, peer.ID)
grpsInfoMap := groups.ToGroupsInfoMap(grps, 0) grpsInfoMap := groups.ToGroupsInfoMap(grps, 0)
validPeers, invalidPeers, err := h.accountManager.GetValidatedPeers(r.Context(), userAuth.AccountId) validPeers, invalidPeers, err := h.accountManager.GetValidatedPeers(r.Context(), userAuth.AccountId)
@@ -167,7 +194,7 @@ func (h *Handler) GetPeer(w http.ResponseWriter, r *http.Request, userAuth *auth
_, valid := validPeers[peer.ID] _, valid := validPeers[peer.ID]
reason := invalidPeers[peer.ID] reason := invalidPeers[peer.ID]
util.WriteJSONObject(r.Context(), w, toSinglePeerResponse(peer, grpsInfoMap[peerID], dnsDomain, valid, reason)) util.WriteJSONObject(r.Context(), w, toSinglePeerResponse(peer, grpsInfoMap[peer.ID], dnsDomain, valid, reason))
} }
// UpdatePeer handles PUT request to update a peer // UpdatePeer handles PUT request to update a peer
@@ -280,10 +307,19 @@ func (h *Handler) DeletePeer(w http.ResponseWriter, r *http.Request, userAuth *a
// GetAllPeers returns a list of all peers associated with a provided account // GetAllPeers returns a list of all peers associated with a provided account
func (h *Handler) GetAllPeers(w http.ResponseWriter, r *http.Request, userAuth *auth.UserAuth) { func (h *Handler) GetAllPeers(w http.ResponseWriter, r *http.Request, userAuth *auth.UserAuth) {
h.listPeers(w, r, userAuth, true)
}
func (h *Handler) getOwnPeers(w http.ResponseWriter, r *http.Request, userAuth *auth.UserAuth) bool {
h.listPeers(w, r, userAuth, false)
return true
}
func (h *Handler) listPeers(w http.ResponseWriter, r *http.Request, userAuth *auth.UserAuth, all bool) {
nameFilter := r.URL.Query().Get("name") nameFilter := r.URL.Query().Get("name")
ipFilter := r.URL.Query().Get("ip") ipFilter := r.URL.Query().Get("ip")
peers, err := h.accountManager.GetPeers(r.Context(), userAuth.AccountId, userAuth.UserId, nameFilter, ipFilter, true) peers, err := h.accountManager.GetPeers(r.Context(), userAuth.AccountId, userAuth.UserId, nameFilter, ipFilter, all)
if err != nil { if err != nil {
util.WriteError(r.Context(), err, w) util.WriteError(r.Context(), err, w)
return return
@@ -354,40 +390,36 @@ func (h *Handler) GetAccessiblePeers(w http.ResponseWriter, r *http.Request, use
return return
} }
user, err := h.accountManager.GetUserByID(r.Context(), userAuth.UserId)
if err != nil {
util.WriteError(r.Context(), err, w)
return
}
account, err := h.accountManager.GetAccountByID(r.Context(), userAuth.AccountId, activity.SystemInitiator) account, err := h.accountManager.GetAccountByID(r.Context(), userAuth.AccountId, activity.SystemInitiator)
if err != nil { if err != nil {
util.WriteError(r.Context(), err, w) util.WriteError(r.Context(), err, w)
return return
} }
// Check if user is an admin/service user through their role h.writeAccessiblePeers(w, r, account, peerID)
isAdmin := user.Role == types.UserRoleAdmin || user.Role == types.UserRoleOwner }
if !isAdmin && !user.IsServiceUser && !userAuth.IsChild { func (h *Handler) getOwnAccessiblePeers(w http.ResponseWriter, r *http.Request, userAuth *auth.UserAuth) bool {
if account.Settings.RegularUsersViewBlocked { peerID := mux.Vars(r)["peerId"]
util.WriteJSONObject(r.Context(), w, []api.AccessiblePeer{})
return
}
peer, ok := account.Peers[peerID] account, err := h.accountManager.GetAccountByID(r.Context(), userAuth.AccountId, activity.SystemInitiator)
if !ok { if err != nil {
util.WriteError(r.Context(), status.Errorf(status.NotFound, "peer not found"), w) util.WriteError(r.Context(), err, w)
return return true
}
if peer.UserID != user.Id {
util.WriteJSONObject(r.Context(), w, []api.AccessiblePeer{})
return
}
} }
validPeers, _, err := h.accountManager.GetValidatedPeers(r.Context(), userAuth.AccountId) peer, ok := account.Peers[peerID]
if account.Settings.RegularUsersViewBlocked || !ok || peer.UserID != userAuth.UserId {
util.WriteJSONObject(r.Context(), w, []api.AccessiblePeer{})
return true
}
h.writeAccessiblePeers(w, r, account, peerID)
return true
}
func (h *Handler) writeAccessiblePeers(w http.ResponseWriter, r *http.Request, account *types.Account, peerID string) {
validPeers, _, err := h.accountManager.GetValidatedPeers(r.Context(), account.Id)
if err != nil { if err != nil {
log.WithContext(r.Context()).Errorf("failed to list approved peers: %v", err) log.WithContext(r.Context()).Errorf("failed to list approved peers: %v", err)
util.WriteError(r.Context(), fmt.Errorf("internal error"), w) util.WriteError(r.Context(), fmt.Errorf("internal error"), w)
@@ -409,9 +441,18 @@ func (h *Handler) CreateTemporaryAccess(w http.ResponseWriter, r *http.Request,
return return
} }
var req api.PeerTemporaryAccessRequest allowed, _, err := h.permissionsManager.ValidateUserPermissions(r.Context(), userAuth.AccountId, userAuth.UserId, modules.Policies, operations.Create)
err := json.NewDecoder(r.Body).Decode(&req)
if err != nil { if err != nil {
util.WriteError(r.Context(), status.NewPermissionValidationError(err), w)
return
}
if !allowed {
util.WriteError(r.Context(), status.NewPermissionDeniedError(), w)
return
}
var req api.PeerTemporaryAccessRequest
if err = json.NewDecoder(r.Body).Decode(&req); err != nil {
util.WriteErrorResponse("couldn't parse JSON request", http.StatusBadRequest, w) util.WriteErrorResponse("couldn't parse JSON request", http.StatusBadRequest, w)
return return
} }
@@ -501,7 +501,7 @@ func TestGetAccessiblePeers(t *testing.T) {
}) })
router := mux.NewRouter() router := mux.NewRouter()
router.HandleFunc("/api/peers/{peerId}/accessible-peers", permissions.WrapHandler(p.GetAccessiblePeers)).Methods("GET") router.HandleFunc("/api/peers/{peerId}/accessible-peers", permissions.WithPermission(p.permissionsManager, modules.Peers, operations.Read, p.GetAccessiblePeers, p.getOwnAccessiblePeers)).Methods("GET")
router.ServeHTTP(recorder, req) router.ServeHTTP(recorder, req)
res := recorder.Result() res := recorder.Result()
@@ -7,8 +7,6 @@ import (
"github.com/gorilla/mux" "github.com/gorilla/mux"
"github.com/netbirdio/netbird/management/internals/modules/permissions" "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/account"
"github.com/netbirdio/netbird/management/server/geolocation" "github.com/netbirdio/netbird/management/server/geolocation"
"github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/auth"
@@ -30,8 +28,8 @@ type geolocationsHandler struct {
func AddLocationsEndpoints(accountManager account.Manager, locationManager geolocation.Geolocation, permissionsManager permissions.Manager, router *mux.Router) { func AddLocationsEndpoints(accountManager account.Manager, locationManager geolocation.Geolocation, permissionsManager permissions.Manager, router *mux.Router) {
locationHandler := newGeolocationsHandlerHandler(accountManager, locationManager, permissionsManager) locationHandler := newGeolocationsHandlerHandler(accountManager, locationManager, permissionsManager)
router.HandleFunc("/locations/countries", permissionsManager.WithPermission(modules.Policies, operations.Read, locationHandler.getAllCountries)).Methods("GET", "OPTIONS") router.HandleFunc("/locations/countries", permissions.WrapHandler(locationHandler.getAllCountries)).Methods("GET", "OPTIONS")
router.HandleFunc("/locations/countries/{country}/cities", permissionsManager.WithPermission(modules.Policies, operations.Read, locationHandler.getCitiesByCountry)).Methods("GET", "OPTIONS") router.HandleFunc("/locations/countries/{country}/cities", permissions.WrapHandler(locationHandler.getCitiesByCountry)).Methods("GET", "OPTIONS")
} }
// newGeolocationsHandlerHandler creates a new Geolocations handler // newGeolocationsHandlerHandler creates a new Geolocations handler
@@ -414,10 +414,6 @@ type testServiceManager struct {
store store.Store store store.Store
} }
func (m *testServiceManager) DeleteAllServices(ctx context.Context, accountID, userID string) error {
return nil
}
func (m *testServiceManager) GetAllServices(_ context.Context, _, _ string) ([]*service.Service, error) { func (m *testServiceManager) GetAllServices(_ context.Context, _, _ string) ([]*service.Service, error) {
return nil, nil return nil, nil
} }
@@ -28,14 +28,14 @@ type handler struct {
func AddEndpoints(accountManager account.Manager, router *mux.Router, permissionsManager permissions.Manager) { func AddEndpoints(accountManager account.Manager, router *mux.Router, permissionsManager permissions.Manager) {
userHandler := newHandler(accountManager) userHandler := newHandler(accountManager)
router.HandleFunc("/users", permissionsManager.WithPermission(modules.Users, operations.Read, userHandler.getAllUsers, userHandler.getOwnUser)).Methods("GET", "OPTIONS") router.HandleFunc("/users", permissionsManager.WithPermission(modules.Users, operations.Read, userHandler.getAllUsers, userHandler.getOwnUser)).Methods("GET", "OPTIONS")
router.HandleFunc("/users/current", permissionsManager.WithPermission(modules.Users, operations.Read, userHandler.getCurrentUser, userHandler.getCurrentUserFallback)).Methods("GET", "OPTIONS") router.HandleFunc("/users/current", permissions.WrapHandler(userHandler.getCurrentUser)).Methods("GET", "OPTIONS")
router.HandleFunc("/users/{userId}", permissionsManager.WithPermission(modules.Users, operations.Update, userHandler.updateUser)).Methods("PUT", "OPTIONS") router.HandleFunc("/users/{userId}", permissionsManager.WithPermission(modules.Users, operations.Update, userHandler.updateUser)).Methods("PUT", "OPTIONS")
router.HandleFunc("/users/{userId}", permissionsManager.WithPermission(modules.Users, operations.Delete, userHandler.deleteUser)).Methods("DELETE", "OPTIONS") router.HandleFunc("/users/{userId}", permissionsManager.WithPermission(modules.Users, operations.Delete, userHandler.deleteUser)).Methods("DELETE", "OPTIONS")
router.HandleFunc("/users", permissionsManager.WithPermission(modules.Users, operations.Create, userHandler.createUser)).Methods("POST", "OPTIONS") router.HandleFunc("/users", permissionsManager.WithPermission(modules.Users, operations.Create, userHandler.createUser)).Methods("POST", "OPTIONS")
router.HandleFunc("/users/{userId}/invite", permissionsManager.WithPermission(modules.Users, operations.Create, userHandler.inviteUser)).Methods("POST", "OPTIONS") router.HandleFunc("/users/{userId}/invite", permissionsManager.WithPermission(modules.Users, operations.Create, userHandler.inviteUser)).Methods("POST", "OPTIONS")
router.HandleFunc("/users/{userId}/approve", permissionsManager.WithPermission(modules.Users, operations.Update, userHandler.approveUser)).Methods("POST", "OPTIONS") router.HandleFunc("/users/{userId}/approve", permissionsManager.WithPermission(modules.Users, operations.Update, userHandler.approveUser)).Methods("POST", "OPTIONS")
router.HandleFunc("/users/{userId}/reject", permissionsManager.WithPermission(modules.Users, operations.Delete, userHandler.rejectUser)).Methods("DELETE", "OPTIONS") router.HandleFunc("/users/{userId}/reject", permissionsManager.WithPermission(modules.Users, operations.Delete, userHandler.rejectUser)).Methods("DELETE", "OPTIONS")
router.HandleFunc("/users/{userId}/password", permissionsManager.WithPermission(modules.Users, operations.Update, userHandler.changePassword)).Methods("PUT", "OPTIONS") router.HandleFunc("/users/{userId}/password", permissionsManager.WithPermission(modules.Users, operations.Update, userHandler.changePassword, userHandler.changeOwnPassword)).Methods("PUT", "OPTIONS")
addUsersTokensEndpoint(accountManager, router, permissionsManager) addUsersTokensEndpoint(accountManager, router, permissionsManager)
} }
@@ -405,28 +405,16 @@ func (h *handler) changePassword(w http.ResponseWriter, r *http.Request, userAut
util.WriteJSONObject(r.Context(), w, util.EmptyObject{}) util.WriteJSONObject(r.Context(), w, util.EmptyObject{})
} }
func (h *handler) getCurrentUserFallback(w http.ResponseWriter, r *http.Request, userAuth *auth.UserAuth, err error) bool { func (h *handler) changeOwnPassword(w http.ResponseWriter, r *http.Request, userAuth *auth.UserAuth) bool {
s, ok := status.FromError(err) if mux.Vars(r)["userId"] != userAuth.UserId {
if !ok || s.ErrorType != status.PermissionDenied {
return false return false
} }
user, userErr := h.accountManager.GetCurrentUserInfo(r.Context(), *userAuth) h.changePassword(w, r, userAuth)
if userErr != nil {
util.WriteError(r.Context(), userErr, w)
return true
}
util.WriteJSONObject(r.Context(), w, toUserWithPermissionsResponse(user, userAuth.UserId))
return true return true
} }
func (h *handler) getOwnUser(w http.ResponseWriter, r *http.Request, userAuth *auth.UserAuth, err error) bool { func (h *handler) getOwnUser(w http.ResponseWriter, r *http.Request, userAuth *auth.UserAuth) bool {
s, ok := status.FromError(err)
if !ok || s.ErrorType != status.PermissionDenied {
return false
}
if r.URL.Query().Get("service_user") != "" { if r.URL.Query().Get("service_user") != "" {
return false return false
} }
@@ -26,31 +26,37 @@ func Test_Peers_GetAll(t *testing.T) {
name string name string
userId string userId string
expectResponse bool expectResponse bool
expectedPeers int
}{ }{
{ {
name: "Regular user", name: "Regular user",
userId: testing_tools.TestUserId, userId: testing_tools.TestUserId,
expectResponse: false, expectResponse: true,
expectedPeers: 1,
}, },
{ {
name: "Admin user", name: "Admin user",
userId: testing_tools.TestAdminId, userId: testing_tools.TestAdminId,
expectResponse: true, expectResponse: true,
expectedPeers: 2,
}, },
{ {
name: "Owner user", name: "Owner user",
userId: testing_tools.TestOwnerId, userId: testing_tools.TestOwnerId,
expectResponse: true, expectResponse: true,
expectedPeers: 2,
}, },
{ {
name: "Regular service user", name: "Regular service user",
userId: testing_tools.TestServiceUserId, userId: testing_tools.TestServiceUserId,
expectResponse: false, expectResponse: true,
expectedPeers: 0,
}, },
{ {
name: "Admin service user", name: "Admin service user",
userId: testing_tools.TestServiceAdminId, userId: testing_tools.TestServiceAdminId,
expectResponse: true, expectResponse: true,
expectedPeers: 2,
}, },
{ {
name: "Blocked user", name: "Blocked user",
@@ -88,7 +94,7 @@ func Test_Peers_GetAll(t *testing.T) {
t.Fatalf("Sent content is not in correct json format; %v", err) t.Fatalf("Sent content is not in correct json format; %v", err)
} }
assert.GreaterOrEqual(t, len(got), 2, "Expected at least 2 peers") assert.Len(t, got, user.expectedPeers, "regular users must only see their own peers")
select { select {
case <-done: case <-done:
@@ -99,17 +105,36 @@ func Test_Peers_GetAll(t *testing.T) {
} }
} }
func Test_Peers_GetById_RegularUser(t *testing.T) {
tt := []struct {
name string
peerId string
expectedStatus int
}{
{"Own peer", testing_tools.TestPeerId, http.StatusOK},
{"Peer of another user", testPeerId2, http.StatusNotFound},
{"Non-existing peer", "nonExistingPeerId", http.StatusNotFound},
}
for _, tc := range tt {
t.Run(tc.name, func(t *testing.T) {
apiHandler, _, _ := channel.BuildApiBlackBoxWithDBState(t, "../testdata/peers_integration.sql", nil, false)
req := testing_tools.BuildRequest(t, []byte{}, http.MethodGet, "/api/peers/"+tc.peerId, testing_tools.TestUserId)
recorder := httptest.NewRecorder()
apiHandler.ServeHTTP(recorder, req)
assert.Equal(t, tc.expectedStatus, recorder.Code, "unexpected status, body: %s", recorder.Body.String())
})
}
}
func Test_Peers_GetById(t *testing.T) { func Test_Peers_GetById(t *testing.T) {
users := []struct { users := []struct {
name string name string
userId string userId string
expectResponse bool expectResponse bool
}{ }{
{
name: "Regular user",
userId: testing_tools.TestUserId,
expectResponse: false,
},
{ {
name: "Admin user", name: "Admin user",
userId: testing_tools.TestAdminId, userId: testing_tools.TestAdminId,
@@ -120,6 +145,11 @@ func Test_Peers_GetById(t *testing.T) {
userId: testing_tools.TestOwnerId, userId: testing_tools.TestOwnerId,
expectResponse: true, expectResponse: true,
}, },
{
name: "Auditor user",
userId: testing_tools.TestAuditorId,
expectResponse: true,
},
{ {
name: "Regular service user", name: "Regular service user",
userId: testing_tools.TestServiceUserId, userId: testing_tools.TestServiceUserId,
@@ -508,7 +538,7 @@ func Test_Peers_GetAccessiblePeers(t *testing.T) {
{ {
name: "Regular user", name: "Regular user",
userId: testing_tools.TestUserId, userId: testing_tools.TestUserId,
expectResponse: false, expectResponse: true,
}, },
{ {
name: "Admin user", name: "Admin user",
@@ -523,7 +553,7 @@ func Test_Peers_GetAccessiblePeers(t *testing.T) {
{ {
name: "Regular service user", name: "Regular service user",
userId: testing_tools.TestServiceUserId, userId: testing_tools.TestServiceUserId,
expectResponse: false, expectResponse: true,
}, },
{ {
name: "Admin service user", name: "Admin service user",
@@ -26,6 +26,7 @@ func Test_Users_GetAll(t *testing.T) {
{"Regular user", testing_tools.TestUserId, true}, {"Regular user", testing_tools.TestUserId, true},
{"Admin user", testing_tools.TestAdminId, true}, {"Admin user", testing_tools.TestAdminId, true},
{"Owner user", testing_tools.TestOwnerId, true}, {"Owner user", testing_tools.TestOwnerId, true},
{"Auditor user", testing_tools.TestAuditorId, true},
{"Regular service user", testing_tools.TestServiceUserId, false}, {"Regular service user", testing_tools.TestServiceUserId, false},
{"Admin service user", testing_tools.TestServiceAdminId, true}, {"Admin service user", testing_tools.TestServiceAdminId, true},
{"Blocked user", testing_tools.BlockedUserId, false}, {"Blocked user", testing_tools.BlockedUserId, false},
@@ -62,6 +63,49 @@ func Test_Users_GetAll(t *testing.T) {
} }
} }
func Test_Users_GetAll_ReadOnlyRoleSeesAllUsers(t *testing.T) {
apiHandler, _, _ := channel.BuildApiBlackBoxWithDBState(t, "../testdata/users_integration.sql", nil, false)
req := testing_tools.BuildRequest(t, []byte{}, http.MethodGet, "/api/users", testing_tools.TestAuditorId)
recorder := httptest.NewRecorder()
apiHandler.ServeHTTP(recorder, req)
content, _ := testing_tools.ReadResponse(t, recorder, http.StatusOK, true)
got := []api.User{}
if err := json.Unmarshal(content, &got); err != nil {
t.Fatalf("Sent content is not in correct json format; %v", err)
}
assert.Greater(t, len(got), 1, "auditor must see every user of the account, not only themselves")
}
func Test_Users_ChangePassword(t *testing.T) {
tt := []struct {
name string
userId string
targetUserId string
expectedStatus int
}{
{"Regular user changes own password", testing_tools.TestUserId, testing_tools.TestUserId, http.StatusPreconditionFailed},
{"Regular user changes another user's password", testing_tools.TestUserId, testing_tools.TestAdminId, http.StatusForbidden},
{"Admin changes another user's password", testing_tools.TestAdminId, testing_tools.TestUserId, http.StatusPreconditionFailed},
}
body := []byte(`{"old_password":"OldPass123!","new_password":"NewPass456!"}`)
for _, tc := range tt {
t.Run(tc.name, func(t *testing.T) {
apiHandler, _, _ := channel.BuildApiBlackBoxWithDBState(t, "../testdata/users_integration.sql", nil, false)
req := testing_tools.BuildRequest(t, body, http.MethodPut, "/api/users/"+tc.targetUserId+"/password", tc.userId)
recorder := httptest.NewRecorder()
apiHandler.ServeHTTP(recorder, req)
assert.Equal(t, tc.expectedStatus, recorder.Code, "unexpected status, body: %s", recorder.Body.String())
})
}
}
func Test_Users_GetAll_ServiceUsers(t *testing.T) { func Test_Users_GetAll_ServiceUsers(t *testing.T) {
users := []struct { users := []struct {
name string name string
+1
View File
@@ -7,6 +7,7 @@ INSERT INTO accounts VALUES('testAccountId','','2024-10-02 16:01:38.000000000+00
INSERT INTO users VALUES('testUserId','testAccountId','user',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,''); INSERT INTO users VALUES('testUserId','testAccountId','user',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
INSERT INTO users VALUES('testAdminId','testAccountId','admin',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,''); INSERT INTO users VALUES('testAdminId','testAccountId','admin',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
INSERT INTO users VALUES('testOwnerId','testAccountId','owner',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,''); INSERT INTO users VALUES('testOwnerId','testAccountId','owner',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
INSERT INTO users VALUES('testAuditorId','testAccountId','auditor',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
INSERT INTO users VALUES('testServiceUserId','testAccountId','user',1,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,''); INSERT INTO users VALUES('testServiceUserId','testAccountId','user',1,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
INSERT INTO users VALUES('testServiceAdminId','testAccountId','admin',1,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,''); INSERT INTO users VALUES('testServiceAdminId','testAccountId','admin',1,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
INSERT INTO users VALUES('blockedUserId','testAccountId','admin',0,0,'','[]',1,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,''); INSERT INTO users VALUES('blockedUserId','testAccountId','admin',0,0,'','[]',1,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
@@ -8,6 +8,7 @@ INSERT INTO accounts VALUES('testAccountId','','2024-10-02 16:01:38.000000000+00
INSERT INTO users VALUES('testUserId','testAccountId','user',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,''); INSERT INTO users VALUES('testUserId','testAccountId','user',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
INSERT INTO users VALUES('testAdminId','testAccountId','admin',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,''); INSERT INTO users VALUES('testAdminId','testAccountId','admin',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
INSERT INTO users VALUES('testOwnerId','testAccountId','owner',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,''); INSERT INTO users VALUES('testOwnerId','testAccountId','owner',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
INSERT INTO users VALUES('testAuditorId','testAccountId','auditor',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
INSERT INTO users VALUES('testServiceUserId','testAccountId','user',1,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,''); INSERT INTO users VALUES('testServiceUserId','testAccountId','user',1,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
INSERT INTO users VALUES('testServiceAdminId','testAccountId','admin',1,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,''); INSERT INTO users VALUES('testServiceAdminId','testAccountId','admin',1,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
INSERT INTO users VALUES('blockedUserId','testAccountId','admin',0,0,'','[]',1,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,''); INSERT INTO users VALUES('blockedUserId','testAccountId','admin',0,0,'','[]',1,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
@@ -10,6 +10,7 @@ INSERT INTO accounts VALUES('testAccountId','','2024-10-02 16:01:38.000000000+00
INSERT INTO users VALUES('testUserId','testAccountId','user',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,''); INSERT INTO users VALUES('testUserId','testAccountId','user',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
INSERT INTO users VALUES('testAdminId','testAccountId','admin',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,''); INSERT INTO users VALUES('testAdminId','testAccountId','admin',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
INSERT INTO users VALUES('testOwnerId','testAccountId','owner',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,''); INSERT INTO users VALUES('testOwnerId','testAccountId','owner',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
INSERT INTO users VALUES('testAuditorId','testAccountId','auditor',0,0,'','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
INSERT INTO users VALUES('testServiceUserId','testAccountId','user',1,0,'testServiceUser','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,''); INSERT INTO users VALUES('testServiceUserId','testAccountId','user',1,0,'testServiceUser','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
INSERT INTO users VALUES('testServiceAdminId','testAccountId','admin',1,0,'testServiceAdmin','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,''); INSERT INTO users VALUES('testServiceAdminId','testAccountId','admin',1,0,'testServiceAdmin','[]',0,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
INSERT INTO users VALUES('blockedUserId','testAccountId','admin',0,0,'','[]',1,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,''); INSERT INTO users VALUES('blockedUserId','testAccountId','admin',0,0,'','[]',1,NULL,'2024-10-02 16:01:38.000000000+00:00','api',0,'');
@@ -298,7 +298,7 @@ func mockValidateAndParseToken(_ context.Context, token string) (auth.UserAuth,
userAuth := auth.UserAuth{} userAuth := auth.UserAuth{}
switch token { switch token {
case "testUserId", "testAdminId", "testOwnerId", "testServiceUserId", "testServiceAdminId", "blockedUserId": case "testUserId", "testAdminId", "testOwnerId", "testAuditorId", "testServiceUserId", "testServiceAdminId", "blockedUserId":
userAuth.UserId = token userAuth.UserId = token
userAuth.AccountId = "testAccountId" userAuth.AccountId = "testAccountId"
userAuth.Domain = "test.com" userAuth.Domain = "test.com"
@@ -32,6 +32,7 @@ const (
TestUserId = "testUserId" TestUserId = "testUserId"
TestAdminId = "testAdminId" TestAdminId = "testAdminId"
TestOwnerId = "testOwnerId" TestOwnerId = "testOwnerId"
TestAuditorId = "testAuditorId"
TestServiceUserId = "testServiceUserId" TestServiceUserId = "testServiceUserId"
TestServiceAdminId = "testServiceAdminId" TestServiceAdminId = "testServiceAdminId"
BlockedUserId = "blockedUserId" BlockedUserId = "blockedUserId"
+8 -8
View File
@@ -132,15 +132,15 @@ func (am *DefaultAccountManager) GetIdentityProvider(ctx context.Context, accoun
// CreateIdentityProvider creates a new identity provider // CreateIdentityProvider creates a new identity provider
func (am *DefaultAccountManager) CreateIdentityProvider(ctx context.Context, accountID, userID string, idpConfig *types.IdentityProvider) (*types.IdentityProvider, error) { func (am *DefaultAccountManager) CreateIdentityProvider(ctx context.Context, accountID, userID string, idpConfig *types.IdentityProvider) (*types.IdentityProvider, error) {
if err := validateIdentityProviderConfig(ctx, idpConfig); err != nil {
return nil, err
}
embeddedManager, ok := am.idpManager.(*idp.EmbeddedIdPManager) embeddedManager, ok := am.idpManager.(*idp.EmbeddedIdPManager)
if !ok { if !ok {
return nil, status.Errorf(status.Internal, "identity provider management requires embedded IdP") return nil, status.Errorf(status.Internal, "identity provider management requires embedded IdP")
} }
if err := validateIdentityProviderConfig(ctx, idpConfig); err != nil {
return nil, err
}
// Generate ID if not provided // Generate ID if not provided
if idpConfig.ID == "" { if idpConfig.ID == "" {
idpConfig.ID = generateIdentityProviderID(idpConfig.Type) idpConfig.ID = generateIdentityProviderID(idpConfig.Type)
@@ -161,15 +161,15 @@ func (am *DefaultAccountManager) CreateIdentityProvider(ctx context.Context, acc
// UpdateIdentityProvider updates an existing identity provider // UpdateIdentityProvider updates an existing identity provider
func (am *DefaultAccountManager) UpdateIdentityProvider(ctx context.Context, accountID, idpID, userID string, idpConfig *types.IdentityProvider) (*types.IdentityProvider, error) { func (am *DefaultAccountManager) UpdateIdentityProvider(ctx context.Context, accountID, idpID, userID string, idpConfig *types.IdentityProvider) (*types.IdentityProvider, error) {
if err := validateIdentityProviderConfig(ctx, idpConfig); err != nil {
return nil, err
}
embeddedManager, ok := am.idpManager.(*idp.EmbeddedIdPManager) embeddedManager, ok := am.idpManager.(*idp.EmbeddedIdPManager)
if !ok { if !ok {
return nil, status.Errorf(status.Internal, "identity provider management requires embedded IdP") return nil, status.Errorf(status.Internal, "identity provider management requires embedded IdP")
} }
if err := validateIdentityProviderConfig(ctx, idpConfig); err != nil {
return nil, err
}
idpConfig.ID = idpID idpConfig.ID = idpID
idpConfig.AccountID = accountID idpConfig.AccountID = accountID
+4 -19
View File
@@ -51,7 +51,7 @@ func (am *DefaultAccountManager) GetPeers(ctx context.Context, accountID, userID
return nil, err return nil, err
} }
if all || user.IsAdminOrServiceUser() { if all || user.HasAdminPower() {
return am.Store.GetAccountPeers(ctx, store.LockingStrengthNone, accountID, nameFilter, ipFilter) return am.Store.GetAccountPeers(ctx, store.LockingStrengthNone, accountID, nameFilter, ipFilter)
} }
@@ -1453,25 +1453,10 @@ func peerLoginExpired(ctx context.Context, peer *nbpeer.Peer, settings *types.Se
return false return false
} }
// GetPeer returns a peer visible to the user within an account. // GetPeer returns a peer within an account. Callers are expected to have passed the
// Users with "peers:read" permission can access any peer. Otherwise, users can access only their own peer. // "peers:read" permission check at the HTTP layer.
func (am *DefaultAccountManager) GetPeer(ctx context.Context, accountID, peerID, userID string) (*nbpeer.Peer, error) { func (am *DefaultAccountManager) GetPeer(ctx context.Context, accountID, peerID, userID string) (*nbpeer.Peer, error) {
peer, err := am.Store.GetPeerByID(ctx, store.LockingStrengthNone, accountID, peerID) return am.Store.GetPeerByID(ctx, store.LockingStrengthNone, accountID, peerID)
if err != nil {
return nil, err
}
user, err := am.Store.GetUserByUserID(ctx, store.LockingStrengthNone, userID)
if err != nil {
return nil, err
}
// if admin or user owns this peer, return peer
if user.IsAdminOrServiceUser() || peer.UserID == userID {
return peer, nil
}
return nil, status.Errorf(status.Internal, "user %s has no access to peer %s under account %s", userID, peer.ID, accountID)
} }
// UpdateAccountPeers updates all peers that belong to an account. // UpdateAccountPeers updates all peers that belong to an account.
+9 -8
View File
@@ -553,7 +553,7 @@ func TestDefaultAccountManager_GetPeer(t *testing.T) {
return return
} }
// the user can see its own peer // authorization is enforced at the HTTP layer, the manager returns any peer of the account
peer, err := manager.GetPeer(context.Background(), accountID, peer1.ID, someUser) peer, err := manager.GetPeer(context.Background(), accountID, peer1.ID, someUser)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
@@ -561,12 +561,13 @@ func TestDefaultAccountManager_GetPeer(t *testing.T) {
} }
assert.NotNil(t, peer) assert.NotNil(t, peer)
// the user can NOT see peer2 because it is not owned by them. peer, err = manager.GetPeer(context.Background(), accountID, peer2.ID, someUser)
// Regular users only see peers they directly own. if err != nil {
_, err = manager.GetPeer(context.Background(), accountID, peer2.ID, someUser) t.Fatal(err)
assert.Error(t, err) return
}
assert.NotNil(t, peer)
// admin users can always access all the peers
peer, err = manager.GetPeer(context.Background(), accountID, peer1.ID, adminUser) peer, err = manager.GetPeer(context.Background(), accountID, peer1.ID, adminUser)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
@@ -602,7 +603,7 @@ func TestDefaultAccountManager_GetPeers(t *testing.T) {
role: types.UserRoleUser, role: types.UserRoleUser,
limitedViewSettings: false, limitedViewSettings: false,
isServiceUser: true, isServiceUser: true,
expectedPeerCount: 2, expectedPeerCount: 1,
}, },
{ {
name: "Regular user, limited view settings", name: "Regular user, limited view settings",
@@ -616,7 +617,7 @@ func TestDefaultAccountManager_GetPeers(t *testing.T) {
role: types.UserRoleUser, role: types.UserRoleUser,
limitedViewSettings: true, limitedViewSettings: true,
isServiceUser: true, isServiceUser: true,
expectedPeerCount: 2, expectedPeerCount: 0,
}, },
{ {
name: "Admin, no limited view settings, not a service user", name: "Admin, no limited view settings, not a service user",
+12 -7
View File
@@ -16,6 +16,8 @@ import (
log "github.com/sirupsen/logrus" log "github.com/sirupsen/logrus"
"github.com/netbirdio/netbird/idp/dex" "github.com/netbirdio/netbird/idp/dex"
"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/account"
"github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/activity"
"github.com/netbirdio/netbird/management/server/affectedpeers" "github.com/netbirdio/netbird/management/server/affectedpeers"
@@ -965,8 +967,12 @@ 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 // GetUsersFromAccount performs a batched request for users from IDP by account ID apply filter on what data to return
// based on provided user role. // based on provided user role.
func (am *DefaultAccountManager) GetUsersFromAccount(ctx context.Context, accountID, initiatorUserID string) (map[string]*types.UserInfo, error) { func (am *DefaultAccountManager) GetUsersFromAccount(ctx context.Context, accountID, initiatorUserID string) (map[string]*types.UserInfo, error) {
allowed, ctx, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, initiatorUserID, modules.Users, operations.Read)
if err != nil {
return nil, status.NewPermissionValidationError(err)
}
var user *types.User var user *types.User
var err error
if initiatorUserID != activity.SystemInitiator { if initiatorUserID != activity.SystemInitiator {
result, err := am.Store.GetUserByUserID(ctx, store.LockingStrengthNone, initiatorUserID) result, err := am.Store.GetUserByUserID(ctx, store.LockingStrengthNone, initiatorUserID)
if err != nil { if err != nil {
@@ -976,11 +982,8 @@ func (am *DefaultAccountManager) GetUsersFromAccount(ctx context.Context, accoun
} }
accountUsers := []*types.User{} accountUsers := []*types.User{}
hasFullAccess := initiatorUserID == activity.SystemInitiator || user.HasAdminPower() || user.IsServiceUser
switch { switch {
case hasFullAccess: case allowed:
start := time.Now() start := time.Now()
accountUsers, err = am.Store.GetAccountUsers(ctx, store.LockingStrengthNone, accountID) accountUsers, err = am.Store.GetAccountUsers(ctx, store.LockingStrengthNone, accountID)
if err != nil { if err != nil {
@@ -1465,8 +1468,10 @@ func (am *DefaultAccountManager) GetCurrentUserInfo(ctx context.Context, userAut
return nil, status.NewPermissionDeniedError() return nil, status.NewPermissionDeniedError()
} }
// Permission checks are now handled by the HTTP middleware via WithPermission wrapper ctx, err = am.permissionsManager.ValidateAccountAccess(ctx, accountID, user, false)
// User account association is already validated above by GetUserByUserID if err != nil {
return nil, err
}
settings, err := am.Store.GetAccountSettings(ctx, store.LockingStrengthNone, accountID) settings, err := am.Store.GetAccountSettings(ctx, store.LockingStrengthNone, accountID)
if err != nil { if err != nil {
+2 -2
View File
@@ -1414,8 +1414,8 @@ func TestUser_GetUsersFromAccount_ForUser(t *testing.T) {
t.Fatalf("Error when getting users from account: %s", err) t.Fatalf("Error when getting users from account: %s", err)
} }
// Service users should see all users // Service users follow their role like any other user, a role user only sees themselves
assert.Equal(t, 2, len(users)) assert.Equal(t, 1, len(users))
} }
func TestDefaultAccountManager_SaveUser(t *testing.T) { func TestDefaultAccountManager_SaveUser(t *testing.T) {
-4
View File
@@ -293,10 +293,6 @@ type storeBackedServiceManager struct {
tokenStore *nbgrpc.OneTimeTokenStore tokenStore *nbgrpc.OneTimeTokenStore
} }
func (m *storeBackedServiceManager) DeleteAllServices(ctx context.Context, accountID, userID string) error {
return nil
}
func (m *storeBackedServiceManager) GetAllServices(ctx context.Context, accountID, userID string) ([]*service.Service, error) { func (m *storeBackedServiceManager) GetAllServices(ctx context.Context, accountID, userID string) ([]*service.Service, error) {
return m.store.GetAccountServices(ctx, store.LockingStrengthNone, accountID) return m.store.GetAccountServices(ctx, store.LockingStrengthNone, accountID)
} }