diff --git a/ee/server/service/errors.go b/ee/server/service/errors.go index 433854e9a9..7885f6df8e 100644 --- a/ee/server/service/errors.go +++ b/ee/server/service/errors.go @@ -10,13 +10,13 @@ type notFoundError struct { fleet.ErrorWithUUID } -func (e notFoundError) Error() string { +func (e *notFoundError) Error() string { return "not found" } // IsNotFound implements the service.IsNotFound interface (from the non-premium // service package) so that the handler returns 404 for this error. -func (e notFoundError) IsNotFound() bool { +func (e *notFoundError) IsNotFound() bool { return true } diff --git a/ee/server/service/mdm.go b/ee/server/service/mdm.go index 37092a9a66..cef52b6296 100644 --- a/ee/server/service/mdm.go +++ b/ee/server/service/mdm.go @@ -56,7 +56,7 @@ func (svc *Service) GetAppleBM(ctx context.Context) (*fleet.AppleBM, error) { } if len(tokens) == 0 { - return nil, notFoundError{} + return nil, ¬FoundError{} } if len(tokens) > 1 { diff --git a/ee/server/service/mdm_external_test.go b/ee/server/service/mdm_external_test.go index 7bbee9f5cb..6d6bc333dd 100644 --- a/ee/server/service/mdm_external_test.go +++ b/ee/server/service/mdm_external_test.go @@ -390,7 +390,7 @@ func TestGetOrCreatePreassignTeam(t *testing.T) { } asst := setupAsstByTeam[tmID] if asst == nil { - return nil, eeservice.NotFoundError{} + return nil, &eeservice.NotFoundError{} } return asst, nil } diff --git a/ee/server/service/software_installers.go b/ee/server/service/software_installers.go index d46efbde57..b130b6effd 100644 --- a/ee/server/service/software_installers.go +++ b/ee/server/service/software_installers.go @@ -1153,7 +1153,7 @@ func (svc *Service) getSoftwareInstallerBinary(ctx context.Context, storageID st return nil, ctxerr.Wrap(ctx, err, "checking if installer exists") } if !exists { - return nil, ctxerr.Wrapf(ctx, notFoundError{}, "%s with filename %s does not exist in software installer store", storageID, + return nil, ctxerr.Wrapf(ctx, ¬FoundError{}, "%s with filename %s does not exist in software installer store", storageID, filename) } @@ -2705,7 +2705,7 @@ func (svc *Service) GetBatchSetSoftwareInstallersResult(ctx context.Context, tmN return "", "", nil, ctxerr.Wrap(ctx, err, "failed to get result") } if result == nil { - return "", "", nil, ctxerr.Wrap(ctx, notFoundError{}, "request_uuid not found") + return "", "", nil, ctxerr.Wrap(ctx, ¬FoundError{}, "request_uuid not found") } switch { diff --git a/ee/server/service/software_installers_test.go b/ee/server/service/software_installers_test.go index 55030ed6a4..263352d305 100644 --- a/ee/server/service/software_installers_test.go +++ b/ee/server/service/software_installers_test.go @@ -418,7 +418,7 @@ func TestGetInHouseAppManifest(t *testing.T) { }, nil } - return nil, notFoundError{} + return nil, ¬FoundError{} } expected := ` diff --git a/server/fleet/errors.go b/server/fleet/errors.go index 26d3669990..b4869a9d16 100644 --- a/server/fleet/errors.go +++ b/server/fleet/errors.go @@ -56,11 +56,11 @@ type ErrWithRetryAfter = platform_http.ErrWithRetryAfter type ErrWithIsClientError = platform_errors.ErrWithIsClientError type invalidArgWithStatusError struct { - InvalidArgumentError + *InvalidArgumentError code int } -func (e invalidArgWithStatusError) Status() int { +func (e *invalidArgWithStatusError) Status() int { if e.code == 0 { // 422 is the default code for invalid args return http.StatusUnprocessableEntity @@ -118,7 +118,7 @@ func (e *InvalidArgumentError) Appendf(name, reasonFmt string, args ...interface // WithStatus returns an error that combines the InvalidArgumentError // with a custom status code. func (e *InvalidArgumentError) WithStatus(code int) error { - return &invalidArgWithStatusError{*e, code} + return &invalidArgWithStatusError{e, code} } func (e *InvalidArgumentError) HasErrors() bool { @@ -178,17 +178,17 @@ func NewPermissionError(message string) *PermissionError { return &PermissionError{message: message} } -func (e PermissionError) Error() string { +func (e *PermissionError) Error() string { return e.message } -func (e PermissionError) PermissionError() []map[string]string { +func (e *PermissionError) PermissionError() []map[string]string { var forbidden []map[string]string return forbidden } // IsClientError implements ErrWithIsClientError. -func (e PermissionError) IsClientError() bool { +func (e *PermissionError) IsClientError() bool { return true } @@ -207,15 +207,15 @@ type OTAForbiddenError struct { InternalErr error } -func (e OTAForbiddenError) Error() string { +func (e *OTAForbiddenError) Error() string { return "Couldn't install the profile. Invalid enroll secret. Please contact your IT admin." } -func (e OTAForbiddenError) StatusCode() int { +func (e *OTAForbiddenError) StatusCode() int { return http.StatusForbidden } -func (e OTAForbiddenError) Internal() string { +func (e *OTAForbiddenError) Internal() string { if e.InternalErr == nil { return "" } @@ -223,7 +223,7 @@ func (e OTAForbiddenError) Internal() string { } // IsClientError implements ErrWithIsClientError. -func (e OTAForbiddenError) IsClientError() bool { +func (e *OTAForbiddenError) IsClientError() bool { return true } @@ -232,16 +232,16 @@ type licenseError struct { ErrorWithUUID } -func (e licenseError) Error() string { +func (e *licenseError) Error() string { return "Requires Fleet Premium license" } -func (e licenseError) StatusCode() int { +func (e *licenseError) StatusCode() int { return http.StatusPaymentRequired } // IsClientError implements ErrWithIsClientError. -func (e licenseError) IsClientError() bool { +func (e *licenseError) IsClientError() bool { return true } diff --git a/server/fleet/users_test.go b/server/fleet/users_test.go index 1c4e45a655..cb981c492e 100644 --- a/server/fleet/users_test.go +++ b/server/fleet/users_test.go @@ -234,7 +234,7 @@ func TestAdminCreateValidate(t *testing.T) { ierr := err.(*InvalidArgumentError) require.Equal(t, len(tc.errContains), len(ierr.Errors)) for _, expected := range tc.errContains { - assertContainsErrorName(t, *ierr, expected) + assertContainsErrorName(t, ierr, expected) } } }) @@ -282,7 +282,7 @@ func TestInviteCreateValidate(t *testing.T) { ierr := err.(*InvalidArgumentError) for _, expected := range tc.errContains { require.Equal(t, len(tc.errContains), len(ierr.Errors)) - assertContainsErrorName(t, *ierr, expected) + assertContainsErrorName(t, ierr, expected) } } }) @@ -314,7 +314,7 @@ func TestValidateEmail(t *testing.T) { } } -func assertContainsErrorName(t *testing.T, invalid InvalidArgumentError, name string) { +func assertContainsErrorName(t *testing.T, invalid *InvalidArgumentError, name string) { for _, argErr := range invalid.Errors { if argErr.name == name { return diff --git a/server/platform/http/errors.go b/server/platform/http/errors.go index e3be307c0d..5957f74204 100644 --- a/server/platform/http/errors.go +++ b/server/platform/http/errors.go @@ -9,6 +9,7 @@ import ( "regexp" "strconv" "strings" + "sync" "github.com/docker/go-units" platform_errors "github.com/fleetdm/fleet/v4/server/platform/errors" @@ -39,21 +40,23 @@ type ErrorUUIDer interface { } // ErrorWithUUID can be embedded in error types to implement ErrorUUIDer. +// The UUID is lazily generated on first access and is safe for concurrent use. type ErrorWithUUID struct { - uuid string + uuidOnce sync.Once + uuid string } var _ ErrorUUIDer = (*ErrorWithUUID)(nil) // UUID implements the ErrorUUIDer interface. func (e *ErrorWithUUID) UUID() string { - if e.uuid == "" { + e.uuidOnce.Do(func() { u, err := uuid.NewRandom() if err != nil { panic(err) } e.uuid = u.String() - } + }) return e.uuid } @@ -77,7 +80,7 @@ func (e *BadRequestError) BadRequestError() []map[string]string { } // Internal implements the ErrWithInternal interface. -func (e BadRequestError) Internal() string { +func (e *BadRequestError) Internal() string { if e.InternalErr != nil { return e.InternalErr.Error() } @@ -86,12 +89,12 @@ func (e BadRequestError) Internal() string { // We implement the second type of Unwrap that returns an error array, which still works for errors.Is/As, but is not supported in errors.Unwrap // This allows us to check the error chain, but not log the most inner error in the HTTP response. -func (e BadRequestError) Unwrap() []error { +func (e *BadRequestError) Unwrap() []error { return []error{e.InternalErr} } // IsClientError implements ErrWithIsClientError. -func (e BadRequestError) IsClientError() bool { +func (e *BadRequestError) IsClientError() bool { return true } @@ -150,7 +153,7 @@ func NewUserMessageError(err error, statusCode int) *UserMessageError { } // StatusCode returns the HTTP status code for this error. -func (e UserMessageError) StatusCode() int { +func (e *UserMessageError) StatusCode() int { if e.statusCode > 0 { return e.statusCode } @@ -159,7 +162,7 @@ func (e UserMessageError) StatusCode() int { // IsClientError implements ErrWithIsClientError. // Returns true for 4xx status codes, false for 5xx. -func (e UserMessageError) IsClientError() bool { +func (e *UserMessageError) IsClientError() bool { code := e.StatusCode() return code >= 400 && code < 500 } @@ -186,7 +189,7 @@ func GetJSONUnknownField(err error) *string { // UserMessage implements the user-friendly translation of the error if its // root cause is one of the supported types, otherwise it returns the error // message. -func (e UserMessageError) UserMessage() string { +func (e *UserMessageError) UserMessage() string { cause := platform_errors.Cause(e.error) switch cause := cause.(type) { case *json.UnmarshalTypeError: @@ -271,22 +274,22 @@ func NewAuthFailedError(internal string) *AuthFailedError { } // Error implements the error interface. -func (e AuthFailedError) Error() string { +func (e *AuthFailedError) Error() string { return "Authentication failed" } // Internal implements ErrWithInternal. -func (e AuthFailedError) Internal() string { +func (e *AuthFailedError) Internal() string { return e.internal } // StatusCode implements kithttp.StatusCoder. -func (e AuthFailedError) StatusCode() int { +func (e *AuthFailedError) StatusCode() int { return http.StatusUnauthorized } // IsClientError implements ErrWithIsClientError. -func (e AuthFailedError) IsClientError() bool { +func (e *AuthFailedError) IsClientError() bool { return true } @@ -304,22 +307,22 @@ func NewAuthRequiredError(internal string) *AuthRequiredError { } // Error implements the error interface. -func (e AuthRequiredError) Error() string { +func (e *AuthRequiredError) Error() string { return "Authentication required" } // Internal implements ErrWithInternal. -func (e AuthRequiredError) Internal() string { +func (e *AuthRequiredError) Internal() string { return e.internal } // StatusCode implements kithttp.StatusCoder. -func (e AuthRequiredError) StatusCode() int { +func (e *AuthRequiredError) StatusCode() int { return http.StatusUnauthorized } // IsClientError implements ErrWithIsClientError. -func (e AuthRequiredError) IsClientError() bool { +func (e *AuthRequiredError) IsClientError() bool { return true } @@ -339,22 +342,22 @@ func NewAuthHeaderRequiredError(internal string) *AuthHeaderRequiredError { } // Error implements the error interface. -func (e AuthHeaderRequiredError) Error() string { +func (e *AuthHeaderRequiredError) Error() string { return "Authorization header required" } // Internal implements ErrWithInternal. -func (e AuthHeaderRequiredError) Internal() string { +func (e *AuthHeaderRequiredError) Internal() string { return e.internal } // StatusCode implements kithttp.StatusCoder. -func (e AuthHeaderRequiredError) StatusCode() int { +func (e *AuthHeaderRequiredError) StatusCode() int { return http.StatusUnauthorized } // IsClientError implements ErrWithIsClientError. -func (e AuthHeaderRequiredError) IsClientError() bool { +func (e *AuthHeaderRequiredError) IsClientError() bool { return true } @@ -366,17 +369,17 @@ type passwordResetRequiredError struct { } // Error implements the error interface. -func (e passwordResetRequiredError) Error() string { +func (e *passwordResetRequiredError) Error() string { return "password reset required" } // StatusCode implements kithttp.StatusCoder. -func (e passwordResetRequiredError) StatusCode() int { +func (e *passwordResetRequiredError) StatusCode() int { return http.StatusUnauthorized } // IsClientError implements ErrWithIsClientError. -func (e passwordResetRequiredError) IsClientError() bool { +func (e *passwordResetRequiredError) IsClientError() bool { return true } diff --git a/server/service/base_client.go b/server/service/base_client.go index 2663c1ee08..29e2bb05af 100644 --- a/server/service/base_client.go +++ b/server/service/base_client.go @@ -45,7 +45,7 @@ type baseClient struct { func (bc *baseClient) parseResponse(verb, path string, response *http.Response, responseDest interface{}) error { switch response.StatusCode { case http.StatusNotFound: - return notFoundErr{ + return ¬FoundErr{ msg: extractServerErrorText(response.Body), } case http.StatusUnauthorized: diff --git a/server/service/base_client_errors.go b/server/service/base_client_errors.go index 41cc38271b..052452266d 100644 --- a/server/service/base_client_errors.go +++ b/server/service/base_client_errors.go @@ -66,22 +66,34 @@ type notFoundErr struct { fleet.ErrorWithUUID } -func (e notFoundErr) Error() string { +func (e *notFoundErr) Error() string { if e.msg != "" { return e.msg } return "The resource was not found" } -func (e notFoundErr) NotFound() bool { +func (e *notFoundErr) NotFound() bool { return true } // Implement Is so that errors.Is(err, sql.ErrNoRows) returns true for an // error of type *notFoundError, without having to wrap sql.ErrNoRows -// explicitly. -func (e notFoundErr) Is(other error) bool { - return other == sql.ErrNoRows +// explicitly. It also matches other *notFoundErr targets so that pointer-based +// comparison works (pointers to distinct structs are never == even if their +// contents are identical). +func (e *notFoundErr) Is(other error) bool { + if other == sql.ErrNoRows { + return true + } + _, ok := other.(*notFoundErr) + return ok +} + +// isNotFoundErr reports whether err's chain contains a *notFoundErr. +func isNotFoundErr(err error) bool { + var nfe *notFoundErr + return errors.As(err, &nfe) } type ConflictErr interface { diff --git a/server/service/base_client_test.go b/server/service/base_client_test.go index 0d426ff155..1dd73b870f 100644 --- a/server/service/base_client_test.go +++ b/server/service/base_client_test.go @@ -37,7 +37,7 @@ func TestParseResponseKnownErrors(t *testing.T) { code int out error }{ - {"not found errors", http.StatusNotFound, notFoundErr{}}, + {"not found errors", http.StatusNotFound, ¬FoundErr{}}, {"unauthenticated errors", http.StatusUnauthorized, ErrUnauthenticated}, {"license errors", http.StatusPaymentRequired, ErrMissingLicense}, } diff --git a/server/service/client_mdm.go b/server/service/client_mdm.go index 3429d93c1b..a722325f06 100644 --- a/server/service/client_mdm.go +++ b/server/service/client_mdm.go @@ -80,7 +80,7 @@ func (c *Client) GetBootstrapPackageMetadata(teamID uint, forUpdate bool) (*flee func (c *Client) DeleteBootstrapPackageIfNeeded(teamID uint, dryRun bool) error { _, err := c.GetBootstrapPackageMetadata(teamID, true) switch { - case errors.As(err, ¬FoundErr{}): + case isNotFoundErr(err): // not found is OK, it means there is nothing to delete return nil case err != nil: @@ -149,7 +149,7 @@ func (c *Client) UploadBootstrapPackageIfNeeded(bp *fleet.MDMAppleBootstrapPacka oldMeta, err := c.GetBootstrapPackageMetadata(teamID, true) if err != nil { // not found is OK, it means this is our first time uploading a package - if !errors.As(err, ¬FoundErr{}) { + if !isNotFoundErr(err) { return fmt.Errorf("getting bootstrap package metadata: %w", err) } isFirstTime = true @@ -449,7 +449,7 @@ func (c *Client) GetEULAMetadata() (*fleet.MDMEULA, error) { func (c *Client) DeleteEULAIfNeeded(dryRun bool) error { eula, err := c.GetEULAMetadata() switch { - case errors.As(err, ¬FoundErr{}): + case isNotFoundErr(err): // not found is OK, it means there is nothing to delete return nil case err != nil: @@ -475,7 +475,7 @@ func (c *Client) UploadEULAIfNeeded(eulaPath string, dryRun bool) error { oldMeta, err := c.GetEULAMetadata() if err != nil { // not found is OK, it means this is our first time uploading a eula - if !errors.As(err, ¬FoundErr{}) { + if !isNotFoundErr(err) { return fmt.Errorf("getting eula metadata: %w", err) } isFirstTime = true diff --git a/server/service/client_profiles.go b/server/service/client_profiles.go index 8739b8fc57..ca80df870d 100644 --- a/server/service/client_profiles.go +++ b/server/service/client_profiles.go @@ -156,8 +156,7 @@ func (c *Client) GetAppleMDMEnrollmentProfile(teamID uint) (*fleet.MDMAppleSetup } var responseBody createMDMAppleSetupAssistantResponse if err := c.authenticatedRequestWithQuery(nil, verb, path, &responseBody, query); err != nil { - var notFoundErr notFoundErr - if errors.As(err, ¬FoundErr) { + if isNotFoundErr(err) { // If the profile is not found, return nil instead of an error. return nil, nil } diff --git a/server/service/client_scripts.go b/server/service/client_scripts.go index 0bde1ac468..cafd07c68a 100644 --- a/server/service/client_scripts.go +++ b/server/service/client_scripts.go @@ -253,8 +253,7 @@ func (c *Client) GetSetupExperienceScript(teamID uint) (*fleet.Script, error) { var responseBody getSetupExperienceScriptResponse err := c.authenticatedRequestWithQuery(nil, verb, path, &responseBody, query) if err != nil { - var notFoundErr notFoundErr - if errors.As(err, ¬FoundErr) { + if isNotFoundErr(err) { // If the script is not found, we return nil instead of an error. return nil, nil } diff --git a/server/service/client_trigger.go b/server/service/client_trigger.go index 486df18a34..66aae829d2 100644 --- a/server/service/client_trigger.go +++ b/server/service/client_trigger.go @@ -32,7 +32,7 @@ func (c *Client) TriggerCronSchedule(name string) error { if err != nil { return err } - return notFoundErr{msg: msg} + return ¬FoundErr{msg: msg} default: return c.parseResponse(verb, path, response, nil) } diff --git a/server/service/device_client.go b/server/service/device_client.go index 386abdcc3d..d780e67d7d 100644 --- a/server/service/device_client.go +++ b/server/service/device_client.go @@ -171,7 +171,7 @@ func (dc *DeviceClient) CheckToken(token string) error { verb, path := "HEAD", "/api/latest/fleet/device/%s/ping" err := dc.request(verb, path, token, "", nil, nil) - if errors.As(err, ¬FoundErr{}) { + if isNotFoundErr(err) { // notFound is ok, it means an old server without the auth ping endpoint, // so we fall back to previously-used endpoint _, err = dc.DesktopSummary(token) @@ -184,7 +184,7 @@ func (dc *DeviceClient) Ping() error { verb, path := "HEAD", "/api/fleet/device/ping" err := dc.request(verb, path, "-", "", nil, nil) - if err == nil || errors.Is(err, notFoundErr{}) { + if err == nil || isNotFoundErr(err) { // notFound is ok, it means an old server without the ping endpoint + // capabilities header return nil @@ -215,7 +215,7 @@ func (dc *DeviceClient) DesktopSummary(token string) (*fleetDesktopResponse, err return &r, nil } - if errors.Is(err, notFoundErr{}) { + if isNotFoundErr(err) { policies, err := dc.getListDevicePolicies(token) if err != nil { return nil, err diff --git a/server/service/labels_test.go b/server/service/labels_test.go index 870f51093f..21746eae42 100644 --- a/server/service/labels_test.go +++ b/server/service/labels_test.go @@ -159,7 +159,7 @@ func TestLabelsAuth(t *testing.T) { case team2LabelID: // team2 label return &fleet.LabelWithTeamName{Label: team2Label}, nil, nil } - return nil, nil, ctxerr.Wrap(ctx, notFoundErr{"label", fleet.ErrorWithUUID{}}) + return nil, nil, ctxerr.Wrap(ctx, ¬FoundErr{msg: "label"}) } ds.LabelByNameFunc = func(ctx context.Context, name string, filter fleet.TeamFilter) (*fleet.Label, error) { diff --git a/server/service/orbit_client.go b/server/service/orbit_client.go index 47191ff924..74708b8e01 100644 --- a/server/service/orbit_client.go +++ b/server/service/orbit_client.go @@ -497,7 +497,7 @@ func (oc *OrbitClient) DownloadAndDiscardSoftwareInstaller(installerID uint) err func (oc *OrbitClient) Ping() error { verb, path := "HEAD", "/api/fleet/orbit/ping" err := oc.request(verb, path, nil, nil) - if err == nil || errors.Is(err, notFoundErr{}) { + if err == nil || isNotFoundErr(err) { // notFound is ok, it means an old server without the capabilities header return nil } @@ -563,7 +563,7 @@ func (oc *OrbitClient) getNodeKeyOrEnroll() (string, error) { retry.WithErrorFilter(func(err error) (errorOutcome retry.ErrorOutcome) { log.Info().Err(err).Msg("orbit enroll attempt failed") switch { - case errors.Is(err, notFoundErr{}): + case isNotFoundErr(err): // Do not retry if the endpoint does not exist. return retry.ErrorOutcomeDoNotRetry case errors.Is(err, ErrEndUserAuthRequired): @@ -595,7 +595,7 @@ func (oc *OrbitClient) getNodeKeyOrEnroll() (string, error) { } }), ); err != nil { - if errors.Is(err, notFoundErr{}) { + if isNotFoundErr(err) { return "", errors.New("enroll endpoint does not exist") } return "", fmt.Errorf("orbit node key enroll failed, attempts=%d", constant.OrbitEnrollMaxRetries) diff --git a/server/service/sessions_test.go b/server/service/sessions_test.go index 8f6c2fd460..901b96e97b 100644 --- a/server/service/sessions_test.go +++ b/server/service/sessions_test.go @@ -194,7 +194,7 @@ func TestMFA(t *testing.T) { if token == mfaToken { return session, mfaUser, nil } - return nil, nil, notFoundErr{} + return nil, nil, ¬FoundErr{} } resp, err := sessionCreateEndpoint(ctx, &sessionCreateRequest{Token: "foo"}, svc) require.NoError(t, err) diff --git a/server/service/users_test.go b/server/service/users_test.go index a7da5cf77b..fb46a6a207 100644 --- a/server/service/users_test.go +++ b/server/service/users_test.go @@ -457,10 +457,10 @@ func TestModifyUserEmail(t *testing.T) { return user, nil } ms.UserByEmailFunc = func(ctx context.Context, email string) (*fleet.User, error) { - return nil, notFoundErr{} + return nil, ¬FoundErr{} } ms.InviteByEmailFunc = func(ctx context.Context, email string) (*fleet.Invite, error) { - return nil, notFoundErr{} + return nil, ¬FoundErr{} } ms.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) { config := &fleet.AppConfig{ @@ -574,7 +574,7 @@ func TestMFAHandling(t *testing.T) { payload.SSOEnabled = nil ms.InviteByEmailFunc = func(ctx context.Context, email string) (*fleet.Invite, error) { - return nil, notFoundErr{} + return nil, ¬FoundErr{} } _, _, err = svc.CreateUser(ctx, payload) require.ErrorContains(t, err, "mail") @@ -690,10 +690,10 @@ func TestModifyAdminUserEmailPassword(t *testing.T) { return nil } ms.UserByEmailFunc = func(ctx context.Context, email string) (*fleet.User, error) { - return nil, notFoundErr{} + return nil, ¬FoundErr{} } ms.InviteByEmailFunc = func(ctx context.Context, email string) (*fleet.Invite, error) { - return nil, notFoundErr{} + return nil, ¬FoundErr{} } ms.UserByIDFunc = func(ctx context.Context, id uint) (*fleet.User, error) { return user, nil