From 4915d7cff93b2b2252ba3bfc7a7885b6a88ccb00 Mon Sep 17 00:00:00 2001 From: Michel Laterman <82832767+michel-laterman@users.noreply.github.com> Date: Tue, 31 Dec 2024 19:36:59 -0300 Subject: [PATCH] Remove PGP endpoint auth requirement (#4256) Remove PGP endpoint auth requirement --- ...uth-requirement-from-PGP-key-endpoint.yaml | 32 +++++++++++++++++++ internal/pkg/api/error.go | 12 ++----- internal/pkg/api/handlePGPRequest.go | 12 +++---- internal/pkg/api/openapi.gen.go | 2 -- model/openapi.yml | 4 --- pkg/api/client.gen.go | 8 ----- testing/e2e/api_version/client_api_current.go | 9 ++---- 7 files changed, 42 insertions(+), 37 deletions(-) create mode 100644 changelog/fragments/1735593163-Remove-auth-requirement-from-PGP-key-endpoint.yaml diff --git a/changelog/fragments/1735593163-Remove-auth-requirement-from-PGP-key-endpoint.yaml b/changelog/fragments/1735593163-Remove-auth-requirement-from-PGP-key-endpoint.yaml new file mode 100644 index 000000000..cc54202e0 --- /dev/null +++ b/changelog/fragments/1735593163-Remove-auth-requirement-from-PGP-key-endpoint.yaml @@ -0,0 +1,32 @@ +# Kind can be one of: +# - breaking-change: a change to previously-documented behavior +# - deprecation: functionality that is being removed in a later release +# - bug-fix: fixes a problem in a previous version +# - enhancement: extends functionality but does not break or fix existing behavior +# - feature: new functionality +# - known-issue: problems that we are aware of in a given version +# - security: impacts on the security of a product or a user’s deployment. +# - upgrade: important information for someone upgrading from a prior version +# - other: does not fit into any of the other categories +kind: bug-fix + +# Change summary; a 80ish characters long description of the change. +summary: Remove auth requirement from PGP key endpoint + +# Long description; in case the summary is not enough to describe the change +# this field accommodate a description without length limits. +# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment. +#description: + +# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc. +component: fleet-server + +# PR URL; optional; the PR number that added the changeset. +# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added. +# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number. +# Please provide it if you are adding a fragment for a different PR. +#pr: https://github.com/owner/repo/1234 + +# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of). +# If not present is automatically filled by the tooling with the issue linked to the PR number. +issue: https://github.com/elastic/fleet-server/issues/4255 diff --git a/internal/pkg/api/error.go b/internal/pkg/api/error.go index 4ac57ec2c..6c60f93ee 100644 --- a/internal/pkg/api/error.go +++ b/internal/pkg/api/error.go @@ -14,6 +14,8 @@ import ( "strings" "time" + "go.elastic.co/apm/v2" + "github.com/elastic/fleet-server/v7/internal/pkg/apikey" "github.com/elastic/fleet-server/v7/internal/pkg/dl" "github.com/elastic/fleet-server/v7/internal/pkg/es" @@ -22,7 +24,6 @@ import ( "github.com/elastic/fleet-server/v7/internal/pkg/file/uploader" "github.com/elastic/fleet-server/v7/internal/pkg/limit" "github.com/elastic/fleet-server/v7/internal/pkg/logger" - "go.elastic.co/apm/v2" "github.com/rs/zerolog" "github.com/rs/zerolog/hlog" @@ -252,15 +253,6 @@ func NewHTTPErrResp(err error) HTTPErrResp { zerolog.InfoLevel, }, }, - { - ErrPGPPermissions, - HTTPErrResp{ - http.StatusInternalServerError, - "ErrPGPPermissions", - "fleet-server PGP key has incorrect permissions", - zerolog.ErrorLevel, - }, - }, // apikey { apikey.ErrNoAuthHeader, diff --git a/internal/pkg/api/handlePGPRequest.go b/internal/pkg/api/handlePGPRequest.go index 266931efa..6454dc253 100644 --- a/internal/pkg/api/handlePGPRequest.go +++ b/internal/pkg/api/handlePGPRequest.go @@ -15,11 +15,12 @@ import ( "os" "path/filepath" + "github.com/rs/zerolog" + "go.elastic.co/apm/v2" + "github.com/elastic/fleet-server/v7/internal/pkg/bulk" "github.com/elastic/fleet-server/v7/internal/pkg/cache" "github.com/elastic/fleet-server/v7/internal/pkg/config" - "github.com/rs/zerolog" - "go.elastic.co/apm/v2" ) const ( @@ -51,11 +52,8 @@ func (pt *PGPRetrieverT) handlePGPKey(zlog zerolog.Logger, w http.ResponseWriter if r.TLS == nil { return ErrTLSRequired } - key, err := authAPIKey(r, pt.bulker, pt.cache) - if err != nil { - return err - } - zlog = zlog.With().Str(LogEnrollAPIKeyID, key.ID).Logger() + // auth is not required for this endpoint. + // we also do not check if an API key is present in order to avoid making a round trip. ctx := zlog.WithContext(r.Context()) p, err := pt.getPGPKey(ctx, zlog) diff --git a/internal/pkg/api/openapi.gen.go b/internal/pkg/api/openapi.gen.go index 349d5b275..0e4bb10d0 100644 --- a/internal/pkg/api/openapi.gen.go +++ b/internal/pkg/api/openapi.gen.go @@ -1790,8 +1790,6 @@ func (siw *ServerInterfaceWrapper) GetPGPKey(w http.ResponseWriter, r *http.Requ return } - ctx = context.WithValue(ctx, ApiKeyScopes, []string{}) - // Parameter object where we will unmarshal all parameters from the context var params GetPGPKeyParams diff --git a/model/openapi.yml b/model/openapi.yml index 317863191..bc936d43e 100644 --- a/model/openapi.yml +++ b/model/openapi.yml @@ -1764,8 +1764,6 @@ paths: operationId: getPGPKey summary: retrieve a PGP key from the fleet-server's local storage. description: "Get a PGP key that can be used to verify agent upgrades. Key is stored on (fleet-server's) disk." - security: - - apiKey: [] parameters: - name: major in: path @@ -1802,8 +1800,6 @@ paths: format: binary "400": $ref: "#/components/responses/badRequest" - "401": - $ref: "#/components/responses/keyNotEnabled" "500": description: The server has an error retrieving or reading the local key. headers: diff --git a/pkg/api/client.gen.go b/pkg/api/client.gen.go index 3740640a6..74e12cd07 100644 --- a/pkg/api/client.gen.go +++ b/pkg/api/client.gen.go @@ -1221,7 +1221,6 @@ type GetPGPKeyResponse struct { Body []byte HTTPResponse *http.Response JSON400 *BadRequest - JSON401 *KeyNotEnabled } // Status returns HTTPResponse.Status @@ -1682,13 +1681,6 @@ func ParseGetPGPKeyResponse(rsp *http.Response) (*GetPGPKeyResponse, error) { } response.JSON400 = &dest - case strings.Contains(rsp.Header.Get("Content-Type"), "json") && rsp.StatusCode == 401: - var dest KeyNotEnabled - if err := json.Unmarshal(bodyBytes, &dest); err != nil { - return nil, err - } - response.JSON401 = &dest - } return response, nil diff --git a/testing/e2e/api_version/client_api_current.go b/testing/e2e/api_version/client_api_current.go index 6009f6c81..643474c79 100644 --- a/testing/e2e/api_version/client_api_current.go +++ b/testing/e2e/api_version/client_api_current.go @@ -269,11 +269,8 @@ func (tester *ClientAPITester) Artifact(ctx context.Context, apiKey, id, sha2, e tester.Require().Equal(encodedSHA, fmt.Sprintf("%x", hash[:])) } -func (tester *ClientAPITester) GetPGPKey(ctx context.Context, apiKey string) []byte { - client, err := api.NewClientWithResponses(tester.endpoint, api.WithHTTPClient(tester.Client), api.WithRequestEditorFn(func(ctx context.Context, req *http.Request) error { - req.Header.Set("Authorization", "ApiKey "+apiKey) - return nil - })) +func (tester *ClientAPITester) GetPGPKey(ctx context.Context) []byte { + client, err := api.NewClientWithResponses(tester.endpoint, api.WithHTTPClient(tester.Client)) tester.Require().NoError(err) resp, err := client.GetPGPKeyWithResponse(ctx, 1, 2, 3, nil) @@ -398,7 +395,7 @@ func (tester *ClientAPITester) TestArtifact() { func (tester *ClientAPITester) TestGetPGPKey() { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - tester.GetPGPKey(ctx, tester.enrollmentKey) + tester.GetPGPKey(ctx) } func (tester *ClientAPITester) TestEnrollAuditUnenroll() {