mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-25 22:09:08 +01:00
merge main\
This commit is contained in:
@@ -34,8 +34,11 @@ func Test_GetAllNetworksReturnsNetworks(t *testing.T) {
|
||||
|
||||
networks, err := manager.GetAllNetworks(ctx, accountID, userID)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, networks, 1)
|
||||
require.Equal(t, "testNetworkId", networks[0].ID)
|
||||
ids := make([]string, 0, len(networks))
|
||||
for _, n := range networks {
|
||||
ids = append(ids, n.ID)
|
||||
}
|
||||
require.ElementsMatch(t, []string{"testNetworkId", "secondNetworkId"}, ids)
|
||||
}
|
||||
|
||||
func Test_GetAllNetworksReturnsPermissionDenied(t *testing.T) {
|
||||
|
||||
@@ -108,7 +108,7 @@ func (m *managerImpl) CreateRouter(ctx context.Context, userID string, router *t
|
||||
}
|
||||
router.AccountSeqID = seq
|
||||
|
||||
err = transaction.SaveNetworkRouter(ctx, router)
|
||||
err = transaction.CreateNetworkRouter(ctx, router)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to create network router: %w", err)
|
||||
}
|
||||
@@ -168,27 +168,28 @@ func (m *managerImpl) UpdateRouter(ctx context.Context, userID string, router *t
|
||||
return fmt.Errorf("failed to get network: %w", err)
|
||||
}
|
||||
|
||||
if network.ID != router.NetworkID {
|
||||
existing, err := transaction.GetNetworkRouterByID(ctx, store.LockingStrengthUpdate, router.AccountID, router.ID)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to get network router: %w", err)
|
||||
}
|
||||
|
||||
if existing.AccountID != router.AccountID {
|
||||
return status.NewNetworkRouterNotFoundError(router.ID)
|
||||
}
|
||||
|
||||
if existing.NetworkID != router.NetworkID {
|
||||
return status.NewRouterNotPartOfNetworkError(router.ID, router.NetworkID)
|
||||
}
|
||||
|
||||
oldRouter, err := transaction.GetNetworkRouterByID(ctx, store.LockingStrengthNone, router.AccountID, router.ID)
|
||||
if err == nil {
|
||||
router.AccountSeqID = oldRouter.AccountSeqID
|
||||
} else if e, ok := status.FromError(err); ok && e.Type() == status.NotFound {
|
||||
// PUT-as-upsert: caller may target a brand-new router id (used by
|
||||
// the dashboard's "save" flow). Allocate a fresh account_seq_id so
|
||||
// the upsert behaves the same as Create().
|
||||
seq, allocErr := transaction.AllocateAccountSeqID(ctx, router.AccountID, serverTypes.AccountSeqEntityNetworkRouter)
|
||||
if allocErr != nil {
|
||||
return fmt.Errorf("failed to allocate network router seq id: %w", allocErr)
|
||||
}
|
||||
router.AccountSeqID = seq
|
||||
} else {
|
||||
return fmt.Errorf("failed to get existing network router: %w", err)
|
||||
}
|
||||
// Preserve AccountSeqID from the existing router so the upstream
|
||||
// UpdateNetworkRouter (which does Updates(router) with Select("*"))
|
||||
// doesn't clobber it with the request's zero value. Main's split of
|
||||
// Save → Create/Update removed the PUT-as-upsert path that the
|
||||
// earlier branch carried, so callers must always pass a real router
|
||||
// id — UpdateNetworkRouter returns NotFound otherwise.
|
||||
router.AccountSeqID = existing.AccountSeqID
|
||||
|
||||
err = transaction.SaveNetworkRouter(ctx, router)
|
||||
err = transaction.UpdateNetworkRouter(ctx, router)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to update network router: %w", err)
|
||||
}
|
||||
|
||||
@@ -211,6 +211,102 @@ func Test_UpdateRouterSuccessfully(t *testing.T) {
|
||||
require.Equal(t, router.Metric, updatedRouter.Metric)
|
||||
}
|
||||
|
||||
func Test_UpdateRouterRejectsCrossAccountID(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
userID := "testAdminId"
|
||||
|
||||
// Admin of testAccountId tries to update a router that belongs to otherAccountId
|
||||
// by passing the other account's router ID through the URL.
|
||||
router, err := types.NewNetworkRouter("testAccountId", "testNetworkId", "testPeerId", []string{}, false, 1, true)
|
||||
if err != nil {
|
||||
require.NoError(t, err)
|
||||
}
|
||||
router.ID = "otherRouterId"
|
||||
|
||||
s, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Cleanup(cleanUp)
|
||||
permissionsManager := permissions.NewManager(s)
|
||||
am := mock_server.MockAccountManager{}
|
||||
manager := NewManager(s, permissionsManager, &am)
|
||||
|
||||
updatedRouter, err := manager.UpdateRouter(ctx, userID, router)
|
||||
require.Error(t, err)
|
||||
require.Nil(t, updatedRouter)
|
||||
|
||||
// The other account's router must be untouched.
|
||||
stored, err := s.GetNetworkRouterByID(ctx, store.LockingStrengthNone, "otherAccountId", "otherRouterId")
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "otherAccountId", stored.AccountID)
|
||||
require.Equal(t, "otherNetworkId", stored.NetworkID)
|
||||
require.Equal(t, "otherPeer", stored.Peer)
|
||||
require.Equal(t, 1, stored.Metric)
|
||||
}
|
||||
|
||||
func Test_CreateRouterRejectsCrossAccountID(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
userID := "testAdminId"
|
||||
|
||||
// Admin of testAccountId tries to create a router in otherAccountId's network.
|
||||
// The permission check is on router.AccountID (their own), but the network
|
||||
// lookup must fail because (testAccountId, otherNetworkId) does not exist.
|
||||
router, err := types.NewNetworkRouter("testAccountId", "otherNetworkId", "testPeerId", []string{}, false, 1, true)
|
||||
if err != nil {
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
s, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Cleanup(cleanUp)
|
||||
permissionsManager := permissions.NewManager(s)
|
||||
am := mock_server.MockAccountManager{}
|
||||
manager := NewManager(s, permissionsManager, &am)
|
||||
|
||||
createdRouter, err := manager.CreateRouter(ctx, userID, router)
|
||||
require.Error(t, err)
|
||||
require.Nil(t, createdRouter)
|
||||
|
||||
// No router should have been created in either account's scope under otherNetworkId.
|
||||
routersInOther, err := s.GetNetworkRoutersByNetID(ctx, store.LockingStrengthNone, "otherAccountId", "otherNetworkId")
|
||||
require.NoError(t, err)
|
||||
require.Len(t, routersInOther, 1)
|
||||
require.Equal(t, "otherRouterId", routersInOther[0].ID)
|
||||
}
|
||||
|
||||
func Test_UpdateRouterRejectsNetworkMismatch(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
userID := "testAdminId"
|
||||
|
||||
// The router exists in testNetworkId, but the caller submits secondNetworkId
|
||||
// (a different network in the same account). The update must be refused.
|
||||
router, err := types.NewNetworkRouter("testAccountId", "secondNetworkId", "testPeerId", []string{}, false, 1, true)
|
||||
if err != nil {
|
||||
require.NoError(t, err)
|
||||
}
|
||||
router.ID = "testRouterId"
|
||||
|
||||
s, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Cleanup(cleanUp)
|
||||
permissionsManager := permissions.NewManager(s)
|
||||
am := mock_server.MockAccountManager{}
|
||||
manager := NewManager(s, permissionsManager, &am)
|
||||
|
||||
updatedRouter, err := manager.UpdateRouter(ctx, userID, router)
|
||||
require.Error(t, err)
|
||||
require.Nil(t, updatedRouter)
|
||||
|
||||
stored, err := s.GetNetworkRouterByID(ctx, store.LockingStrengthNone, "testAccountId", "testRouterId")
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "testNetworkId", stored.NetworkID)
|
||||
}
|
||||
|
||||
func Test_UpdateRouterFailsWithPermissionDenied(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
userID := "testUserId"
|
||||
|
||||
Reference in New Issue
Block a user