Revert "Created check for device trust after MFA before creating web session (#62019) (#63260)"

21KennethTran · 21KennethTran · commit 9d68b5fde4d6 · 2026-02-05T16:40:07.000-08:00
This reverts commit 64e28c6.
diff --git a/integration/appaccess/appaccess_test.go b/integration/appaccess/appaccess_test.go
@@ -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,
diff --git a/lib/auth/sessions.go b/lib/auth/sessions.go
@@ -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
 
@@ -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 {
@@ -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 {
@@ -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
diff --git a/lib/auth/sessions_test.go b/lib/auth/sessions_test.go
@@ -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) {
@@ -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")
-		})
-	}
-}
diff --git a/lib/services/access_checker.go b/lib/services/access_checker.go
@@ -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
 
diff --git a/lib/services/role.go b/lib/services/role.go

Original file line number	Diff line number	Diff line change
`@@ -588,6 +588,7 @@ func testAuditEvents(p Pack, t testing.T) {`
`588`	`588`	`AppPublicAddr: p.rootAppPublicAddr,`
`589`	`589`	`AppName: p.rootAppName,`
`590`	`590`	`},`
	`591`	`+ PublicAddr: p.rootAppPublicAddr,`
`591`	`592`	`}`
`592`	`593`	`return len(cmp.Diff(`
`593`	`594`	`expectedEvent,`