Merge branch 'branch/v15' into bot/backport-39033-branch/v15

api/proto/teleport/mfa/v1/mfa.proto

@@ -66,6 +66,8 @@ enum ChallengeScope {
   // more precise scope. Instead, new scopes should be added. This scope may
   // also be split into multiple smaller scopes in the future.
   CHALLENGE_SCOPE_ADMIN_ACTION = 7;
+  // Used for changing user's password.
+  CHALLENGE_SCOPE_CHANGE_PASSWORD = 8;
 }
 
 // ChallengeAllowReuse determines whether an MFA challenge response can be used

lib/auth/accountrecovery_test.go

@@ -34,6 +34,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/types"
 	apievents "github.com/gravitational/teleport/api/types/events"
 	wanpb "github.com/gravitational/teleport/api/types/webauthn"
@@ -1232,10 +1233,14 @@ func TestGetAccountRecoveryCodes(t *testing.T) {
 
 func triggerLoginLock(t *testing.T, srv *Server, username string) {
 	for i := 1; i <= defaults.MaxLoginAttempts; i++ {
-		_, _, _, err := srv.authenticateUser(context.Background(), AuthenticateUserRequest{
-			Username: username,
-			OTP:      &OTPCreds{},
-		})
+		_, _, _, err := srv.authenticateUser(
+			context.Background(),
+			AuthenticateUserRequest{
+				Username: username,
+				OTP:      &OTPCreds{},
+			},
+			mfav1.ChallengeExtensions{Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN},
+		)
 		require.True(t, trace.IsAccessDenied(err))
 
 		// Test last attempt returns locked error.

lib/auth/auth_login_test.go

@@ -968,7 +968,7 @@ func TestServer_Authenticate_nonPasswordlessRequiresUsername(t *testing.T) {
 		{
 			name:    "WebAuthn",
 			dev:     mfa.WebDev,
-			wantErr: "invalid Webauthn response", // generic error as it _could_ be a passwordless attempt
+			wantErr: "invalid credentials", // generic error as it _could_ be a passwordless attempt
 		},
 	}
 	for _, test := range tests {

lib/auth/auth_test.go

@@ -51,6 +51,7 @@ import (
 	"github.com/gravitational/teleport/api/client/proto"
 	"github.com/gravitational/teleport/api/constants"
 	apidefaults "github.com/gravitational/teleport/api/defaults"
+	mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1"
 	"github.com/gravitational/teleport/api/types"
 	apievents "github.com/gravitational/teleport/api/types/events"
 	"github.com/gravitational/teleport/api/types/header"
@@ -624,7 +625,12 @@ func TestAuthenticateUser_mfaDeviceLocked(t *testing.T) {
 
 	t.Run("locked device password change", func(t *testing.T) {
 		chal, err := userClient.CreateAuthenticateChallenge(ctx, &proto.CreateAuthenticateChallengeRequest{
-			Request: &proto.CreateAuthenticateChallengeRequest_ContextUser{},
+			Request: &proto.CreateAuthenticateChallengeRequest_ContextUser{
+				ContextUser: &proto.ContextUser{},
+			},
+			ChallengeExtensions: &mfav1.ChallengeExtensions{
+				Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_CHANGE_PASSWORD,
+			},
 		})
 		require.NoError(t, err, "CreateAuthenticateChallenge")
 

lib/auth/methods.go

@@ -120,7 +120,11 @@ type SessionCreds struct {
 func (a *Server) AuthenticateUser(ctx context.Context, req AuthenticateUserRequest) (services.UserState, services.AccessChecker, error) {
 	username := req.Username
 
-	verifyMFALocks, mfaDev, actualUsername, err := a.authenticateUser(ctx, req)
+	requiredExt := mfav1.ChallengeExtensions{
+		Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN,
+	}
+	verifyMFALocks, mfaDev, actualUsername, err := a.authenticateUser(
+		ctx, req, requiredExt)
 	if err != nil {
 		// Log event after authentication failure
 		if err := a.emitAuthAuditEvent(ctx, authAuditProps{
@@ -259,7 +263,7 @@ var (
 	authenticateHeadlessError = trace.AccessDenied("headless authentication failed")
 	// authenticateWebauthnError is the generic error returned for failed WebAuthn
 	// authentication attempts.
-	authenticateWebauthnError = trace.AccessDenied("invalid Webauthn response")
+	authenticateWebauthnError = trace.AccessDenied("invalid credentials")
 	// invalidUserPassError is the error for when either the provided username or
 	// password is incorrect.
 	invalidUserPassError = trace.AccessDenied("invalid username or password")
@@ -285,7 +289,19 @@ type verifyMFADeviceLocksParams struct {
 }
 
 // authenticateUser authenticates a user through various methods (password, MFA,
-// passwordless)
+// passwordless). Note that "authenticate" may mean different things, depending
+// on the request and required extensions. The caller is responsible for making
+// sure that the request and extensions correctly describe the conditions that
+// are being asserted during the authentication process. One important caveat is
+// that if the request contains a WebAuthn challenge response, the user name is
+// known, and `authenticateUser` is called without a password, the function only
+// performs a second factor check; in this case, the caller must provide
+// appropriate required extensions to validate the WebAuthn challenge response.
+//
+// The `requiredExt` parameter is used to assert that the MFA challenge has a
+// correct extensions. It is ignored for passwordless authentication, where we
+// override it with an extension that has scope set to
+// `mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN`.
 //
 // Returns a callback to verify MFA device locks, the MFA device used to
 // authenticate (if applicable), and the authenticated user name.
@@ -294,8 +310,9 @@ type verifyMFADeviceLocksParams struct {
 func (a *Server) authenticateUser(
 	ctx context.Context,
 	req AuthenticateUserRequest,
+	requiredExt mfav1.ChallengeExtensions,
 ) (verifyLocks func(verifyMFADeviceLocksParams) error, mfaDev *types.MFADevice, user string, err error) {
-	mfaDev, user, err = a.authenticateUserInternal(ctx, req)
+	mfaDev, user, err = a.authenticateUserInternal(ctx, req, requiredExt)
 	if err != nil || mfaDev == nil {
 		return func(verifyMFADeviceLocksParams) error { return nil }, mfaDev, user, trace.Wrap(err)
 	}
@@ -340,7 +357,11 @@ func (a *Server) authenticateUser(
 }
 
 // Do not use this method directly, use authenticateUser instead.
-func (a *Server) authenticateUserInternal(ctx context.Context, req AuthenticateUserRequest) (mfaDev *types.MFADevice, user string, err error) {
+func (a *Server) authenticateUserInternal(
+	ctx context.Context,
+	req AuthenticateUserRequest,
+	requiredExt mfav1.ChallengeExtensions,
+) (mfaDev *types.MFADevice, user string, err error) {
 	if err := req.CheckAndSetDefaults(); err != nil {
 		return nil, "", trace.Wrap(err)
 	}
@@ -349,6 +370,10 @@ func (a *Server) authenticateUserInternal(ctx context.Context, req AuthenticateU
 
 	// Only one path if passwordless, other variants shouldn't see an empty user.
 	if passwordless {
+		if requiredExt.Scope != mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN {
+			return nil, "", trace.BadParameter(
+				"passwordless authentication can only be used with the PASSWORDLESS scope")
+		}
 		return a.authenticatePasswordless(ctx, req)
 	}
 
@@ -381,8 +406,7 @@ func (a *Server) authenticateUserInternal(ctx context.Context, req AuthenticateU
 					Webauthn: wantypes.CredentialAssertionResponseToProto(req.Webauthn),
 				},
 			}
-			requiredExt := &mfav1.ChallengeExtensions{Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN}
-			mfaData, err := a.ValidateMFAAuthResponse(ctx, mfaResponse, user, requiredExt)
+			mfaData, err := a.ValidateMFAAuthResponse(ctx, mfaResponse, user, &requiredExt)
 			if err != nil {
 				return nil, trace.Wrap(err)
 			}

lib/auth/password.go

@@ -23,6 +23,7 @@ import (
 	"crypto/subtle"
 	"net/mail"
 
+	"github.com/go-webauthn/webauthn/protocol"
 	"github.com/gravitational/trace"
 	"github.com/pquerna/otp"
 	"github.com/pquerna/otp/totp"
@@ -31,6 +32,7 @@ import (
 	"github.com/gravitational/teleport"
 	"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/types"
 	apievents "github.com/gravitational/teleport/api/types/events"
 	"github.com/gravitational/teleport/api/utils/keys"
@@ -129,16 +131,26 @@ func (a *Server) ChangePassword(ctx context.Context, req *proto.ChangePasswordRe
 		Username: user,
 		Webauthn: wantypes.CredentialAssertionResponseFromProto(req.Webauthn),
 	}
-	authReq.Pass = &PassCreds{
-		Password: req.OldPassword,
+	requiredExt := mfav1.ChallengeExtensions{
+		Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_CHANGE_PASSWORD,
+	}
+	if len(req.OldPassword) > 0 {
+		authReq.Pass = &PassCreds{
+			Password: req.OldPassword,
+		}
+	} else {
+		// If the user didn't provide their old password, we need to require
+		// identity verification (i.e. make sure that a resident token used for
+		// MFA).
+		requiredExt.UserVerificationRequirement = string(protocol.VerificationRequired)
 	}
 	if req.SecondFactorToken != "" {
 		authReq.OTP = &OTPCreds{
 			Password: req.OldPassword,
 			Token:    req.SecondFactorToken,
 		}
 	}
-	verifyMFALocks, _, _, err := a.authenticateUser(ctx, authReq)
+	verifyMFALocks, _, _, err := a.authenticateUser(ctx, authReq, requiredExt)
 	if err != nil {
 		return trace.Wrap(err)
 	}