diff --git a/server/mdm/android/service/pubsub.go b/server/mdm/android/service/pubsub.go index 65b26f3532..1943f07aa6 100644 --- a/server/mdm/android/service/pubsub.go +++ b/server/mdm/android/service/pubsub.go @@ -4,6 +4,7 @@ import ( "context" "encoding/base64" "fmt" + "net/http" "strings" "time" @@ -69,7 +70,9 @@ func (svc *Service) ProcessPubSubPush(ctx context.Context, token string, message func (svc *Service) authenticatePubSub(ctx context.Context, token string) error { svc.authz.SkipAuthorization(ctx) - _, err := svc.checkIfAndroidNotConfigured(ctx) + // On a simple not configured error return status OK to avoid PubSub retry looping after + // disabling Android MDM + _, err := svc.checkIfAndroidNotConfigured(ctx, http.StatusOK) if err != nil { return err } @@ -107,12 +110,13 @@ func (svc *Service) getClientAuthenticationSecret(ctx context.Context) (string, } func (svc *Service) handlePubSubStatusReport(ctx context.Context, token string, rawData []byte) error { - // We allow DELETED notification type to be received since user may be in the process of disabling Android MDM. - // Otherwise, we authenticate below in authenticatePubSub - svc.authz.SkipAuthorization(ctx) + err := svc.authenticatePubSub(ctx, token) + if err != nil { + return err + } var device androidmanagement.Device - err := json.Unmarshal(rawData, &device) + err = json.Unmarshal(rawData, &device) if err != nil { return ctxerr.Wrap(ctx, err, "unmarshal Android status report message") } @@ -187,11 +191,6 @@ func (svc *Service) handlePubSubStatusReport(ctx context.Context, token string, return nil } - err = svc.authenticatePubSub(ctx, token) - if err != nil { - return err - } - host, err := svc.getExistingHost(ctx, &device) if err != nil { return ctxerr.Wrap(ctx, err, "getting existing Android host") diff --git a/server/mdm/android/service/pubsub_test.go b/server/mdm/android/service/pubsub_test.go index f0ebbf5708..dd5517092e 100644 --- a/server/mdm/android/service/pubsub_test.go +++ b/server/mdm/android/service/pubsub_test.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "encoding/json" "errors" + "net/http" "os" "strconv" "strings" @@ -84,8 +85,10 @@ func TestPubSubEnrollment(t *testing.T) { EnrollmentTokenData: string(enrollTokenData), }) err = svc.ProcessPubSubPush(context.Background(), "invalid", enrollmentMessage) - require.Error(t, err) require.Equal(t, "validation failed: android Android MDM is NOT configured", err.Error()) + sc, ok := err.(interface{ Status() int }) + require.True(t, ok, "error should implement Status() interface") + require.Equal(t, http.StatusOK, sc.Status()) }) t.Run("if android token is invalid", func(t *testing.T) { diff --git a/server/mdm/android/service/service.go b/server/mdm/android/service/service.go index dbd52e65f6..c625274e9e 100644 --- a/server/mdm/android/service/service.go +++ b/server/mdm/android/service/service.go @@ -532,7 +532,7 @@ func (svc *Service) CreateEnrollmentToken(ctx context.Context, enrollSecret, idp // We call SkipAuthorization here to avoid explicitly calling it when errors occur. svc.authz.SkipAuthorization(ctx) - _, err := svc.checkIfAndroidNotConfigured(ctx) + _, err := svc.checkIfAndroidNotConfigured(ctx, http.StatusConflict) if err != nil { return nil, err } @@ -606,7 +606,7 @@ func (svc *Service) CreateEnrollmentToken(ctx context.Context, enrollSecret, idp }, nil } -func (svc *Service) checkIfAndroidNotConfigured(ctx context.Context) (*fleet.AppConfig, error) { +func (svc *Service) checkIfAndroidNotConfigured(ctx context.Context, statusOfError int) (*fleet.AppConfig, error) { // This call uses cached_mysql implementation, so it's safe to call it multiple times appConfig, err := svc.ds.AppConfig(ctx) if err != nil { @@ -614,7 +614,7 @@ func (svc *Service) checkIfAndroidNotConfigured(ctx context.Context) (*fleet.App } if !appConfig.MDM.AndroidEnabledAndConfigured { return nil, fleet.NewInvalidArgumentError("android", - "Android MDM is NOT configured").WithStatus(http.StatusConflict) + "Android MDM is NOT configured").WithStatus(statusOfError) } return appConfig, nil }