diff --git a/changes/15832-device_token-improvements b/changes/15832-device_token-improvements new file mode 100644 index 0000000000..a3cbf6a10a --- /dev/null +++ b/changes/15832-device_token-improvements @@ -0,0 +1,4 @@ +Fixed badly formatted error message in /api/fleet/orbit/device_token endpoint and others. +In /api/fleet/orbit/device_token: +- Added token validation -- empty token not allowed +- Replaced 500 error with 409 when token conflicts with another host diff --git a/orbit/pkg/packaging/macos_rcodesign.go b/orbit/pkg/packaging/macos_rcodesign.go index b65f2a9396..50bc6cab3a 100644 --- a/orbit/pkg/packaging/macos_rcodesign.go +++ b/orbit/pkg/packaging/macos_rcodesign.go @@ -16,7 +16,7 @@ func rSign(pkgPath, cert string) error { defer os.Remove(pemPath) err := os.WriteFile(pemPath, []byte(cert), 0o600) if err != nil { - return fmt.Errorf("writing cert data: %e", err) + return fmt.Errorf("writing cert data: %s", err) } return retry.Do(func() error { @@ -41,7 +41,7 @@ func rNotarizeStaple(pkg, apiKeyID, apiKeyIssuer, apiKeyContent string) error { path, err := writeAPIKeys(apiKeyIssuer, apiKeyID, apiKeyContent) defer os.Remove(path) if err != nil { - return fmt.Errorf("writing API keys: %e", err) + return fmt.Errorf("writing API keys: %s", err) } return retry.Do(func() error { @@ -66,19 +66,19 @@ func rNotarizeStaple(pkg, apiKeyID, apiKeyIssuer, apiKeyContent string) error { func writeAPIKeys(issuer, id, content string) (string, error) { homedir, err := os.UserHomeDir() if err != nil { - return "", fmt.Errorf("finding home dir: %e", err) + return "", fmt.Errorf("finding home dir: %s", err) } // The underliying tools (rcodesign and Transporter) expect to find a // certificate key in this path. path := filepath.Join(homedir, ".appstoreconnect", "private_keys") if err = secure.MkdirAll(path, 0o600); err != nil { - return "", fmt.Errorf("finding home dir: %e", err) + return "", fmt.Errorf("finding home dir: %s", err) } keyPath := filepath.Join(path, fmt.Sprintf("AuthKey_%s.p8", id)) if err = os.WriteFile(keyPath, []byte(content), 0o600); err != nil { - return "", fmt.Errorf("writing api key contents: %e", err) + return "", fmt.Errorf("writing api key contents: %s", err) } return keyPath, nil diff --git a/orbit/pkg/update/nudge.go b/orbit/pkg/update/nudge.go index aa5327a225..a6c168d410 100644 --- a/orbit/pkg/update/nudge.go +++ b/orbit/pkg/update/nudge.go @@ -121,7 +121,7 @@ func (n *NudgeConfigFetcher) setTargetsAndHashes() error { // we don't want to keep nudge as a target if we failed to update the // cached hashes in the runner. if err := n.opt.UpdateRunner.StoreLocalHash("nudge"); err != nil { - log.Debug().Msgf("removing nudge from target options, error updating local hashes: %e", err) + log.Debug().Msgf("removing nudge from target options, error updating local hashes: %s", err) n.opt.UpdateRunner.RemoveRunnerOptTarget("nudge") n.opt.UpdateRunner.updater.RemoveTargetInfo("nudge") return err diff --git a/orbit/pkg/update/swift_dialog.go b/orbit/pkg/update/swift_dialog.go index ebd0543752..f5df2660e1 100644 --- a/orbit/pkg/update/swift_dialog.go +++ b/orbit/pkg/update/swift_dialog.go @@ -54,7 +54,7 @@ func (s *SwiftDialogDownloader) GetConfig() (*fleet.OrbitConfig, error) { // we don't want to keep swiftDialog as a target if we failed to update the // cached hashes in the runner. if err := s.UpdateRunner.StoreLocalHash("swiftDialog"); err != nil { - log.Debug().Msgf("removing swiftDialog from target options, error updating local hashes: %e", err) + log.Debug().Msgf("removing swiftDialog from target options, error updating local hashes: %s", err) s.UpdateRunner.RemoveRunnerOptTarget("swiftDialog") s.UpdateRunner.updater.RemoveTargetInfo("swiftDialog") return cfg, err diff --git a/server/datastore/mysql/hosts.go b/server/datastore/mysql/hosts.go index 31f8f787bc..54419a2548 100644 --- a/server/datastore/mysql/hosts.go +++ b/server/datastore/mysql/hosts.go @@ -2266,6 +2266,9 @@ func (ds *Datastore) SetOrUpdateDeviceAuthToken(ctx context.Context, hostID uint ` _, err := ds.writer(ctx).ExecContext(ctx, stmt, hostID, authToken) if err != nil { + if isDuplicate(err) { + return fleet.ConflictError{Message: "auth token conflicts with another host"} + } return ctxerr.Wrap(ctx, err, "upsert host's device auth token") } return nil diff --git a/server/fleet/errors.go b/server/fleet/errors.go index 4047492b37..a8a1610a94 100644 --- a/server/fleet/errors.go +++ b/server/fleet/errors.go @@ -543,3 +543,18 @@ const ( RunScriptAlreadyRunningErrMsg = "A script is already running on this host. Please wait about 1 minute to let it finish." RunScriptHostTimeoutErrMsg = "Fleet hasn’t heard from the host in over 1 minute. Fleet doesn’t know if the script ran because the host went offline." ) + +// ConflictError is used to indicate a conflict, such as a UUID conflict in the DB. +type ConflictError struct { + Message string +} + +// Error implements the error interface for the ConflictError. +func (e ConflictError) Error() string { + return e.Message +} + +// StatusCode implements the kithttp.StatusCoder interface. +func (e ConflictError) StatusCode() int { + return http.StatusConflict +} diff --git a/server/mdm/apple/commander.go b/server/mdm/apple/commander.go index b5a4730eda..a7c4f9a4ac 100644 --- a/server/mdm/apple/commander.go +++ b/server/mdm/apple/commander.go @@ -242,7 +242,7 @@ type APNSDeliveryError struct { } func (e *APNSDeliveryError) Error() string { - return fmt.Sprintf("APNS delivery failed with: %e, for UUIDs: %v", e.Err, e.FailedUUIDs) + return fmt.Sprintf("APNS delivery failed with: %s, for UUIDs: %v", e.Err, e.FailedUUIDs) } func (e *APNSDeliveryError) Unwrap() error { return e.Err } diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index 02c7fe44df..458cabb520 100644 --- a/server/service/integration_core_test.go +++ b/server/service/integration_core_test.go @@ -9335,3 +9335,49 @@ func (s *integrationTestSuite) TestHostHealth() { assert.Empty(t, resp.HostHealth.FailingPolicies) assert.Nil(t, resp.HostHealth.TeamID) } + +func (s *integrationTestSuite) TestHostDeviceToken() { + t := s.T() + type response struct { + Err string `json:"error"` + } + + orbitHost := createOrbitEnrolledHost(t, "windows", "device_token", s.ds) + + // Write empty token + body := setOrUpdateDeviceTokenRequest{ + OrbitNodeKey: *orbitHost.OrbitNodeKey, + DeviceAuthToken: "", + } + s.DoJSON("POST", "/api/fleet/orbit/device_token", body, http.StatusBadRequest, &response{}) + + // Write bad node key + body = setOrUpdateDeviceTokenRequest{ + OrbitNodeKey: "", + DeviceAuthToken: "token", + } + s.DoJSON("POST", "/api/fleet/orbit/device_token", body, http.StatusUnauthorized, &response{}) + + // Write a good token. + body = setOrUpdateDeviceTokenRequest{ + OrbitNodeKey: *orbitHost.OrbitNodeKey, + DeviceAuthToken: "token", + } + s.DoJSON("POST", "/api/fleet/orbit/device_token", body, http.StatusOK, &response{}) + + // Try to write the token again for a different host. + // First write a valid token. + orbitHost2 := createOrbitEnrolledHost(t, "darwin", "device_token2", s.ds) + body = setOrUpdateDeviceTokenRequest{ + OrbitNodeKey: *orbitHost2.OrbitNodeKey, + DeviceAuthToken: "token2", + } + s.DoJSON("POST", "/api/fleet/orbit/device_token", body, http.StatusOK, &response{}) + + // Now write a duplicate token, which will result in a conflict with the first host. + body = setOrUpdateDeviceTokenRequest{ + OrbitNodeKey: *orbitHost2.OrbitNodeKey, + DeviceAuthToken: "token", + } + s.DoJSON("POST", "/api/fleet/orbit/device_token", body, http.StatusConflict, &response{}) +} diff --git a/server/service/orbit.go b/server/service/orbit.go index 37431c7e91..19e57b9e0a 100644 --- a/server/service/orbit.go +++ b/server/service/orbit.go @@ -3,6 +3,7 @@ package service import ( "context" "encoding/json" + "errors" "fmt" "net/http" "time" @@ -423,13 +424,20 @@ func (svc *Service) SetOrUpdateDeviceAuthToken(ctx context.Context, deviceAuthTo // this is not a user-authenticated endpoint svc.authz.SkipAuthorization(ctx) + if len(deviceAuthToken) == 0 { + return badRequest("device auth token cannot be empty") + } + host, ok := hostctx.FromContext(ctx) if !ok { return newOsqueryError("internal error: missing host from request context") } if err := svc.ds.SetOrUpdateDeviceAuthToken(ctx, host.ID, deviceAuthToken); err != nil { - return newOsqueryError(fmt.Sprintf("internal error: failed to set or update device auth token: %e", err)) + if errors.As(err, &fleet.ConflictError{}) { + return err + } + return newOsqueryError(fmt.Sprintf("internal error: failed to set or update device auth token: %s", err)) } return nil