From f812321f2bb3a18c84a27e4e67c43775829b5685 Mon Sep 17 00:00:00 2001 From: joerger Date: Fri, 12 Jan 2024 17:26:28 -0800 Subject: [PATCH 01/11] Use SessionData with extensions in Webauthn flow. --- lib/auth/webauthn/login.go | 10 ++++++++-- lib/auth/webauthn/login_mfa.go | 7 ++++++- lib/auth/webauthn/login_passwordless.go | 7 ++++++- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/lib/auth/webauthn/login.go b/lib/auth/webauthn/login.go index ae4e71cbb5c5d..367f112336016 100644 --- a/lib/auth/webauthn/login.go +++ b/lib/auth/webauthn/login.go @@ -32,6 +32,7 @@ import ( "github.com/gravitational/trace" log "github.com/sirupsen/logrus" + mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" "github.com/gravitational/teleport/api/types" wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes" ) @@ -66,7 +67,12 @@ type loginFlow struct { sessionData sessionIdentity } -func (f *loginFlow) begin(ctx context.Context, user string, passwordless bool) (*wantypes.CredentialAssertion, error) { +func (f *loginFlow) begin(ctx context.Context, user string, ext mfav1.ChallengeExtensions) (*wantypes.CredentialAssertion, error) { + if ext.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES && ext.Scope != mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION { + return nil, trace.BadParameter("mfa challenges with scope %s cannot allow reuse", ext.Scope) + } + + passwordless := ext.Scope == mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN if user == "" && !passwordless { return nil, trace.BadParameter("user required") } @@ -166,7 +172,7 @@ func (f *loginFlow) begin(ctx context.Context, user string, passwordless bool) ( if err != nil { return nil, trace.Wrap(err) } - // TODO(Joerger): set challenge extensions from caller + sd.ChallengeExtensions = &ext if err := f.sessionData.Upsert(ctx, user, sd); err != nil { return nil, trace.Wrap(err) diff --git a/lib/auth/webauthn/login_mfa.go b/lib/auth/webauthn/login_mfa.go index 8b083e4a0c706..e0c97d67f0140 100644 --- a/lib/auth/webauthn/login_mfa.go +++ b/lib/auth/webauthn/login_mfa.go @@ -24,6 +24,7 @@ import ( "github.com/gravitational/trace" + mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" "github.com/gravitational/teleport/api/types" wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes" ) @@ -100,7 +101,11 @@ func (f *LoginFlow) Begin(ctx context.Context, user string) (*wantypes.Credentia identity: mfaIdentity{f.Identity}, sessionData: (*userSessionStorage)(f), } - return lf.begin(ctx, user, false /* passwordless */) + ext := mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, + } + return lf.begin(ctx, user, ext) } // Finish is the second and last step of the LoginFlow. diff --git a/lib/auth/webauthn/login_passwordless.go b/lib/auth/webauthn/login_passwordless.go index 0c28d0d5a463c..cdceefdffa96e 100644 --- a/lib/auth/webauthn/login_passwordless.go +++ b/lib/auth/webauthn/login_passwordless.go @@ -23,6 +23,7 @@ import ( "encoding/base64" "errors" + mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" "github.com/gravitational/teleport/api/types" wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes" ) @@ -54,7 +55,11 @@ func (f *PasswordlessFlow) Begin(ctx context.Context) (*wantypes.CredentialAsser identity: passwordlessIdentity{f.Identity}, sessionData: (*globalSessionStorage)(f), } - return lf.begin(ctx, "" /* user */, true /* passwordless */) + ext := mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, + } + return lf.begin(ctx, "" /* user */, ext) } // Finish is the last step of the passwordless login flow. From 4ab970f57feaea792e4b2869b1dc3dc815727ecd Mon Sep 17 00:00:00 2001 From: joerger Date: Fri, 12 Jan 2024 17:56:27 -0800 Subject: [PATCH 02/11] Pass MFAChallengeExtensions through webauthn flow. --- lib/auth/auth.go | 15 ++++++- lib/auth/webauthn/login.go | 4 +- lib/auth/webauthn/login_mfa.go | 12 ++---- lib/auth/webauthn/login_passwordless.go | 6 ++- lib/auth/webauthn/login_test.go | 57 +++++++++++++++++++------ lib/auth/webauthncli/u2f_login_test.go | 29 ++++++++++--- 6 files changed, 90 insertions(+), 33 deletions(-) diff --git a/lib/auth/auth.go b/lib/auth/auth.go index a66967ba27739..3b884e1d9751f 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -66,6 +66,7 @@ import ( "github.com/gravitational/teleport/api/constants" apidefaults "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/gen/proto/go/assist/v1" + mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" "github.com/gravitational/teleport/api/internalutils/stream" "github.com/gravitational/teleport/api/metadata" "github.com/gravitational/teleport/api/types" @@ -5806,7 +5807,12 @@ func (a *Server) mfaAuthChallenge(ctx context.Context, user string, passwordless Webauthn: webConfig, Identity: wanlib.WithDevices(a.Services, groupedDevs.Webauthn), } - assertion, err := webLogin.Begin(ctx, user) + // TODO(Joerger): Get extensions from caller. + ext := mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_UNSPECIFIED, + } + assertion, err := webLogin.Begin(ctx, user, ext) if err != nil { return nil, trace.Wrap(err) } @@ -5932,7 +5938,12 @@ func (a *Server) ValidateMFAAuthResponse(ctx context.Context, resp *proto.MFAAut Webauthn: webConfig, Identity: a.Services, } - dev, err = webLogin.Finish(ctx, user, wantypes.CredentialAssertionResponseFromProto(res.Webauthn)) + // TODO(Joerger): Get extensions from caller. + ext := mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_UNSPECIFIED, + } + dev, err = webLogin.Finish(ctx, user, wantypes.CredentialAssertionResponseFromProto(res.Webauthn), ext) } if err != nil { return nil, "", trace.AccessDenied("MFA response validation failed: %v", err) diff --git a/lib/auth/webauthn/login.go b/lib/auth/webauthn/login.go index 367f112336016..866664c7ad852 100644 --- a/lib/auth/webauthn/login.go +++ b/lib/auth/webauthn/login.go @@ -192,7 +192,9 @@ func (f *loginFlow) getWebID(ctx context.Context, user string) ([]byte, error) { return wla.UserID, nil } -func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.CredentialAssertionResponse, passwordless bool) (*types.MFADevice, string, error) { +func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.CredentialAssertionResponse, requiredExtensions mfav1.ChallengeExtensions) (*types.MFADevice, string, error) { + passwordless := requiredExtensions.Scope == mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN + switch { case user == "" && !passwordless: return nil, "", trace.BadParameter("user required") diff --git a/lib/auth/webauthn/login_mfa.go b/lib/auth/webauthn/login_mfa.go index e0c97d67f0140..136616435dbab 100644 --- a/lib/auth/webauthn/login_mfa.go +++ b/lib/auth/webauthn/login_mfa.go @@ -94,32 +94,28 @@ type LoginFlow struct { // assertion. // As a side effect Begin may assign (and record in storage) a WebAuthn ID for // the user. -func (f *LoginFlow) Begin(ctx context.Context, user string) (*wantypes.CredentialAssertion, error) { +func (f *LoginFlow) Begin(ctx context.Context, user string, challengeExtensions mfav1.ChallengeExtensions) (*wantypes.CredentialAssertion, error) { lf := &loginFlow{ U2F: f.U2F, Webauthn: f.Webauthn, identity: mfaIdentity{f.Identity}, sessionData: (*userSessionStorage)(f), } - ext := mfav1.ChallengeExtensions{ - Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, - AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, - } - return lf.begin(ctx, user, ext) + return lf.begin(ctx, user, challengeExtensions) } // Finish is the second and last step of the LoginFlow. // It returns the MFADevice used to solve the challenge. If login is successful, // Finish has the side effect of updating the counter and last used timestamp of // the returned device. -func (f *LoginFlow) Finish(ctx context.Context, user string, resp *wantypes.CredentialAssertionResponse) (*types.MFADevice, error) { +func (f *LoginFlow) Finish(ctx context.Context, user string, resp *wantypes.CredentialAssertionResponse, requiredExtensions mfav1.ChallengeExtensions) (*types.MFADevice, error) { lf := &loginFlow{ U2F: f.U2F, Webauthn: f.Webauthn, identity: mfaIdentity{f.Identity}, sessionData: (*userSessionStorage)(f), } - dev, _, err := lf.finish(ctx, user, resp, false /* passwordless */) + dev, _, err := lf.finish(ctx, user, resp, requiredExtensions) return dev, trace.Wrap(err) } diff --git a/lib/auth/webauthn/login_passwordless.go b/lib/auth/webauthn/login_passwordless.go index cdceefdffa96e..09d058389ae90 100644 --- a/lib/auth/webauthn/login_passwordless.go +++ b/lib/auth/webauthn/login_passwordless.go @@ -71,7 +71,11 @@ func (f *PasswordlessFlow) Finish(ctx context.Context, resp *wantypes.Credential identity: passwordlessIdentity{f.Identity}, sessionData: (*globalSessionStorage)(f), } - return lf.finish(ctx, "" /* user */, resp, true /* passwordless */) + requiredExt := mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, + } + return lf.finish(ctx, "" /* user */, resp, requiredExt) } type passwordlessIdentity struct { diff --git a/lib/auth/webauthn/login_test.go b/lib/auth/webauthn/login_test.go index 7c3c0aefd827e..c0d24f84a3ebe 100644 --- a/lib/auth/webauthn/login_test.go +++ b/lib/auth/webauthn/login_test.go @@ -31,6 +31,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/auth/mocku2f" wanlib "github.com/gravitational/teleport/lib/auth/webauthn" @@ -118,7 +119,9 @@ func TestLoginFlow_BeginFinish(t *testing.T) { } // 1st step of the login ceremony. - assertion, err := webLogin.Begin(ctx, user) + assertion, err := webLogin.Begin(ctx, user, mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) // We care about a few specific settings, for everything else defaults are // OK. @@ -146,7 +149,9 @@ func TestLoginFlow_BeginFinish(t *testing.T) { // 2nd and last step of the login ceremony. beforeLastUsed := time.Now().Add(-1 * time.Second) - loginDevice, err := webLogin.Finish(ctx, user, assertionResp) + loginDevice, err := webLogin.Finish(ctx, user, assertionResp, mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) // Last used time and counter are updated. require.True(t, beforeLastUsed.Before(loginDevice.LastUsed)) @@ -220,7 +225,9 @@ func TestLoginFlow_Begin_errors(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - _, err := webLogin.Begin(ctx, test.user) + _, err := webLogin.Begin(ctx, test.user, mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.True(t, test.assertErrType(err), "got err = %v, want BadParameter", err) require.Contains(t, err.Error(), test.wantErr) }) @@ -258,7 +265,9 @@ func TestLoginFlow_Finish_errors(t *testing.T) { Webauthn: webConfig, Identity: identity, } - assertion, err := webLogin.Begin(ctx, user) + assertion, err := webLogin.Begin(ctx, user, mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) okResp, err := key.SignAssertion(webOrigin, assertion) require.NoError(t, err) @@ -287,7 +296,9 @@ func TestLoginFlow_Finish_errors(t *testing.T) { name: "NOK assertion with bad origin", user: user, createResp: func() *wantypes.CredentialAssertionResponse { - assertion, err := webLogin.Begin(ctx, user) + assertion, err := webLogin.Begin(ctx, user, mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) resp, err := key.SignAssertion("https://badorigin.com", assertion) require.NoError(t, err) @@ -298,7 +309,9 @@ func TestLoginFlow_Finish_errors(t *testing.T) { name: "NOK assertion with bad RPID", user: user, createResp: func() *wantypes.CredentialAssertionResponse { - assertion, err := webLogin.Begin(ctx, user) + assertion, err := webLogin.Begin(ctx, user, mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) assertion.Response.RelyingPartyID = "badrpid.com" @@ -311,7 +324,9 @@ func TestLoginFlow_Finish_errors(t *testing.T) { name: "NOK assertion signed by unknown device", user: user, createResp: func() *wantypes.CredentialAssertionResponse { - assertion, err := webLogin.Begin(ctx, user) + assertion, err := webLogin.Begin(ctx, user, mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) unknownKey, err := mocku2f.Create() @@ -328,7 +343,9 @@ func TestLoginFlow_Finish_errors(t *testing.T) { name: "NOK assertion with invalid signature", user: user, createResp: func() *wantypes.CredentialAssertionResponse { - assertion, err := webLogin.Begin(ctx, user) + assertion, err := webLogin.Begin(ctx, user, mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) // Flip a challenge bit, this should be enough to consistently fail // signature checking. @@ -342,7 +359,9 @@ func TestLoginFlow_Finish_errors(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - _, err := webLogin.Finish(ctx, test.user, test.createResp()) + _, err := webLogin.Finish(ctx, test.user, test.createResp(), mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.Error(t, err) }) } @@ -565,7 +584,9 @@ func TestCredentialRPID(t *testing.T) { Identity: identity, } - _, err := webLogin.Begin(ctx, user) + _, err := webLogin.Begin(ctx, user, mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) assert.NoError(t, err, "Begin failed, expected assertion for `dev1`") }) @@ -575,13 +596,17 @@ func TestCredentialRPID(t *testing.T) { Identity: identity, } - assertion, err := webLogin.Begin(ctx, user) + assertion, err := webLogin.Begin(ctx, user, mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err, "Begin failed") car, err := dev1Key.SignAssertion(origin, assertion) require.NoError(t, err, "SignAssertion failed") - mfaDev, err := webLogin.Finish(ctx, user, car) + mfaDev, err := webLogin.Finish(ctx, user, car, mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err, "Finish failed") assert.Equal(t, rpID, mfaDev.GetWebauthn().CredentialRpId, "CredentialRpId mismatch") }) @@ -592,7 +617,9 @@ func TestCredentialRPID(t *testing.T) { Identity: identity, } - _, err := webLogin.Begin(ctx, user) + _, err := webLogin.Begin(ctx, user, mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) assert.ErrorIs(t, err, wanlib.ErrInvalidCredentials, "Begin error mismatch") }) @@ -609,7 +636,9 @@ func TestCredentialRPID(t *testing.T) { Webauthn: webOtherRP, Identity: identity, } - assertion, err := webLogin.Begin(ctx, user) + assertion, err := webLogin.Begin(ctx, user, mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err, "Begin failed, expected assertion for device `other1`") // Verify that we got the correct device. diff --git a/lib/auth/webauthncli/u2f_login_test.go b/lib/auth/webauthncli/u2f_login_test.go index cabd5d8895782..3ad94ff34a410 100644 --- a/lib/auth/webauthncli/u2f_login_test.go +++ b/lib/auth/webauthncli/u2f_login_test.go @@ -36,6 +36,7 @@ import ( "github.com/gravitational/trace" "github.com/stretchr/testify/require" + mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/auth/mocku2f" wanlib "github.com/gravitational/teleport/lib/auth/webauthn" @@ -140,7 +141,9 @@ func TestLogin(t *testing.T) { } test.setUserPresence.SetUserPresence(true) - assertion, err := loginFlow.Begin(ctx, username) + assertion, err := loginFlow.Begin(ctx, username, mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) if test.removeAppID { assertion.Response.Extensions = nil @@ -159,7 +162,9 @@ func TestLogin(t *testing.T) { require.NotNil(t, mfaResp.GetWebauthn()) require.Equal(t, test.wantRawID, mfaResp.GetWebauthn().RawId) - _, err = loginFlow.Finish(ctx, username, wantypes.CredentialAssertionResponseFromProto(mfaResp.GetWebauthn())) + _, err = loginFlow.Finish(ctx, username, wantypes.CredentialAssertionResponseFromProto(mfaResp.GetWebauthn()), mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) }) } @@ -182,7 +187,9 @@ func TestLogin_errors(t *testing.T) { const user = "llama" const origin = "https://localhost" ctx := context.Background() - okAssertion, err := loginFlow.Begin(ctx, user) + okAssertion, err := loginFlow.Begin(ctx, user, mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) tests := []struct { @@ -215,7 +222,9 @@ func TestLogin_errors(t *testing.T) { name: "NOK assertion missing challenge", origin: origin, getAssertion: func() *wantypes.CredentialAssertion { - assertion, err := loginFlow.Begin(ctx, user) + assertion, err := loginFlow.Begin(ctx, user, mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) assertion.Response.Challenge = nil return assertion @@ -225,7 +234,9 @@ func TestLogin_errors(t *testing.T) { name: "NOK assertion missing RPID", origin: origin, getAssertion: func() *wantypes.CredentialAssertion { - assertion, err := loginFlow.Begin(ctx, user) + assertion, err := loginFlow.Begin(ctx, user, mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) assertion.Response.RelyingPartyID = "" return assertion @@ -235,7 +246,9 @@ func TestLogin_errors(t *testing.T) { name: "NOK assertion missing credentials", origin: origin, getAssertion: func() *wantypes.CredentialAssertion { - assertion, err := loginFlow.Begin(ctx, user) + assertion, err := loginFlow.Begin(ctx, user, mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) assertion.Response.AllowedCredentials = nil return assertion @@ -245,7 +258,9 @@ func TestLogin_errors(t *testing.T) { name: "NOK assertion invalid user verification requirement", origin: origin, getAssertion: func() *wantypes.CredentialAssertion { - assertion, err := loginFlow.Begin(ctx, user) + assertion, err := loginFlow.Begin(ctx, user, mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) assertion.Response.UserVerification = protocol.VerificationRequired return assertion From 23973456d16f38c69065e21389dbc6c3aa263061 Mon Sep 17 00:00:00 2001 From: joerger Date: Fri, 12 Jan 2024 17:59:16 -0800 Subject: [PATCH 03/11] Opportunistically enforce Webauthn challenge scope. --- lib/auth/webauthn/login.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/auth/webauthn/login.go b/lib/auth/webauthn/login.go index 866664c7ad852..15202162078e4 100644 --- a/lib/auth/webauthn/login.go +++ b/lib/auth/webauthn/login.go @@ -267,6 +267,16 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred if err != nil { return nil, "", trace.Wrap(err) } + + // Check if the given scope is satisfied by the challenge scope. + if requiredExtensions.Scope != sd.ChallengeExtensions.Scope && requiredExtensions.Scope != mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED { + // old clients do not yet provide a scope, so we only enforce scope opportunistically. + // TODO(Joerger): DELETE IN v16.0.0 + if sd.ChallengeExtensions.Scope != mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED { + return nil, "", trace.AccessDenied("required scope %q is not satisfied by the given webauthn session with scope %q", requiredExtensions.Scope, sd.ChallengeExtensions.Scope) + } + } + sessionData := wantypes.SessionDataToProtocol(sd) // Make sure _all_ credentials in the session are accounted for by the user. From 3627c58cf72957ee25591b41b1c5b509ac758541 Mon Sep 17 00:00:00 2001 From: joerger Date: Fri, 12 Jan 2024 18:00:23 -0800 Subject: [PATCH 04/11] Don't delete webauthn session data when reuse is allowed. --- lib/auth/webauthn/login.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/auth/webauthn/login.go b/lib/auth/webauthn/login.go index 15202162078e4..c5f070e4ea3dd 100644 --- a/lib/auth/webauthn/login.go +++ b/lib/auth/webauthn/login.go @@ -332,9 +332,11 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred } // The user just solved the challenge, so let's make sure it won't be used - // again. - if err := f.sessionData.Delete(ctx, user, challenge); err != nil { - log.Warnf("WebAuthn: failed to delete login SessionData for user %v (passwordless = %v)", user, passwordless) + // again, unless reuse is explicitly allowed. + if sd.ChallengeExtensions.AllowReuse != mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES { + if err := f.sessionData.Delete(ctx, user, challenge); err != nil { + log.Warnf("WebAuthn: failed to delete login SessionData for user %v (scope = %s)", user, sd.ChallengeExtensions.Scope) + } } return dev, user, nil From ef63d57418a3cec1b6034e3806bd440a47409068 Mon Sep 17 00:00:00 2001 From: joerger Date: Fri, 12 Jan 2024 18:11:35 -0800 Subject: [PATCH 05/11] Return more login data from webauthn flow. --- lib/auth/auth.go | 10 +++-- lib/auth/webauthn/login.go | 51 ++++++++++++++++--------- lib/auth/webauthn/login_mfa.go | 7 +--- lib/auth/webauthn/login_passwordless.go | 2 +- lib/auth/webauthn/login_test.go | 20 +++++----- 5 files changed, 52 insertions(+), 38 deletions(-) diff --git a/lib/auth/auth.go b/lib/auth/auth.go index 3b884e1d9751f..6ce5593c94497 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -5925,13 +5925,13 @@ func (a *Server) ValidateMFAAuthResponse(ctx context.Context, resp *proto.MFAAut } assertionResp := wantypes.CredentialAssertionResponseFromProto(res.Webauthn) - var dev *types.MFADevice + var loginData *wanlib.LoginData if passwordless { webLogin := &wanlib.PasswordlessFlow{ Webauthn: webConfig, Identity: a.Services, } - dev, user, err = webLogin.Finish(ctx, assertionResp) + loginData, err = webLogin.Finish(ctx, assertionResp) } else { webLogin := &wanlib.LoginFlow{ U2F: u2f, @@ -5943,12 +5943,14 @@ func (a *Server) ValidateMFAAuthResponse(ctx context.Context, resp *proto.MFAAut Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_UNSPECIFIED, } - dev, err = webLogin.Finish(ctx, user, wantypes.CredentialAssertionResponseFromProto(res.Webauthn), ext) + loginData, err = webLogin.Finish(ctx, user, wantypes.CredentialAssertionResponseFromProto(res.Webauthn), ext) } if err != nil { return nil, "", trace.AccessDenied("MFA response validation failed: %v", err) } - return dev, user, nil + // TODO(Joerger): Refactor Validate to also return AllowReuse + // for further validation by caller when necessary. + return loginData.Device, loginData.User, nil case *proto.MFAAuthenticateResponse_TOTP: dev, err := a.checkOTP(user, res.TOTP.Code) diff --git a/lib/auth/webauthn/login.go b/lib/auth/webauthn/login.go index c5f070e4ea3dd..411546e35a01a 100644 --- a/lib/auth/webauthn/login.go +++ b/lib/auth/webauthn/login.go @@ -192,46 +192,57 @@ func (f *loginFlow) getWebID(ctx context.Context, user string) ([]byte, error) { return wla.UserID, nil } -func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.CredentialAssertionResponse, requiredExtensions mfav1.ChallengeExtensions) (*types.MFADevice, string, error) { +// LoginData is data gathered from a successful webauthn login. +type LoginData struct { + // User is a Teleport user. + User string + // Device is the MFA device used to authenticate the user. + Device *types.MFADevice + // Reusable is whether the webauthn challenge used for this login + // can be reused by the user for subsequent logins. + Reusable bool +} + +func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.CredentialAssertionResponse, requiredExtensions mfav1.ChallengeExtensions) (*LoginData, error) { passwordless := requiredExtensions.Scope == mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN switch { case user == "" && !passwordless: - return nil, "", trace.BadParameter("user required") + return nil, trace.BadParameter("user required") case resp == nil: // resp != nil is good enough to proceed, we leave remaining validations to // duo-labs/webauthn. - return nil, "", trace.BadParameter("credential assertion response required") + return nil, trace.BadParameter("credential assertion response required") } parsedResp, err := parseCredentialResponse(resp) if err != nil { - return nil, "", trace.Wrap(err) + return nil, trace.Wrap(err) } origin := parsedResp.Response.CollectedClientData.Origin if err := validateOrigin(origin, f.Webauthn.RPID); err != nil { log.WithError(err).Debugf("WebAuthn: origin validation failed") - return nil, "", trace.Wrap(err) + return nil, trace.Wrap(err) } var webID []byte if passwordless { webID = parsedResp.Response.UserHandle if len(webID) == 0 { - return nil, "", trace.BadParameter("webauthn user handle required for passwordless") + return nil, trace.BadParameter("webauthn user handle required for passwordless") } // Fetch user from WebAuthn UserHandle (aka User ID). teleportUser, err := f.identity.GetTeleportUserByWebauthnID(ctx, webID) if err != nil { - return nil, "", trace.Wrap(err) + return nil, trace.Wrap(err) } user = teleportUser } else { webID, err = f.getWebID(ctx, user) if err != nil { - return nil, "", trace.Wrap(err) + return nil, trace.Wrap(err) } } @@ -239,11 +250,11 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred // registered device. devices, err := f.identity.GetMFADevices(ctx, user, false /* withSecrets */) if err != nil { - return nil, "", trace.Wrap(err) + return nil, trace.Wrap(err) } dev, ok := findDeviceByID(devices, parsedResp.RawID) if !ok { - return nil, "", trace.BadParameter( + return nil, trace.BadParameter( "unknown device credential: %q", base64.RawURLEncoding.EncodeToString(parsedResp.RawID)) } @@ -254,7 +265,7 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred rpID := f.Webauthn.RPID switch { case dev.GetU2F() != nil && f.U2F == nil: - return nil, "", trace.BadParameter("U2F device attempted login, but U2F configuration not present") + return nil, trace.BadParameter("U2F device attempted login, but U2F configuration not present") case dev.GetU2F() != nil: rpID = f.U2F.AppID } @@ -265,7 +276,7 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred challenge := parsedResp.Response.CollectedClientData.Challenge sd, err := f.sessionData.Get(ctx, user, challenge) if err != nil { - return nil, "", trace.Wrap(err) + return nil, trace.Wrap(err) } // Check if the given scope is satisfied by the challenge scope. @@ -273,7 +284,7 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred // old clients do not yet provide a scope, so we only enforce scope opportunistically. // TODO(Joerger): DELETE IN v16.0.0 if sd.ChallengeExtensions.Scope != mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED { - return nil, "", trace.AccessDenied("required scope %q is not satisfied by the given webauthn session with scope %q", requiredExtensions.Scope, sd.ChallengeExtensions.Scope) + return nil, trace.AccessDenied("required scope %q is not satisfied by the given webauthn session with scope %q", requiredExtensions.Scope, sd.ChallengeExtensions.Scope) } } @@ -299,7 +310,7 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred requireUserVerification: passwordless, }) if err != nil { - return nil, "", trace.Wrap(err) + return nil, trace.Wrap(err) } var credential *wan.Credential @@ -310,7 +321,7 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred credential, err = web.ValidateLogin(u, *sessionData, parsedResp) } if err != nil { - return nil, "", trace.Wrap(err) + return nil, trace.Wrap(err) } if credential.Authenticator.CloneWarning { log.Warnf( @@ -319,7 +330,7 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred // Update last used timestamp and device counter. if err := setCounterAndTimestamps(dev, credential); err != nil { - return nil, "", trace.Wrap(err) + return nil, trace.Wrap(err) } // Retroactively write the credential RPID, now that it cleared authn. if webDev := dev.GetWebauthn(); webDev != nil && webDev.CredentialRpId == "" { @@ -328,7 +339,7 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred } if err := f.identity.UpsertMFADevice(ctx, user, dev); err != nil { - return nil, "", trace.Wrap(err) + return nil, trace.Wrap(err) } // The user just solved the challenge, so let's make sure it won't be used @@ -339,7 +350,11 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred } } - return dev, user, nil + return &LoginData{ + User: user, + Device: dev, + Reusable: sd.ChallengeExtensions.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + }, nil } func parseCredentialResponse(resp *wantypes.CredentialAssertionResponse) (*protocol.ParsedCredentialAssertionData, error) { diff --git a/lib/auth/webauthn/login_mfa.go b/lib/auth/webauthn/login_mfa.go index 136616435dbab..ba2a630d9e15c 100644 --- a/lib/auth/webauthn/login_mfa.go +++ b/lib/auth/webauthn/login_mfa.go @@ -22,8 +22,6 @@ import ( "context" "errors" - "github.com/gravitational/trace" - mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" "github.com/gravitational/teleport/api/types" wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes" @@ -108,15 +106,14 @@ func (f *LoginFlow) Begin(ctx context.Context, user string, challengeExtensions // It returns the MFADevice used to solve the challenge. If login is successful, // Finish has the side effect of updating the counter and last used timestamp of // the returned device. -func (f *LoginFlow) Finish(ctx context.Context, user string, resp *wantypes.CredentialAssertionResponse, requiredExtensions mfav1.ChallengeExtensions) (*types.MFADevice, error) { +func (f *LoginFlow) Finish(ctx context.Context, user string, resp *wantypes.CredentialAssertionResponse, requiredExtensions mfav1.ChallengeExtensions) (*LoginData, error) { lf := &loginFlow{ U2F: f.U2F, Webauthn: f.Webauthn, identity: mfaIdentity{f.Identity}, sessionData: (*userSessionStorage)(f), } - dev, _, err := lf.finish(ctx, user, resp, requiredExtensions) - return dev, trace.Wrap(err) + return lf.finish(ctx, user, resp, requiredExtensions) } type mfaIdentity struct { diff --git a/lib/auth/webauthn/login_passwordless.go b/lib/auth/webauthn/login_passwordless.go index 09d058389ae90..01dc3455e23b7 100644 --- a/lib/auth/webauthn/login_passwordless.go +++ b/lib/auth/webauthn/login_passwordless.go @@ -65,7 +65,7 @@ func (f *PasswordlessFlow) Begin(ctx context.Context) (*wantypes.CredentialAsser // Finish is the last step of the passwordless login flow. // It works similarly to LoginFlow.Finish, but the user identity is established // via the response UserHandle, instead of an explicit Teleport username. -func (f *PasswordlessFlow) Finish(ctx context.Context, resp *wantypes.CredentialAssertionResponse) (*types.MFADevice, string, error) { +func (f *PasswordlessFlow) Finish(ctx context.Context, resp *wantypes.CredentialAssertionResponse) (*LoginData, error) { lf := &loginFlow{ Webauthn: f.Webauthn, identity: passwordlessIdentity{f.Identity}, diff --git a/lib/auth/webauthn/login_test.go b/lib/auth/webauthn/login_test.go index c0d24f84a3ebe..7e12d2b9d9705 100644 --- a/lib/auth/webauthn/login_test.go +++ b/lib/auth/webauthn/login_test.go @@ -149,17 +149,17 @@ func TestLoginFlow_BeginFinish(t *testing.T) { // 2nd and last step of the login ceremony. beforeLastUsed := time.Now().Add(-1 * time.Second) - loginDevice, err := webLogin.Finish(ctx, user, assertionResp, mfav1.ChallengeExtensions{ + loginData, err := webLogin.Finish(ctx, user, assertionResp, mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.NoError(t, err) // Last used time and counter are updated. - require.True(t, beforeLastUsed.Before(loginDevice.LastUsed)) - require.Equal(t, wantCounter, getSignatureCounter(loginDevice)) + require.True(t, beforeLastUsed.Before(loginData.Device.LastUsed)) + require.Equal(t, wantCounter, getSignatureCounter(loginData.Device)) // Did we update the device in storage? require.NotEmpty(t, identity.UpdatedDevices) got := identity.UpdatedDevices[len(identity.UpdatedDevices)-1] - if diff := cmp.Diff(loginDevice, got); diff != "" { + if diff := cmp.Diff(loginData, got); diff != "" { t.Errorf("Updated device mismatch (-want +got):\n%s", diff) } // Did we delete the challenge? @@ -455,10 +455,10 @@ func TestPasswordlessFlow_BeginAndFinish(t *testing.T) { assertionResp.AssertionResponse.UserHandle = wla.UserID // 2nd and last step of the login ceremony. - mfaDevice, user, err := webLogin.Finish(ctx, assertionResp) + loginData, err := webLogin.Finish(ctx, assertionResp) require.NoError(t, err) - require.NotNil(t, mfaDevice) - require.Equal(t, test.user, user) + require.NotNil(t, loginData.Device) + require.Equal(t, test.user, loginData.User) }) } } @@ -517,7 +517,7 @@ func TestPasswordlessFlow_Finish_errors(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - _, _, err := webLogin.Finish(ctx, test.createResp()) + _, err := webLogin.Finish(ctx, test.createResp()) require.True(t, test.assertErrType(err), "assertErrType failed, err = %v", err) require.Contains(t, err.Error(), test.wantErrMsg) }) @@ -604,11 +604,11 @@ func TestCredentialRPID(t *testing.T) { car, err := dev1Key.SignAssertion(origin, assertion) require.NoError(t, err, "SignAssertion failed") - mfaDev, err := webLogin.Finish(ctx, user, car, mfav1.ChallengeExtensions{ + loginData, err := webLogin.Finish(ctx, user, car, mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.NoError(t, err, "Finish failed") - assert.Equal(t, rpID, mfaDev.GetWebauthn().CredentialRpId, "CredentialRpId mismatch") + assert.Equal(t, rpID, loginData.Device.GetWebauthn().CredentialRpId, "CredentialRpId mismatch") }) t.Run("login doesn't issue challenges for the wrong RPIDs", func(t *testing.T) { From 5c6c1787478f4d3ec05e131894e5125fe30b0555 Mon Sep 17 00:00:00 2001 From: joerger Date: Fri, 12 Jan 2024 18:28:06 -0800 Subject: [PATCH 06/11] Enforce reuse when provided by the caller. --- lib/auth/webauthn/login.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/auth/webauthn/login.go b/lib/auth/webauthn/login.go index 411546e35a01a..8f41f0327e59a 100644 --- a/lib/auth/webauthn/login.go +++ b/lib/auth/webauthn/login.go @@ -288,6 +288,11 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred } } + // If this session is reusable, but this login forbids reusable sessions, return an error. + if requiredExtensions.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO && sd.ChallengeExtensions.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES { + return nil, trace.AccessDenied("the given webauthn session allows reuse, but reuse is not permitted in this context") + } + sessionData := wantypes.SessionDataToProtocol(sd) // Make sure _all_ credentials in the session are accounted for by the user. From 3f1270e917c14aab73fd473b15256b7519f3546d Mon Sep 17 00:00:00 2001 From: joerger Date: Tue, 16 Jan 2024 13:12:54 -0800 Subject: [PATCH 07/11] Address comments. --- lib/auth/auth.go | 4 ++-- lib/auth/webauthn/login.go | 20 ++++++++++---------- lib/auth/webauthn/login_mfa.go | 12 +++++++++--- lib/auth/webauthn/login_passwordless.go | 4 ++-- 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/lib/auth/auth.go b/lib/auth/auth.go index 6ce5593c94497..c05efbcff769f 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -5810,7 +5810,7 @@ func (a *Server) mfaAuthChallenge(ctx context.Context, user string, passwordless // TODO(Joerger): Get extensions from caller. ext := mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, - AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_UNSPECIFIED, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, } assertion, err := webLogin.Begin(ctx, user, ext) if err != nil { @@ -5941,7 +5941,7 @@ func (a *Server) ValidateMFAAuthResponse(ctx context.Context, resp *proto.MFAAut // TODO(Joerger): Get extensions from caller. ext := mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, - AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_UNSPECIFIED, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, } loginData, err = webLogin.Finish(ctx, user, wantypes.CredentialAssertionResponseFromProto(res.Webauthn), ext) } diff --git a/lib/auth/webauthn/login.go b/lib/auth/webauthn/login.go index 8f41f0327e59a..3f562ade47489 100644 --- a/lib/auth/webauthn/login.go +++ b/lib/auth/webauthn/login.go @@ -67,12 +67,12 @@ type loginFlow struct { sessionData sessionIdentity } -func (f *loginFlow) begin(ctx context.Context, user string, ext mfav1.ChallengeExtensions) (*wantypes.CredentialAssertion, error) { - if ext.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES && ext.Scope != mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION { - return nil, trace.BadParameter("mfa challenges with scope %s cannot allow reuse", ext.Scope) +func (f *loginFlow) begin(ctx context.Context, user string, requestedChallengeExt mfav1.ChallengeExtensions) (*wantypes.CredentialAssertion, error) { + if requestedChallengeExt.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES && requestedChallengeExt.Scope != mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION { + return nil, trace.BadParameter("mfa challenges with scope %s cannot allow reuse", requestedChallengeExt.Scope) } - passwordless := ext.Scope == mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN + passwordless := requestedChallengeExt.Scope == mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN if user == "" && !passwordless { return nil, trace.BadParameter("user required") } @@ -172,7 +172,7 @@ func (f *loginFlow) begin(ctx context.Context, user string, ext mfav1.ChallengeE if err != nil { return nil, trace.Wrap(err) } - sd.ChallengeExtensions = &ext + sd.ChallengeExtensions = &requestedChallengeExt if err := f.sessionData.Upsert(ctx, user, sd); err != nil { return nil, trace.Wrap(err) @@ -198,9 +198,9 @@ type LoginData struct { User string // Device is the MFA device used to authenticate the user. Device *types.MFADevice - // Reusable is whether the webauthn challenge used for this login + // AllowReuse is whether the webauthn challenge used for this login // can be reused by the user for subsequent logins. - Reusable bool + AllowReuse mfav1.ChallengeAllowReuse } func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.CredentialAssertionResponse, requiredExtensions mfav1.ChallengeExtensions) (*LoginData, error) { @@ -356,9 +356,9 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred } return &LoginData{ - User: user, - Device: dev, - Reusable: sd.ChallengeExtensions.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + User: user, + Device: dev, + AllowReuse: sd.ChallengeExtensions.AllowReuse, }, nil } diff --git a/lib/auth/webauthn/login_mfa.go b/lib/auth/webauthn/login_mfa.go index ba2a630d9e15c..d3e426e910394 100644 --- a/lib/auth/webauthn/login_mfa.go +++ b/lib/auth/webauthn/login_mfa.go @@ -92,6 +92,9 @@ type LoginFlow struct { // assertion. // As a side effect Begin may assign (and record in storage) a WebAuthn ID for // the user. +// Requested challenge extensions will be stored on the stored webauthn challenge +// record. These extensions indicate additional rules/properties of the webauthn +// challenge that can be validated in the final login step. func (f *LoginFlow) Begin(ctx context.Context, user string, challengeExtensions mfav1.ChallengeExtensions) (*wantypes.CredentialAssertion, error) { lf := &loginFlow{ U2F: f.U2F, @@ -103,9 +106,12 @@ func (f *LoginFlow) Begin(ctx context.Context, user string, challengeExtensions } // Finish is the second and last step of the LoginFlow. -// It returns the MFADevice used to solve the challenge. If login is successful, -// Finish has the side effect of updating the counter and last used timestamp of -// the returned device. +// Expected challenge extensions will be validated against the stored webauthn +// challenge record. +// It returns the MFADevice used to solve the challenge, the associated Teleport +// user name, and other login properties. If login is successful, Finish has the +// side effect of updating the counter and last used timestamp of the MFADevice +// used. func (f *LoginFlow) Finish(ctx context.Context, user string, resp *wantypes.CredentialAssertionResponse, requiredExtensions mfav1.ChallengeExtensions) (*LoginData, error) { lf := &loginFlow{ U2F: f.U2F, diff --git a/lib/auth/webauthn/login_passwordless.go b/lib/auth/webauthn/login_passwordless.go index 01dc3455e23b7..70adc77c7f54a 100644 --- a/lib/auth/webauthn/login_passwordless.go +++ b/lib/auth/webauthn/login_passwordless.go @@ -55,11 +55,11 @@ func (f *PasswordlessFlow) Begin(ctx context.Context) (*wantypes.CredentialAsser identity: passwordlessIdentity{f.Identity}, sessionData: (*globalSessionStorage)(f), } - ext := mfav1.ChallengeExtensions{ + chalExt := mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN, AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, } - return lf.begin(ctx, "" /* user */, ext) + return lf.begin(ctx, "" /* user */, chalExt) } // Finish is the last step of the passwordless login flow. From 1e0e77a66e1cc479209852a27b201f2230a5f7ae Mon Sep 17 00:00:00 2001 From: joerger Date: Tue, 16 Jan 2024 14:01:39 -0800 Subject: [PATCH 08/11] Fix test. --- lib/auth/webauthn/login_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/auth/webauthn/login_test.go b/lib/auth/webauthn/login_test.go index 7e12d2b9d9705..9650b4a00a09b 100644 --- a/lib/auth/webauthn/login_test.go +++ b/lib/auth/webauthn/login_test.go @@ -159,7 +159,7 @@ func TestLoginFlow_BeginFinish(t *testing.T) { // Did we update the device in storage? require.NotEmpty(t, identity.UpdatedDevices) got := identity.UpdatedDevices[len(identity.UpdatedDevices)-1] - if diff := cmp.Diff(loginData, got); diff != "" { + if diff := cmp.Diff(loginData.Device, got); diff != "" { t.Errorf("Updated device mismatch (-want +got):\n%s", diff) } // Did we delete the challenge? @@ -440,6 +440,10 @@ func TestPasswordlessFlow_BeginAndFinish(t *testing.T) { AllowCredentials: [][]uint8{}, // aka unset ResidentKey: false, // irrelevant for login UserVerification: string(protocol.VerificationRequired), + ChallengeExtensions: mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, + }, } if diff := cmp.Diff(wantSD, sd); diff != "" { t.Fatalf("SessionData mismatch (-want +got):\n%s", diff) From 1ecb6617ddcf813736add4607bb9dba9c08b5b55 Mon Sep 17 00:00:00 2001 From: joerger Date: Tue, 16 Jan 2024 19:29:52 -0800 Subject: [PATCH 09/11] Add unit test for scope and reuse. --- lib/auth/webauthn/login_test.go | 156 ++++++++++++++++++++++++++++++++ 1 file changed, 156 insertions(+) diff --git a/lib/auth/webauthn/login_test.go b/lib/auth/webauthn/login_test.go index 9650b4a00a09b..6dd6a4baba526 100644 --- a/lib/auth/webauthn/login_test.go +++ b/lib/auth/webauthn/login_test.go @@ -654,6 +654,162 @@ func TestCredentialRPID(t *testing.T) { }) } +func TestLogin_scopeAndReuse(t *testing.T) { + // webUser gets a newly registered device and a webID. + const webUser = "llama" + webIdentity := newFakeIdentity(webUser) + webConfig := &types.Webauthn{RPID: "example.com"} + + const webOrigin = "https://example.com" + ctx := context.Background() + + // Register a Webauthn device. + // Last registration step creates the user webID and adds the new device to + // identity. + webKey, err := mocku2f.Create() + require.NoError(t, err) + webKey.PreferRPID = true // Webauthn-registered device + webRegistration := &wanlib.RegistrationFlow{ + Webauthn: webConfig, + Identity: webIdentity, + } + cc, err := webRegistration.Begin(ctx, webUser, false /* passwordless */) + require.NoError(t, err) + ccr, err := webKey.SignCredentialCreation(webOrigin, cc) + require.NoError(t, err) + device, err := webRegistration.Finish(ctx, wanlib.RegisterResponse{ + User: webUser, + DeviceName: "webauthn1", + CreationResponse: ccr, + }) + require.NoError(t, err) + + tests := []struct { + name string + challengeExt mfav1.ChallengeExtensions + wantBeginErr error + requiredExt mfav1.ChallengeExtensions + wantFinishErr error + }{ + { + name: "NOK scope not satisfied", + challengeExt: mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + }, + requiredExt: mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }, + wantFinishErr: trace.AccessDenied("required scope %q is not satisfied by the given webauthn session with scope %q", mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION), + }, { + // Old clients do not yet provide a scope, so we only enforce scope + // opportunistically during login finish. + // TODO(Joerger): DELETE IN v16.0.0 - change to NOK + name: "OK scope not specified", + challengeExt: mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, + }, + requiredExt: mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + }, + }, { + name: "OK scope not required", + challengeExt: mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + }, + requiredExt: mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, + }, + }, { + name: "OK required scope satisfied", + challengeExt: mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + }, + requiredExt: mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + }, + }, { + name: "NOK reuse not allowed for scope", + challengeExt: mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + }, + wantBeginErr: trace.BadParameter("mfa challenges with scope %s cannot allow reuse", mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN), + }, { + name: "NOK reuse requested but not allowed", + challengeExt: mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + }, + requiredExt: mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, + }, + wantFinishErr: trace.AccessDenied("the given webauthn session allows reuse, but reuse is not permitted in this context"), + }, { + name: "OK reuse not requested but allowed", + challengeExt: mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, + }, + requiredExt: mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + }, + }, { + name: "OK reuse requested and allowed", + challengeExt: mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + }, + requiredExt: mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + identity := webIdentity + user := webUser + + webLogin := &wanlib.LoginFlow{ + Webauthn: webConfig, + Identity: webIdentity, + } + + assertion, err := webLogin.Begin(ctx, user, test.challengeExt) + if test.wantBeginErr != nil { + require.ErrorIs(t, err, test.wantBeginErr) + return + } + require.NoError(t, err) + + assertionResp, err := webKey.SignAssertion(webOrigin, assertion) + require.NoError(t, err) + + loginData, err := webLogin.Finish(ctx, user, assertionResp, test.requiredExt) + if test.wantFinishErr != nil { + require.ErrorIs(t, err, test.wantFinishErr) + return + } + + require.NoError(t, err) + require.Equal(t, loginData, &wanlib.LoginData{ + Device: device, + User: user, + AllowReuse: loginData.AllowReuse, + }) + + // Session data should only be deleted if reuse was not requested on begin. + if test.challengeExt.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES { + require.NotEmpty(t, identity.SessionData) + } else { + require.Empty(t, identity.SessionData) + } + }) + } +} + type fakeIdentity struct { User *types.UserV2 // MappedUser is used as the reply to GetTeleportUserByWebauthnID. From 6223c8f9cfd1033af7a92164dd6020748441e305 Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 17 Jan 2024 10:41:23 -0800 Subject: [PATCH 10/11] use pointer for challenge extension parameters. --- lib/auth/auth.go | 4 +- lib/auth/webauthn/login.go | 20 +++++-- lib/auth/webauthn/login_mfa.go | 4 +- lib/auth/webauthn/login_passwordless.go | 4 +- lib/auth/webauthn/login_test.go | 75 ++++++++++++++----------- lib/auth/webauthncli/u2f_login_test.go | 14 ++--- 6 files changed, 70 insertions(+), 51 deletions(-) diff --git a/lib/auth/auth.go b/lib/auth/auth.go index c05efbcff769f..1dbbe9678e5a7 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -5808,7 +5808,7 @@ func (a *Server) mfaAuthChallenge(ctx context.Context, user string, passwordless Identity: wanlib.WithDevices(a.Services, groupedDevs.Webauthn), } // TODO(Joerger): Get extensions from caller. - ext := mfav1.ChallengeExtensions{ + ext := &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, } @@ -5939,7 +5939,7 @@ func (a *Server) ValidateMFAAuthResponse(ctx context.Context, resp *proto.MFAAut Identity: a.Services, } // TODO(Joerger): Get extensions from caller. - ext := mfav1.ChallengeExtensions{ + ext := &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, } diff --git a/lib/auth/webauthn/login.go b/lib/auth/webauthn/login.go index 3f562ade47489..8cc4a6bb952f6 100644 --- a/lib/auth/webauthn/login.go +++ b/lib/auth/webauthn/login.go @@ -67,12 +67,16 @@ type loginFlow struct { sessionData sessionIdentity } -func (f *loginFlow) begin(ctx context.Context, user string, requestedChallengeExt mfav1.ChallengeExtensions) (*wantypes.CredentialAssertion, error) { - if requestedChallengeExt.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES && requestedChallengeExt.Scope != mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION { - return nil, trace.BadParameter("mfa challenges with scope %s cannot allow reuse", requestedChallengeExt.Scope) +func (f *loginFlow) begin(ctx context.Context, user string, challengeExtensions *mfav1.ChallengeExtensions) (*wantypes.CredentialAssertion, error) { + if challengeExtensions == nil { + return nil, trace.BadParameter("requested challenge extensions must be supplied.") } - passwordless := requestedChallengeExt.Scope == mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN + if challengeExtensions.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES && challengeExtensions.Scope != mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION { + return nil, trace.BadParameter("mfa challenges with scope %s cannot allow reuse", challengeExtensions.Scope) + } + + passwordless := challengeExtensions.Scope == mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN if user == "" && !passwordless { return nil, trace.BadParameter("user required") } @@ -172,7 +176,7 @@ func (f *loginFlow) begin(ctx context.Context, user string, requestedChallengeEx if err != nil { return nil, trace.Wrap(err) } - sd.ChallengeExtensions = &requestedChallengeExt + sd.ChallengeExtensions = challengeExtensions if err := f.sessionData.Upsert(ctx, user, sd); err != nil { return nil, trace.Wrap(err) @@ -203,7 +207,11 @@ type LoginData struct { AllowReuse mfav1.ChallengeAllowReuse } -func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.CredentialAssertionResponse, requiredExtensions mfav1.ChallengeExtensions) (*LoginData, error) { +func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.CredentialAssertionResponse, requiredExtensions *mfav1.ChallengeExtensions) (*LoginData, error) { + if requiredExtensions == nil { + return nil, trace.BadParameter("requested challenge extensions must be supplied.") + } + passwordless := requiredExtensions.Scope == mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN switch { diff --git a/lib/auth/webauthn/login_mfa.go b/lib/auth/webauthn/login_mfa.go index d3e426e910394..fc5312f259e3a 100644 --- a/lib/auth/webauthn/login_mfa.go +++ b/lib/auth/webauthn/login_mfa.go @@ -95,7 +95,7 @@ type LoginFlow struct { // Requested challenge extensions will be stored on the stored webauthn challenge // record. These extensions indicate additional rules/properties of the webauthn // challenge that can be validated in the final login step. -func (f *LoginFlow) Begin(ctx context.Context, user string, challengeExtensions mfav1.ChallengeExtensions) (*wantypes.CredentialAssertion, error) { +func (f *LoginFlow) Begin(ctx context.Context, user string, challengeExtensions *mfav1.ChallengeExtensions) (*wantypes.CredentialAssertion, error) { lf := &loginFlow{ U2F: f.U2F, Webauthn: f.Webauthn, @@ -112,7 +112,7 @@ func (f *LoginFlow) Begin(ctx context.Context, user string, challengeExtensions // user name, and other login properties. If login is successful, Finish has the // side effect of updating the counter and last used timestamp of the MFADevice // used. -func (f *LoginFlow) Finish(ctx context.Context, user string, resp *wantypes.CredentialAssertionResponse, requiredExtensions mfav1.ChallengeExtensions) (*LoginData, error) { +func (f *LoginFlow) Finish(ctx context.Context, user string, resp *wantypes.CredentialAssertionResponse, requiredExtensions *mfav1.ChallengeExtensions) (*LoginData, error) { lf := &loginFlow{ U2F: f.U2F, Webauthn: f.Webauthn, diff --git a/lib/auth/webauthn/login_passwordless.go b/lib/auth/webauthn/login_passwordless.go index 70adc77c7f54a..c79bbf638c8d0 100644 --- a/lib/auth/webauthn/login_passwordless.go +++ b/lib/auth/webauthn/login_passwordless.go @@ -55,7 +55,7 @@ func (f *PasswordlessFlow) Begin(ctx context.Context) (*wantypes.CredentialAsser identity: passwordlessIdentity{f.Identity}, sessionData: (*globalSessionStorage)(f), } - chalExt := mfav1.ChallengeExtensions{ + chalExt := &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN, AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, } @@ -71,7 +71,7 @@ func (f *PasswordlessFlow) Finish(ctx context.Context, resp *wantypes.Credential identity: passwordlessIdentity{f.Identity}, sessionData: (*globalSessionStorage)(f), } - requiredExt := mfav1.ChallengeExtensions{ + requiredExt := &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN, AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, } diff --git a/lib/auth/webauthn/login_test.go b/lib/auth/webauthn/login_test.go index 6dd6a4baba526..9de4b129a947c 100644 --- a/lib/auth/webauthn/login_test.go +++ b/lib/auth/webauthn/login_test.go @@ -119,7 +119,7 @@ func TestLoginFlow_BeginFinish(t *testing.T) { } // 1st step of the login ceremony. - assertion, err := webLogin.Begin(ctx, user, mfav1.ChallengeExtensions{ + assertion, err := webLogin.Begin(ctx, user, &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.NoError(t, err) @@ -149,7 +149,7 @@ func TestLoginFlow_BeginFinish(t *testing.T) { // 2nd and last step of the login ceremony. beforeLastUsed := time.Now().Add(-1 * time.Second) - loginData, err := webLogin.Finish(ctx, user, assertionResp, mfav1.ChallengeExtensions{ + loginData, err := webLogin.Finish(ctx, user, assertionResp, &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.NoError(t, err) @@ -225,7 +225,7 @@ func TestLoginFlow_Begin_errors(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - _, err := webLogin.Begin(ctx, test.user, mfav1.ChallengeExtensions{ + _, err := webLogin.Begin(ctx, test.user, &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.True(t, test.assertErrType(err), "got err = %v, want BadParameter", err) @@ -265,7 +265,7 @@ func TestLoginFlow_Finish_errors(t *testing.T) { Webauthn: webConfig, Identity: identity, } - assertion, err := webLogin.Begin(ctx, user, mfav1.ChallengeExtensions{ + assertion, err := webLogin.Begin(ctx, user, &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.NoError(t, err) @@ -296,7 +296,7 @@ func TestLoginFlow_Finish_errors(t *testing.T) { name: "NOK assertion with bad origin", user: user, createResp: func() *wantypes.CredentialAssertionResponse { - assertion, err := webLogin.Begin(ctx, user, mfav1.ChallengeExtensions{ + assertion, err := webLogin.Begin(ctx, user, &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.NoError(t, err) @@ -309,7 +309,7 @@ func TestLoginFlow_Finish_errors(t *testing.T) { name: "NOK assertion with bad RPID", user: user, createResp: func() *wantypes.CredentialAssertionResponse { - assertion, err := webLogin.Begin(ctx, user, mfav1.ChallengeExtensions{ + assertion, err := webLogin.Begin(ctx, user, &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.NoError(t, err) @@ -324,7 +324,7 @@ func TestLoginFlow_Finish_errors(t *testing.T) { name: "NOK assertion signed by unknown device", user: user, createResp: func() *wantypes.CredentialAssertionResponse { - assertion, err := webLogin.Begin(ctx, user, mfav1.ChallengeExtensions{ + assertion, err := webLogin.Begin(ctx, user, &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.NoError(t, err) @@ -343,7 +343,7 @@ func TestLoginFlow_Finish_errors(t *testing.T) { name: "NOK assertion with invalid signature", user: user, createResp: func() *wantypes.CredentialAssertionResponse { - assertion, err := webLogin.Begin(ctx, user, mfav1.ChallengeExtensions{ + assertion, err := webLogin.Begin(ctx, user, &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.NoError(t, err) @@ -359,7 +359,7 @@ func TestLoginFlow_Finish_errors(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - _, err := webLogin.Finish(ctx, test.user, test.createResp(), mfav1.ChallengeExtensions{ + _, err := webLogin.Finish(ctx, test.user, test.createResp(), &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.Error(t, err) @@ -440,7 +440,7 @@ func TestPasswordlessFlow_BeginAndFinish(t *testing.T) { AllowCredentials: [][]uint8{}, // aka unset ResidentKey: false, // irrelevant for login UserVerification: string(protocol.VerificationRequired), - ChallengeExtensions: mfav1.ChallengeExtensions{ + ChallengeExtensions: &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN, AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, }, @@ -588,7 +588,7 @@ func TestCredentialRPID(t *testing.T) { Identity: identity, } - _, err := webLogin.Begin(ctx, user, mfav1.ChallengeExtensions{ + _, err := webLogin.Begin(ctx, user, &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) assert.NoError(t, err, "Begin failed, expected assertion for `dev1`") @@ -600,7 +600,7 @@ func TestCredentialRPID(t *testing.T) { Identity: identity, } - assertion, err := webLogin.Begin(ctx, user, mfav1.ChallengeExtensions{ + assertion, err := webLogin.Begin(ctx, user, &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.NoError(t, err, "Begin failed") @@ -608,7 +608,7 @@ func TestCredentialRPID(t *testing.T) { car, err := dev1Key.SignAssertion(origin, assertion) require.NoError(t, err, "SignAssertion failed") - loginData, err := webLogin.Finish(ctx, user, car, mfav1.ChallengeExtensions{ + loginData, err := webLogin.Finish(ctx, user, car, &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.NoError(t, err, "Finish failed") @@ -621,7 +621,7 @@ func TestCredentialRPID(t *testing.T) { Identity: identity, } - _, err := webLogin.Begin(ctx, user, mfav1.ChallengeExtensions{ + _, err := webLogin.Begin(ctx, user, &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) assert.ErrorIs(t, err, wanlib.ErrInvalidCredentials, "Begin error mismatch") @@ -640,7 +640,7 @@ func TestCredentialRPID(t *testing.T) { Webauthn: webOtherRP, Identity: identity, } - assertion, err := webLogin.Begin(ctx, user, mfav1.ChallengeExtensions{ + assertion, err := webLogin.Begin(ctx, user, &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.NoError(t, err, "Begin failed, expected assertion for device `other1`") @@ -686,17 +686,28 @@ func TestLogin_scopeAndReuse(t *testing.T) { tests := []struct { name string - challengeExt mfav1.ChallengeExtensions + challengeExt *mfav1.ChallengeExtensions wantBeginErr error - requiredExt mfav1.ChallengeExtensions + requiredExt *mfav1.ChallengeExtensions wantFinishErr error }{ { + name: "NOK challenge extensions not provided", + challengeExt: nil, + wantBeginErr: trace.BadParameter("requested challenge extensions must be supplied."), + }, { + name: "NOK required challenge extensions not provided", + challengeExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + }, + requiredExt: nil, + wantFinishErr: trace.BadParameter("requested challenge extensions must be supplied."), + }, { name: "NOK scope not satisfied", - challengeExt: mfav1.ChallengeExtensions{ + challengeExt: &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, }, - requiredExt: mfav1.ChallengeExtensions{ + requiredExt: &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }, wantFinishErr: trace.AccessDenied("required scope %q is not satisfied by the given webauthn session with scope %q", mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION), @@ -705,63 +716,63 @@ func TestLogin_scopeAndReuse(t *testing.T) { // opportunistically during login finish. // TODO(Joerger): DELETE IN v16.0.0 - change to NOK name: "OK scope not specified", - challengeExt: mfav1.ChallengeExtensions{ + challengeExt: &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, }, - requiredExt: mfav1.ChallengeExtensions{ + requiredExt: &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, }, }, { name: "OK scope not required", - challengeExt: mfav1.ChallengeExtensions{ + challengeExt: &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, }, - requiredExt: mfav1.ChallengeExtensions{ + requiredExt: &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, }, }, { name: "OK required scope satisfied", - challengeExt: mfav1.ChallengeExtensions{ + challengeExt: &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, }, - requiredExt: mfav1.ChallengeExtensions{ + requiredExt: &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, }, }, { name: "NOK reuse not allowed for scope", - challengeExt: mfav1.ChallengeExtensions{ + challengeExt: &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, }, wantBeginErr: trace.BadParameter("mfa challenges with scope %s cannot allow reuse", mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN), }, { name: "NOK reuse requested but not allowed", - challengeExt: mfav1.ChallengeExtensions{ + challengeExt: &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, }, - requiredExt: mfav1.ChallengeExtensions{ + requiredExt: &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, }, wantFinishErr: trace.AccessDenied("the given webauthn session allows reuse, but reuse is not permitted in this context"), }, { name: "OK reuse not requested but allowed", - challengeExt: mfav1.ChallengeExtensions{ + challengeExt: &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, }, - requiredExt: mfav1.ChallengeExtensions{ + requiredExt: &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, }, }, { name: "OK reuse requested and allowed", - challengeExt: mfav1.ChallengeExtensions{ + challengeExt: &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, }, - requiredExt: mfav1.ChallengeExtensions{ + requiredExt: &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, }, diff --git a/lib/auth/webauthncli/u2f_login_test.go b/lib/auth/webauthncli/u2f_login_test.go index 3ad94ff34a410..082979d0da0f9 100644 --- a/lib/auth/webauthncli/u2f_login_test.go +++ b/lib/auth/webauthncli/u2f_login_test.go @@ -141,7 +141,7 @@ func TestLogin(t *testing.T) { } test.setUserPresence.SetUserPresence(true) - assertion, err := loginFlow.Begin(ctx, username, mfav1.ChallengeExtensions{ + assertion, err := loginFlow.Begin(ctx, username, &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.NoError(t, err) @@ -162,7 +162,7 @@ func TestLogin(t *testing.T) { require.NotNil(t, mfaResp.GetWebauthn()) require.Equal(t, test.wantRawID, mfaResp.GetWebauthn().RawId) - _, err = loginFlow.Finish(ctx, username, wantypes.CredentialAssertionResponseFromProto(mfaResp.GetWebauthn()), mfav1.ChallengeExtensions{ + _, err = loginFlow.Finish(ctx, username, wantypes.CredentialAssertionResponseFromProto(mfaResp.GetWebauthn()), &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.NoError(t, err) @@ -187,7 +187,7 @@ func TestLogin_errors(t *testing.T) { const user = "llama" const origin = "https://localhost" ctx := context.Background() - okAssertion, err := loginFlow.Begin(ctx, user, mfav1.ChallengeExtensions{ + okAssertion, err := loginFlow.Begin(ctx, user, &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.NoError(t, err) @@ -222,7 +222,7 @@ func TestLogin_errors(t *testing.T) { name: "NOK assertion missing challenge", origin: origin, getAssertion: func() *wantypes.CredentialAssertion { - assertion, err := loginFlow.Begin(ctx, user, mfav1.ChallengeExtensions{ + assertion, err := loginFlow.Begin(ctx, user, &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.NoError(t, err) @@ -234,7 +234,7 @@ func TestLogin_errors(t *testing.T) { name: "NOK assertion missing RPID", origin: origin, getAssertion: func() *wantypes.CredentialAssertion { - assertion, err := loginFlow.Begin(ctx, user, mfav1.ChallengeExtensions{ + assertion, err := loginFlow.Begin(ctx, user, &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.NoError(t, err) @@ -246,7 +246,7 @@ func TestLogin_errors(t *testing.T) { name: "NOK assertion missing credentials", origin: origin, getAssertion: func() *wantypes.CredentialAssertion { - assertion, err := loginFlow.Begin(ctx, user, mfav1.ChallengeExtensions{ + assertion, err := loginFlow.Begin(ctx, user, &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.NoError(t, err) @@ -258,7 +258,7 @@ func TestLogin_errors(t *testing.T) { name: "NOK assertion invalid user verification requirement", origin: origin, getAssertion: func() *wantypes.CredentialAssertion { - assertion, err := loginFlow.Begin(ctx, user, mfav1.ChallengeExtensions{ + assertion, err := loginFlow.Begin(ctx, user, &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }) require.NoError(t, err) From 160848242fd6b7ac6c776a3b6d77ed66452b610c Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 17 Jan 2024 10:51:46 -0800 Subject: [PATCH 11/11] Address comments. --- lib/auth/webauthn/login.go | 6 +- lib/auth/webauthn/login_test.go | 292 ++++++++++++++++++-------------- 2 files changed, 170 insertions(+), 128 deletions(-) diff --git a/lib/auth/webauthn/login.go b/lib/auth/webauthn/login.go index 8cc4a6bb952f6..7d185b027c957 100644 --- a/lib/auth/webauthn/login.go +++ b/lib/auth/webauthn/login.go @@ -198,12 +198,12 @@ func (f *loginFlow) getWebID(ctx context.Context, user string) ([]byte, error) { // LoginData is data gathered from a successful webauthn login. type LoginData struct { - // User is a Teleport user. + // User is the Teleport user. User string // Device is the MFA device used to authenticate the user. Device *types.MFADevice // AllowReuse is whether the webauthn challenge used for this login - // can be reused by the user for subsequent logins. + // can be reused by the user for subsequent logins, until it expires. AllowReuse mfav1.ChallengeAllowReuse } @@ -357,6 +357,8 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred // The user just solved the challenge, so let's make sure it won't be used // again, unless reuse is explicitly allowed. + // Note that even reusable sessions are deleted when their expiration time + // passes. if sd.ChallengeExtensions.AllowReuse != mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES { if err := f.sessionData.Delete(ctx, user, challenge); err != nil { log.Warnf("WebAuthn: failed to delete login SessionData for user %v (scope = %s)", user, sd.ChallengeExtensions.Scope) diff --git a/lib/auth/webauthn/login_test.go b/lib/auth/webauthn/login_test.go index 9de4b129a947c..6f36b28cf8e05 100644 --- a/lib/auth/webauthn/login_test.go +++ b/lib/auth/webauthn/login_test.go @@ -684,141 +684,181 @@ func TestLogin_scopeAndReuse(t *testing.T) { }) require.NoError(t, err) - tests := []struct { - name string - challengeExt *mfav1.ChallengeExtensions - wantBeginErr error - requiredExt *mfav1.ChallengeExtensions - wantFinishErr error - }{ - { - name: "NOK challenge extensions not provided", - challengeExt: nil, - wantBeginErr: trace.BadParameter("requested challenge extensions must be supplied."), - }, { - name: "NOK required challenge extensions not provided", - challengeExt: &mfav1.ChallengeExtensions{ - Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, - }, - requiredExt: nil, - wantFinishErr: trace.BadParameter("requested challenge extensions must be supplied."), - }, { - name: "NOK scope not satisfied", - challengeExt: &mfav1.ChallengeExtensions{ - Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, - }, - requiredExt: &mfav1.ChallengeExtensions{ - Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, - }, - wantFinishErr: trace.AccessDenied("required scope %q is not satisfied by the given webauthn session with scope %q", mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION), - }, { - // Old clients do not yet provide a scope, so we only enforce scope - // opportunistically during login finish. - // TODO(Joerger): DELETE IN v16.0.0 - change to NOK - name: "OK scope not specified", - challengeExt: &mfav1.ChallengeExtensions{ - Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, - }, - requiredExt: &mfav1.ChallengeExtensions{ - Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, - }, - }, { - name: "OK scope not required", - challengeExt: &mfav1.ChallengeExtensions{ - Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, - }, - requiredExt: &mfav1.ChallengeExtensions{ - Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, - }, - }, { - name: "OK required scope satisfied", - challengeExt: &mfav1.ChallengeExtensions{ - Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, - }, - requiredExt: &mfav1.ChallengeExtensions{ - Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, - }, - }, { - name: "NOK reuse not allowed for scope", - challengeExt: &mfav1.ChallengeExtensions{ - Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, - AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, - }, - wantBeginErr: trace.BadParameter("mfa challenges with scope %s cannot allow reuse", mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN), - }, { - name: "NOK reuse requested but not allowed", - challengeExt: &mfav1.ChallengeExtensions{ - Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, - AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, - }, - requiredExt: &mfav1.ChallengeExtensions{ - Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, - AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, - }, - wantFinishErr: trace.AccessDenied("the given webauthn session allows reuse, but reuse is not permitted in this context"), - }, { - name: "OK reuse not requested but allowed", - challengeExt: &mfav1.ChallengeExtensions{ - Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, - AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, - }, - requiredExt: &mfav1.ChallengeExtensions{ - Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, - AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + t.Run("Begin", func(t *testing.T) { + tests := []struct { + name string + challengeExt *mfav1.ChallengeExtensions + assertErr require.ErrorAssertionFunc + }{ + { + name: "NOK challenge extensions not provided", + challengeExt: nil, + assertErr: func(tt require.TestingT, err error, i ...interface{}) { + require.True(t, trace.IsBadParameter(err), "expected bad parameter err but got %T", err) + require.ErrorContains(t, err, "extensions must be supplied") + }, }, - }, { - name: "OK reuse requested and allowed", - challengeExt: &mfav1.ChallengeExtensions{ - Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, - AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + { + name: "NOK reuse not allowed for scope", + challengeExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + }, + assertErr: func(tt require.TestingT, err error, i ...interface{}) { + require.True(t, trace.IsBadParameter(err), "expected bad parameter err but got %T", err) + require.ErrorContains(t, err, "cannot allow reuse") + }, }, - requiredExt: &mfav1.ChallengeExtensions{ - Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, - AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + user := webUser + + webLogin := &wanlib.LoginFlow{ + Webauthn: webConfig, + Identity: webIdentity, + } + + _, err := webLogin.Begin(ctx, user, test.challengeExt) + if test.assertErr != nil { + test.assertErr(t, err) + return + } + require.NoError(t, err) + }) + } + }) + + t.Run("Finish", func(t *testing.T) { + tests := []struct { + name string + challengeExt *mfav1.ChallengeExtensions + requiredExt *mfav1.ChallengeExtensions + assertErr require.ErrorAssertionFunc + }{ + { + name: "NOK required challenge extensions not provided", + challengeExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + }, + requiredExt: nil, + assertErr: func(tt require.TestingT, err error, i ...interface{}) { + require.True(t, trace.IsBadParameter(err), "expected bad parameter err but got %T", err) + require.ErrorContains(t, err, "extensions must be supplied") + }, + }, { + name: "NOK scope not satisfied", + challengeExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + }, + requiredExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }, + assertErr: func(tt require.TestingT, err error, i ...interface{}) { + require.True(t, trace.IsAccessDenied(err), "expected access denied err but got %T", err) + require.ErrorContains(t, err, "is not satisfied") + }, + }, { + // Old clients do not yet provide a scope, so we only enforce scope + // opportunistically during login finish. + // TODO(Joerger): DELETE IN v16.0.0 - change to NOK + name: "OK scope not specified", + challengeExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, + }, + requiredExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + }, + }, { + name: "OK scope not required", + challengeExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + }, + requiredExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, + }, + }, { + name: "OK required scope satisfied", + challengeExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + }, + requiredExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + }, + }, { + name: "NOK reuse requested but not allowed", + challengeExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + }, + requiredExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, + }, + assertErr: func(tt require.TestingT, err error, i ...interface{}) { + require.True(t, trace.IsAccessDenied(err), "expected access denied err but got %T", err) + require.ErrorContains(t, err, "reuse is not permitted") + }, + }, { + name: "OK reuse not requested but allowed", + challengeExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, + }, + requiredExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + }, + }, { + name: "OK reuse requested and allowed", + challengeExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + }, + requiredExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + }, }, - }, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - identity := webIdentity - user := webUser + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + identity := webIdentity + user := webUser - webLogin := &wanlib.LoginFlow{ - Webauthn: webConfig, - Identity: webIdentity, - } + webLogin := &wanlib.LoginFlow{ + Webauthn: webConfig, + Identity: webIdentity, + } - assertion, err := webLogin.Begin(ctx, user, test.challengeExt) - if test.wantBeginErr != nil { - require.ErrorIs(t, err, test.wantBeginErr) - return - } - require.NoError(t, err) + assertion, err := webLogin.Begin(ctx, user, test.challengeExt) + require.NoError(t, err) - assertionResp, err := webKey.SignAssertion(webOrigin, assertion) - require.NoError(t, err) + assertionResp, err := webKey.SignAssertion(webOrigin, assertion) + require.NoError(t, err) - loginData, err := webLogin.Finish(ctx, user, assertionResp, test.requiredExt) - if test.wantFinishErr != nil { - require.ErrorIs(t, err, test.wantFinishErr) - return - } + loginData, err := webLogin.Finish(ctx, user, assertionResp, test.requiredExt) + if test.assertErr != nil { + test.assertErr(t, err) + return + } - require.NoError(t, err) - require.Equal(t, loginData, &wanlib.LoginData{ - Device: device, - User: user, - AllowReuse: loginData.AllowReuse, - }) + require.NoError(t, err) + require.Equal(t, loginData, &wanlib.LoginData{ + Device: device, + User: user, + AllowReuse: loginData.AllowReuse, + }) - // Session data should only be deleted if reuse was not requested on begin. - if test.challengeExt.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES { - require.NotEmpty(t, identity.SessionData) - } else { - require.Empty(t, identity.SessionData) - } - }) - } + // Session data should only be deleted if reuse was not requested on begin. + if test.challengeExt.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES { + require.NotEmpty(t, identity.SessionData) + } else { + require.Empty(t, identity.SessionData) + } + }) + } + }) } type fakeIdentity struct {