Fix list resources when in-band MFA is required for all SSH connections (#65464)

* Fix list resources when TELEPORT_UNSTABLE_FORCE_IN_BAND_MFA=yes.

Signed-off-by: Chris Thach <chris.thach@goteleport.com>

* Improve comments.

Signed-off-by: Chris Thach <chris.thach@goteleport.com>

* Minify test name.

Signed-off-by: Chris Thach <chris.thach@goteleport.com>

---------

Signed-off-by: Chris Thach <chris.thach@goteleport.com>
This commit is contained in:
Chris Thach
2026-04-16 14:57:19 +00:00
committed by GitHub
parent 035aa48cd0
commit fcd89d4d23
2 changed files with 52 additions and 4 deletions
+43
View File
@@ -924,6 +924,49 @@ func TestAccessChecker_CheckConditionalAccess_RoleRequiresMFA_ForceInBandMFAEnv_
)
}
// TODO(cthach): Remove in v20.0 when the legacy out-of-band MFA flow is removed.
func TestAccessChecker_CheckAccess_ReadOnlyBypassWhenMFAForced(t *testing.T) {
t.Setenv("TELEPORT_UNSTABLE_FORCE_IN_BAND_MFA", "yes")
const roleName = "mfa-required"
roleSet := NewRoleSet(newRole(func(r *types.RoleV6) {
r.SetName(roleName)
r.SetOptions(types.RoleOptions{
RequireMFAType: types.RequireMFAType_SESSION,
})
}))
accessInfo := &AccessInfo{
Roles: []string{roleName},
}
accessChecker := NewAccessCheckerWithRoleSet(accessInfo, "cluster", roleSet)
srv, err := types.NewServer(
"test-server",
types.KindNode,
types.ServerSpecV2{},
)
require.NoError(t, err)
node := &serverStub{Server: srv}
err = accessChecker.CheckAccess(
node,
AccessState{
// Simulate a read-only access check that is being allowed to bypass MFA requirements even when the role
// requires MFA and the force in-band MFA env var is set. This is to allow users to perform read-only
// operations like listing nodes without being forced to complete MFA verification.
MFARequired: MFARequiredPerRole,
MFAVerified: true,
ReturnPreconditions: false,
},
)
require.NoError(t, err)
}
func TestSSHPortForwarding(t *testing.T) {
anyLabels := types.Labels{"*": {"*"}}
localCluster := "cluster"
+9 -4
View File
@@ -2776,13 +2776,18 @@ func (set RoleSet) checkAccess(
// MFA checks can be bypassed if either:
// 1. The cluster doesn't require per-session MFA (MFARequiredNever), OR
// 2. The legacy out-of-band MFA flow is allowed (see below) AND MFA has already been verified for the session.
// 2. Legacy out-of-band MFA has already been verified for the session AND
// a. The legacy out-of-band MFA flow is allowed (TELEPORT_UNSTABLE_FORCE_IN_BAND_MFA is not set to "yes") OR
// b. The caller doesn't want preconditions returned (state.ReturnPreconditions is false)
//
// The legacy out-of-band MFA flow is allowed as long as TELEPORT_UNSTABLE_FORCE_IN_BAND_MFA is not set to "yes"
// and MFA has already been verified for this session.
// Listing resources sets state.MFAVerified to true and state.ReturnPreconditions to false to allow bypassing MFA
// checks for resources that require per-session MFA. This is because listing resources is a read-only operation and
// MFA is not required to list resources, even if MFA is required to access the resource. The actual enforcement
// will happen at connection time, so this is not a concern from a security perspective.
//
// TODO(cthach): Remove in v20.0 when the legacy out-of-band MFA flow is removed.
bypassMFAChecks := state.MFARequired == MFARequiredNever || (os.Getenv("TELEPORT_UNSTABLE_FORCE_IN_BAND_MFA") != "yes" && state.MFAVerified)
bypassMFAChecks := state.MFARequired == MFARequiredNever ||
(state.MFAVerified && (os.Getenv("TELEPORT_UNSTABLE_FORCE_IN_BAND_MFA") != "yes" || !state.ReturnPreconditions))
// TODO(codingllama): Consider making EnableDeviceVerification opt-out instead
// of opt-in.