Skip to content

[v17] Revert "Created check for device trust after MFA before creating web session (#63260)" #63528

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zmb3 merged 1 commit into from
Feb 6, 2026
Merged

[v17] Revert "Created check for device trust after MFA before creating web session (#63260)" #63528

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions integration/appaccess/appaccess_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,7 @@ func testAuditEvents(p *Pack, t *testing.T) {
AppPublicAddr: p.rootAppPublicAddr,
AppName: p.rootAppName,
},
PublicAddr: p.rootAppPublicAddr,
}
return len(cmp.Diff(
expectedEvent,
Expand Down
107 changes: 32 additions & 75 deletions lib/auth/sessions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,8 @@ type NewAppSessionRequest struct {
AppURI string
// AppTargetPort signifies that the session is made to a specific port of a multi-port TCP app.
AppTargetPort int
// Identity is the identity of the user.
Identity tlsca.Identity
// ClientAddr is a client (user's) address.
ClientAddr string

Expand Down Expand Up @@ -496,8 +498,6 @@ func (a *Server) CreateAppSession(ctx context.Context, req *proto.CreateAppSessi
MFAVerified: verifiedMFADeviceID,
AppName: req.AppName,
AppURI: req.URI,
BotName: identity.BotName,
BotInstanceID: identity.BotInstanceID,
DeviceExtensions: DeviceExtensions(identity.DeviceExtensions),
})
if err != nil {
Expand Down Expand Up @@ -543,69 +543,6 @@ func (a *Server) CreateAppSessionFromReq(ctx context.Context, req NewAppSessionR
return nil, trace.Wrap(err)
}

// Audit fields used for both success and failure
sessionStartEvent := &apievents.AppSessionStart{
Metadata: apievents.Metadata{
Type: events.AppSessionStartEvent,
ClusterName: req.ClusterName,
},
ServerMetadata: apievents.ServerMetadata{
ServerVersion: teleport.Version,
ServerID: a.ServerID,
ServerNamespace: apidefaults.Namespace,
},
ConnectionMetadata: apievents.ConnectionMetadata{
RemoteAddr: req.ClientAddr,
},
AppMetadata: apievents.AppMetadata{
AppURI: req.AppURI,
AppPublicAddr: req.PublicAddr,
AppName: req.AppName,
AppTargetPort: uint32(req.AppTargetPort),
},
}

// Enforce device trust early via the AccessChecker.
if err = checker.CheckDeviceAccess(services.AccessState{
DeviceVerified: dtauthz.IsTLSDeviceVerified((*tlsca.DeviceExtensions)(&req.DeviceExtensions)),
EnableDeviceVerification: true,
IsBot: req.BotName != "",
}); err != nil {
userKind := apievents.UserKind_USER_KIND_HUMAN
if req.BotName != "" {
userKind = apievents.UserKind_USER_KIND_BOT
}

userMetadata := apievents.UserMetadata{
User: req.User,
BotName: req.BotName,
BotInstanceID: req.BotInstanceID,
UserKind: userKind,
AWSRoleARN: req.AWSRoleARN,
}

if req.DeviceExtensions.DeviceID != "" {
userMetadata.TrustedDevice = &apievents.DeviceMetadata{
DeviceId: req.DeviceExtensions.DeviceID,
AssetTag: req.DeviceExtensions.AssetTag,
CredentialId: req.DeviceExtensions.CredentialID,
}
}
errMsg := "requires a trusted device"

sessionStartEvent.Metadata.SetCode(events.AppSessionStartFailureCode)
sessionStartEvent.UserMetadata = userMetadata
sessionStartEvent.SessionMetadata = apievents.SessionMetadata{
WithMFA: req.MFAVerified,
}
sessionStartEvent.Error = err.Error()
sessionStartEvent.UserMessage = errMsg

a.emitter.EmitAuditEvent(a.closeCtx, sessionStartEvent)
// err swallowed/obscured on purpose.
return nil, trace.AccessDenied("%s", errMsg)
}

// Create certificate for this session.
priv, err := cryptosuites.GenerateKey(ctx, cryptosuites.GetCurrentSuiteFromAuthPreference(a), cryptosuites.UserTLS)
if err != nil {
Expand Down Expand Up @@ -704,16 +641,36 @@ func (a *Server) CreateAppSessionFromReq(ctx context.Context, req NewAppSessionR
userMetadata.User = session.GetUser()
userMetadata.AWSRoleARN = req.AWSRoleARN

sessionStartEvent.Metadata.SetCode(events.AppSessionStartCode)
sessionStartEvent.UserMetadata = userMetadata
sessionStartEvent.SessionMetadata = apievents.SessionMetadata{
SessionID: session.GetName(),
WithMFA: req.MFAVerified,
PrivateKeyPolicy: string(identity.PrivateKeyPolicy),
}

if err := a.emitter.EmitAuditEvent(a.closeCtx, sessionStartEvent); err != nil {
a.logger.WarnContext(ctx, "Failed to emit app session start event", "error", err)
err = a.emitter.EmitAuditEvent(a.closeCtx, &apievents.AppSessionStart{
Metadata: apievents.Metadata{
Type: events.AppSessionStartEvent,
Code: events.AppSessionStartCode,
ClusterName: req.ClusterName,
},
ServerMetadata: apievents.ServerMetadata{
ServerVersion: teleport.Version,
ServerID: a.ServerID,
ServerNamespace: apidefaults.Namespace,
},
SessionMetadata: apievents.SessionMetadata{
SessionID: session.GetName(),
WithMFA: req.MFAVerified,
PrivateKeyPolicy: string(req.Identity.PrivateKeyPolicy),
},
UserMetadata: userMetadata,
ConnectionMetadata: apievents.ConnectionMetadata{
RemoteAddr: req.ClientAddr,
},
PublicAddr: req.PublicAddr,
AppMetadata: apievents.AppMetadata{
AppURI: req.AppURI,
AppPublicAddr: req.PublicAddr,
AppName: req.AppName,
AppTargetPort: uint32(req.AppTargetPort),
},
})
if err != nil {
log.WithError(err).Warn("Failed to emit app session start event")
}

return session, nil
Expand Down
225 changes: 0 additions & 225 deletions lib/auth/sessions_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,23 +22,13 @@ import (
"time"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/google/uuid"
"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/gravitational/teleport/api/client/proto"
"github.com/gravitational/teleport/api/constants"
devicepb "github.com/gravitational/teleport/api/gen/proto/go/teleport/devicetrust/v1"
"github.com/gravitational/teleport/api/types"
apievents "github.com/gravitational/teleport/api/types/events"
"github.com/gravitational/teleport/lib/auth"
"github.com/gravitational/teleport/lib/auth/authtest"
"github.com/gravitational/teleport/lib/events"
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/tlsca"
)

func TestServer_CreateWebSessionFromReq_deviceWebToken(t *testing.T) {
Expand Down Expand Up @@ -95,218 +85,3 @@ func TestServer_CreateWebSessionFromReq_deviceWebToken(t *testing.T) {
}
})
}

func TestCreateAppSession_DeviceTrust(t *testing.T) {
t.Parallel()

fakeClock := clockwork.NewFakeClock()

testAuthServer, err := authtest.NewAuthServer(authtest.AuthServerConfig{
Clock: fakeClock,
Dir: t.TempDir(),
})
require.NoError(t, err)
t.Cleanup(func() { testAuthServer.Close() })

authServer := testAuthServer.AuthServer

tests := []struct {
name string
trustMode string
botName string
deviceExtensions auth.DeviceExtensions
wantErr string
}{
{
name: "Access Denied - Trusted Device Required but Missing",
trustMode: constants.DeviceTrustModeRequired,
deviceExtensions: auth.DeviceExtensions{},
wantErr: "requires a trusted device",
},
{
name: "Success - Trust Optional and Device Missing",
trustMode: constants.DeviceTrustModeOptional,
deviceExtensions: auth.DeviceExtensions{},
},
{
name: "Success - Trust Mode Not Set",
trustMode: "",
deviceExtensions: auth.DeviceExtensions{},
},
{
name: "Success - Trusted Device Required and Provided",
trustMode: constants.DeviceTrustModeRequired,
deviceExtensions: auth.DeviceExtensions{
DeviceID: "macbook-id-123",
AssetTag: "asset-tag-123",
CredentialID: "cred-id-123",
},
},
{
name: "Success - Bot Access with RequiredForHumans and Missing Device",
trustMode: constants.DeviceTrustModeRequiredForHumans,
botName: "example-bot",
deviceExtensions: auth.DeviceExtensions{},
},
{
name: "Access Denied - Bot Access with Device Trust Required",
trustMode: constants.DeviceTrustModeRequired,
botName: "example-bot",
deviceExtensions: auth.DeviceExtensions{},
wantErr: "requires a trusted device",
},
{
name: "Access Denied - Human Access with RequiredForHumans but Missing Device",
trustMode: constants.DeviceTrustModeRequiredForHumans,
botName: "",
deviceExtensions: auth.DeviceExtensions{},
wantErr: "requires a trusted device",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
ctx := t.Context()
suffix := uuid.NewString()
username := "user-" + suffix
roleName := "role-" + suffix
publicAddr := "www-" + suffix + ".example.com"

role, err := types.NewRole(roleName, types.RoleSpecV6{
Options: types.RoleOptions{
DeviceTrustMode: tt.trustMode,
},
Allow: types.RoleConditions{
AppLabels: types.Labels{"*": []string{"*"}},
},
})
require.NoError(t, err)
_, err = authServer.CreateRole(ctx, role)
require.NoError(t, err)

traits := map[string][]string{
"groups": {"admins", "devs"},
"email": {"alice@example.com"},
}
user, err := types.NewUser(username)
require.NoError(t, err)
user.SetTraits(traits)
user.AddRole(roleName)
_, err = authServer.CreateUser(ctx, user)
require.NoError(t, err)

identity := tlsca.Identity{
Username: username,
BotName: tt.botName,
DeviceExtensions: tlsca.DeviceExtensions(tt.deviceExtensions),
}

cName, err := authServer.GetClusterName()
require.NoError(t, err)
clusterName := cName.GetClusterName()

checker, err := services.NewAccessChecker(&services.AccessInfo{
Username: username,
Roles: user.GetRoles(),
}, clusterName, authServer)
require.NoError(t, err)

req := &proto.CreateAppSessionRequest{
Username: username,
AppName: "example-app",
URI: "http://example.com",
ClusterName: clusterName,
PublicAddr: publicAddr,
}

startTime := fakeClock.Now()
_, err = authServer.CreateAppSession(ctx, req, identity, checker)
endTime := fakeClock.Now()

if tt.wantErr != "" {
require.ErrorContains(t, err, tt.wantErr)
assert.True(t, trace.IsAccessDenied(err), "Expected AccessDenied, got %v", err)
} else {
require.NoError(t, err)
}

logEntries, _, err := testAuthServer.AuditLog.SearchEvents(ctx, events.SearchEventsRequest{
From: startTime.Add(-time.Second),
To: endTime.Add(time.Second),
Limit: 100, // arbitrary, enough to allow for events from concurrent tests
})
require.NoError(t, err)

expectedKind := apievents.UserKind_USER_KIND_HUMAN
if tt.botName != "" {
expectedKind = apievents.UserKind_USER_KIND_BOT
}

expectedEvent := &apievents.AppSessionStart{
Metadata: apievents.Metadata{
Type: events.AppSessionStartEvent,
ClusterName: clusterName,
},
UserMetadata: apievents.UserMetadata{
User: username,
BotName: tt.botName,
UserKind: expectedKind,
},
AppMetadata: apievents.AppMetadata{
AppName: "example-app",
AppURI: "http://example.com",
AppPublicAddr: publicAddr,
},
}

if tt.wantErr == "" {
expectedEvent.Metadata.Code = events.AppSessionStartCode
expectedEvent.SessionMetadata.PrivateKeyPolicy = "none"
} else {
expectedEvent.Metadata.Code = events.AppSessionStartFailureCode
expectedEvent.UserMessage = "requires a trusted device"
expectedEvent.Error = "access to resource requires a trusted device"
}

if tt.deviceExtensions.DeviceID != "" {
expectedEvent.UserMetadata.TrustedDevice = &apievents.DeviceMetadata{
DeviceId: tt.deviceExtensions.DeviceID,
AssetTag: tt.deviceExtensions.AssetTag,
CredentialId: tt.deviceExtensions.CredentialID,
}
}

var eventFound bool
for _, event := range logEntries {
appStart, ok := event.(*apievents.AppSessionStart)
if !ok || appStart.UserMetadata.User != username {
continue
}

eventFound = true

diff := cmp.Diff(expectedEvent, appStart,
cmpopts.IgnoreUnexported(
apievents.AppSessionStart{},
apievents.Metadata{},
apievents.UserMetadata{},
apievents.AppMetadata{},
apievents.SessionMetadata{},
apievents.ConnectionMetadata{},
apievents.ServerMetadata{},
apievents.DeviceMetadata{},
),
cmpopts.IgnoreFields(apievents.Metadata{}, "ID", "Time", "Index"),
cmpopts.IgnoreFields(apievents.ServerMetadata{}, "ServerID", "ServerVersion", "ServerNamespace"),
cmpopts.IgnoreFields(apievents.ConnectionMetadata{}, "RemoteAddr", "Protocol"),
cmpopts.IgnoreFields(apievents.SessionMetadata{}, "SessionID"),
cmpopts.IgnoreFields(apievents.AppSessionStart{}, "PublicAddr"), // deprecated field
)

require.Empty(t, diff, "Audit event mismatch for case: %s\n%s", tt.name, diff)
}
require.True(t, eventFound, "Expected AppSessionStart event was not found in audit log")
})
}
}
4 changes: 0 additions & 4 deletions lib/services/access_checker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,6 @@ type AccessChecker interface {
// CheckAccess checks access to the specified resource.
CheckAccess(r AccessCheckable, state AccessState, matchers ...RoleMatcher) error

// CheckDeviceAccess verifies if the current device state satisfies the
// device trust requirements of the user's RoleSet.
CheckDeviceAccess(state AccessState) error

// CheckAccessToRemoteCluster checks access to remote cluster
CheckAccessToRemoteCluster(cluster types.RemoteCluster) error

Expand Down
Loading
Loading