feat: add tests for two step login (#3959)

.github/workflows/ci.yaml

@@ -289,6 +289,7 @@ jobs:
  uses: actions/checkout@v3
  with:
  repository: ory/kratos-selfservice-ui-node
+ ref: jonas-jonas/two-step
  path: node-ui
  - run: |
  cd node-ui

.gitignore

@@ -65,3 +65,4 @@ test/e2e/kratos.*.yml
 # VSCode debug artifact
 __debug_bin
 .debug.sqlite.db
+.last-run.json

selfservice/flow/login/strategy_form_hydrator.go

@@ -41,7 +41,6 @@ var ErrBreakLoginPopulate = errors.New("skip rest of login form population")
 
 type FormHydratorOptions struct {
  IdentityHint *identity.Identity
- Identifier string
 }
 
 type FormHydratorModifier func(o *FormHydratorOptions)
@@ -52,12 +51,6 @@ func WithIdentityHint(i *identity.Identity) FormHydratorModifier {
  }
 }
 
-func WithIdentifier(i string) FormHydratorModifier {
- return func(o *FormHydratorOptions) {
- o.Identifier = i
- }
-}
-
 func NewFormHydratorOptions(modifiers []FormHydratorModifier) *FormHydratorOptions {
  o := new(FormHydratorOptions)
  for _, m := range modifiers {

selfservice/flow/login/strategy_form_hydrator_test.go

@@ -16,9 +16,3 @@ func TestWithIdentityHint(t *testing.T) {
  opts := NewFormHydratorOptions([]FormHydratorModifier{WithIdentityHint(expected)})
  assert.Equal(t, expected, opts.IdentityHint)
 }
-
-func TestWithIdentifier(t *testing.T) {
- expected := "identifier"
- opts := NewFormHydratorOptions([]FormHydratorModifier{WithIdentifier(expected)})
- assert.Equal(t, expected, opts.Identifier)
-}

selfservice/strategy/code/.snapshots/TestFormHydration-method=PopulateLoginMethodIdentifierFirstCredentials-case=WithIdentityHint-case=account_enumeration_mitigation_disabled-case=with_no_identity-case=code_is_used_for_2fa.json

@@ -0,0 +1 @@
+null

selfservice/strategy/code/.snapshots/TestFormHydration-method=PopulateLoginMethodIdentifierFirstCredentials-case=WithIdentityHint-case=account_enumeration_mitigation_disabled-case=with_no_identity-case=code_is_used_for_passwordless_login.json

@@ -0,0 +1 @@
+null

selfservice/strategy/code/strategy_login.go

@@ -32,8 +32,10 @@ import (
  "github.com/ory/x/decoderx"
 )
 
-var _ login.FormHydrator = new(Strategy)
-var _ login.Strategy = new(Strategy)
+var (
+ _ login.FormHydrator = new(Strategy)
+ _ login.Strategy = new(Strategy)
+)
 
 // Update Login flow using the code method
 //
@@ -389,16 +391,21 @@ func (s *Strategy) PopulateLoginMethodSecondFactorRefresh(r *http.Request, f *lo
  return s.PopulateMethod(r, f)
 }
 
-func (s *Strategy) PopulateLoginMethodIdentifierFirstCredentials(r *http.Request, f *login.Flow, _ ...login.FormHydratorModifier) error {
+func (s *Strategy) PopulateLoginMethodIdentifierFirstCredentials(r *http.Request, f *login.Flow, opts ...login.FormHydratorModifier) error {
  if !s.deps.Config().SelfServiceCodeStrategy(r.Context()).PasswordlessEnabled {
  // We only return this if passwordless is disabled, because if it is enabled we can always sign in using this method.
- return idfirst.ErrNoCredentialsFound
+ return errors.WithStack(idfirst.ErrNoCredentialsFound)
+ }
+ o := login.NewFormHydratorOptions(opts)
+
+ // If the identity hint is nil and account enumeration mitigation is disabled, we return an error.
+ if o.IdentityHint == nil && !s.deps.Config().SecurityAccountEnumerationMitigate(r.Context()) {
+ return errors.WithStack(idfirst.ErrNoCredentialsFound)
  }
 
  f.GetUI().Nodes.Append(
  node.NewInputField("method", s.ID(), node.CodeGroup, node.InputAttributeTypeSubmit).WithMetaLabel(text.NewInfoSelfServiceLoginCode()),
  )
-
  return nil
 }
 

selfservice/strategy/code/strategy_login_test.go

@@ -915,28 +915,14 @@ func TestFormHydration(t *testing.T) {
  })
  })
 
