Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/auth/authclient/clt.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,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 @@ -562,10 +562,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 @@ -720,18 +722,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