diff --git a/lib/auth/authclient/clt.go b/lib/auth/authclient/clt.go index e938e91cd97c7..b020624a65709 100644 --- a/lib/auth/authclient/clt.go +++ b/lib/auth/authclient/clt.go @@ -83,7 +83,7 @@ var ( // ErrNoMFADevices is returned when an MFA ceremony is performed without possible devices to // complete the challenge with. ErrNoMFADevices = &trace.AccessDeniedError{ - Message: "MFA is required to access this resource but user has no MFA devices; see Account Settings in the Web UI or use 'tsh mfa add' to register MFA devices", + Message: "Multi-factor authentication (MFA) is required to access this resource but the current user has no supported MFA devices enrolled; see Account Settings in the Web UI or use 'tsh mfa add' to register an MFA device", } // InvalidUserPassError is the error for when either the provided username or // password is incorrect. diff --git a/lib/client/cluster_client.go b/lib/client/cluster_client.go index 40962d53ccc74..ecf82be99005e 100644 --- a/lib/client/cluster_client.go +++ b/lib/client/cluster_client.go @@ -801,11 +801,11 @@ func PerformSessionMFACeremony(ctx context.Context, params PerformSessionMFACere return nil, trace.Wrap(err) } - // If mfaResp is nil, the ceremony was a no-op (no devices registered). - // TODO(Joerger): CreateAuthenticateChallenge, should return - // this error directly instead of an empty challenge, without - // regressing https://github.com/gravitational/teleport/issues/36482. - if mfaResp == nil { + // If mfaResp.GetResponse() is nil, the ceremony was a no-op (no devices + // registered). TODO(Joerger): CreateAuthenticateChallenge, should return + // this error directly instead of an empty challenge, without regressing + // https://github.com/gravitational/teleport/issues/36482. + if mfaResp.GetResponse() == nil { return nil, trace.Wrap(authclient.ErrNoMFADevices) } diff --git a/lib/srv/app/connections_handler.go b/lib/srv/app/connections_handler.go index 33948663e847b..152f057341a33 100644 --- a/lib/srv/app/connections_handler.go +++ b/lib/srv/app/connections_handler.go @@ -561,10 +561,12 @@ func (c *ConnectionsHandler) authorizeContext(ctx context.Context) (*authz.Conte app, state, matchers...); { - case errors.Is(err, services.ErrTrustedDeviceRequired): - // Let the trusted device error through for clarity. - return nil, nil, trace.Wrap(services.ErrTrustedDeviceRequired) + case errors.Is(err, services.ErrTrustedDeviceRequired) || errors.Is(err, services.ErrSessionMFARequired): + // When access is denied due to trusted device or session MFA requirements, these specific errors + // are returned directly to provide clarity to the client about the additional authentication steps needed. + return nil, nil, trace.Wrap(err) case err != nil: + // Other access denial errors are wrapped and obfuscated to prevent leaking sensitive details. c.log.WarnContext(c.closeContext, "Access denied to application.", "app", app.GetName(), "error", err, @@ -719,18 +721,21 @@ func (c *ConnectionsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if err != nil { c.log.WarnContext(c.closeContext, "Failed to serve request", "error", err) - // Covert trace error type to HTTP and write response, make sure we close the + // Convert trace error type to HTTP and write response, make sure we close the // connection afterwards so that the monitor is recreated if needed. code := trace.ErrorToCode(err) var text string - if errors.Is(err, services.ErrTrustedDeviceRequired) { - // Return a nicer error message for device trust errors. - text = `Access to this app requires a trusted device. + switch { + case errors.Is(err, services.ErrTrustedDeviceRequired): + text = `A trusted device is required to access this resource but this device has not been registered as a trusted device; use 'tsh device enroll' to register as a trusted device. See https://goteleport.com/docs/admin-guides/access-controls/device-trust/device-management/#troubleshooting for help. ` - } else { + case errors.Is(err, services.ErrSessionMFARequired): + text = authclient.ErrNoMFADevices.Error() + + default: text = http.StatusText(code) } diff --git a/lib/srv/app/server_test.go b/lib/srv/app/server_test.go index 489cb6b3d668a..57fbb2572d1dd 100644 --- a/lib/srv/app/server_test.go +++ b/lib/srv/app/server_test.go @@ -923,6 +923,7 @@ func TestAuthorize(t *testing.T) { roleAppLabels types.Labels appLabels map[string]string requireTrustedDevice bool // assigns user to a role that requires trusted devices + requireMFADevice bool // assigns user to a role that requires an enrolled MFA device wantStatus int assertBody func(t *testing.T, gotBody string) bool // optional, matched against s.message if nil }{ @@ -966,7 +967,16 @@ func TestAuthorize(t *testing.T) { requireTrustedDevice: true, wantStatus: http.StatusForbidden, assertBody: func(t *testing.T, gotBody string) bool { - const want = "app requires a trusted device" + const want = "trusted device is required to access this resource" + return assert.Contains(t, gotBody, want, "response body mismatch") + }, + }, + { + name: "enrolled MFA device required", + requireMFADevice: true, + wantStatus: http.StatusForbidden, + assertBody: func(t *testing.T, gotBody string) bool { + const want = "Multi-factor authentication (MFA) is required to access this resource" return assert.Contains(t, gotBody, want, "response body mismatch") }, }, @@ -981,11 +991,24 @@ func TestAuthorize(t *testing.T) { RoleAppLabels: test.roleAppLabels, }) - if test.requireTrustedDevice { + // Helper to create and assign a role to the user, then refresh certificate. + assignRoleAndRefreshCert := func(roleName string, roleSpec types.RoleSpecV6) { authServer := s.authServer.AuthServer + role, err := types.NewRole(roleName, roleSpec) + require.NoError(t, err, "NewRole") + role, err = authServer.CreateRole(ctx, role) + require.NoError(t, err, "CreateRole") + + user := s.user + user.AddRole(role.GetName()) + user, err = authServer.Services.UpdateUser(ctx, user) + require.NoError(t, err, "UpdateUser") + + s.clientCertificate = s.generateCertificate(t, user, s.appFoo.GetPublicAddr(), "" /* awsRoleARN */) + } - // Create a role that requires a trusted device. - requiredDevRole, err := types.NewRole("require-trusted-devices-app", types.RoleSpecV6{ + if test.requireTrustedDevice { + assignRoleAndRefreshCert("require-trusted-devices-app", types.RoleSpecV6{ Options: types.RoleOptions{ DeviceTrustMode: constants.DeviceTrustModeRequired, }, @@ -993,18 +1016,17 @@ func TestAuthorize(t *testing.T) { AppLabels: types.Labels{"*": []string{"*"}}, }, }) - require.NoError(t, err, "NewRole") - requiredDevRole, err = authServer.CreateRole(ctx, requiredDevRole) - require.NoError(t, err, "CreateRole") - - // Add role to test user. - user := s.user - user.AddRole(requiredDevRole.GetName()) - user, err = authServer.Services.UpdateUser(ctx, user) - require.NoError(t, err, "UpdateUser") + } - // Refresh user certificate. - s.clientCertificate = s.generateCertificate(t, user, s.appFoo.GetPublicAddr(), "" /* awsRoleARN */) + if test.requireMFADevice { + assignRoleAndRefreshCert("require-mfa-app", types.RoleSpecV6{ + Options: types.RoleOptions{ + RequireMFAType: types.RequireMFAType_SESSION, + }, + Allow: types.RoleConditions{ + AppLabels: types.Labels{"*": []string{"*"}}, + }, + }) } if test.cloudLabels != nil { diff --git a/lib/web/desktop.go b/lib/web/desktop.go index 45301cf1cb61a..7e7f8b377f2fa 100644 --- a/lib/web/desktop.go +++ b/lib/web/desktop.go @@ -38,6 +38,7 @@ import ( "github.com/gravitational/teleport/api/mfa" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/api/utils/keys" + "github.com/gravitational/teleport/lib/auth/authclient" wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes" "github.com/gravitational/teleport/lib/authz" "github.com/gravitational/teleport/lib/client" @@ -416,7 +417,7 @@ func (h *Handler) performSessionMFACeremony( } if chal.WebauthnChallenge == nil && chal.SSOChallenge == nil { - return nil, trace.AccessDenied("Only WebAuthn and SSO MFA methods are supported on the web, please register a supported MFA method to connect to this desktop") + return nil, trace.Wrap(authclient.ErrNoMFADevices) } // Send the challenge over the socket. diff --git a/lib/web/terminal.go b/lib/web/terminal.go index a0ab57995145d..c5e8d5d4582ba 100644 --- a/lib/web/terminal.go +++ b/lib/web/terminal.go @@ -654,7 +654,7 @@ func newMFACeremony(stream *terminal.WSStream, createAuthenticateChallenge mfa.C } if chal.WebauthnChallenge == nil && chal.SSOChallenge == nil { - return nil, trace.AccessDenied("only WebAuthn and SSO MFA methods are supported on the web terminal, please register a supported mfa method to connect to this server") + return nil, trace.Wrap(authclient.ErrNoMFADevices) } var codec protobufMFACodec