Be more strict when handling the access token (elastic#9624)

Add validations for the Kibana response with the access token,
and enforce that the access token is not empty when unpacking the
configuration. Since the access token in the keystore could be edited.

Fixes: elastic#9621

CHANGELOG.asciidoc

@@ -56,6 +56,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha1...master[Check the HEAD d
 - When collecting swap metrics for beats telemetry or system metricbeat module handle cases of free swap being bigger than total swap by assuming no swap is being used. {issue}6271[6271] {pull}9383[9383]
 - Adding logging traces at debug level when the pipeline client receives the following events: onFilteredOut, onDroppedOnPublish. {pull}9016[9016]
 - Ignore non index fields in default_field for Elasticsearch. {pull}9549[9549]
+- Enforce validation for the Central Management access token. {issue}9621[9621]
 
 *Auditbeat*
 

x-pack/libbeat/management/api/enroll.go

@@ -8,10 +8,22 @@ import (
 	"net/http"
 
 	"github.com/gofrs/uuid"
+	"github.com/pkg/errors"
 
 	"github.com/elastic/beats/libbeat/common"
 )
 
+type enrollResponse struct {
+	AccessToken string `json:"access_token"`
+}
+
+func (e *enrollResponse) Validate() error {
+	if len(e.AccessToken) == 0 {
+		return errors.New("empty access_token")
+	}
+	return nil
+}
+
 // Enroll a beat in central management, this call returns a valid access token to retrieve configurations
 func (c *Client) Enroll(beatType, beatName, beatVersion, hostname string, beatUUID uuid.UUID, enrollmentToken string) (string, error) {
 	params := common.MapStr{
@@ -21,9 +33,7 @@ func (c *Client) Enroll(beatType, beatName, beatVersion, hostname string, beatUU
 		"host_name": hostname,
 	}
 
-	resp := struct {
-		AccessToken string `json:"access_token"`
-	}{}
+	resp := enrollResponse{}
 
 	headers := http.Header{}
 	headers.Set("kbn-beats-enrollment-token", enrollmentToken)
@@ -33,5 +43,9 @@ func (c *Client) Enroll(beatType, beatName, beatVersion, hostname string, beatUU
 		return "", err
 	}
 
+	if err := resp.Validate(); err != nil {
+		return "", err
+	}
+
 	return resp.AccessToken, err
 }

x-pack/libbeat/management/api/enroll_test.go

@@ -62,18 +62,46 @@ func TestEnrollValid(t *testing.T) {
 }
 
 func TestEnrollError(t *testing.T) {
-	beatUUID, err := uuid.NewV4()
-	if err != nil {
-		t.Fatal(err)
+	tests := map[string]struct {
+		body         string
+		responseCode int
+	}{
+		"invalid enrollment token": {
+			body:         `{"message": "Invalid enrollment token"}`,
+			responseCode: 400,
+		},
+		"invalid token response": {
+			body:         `{"access_token": ""}`,
+			responseCode: 200,
+		},
 	}
 
-	server, client := newServerClientPair(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		http.Error(w, `{"message": "Invalid enrollment token"}`, 400)
-	}))
-	defer server.Close()
-
-	accessToken, err := client.Enroll("metricbeat", "beatname", "6.3.0", "myhostname.lan", beatUUID, "thisismyenrollmenttoken")
-
-	assert.NotNil(t, err)
-	assert.Equal(t, "", accessToken)
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			beatUUID, err := uuid.NewV4()
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			server, client := newServerClientPair(t, http.HandlerFunc(func(
+				w http.ResponseWriter,
+				r *http.Request,
+			) {
+				http.Error(w, test.body, test.responseCode)
+			}))
+			defer server.Close()
+
+			accessToken, err := client.Enroll(
+				"metricbeat",
+				"beatname",
+				"6.3.0",
+				"myhostname.lan",
+				beatUUID,
+				"thisismyenrollmenttoken",
+			)
+
+			assert.NotNil(t, err)
+			assert.Equal(t, "", accessToken)
+		})
+	}
 }

x-pack/libbeat/management/config.go

@@ -5,6 +5,7 @@
 package management
 
 import (
+	"errors"
 	"io"
 	"text/template"
 	"time"
@@ -66,6 +67,8 @@ const ManagedConfigTemplate = `
 #xpack.monitoring.elasticsearch:
 `
 
+var errEmptyAccessToken = errors.New("access_token is empty, you must reenroll your Beat")
+
 // Config for central management
 type Config struct {
 	// true when enrolled
@@ -81,6 +84,14 @@ type Config struct {
 	Blacklist ConfigBlacklistSettings `config:"blacklist" yaml:"blacklist"`
 }
 
+// Validate validates the fields in the config.
+func (c *Config) Validate() error {
+	if len(c.AccessToken) == 0 {
+		return errEmptyAccessToken
+	}
+	return nil
+}
+
 func defaultConfig() *Config {
 	return &Config{
 		Period: 60 * time.Second,

x-pack/libbeat/management/config_test.go

@@ -0,0 +1,41 @@
+// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+// or more contributor license agreements. Licensed under the Elastic License;
+// you may not use this file except in compliance with the Elastic License.
+
+package management
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/elastic/beats/libbeat/common"
+)
+
+func TestConfigValidate(t *testing.T) {
+	tests := map[string]struct {
+		config *common.Config
+		err    bool
+	}{
+		"missing access_token": {
+			config: common.MustNewConfigFrom(map[string]interface{}{}),
+			err:    true,
+		},
+		"access_token is present": {
+			config: common.MustNewConfigFrom(map[string]interface{}{"access_token": "abc1234"}),
+			err:    false,
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			c := defaultConfig()
+			err := test.config.Unpack(c)
+			if test.err {
+				assert.Error(t, err)
+				return
+			}
+			assert.NoError(t, err)
+		})
+	}
+}