Skip to content

Commit

Permalink
Allow removal of preshared keys (#1385)
Browse files Browse the repository at this point in the history
* update cli commands to respect an empty string and handle different from undefined

* remove test for unintended behaviour

* remove test for unintended behaviour
  • Loading branch information
pascal-fischer authored Dec 14, 2023
1 parent 19fa071 commit f73a2e2
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 26 deletions.
2 changes: 1 addition & 1 deletion client/cmd/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ var loginCmd = &cobra.Command{
AdminURL: adminURL,
ConfigPath: configPath,
}
if preSharedKey != "" {
if rootCmd.PersistentFlags().Changed(preSharedKeyFlag) {
ic.PreSharedKey = &preSharedKey
}

Expand Down
3 changes: 2 additions & 1 deletion client/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

const (
externalIPMapFlag = "external-ip-map"
preSharedKeyFlag = "preshared-key"
dnsResolverAddress = "dns-resolver-address"
)

Expand Down Expand Up @@ -94,7 +95,7 @@ func init() {
rootCmd.PersistentFlags().StringVarP(&logLevel, "log-level", "l", "info", "sets Netbird log level")
rootCmd.PersistentFlags().StringVar(&logFile, "log-file", defaultLogFile, "sets Netbird log path. If console is specified the log will be output to stdout")
rootCmd.PersistentFlags().StringVarP(&setupKey, "setup-key", "k", "", "Setup key obtained from the Management Service Dashboard (used to register peer)")
rootCmd.PersistentFlags().StringVar(&preSharedKey, "preshared-key", "", "Sets Wireguard PreSharedKey property. If set, then only peers that have the same key can communicate.")
rootCmd.PersistentFlags().StringVar(&preSharedKey, preSharedKeyFlag, "", "Sets Wireguard PreSharedKey property. If set, then only peers that have the same key can communicate.")
rootCmd.PersistentFlags().StringVarP(&hostName, "hostname", "n", "", "Sets a custom hostname for the device")
rootCmd.AddCommand(serviceCmd)
rootCmd.AddCommand(upCmd)
Expand Down
3 changes: 2 additions & 1 deletion client/cmd/up.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ func runInForegroundMode(ctx context.Context, cmd *cobra.Command) error {
NATExternalIPs: natExternalIPs,
CustomDNSAddress: customDNSAddressConverted,
}
if preSharedKey != "" {

if rootCmd.PersistentFlags().Changed(preSharedKeyFlag) {
ic.PreSharedKey = &preSharedKey
}

Expand Down
9 changes: 3 additions & 6 deletions client/internal/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,12 +215,9 @@ func update(input ConfigInput) (*Config, error) {
}

if input.PreSharedKey != nil && config.PreSharedKey != *input.PreSharedKey {
if *input.PreSharedKey != "" {
log.Infof("new pre-shared key provides, updated to %s (old value %s)",
*input.PreSharedKey, config.PreSharedKey)
config.PreSharedKey = *input.PreSharedKey
refresh = true
}
log.Infof("new pre-shared key provided, replacing old key")
config.PreSharedKey = *input.PreSharedKey
refresh = true
}

if config.SSHKey == "" {
Expand Down
20 changes: 3 additions & 17 deletions client/internal/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ import (
"path/filepath"
"testing"

"github.com/netbirdio/netbird/util"
"github.com/stretchr/testify/assert"

"github.com/netbirdio/netbird/util"
)

func TestGetConfig(t *testing.T) {
Expand Down Expand Up @@ -60,22 +61,7 @@ func TestGetConfig(t *testing.T) {
assert.Equal(t, config.ManagementURL.String(), managementURL)
assert.Equal(t, config.PreSharedKey, preSharedKey)

// case 4: new empty pre-shared key config -> fetch it
newPreSharedKey := ""
config, err = UpdateOrCreateConfig(ConfigInput{
ManagementURL: managementURL,
AdminURL: adminURL,
ConfigPath: path,
PreSharedKey: &newPreSharedKey,
})
if err != nil {
return
}

assert.Equal(t, config.ManagementURL.String(), managementURL)
assert.Equal(t, config.PreSharedKey, preSharedKey)

// case 5: existing config, but new managementURL has been provided -> update config
// case 4: existing config, but new managementURL has been provided -> update config
newManagementURL := "https://test.newManagement.url:33071"
config, err = UpdateOrCreateConfig(ConfigInput{
ManagementURL: newManagementURL,
Expand Down

0 comments on commit f73a2e2

Please sign in to comment.