From 2dfcf42d7167d97781a3089676c61db659756b9c Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Thu, 7 Nov 2024 00:19:32 +0300 Subject: [PATCH] fix tests Signed-off-by: bcmmbaga --- dns/nameserver.go | 2 + management/server/account_test.go | 11 +- management/server/dns.go | 4 +- management/server/ephemeral_test.go | 26 ++--- management/server/group.go | 2 +- management/server/group_test.go | 71 ++++++----- management/server/management_proto_test.go | 2 +- management/server/nameserver.go | 1 + management/server/nameserver_test.go | 4 +- management/server/peer_test.go | 7 +- management/server/policy.go | 7 +- management/server/policy_test.go | 66 +++++++---- management/server/posture/checks.go | 2 +- management/server/posture_checks.go | 8 +- management/server/posture_checks_test.go | 110 +++++++++++------- management/server/route.go | 10 +- management/server/setupkey.go | 6 +- management/server/status/error.go | 33 ------ .../testdata/store_with_expired_peers.sql | 3 + management/server/user.go | 2 +- route/route.go | 54 ++++----- 21 files changed, 232 insertions(+), 199 deletions(-) diff --git a/dns/nameserver.go b/dns/nameserver.go index bb904b165..9f5312467 100644 --- a/dns/nameserver.go +++ b/dns/nameserver.go @@ -136,6 +136,7 @@ func ParseNameServerURL(nsURL string) (NameServer, error) { func (g *NameServerGroup) Copy() *NameServerGroup { nsGroup := &NameServerGroup{ ID: g.ID, + AccountID: g.AccountID, Name: g.Name, Description: g.Description, NameServers: make([]NameServer, len(g.NameServers)), @@ -156,6 +157,7 @@ func (g *NameServerGroup) Copy() *NameServerGroup { // IsEqual compares one nameserver group with the other func (g *NameServerGroup) IsEqual(other *NameServerGroup) bool { return other.ID == g.ID && + other.AccountID == g.AccountID && other.Name == g.Name && other.Description == g.Description && other.Primary == g.Primary && diff --git a/management/server/account_test.go b/management/server/account_test.go index 0e2198e03..3b437b5e3 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -1808,10 +1808,13 @@ func TestDefaultAccountManager_MarkPeerConnected_PeerLoginExpiration(t *testing. LoginExpirationEnabled: true, }) require.NoError(t, err, "unable to add peer") - _, err = manager.UpdateAccountSettings(context.Background(), accountID, userID, &Settings{ - PeerLoginExpiration: time.Hour, - PeerLoginExpirationEnabled: true, - }) + + settings, err := manager.GetAccountSettings(context.Background(), accountID, userID) + require.NoError(t, err, "failed to get account settings") + + settings.PeerLoginExpirationEnabled = true + settings.PeerLoginExpiration = time.Hour + _, err = manager.UpdateAccountSettings(context.Background(), accountID, userID, settings) require.NoError(t, err, "expecting to update account settings successfully but got error") wg := &sync.WaitGroup{} diff --git a/management/server/dns.go b/management/server/dns.go index 719b73307..ee41b383a 100644 --- a/management/server/dns.go +++ b/management/server/dns.go @@ -91,7 +91,7 @@ func (am *DefaultAccountManager) GetDNSSettings(ctx context.Context, accountID s } if user.IsRegularUser() { - return nil, status.NewUnauthorizedToViewDNSSettingsError() + return nil, status.NewAdminPermissionError() } return am.Store.GetAccountDNSSettings(ctx, LockingStrengthShare, accountID) @@ -113,7 +113,7 @@ func (am *DefaultAccountManager) SaveDNSSettings(ctx context.Context, accountID } if !user.HasAdminPower() { - return status.NewUnauthorizedToViewDNSSettingsError() + return status.NewAdminPermissionError() } oldSettings, err := am.Store.GetAccountDNSSettings(ctx, LockingStrengthUpdate, accountID) diff --git a/management/server/ephemeral_test.go b/management/server/ephemeral_test.go index c1f79aad3..eae70dae5 100644 --- a/management/server/ephemeral_test.go +++ b/management/server/ephemeral_test.go @@ -15,20 +15,6 @@ type MockStore struct { accountID string } -//func (s *MockStore) GetAllAccounts(_ context.Context) []*Account { -// return []*Account{s.account} -//} - -//func (s *MockStore) GetAccountByPeerID(_ context.Context, peerId string) (*Account, error) { -// -// _, ok := s.account.Peers[peerId] -// if ok { -// return s.account, nil -// } -// -// return nil, status.NewPeerNotFoundError(peerId) -//} - type MocAccountManager struct { AccountManager store *MockStore @@ -72,7 +58,9 @@ func TestNewManagerPeerConnected(t *testing.T) { return startTime } - store := &MockStore{} + store := &MockStore{ + Store: newStore(t), + } am := MocAccountManager{ store: store, } @@ -104,7 +92,9 @@ func TestNewManagerPeerDisconnected(t *testing.T) { return startTime } - store := &MockStore{} + store := &MockStore{ + Store: newStore(t), + } am := MocAccountManager{ store: store, } @@ -151,7 +141,7 @@ func seedPeers(store *MockStore, numberOfPeers int, numberOfEphemeralPeers int) AccountID: accountID, Ephemeral: false, } - err = store.SavePeer(context.Background(), LockingStrengthUpdate, accountID, p) + err = store.AddPeerToAccount(context.Background(), p) if err != nil { return err } @@ -164,7 +154,7 @@ func seedPeers(store *MockStore, numberOfPeers int, numberOfEphemeralPeers int) AccountID: accountID, Ephemeral: true, } - err = store.SavePeer(context.Background(), LockingStrengthUpdate, accountID, p) + err = store.AddPeerToAccount(context.Background(), p) if err != nil { return err } diff --git a/management/server/group.go b/management/server/group.go index ec3fc4680..f50de2f22 100644 --- a/management/server/group.go +++ b/management/server/group.go @@ -42,7 +42,7 @@ func (am *DefaultAccountManager) CheckGroupPermissions(ctx context.Context, acco } if user.IsRegularUser() && settings.RegularUsersViewBlocked { - return status.NewUnauthorizedToViewGroupsError() + return status.NewAdminPermissionError() } return nil diff --git a/management/server/group_test.go b/management/server/group_test.go index 9012832e5..b60be0760 100644 --- a/management/server/group_test.go +++ b/management/server/group_test.go @@ -424,24 +424,28 @@ func TestGroupAccountPeersUpdate(t *testing.T) { err := manager.SaveGroups(context.Background(), account.Id, userID, []*nbgroup.Group{ { - ID: "groupA", - Name: "GroupA", - Peers: []string{peer1.ID, peer2.ID}, + ID: "groupA", + AccountID: account.Id, + Name: "GroupA", + Peers: []string{peer1.ID, peer2.ID}, }, { - ID: "groupB", - Name: "GroupB", - Peers: []string{}, + ID: "groupB", + AccountID: account.Id, + Name: "GroupB", + Peers: []string{}, }, { - ID: "groupC", - Name: "GroupC", - Peers: []string{peer1.ID, peer3.ID}, + ID: "groupC", + AccountID: account.Id, + Name: "GroupC", + Peers: []string{peer1.ID, peer3.ID}, }, { - ID: "groupD", - Name: "GroupD", - Peers: []string{}, + ID: "groupD", + AccountID: account.Id, + Name: "GroupD", + Peers: []string{}, }, }) assert.NoError(t, err) @@ -460,9 +464,10 @@ func TestGroupAccountPeersUpdate(t *testing.T) { }() err := manager.SaveGroup(context.Background(), account.Id, userID, &nbgroup.Group{ - ID: "groupB", - Name: "GroupB", - Peers: []string{peer1.ID, peer2.ID}, + ID: "groupB", + AccountID: account.Id, + Name: "GroupB", + Peers: []string{peer1.ID, peer2.ID}, }) assert.NoError(t, err) @@ -531,10 +536,13 @@ func TestGroupAccountPeersUpdate(t *testing.T) { // adding a group to policy err = manager.SavePolicy(context.Background(), account.Id, userID, &Policy{ - ID: "policy", - Enabled: true, + ID: "policy", + AccountID: account.Id, + Enabled: true, Rules: []*PolicyRule{ { + ID: "rule", + PolicyID: "policy", Enabled: true, Sources: []string{"groupA"}, Destinations: []string{"groupA"}, @@ -554,9 +562,10 @@ func TestGroupAccountPeersUpdate(t *testing.T) { }() err := manager.SaveGroup(context.Background(), account.Id, userID, &nbgroup.Group{ - ID: "groupA", - Name: "GroupA", - Peers: []string{peer1.ID, peer2.ID}, + ID: "groupA", + AccountID: account.Id, + Name: "GroupA", + Peers: []string{peer1.ID, peer2.ID}, }) assert.NoError(t, err) @@ -623,9 +632,10 @@ func TestGroupAccountPeersUpdate(t *testing.T) { }() err := manager.SaveGroup(context.Background(), account.Id, userID, &nbgroup.Group{ - ID: "groupC", - Name: "GroupC", - Peers: []string{peer1.ID, peer3.ID}, + ID: "groupC", + AccountID: account.Id, + Name: "GroupC", + Peers: []string{peer1.ID, peer3.ID}, }) assert.NoError(t, err) @@ -640,6 +650,7 @@ func TestGroupAccountPeersUpdate(t *testing.T) { t.Run("saving group linked to route", func(t *testing.T) { newRoute := route.Route{ ID: "route", + AccountID: account.Id, Network: netip.MustParsePrefix("192.168.0.0/16"), NetID: "superNet", NetworkType: route.IPv4Network, @@ -664,9 +675,10 @@ func TestGroupAccountPeersUpdate(t *testing.T) { }() err = manager.SaveGroup(context.Background(), account.Id, userID, &nbgroup.Group{ - ID: "groupA", - Name: "GroupA", - Peers: []string{peer1.ID, peer2.ID, peer3.ID}, + ID: "groupA", + AccountID: account.Id, + Name: "GroupA", + Peers: []string{peer1.ID, peer2.ID, peer3.ID}, }) assert.NoError(t, err) @@ -691,9 +703,10 @@ func TestGroupAccountPeersUpdate(t *testing.T) { }() err = manager.SaveGroup(context.Background(), account.Id, userID, &nbgroup.Group{ - ID: "groupD", - Name: "GroupD", - Peers: []string{peer1.ID}, + ID: "groupD", + AccountID: account.Id, + Name: "GroupD", + Peers: []string{peer1.ID}, }) assert.NoError(t, err) diff --git a/management/server/management_proto_test.go b/management/server/management_proto_test.go index dc8765e19..3c3f7d2a0 100644 --- a/management/server/management_proto_test.go +++ b/management/server/management_proto_test.go @@ -461,7 +461,7 @@ func createRawClient(addr string) (mgmtProto.ManagementServiceClient, *grpc.Clie grpc.WithBlock(), grpc.WithKeepaliveParams(keepalive.ClientParameters{ Time: 10 * time.Second, - Timeout: 2 * time.Second, + Timeout: 200 * time.Second, })) if err != nil { return nil, nil, err diff --git a/management/server/nameserver.go b/management/server/nameserver.go index 883150510..149d7e459 100644 --- a/management/server/nameserver.go +++ b/management/server/nameserver.go @@ -114,6 +114,7 @@ func (am *DefaultAccountManager) SaveNameServerGroup(ctx context.Context, accoun if err != nil { return err } + nsGroupToSave.AccountID = accountID if err = am.validateNameServerGroup(ctx, accountID, nsGroupToSave); err != nil { return err diff --git a/management/server/nameserver_test.go b/management/server/nameserver_test.go index e03833535..e0f0de11a 100644 --- a/management/server/nameserver_test.go +++ b/management/server/nameserver_test.go @@ -409,7 +409,7 @@ func TestCreateNameServerGroup(t *testing.T) { // assign generated ID testCase.expectedNSGroup.ID = outNSGroup.ID - + testCase.expectedNSGroup.AccountID = accountID if !testCase.expectedNSGroup.IsEqual(outNSGroup) { t.Errorf("new nameserver group didn't match expected ns group:\nGot %#v\nExpected:%#v\n", outNSGroup, testCase.expectedNSGroup) } @@ -649,7 +649,6 @@ func TestSaveNameServerGroup(t *testing.T) { } err = am.SaveNameServerGroup(context.Background(), accountID, userID, nsGroupToSave) - testCase.errFunc(t, err) if !testCase.shouldCreate { @@ -659,6 +658,7 @@ func TestSaveNameServerGroup(t *testing.T) { savedNSGroup, err := am.Store.GetNameServerGroupByID(context.Background(), LockingStrengthShare, accountID, testCase.expectedNSGroup.ID) require.NoError(t, err, "failed to get saved nameserver group") + testCase.expectedNSGroup.AccountID = accountID if !testCase.expectedNSGroup.IsEqual(savedNSGroup) { t.Errorf("new nameserver group didn't match expected group:\nGot %#v\nExpected:%#v\n", savedNSGroup, testCase.expectedNSGroup) } diff --git a/management/server/peer_test.go b/management/server/peer_test.go index 573e57a21..ea0d7e36f 100644 --- a/management/server/peer_test.go +++ b/management/server/peer_test.go @@ -1438,10 +1438,13 @@ func TestPeerAccountPeersUpdate(t *testing.T) { // Adding peer to group linked with policy should update account peers and send peer update t.Run("adding peer to group linked with policy", func(t *testing.T) { err = manager.SavePolicy(context.Background(), account.Id, userID, &Policy{ - ID: "policy", - Enabled: true, + ID: "policy", + AccountID: account.Id, + Enabled: true, Rules: []*PolicyRule{ { + ID: "rule", + PolicyID: "policy", Enabled: true, Sources: []string{"groupA"}, Destinations: []string{"groupA"}, diff --git a/management/server/policy.go b/management/server/policy.go index 1a9f3b8e2..ba9a31aa1 100644 --- a/management/server/policy.go +++ b/management/server/policy.go @@ -171,6 +171,7 @@ type Policy struct { func (p *Policy) Copy() *Policy { c := &Policy{ ID: p.ID, + AccountID: p.AccountID, Name: p.Name, Description: p.Description, Enabled: p.Enabled, @@ -347,7 +348,7 @@ func (am *DefaultAccountManager) GetPolicy(ctx context.Context, accountID, polic } if user.IsRegularUser() { - return nil, status.NewUnauthorizedToViewPoliciesError() + return nil, status.NewAdminPermissionError() } return am.Store.GetPolicyByID(ctx, LockingStrengthShare, accountID, policyID) @@ -365,7 +366,7 @@ func (am *DefaultAccountManager) SavePolicy(ctx context.Context, accountID, user } if user.IsRegularUser() { - return status.NewUnauthorizedToViewPoliciesError() + return status.NewAdminPermissionError() } groups, err := am.Store.GetAccountGroups(ctx, LockingStrengthShare, accountID) @@ -476,7 +477,7 @@ func (am *DefaultAccountManager) ListPolicies(ctx context.Context, accountID, us } if user.IsRegularUser() { - return nil, status.NewUnauthorizedToViewPoliciesError() + return nil, status.NewAdminPermissionError() } return am.Store.GetAccountPolicies(ctx, LockingStrengthShare, accountID) diff --git a/management/server/policy_test.go b/management/server/policy_test.go index e7f0f9cd2..4c551a019 100644 --- a/management/server/policy_test.go +++ b/management/server/policy_test.go @@ -832,24 +832,28 @@ func TestPolicyAccountPeersUpdate(t *testing.T) { err := manager.SaveGroups(context.Background(), account.Id, userID, []*nbgroup.Group{ { - ID: "groupA", - Name: "GroupA", - Peers: []string{peer1.ID, peer3.ID}, + ID: "groupA", + AccountID: account.Id, + Name: "GroupA", + Peers: []string{peer1.ID, peer3.ID}, }, { - ID: "groupB", - Name: "GroupB", - Peers: []string{}, + ID: "groupB", + AccountID: account.Id, + Name: "GroupB", + Peers: []string{}, }, { - ID: "groupC", - Name: "GroupC", - Peers: []string{}, + ID: "groupC", + AccountID: account.Id, + Name: "GroupC", + Peers: []string{}, }, { - ID: "groupD", - Name: "GroupD", - Peers: []string{peer1.ID, peer2.ID}, + ID: "groupD", + AccountID: account.Id, + Name: "GroupD", + Peers: []string{peer1.ID, peer2.ID}, }, }) assert.NoError(t, err) @@ -862,11 +866,13 @@ func TestPolicyAccountPeersUpdate(t *testing.T) { // Saving policy with rule groups with no peers should not update account's peers and not send peer update t.Run("saving policy with rule groups with no peers", func(t *testing.T) { policy := Policy{ - ID: "policy-rule-groups-no-peers", - Enabled: true, + ID: "policy-rule-groups-no-peers", + AccountID: account.Id, + Enabled: true, Rules: []*PolicyRule{ { ID: xid.New().String(), + PolicyID: "policy-rule-groups-no-peers", Enabled: true, Sources: []string{"groupB"}, Destinations: []string{"groupC"}, @@ -896,11 +902,13 @@ func TestPolicyAccountPeersUpdate(t *testing.T) { // update account's peers and send peer update t.Run("saving policy where source has peers but destination does not", func(t *testing.T) { policy := Policy{ - ID: "policy-source-has-peers-destination-none", - Enabled: true, + ID: "policy-source-has-peers-destination-none", + AccountID: account.Id, + Enabled: true, Rules: []*PolicyRule{ { ID: xid.New().String(), + PolicyID: "policy-source-has-peers-destination-none", Enabled: true, Sources: []string{"groupA"}, Destinations: []string{"groupB"}, @@ -931,11 +939,13 @@ func TestPolicyAccountPeersUpdate(t *testing.T) { // update account's peers and send peer update t.Run("saving policy where destination has peers but source does not", func(t *testing.T) { policy := Policy{ - ID: "policy-destination-has-peers-source-none", - Enabled: true, + ID: "policy-destination-has-peers-source-none", + AccountID: account.Id, + Enabled: true, Rules: []*PolicyRule{ { ID: xid.New().String(), + PolicyID: "policy-destination-has-peers-source-none", Enabled: false, Sources: []string{"groupC"}, Destinations: []string{"groupD"}, @@ -966,11 +976,13 @@ func TestPolicyAccountPeersUpdate(t *testing.T) { // and send peer update t.Run("saving policy with source and destination groups with peers", func(t *testing.T) { policy := Policy{ - ID: "policy-source-destination-peers", - Enabled: true, + ID: "policy-source-destination-peers", + AccountID: account.Id, + Enabled: true, Rules: []*PolicyRule{ { ID: xid.New().String(), + PolicyID: "policy-source-destination-peers", Enabled: true, Sources: []string{"groupA"}, Destinations: []string{"groupD"}, @@ -1000,11 +1012,13 @@ func TestPolicyAccountPeersUpdate(t *testing.T) { // and send peer update t.Run("disabling policy with source and destination groups with peers", func(t *testing.T) { policy := Policy{ - ID: "policy-source-destination-peers", - Enabled: false, + ID: "policy-source-destination-peers", + AccountID: account.Id, + Enabled: false, Rules: []*PolicyRule{ { ID: xid.New().String(), + PolicyID: "policy-source-destination-peers", Enabled: true, Sources: []string{"groupA"}, Destinations: []string{"groupD"}, @@ -1035,11 +1049,13 @@ func TestPolicyAccountPeersUpdate(t *testing.T) { t.Run("updating disabled policy with source and destination groups with peers", func(t *testing.T) { policy := Policy{ ID: "policy-source-destination-peers", + AccountID: account.Id, Description: "updated description", Enabled: false, Rules: []*PolicyRule{ { ID: xid.New().String(), + PolicyID: "policy-source-destination-peers", Enabled: true, Sources: []string{"groupA"}, Destinations: []string{"groupA"}, @@ -1069,11 +1085,13 @@ func TestPolicyAccountPeersUpdate(t *testing.T) { // and send peer update t.Run("enabling policy with source and destination groups with peers", func(t *testing.T) { policy := Policy{ - ID: "policy-source-destination-peers", - Enabled: true, + ID: "policy-source-destination-peers", + AccountID: account.Id, + Enabled: true, Rules: []*PolicyRule{ { ID: xid.New().String(), + PolicyID: "policy-source-destination-peers", Enabled: true, Sources: []string{"groupA"}, Destinations: []string{"groupD"}, diff --git a/management/server/posture/checks.go b/management/server/posture/checks.go index 16d87b7bf..f2739dddf 100644 --- a/management/server/posture/checks.go +++ b/management/server/posture/checks.go @@ -41,7 +41,7 @@ type Checks struct { ID string `gorm:"primaryKey"` // Name of the posture checks - Name string `gorm:"unique"` + Name string // Description of the posture checks visible in the UI Description string diff --git a/management/server/posture_checks.go b/management/server/posture_checks.go index 2c1378944..8e82e9292 100644 --- a/management/server/posture_checks.go +++ b/management/server/posture_checks.go @@ -23,7 +23,7 @@ func (am *DefaultAccountManager) GetPostureChecks(ctx context.Context, accountID } if !user.HasAdminPower() { - return nil, status.NewUnauthorizedToViewPostureChecksError() + return nil, status.NewAdminPermissionError() } return am.Store.GetPostureChecksByID(ctx, LockingStrengthShare, accountID, postureChecksID) @@ -41,7 +41,7 @@ func (am *DefaultAccountManager) SavePostureChecks(ctx context.Context, accountI } if !user.HasAdminPower() { - return status.NewUnauthorizedToViewPostureChecksError() + return status.NewAdminPermissionError() } if err = am.validatePostureChecks(ctx, accountID, postureChecks); err != nil { @@ -116,7 +116,7 @@ func (am *DefaultAccountManager) DeletePostureChecks(ctx context.Context, accoun } if !user.HasAdminPower() { - return status.NewUnauthorizedToViewPostureChecksError() + return status.NewAdminPermissionError() } postureChecks, err := am.Store.GetPostureChecksByID(ctx, LockingStrengthShare, accountID, postureChecksID) @@ -159,7 +159,7 @@ func (am *DefaultAccountManager) ListPostureChecks(ctx context.Context, accountI } if !user.HasAdminPower() { - return nil, status.NewUnauthorizedToViewPostureChecksError() + return nil, status.NewAdminPermissionError() } return am.Store.GetAccountPostureChecks(ctx, LockingStrengthShare, accountID) diff --git a/management/server/posture_checks_test.go b/management/server/posture_checks_test.go index c4a6525c1..decdc8d3e 100644 --- a/management/server/posture_checks_test.go +++ b/management/server/posture_checks_test.go @@ -5,7 +5,7 @@ import ( "testing" "time" - "github.com/rs/xid" + "github.com/netbirdio/netbird/management/server/status" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -43,8 +43,9 @@ func TestDefaultAccountManager_PostureCheck(t *testing.T) { // should be possible to create posture check with uniq name err = am.SavePostureChecks(context.Background(), accountID, adminUserID, &posture.Checks{ - ID: postureCheckID, - Name: postureCheckName, + ID: postureCheckID, + AccountID: accountID, + Name: postureCheckName, Checks: posture.ChecksDefinition{ NBVersionCheck: &posture.NBVersionCheck{ MinVersion: "0.26.0", @@ -60,8 +61,9 @@ func TestDefaultAccountManager_PostureCheck(t *testing.T) { // should not be possible to create posture check with non uniq name err = am.SavePostureChecks(context.Background(), accountID, adminUserID, &posture.Checks{ - ID: "new-id", - Name: postureCheckName, + ID: "new-id", + AccountID: accountID, + Name: postureCheckName, Checks: posture.ChecksDefinition{ GeoLocationCheck: &posture.GeoLocationCheck{ Locations: []posture.Location{ @@ -76,8 +78,9 @@ func TestDefaultAccountManager_PostureCheck(t *testing.T) { // admins can update posture checks err = am.SavePostureChecks(context.Background(), accountID, adminUserID, &posture.Checks{ - ID: postureCheckID, - Name: postureCheckName, + ID: postureCheckID, + AccountID: accountID, + Name: postureCheckName, Checks: posture.ChecksDefinition{ NBVersionCheck: &posture.NBVersionCheck{ MinVersion: "0.27.0", @@ -132,19 +135,22 @@ func TestPostureCheckAccountPeersUpdate(t *testing.T) { err := manager.SaveGroups(context.Background(), account.Id, userID, []*group.Group{ { - ID: "groupA", - Name: "GroupA", - Peers: []string{peer1.ID, peer2.ID, peer3.ID}, + ID: "groupA", + AccountID: account.Id, + Name: "GroupA", + Peers: []string{peer1.ID, peer2.ID, peer3.ID}, }, { - ID: "groupB", - Name: "GroupB", - Peers: []string{}, + ID: "groupB", + AccountID: account.Id, + Name: "GroupB", + Peers: []string{}, }, { - ID: "groupC", - Name: "GroupC", - Peers: []string{}, + ID: "groupC", + AccountID: account.Id, + Name: "GroupC", + Peers: []string{}, }, }) assert.NoError(t, err) @@ -207,11 +213,13 @@ func TestPostureCheckAccountPeersUpdate(t *testing.T) { }) policy := Policy{ - ID: "policyA", - Enabled: true, + ID: "policyA", + AccountID: account.Id, + Enabled: true, Rules: []*PolicyRule{ { - ID: xid.New().String(), + ID: "ruleA", + PolicyID: "policyA", Enabled: true, Sources: []string{"groupA"}, Destinations: []string{"groupA"}, @@ -313,11 +321,13 @@ func TestPostureCheckAccountPeersUpdate(t *testing.T) { // Updating linked posture check to policy with no peers should not trigger account peers update and not send peer update t.Run("updating linked posture check to policy with no peers", func(t *testing.T) { policy = Policy{ - ID: "policyB", - Enabled: true, + ID: "policyB", + AccountID: account.Id, + Enabled: true, Rules: []*PolicyRule{ { - ID: xid.New().String(), + ID: "ruleB", + PolicyID: "policyB", Enabled: true, Sources: []string{"groupB"}, Destinations: []string{"groupC"}, @@ -359,11 +369,13 @@ func TestPostureCheckAccountPeersUpdate(t *testing.T) { manager.peersUpdateManager.CloseChannel(context.Background(), peer2.ID) }) policy = Policy{ - ID: "policyB", - Enabled: true, + ID: "policyB", + AccountID: account.Id, + Enabled: true, Rules: []*PolicyRule{ { - ID: xid.New().String(), + ID: "ruleB", + PolicyID: "policyB", Enabled: true, Sources: []string{"groupB"}, Destinations: []string{"groupA"}, @@ -402,10 +414,13 @@ func TestPostureCheckAccountPeersUpdate(t *testing.T) { // should trigger account peers update and send peer update t.Run("updating linked posture check to policy where source has peers but destination does not", func(t *testing.T) { policy = Policy{ - ID: "policyB", - Enabled: true, + ID: "policyB", + AccountID: account.Id, + Enabled: true, Rules: []*PolicyRule{ { + ID: "ruleB", + PolicyID: "policyB", Enabled: true, Sources: []string{"groupA"}, Destinations: []string{"groupB"}, @@ -458,7 +473,7 @@ func TestArePostureCheckChangesAffectingPeers(t *testing.T) { } groupB := &group.Group{ - ID: "groupA", + ID: "groupB", AccountID: accountID, Peers: []string{}, } @@ -470,6 +485,8 @@ func TestArePostureCheckChangesAffectingPeers(t *testing.T) { AccountID: accountID, Rules: []*PolicyRule{ { + ID: "ruleA", + PolicyID: "policyA", Enabled: true, Sources: []string{"groupA"}, Destinations: []string{"groupA"}, @@ -482,16 +499,24 @@ func TestArePostureCheckChangesAffectingPeers(t *testing.T) { postureCheckA := &posture.Checks{ ID: "checkA", + Name: "checkA", AccountID: accountID, + Checks: posture.ChecksDefinition{ + NBVersionCheck: &posture.NBVersionCheck{MinVersion: "0.33.1"}, + }, } - err = manager.Store.SavePostureChecks(context.Background(), LockingStrengthUpdate, postureCheckA) + err = manager.SavePostureChecks(context.Background(), accountID, adminUserID, postureCheckA, false) require.NoError(t, err, "failed to save postureCheckA") postureCheckB := &posture.Checks{ ID: "checkB", + Name: "checkB", AccountID: accountID, + Checks: posture.ChecksDefinition{ + NBVersionCheck: &posture.NBVersionCheck{MinVersion: "0.33.1"}, + }, } - err = manager.Store.SavePostureChecks(context.Background(), LockingStrengthUpdate, postureCheckB) + err = manager.SavePostureChecks(context.Background(), accountID, adminUserID, postureCheckB, false) require.NoError(t, err, "failed to save postureCheckB") t.Run("posture check exists and is linked to policy with peers", func(t *testing.T) { @@ -534,17 +559,6 @@ func TestArePostureCheckChangesAffectingPeers(t *testing.T) { assert.True(t, result) }) - t.Run("posture check is linked to policy with non-existent group", func(t *testing.T) { - policy.Rules[0].Sources = []string{"nonExistentGroup"} - policy.Rules[0].Destinations = []string{"nonExistentGroup"} - err = manager.Store.SavePolicy(context.Background(), LockingStrengthUpdate, policy) - require.NoError(t, err, "failed to update policy") - - result, err := manager.arePostureCheckChangesAffectPeers(context.Background(), accountID, "checkA", true) - require.NoError(t, err) - assert.False(t, result) - }) - t.Run("posture check is linked to policy but no peers in groups", func(t *testing.T) { groupA.Peers = []string{} err = manager.Store.SaveGroup(context.Background(), LockingStrengthUpdate, groupA) @@ -554,4 +568,18 @@ func TestArePostureCheckChangesAffectingPeers(t *testing.T) { require.NoError(t, err) assert.False(t, result) }) + + t.Run("posture check is linked to policy with non-existent group", func(t *testing.T) { + policy.Rules[0].Sources = []string{"nonExistentGroup"} + policy.Rules[0].Destinations = []string{"nonExistentGroup"} + err = manager.Store.SavePolicy(context.Background(), LockingStrengthUpdate, policy) + require.NoError(t, err, "failed to update policy") + + result, err := manager.arePostureCheckChangesAffectPeers(context.Background(), accountID, "checkA", true) + require.Error(t, err) + sErr, ok := status.FromError(err) + require.True(t, ok) + require.Equal(t, status.NotFound, sErr.Type()) + assert.False(t, result) + }) } diff --git a/management/server/route.go b/management/server/route.go index 60914e8e1..246fea85b 100644 --- a/management/server/route.go +++ b/management/server/route.go @@ -57,7 +57,7 @@ func (am *DefaultAccountManager) GetRoute(ctx context.Context, accountID string, } if user.IsRegularUser() { - return nil, status.NewUnauthorizedToViewRoutesError() + return nil, status.NewAdminPermissionError() } return am.Store.GetRouteByID(ctx, LockingStrengthShare, accountID, string(routeID)) @@ -137,7 +137,7 @@ func (am *DefaultAccountManager) checkRoutePrefixOrDomainsExistForPeers(ctx cont // check that peerGroupIDs are not in any route peerGroups list for _, groupID := range peerGroupIDs { // we validated the group existence before entering this function, no need to check again. - group, err := am.Store.GetGroupByID(context.Background(), LockingStrengthShare, groupID, accountID) + group, err := am.Store.GetGroupByID(context.Background(), LockingStrengthShare, accountID, groupID) if err != nil || group == nil { return status.Errorf(status.InvalidArgument, "group with ID %s not found", peerID) } @@ -151,7 +151,7 @@ func (am *DefaultAccountManager) checkRoutePrefixOrDomainsExistForPeers(ctx cont // check that the peers from peerGroupIDs groups are not the same peers we saw in routesWithPrefix for _, id := range group.Peers { if _, ok := seenPeers[id]; ok { - peer, err := am.Store.GetPeerByID(context.Background(), LockingStrengthShare, peerID, accountID) + peer, err := am.Store.GetPeerByID(context.Background(), LockingStrengthShare, accountID, peerID) if err != nil { return status.Errorf(status.InvalidArgument, "peer with ID %s not found", peerID) } @@ -217,6 +217,7 @@ func (am *DefaultAccountManager) CreateRoute(ctx context.Context, accountID stri var newRoute route.Route newRoute.ID = route.ID(xid.New().String()) + newRoute.AccountID = accountID accountGroups, err := am.Store.GetAccountGroups(ctx, LockingStrengthShare, accountID) if err != nil { @@ -393,6 +394,7 @@ func (am *DefaultAccountManager) SaveRoute(ctx context.Context, accountID, userI if err != nil { return err } + routeToSave.AccountID = accountID err = am.Store.ExecuteInTransaction(ctx, func(transaction Store) error { if err = transaction.IncrementNetworkSerial(ctx, LockingStrengthUpdate, accountID); err != nil { @@ -472,7 +474,7 @@ func (am *DefaultAccountManager) ListRoutes(ctx context.Context, accountID, user } if user.IsRegularUser() { - return nil, status.NewUnauthorizedToViewRoutesError() + return nil, status.NewAdminPermissionError() } return am.Store.GetAccountRoutes(ctx, LockingStrengthShare, accountID) diff --git a/management/server/setupkey.go b/management/server/setupkey.go index a3d39a217..78805347a 100644 --- a/management/server/setupkey.go +++ b/management/server/setupkey.go @@ -366,7 +366,7 @@ func (am *DefaultAccountManager) ListSetupKeys(ctx context.Context, accountID, u } if user.IsRegularUser() { - return nil, status.NewUnauthorizedToViewSetupKeysError() + return nil, status.NewAdminPermissionError() } setupKeys, err := am.Store.GetAccountSetupKeys(ctx, LockingStrengthShare, accountID) @@ -389,7 +389,7 @@ func (am *DefaultAccountManager) GetSetupKey(ctx context.Context, accountID, use } if user.IsRegularUser() { - return nil, status.NewUnauthorizedToViewSetupKeysError() + return nil, status.NewAdminPermissionError() } setupKey, err := am.Store.GetSetupKeyByID(ctx, LockingStrengthShare, keyID, accountID) @@ -417,7 +417,7 @@ func (am *DefaultAccountManager) DeleteSetupKey(ctx context.Context, accountID, } if user.IsRegularUser() { - return status.NewUnauthorizedToViewSetupKeysError() + return status.NewAdminPermissionError() } deletedSetupKey, err := am.Store.GetSetupKeyByID(ctx, LockingStrengthShare, keyID, accountID) diff --git a/management/server/status/error.go b/management/server/status/error.go index 7c794dfc4..5ddcd2531 100644 --- a/management/server/status/error.go +++ b/management/server/status/error.go @@ -131,10 +131,6 @@ func NewOwnerDeletePermissionError() error { return Errorf(PermissionDenied, "can't delete a user with the owner role") } -func NewUnauthorizedToViewServiceUsersError() error { - return Errorf(PermissionDenied, "only users with admin power can view service users") -} - // NewServiceUserRoleInvalidError creates a new Error with InvalidArgument type for creating a service user with owner role func NewServiceUserRoleInvalidError() error { return Errorf(InvalidArgument, "can't create a service user with owner role") @@ -150,19 +146,6 @@ func NewSetupKeyNotFoundError(err error) error { return Errorf(NotFound, "setup key not found: %s", err) } -// NewUnauthorizedToViewSetupKeysError creates a new Error with Unauthorized type for an issue getting a setup key -func NewUnauthorizedToViewSetupKeysError() error { - return Errorf(PermissionDenied, "only users with admin power can view setup keys") -} - -func NewGroupNotFoundError() error { - return Errorf(NotFound, "group not found") -} - -func NewUnauthorizedToViewGroupsError() error { - return Errorf(PermissionDenied, "only users with admin power can view groups") -} - func NewPATNotFoundError() error { return Errorf(NotFound, "PAT not found") } @@ -171,26 +154,10 @@ func NewGetPATFromStoreError() error { return Errorf(Internal, "issue getting pat from store") } -func NewUnauthorizedToViewPoliciesError() error { - return Errorf(PermissionDenied, "only users with admin power can view policies") -} - -func NewUnauthorizedToViewPostureChecksError() error { - return Errorf(PermissionDenied, "only users with admin power can view posture checks") -} - -func NewUnauthorizedToViewDNSSettingsError() error { - return Errorf(PermissionDenied, "only users with admin power can view dns settings") -} - func NewUnauthorizedToViewNSGroupsError() error { return Errorf(PermissionDenied, "only users with admin power can view name server groups") } -func NewUnauthorizedToViewRoutesError() error { - return Errorf(PermissionDenied, "only users with admin power can view network routes") -} - // NewStoreContextCanceledError creates a new Error with Internal type for a canceled store context func NewStoreContextCanceledError(duration time.Duration) error { return Errorf(Internal, "store access: context canceled after %v", duration) diff --git a/management/server/testdata/store_with_expired_peers.sql b/management/server/testdata/store_with_expired_peers.sql index 100a6470f..bf1b0b0e2 100644 --- a/management/server/testdata/store_with_expired_peers.sql +++ b/management/server/testdata/store_with_expired_peers.sql @@ -32,4 +32,7 @@ INSERT INTO peers VALUES('cg05lnblo1hkg2j514p0','bf1c8084-ba50-4ce7-9439-3465300 INSERT INTO peers VALUES('cg3161rlo1hs9cq94gdg','bf1c8084-ba50-4ce7-9439-34653001fc3b','mVABSKj28gv+JRsf7e0NEGKgSOGTfU/nPB2cpuG56HU=','','"100.64.117.96"','testhost','linux','Linux','22.04','x86_64','Ubuntu','','development','','',NULL,'','','','{"Cloud":"","Platform":""}',NULL,'testhost','testhost','2023-03-06 18:21:27.252010027+01:00',0,0,0,'edafee4e-63fb-11ec-90d6-0242ac120003','ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAINWvvUkFFcrj48CWTkNUb/do/n52i1L5dH4DhGu+4ZuM',0,0,'2023-03-07 09:02:47.442857106+01:00','2024-10-02 17:00:32.527947+02:00',0,'""','','',0); INSERT INTO users VALUES('f4f6d672-63fb-11ec-90d6-0242ac120003','bf1c8084-ba50-4ce7-9439-34653001fc3b','user',0,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 17:00:32.528196+02:00','api',0,''); INSERT INTO users VALUES('edafee4e-63fb-11ec-90d6-0242ac120003','bf1c8084-ba50-4ce7-9439-34653001fc3b','admin',0,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 17:00:32.528196+02:00','api',0,''); +INSERT INTO "groups" VALUES('cs1tnh0hhcjnqoiuebeg','bf1c8084-ba50-4ce7-9439-34653001fc3b','All','api','["cfvprsrlo1hqoo49ohog", "cg3161rlo1hs9cq94gdg", "cg05lnblo1hkg2j514p0"]',0,''); +INSERT INTO policies VALUES('cs1tnh0hhcjnqoiuebf0','bf1c8084-ba50-4ce7-9439-34653001fc3b','Default','This is a default rule that allows connections between all the resources',1,'[]'); +INSERT INTO policy_rules VALUES('cs387mkv2d4bgq41b6n0','cs1tnh0hhcjnqoiuebf0','Default','This is a default rule that allows connections between all the resources',1,'accept','["cs1tnh0hhcjnqoiuebeg"]','["cs1tnh0hhcjnqoiuebeg"]',1,'all',NULL,NULL); INSERT INTO installations VALUES(1,''); diff --git a/management/server/user.go b/management/server/user.go index e7a3708e4..bd72e04a0 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -231,7 +231,7 @@ func (am *DefaultAccountManager) createServiceUser(ctx context.Context, accountI } if !initiatorUser.HasAdminPower() { - return nil, status.NewUnauthorizedToViewServiceUsersError() + return nil, status.NewAdminPermissionError() } if role == UserRoleOwner { diff --git a/route/route.go b/route/route.go index e23801e6e..964850640 100644 --- a/route/route.go +++ b/route/route.go @@ -88,18 +88,18 @@ type Route struct { // AccountID is a reference to Account that this object belongs AccountID string `gorm:"index"` // Network and Domains are mutually exclusive - Network netip.Prefix `gorm:"serializer:json"` - Domains domain.List `gorm:"serializer:json"` - KeepRoute bool - NetID NetID - Description string - Peer string - PeerGroups []string `gorm:"serializer:json"` - NetworkType NetworkType - Masquerade bool - Metric int - Enabled bool - Groups []string `gorm:"serializer:json"` + Network netip.Prefix `gorm:"serializer:json"` + Domains domain.List `gorm:"serializer:json"` + KeepRoute bool + NetID NetID + Description string + Peer string + PeerGroups []string `gorm:"serializer:json"` + NetworkType NetworkType + Masquerade bool + Metric int + Enabled bool + Groups []string `gorm:"serializer:json"` AccessControlGroups []string `gorm:"serializer:json"` } @@ -111,19 +111,20 @@ func (r *Route) EventMeta() map[string]any { // Copy copies a route object func (r *Route) Copy() *Route { route := &Route{ - ID: r.ID, - Description: r.Description, - NetID: r.NetID, - Network: r.Network, - Domains: slices.Clone(r.Domains), - KeepRoute: r.KeepRoute, - NetworkType: r.NetworkType, - Peer: r.Peer, - PeerGroups: slices.Clone(r.PeerGroups), - Metric: r.Metric, - Masquerade: r.Masquerade, - Enabled: r.Enabled, - Groups: slices.Clone(r.Groups), + ID: r.ID, + AccountID: r.AccountID, + Description: r.Description, + NetID: r.NetID, + Network: r.Network, + Domains: slices.Clone(r.Domains), + KeepRoute: r.KeepRoute, + NetworkType: r.NetworkType, + Peer: r.Peer, + PeerGroups: slices.Clone(r.PeerGroups), + Metric: r.Metric, + Masquerade: r.Masquerade, + Enabled: r.Enabled, + Groups: slices.Clone(r.Groups), AccessControlGroups: slices.Clone(r.AccessControlGroups), } return route @@ -138,6 +139,7 @@ func (r *Route) IsEqual(other *Route) bool { } return other.ID == r.ID && + other.AccountID == r.AccountID && other.Description == r.Description && other.NetID == r.NetID && other.Network == r.Network && @@ -149,7 +151,7 @@ func (r *Route) IsEqual(other *Route) bool { other.Masquerade == r.Masquerade && other.Enabled == r.Enabled && slices.Equal(r.Groups, other.Groups) && - slices.Equal(r.PeerGroups, other.PeerGroups)&& + slices.Equal(r.PeerGroups, other.PeerGroups) && slices.Equal(r.AccessControlGroups, other.AccessControlGroups) }