From aac478001b0d025d3bdaebb57dafe88572a3c132 Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky <2685025+getvictor@users.noreply.github.com> Date: Thu, 7 Aug 2025 17:05:15 +0200 Subject: [PATCH] Added additional logging information for Windows MDM discovery endpoint. (#31691) Fixes #31690 No functional changes: extra logging and refactoring # Checklist for submitter - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. ## Summary by CodeRabbit * **New Features** * Enhanced error messages for Windows MDM discovery, providing more detailed information about unsupported request versions. * **Bug Fixes** * Improved logging for errors encountered during the Windows MDM discovery process, aiding in issue diagnosis. * **Refactor** * Streamlined the Windows MDM discovery endpoint to centralize validation and response logic for better maintainability. --- changes/31690-windows-discovery-errors | 1 + server/fleet/microsoft_mdm.go | 4 +-- server/fleet/service.go | 3 ++ server/service/microsoft_mdm.go | 47 +++++++++++++++++--------- 4 files changed, 37 insertions(+), 18 deletions(-) create mode 100644 changes/31690-windows-discovery-errors diff --git a/changes/31690-windows-discovery-errors b/changes/31690-windows-discovery-errors new file mode 100644 index 0000000000..35dc199261 --- /dev/null +++ b/changes/31690-windows-discovery-errors @@ -0,0 +1 @@ +Added additional logging information for Windows MDM discovery endpoint when errors occur. diff --git a/server/fleet/microsoft_mdm.go b/server/fleet/microsoft_mdm.go index b35118a17d..01f3912bd3 100644 --- a/server/fleet/microsoft_mdm.go +++ b/server/fleet/microsoft_mdm.go @@ -155,9 +155,9 @@ func (req *SoapRequest) IsValidDiscoveryMsg() error { break } } - if !versionFound { - return errors.New("invalid discover message: Request.RequestVersion") + return fmt.Errorf("invalid discover message: Request.RequestVersion=%q not in supported versions %v", + req.Body.Discover.Request.RequestVersion, syncml.SupportedEnrollmentVersions) } // Traverse the AuthPolicies slice and check for valid values diff --git a/server/fleet/service.go b/server/fleet/service.go index 4131377d25..4bf41efbe4 100644 --- a/server/fleet/service.go +++ b/server/fleet/service.go @@ -1039,6 +1039,9 @@ type Service interface { /////////////////////////////////////////////////////////////////////////////// // Windows MDM + // ProcessMDMMicrosoftDiscovery handles the Discovery message validation and response + ProcessMDMMicrosoftDiscovery(ctx context.Context, req *SoapRequest) (*SoapResponse, error) + // GetMDMMicrosoftDiscoveryResponse returns a valid DiscoveryResponse message GetMDMMicrosoftDiscoveryResponse(ctx context.Context, upnEmail string) (*DiscoverResponse, error) diff --git a/server/service/microsoft_mdm.go b/server/service/microsoft_mdm.go index e1351f107c..ff83b8329e 100644 --- a/server/service/microsoft_mdm.go +++ b/server/service/microsoft_mdm.go @@ -762,28 +762,15 @@ func NewProvisioningDoc(certStoreData mdm_types.Characteristic, applicationData func mdmMicrosoftDiscoveryEndpoint(ctx context.Context, request interface{}, svc fleet.Service) (mdm_types.Errorer, error) { req := request.(*SoapRequestContainer).Data - // Checking first if Discovery message is valid and returning error if this is not the case - if err := req.IsValidDiscoveryMsg(); err != nil { - soapFault := svc.GetAuthorizedSoapFault(ctx, syncml.SoapErrorMessageFormat, mdm_types.MDEDiscovery, err) - return getSoapResponseFault(req.GetMessageID(), soapFault), nil - } - - // Getting the DiscoveryResponse message - discoveryResponseMsg, err := svc.GetMDMMicrosoftDiscoveryResponse(ctx, req.Body.Discover.Request.EmailAddress) - if err != nil { - soapFault := svc.GetAuthorizedSoapFault(ctx, syncml.SoapErrorMessageFormat, mdm_types.MDEDiscovery, err) - return getSoapResponseFault(req.GetMessageID(), soapFault), nil - } - - // Embedding the DiscoveryResponse message inside of a SoapResponse - response, err := NewSoapResponse(discoveryResponseMsg, req.GetMessageID()) + // Process the discovery request using the Service method which handles validation, logging, and response generation + response, err := svc.ProcessMDMMicrosoftDiscovery(ctx, req) if err != nil { soapFault := svc.GetAuthorizedSoapFault(ctx, syncml.SoapErrorMessageFormat, mdm_types.MDEDiscovery, err) return getSoapResponseFault(req.GetMessageID(), soapFault), nil } return SoapResponseContainer{ - Data: &response, + Data: response, Err: nil, }, nil } @@ -1027,6 +1014,34 @@ func (svc *Service) authBinarySecurityToken(ctx context.Context, authToken *flee return "", "", errors.New("token is not authorized") } +// ProcessMDMMicrosoftDiscovery handles the Discovery message validation and response +func (svc *Service) ProcessMDMMicrosoftDiscovery(ctx context.Context, req *fleet.SoapRequest) (*fleet.SoapResponse, error) { + // Checking first if Discovery message is valid and returning error if this is not the case + if err := req.IsValidDiscoveryMsg(); err != nil { + // Log the raw XML request for debugging invalid messages + level.Debug(svc.logger).Log( + "msg", "invalid discover message", + "err", err.Error(), + "request_xml", string(req.Raw), + ) + return nil, err + } + + // Getting the DiscoveryResponse message + discoveryResponseMsg, err := svc.GetMDMMicrosoftDiscoveryResponse(ctx, req.Body.Discover.Request.EmailAddress) + if err != nil { + return nil, err + } + + // Embedding the DiscoveryResponse message inside of a SoapResponse + response, err := NewSoapResponse(discoveryResponseMsg, req.GetMessageID()) + if err != nil { + return nil, err + } + + return &response, nil +} + // GetMDMMicrosoftDiscoveryResponse returns a valid DiscoveryResponse message func (svc *Service) GetMDMMicrosoftDiscoveryResponse(ctx context.Context, upnEmail string) (*fleet.DiscoverResponse, error) { // skipauth: This endpoint does not use authentication