From 442e03b276252b307cd707d0c246215212219e17 Mon Sep 17 00:00:00 2001 From: Roberto Dip Date: Wed, 26 Jul 2023 14:20:36 -0300 Subject: [PATCH] Improve the error handling for MDM SSO during DEP enrollment (#12966) For #12692 --- changes/12692-macos-helpful-message | 1 + ee/server/service/mdm.go | 56 ++++++++++++------- frontend/components/MDM/SSOError/SSOError.tsx | 5 +- .../MDMAppleSSOCallbackPage.tsx | 7 ++- server/fleet/service.go | 7 ++- server/mdm/apple/apple_mdm.go | 4 ++ server/service/apple_mdm.go | 22 ++++---- server/service/integration_mdm_test.go | 14 ++++- 8 files changed, 79 insertions(+), 37 deletions(-) create mode 100644 changes/12692-macos-helpful-message diff --git a/changes/12692-macos-helpful-message b/changes/12692-macos-helpful-message new file mode 100644 index 0000000000..09ae83e0ef --- /dev/null +++ b/changes/12692-macos-helpful-message @@ -0,0 +1 @@ +* Improve error handling and messaging of SSO login during AEP(DEP) enrollments. diff --git a/ee/server/service/mdm.go b/ee/server/service/mdm.go index 89a9729b25..fb4c7d5728 100644 --- a/ee/server/service/mdm.go +++ b/ee/server/service/mdm.go @@ -8,6 +8,7 @@ import ( "encoding/base64" "encoding/json" "errors" + "fmt" "io" "net/http" "net/url" @@ -653,21 +654,41 @@ func (svc *Service) InitiateMDMAppleSSO(ctx context.Context) (string, error) { return idpURL, nil } -func (svc *Service) InitiateMDMAppleSSOCallback(ctx context.Context, auth fleet.Auth) (string, error) { +func (svc *Service) InitiateMDMAppleSSOCallback(ctx context.Context, auth fleet.Auth) string { // skipauth: User context does not yet exist. Unauthenticated users may // hit the SSO callback. svc.authz.SkipAuthorization(ctx) logging.WithLevel(logging.WithNoUser(ctx), level.Info) + profileToken, enrollmentRef, eulaToken, err := svc.mdmSSOHandleCallbackAuth(ctx, auth) + + if err != nil { + logging.WithErr(ctx, err) + return apple_mdm.FleetUISSOCallbackPath + "?error=true" + } + + q := url.Values{ + "profile_token": {profileToken}, + "enrollment_reference": {enrollmentRef}, + } + + if eulaToken != "" { + q.Add("eula_token", eulaToken) + } + + return fmt.Sprintf("%s?%s", apple_mdm.FleetUISSOCallbackPath, q.Encode()) +} + +func (svc *Service) mdmSSOHandleCallbackAuth(ctx context.Context, auth fleet.Auth) (string, string, string, error) { appConfig, err := svc.ds.AppConfig(ctx) if err != nil { - return "", ctxerr.Wrap(ctx, err, "get config for sso") + return "", "", "", ctxerr.Wrap(ctx, err, "get config for sso") } _, metadata, err := svc.ssoSessionStore.Fullfill(auth.RequestID()) if err != nil { - return "", ctxerr.Wrap(ctx, err, "validate request in session") + return "", "", "", ctxerr.Wrap(ctx, err, "validate request in session") } var ssoSettings fleet.SSOSettings @@ -684,7 +705,7 @@ func (svc *Service) InitiateMDMAppleSSOCallback(ctx context.Context, auth fleet. ) if err != nil { - return "", ctxerr.Wrap(ctx, err, "validating sso response") + return "", "", "", ctxerr.Wrap(ctx, err, "validating sso response") } // Store information for automatic account population/creation @@ -704,35 +725,32 @@ func (svc *Service) InitiateMDMAppleSSOCallback(ctx context.Context, auth fleet. Fullname: auth.UserDisplayName(), } if err := svc.ds.InsertMDMIdPAccount(ctx, &idpAcc); err != nil { - return "", ctxerr.Wrap(ctx, err, "saving account data from IdP") + return "", "", "", ctxerr.Wrap(ctx, err, "saving account data from IdP") } eula, err := svc.ds.MDMAppleGetEULAMetadata(ctx) if err != nil && !fleet.IsNotFound(err) { - return "", ctxerr.Wrap(ctx, err, "getting EULA metadata") + return "", "", "", ctxerr.Wrap(ctx, err, "getting EULA metadata") + } + + var eulaToken string + if eula != nil { + eulaToken = eula.Token } // get the automatic profile to access the authentication token. depProf, err := svc.getAutomaticEnrollmentProfile(ctx) if err != nil { - return "", ctxerr.Wrap(ctx, err, "listing profiles") + return "", "", "", ctxerr.Wrap(ctx, err, "listing profiles") } if depProf == nil { - return "", ctxerr.Wrap(ctx, err, "missing profile") + return "", "", "", ctxerr.Wrap(ctx, err, "missing profile") } - q := url.Values{ - "profile_token": {depProf.Token}, - // using the idp token as a reference just because that's the - // only thing we're referencing later on during enrollment. - "enrollment_reference": {idpAcc.UUID}, - } - if eula != nil { - q.Add("eula_token", eula.Token) - } - - return appConfig.ServerSettings.ServerURL + "/mdm/sso/callback?" + q.Encode(), nil + // using the idp token as a reference just because that's the + // only thing we're referencing later on during enrollment. + return depProf.Token, idpAcc.UUID, eulaToken, nil } func (svc *Service) mdmAppleSyncDEPProfiles(ctx context.Context) error { diff --git a/frontend/components/MDM/SSOError/SSOError.tsx b/frontend/components/MDM/SSOError/SSOError.tsx index f0fece9ef6..43e5afa7bc 100644 --- a/frontend/components/MDM/SSOError/SSOError.tsx +++ b/frontend/components/MDM/SSOError/SSOError.tsx @@ -14,7 +14,10 @@ const SSOError = ({ className }: ISSOErrorProps) => { return ( -

Please contact your IT admin at +1-(415)-651-2575.

+

+ Select Cancel and try again. If this keeps happening, + please contact IT support. +

); }; diff --git a/frontend/pages/MDMAppleSSOCallbackPage/MDMAppleSSOCallbackPage.tsx b/frontend/pages/MDMAppleSSOCallbackPage/MDMAppleSSOCallbackPage.tsx index e1e09fd6fb..8aa14b207d 100644 --- a/frontend/pages/MDMAppleSSOCallbackPage/MDMAppleSSOCallbackPage.tsx +++ b/frontend/pages/MDMAppleSSOCallbackPage/MDMAppleSSOCallbackPage.tsx @@ -18,16 +18,18 @@ interface IEnrollmentGateProps { profileToken?: string; eulaToken?: string; enrollmentReference?: string; + error?: boolean; } const EnrollmentGate = ({ profileToken, eulaToken, enrollmentReference, + error, }: IEnrollmentGateProps) => { const [showEULA, setShowEULA] = useState(Boolean(eulaToken)); - if (!profileToken) { + if (!profileToken || error) { return ; } @@ -65,6 +67,7 @@ interface IMDMSSOCallbackQuery { eula_token?: string; profile_token?: string; enrollment_reference?: string; + error?: boolean; } const MDMAppleSSOCallbackPage = ( @@ -74,6 +77,7 @@ const MDMAppleSSOCallbackPage = ( eula_token, profile_token, enrollment_reference, + error, } = props.location.query; return (
@@ -81,6 +85,7 @@ const MDMAppleSSOCallbackPage = ( eulaToken={eula_token} profileToken={profile_token} enrollmentReference={enrollment_reference} + error={error} />
); diff --git a/server/fleet/service.go b/server/fleet/service.go index 98cb462ba5..042107e833 100644 --- a/server/fleet/service.go +++ b/server/fleet/service.go @@ -170,9 +170,10 @@ type Service interface { // are valid InitSSOCallback(ctx context.Context, auth Auth) (string, error) - // InitSSOCallback handles the IDP response and ensures the credentials - // are valid, then responds with an enrollment profile. - InitiateMDMAppleSSOCallback(ctx context.Context, auth Auth) (string, error) + // InitiateMDMAppleSSOCallback handles the IDP response and ensures the + // credentials are valid, then responds with an URL to the Fleet UI to + // handle next steps based on the query parameters provided. + InitiateMDMAppleSSOCallback(ctx context.Context, auth Auth) string // GetSSOUser handles retrieval of an user that is trying to authenticate // via SSO diff --git a/server/mdm/apple/apple_mdm.go b/server/mdm/apple/apple_mdm.go index 3177aa8bd7..1a09a8bf45 100644 --- a/server/mdm/apple/apple_mdm.go +++ b/server/mdm/apple/apple_mdm.go @@ -43,6 +43,10 @@ const ( // InstallerPath is the HTTP path that serves installers to Apple devices. InstallerPath = "/api/mdm/apple/installer" + // FleetUISSOCallbackPath is the front-end route used to + // redirect after the SSO flow is completed. + FleetUISSOCallbackPath = "/mdm/sso/callback" + // FleetPayloadIdentifier is the value for the "PayloadIdentifier" // used by Fleet MDM on the enrollment profile. FleetPayloadIdentifier = "com.fleetdm.fleet.mdm.apple" diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index c59d4fa21f..4cad0b2d56 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -2143,6 +2143,10 @@ func (svc *Service) InitiateMDMAppleSSO(ctx context.Context) (string, error) { type callbackMDMAppleSSORequest struct{} +// TODO: these errors will result in JSON being returned, but we should +// redirect to the UI and let the UI display an error instead. The errors are +// rare enough (malformed data coming from the SSO provider) so they shouldn't +// affect many users. func (callbackMDMAppleSSORequest) DecodeRequest(ctx context.Context, r *http.Request) (interface{}, error) { err := r.ParseForm() if err != nil { @@ -2162,8 +2166,6 @@ func (callbackMDMAppleSSORequest) DecodeRequest(ctx context.Context, r *http.Req } type callbackMDMAppleSSOResponse struct { - Err error `json:"error,omitempty"` - redirectURL string } @@ -2172,25 +2174,23 @@ func (r callbackMDMAppleSSOResponse) hijackRender(ctx context.Context, w http.Re w.WriteHeader(http.StatusTemporaryRedirect) } -func (r callbackMDMAppleSSOResponse) error() error { return r.Err } +// Error will always be nil because errors are handled by sending a query +// parameter in the URL response, this way the UI is able to display an erorr +// message. +func (r callbackMDMAppleSSOResponse) error() error { return nil } func callbackMDMAppleSSOEndpoint(ctx context.Context, request interface{}, svc fleet.Service) (errorer, error) { auth := request.(fleet.Auth) - - // validate that the SSO response is valid - redirectURL, err := svc.InitiateMDMAppleSSOCallback(ctx, auth) - if err != nil { - return callbackMDMAppleSSOResponse{Err: err}, nil - } + redirectURL := svc.InitiateMDMAppleSSOCallback(ctx, auth) return callbackMDMAppleSSOResponse{redirectURL: redirectURL}, nil } -func (svc *Service) InitiateMDMAppleSSOCallback(ctx context.Context, auth fleet.Auth) (string, error) { +func (svc *Service) InitiateMDMAppleSSOCallback(ctx context.Context, auth fleet.Auth) string { // skipauth: No authorization check needed due to implementation // returning only license error. svc.authz.SkipAuthorization(ctx) - return "", fleet.ErrMissingLicense + return apple_mdm.FleetUISSOCallbackPath + "?error=true" } //////////////////////////////////////////////////////////////////////////////// diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index a4b836798a..6d4f018219 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -5167,6 +5167,7 @@ func (s *integrationMDMTestSuite) TestSSO() { require.False(t, q.Has("eula_token")) require.True(t, q.Has("profile_token")) require.True(t, q.Has("enrollment_reference")) + require.False(t, q.Has("error")) // the url retrieves a valid profile s.downloadAndVerifyEnrollmentProfile( fmt.Sprintf( @@ -5191,6 +5192,7 @@ func (s *integrationMDMTestSuite) TestSSO() { require.True(t, q.Has("eula_token")) require.True(t, q.Has("profile_token")) require.True(t, q.Has("enrollment_reference")) + require.False(t, q.Has("error")) // the url retrieves a valid profile prof := s.downloadAndVerifyEnrollmentProfile( fmt.Sprintf( @@ -5260,9 +5262,17 @@ func (s *integrationMDMTestSuite) TestSSO() { require.Contains(t, lastSubmittedProfile.URL, "https://example.com/api/mdm/apple/enroll?token=") require.Equal(t, "https://example.com/mdm/sso", lastSubmittedProfile.ConfigurationWebURL) - // hitting the callback with an invalid session id results in a 4xx + // hitting the callback with an invalid session id redirects the user to the UI rawSSOResp := base64.StdEncoding.EncodeToString([]byte(``)) - s.DoRawNoAuth("POST", "/api/v1/fleet/mdm/sso/callback?SAMLResponse="+url.QueryEscape(rawSSOResp), nil, http.StatusUnauthorized) + res = s.DoRawNoAuth("POST", "/api/v1/fleet/mdm/sso/callback?SAMLResponse="+url.QueryEscape(rawSSOResp), nil, http.StatusTemporaryRedirect) + require.NotEmpty(t, res.Header.Get("Location")) + u, err = url.Parse(res.Header.Get("Location")) + require.NoError(t, err) + q = u.Query() + require.False(t, q.Has("eula_token")) + require.False(t, q.Has("profile_token")) + require.False(t, q.Has("enrollment_reference")) + require.True(t, q.Has("error")) } type scepPayload struct {