From e22aad362ec5975a65ca3489d23a851213a75515 Mon Sep 17 00:00:00 2001 From: Alan Parra Date: Tue, 29 Oct 2024 17:02:27 -0300 Subject: [PATCH] Warn users about upcoming OTP for session MFA deprecation (#48096) --- api/mfa/ceremony.go | 7 +++++++ api/mfa/prompt.go | 27 ++++++++++++++++++++++++++- lib/client/mfa/cli.go | 10 ++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/api/mfa/ceremony.go b/api/mfa/ceremony.go index 67b55e8fea379..c542ba36c42b8 100644 --- a/api/mfa/ceremony.go +++ b/api/mfa/ceremony.go @@ -18,6 +18,7 @@ package mfa import ( "context" + "slices" "github.com/gravitational/trace" @@ -74,6 +75,12 @@ func (c *Ceremony) Run(ctx context.Context, req *proto.CreateAuthenticateChallen return nil, trace.Wrap(&ErrMFANotSupported, "mfa ceremony must have PromptConstructor set in order to succeed") } + // Set challenge extensions in the prompt, if present, but set it first so the + // caller can still override it. + if req != nil && req.ChallengeExtensions != nil { + promptOpts = slices.Insert(promptOpts, 0, WithPromptChallengeExtensions(req.ChallengeExtensions)) + } + resp, err := c.PromptConstructor(promptOpts...).Run(ctx, chal) return resp, trace.Wrap(err) } diff --git a/api/mfa/prompt.go b/api/mfa/prompt.go index e139ff561fa64..1d5c0673d8c1d 100644 --- a/api/mfa/prompt.go +++ b/api/mfa/prompt.go @@ -21,6 +21,7 @@ import ( "fmt" "github.com/gravitational/teleport/api/client/proto" + mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" ) // Prompt is an MFA prompt. @@ -50,6 +51,9 @@ type PromptConfig struct { DeviceType DeviceDescriptor // Quiet suppresses users prompts. Quiet bool + // Extensions are the challenge extensions used to create the prompt's challenge. + // Used to enrich certain prompts. + Extensions *mfav1.ChallengeExtensions } // DeviceDescriptor is a descriptor for a device, such as "registered". @@ -83,7 +87,18 @@ func WithPromptReasonAdminAction() PromptOpt { // WithPromptReasonSessionMFA sets the prompt's PromptReason field to a standard session mfa message. func WithPromptReasonSessionMFA(serviceType, serviceName string) PromptOpt { - return WithPromptReason(fmt.Sprintf("MFA is required to access %s %q", serviceType, serviceName)) + return func(cfg *PromptConfig) { + cfg.PromptReason = fmt.Sprintf("MFA is required to access %s %q", serviceType, serviceName) + + // Set the extensions to scope USER_SESSION, which we know is true, but + // don't override any explicitly-set extensions (as they are likely more + // complete). + if cfg.Extensions == nil { + cfg.Extensions = &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_USER_SESSION, + } + } + } } // WithPromptDeviceType sets the prompt's DeviceType field. @@ -92,3 +107,13 @@ func WithPromptDeviceType(deviceType DeviceDescriptor) PromptOpt { cfg.DeviceType = deviceType } } + +// WithPromptChallengeExtensions sets the challenge extensions used to create +// the prompt's challenge. +// While not mandatory, informing the prompt of the extensions used allows for +// better user messaging. +func WithPromptChallengeExtensions(exts *mfav1.ChallengeExtensions) PromptOpt { + return func(cfg *PromptConfig) { + cfg.Extensions = exts + } +} diff --git a/lib/client/mfa/cli.go b/lib/client/mfa/cli.go index a5e4cc8f26178..309a8d7000636 100644 --- a/lib/client/mfa/cli.go +++ b/lib/client/mfa/cli.go @@ -31,6 +31,7 @@ import ( "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/api/constants" + mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" "github.com/gravitational/teleport/api/utils/prompt" wancli "github.com/gravitational/teleport/lib/auth/webauthncli" wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes" @@ -140,6 +141,15 @@ func (c *CLIPrompt) Run(ctx context.Context, chal *proto.MFAAuthenticateChalleng promptOTP = false } + // DELETE IN v18.0 after TOTP session MFA support is removed (codingllama) + // Technically we could remove api/mfa.WithPromptChallengeExtensions along + // with this, as it's likely its only use, although arguably keeping it could + // prove useful. + usageSessionMFA := c.cfg.Extensions.GetScope() == mfav1.ChallengeScope_CHALLENGE_SCOPE_USER_SESSION + if promptOTP && usageSessionMFA { + fmt.Fprint(c.writer(), "\nWARNING: Starting with Teleport 18, OTP will not be accepted for per-session MFA.\n\n") + } + switch { case promptOTP && promptWebauthn: resp, err := c.promptWebauthnAndOTP(ctx, chal)