Browse Source

cscli capi status -> message for missing credentials (#2730)

* cscli capi status -> message for missing credentials
* lint
mmetc 1 year ago
parent
commit
fca8883cd9
4 changed files with 75 additions and 24 deletions
  1. 12 13
      cmd/crowdsec-cli/capi.go
  2. 33 3
      pkg/csconfig/api.go
  3. 2 1
      pkg/csconfig/api_test.go
  4. 28 7
      test/bats/04_capi.bats

+ 12 - 13
cmd/crowdsec-cli/capi.go

@@ -38,7 +38,7 @@ func (cli cliCapi) NewCommand() *cobra.Command {
 		Short:             "Manage interaction with Central API (CAPI)",
 		Args:              cobra.MinimumNArgs(1),
 		DisableAutoGenTag: true,
-		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
+		PersistentPreRunE: func(_ *cobra.Command, _ []string) error {
 			if err := require.LAPI(csConfig); err != nil {
 				return err
 			}
@@ -58,15 +58,17 @@ func (cli cliCapi) NewCommand() *cobra.Command {
 }
 
 func (cli cliCapi) NewRegisterCmd() *cobra.Command {
-	var capiUserPrefix string
-	var outputFile string
+	var (
+		capiUserPrefix string
+		outputFile string
+	)
 
 	var cmd = &cobra.Command{
 		Use:               "register",
 		Short:             "Register to Central API (CAPI)",
 		Args:              cobra.MinimumNArgs(0),
 		DisableAutoGenTag: true,
-		RunE: func(cmd *cobra.Command, args []string) error {
+		RunE: func(_ *cobra.Command, _ []string) error {
 			var err error
 			capiUser, err := generateID(capiUserPrefix)
 			if err != nil {
@@ -115,7 +117,7 @@ func (cli cliCapi) NewRegisterCmd() *cobra.Command {
 				}
 				log.Printf("Central API credentials written to '%s'", dumpFile)
 			} else {
-				fmt.Printf("%s\n", string(apiConfigDump))
+				fmt.Println(string(apiConfigDump))
 			}
 
 			log.Warning(ReloadMessage())
@@ -126,6 +128,7 @@ func (cli cliCapi) NewRegisterCmd() *cobra.Command {
 
 	cmd.Flags().StringVarP(&outputFile, "file", "f", "", "output file destination")
 	cmd.Flags().StringVar(&capiUserPrefix, "schmilblick", "", "set a schmilblick (use in tests only)")
+
 	if err := cmd.Flags().MarkHidden("schmilblick"); err != nil {
 		log.Fatalf("failed to hide flag: %s", err)
 	}
@@ -134,18 +137,14 @@ func (cli cliCapi) NewRegisterCmd() *cobra.Command {
 }
 
 func (cli cliCapi) NewStatusCmd() *cobra.Command {
-	var cmd = &cobra.Command{
+	cmd := &cobra.Command{
 		Use:               "status",
 		Short:             "Check status with the Central API (CAPI)",
 		Args:              cobra.MinimumNArgs(0),
 		DisableAutoGenTag: true,
-		RunE: func(cmd *cobra.Command, args []string) error {
-			if csConfig.API.Server.OnlineClient == nil {
-				return fmt.Errorf("please provide credentials for the Central API (CAPI) in '%s'", csConfig.API.Server.OnlineClient.CredentialsFilePath)
-			}
-
-			if csConfig.API.Server.OnlineClient.Credentials == nil {
-				return fmt.Errorf("no credentials for Central API (CAPI) in '%s'", csConfig.API.Server.OnlineClient.CredentialsFilePath)
+		RunE: func(_ *cobra.Command, _ []string) error {
+			if err := require.CAPIRegistered(csConfig); err != nil {
+				return err
 			}
 
 			password := strfmt.Password(csConfig.API.Server.OnlineClient.Credentials.Password)

+ 33 - 3
pkg/csconfig/api.go

@@ -61,36 +61,51 @@ func (a *CTICfg) Load() error {
 	if a.Key == nil {
 		*a.Enabled = false
 	}
+
 	if a.Key != nil && *a.Key == "" {
 		return fmt.Errorf("empty cti key")
 	}
+
 	if a.Enabled == nil {
 		a.Enabled = new(bool)
 		*a.Enabled = true
 	}
+
 	if a.CacheTimeout == nil {
 		a.CacheTimeout = new(time.Duration)
 		*a.CacheTimeout = 10 * time.Minute
 	}
+
 	if a.CacheSize == nil {
 		a.CacheSize = new(int)
 		*a.CacheSize = 100
 	}
+
 	return nil
 }
 
 func (o *OnlineApiClientCfg) Load() error {
 	o.Credentials = new(ApiCredentialsCfg)
+
 	fcontent, err := os.ReadFile(o.CredentialsFilePath)
 	if err != nil {
-		return fmt.Errorf("failed to read api server credentials configuration file '%s': %w", o.CredentialsFilePath, err)
+		return err
 	}
+
 	err = yaml.UnmarshalStrict(fcontent, o.Credentials)
 	if err != nil {
 		return fmt.Errorf("failed unmarshaling api server credentials configuration file '%s': %w", o.CredentialsFilePath, err)
 	}
-	if o.Credentials.Login == "" || o.Credentials.Password == "" || o.Credentials.URL == "" {
-		log.Warningf("can't load CAPI credentials from '%s' (missing field)", o.CredentialsFilePath)
+
+	switch {
+	case o.Credentials.Login == "":
+		log.Warningf("can't load CAPI credentials from '%s' (missing login field)", o.CredentialsFilePath)
+		o.Credentials = nil
+	case o.Credentials.Password == "":
+		log.Warningf("can't load CAPI credentials from '%s' (missing password field)", o.CredentialsFilePath)
+		o.Credentials = nil
+	case o.Credentials.URL == "":
+		log.Warningf("can't load CAPI credentials from '%s' (missing url field)", o.CredentialsFilePath)
 		o.Credentials = nil
 	}
 
@@ -99,14 +114,17 @@ func (o *OnlineApiClientCfg) Load() error {
 
 func (l *LocalApiClientCfg) Load() error {
 	patcher := yamlpatch.NewPatcher(l.CredentialsFilePath, ".local")
+
 	fcontent, err := patcher.MergedPatchContent()
 	if err != nil {
 		return err
 	}
+
 	err = yaml.UnmarshalStrict(fcontent, &l.Credentials)
 	if err != nil {
 		return fmt.Errorf("failed unmarshaling api client credential configuration file '%s': %w", l.CredentialsFilePath, err)
 	}
+
 	if l.Credentials == nil || l.Credentials.URL == "" {
 		return fmt.Errorf("no credentials or URL found in api client configuration '%s'", l.CredentialsFilePath)
 	}
@@ -137,9 +155,11 @@ func (l *LocalApiClientCfg) Load() error {
 		if err != nil {
 			log.Warningf("Error loading system CA certificates: %s", err)
 		}
+
 		if caCertPool == nil {
 			caCertPool = x509.NewCertPool()
 		}
+
 		caCertPool.AppendCertsFromPEM(caCert)
 		apiclient.CaCertPool = caCertPool
 	}
@@ -160,12 +180,15 @@ func (lapiCfg *LocalApiServerCfg) GetTrustedIPs() ([]net.IPNet, error) {
 	trustedIPs := make([]net.IPNet, 0)
 	for _, ip := range lapiCfg.TrustedIPs {
 		cidr := toValidCIDR(ip)
+
 		_, ipNet, err := net.ParseCIDR(cidr)
 		if err != nil {
 			return nil, err
 		}
+
 		trustedIPs = append(trustedIPs, *ipNet)
 	}
+
 	return trustedIPs, nil
 }
 
@@ -177,6 +200,7 @@ func toValidCIDR(ip string) string {
 	if strings.Contains(ip, ":") {
 		return ip + "/128"
 	}
+
 	return ip + "/32"
 }
 
@@ -327,24 +351,30 @@ func parseCapiWhitelists(fd io.Reader) (*CapiWhitelist, error) {
 		if errors.Is(err, io.EOF) {
 			return nil, fmt.Errorf("empty file")
 		}
+
 		return nil, err
 	}
+
 	ret := &CapiWhitelist{
 		Ips:   make([]net.IP, len(fromCfg.Ips)),
 		Cidrs: make([]*net.IPNet, len(fromCfg.Cidrs)),
 	}
+
 	for idx, v := range fromCfg.Ips {
 		ip := net.ParseIP(v)
 		if ip == nil {
 			return nil, fmt.Errorf("invalid IP address: %s", v)
 		}
+
 		ret.Ips[idx] = ip
 	}
+
 	for idx, v := range fromCfg.Cidrs {
 		_, tnet, err := net.ParseCIDR(v)
 		if err != nil {
 			return nil, err
 		}
+
 		ret.Cidrs[idx] = tnet
 	}
 

+ 2 - 1
pkg/csconfig/api_test.go

@@ -116,7 +116,7 @@ func TestLoadOnlineApiClientCfg(t *testing.T) {
 				CredentialsFilePath: "./testdata/nonexist_online-api-secrets.yaml",
 			},
 			expected:    &ApiCredentialsCfg{},
-			expectedErr: "failed to read api server credentials",
+			expectedErr: "open ./testdata/nonexist_online-api-secrets.yaml: " + cstest.FileNotFoundMessage,
 		},
 	}
 
@@ -254,6 +254,7 @@ func TestLoadAPIServer(t *testing.T) {
 func mustParseCIDRNet(t *testing.T, s string) *net.IPNet {
 	_, ipNet, err := net.ParseCIDR(s)
 	require.NoError(t, err)
+
 	return ipNet
 }
 

+ 28 - 7
test/bats/04_capi.bats

@@ -19,7 +19,35 @@ setup() {
 
 #----------
 
+@test "cscli capi status: fails without credentials" {
+    config_enable_capi
+    ONLINE_API_CREDENTIALS_YAML="$(config_get '.api.server.online_client.credentials_path')"
+    # bogus values, won't be used
+    echo '{"login":"login","password":"password","url":"url"}' > "${ONLINE_API_CREDENTIALS_YAML}"
+
+    config_set "$ONLINE_API_CREDENTIALS_YAML" 'del(.url)'
+    rune -1 cscli capi status
+    assert_stderr --partial "can't load CAPI credentials from '$ONLINE_API_CREDENTIALS_YAML' (missing url field)"
+
+    config_set "$ONLINE_API_CREDENTIALS_YAML" 'del(.password)'
+    rune -1 cscli capi status
+    assert_stderr --partial "can't load CAPI credentials from '$ONLINE_API_CREDENTIALS_YAML' (missing password field)"
+
+    config_set "$ONLINE_API_CREDENTIALS_YAML" 'del(.login)'
+    rune -1 cscli capi status
+    assert_stderr --partial "can't load CAPI credentials from '$ONLINE_API_CREDENTIALS_YAML' (missing login field)"
+
+    rm "${ONLINE_API_CREDENTIALS_YAML}"
+    rune -1 cscli capi status
+    assert_stderr --partial "failed to load Local API: loading online client credentials: open ${ONLINE_API_CREDENTIALS_YAML}: no such file or directory"
+
+    config_set 'del(.api.server.online_client)'
+    rune -1 cscli capi status
+    assert_stderr --partial "no configuration for Central API (CAPI) in '$CONFIG_YAML'"
+}
+
 @test "cscli capi status" {
+    ./instance-data load
     config_enable_capi
     rune -0 cscli capi register --schmilblick githubciXXXXXXXXXXXXXXXXXXXXXXXX
     rune -1 cscli capi status
@@ -60,13 +88,6 @@ setup() {
     assert_stderr --partial "You can successfully interact with Central API (CAPI)"
 }
 
-@test "cscli capi status: fails without credentials" {
-    ONLINE_API_CREDENTIALS_YAML="$(config_get '.api.server.online_client.credentials_path')"
-    rm "${ONLINE_API_CREDENTIALS_YAML}"
-    rune -1 cscli capi status
-    assert_stderr --partial "failed to load Local API: loading online client credentials: failed to read api server credentials configuration file '${ONLINE_API_CREDENTIALS_YAML}': open ${ONLINE_API_CREDENTIALS_YAML}: no such file or directory"
-}
-
 @test "capi register must be run from lapi" {
     config_disable_lapi
     rune -1 cscli capi register --schmilblick githubciXXXXXXXXXXXXXXXXXXXXXXXX