device_token endpoint improvements (#15849)

Fixed badly formatted error messages 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

#15832 

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

<!-- Note that API documentation changes are now addressed by the
product design team. -->

- [x] Changes file added for user-visible changes in `changes/` or
`orbit/changes/`.
See [Changes
files](https://fleetdm.com/docs/contributing/committing-changes#changes-files)
for more information.
- [x] Added/updated tests
- [x] Manual QA

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

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

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

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

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

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
+}

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 }

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{})
+}

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