Fix nil pointer exception in group delete (#1211)

Fix group delete panic

In case if in the db the DNSSettings is null then can cause panic in delete group function
because this field is pointer and it was not checked. Because of in the future implementation
this variable will be filled in any case then make no sense to keep the pointer type.

Fix DNSSettings copy function
This commit is contained in:
Zoltan Papp 2023-10-11 23:00:56 +02:00 committed by GitHub
parent 659110f0d5
commit b8599f634c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 19 additions and 40 deletions

View File

@ -180,7 +180,7 @@ type Account struct {
Policies []*Policy Policies []*Policy
Routes map[string]*route.Route Routes map[string]*route.Route
NameServerGroups map[string]*nbdns.NameServerGroup NameServerGroups map[string]*nbdns.NameServerGroup
DNSSettings *DNSSettings DNSSettings DNSSettings
// Settings is a dictionary of Account settings // Settings is a dictionary of Account settings
Settings *Settings Settings *Settings
} }
@ -513,13 +513,11 @@ func (a *Account) getUserGroups(userID string) ([]string, error) {
func (a *Account) getPeerDNSManagementStatus(peerID string) bool { func (a *Account) getPeerDNSManagementStatus(peerID string) bool {
peerGroups := a.getPeerGroups(peerID) peerGroups := a.getPeerGroups(peerID)
enabled := true enabled := true
if a.DNSSettings != nil { for _, groupID := range a.DNSSettings.DisabledManagementGroups {
for _, groupID := range a.DNSSettings.DisabledManagementGroups { _, found := peerGroups[groupID]
_, found := peerGroups[groupID] if found {
if found { enabled = false
enabled = false break
break
}
} }
} }
return enabled return enabled
@ -606,10 +604,7 @@ func (a *Account) Copy() *Account {
nsGroups[id] = nsGroup.Copy() nsGroups[id] = nsGroup.Copy()
} }
var dnsSettings *DNSSettings dnsSettings := a.DNSSettings.Copy()
if a.DNSSettings != nil {
dnsSettings = a.DNSSettings.Copy()
}
var settings *Settings var settings *Settings
if a.Settings != nil { if a.Settings != nil {
@ -1618,7 +1613,7 @@ func newAccountWithId(accountID, userID, domain string) *Account {
setupKeys := map[string]*SetupKey{} setupKeys := map[string]*SetupKey{}
nameServersGroups := make(map[string]*nbdns.NameServerGroup) nameServersGroups := make(map[string]*nbdns.NameServerGroup)
users[userID] = NewAdminUser(userID) users[userID] = NewAdminUser(userID)
dnsSettings := &DNSSettings{ dnsSettings := DNSSettings{
DisabledManagementGroups: make([]string, 0), DisabledManagementGroups: make([]string, 0),
} }
log.Debugf("created new account %s", accountID) log.Debugf("created new account %s", accountID)

View File

@ -1374,7 +1374,7 @@ func TestAccount_Copy(t *testing.T) {
NameServers: []nbdns.NameServer{}, NameServers: []nbdns.NameServer{},
}, },
}, },
DNSSettings: &DNSSettings{DisabledManagementGroups: []string{}}, DNSSettings: DNSSettings{DisabledManagementGroups: []string{}},
Settings: &Settings{}, Settings: &Settings{},
} }
err := hasNilField(account) err := hasNilField(account)

View File

@ -24,19 +24,11 @@ type DNSSettings struct {
} }
// Copy returns a copy of the DNS settings // Copy returns a copy of the DNS settings
func (d *DNSSettings) Copy() *DNSSettings { func (d DNSSettings) Copy() DNSSettings {
settings := &DNSSettings{ settings := DNSSettings{
DisabledManagementGroups: make([]string, 0), DisabledManagementGroups: make([]string, len(d.DisabledManagementGroups)),
} }
copy(settings.DisabledManagementGroups, d.DisabledManagementGroups)
if d == nil {
return settings
}
if d.DisabledManagementGroups != nil && len(d.DisabledManagementGroups) > 0 {
settings.DisabledManagementGroups = d.DisabledManagementGroups[:]
}
return settings return settings
} }
@ -58,12 +50,8 @@ func (am *DefaultAccountManager) GetDNSSettings(accountID string, userID string)
if !user.IsAdmin() { if !user.IsAdmin() {
return nil, status.Errorf(status.PermissionDenied, "only admins are allowed to view DNS settings") return nil, status.Errorf(status.PermissionDenied, "only admins are allowed to view DNS settings")
} }
dnsSettings := account.DNSSettings.Copy()
if account.DNSSettings == nil { return &dnsSettings, nil
return &DNSSettings{}, nil
}
return account.DNSSettings.Copy(), nil
} }
// SaveDNSSettings validates a user role and updates the account's DNS settings // SaveDNSSettings validates a user role and updates the account's DNS settings
@ -96,11 +84,7 @@ func (am *DefaultAccountManager) SaveDNSSettings(accountID string, userID string
} }
} }
oldSettings := &DNSSettings{} oldSettings := account.DNSSettings.Copy()
if account.DNSSettings != nil {
oldSettings = account.DNSSettings.Copy()
}
account.DNSSettings = dnsSettingsToSave.Copy() account.DNSSettings = dnsSettingsToSave.Copy()
account.Network.IncSerial() account.Network.IncSerial()

View File

@ -42,7 +42,7 @@ func TestGetDNSSettings(t *testing.T) {
t.Fatal("DNS settings for new accounts shouldn't return nil") t.Fatal("DNS settings for new accounts shouldn't return nil")
} }
account.DNSSettings = &DNSSettings{ account.DNSSettings = DNSSettings{
DisabledManagementGroups: []string{group1ID}, DisabledManagementGroups: []string{group1ID},
} }

View File

@ -26,7 +26,7 @@ const (
testDNSSettingsUserID = "test_user" testDNSSettingsUserID = "test_user"
) )
var baseExistingDNSSettings = &server.DNSSettings{ var baseExistingDNSSettings = server.DNSSettings{
DisabledManagementGroups: []string{testDNSSettingsExistingGroup}, DisabledManagementGroups: []string{testDNSSettingsExistingGroup},
} }
@ -43,7 +43,7 @@ func initDNSSettingsTestData() *DNSSettingsHandler {
return &DNSSettingsHandler{ return &DNSSettingsHandler{
accountManager: &mock_server.MockAccountManager{ accountManager: &mock_server.MockAccountManager{
GetDNSSettingsFunc: func(accountID string, userID string) (*server.DNSSettings, error) { GetDNSSettingsFunc: func(accountID string, userID string) (*server.DNSSettings, error) {
return testingDNSSettingsAccount.DNSSettings, nil return &testingDNSSettingsAccount.DNSSettings, nil
}, },
SaveDNSSettingsFunc: func(accountID string, userID string, dnsSettingsToSave *server.DNSSettings) error { SaveDNSSettingsFunc: func(accountID string, userID string, dnsSettingsToSave *server.DNSSettings) error {
if dnsSettingsToSave != nil { if dnsSettingsToSave != nil {