From 3e06b0f5912b3b9d353b4bd83efdf1b94f7c36ba Mon Sep 17 00:00:00 2001 From: Julia Bardi Date: Thu, 7 Dec 2023 16:53:03 +0100 Subject: [PATCH 01/11] invalidate remote api keys on unenroll --- internal/pkg/api/handleAck.go | 10 +++------- internal/pkg/model/ext.go | 26 +++++++++++++++++--------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/internal/pkg/api/handleAck.go b/internal/pkg/api/handleAck.go index 489aefba5..f9752fe8f 100644 --- a/internal/pkg/api/handleAck.go +++ b/internal/pkg/api/handleAck.go @@ -615,14 +615,10 @@ func (ack *AckT) invalidateAPIKeys(ctx context.Context, zlog zerolog.Logger, toR func (ack *AckT) handleUnenroll(ctx context.Context, zlog zerolog.Logger, agent *model.Agent) error { span, ctx := apm.StartSpan(ctx, "ackUnenroll", "process") defer span.End() - apiKeys := agent.APIKeyIDs() - if len(apiKeys) > 0 { - zlog = zlog.With().Strs(LogAPIKeyID, apiKeys).Logger() - if err := ack.bulk.APIKeyInvalidate(ctx, apiKeys...); err != nil { - return fmt.Errorf("handleUnenroll invalidate apikey: %w", err) - } - } + apiKeys := agent.APIKeyIDs() + zlog.Info().Any("fleet.policy.apiKeyIDsToRetire", apiKeys).Msg("handleUnenroll invalidate API keys") + ack.invalidateAPIKeys(ctx, zlog, apiKeys, "") now := time.Now().UTC().Format(time.RFC3339) doc := bulk.UpdateFields{ diff --git a/internal/pkg/model/ext.go b/internal/pkg/model/ext.go index 3c587165c..2608069d5 100644 --- a/internal/pkg/model/ext.go +++ b/internal/pkg/model/ext.go @@ -44,24 +44,32 @@ func (a *Agent) CheckDifferentVersion(ver string) string { // APIKeyIDs returns all the API keys, the valid, in-use as well as the one // marked to be retired. -func (a *Agent) APIKeyIDs() []string { +func (a *Agent) APIKeyIDs() []ToRetireAPIKeyIdsItems { if a == nil { return nil } - keys := make([]string, 0, len(a.Outputs)+1) + keys := make([]ToRetireAPIKeyIdsItems, 0, len(a.Outputs)+1) if a.AccessAPIKeyID != "" { - keys = append(keys, a.AccessAPIKeyID) + keys = append(keys, ToRetireAPIKeyIdsItems{ + ID: a.AccessAPIKeyID, + Output: "", + RetiredAt: "", + }) } - for _, output := range a.Outputs { + for outputName, output := range a.Outputs { if output.APIKeyID != "" { - keys = append(keys, output.APIKeyID) - } - for _, key := range output.ToRetireAPIKeyIds { - if key.ID != "" { - keys = append(keys, key.ID) + name := "" + if outputName != "default" { + name = outputName } + keys = append(keys, ToRetireAPIKeyIdsItems{ + ID: output.APIKeyID, + Output: name, + RetiredAt: "", + }) } + keys = append(keys, output.ToRetireAPIKeyIds...) } return keys From e2679f89dfae4c6334ad7d9faf65926ddb359a04 Mon Sep 17 00:00:00 2001 From: Julia Bardi Date: Mon, 11 Dec 2023 14:39:33 +0100 Subject: [PATCH 02/11] fix test --- internal/pkg/model/ext.go | 6 +++++- internal/pkg/model/ext_test.go | 26 ++++++++++++++++---------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/internal/pkg/model/ext.go b/internal/pkg/model/ext.go index 2608069d5..132cdea13 100644 --- a/internal/pkg/model/ext.go +++ b/internal/pkg/model/ext.go @@ -69,7 +69,11 @@ func (a *Agent) APIKeyIDs() []ToRetireAPIKeyIdsItems { RetiredAt: "", }) } - keys = append(keys, output.ToRetireAPIKeyIds...) + for _, key := range output.ToRetireAPIKeyIds { + if key.ID != "" { + keys = append(keys, key) + } + } } return keys diff --git a/internal/pkg/model/ext_test.go b/internal/pkg/model/ext_test.go index 74a472ae0..9adc14de5 100644 --- a/internal/pkg/model/ext_test.go +++ b/internal/pkg/model/ext_test.go @@ -88,7 +88,7 @@ func TestAgentAPIKeyIDs(t *testing.T) { tcs := []struct { name string agent Agent - want []string + want []ToRetireAPIKeyIdsItems }{ { name: "no API key marked to be retired", @@ -99,7 +99,9 @@ func TestAgentAPIKeyIDs(t *testing.T) { "p2": {APIKeyID: "p2_api_key_id"}, }, }, - want: []string{"access_api_key_id", "p1_api_key_id", "p2_api_key_id"}, + want: []ToRetireAPIKeyIdsItems{{ID: "access_api_key_id", Output: "", RetiredAt: ""}, + {ID: "p1_api_key_id", Output: "p1", RetiredAt: ""}, + {ID: "p2_api_key_id", Output: "p2", RetiredAt: ""}}, }, { name: "with API key marked to be retired", @@ -109,18 +111,22 @@ func TestAgentAPIKeyIDs(t *testing.T) { "p1": { APIKeyID: "p1_api_key_id", ToRetireAPIKeyIds: []ToRetireAPIKeyIdsItems{{ - ID: "p1_to_retire_key", + ID: "p1_to_retire_key", + Output: "remote", }}}, "p2": { APIKeyID: "p2_api_key_id", ToRetireAPIKeyIds: []ToRetireAPIKeyIdsItems{{ - ID: "p2_to_retire_key", + ID: "p2_to_retire_key", + Output: "remote", }}}, }, }, - want: []string{ - "access_api_key_id", "p1_api_key_id", "p2_api_key_id", - "p1_to_retire_key", "p2_to_retire_key"}, + want: []ToRetireAPIKeyIdsItems{{ID: "access_api_key_id", Output: "", RetiredAt: ""}, + {ID: "p1_api_key_id", Output: "p1", RetiredAt: ""}, + {ID: "p2_api_key_id", Output: "p2", RetiredAt: ""}, + {ID: "p1_to_retire_key", Output: "remote", RetiredAt: ""}, + {ID: "p2_to_retire_key", Output: "remote", RetiredAt: ""}}, }, { name: "API key empty", @@ -130,7 +136,7 @@ func TestAgentAPIKeyIDs(t *testing.T) { "p1": {APIKeyID: ""}, }, }, - want: []string{"access_api_key_id"}, + want: []ToRetireAPIKeyIdsItems{{ID: "access_api_key_id", Output: "", RetiredAt: ""}}, }, { name: "retired API key empty", @@ -144,8 +150,8 @@ func TestAgentAPIKeyIDs(t *testing.T) { }}}, }, }, - want: []string{ - "access_api_key_id", "p1_api_key_id"}, + want: []ToRetireAPIKeyIdsItems{{ID: "access_api_key_id", Output: "", RetiredAt: ""}, + {ID: "p1_api_key_id", Output: "p1", RetiredAt: ""}}, }, } From 55803dcc354c917896a2b4b729e19f6a209701a1 Mon Sep 17 00:00:00 2001 From: Julia Bardi Date: Mon, 11 Dec 2023 16:27:58 +0100 Subject: [PATCH 03/11] using latest es snapshot in tests --- dev-tools/integration/.env | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-tools/integration/.env b/dev-tools/integration/.env index 1f4a7a3de..784538099 100644 --- a/dev-tools/integration/.env +++ b/dev-tools/integration/.env @@ -1,4 +1,4 @@ -ELASTICSEARCH_VERSION=8.12.0-9d443b17-SNAPSHOT +ELASTICSEARCH_VERSION=8.13.0-e8db5dbd-SNAPSHOT ELASTICSEARCH_USERNAME=elastic ELASTICSEARCH_PASSWORD=changeme TEST_ELASTICSEARCH_HOSTS=localhost:9200 From 2ec7678d83afb93e8bd76fe85808341123cf7136 Mon Sep 17 00:00:00 2001 From: Julia Bardi Date: Mon, 11 Dec 2023 16:52:52 +0100 Subject: [PATCH 04/11] try with latest 8.12 snapshot --- dev-tools/integration/.env | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-tools/integration/.env b/dev-tools/integration/.env index 784538099..f515930b2 100644 --- a/dev-tools/integration/.env +++ b/dev-tools/integration/.env @@ -1,4 +1,4 @@ -ELASTICSEARCH_VERSION=8.13.0-e8db5dbd-SNAPSHOT +ELASTICSEARCH_VERSION=8.12.0-9bbde39d-SNAPSHOT ELASTICSEARCH_USERNAME=elastic ELASTICSEARCH_PASSWORD=changeme TEST_ELASTICSEARCH_HOSTS=localhost:9200 From 88d1cb8289e172520e9e4c75b34514cfb961f595 Mon Sep 17 00:00:00 2001 From: Julia Bardi Date: Tue, 12 Dec 2023 10:33:12 +0100 Subject: [PATCH 05/11] use new 8.13 snapshot --- dev-tools/integration/.env | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-tools/integration/.env b/dev-tools/integration/.env index f515930b2..9aa2dc3e0 100644 --- a/dev-tools/integration/.env +++ b/dev-tools/integration/.env @@ -1,4 +1,4 @@ -ELASTICSEARCH_VERSION=8.12.0-9bbde39d-SNAPSHOT +ELASTICSEARCH_VERSION=8.13.0-jifrleti-SNAPSHOT ELASTICSEARCH_USERNAME=elastic ELASTICSEARCH_PASSWORD=changeme TEST_ELASTICSEARCH_HOSTS=localhost:9200 From a6efc9b6fdf7a80a88a648b8478d74d516d5d594 Mon Sep 17 00:00:00 2001 From: Julia Bardi Date: Tue, 12 Dec 2023 11:00:52 +0100 Subject: [PATCH 06/11] try without build id --- dev-tools/integration/.env | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-tools/integration/.env b/dev-tools/integration/.env index 9aa2dc3e0..2f164c804 100644 --- a/dev-tools/integration/.env +++ b/dev-tools/integration/.env @@ -1,4 +1,4 @@ -ELASTICSEARCH_VERSION=8.13.0-jifrleti-SNAPSHOT +ELASTICSEARCH_VERSION=8.13.0-SNAPSHOT ELASTICSEARCH_USERNAME=elastic ELASTICSEARCH_PASSWORD=changeme TEST_ELASTICSEARCH_HOSTS=localhost:9200 From 35f35392dff7b7119c63d526e0dbbef19f99c48c Mon Sep 17 00:00:00 2001 From: Julia Bardi Date: Tue, 12 Dec 2023 11:03:55 +0100 Subject: [PATCH 07/11] invalidate remote api key after force unenroll --- internal/pkg/api/auth.go | 2 +- internal/pkg/api/handleAck.go | 90 ++++++++++++++++--------------- internal/pkg/api/handleCheckin.go | 17 ++++++ 3 files changed, 65 insertions(+), 44 deletions(-) diff --git a/internal/pkg/api/auth.go b/internal/pkg/api/auth.go index cc869207f..308bac264 100644 --- a/internal/pkg/api/auth.go +++ b/internal/pkg/api/auth.go @@ -170,7 +170,7 @@ func authAgent(r *http.Request, id *string, bulker bulk.Bulk, c cache.Cache) (*m // Update the cache to mark the api key id associated with this agent as not enabled c.SetAPIKey(*key, false) - return nil, ErrAgentInactive + return agent, ErrAgentInactive } return agent, nil diff --git a/internal/pkg/api/handleAck.go b/internal/pkg/api/handleAck.go index f9752fe8f..a63b1a81b 100644 --- a/internal/pkg/api/handleAck.go +++ b/internal/pkg/api/handleAck.go @@ -567,49 +567,7 @@ func cleanRoles(roles json.RawMessage) (json.RawMessage, int, error) { } func (ack *AckT) invalidateAPIKeys(ctx context.Context, zlog zerolog.Logger, toRetireAPIKeyIDs []model.ToRetireAPIKeyIdsItems, skip string) { - ids := make([]string, 0, len(toRetireAPIKeyIDs)) - remoteIds := make(map[string][]string) - for _, k := range toRetireAPIKeyIDs { - if k.ID == skip || k.ID == "" { - continue - } - if k.Output != "" { - if remoteIds[k.Output] == nil { - remoteIds[k.Output] = make([]string, 0) - } - remoteIds[k.Output] = append(remoteIds[k.Output], k.ID) - } else { - ids = append(ids, k.ID) - } - } - if len(ids) > 0 { - zlog.Info().Strs("fleet.policy.apiKeyIDsToRetire", ids).Msg("Invalidate old API keys") - if err := ack.bulk.APIKeyInvalidate(ctx, ids...); err != nil { - zlog.Info().Err(err).Strs("ids", ids).Msg("Failed to invalidate API keys") - } - } - // using remote es bulker to invalidate api key - for outputName, outputIds := range remoteIds { - outputBulk := ack.bulk.GetBulker(outputName) - - if outputBulk == nil { - // read output config from .fleet-policies, not filtering by policy id as agent could be reassigned - policy, err := dl.QueryOutputFromPolicy(ctx, ack.bulk, outputName) - if err != nil || policy == nil { - zlog.Warn().Str("outputName", outputName).Any("ids", outputIds).Msg("Output policy not found, API keys will be orphaned") - } else { - outputBulk, _, err = ack.bulk.CreateAndGetBulker(ctx, zlog, outputName, policy.Data.Outputs) - if err != nil { - zlog.Warn().Str("outputName", outputName).Any("ids", outputIds).Msg("Failed to recreate output bulker, API keys will be orphaned") - } - } - } - if outputBulk != nil { - if err := outputBulk.APIKeyInvalidate(ctx, outputIds...); err != nil { - zlog.Info().Err(err).Strs("ids", outputIds).Str("outputName", outputName).Msg("Failed to invalidate API keys") - } - } - } + invalidateAPIKeys(ctx, zlog, ack.bulk, toRetireAPIKeyIDs, skip) } func (ack *AckT) handleUnenroll(ctx context.Context, zlog zerolog.Logger, agent *model.Agent) error { @@ -736,3 +694,49 @@ func makeUpdatePolicyBody(policyID string, newRev, coordIdx int64) []byte { return buf.Bytes() } + +func invalidateAPIKeys(ctx context.Context, zlog zerolog.Logger, bulk bulk.Bulk, toRetireAPIKeyIDs []model.ToRetireAPIKeyIdsItems, skip string) { + ids := make([]string, 0, len(toRetireAPIKeyIDs)) + remoteIds := make(map[string][]string) + for _, k := range toRetireAPIKeyIDs { + if k.ID == skip || k.ID == "" { + continue + } + if k.Output != "" { + if remoteIds[k.Output] == nil { + remoteIds[k.Output] = make([]string, 0) + } + remoteIds[k.Output] = append(remoteIds[k.Output], k.ID) + } else { + ids = append(ids, k.ID) + } + } + if len(ids) > 0 { + zlog.Info().Strs("fleet.policy.apiKeyIDsToRetire", ids).Msg("Invalidate old API keys") + if err := bulk.APIKeyInvalidate(ctx, ids...); err != nil { + zlog.Info().Err(err).Strs("ids", ids).Msg("Failed to invalidate API keys") + } + } + // using remote es bulker to invalidate api key + for outputName, outputIds := range remoteIds { + outputBulk := bulk.GetBulker(outputName) + + if outputBulk == nil { + // read output config from .fleet-policies, not filtering by policy id as agent could be reassigned + policy, err := dl.QueryOutputFromPolicy(ctx, bulk, outputName) + if err != nil || policy == nil { + zlog.Warn().Str("outputName", outputName).Any("ids", outputIds).Msg("Output policy not found, API keys will be orphaned") + } else { + outputBulk, _, err = bulk.CreateAndGetBulker(ctx, zlog, outputName, policy.Data.Outputs) + if err != nil { + zlog.Warn().Str("outputName", outputName).Any("ids", outputIds).Msg("Failed to recreate output bulker, API keys will be orphaned") + } + } + } + if outputBulk != nil { + if err := outputBulk.APIKeyInvalidate(ctx, outputIds...); err != nil { + zlog.Info().Err(err).Strs("ids", outputIds).Str("outputName", outputName).Msg("Failed to invalidate API keys") + } + } + } +} diff --git a/internal/pkg/api/handleCheckin.go b/internal/pkg/api/handleCheckin.go index 8512a637c..e1dc3e416 100644 --- a/internal/pkg/api/handleCheckin.go +++ b/internal/pkg/api/handleCheckin.go @@ -119,6 +119,11 @@ func (ct *CheckinT) handleCheckin(zlog zerolog.Logger, w http.ResponseWriter, r agent, err := authAgent(r, &id, ct.bulker, ct.cache) if err != nil { + // invalidate remote API keys of force unenrolled agents + if err == ErrAgentInactive && agent != nil { + ctx := zlog.WithContext(r.Context()) + invalidateAPIKeysOfInactiveAgent(ctx, zlog, ct.bulker, agent) + } return err } @@ -136,6 +141,18 @@ func (ct *CheckinT) handleCheckin(zlog zerolog.Logger, w http.ResponseWriter, r return ct.ProcessRequest(zlog, w, r, start, agent, newVer) } +func invalidateAPIKeysOfInactiveAgent(ctx context.Context, zlog zerolog.Logger, bulker bulk.Bulk, agent *model.Agent) { + remoteAPIKeys := make([]model.ToRetireAPIKeyIdsItems, 0) + apiKeys := agent.APIKeyIDs() + for _, key := range apiKeys { + if key.Output != "" { + remoteAPIKeys = append(remoteAPIKeys, key) + } + } + zlog.Info().Any("fleet.policy.apiKeyIDsToRetire", remoteAPIKeys).Msg("handleCheckin invalidate remote API keys") + invalidateAPIKeys(ctx, zlog, bulker, remoteAPIKeys, "") +} + // validatedCheckin is a struct to wrap all the things that validateRequest returns. type validatedCheckin struct { req *CheckinRequest From b183e55f89595ee2f143a19f823dabc3b6993ab4 Mon Sep 17 00:00:00 2001 From: Julia Bardi Date: Tue, 12 Dec 2023 11:15:17 +0100 Subject: [PATCH 08/11] fix lint --- internal/pkg/api/handleCheckin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/api/handleCheckin.go b/internal/pkg/api/handleCheckin.go index e1dc3e416..651f88581 100644 --- a/internal/pkg/api/handleCheckin.go +++ b/internal/pkg/api/handleCheckin.go @@ -120,7 +120,7 @@ func (ct *CheckinT) handleCheckin(zlog zerolog.Logger, w http.ResponseWriter, r agent, err := authAgent(r, &id, ct.bulker, ct.cache) if err != nil { // invalidate remote API keys of force unenrolled agents - if err == ErrAgentInactive && agent != nil { + if errors.Is(err, ErrAgentInactive) && agent != nil { ctx := zlog.WithContext(r.Context()) invalidateAPIKeysOfInactiveAgent(ctx, zlog, ct.bulker, agent) } From 853745b9d94ac8801fd1fc7682bee0d07a32deec Mon Sep 17 00:00:00 2001 From: Julia Bardi Date: Tue, 12 Dec 2023 11:54:50 +0100 Subject: [PATCH 09/11] added integration test to force unenroll --- .../remote_es_output_integration_test.go | 134 +++++++++++++++++- 1 file changed, 132 insertions(+), 2 deletions(-) diff --git a/internal/pkg/server/remote_es_output_integration_test.go b/internal/pkg/server/remote_es_output_integration_test.go index 9b78eb293..8820cfc66 100644 --- a/internal/pkg/server/remote_es_output_integration_test.go +++ b/internal/pkg/server/remote_es_output_integration_test.go @@ -18,6 +18,7 @@ import ( "time" "github.com/elastic/fleet-server/v7/internal/pkg/apikey" + "github.com/elastic/fleet-server/v7/internal/pkg/bulk" "github.com/elastic/fleet-server/v7/internal/pkg/dl" "github.com/elastic/fleet-server/v7/internal/pkg/model" "github.com/gofrs/uuid" @@ -25,7 +26,7 @@ import ( "github.com/stretchr/testify/require" ) -func Checkin(t *testing.T, ctx context.Context, srv *tserver, agentID, key string, shouldHaveRemoveES bool) (string, string) { +func Checkin(t *testing.T, ctx context.Context, srv *tserver, agentID, key string, shouldHaveRemoteES bool) (string, string) { cli := cleanhttp.DefaultClient() var obj map[string]interface{} @@ -68,7 +69,7 @@ func Checkin(t *testing.T, ctx context.Context, srv *tserver, agentID, key strin outputs, ok := policy["outputs"].(map[string]interface{}) require.True(t, ok, "expected outputs to be map") var remoteAPIKey string - if shouldHaveRemoveES { + if shouldHaveRemoteES { remoteES, ok := outputs["remoteES"].(map[string]interface{}) require.True(t, ok, "expected remoteES to be map") oType, ok := remoteES["type"].(string) @@ -276,3 +277,132 @@ func verifyRemoteAPIKey(t *testing.T, ctx context.Context, remoteESHost, apiKeyI require.Contains(t, string(respString), fmt.Sprintf("\"invalidated\":%t", invalidated)) } + +func Test_Agent_Remote_ES_Output_ForceUnenroll(t *testing.T) { + enrollBody := `{ + "type": "PERMANENT", + "shared_id": "", + "enrollment_id": "", + "metadata": { + "user_provided": {}, + "local": {}, + "tags": [] + } + }` + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Start test server + srv, err := startTestServer(t, ctx, policyData) + require.NoError(t, err) + + t.Log("Create policy with remote ES output") + + var policyRemoteID = uuid.Must(uuid.NewV4()).String() + remoteESHost := "localhost:9201" + var policyDataRemoteES = model.PolicyData{ + Outputs: map[string]map[string]interface{}{ + "default": { + "type": "elasticsearch", + }, + "remoteES": { + "type": "remote_elasticsearch", + "hosts": []string{remoteESHost}, + "service_token": os.Getenv("REMOTE_ELASTICSEARCH_SERVICE_TOKEN"), + }, + }, + OutputPermissions: json.RawMessage(`{"default": {}, "remoteES": {}}`), + Inputs: []map[string]interface{}{}, + Agent: json.RawMessage(`{"monitoring": {"use_output":"remoteES"}}`), + } + + _, err = dl.CreatePolicy(ctx, srv.bulker, model.Policy{ + PolicyID: policyRemoteID, + RevisionIdx: 1, + DefaultFleetServer: false, + Data: &policyDataRemoteES, + }) + if err != nil { + t.Fatal(err) + } + + t.Log("Create API key and enrollment key for new policy") + + newKey, err := apikey.Create(ctx, srv.bulker.Client(), "default", "", "true", []byte(`{ + "fleet-apikey-enroll": { + "cluster": [], + "index": [], + "applications": [{ + "application": "fleet", + "privileges": ["no-privileges"], + "resources": ["*"] + }] + } + }`), map[string]interface{}{ + "managed_by": "fleet", + "managed": true, + "type": "enroll", + "policy_id": policyRemoteID, + }) + if err != nil { + t.Fatal(err) + } + + _, err = dl.CreateEnrollmentAPIKey(ctx, srv.bulker, model.EnrollmentAPIKey{ + Name: "RemoteES", + APIKey: newKey.Key, + APIKeyID: newKey.ID, + PolicyID: policyRemoteID, + Active: true, + }) + if err != nil { + t.Fatal(err) + } + + t.Log("Enroll agent") + srvCopy := srv + srvCopy.enrollKey = newKey.Token() + agentID, key := EnrollAgent(enrollBody, t, ctx, srvCopy) + + // cleanup + defer func() { + err = srv.bulker.Delete(ctx, dl.FleetAgents, agentID) + if err != nil { + t.Log("could not clean up agent") + } + }() + + remoteAPIKey, actionID := Checkin(t, ctx, srvCopy, agentID, key, true) + apiKeyID := strings.Split(remoteAPIKey, ":")[0] + + verifyRemoteAPIKey(t, ctx, remoteESHost, apiKeyID, false) + + Ack(t, ctx, srvCopy, actionID, agentID, key) + + t.Log("Force Unenroll agent - set inactive") + + doc := bulk.UpdateFields{ + "active": false, + } + body, err := doc.Marshal() + require.NoError(t, err) + err = srv.bulker.Update(ctx, dl.FleetAgents, agentID, body, bulk.WithRefresh(), bulk.WithRetryOnConflict(3)) + require.NoError(t, err) + + t.Log("Checkin so that invalidate logic runs") + + cli := cleanhttp.DefaultClient() + + t.Logf("Fake a checkin for agent %s", agentID) + req, err := http.NewRequestWithContext(ctx, "POST", srv.baseURL()+"/api/fleet/agents/"+agentID+"/checkin", strings.NewReader(checkinBody)) + require.NoError(t, err) + req.Header.Set("Authorization", "ApiKey "+key) + req.Header.Set("User-Agent", "elastic agent "+serverVersion) + req.Header.Set("Content-Type", "application/json") + _, err = cli.Do(req) + require.NoError(t, err) + + t.Log("Verify that remote API key is invalidated") + verifyRemoteAPIKey(t, ctx, remoteESHost, apiKeyID, true) + +} From 26dd6187da16843ad087703da7994db43d8779ea Mon Sep 17 00:00:00 2001 From: Julia Bardi Date: Tue, 12 Dec 2023 13:11:22 +0100 Subject: [PATCH 10/11] added integration test on unenroll --- .../remote_es_output_integration_test.go | 141 +++++++++++++++++- 1 file changed, 136 insertions(+), 5 deletions(-) diff --git a/internal/pkg/server/remote_es_output_integration_test.go b/internal/pkg/server/remote_es_output_integration_test.go index 8820cfc66..12d611eb0 100644 --- a/internal/pkg/server/remote_es_output_integration_test.go +++ b/internal/pkg/server/remote_es_output_integration_test.go @@ -26,7 +26,7 @@ import ( "github.com/stretchr/testify/require" ) -func Checkin(t *testing.T, ctx context.Context, srv *tserver, agentID, key string, shouldHaveRemoteES bool) (string, string) { +func Checkin(t *testing.T, ctx context.Context, srv *tserver, agentID, key string, shouldHaveRemoteES bool, actionType string) (string, string) { cli := cleanhttp.DefaultClient() var obj map[string]interface{} @@ -60,7 +60,10 @@ func Checkin(t *testing.T, ctx context.Context, srv *tserver, agentID, key strin require.True(t, ok, "expected action id to be string") typeRaw := action["type"] - require.Equal(t, "POLICY_CHANGE", typeRaw) + require.Equal(t, actionType, typeRaw) + if actionType != "POLICY_CHANGE" { + return "", actionID + } dataRaw := action["data"] data, ok := dataRaw.(map[string]interface{}) require.True(t, ok, "expected data to be map") @@ -85,6 +88,7 @@ func Checkin(t *testing.T, ctx context.Context, srv *tserver, agentID, key strin defaultAPIKey, ok := defaultOutput["api_key"].(string) require.True(t, ok, "expected defaultAPIKey to be string") require.NotEqual(t, remoteAPIKey, defaultAPIKey, "expected remote api key to be different than default") + return remoteAPIKey, actionID } @@ -215,7 +219,7 @@ func Test_Agent_Remote_ES_Output(t *testing.T) { } }() - remoteAPIKey, actionID := Checkin(t, ctx, srvCopy, agentID, key, true) + remoteAPIKey, actionID := Checkin(t, ctx, srvCopy, agentID, key, true, "POLICY_CHANGE") apiKeyID := strings.Split(remoteAPIKey, ":")[0] verifyRemoteAPIKey(t, ctx, remoteESHost, apiKeyID, false) @@ -245,7 +249,7 @@ func Test_Agent_Remote_ES_Output(t *testing.T) { } t.Log("Checkin so that agent gets new policy revision") - _, actionID = Checkin(t, ctx, srvCopy, agentID, key, false) + _, actionID = Checkin(t, ctx, srvCopy, agentID, key, false, "POLICY_CHANGE") t.Log("Ack so that fleet triggers remote api key invalidate") Ack(t, ctx, srvCopy, actionID, agentID, key) @@ -372,7 +376,7 @@ func Test_Agent_Remote_ES_Output_ForceUnenroll(t *testing.T) { } }() - remoteAPIKey, actionID := Checkin(t, ctx, srvCopy, agentID, key, true) + remoteAPIKey, actionID := Checkin(t, ctx, srvCopy, agentID, key, true, "POLICY_CHANGE") apiKeyID := strings.Split(remoteAPIKey, ":")[0] verifyRemoteAPIKey(t, ctx, remoteESHost, apiKeyID, false) @@ -406,3 +410,130 @@ func Test_Agent_Remote_ES_Output_ForceUnenroll(t *testing.T) { verifyRemoteAPIKey(t, ctx, remoteESHost, apiKeyID, true) } + +func Test_Agent_Remote_ES_Output_Unenroll(t *testing.T) { + enrollBody := `{ + "type": "PERMANENT", + "shared_id": "", + "enrollment_id": "", + "metadata": { + "user_provided": {}, + "local": {}, + "tags": [] + } + }` + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Start test server + srv, err := startTestServer(t, ctx, policyData) + require.NoError(t, err) + + t.Log("Create policy with remote ES output") + + var policyRemoteID = uuid.Must(uuid.NewV4()).String() + remoteESHost := "localhost:9201" + var policyDataRemoteES = model.PolicyData{ + Outputs: map[string]map[string]interface{}{ + "default": { + "type": "elasticsearch", + }, + "remoteES": { + "type": "remote_elasticsearch", + "hosts": []string{remoteESHost}, + "service_token": os.Getenv("REMOTE_ELASTICSEARCH_SERVICE_TOKEN"), + }, + }, + OutputPermissions: json.RawMessage(`{"default": {}, "remoteES": {}}`), + Inputs: []map[string]interface{}{}, + Agent: json.RawMessage(`{"monitoring": {"use_output":"remoteES"}}`), + } + + _, err = dl.CreatePolicy(ctx, srv.bulker, model.Policy{ + PolicyID: policyRemoteID, + RevisionIdx: 1, + DefaultFleetServer: false, + Data: &policyDataRemoteES, + }) + if err != nil { + t.Fatal(err) + } + + t.Log("Create API key and enrollment key for new policy") + + newKey, err := apikey.Create(ctx, srv.bulker.Client(), "default", "", "true", []byte(`{ + "fleet-apikey-enroll": { + "cluster": [], + "index": [], + "applications": [{ + "application": "fleet", + "privileges": ["no-privileges"], + "resources": ["*"] + }] + } + }`), map[string]interface{}{ + "managed_by": "fleet", + "managed": true, + "type": "enroll", + "policy_id": policyRemoteID, + }) + if err != nil { + t.Fatal(err) + } + + _, err = dl.CreateEnrollmentAPIKey(ctx, srv.bulker, model.EnrollmentAPIKey{ + Name: "RemoteES", + APIKey: newKey.Key, + APIKeyID: newKey.ID, + PolicyID: policyRemoteID, + Active: true, + }) + if err != nil { + t.Fatal(err) + } + + t.Log("Enroll agent") + srvCopy := srv + srvCopy.enrollKey = newKey.Token() + agentID, key := EnrollAgent(enrollBody, t, ctx, srvCopy) + + // cleanup + defer func() { + err = srv.bulker.Delete(ctx, dl.FleetAgents, agentID) + if err != nil { + t.Log("could not clean up agent") + } + }() + + remoteAPIKey, actionID := Checkin(t, ctx, srvCopy, agentID, key, true, "POLICY_CHANGE") + apiKeyID := strings.Split(remoteAPIKey, ":")[0] + + verifyRemoteAPIKey(t, ctx, remoteESHost, apiKeyID, false) + + Ack(t, ctx, srvCopy, actionID, agentID, key) + + t.Log("Unenroll agent") + + doc := fmt.Sprintf(`{ + "action_id": "unenroll_action1", + "agents": ["%s"], + "@timestamp": "2023-12-11T13:00:00.000Z", + "expiration": "2099-01-10T13:14:36.565Z", + "type": "UNENROLL" + }`, agentID) + client := srv.bulker.Client() + res, err := client.Index(".fleet-actions", strings.NewReader(doc)) + require.NoError(t, err) + require.Equal(t, 201, res.StatusCode) + + t.Log("Checkin so that agent gets unenroll action") + _, actionID = Checkin(t, ctx, srvCopy, agentID, key, false, "UNENROLL") + t.Log(actionID) + + t.Log("Ack so that fleet triggers remote api key invalidate") + Ack(t, ctx, srvCopy, actionID, agentID, key) + + t.Log("Verify that remote API key is invalidated") + verifyRemoteAPIKey(t, ctx, remoteESHost, apiKeyID, true) + +} From 3585b69a15e1fa544b390121df2e1cb04cdffd87 Mon Sep 17 00:00:00 2001 From: Julia Bardi Date: Tue, 12 Dec 2023 13:23:14 +0100 Subject: [PATCH 11/11] fix lint --- .../remote_es_output_integration_test.go | 54 +++++-------------- 1 file changed, 13 insertions(+), 41 deletions(-) diff --git a/internal/pkg/server/remote_es_output_integration_test.go b/internal/pkg/server/remote_es_output_integration_test.go index 12d611eb0..c4478f052 100644 --- a/internal/pkg/server/remote_es_output_integration_test.go +++ b/internal/pkg/server/remote_es_output_integration_test.go @@ -26,6 +26,10 @@ import ( "github.com/stretchr/testify/require" ) +const ( + remoteESHost = "localhost:9201" +) + func Checkin(t *testing.T, ctx context.Context, srv *tserver, agentID, key string, shouldHaveRemoteES bool, actionType string) (string, string) { cli := cleanhttp.DefaultClient() var obj map[string]interface{} @@ -126,16 +130,6 @@ func Ack(t *testing.T, ctx context.Context, srv *tserver, actionID, agentID, key } func Test_Agent_Remote_ES_Output(t *testing.T) { - enrollBody := `{ - "type": "PERMANENT", - "shared_id": "", - "enrollment_id": "", - "metadata": { - "user_provided": {}, - "local": {}, - "tags": [] - } - }` ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -146,7 +140,6 @@ func Test_Agent_Remote_ES_Output(t *testing.T) { t.Log("Create policy with remote ES output") var policyRemoteID = uuid.Must(uuid.NewV4()).String() - remoteESHost := "localhost:9201" var policyDataRemoteES = model.PolicyData{ Outputs: map[string]map[string]interface{}{ "default": { @@ -222,7 +215,7 @@ func Test_Agent_Remote_ES_Output(t *testing.T) { remoteAPIKey, actionID := Checkin(t, ctx, srvCopy, agentID, key, true, "POLICY_CHANGE") apiKeyID := strings.Split(remoteAPIKey, ":")[0] - verifyRemoteAPIKey(t, ctx, remoteESHost, apiKeyID, false) + verifyRemoteAPIKey(t, ctx, apiKeyID, false) Ack(t, ctx, srvCopy, actionID, agentID, key) @@ -254,11 +247,11 @@ func Test_Agent_Remote_ES_Output(t *testing.T) { t.Log("Ack so that fleet triggers remote api key invalidate") Ack(t, ctx, srvCopy, actionID, agentID, key) - verifyRemoteAPIKey(t, ctx, remoteESHost, apiKeyID, true) + verifyRemoteAPIKey(t, ctx, apiKeyID, true) } -func verifyRemoteAPIKey(t *testing.T, ctx context.Context, remoteESHost, apiKeyID string, invalidated bool) { +func verifyRemoteAPIKey(t *testing.T, ctx context.Context, apiKeyID string, invalidated bool) { // need to wait a bit before querying the api key time.Sleep(time.Second) @@ -283,16 +276,6 @@ func verifyRemoteAPIKey(t *testing.T, ctx context.Context, remoteESHost, apiKeyI } func Test_Agent_Remote_ES_Output_ForceUnenroll(t *testing.T) { - enrollBody := `{ - "type": "PERMANENT", - "shared_id": "", - "enrollment_id": "", - "metadata": { - "user_provided": {}, - "local": {}, - "tags": [] - } - }` ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -303,7 +286,6 @@ func Test_Agent_Remote_ES_Output_ForceUnenroll(t *testing.T) { t.Log("Create policy with remote ES output") var policyRemoteID = uuid.Must(uuid.NewV4()).String() - remoteESHost := "localhost:9201" var policyDataRemoteES = model.PolicyData{ Outputs: map[string]map[string]interface{}{ "default": { @@ -379,7 +361,7 @@ func Test_Agent_Remote_ES_Output_ForceUnenroll(t *testing.T) { remoteAPIKey, actionID := Checkin(t, ctx, srvCopy, agentID, key, true, "POLICY_CHANGE") apiKeyID := strings.Split(remoteAPIKey, ":")[0] - verifyRemoteAPIKey(t, ctx, remoteESHost, apiKeyID, false) + verifyRemoteAPIKey(t, ctx, apiKeyID, false) Ack(t, ctx, srvCopy, actionID, agentID, key) @@ -403,25 +385,16 @@ func Test_Agent_Remote_ES_Output_ForceUnenroll(t *testing.T) { req.Header.Set("Authorization", "ApiKey "+key) req.Header.Set("User-Agent", "elastic agent "+serverVersion) req.Header.Set("Content-Type", "application/json") - _, err = cli.Do(req) + res, err := cli.Do(req) require.NoError(t, err) + defer res.Body.Close() t.Log("Verify that remote API key is invalidated") - verifyRemoteAPIKey(t, ctx, remoteESHost, apiKeyID, true) + verifyRemoteAPIKey(t, ctx, apiKeyID, true) } func Test_Agent_Remote_ES_Output_Unenroll(t *testing.T) { - enrollBody := `{ - "type": "PERMANENT", - "shared_id": "", - "enrollment_id": "", - "metadata": { - "user_provided": {}, - "local": {}, - "tags": [] - } - }` ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -432,7 +405,6 @@ func Test_Agent_Remote_ES_Output_Unenroll(t *testing.T) { t.Log("Create policy with remote ES output") var policyRemoteID = uuid.Must(uuid.NewV4()).String() - remoteESHost := "localhost:9201" var policyDataRemoteES = model.PolicyData{ Outputs: map[string]map[string]interface{}{ "default": { @@ -508,7 +480,7 @@ func Test_Agent_Remote_ES_Output_Unenroll(t *testing.T) { remoteAPIKey, actionID := Checkin(t, ctx, srvCopy, agentID, key, true, "POLICY_CHANGE") apiKeyID := strings.Split(remoteAPIKey, ":")[0] - verifyRemoteAPIKey(t, ctx, remoteESHost, apiKeyID, false) + verifyRemoteAPIKey(t, ctx, apiKeyID, false) Ack(t, ctx, srvCopy, actionID, agentID, key) @@ -534,6 +506,6 @@ func Test_Agent_Remote_ES_Output_Unenroll(t *testing.T) { Ack(t, ctx, srvCopy, actionID, agentID, key) t.Log("Verify that remote API key is invalidated") - verifyRemoteAPIKey(t, ctx, remoteESHost, apiKeyID, true) + verifyRemoteAPIKey(t, ctx, apiKeyID, true) }