gravitational/teleport
1
0
Fork 0
mirror of https://github.com/gravitational/teleport.git synced 2026-10-11 22:49:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

Reduce scope of MFARequiredUnknown in PerformSessionMFACeremony (#64765)

Browse Source
This commit is contained in:
Edoardo Spadolini
2026-03-23 11:49:58 +00:00
committed by GitHub
parent 0bf2f90712
commit 389a6ddf99
1 changed files with 21 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/client/cluster_client.go
+21 -18
View File
@@ -767,16 +767,18 @@ func PerformSessionMFACeremony(ctx context.Context, params PerformSessionMFACere
// If connecting to a host in a leaf cluster and MFA failed check to see
// if the leaf cluster requires MFA. If it doesn't return an error indicating
// that MFA was not required instead of the error received from the root cluster.
var mfaKnownToBeRequired bool
if mfaRequiredReq != nil && !params.MFAAgainstRoot {
mfaRequiredResp, err := currentClient.IsMFARequired(ctx, mfaRequiredReq)
log.DebugContext(ctx, "MFA requirement acquired from leaf", "mfa_required", mfaRequiredResp.GetMFARequired())
switch {
case err != nil:
return nil, trace.Wrap(MFARequiredUnknown(err))
return nil, trace.Wrap(MFARequiredUnknown(trace.Unwrap(err)))
case !mfaRequiredResp.Required:
return nil, trace.Wrap(services.ErrSessionMFANotRequired)
}
mfaRequiredReq = nil // Already checked, don't check again at root.
mfaKnownToBeRequired = true
}
allowReuse := mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO
@@ -784,7 +786,20 @@ func PerformSessionMFACeremony(ctx context.Context, params PerformSessionMFACere
allowReuse = mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES
}
params.MFACeremony.CreateAuthenticateChallenge = rootClient.CreateAuthenticateChallenge
params.MFACeremony.CreateAuthenticateChallenge = func(ctx context.Context, req *proto.CreateAuthenticateChallengeRequest) (*proto.MFAAuthenticateChallenge, error) {
chal, err := rootClient.CreateAuthenticateChallenge(ctx, req)
if err != nil {
// not an exhaustive list, but connection problem and limit exceeded are
// almost surely caused by network conditions or general problems rather
// than being from the actual handling of the request
if !mfaKnownToBeRequired && (trace.IsConnectionProblem(err) || trace.IsLimitExceeded(err)) {
return nil, trace.Wrap(MFARequiredUnknown(trace.Unwrap(err)))
}
return nil, trace.Wrap(err)
}
return chal, nil
}
mfaResp, err := params.MFACeremony.Run(ctx, &proto.CreateAuthenticateChallengeRequest{
Request: &proto.CreateAuthenticateChallengeRequest_ContextUser{
ContextUser: &proto.ContextUser{},
@@ -795,22 +810,10 @@ func PerformSessionMFACeremony(ctx context.Context, params PerformSessionMFACere
AllowReuse: allowReuse,
},
}, promptOpts...)
if err != nil {
switch {
case errors.Is(err, &mfa.ErrMFANotRequired):
return nil, trace.Wrap(services.ErrSessionMFANotRequired)
// this is not a completely exhaustive list for errors that are
// definitely not errors coming from the MFA ceremony itself, but we
// (and gRPC in general) don't seem to have a great way of
// distinguishing errors that are definitely coming from the upper
// layers of the connection from errors deep in the server handler;
// connection problem and limit exceeded are errors that are
// overwhelmingly more likely to happen in the former, tho
case trace.IsConnectionProblem(err), trace.IsLimitExceeded(err):
return nil, trace.Wrap(MFARequiredUnknown(trace.Unwrap(err)))
default:
return nil, trace.Wrap(err)
}
if errors.Is(err, &mfa.ErrMFANotRequired) {
return nil, trace.Wrap(services.ErrSessionMFANotRequired)
} else if err != nil {
return nil, trace.Wrap(err)
}
// If mfaResp.GetResponse() is nil, the ceremony was a no-op (no devices