- t.Run("case=WithIdentifier", func(t *testing.T) {
- t.Run("case=code is used for 2fa", func(t *testing.T) {
- r, f := newFlow(mfaEnabled, t)
- require.ErrorIs(t, fh.PopulateLoginMethodIdentifierFirstCredentials(r, f, login.WithIdentifier("foo@bar.com")), idfirst.ErrNoCredentialsFound)
- toSnapshot(t, f)
- })
-
- t.Run("case=code is used for passwordless login", func(t *testing.T) {
- r, f := newFlow(passwordlessEnabled, t)
- require.NoError(t, fh.PopulateLoginMethodIdentifierFirstCredentials(r, f, login.WithIdentifier("foo@bar.com")))
- toSnapshot(t, f)
- })
- })
-
  t.Run("case=WithIdentityHint", func(t *testing.T) {
  t.Run("case=account enumeration mitigation enabled", func(t *testing.T) {
  t.Run("case=code is used for 2fa", func(t *testing.T) {
  r, f := newFlow(
  configtesthelpers.WithConfigValue(mfaEnabled, config.ViperKeySecurityAccountEnumerationMitigate, true),
  t,
  )
- require.ErrorIs(t, fh.PopulateLoginMethodIdentifierFirstCredentials(r, f, login.WithIdentifier("foo@bar.com")), idfirst.ErrNoCredentialsFound)
+ require.ErrorIs(t, fh.PopulateLoginMethodIdentifierFirstCredentials(r, f), idfirst.ErrNoCredentialsFound)
  toSnapshot(t, f)
  })
 
@@ -945,12 +931,25 @@ func TestFormHydration(t *testing.T) {
  configtesthelpers.WithConfigValue(passwordlessEnabled, config.ViperKeySecurityAccountEnumerationMitigate, true),
  t,
  )
- require.NoError(t, fh.PopulateLoginMethodIdentifierFirstCredentials(r, f, login.WithIdentifier("foo@bar.com")))
+ require.NoError(t, fh.PopulateLoginMethodIdentifierFirstCredentials(r, f))
  toSnapshot(t, f)
  })
  })
 
  t.Run("case=account enumeration mitigation disabled", func(t *testing.T) {
+ t.Run("case=with no identity", func(t *testing.T) {
+ t.Run("case=code is used for 2fa", func(t *testing.T) {
+ r, f := newFlow(mfaEnabled, t)
+ require.ErrorIs(t, fh.PopulateLoginMethodIdentifierFirstCredentials(r, f), idfirst.ErrNoCredentialsFound)
+ toSnapshot(t, f)
+ })
+
+ t.Run("case=code is used for passwordless login", func(t *testing.T) {
+ r, f := newFlow(passwordlessEnabled, t)
+ require.ErrorIs(t, fh.PopulateLoginMethodIdentifierFirstCredentials(r, f), idfirst.ErrNoCredentialsFound)
+ toSnapshot(t, f)
+ })
+ })
  t.Run("case=identity has code method", func(t *testing.T) {
  identifier := x.NewUUID().String()
  id := createIdentity(ctx, t, reg, false, identifier)

selfservice/strategy/idfirst/strategy_login.go

@@ -21,9 +21,11 @@ import (
  "github.com/ory/x/sqlcon"
 )
 
-var _ login.FormHydrator = new(Strategy)
-var _ login.Strategy = new(Strategy)
-var ErrNoCredentialsFound = errors.New("no credentials found")
+var (
+ _ login.FormHydrator = new(Strategy)
+ _ login.Strategy = new(Strategy)
+ ErrNoCredentialsFound = errors.New("no credentials found")
+)
 
 func (s *Strategy) handleLoginError(w http.ResponseWriter, r *http.Request, f *login.Flow, payload *updateLoginFlowWithIdentifierFirstMethod, err error) error {
  if f != nil {
@@ -85,7 +87,6 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow,
 
  // Add identity hint
  opts = append(opts, login.WithIdentityHint(identityHint))
- opts = append(opts, login.WithIdentifier(p.Identifier))
 
  didPopulate := false
  for _, ls := range s.d.LoginStrategies(r.Context()) {

selfservice/strategy/idfirst/strategy_login_test.go

@@ -53,10 +53,10 @@ func TestCompleteLogin(t *testing.T) {
 
  // We enable the password method to test the identifier first strategy
 
- //ctx = configtesthelpers.WithConfigValue(ctx, config.ViperKeySelfServiceStrategyConfig+"."+string(identity.CredentialsTypePassword), map[string]interface{}{"enabled": true})
+ // ctx = configtesthelpers.WithConfigValue(ctx, config.ViperKeySelfServiceStrategyConfig+"."+string(identity.CredentialsTypePassword), map[string]interface{}{"enabled": true})
  conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+"."+string(identity.CredentialsTypePassword), map[string]interface{}{"enabled": true})
 
- //ctx = configtesthelpers.WithConfigValue(ctx, config.ViperKeySelfServiceLoginFlowStyle, "identifier_first")
+ // ctx = configtesthelpers.WithConfigValue(ctx, config.ViperKeySelfServiceLoginFlowStyle, "identifier_first")
  conf.MustSet(ctx, config.ViperKeySelfServiceLoginFlowStyle, "identifier_first")
 
  router := x.NewRouterPublic()
@@ -67,16 +67,16 @@ func TestCompleteLogin(t *testing.T) {
  redirTS := testhelpers.NewRedirSessionEchoTS(t, reg)
 
  // Overwrite these two:
- //ctx = configtesthelpers.WithConfigValue(ctx, config.ViperKeySelfServiceErrorUI, errTS.URL+"/error-ts")
+ // ctx = configtesthelpers.WithConfigValue(ctx, config.ViperKeySelfServiceErrorUI, errTS.URL+"/error-ts")
  conf.MustSet(ctx, config.ViperKeySelfServiceErrorUI, errTS.URL+"/error-ts")
 
- //ctx = configtesthelpers.WithConfigValue(ctx, config.ViperKeySelfServiceLoginUI, uiTS.URL+"/login-ts")
+ // ctx = configtesthelpers.WithConfigValue(ctx, config.ViperKeySelfServiceLoginUI, uiTS.URL+"/login-ts")
  conf.MustSet(ctx, config.ViperKeySelfServiceLoginUI, uiTS.URL+"/login-ts")
 
- //ctx = testhelpers.WithDefaultIdentitySchemaFromRaw(ctx, loginSchema)
+ // ctx = testhelpers.WithDefaultIdentitySchemaFromRaw(ctx, loginSchema)
  testhelpers.SetDefaultIdentitySchemaFromRaw(conf, loginSchema)
 
- //ctx = configtesthelpers.WithConfigValue(ctx, config.ViperKeySecretsDefault, []string{"not-a-secure-session-key"})
+ // ctx = configtesthelpers.WithConfigValue(ctx, config.ViperKeySecretsDefault, []string{"not-a-secure-session-key"})
  conf.MustSet(ctx, config.ViperKeySecretsDefault, []string{"not-a-secure-session-key"})
 
  //ensureFieldsExist := func(t *testing.T, body []byte) {
@@ -549,12 +549,6 @@ func TestFormHydration(t *testing.T) {
  toSnapshot(t, f)
  })
 
- t.Run("case=WithIdentifier", func(t *testing.T) {
- r, f := newFlow(ctx, t)
- require.ErrorIs(t, fh.PopulateLoginMethodIdentifierFirstCredentials(r, f, login.WithIdentifier("foo@bar.com")), idfirst.ErrNoCredentialsFound)
- toSnapshot(t, f)
- })
-
  t.Run("case=WithIdentityHint", func(t *testing.T) {
  t.Run("case=account enumeration mitigation enabled", func(t *testing.T) {
  ctx := configtesthelpers.WithConfigValue(ctx, config.ViperKeySecurityAccountEnumerationMitigate, true)

selfservice/strategy/oidc/strategy_login_test.go

@@ -51,7 +51,6 @@ func TestFormHydration(t *testing.T) {
  map[string]interface{}{
  "providers": []map[string]interface{}{
  {
-
  "provider": "generic",
  "id": providerID,
  "client_id": "invalid",
@@ -121,12 +120,6 @@ func TestFormHydration(t *testing.T) {
  toSnapshot(t, f)
  })
 
- t.Run("case=WithIdentifier", func(t *testing.T) {
- r, f := newFlow(ctx, t)
- require.ErrorIs(t, fh.PopulateLoginMethodIdentifierFirstCredentials(r, f, login.WithIdentifier("foo@bar.com")), idfirst.ErrNoCredentialsFound)
- toSnapshot(t, f)
- })
-
  t.Run("case=WithIdentityHint", func(t *testing.T) {
  t.Run("case=account enumeration mitigation enabled", func(t *testing.T) {
  ctx := configtesthelpers.WithConfigValue(ctx, config.ViperKeySecurityAccountEnumerationMitigate, true)

selfservice/strategy/passkey/passkey_login_test.go

@@ -415,22 +415,6 @@ func TestFormHydration(t *testing.T) {
  })
  })
 
- t.Run("case=WithIdentifier", func(t *testing.T) {
- t.Run("case=account enumeration mitigation disabled", func(t *testing.T) {
- ctx := configtesthelpers.WithConfigValue(ctx, config.ViperKeySecurityAccountEnumerationMitigate, false)
- r, f := newFlow(ctx, t)
- require.ErrorIs(t, fh.PopulateLoginMethodIdentifierFirstCredentials(r, f, login.WithIdentifier("foo@bar.com")), idfirst.ErrNoCredentialsFound)
- toSnapshot(t, f)
- })
-
- t.Run("case=account enumeration mitigation enabled", func(t *testing.T) {
- ctx := configtesthelpers.WithConfigValue(ctx, config.ViperKeySecurityAccountEnumerationMitigate, true)
- r, f := newFlow(ctx, t)
- require.ErrorIs(t, fh.PopulateLoginMethodIdentifierFirstCredentials(r, f, login.WithIdentifier("foo@bar.com")), idfirst.ErrNoCredentialsFound)
- toSnapshot(t, f)
- })
- })
-
  t.Run("case=WithIdentityHint", func(t *testing.T) {
  t.Run("case=account enumeration mitigation enabled", func(t *testing.T) {
  ctx := configtesthelpers.WithConfigValue(ctx, config.ViperKeySecurityAccountEnumerationMitigate, true)

selfservice/strategy/password/login.go

@@ -10,31 +10,31 @@ import (
  "net/http"
  "time"
 
- "github.com/ory/kratos/hash"
- "github.com/ory/kratos/selfservice/flowhelpers"
- "github.com/ory/kratos/selfservice/hook"
- "github.com/ory/kratos/selfservice/strategy/idfirst"
- "github.com/ory/kratos/session"
- "github.com/ory/x/stringsx"
  "github.com/gofrs/uuid"
- "github.com/pkg/errors"
  "github.com/ory/herodot"
- "github.com/ory/x/decoderx"
+ "github.com/ory/kratos/hash"
  "github.com/ory/kratos/identity"
  "github.com/ory/kratos/schema"
  "github.com/ory/kratos/selfservice/flow"
  "github.com/ory/kratos/selfservice/flow/login"
+ "github.com/ory/kratos/selfservice/flowhelpers"
+ "github.com/ory/kratos/selfservice/hook"
+ "github.com/ory/kratos/selfservice/strategy/idfirst"
+ "github.com/ory/kratos/session"
  "github.com/ory/kratos/text"
  "github.com/ory/kratos/ui/node"
  "github.com/ory/kratos/x"
+ "github.com/ory/x/decoderx"
+ "github.com/ory/x/stringsx"
+ "github.com/pkg/errors"
 )
 
 var _ login.FormHydrator = new(Strategy)
 
 func (s *Strategy) RegisterLoginRoutes(r *x.RouterPublic) {
 }
 
-func (s *Strategy) handleLoginError(w http.ResponseWriter, r *http.Request, f *login.Flow, payload *updateLoginFlowWithPasswordMethod, err error) error {
+func (s *Strategy) handleLoginError(r *http.Request, f *login.Flow, payload *updateLoginFlowWithPasswordMethod, err error) error {
  if f != nil {
  f.UI.Nodes.ResetNodes("password")
  f.UI.Nodes.SetValueAttribute("identifier", stringsx.Coalesce(payload.Identifier, payload.LegacyIdentifier))
@@ -60,19 +60,19 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow,
  decoderx.HTTPDecoderSetValidatePayloads(true),
  decoderx.MustHTTPRawJSONSchemaCompiler(loginSchema),
  decoderx.HTTPDecoderJSONFollowsFormFormat()); err != nil {
- return nil, s.handleLoginError(w, r, f, &p, err)
+ return nil, s.handleLoginError(r, f, &p, err)
  }
  f.TransientPayload = p.TransientPayload
 
  if err := flow.EnsureCSRF(s.d, r, f.Type, s.d.Config().DisableAPIFlowEnforcement(r.Context()), s.d.GenerateCSRFToken, p.CSRFToken); err != nil {
- return nil, s.handleLoginError(w, r, f, &p, err)
+ return nil, s.handleLoginError(r, f, &p, err)
  }
 
  identifier := stringsx.Coalesce(p.Identifier, p.LegacyIdentifier)
  i, c, err := s.d.PrivilegedIdentityPool().FindByCredentialsIdentifier(r.Context(), s.ID(), identifier)
  if err != nil {
  time.Sleep(x.RandomDelay(s.d.Config().HasherArgon2(r.Context()).ExpectedDuration, s.d.Config().HasherArgon2(r.Context()).ExpectedDeviation))
- return nil, s.handleLoginError(w, r, f, &p, errors.WithStack(schema.NewInvalidCredentialsError()))
+ return nil, s.handleLoginError(r, f, &p, errors.WithStack(schema.NewInvalidCredentialsError()))
  }
 
  var o identity.CredentialsPassword
@@ -90,27 +90,27 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow,
  migrationHook := hook.NewPasswordMigrationHook(s.d, pwHook.Config)
  err = migrationHook.Execute(r.Context(), &hook.PasswordMigrationRequest{Identifier: identifier, Password: p.Password})
  if err != nil {
- return nil, s.handleLoginError(w, r, f, &p, err)
+ return nil, s.handleLoginError(r, f, &p, err)
  }
 
  if err := s.migratePasswordHash(r.Context(), i.ID, []byte(p.Password)); err != nil {
- return nil, s.handleLoginError(w, r, f, &p, err)
+ return nil, s.handleLoginError(r, f, &p, err)
  }
  } else {
  if err := hash.Compare(r.Context(), []byte(p.Password), []byte(o.HashedPassword)); err != nil {
- return nil, s.handleLoginError(w, r, f, &p, errors.WithStack(schema.NewInvalidCredentialsError()))
+ return nil, s.handleLoginError(r, f, &p, errors.WithStack(schema.NewInvalidCredentialsError()))
  }
 
  if !s.d.Hasher(r.Context()).Understands([]byte(o.HashedPassword)) {
  if err := s.migratePasswordHash(r.Context(), i.ID, []byte(p.Password)); err != nil {
- return nil, s.handleLoginError(w, r, f, &p, err)
+ return nil, s.handleLoginError(r, f, &p, err)
  }
  }
  }
 
  f.Active = s.ID()
  if err = s.d.LoginFlowPersister().UpdateLoginFlow(r.Context(), f); err != nil {
- return nil, s.handleLoginError(w, r, f, &p, errors.WithStack(herodot.ErrInternalServerError.WithReason("Could not update flow").WithDebug(err.Error())))
+ return nil, s.handleLoginError(r, f, &p, errors.WithStack(herodot.ErrInternalServerError.WithReason("Could not update flow").WithDebug(err.Error())))
  }
 
  return i, nil

selfservice/strategy/password/login_test.go

@@ -1171,22 +1171,6 @@ func TestFormHydration(t *testing.T) {
  })
  })
 
- t.Run("case=WithIdentifier", func(t *testing.T) {
- t.Run("case=account enumeration mitigation disabled", func(t *testing.T) {
- ctx := configtesthelpers.WithConfigValue(ctx, config.ViperKeySecurityAccountEnumerationMitigate, false)
- r, f := newFlow(ctx, t)
- require.ErrorIs(t, fh.PopulateLoginMethodIdentifierFirstCredentials(r, f, login.WithIdentifier("foo@bar.com")), idfirst.ErrNoCredentialsFound)
- toSnapshot(t, f)
- })
-
- t.Run("case=account enumeration mitigation enabled", func(t *testing.T) {
- ctx := configtesthelpers.WithConfigValue(ctx, config.ViperKeySecurityAccountEnumerationMitigate, true)
- r, f := newFlow(ctx, t)
- require.ErrorIs(t, fh.PopulateLoginMethodIdentifierFirstCredentials(r, f, login.WithIdentifier("foo@bar.com")), idfirst.ErrNoCredentialsFound)
- toSnapshot(t, f)
- })
- })
-
  t.Run("case=WithIdentityHint", func(t *testing.T) {
  t.Run("case=account enumeration mitigation enabled and identity has no password", func(t *testing.T) {
  ctx := configtesthelpers.WithConfigValue(ctx, config.ViperKeySecurityAccountEnumerationMitigate, true)