From ee4fae8d69b8cab029f0d82bd842b9023a3161aa Mon Sep 17 00:00:00 2001 From: Lucas Manuel Rodriguez Date: Thu, 25 Sep 2025 22:52:28 -0300 Subject: [PATCH] Add easy to understand errors when setting up Entra conditional access (#33453) Resolves #32420. Demo of the changes: https://github.com/user-attachments/assets/c5ee28ba-7f67-48bb-aa25-c934a5515de4 - [X] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. ## Testing - [X] QA'd all new/changed functionality manually --- ...20-entra-easy-to-understand-error-messages | 1 + .../ConditionalAccess/ConditionalAccess.tsx | 42 ++++++++++++++++--- .../services/entities/conditional_access.ts | 1 + server/fleet/service.go | 2 +- server/mock/service/service_mock.go | 4 +- .../service/conditional_access_microsoft.go | 33 +++++++++------ .../receive-redirect-from-microsoft.js | 6 ++- 7 files changed, 66 insertions(+), 23 deletions(-) create mode 100644 changes/32420-entra-easy-to-understand-error-messages diff --git a/changes/32420-entra-easy-to-understand-error-messages b/changes/32420-entra-easy-to-understand-error-messages new file mode 100644 index 0000000000..4e9df34760 --- /dev/null +++ b/changes/32420-entra-easy-to-understand-error-messages @@ -0,0 +1 @@ +* Added easy to understand error messages when configuring Entra conditional access in fleet. diff --git a/frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/ConditionalAccess.tsx b/frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/ConditionalAccess.tsx index 7e0cf13ade..99ee47e82a 100644 --- a/frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/ConditionalAccess.tsx +++ b/frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/ConditionalAccess.tsx @@ -174,7 +174,7 @@ const ConditionalAccess = () => { ...DEFAULT_USE_QUERY_OPTIONS, // only make this call at the appropriate UI phase enabled: phase === Phase.ConfirmingConfigured && isPremiumTier, - onSuccess: ({ configuration_completed }) => { + onSuccess: ({ configuration_completed, setup_error }) => { if (configuration_completed) { setPhase(Phase.Configured); renderFlash( @@ -183,10 +183,42 @@ const ConditionalAccess = () => { ); } else { setPhase(Phase.Form); - renderFlash( - "error", - "Could not verify conditional access integration. Please try connecting again." - ); + + if ( + // IT admin did not complete the consent. + !setup_error || + // IT admin clicked "Cancel" in the consent dialog. + setup_error.includes( + "A Microsoft Entra admin did not consent to the permissions requested by the conditional access integration" + ) + ) { + renderFlash( + "error", + "Couldn't update. Fleet didn't get permissions for Entra. Please try again and accept the permissions." + ); + } else if ( + setup_error.includes( + 'No "Fleet conditional access" Entra ID group was found' + ) + ) { + renderFlash( + "error", + `Couldn't connect. The "Fleet conditional access" group doesn't exist in Entra. Please create the group and try again.` + ); + } else { + // For other kind of errors we just show a generic error. + // We won't render the error as is because the error comes from the MS proxy and they may be too big or unformatted + // to display in the banner. + // + // For troubleshooting: + // - The API response contains the setup_error. + // - The Fleet server logs the error. + // - The MS proxy stores the error in its database. + renderFlash( + "error", + "Couldn't connect. Please contact your Fleet administrator." + ); + } } }, onError: () => { diff --git a/frontend/services/entities/conditional_access.ts b/frontend/services/entities/conditional_access.ts index d5ad7c25a0..124380f2a4 100644 --- a/frontend/services/entities/conditional_access.ts +++ b/frontend/services/entities/conditional_access.ts @@ -7,6 +7,7 @@ export type TriggerMSConditionalStatusResponse = { }; export type ConfirmMSConditionalAccessResponse = { configuration_completed: boolean; + setup_error: string; }; const conditionalAccessService = { diff --git a/server/fleet/service.go b/server/fleet/service.go index a44afc99d0..b284c163f5 100644 --- a/server/fleet/service.go +++ b/server/fleet/service.go @@ -1325,7 +1325,7 @@ type Service interface { // ConditionalAccessMicrosoftGet returns the current (currently unique) integration. ConditionalAccessMicrosoftGet(ctx context.Context) (*ConditionalAccessMicrosoftIntegration, error) // ConditionalAccessMicrosoftConfirm finalizes the integration (marks integration as done). - ConditionalAccessMicrosoftConfirm(ctx context.Context) (configurationCompleted bool, err error) + ConditionalAccessMicrosoftConfirm(ctx context.Context) (configurationCompleted bool, setupError string, err error) // ConditionalAccessMicrosoftDelete deletes the integration and deprovisions the tenant on Entra. ConditionalAccessMicrosoftDelete(ctx context.Context) error diff --git a/server/mock/service/service_mock.go b/server/mock/service/service_mock.go index e97d552be4..fd50185d47 100644 --- a/server/mock/service/service_mock.go +++ b/server/mock/service/service_mock.go @@ -811,7 +811,7 @@ type ConditionalAccessMicrosoftCreateIntegrationFunc func(ctx context.Context, t type ConditionalAccessMicrosoftGetFunc func(ctx context.Context) (*fleet.ConditionalAccessMicrosoftIntegration, error) -type ConditionalAccessMicrosoftConfirmFunc func(ctx context.Context) (configurationCompleted bool, err error) +type ConditionalAccessMicrosoftConfirmFunc func(ctx context.Context) (configurationCompleted bool, setupError string, err error) type ConditionalAccessMicrosoftDeleteFunc func(ctx context.Context) error @@ -4835,7 +4835,7 @@ func (s *Service) ConditionalAccessMicrosoftGet(ctx context.Context) (*fleet.Con return s.ConditionalAccessMicrosoftGetFunc(ctx) } -func (s *Service) ConditionalAccessMicrosoftConfirm(ctx context.Context) (configurationCompleted bool, err error) { +func (s *Service) ConditionalAccessMicrosoftConfirm(ctx context.Context) (configurationCompleted bool, setupError string, err error) { s.mu.Lock() s.ConditionalAccessMicrosoftConfirmFuncInvoked = true s.mu.Unlock() diff --git a/server/service/conditional_access_microsoft.go b/server/service/conditional_access_microsoft.go index 554a8748b5..8d33cbd2d7 100644 --- a/server/service/conditional_access_microsoft.go +++ b/server/service/conditional_access_microsoft.go @@ -91,55 +91,62 @@ func (svc *Service) ConditionalAccessMicrosoftCreateIntegration(ctx context.Cont type conditionalAccessMicrosoftConfirmRequest struct{} type conditionalAccessMicrosoftConfirmResponse struct { - ConfigurationCompleted bool `json:"configuration_completed"` - Err error `json:"error,omitempty"` + ConfigurationCompleted bool `json:"configuration_completed"` + SetupError string `json:"setup_error"` + Err error `json:"error,omitempty"` } func (r conditionalAccessMicrosoftConfirmResponse) Error() error { return r.Err } func conditionalAccessMicrosoftConfirmEndpoint(ctx context.Context, request interface{}, svc fleet.Service) (fleet.Errorer, error) { _ = request.(*conditionalAccessMicrosoftConfirmRequest) - configurationCompleted, err := svc.ConditionalAccessMicrosoftConfirm(ctx) + configurationCompleted, setupError, err := svc.ConditionalAccessMicrosoftConfirm(ctx) if err != nil { return conditionalAccessMicrosoftConfirmResponse{Err: err}, nil } return conditionalAccessMicrosoftConfirmResponse{ ConfigurationCompleted: configurationCompleted, + SetupError: setupError, }, nil } -func (svc *Service) ConditionalAccessMicrosoftConfirm(ctx context.Context) (configurationCompleted bool, err error) { +func (svc *Service) ConditionalAccessMicrosoftConfirm(ctx context.Context) (configurationCompleted bool, setupError string, err error) { // Check user is authorized to write integrations. if err := svc.authz.Authorize(ctx, &fleet.ConditionalAccessMicrosoftIntegration{}, fleet.ActionWrite); err != nil { - return false, ctxerr.Wrap(ctx, err, "failed to authorize") + return false, "", ctxerr.Wrap(ctx, err, "failed to authorize") } if !svc.config.MicrosoftCompliancePartner.IsSet() { - return false, &fleet.BadRequestError{Message: "microsoft conditional access configuration not set"} + return false, "", &fleet.BadRequestError{Message: "microsoft conditional access configuration not set"} } // Load current integration. integration, err := svc.ds.ConditionalAccessMicrosoftGet(ctx) if err != nil { - return false, ctxerr.Wrap(ctx, err, "failed to load the integration") + return false, "", ctxerr.Wrap(ctx, err, "failed to load the integration") } if integration.SetupDone { - return true, nil + return true, "", nil } getResponse, err := svc.conditionalAccessMicrosoftProxy.Get(ctx, integration.TenantID, integration.ProxyServerSecret) if err != nil { level.Error(svc.logger).Log("msg", "failed to get integration settings from proxy", "err", err) - return false, nil + return false, "", nil } if !getResponse.SetupDone { - return false, nil + var setupError string + if getResponse.SetupError != nil { + level.Error(svc.logger).Log("msg", "setup is not done", "setup_error", getResponse.SetupError) + setupError = *getResponse.SetupError + } + return false, setupError, nil } if err := svc.ds.ConditionalAccessMicrosoftMarkSetupDone(ctx); err != nil { - return false, ctxerr.Wrap(ctx, err, "failed to mark setup_done=true") + return false, "", ctxerr.Wrap(ctx, err, "failed to mark setup_done=true") } if err := svc.NewActivity( @@ -147,10 +154,10 @@ func (svc *Service) ConditionalAccessMicrosoftConfirm(ctx context.Context) (conf authz.UserFromContext(ctx), fleet.ActivityTypeAddedConditionalAccessIntegrationMicrosoft{}, ); err != nil { - return false, ctxerr.Wrap(ctx, err, "create activity for conditional access integration microsoft") + return false, "", ctxerr.Wrap(ctx, err, "create activity for conditional access integration microsoft") } - return true, nil + return true, "", nil } type conditionalAccessMicrosoftDeleteRequest struct{} diff --git a/website/api/controllers/microsoft-proxy/receive-redirect-from-microsoft.js b/website/api/controllers/microsoft-proxy/receive-redirect-from-microsoft.js index 8889fd52d4..4a3cbac901 100644 --- a/website/api/controllers/microsoft-proxy/receive-redirect-from-microsoft.js +++ b/website/api/controllers/microsoft-proxy/receive-redirect-from-microsoft.js @@ -42,6 +42,7 @@ module.exports = { // If an error or error_description are provided, then the admin did not consent, and we will return a 200 response. if(error || error_description) {// eslint-disable-line camelcase // If an admin did not consent (or a user who started connecting the integration does not have admin permissions), try to match the provided state to a MicrosoftComplianceTenant record, and redirect to that. + // IMPORTANT: Don't change the below setupError value. The error string is checked by the Fleet UI frontend. let newSetupError = 'A Microsoft Entra admin did not consent to the permissions requested by the conditional access integration.'; let tenantRecordWithThisState = await MicrosoftComplianceTenant.findOne({stateTokenForAdminConsent: state}); // If we couldn't find a MicrosoftComplianceTenant record with this state, return a notFound response. @@ -202,6 +203,7 @@ module.exports = { // If the response from the Microsoft Graph API did not contain any groups, log a warning and save a setup error on the database record for this tenant. if(parsedGroupResponse.value.length === 0){ sails.log.warn(`When an Entra tenant (${informationAboutThisTenant.fleetInstanceUrl}) tried setting up a conditional access integration, no "Fleet conditional access" Entra ID group was found on this Entra tenant.`); + // IMPORTANT: Don't change the below setupError value. The error string is checked by the Fleet UI frontend. await MicrosoftComplianceTenant.updateOne({id: informationAboutThisTenant.id}).set({setupError: `No "Fleet conditional access" Entra ID group was found on this Entra tenant.`}); throw {redirect: fleetInstanceUrlToRedirectTo }; } @@ -209,7 +211,7 @@ module.exports = { let groupId = parsedGroupResponse.value[0].id; - // Send a request to assign the new compliance policy to the "All users" group. + // Send a request to assign the new compliance policy to the "Fleet conditional access" group. let assignPolicyResponse = await sails.helpers.http.sendHttpRequest.with({ method: 'POST', url: `${tenantDataSyncUrl}/PartnerCompliancePolicies(guid'${encodeURIComponent(createdPolicyId)}')/Assign?api-version=1.6`, @@ -224,7 +226,7 @@ module.exports = { } }).intercept(async (err)=>{ await MicrosoftComplianceTenant.updateOne({id: informationAboutThisTenant.id}).set({setupError: `${require('util').inspect(err, {depth: null})}`}); - sails.log.warn(`An error occurred when sending a assign a new compliance policy to "All users" on a Microsoft compliance tenant. Full error: ${require('util').inspect(err, {depth: 3})}`); + sails.log.warn(`An error occurred when sending a assign a new compliance policy to "Fleet conditional access" group on a Microsoft compliance tenant. Full error: ${require('util').inspect(err, {depth: 3})}`); return {redirect: fleetInstanceUrlToRedirectTo }; }); // Example response: