Skip to content
2 changes: 1 addition & 1 deletion lib/auth/authclient/clt.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 5 additions & 5 deletions lib/client/cluster_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
21 changes: 13 additions & 8 deletions lib/srv/app/connections_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}

Expand Down
52 changes: 37 additions & 15 deletions lib/srv/app/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}{
Expand Down Expand Up @@ -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")
},
},
Expand All @@ -981,30 +991,42 @@ 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,
},
Allow: types.RoleConditions{
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 {
Expand Down
3 changes: 2 additions & 1 deletion lib/web/desktop.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion lib/web/terminal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading