feat: separate 2fa refresh from 1st factor refresh (#3961)

ory · Jul 11, 2024 · 89355d8 · 89355d8
1 parent e2e81ac
commit 89355d8
Show file tree

Hide file tree

Showing 33 changed files with 187 additions and 197 deletions.
diff --git a/selfservice/flow/login/handler.go b/selfservice/flow/login/handler.go
@@ -217,16 +217,25 @@ preLoginHook:
 		switch strategy := s.(type) {
 		case FormHydrator:
 			switch {
-			case f.IsRefresh():
-				populateErr = strategy.PopulateLoginMethodRefresh(r, f)
 			case f.RequestedAAL == identity.AuthenticatorAssuranceLevel1:
-				if h.d.Config().SelfServiceLoginFlowIdentifierFirstEnabled(r.Context()) {
+				switch {
+				case f.IsRefresh():
+					// Refreshing takes precedence over identifier_first auth which can not be a refresh flow.
+					// Therefor this comes first.
+					populateErr = strategy.PopulateLoginMethodFirstFactorRefresh(r, f)
+				case h.d.Config().SelfServiceLoginFlowIdentifierFirstEnabled(r.Context()):
 					populateErr = strategy.PopulateLoginMethodIdentifierFirstIdentification(r, f)
-				} else {
+				default:
 					populateErr = strategy.PopulateLoginMethodFirstFactor(r, f)
 				}
 			case f.RequestedAAL == identity.AuthenticatorAssuranceLevel2:
-				populateErr = strategy.PopulateLoginMethodSecondFactor(r, f)
+				switch {
+				case f.IsRefresh():
+					// Refresh takes precedence.
+					populateErr = strategy.PopulateLoginMethodSecondFactorRefresh(r, f)
+				default:
+					populateErr = strategy.PopulateLoginMethodSecondFactor(r, f)
+				}
 			}
 		case OneStepFormHydrator:
 			populateErr = strategy.PopulateLoginMethod(r, f.RequestedAAL, f)

diff --git a/selfservice/flow/login/strategy_form_hydrator.go b/selfservice/flow/login/strategy_form_hydrator.go
@@ -16,9 +16,10 @@ type OneStepFormHydrator interface {
 }
 
 type FormHydrator interface {
-	PopulateLoginMethodRefresh(r *http.Request, sr *Flow) error
+	PopulateLoginMethodFirstFactorRefresh(r *http.Request, sr *Flow) error
 	PopulateLoginMethodFirstFactor(r *http.Request, sr *Flow) error
 	PopulateLoginMethodSecondFactor(r *http.Request, sr *Flow) error
+	PopulateLoginMethodSecondFactorRefresh(r *http.Request, sr *Flow) error
 	PopulateLoginMethodIdentifierFirstCredentials(r *http.Request, sr *Flow, options ...FormHydratorModifier) error
 	PopulateLoginMethodIdentifierFirstIdentification(r *http.Request, sr *Flow) error
 }

diff --git a/..._2fa_and_request_is_1fa_with_refresh.json → ...e_is_used_for_2fa_but_request_is_1fa.json b/..._2fa_and_request_is_1fa_with_refresh.json → ...e_is_used_for_2fa_but_request_is_1fa.json
diff --git a/...asswordless_login_and_request_is_1fa.json → ...asswordless_login_and_request_is_1fa.json b/...asswordless_login_and_request_is_1fa.json → ...asswordless_login_and_request_is_1fa.json
diff --git a/...e_is_used_for_2fa_but_request_is_1fa.json → ..._2fa_and_request_is_1fa_with_refresh.json b/...e_is_used_for_2fa_but_request_is_1fa.json → ..._2fa_and_request_is_1fa_with_refresh.json
diff --git a/...ogin_and_request_is_1fa_with_refresh.json → ...ogin_and_request_is_1fa_with_refresh.json b/...ogin_and_request_is_1fa_with_refresh.json → ...ogin_and_request_is_1fa_with_refresh.json
diff --git a/...Hydration-method=PopulateLoginMethodRefresh-case=code_is_used_for_passwordless_login.json b/...Hydration-method=PopulateLoginMethodRefresh-case=code_is_used_for_passwordless_login.json
diff --git a/...e_is_used_for_2fa_and_request_is_2fa.json → ...e_is_used_for_2fa_and_request_is_2fa.json b/...e_is_used_for_2fa_and_request_is_2fa.json → ...e_is_used_for_2fa_and_request_is_2fa.json
diff --git a/...asswordless_login_and_request_is_2fa.json → ...asswordless_login_and_request_is_2fa.json b/...asswordless_login_and_request_is_2fa.json → ...asswordless_login_and_request_is_2fa.json
diff --git a/..._2fa_and_request_is_2fa_with_refresh.json → ..._2fa_and_request_is_2fa_with_refresh.json b/..._2fa_and_request_is_2fa_with_refresh.json → ..._2fa_and_request_is_2fa_with_refresh.json
diff --git a/...ogin_and_request_is_2fa_with_refresh.json → ...ogin_and_request_is_2fa_with_refresh.json b/...ogin_and_request_is_2fa_with_refresh.json → ...ogin_and_request_is_2fa_with_refresh.json
diff --git a/selfservice/strategy/code/strategy_login.go b/selfservice/strategy/code/strategy_login.go
@@ -372,7 +372,7 @@ func (s *Strategy) loginVerifyCode(ctx context.Context, r *http.Request, f *logi
 	return i, nil
 }
 
-func (s *Strategy) PopulateLoginMethodRefresh(r *http.Request, f *login.Flow) error {
+func (s *Strategy) PopulateLoginMethodFirstFactorRefresh(r *http.Request, f *login.Flow) error {
 	return s.PopulateMethod(r, f)
 }
 
@@ -384,6 +384,10 @@ func (s *Strategy) PopulateLoginMethodSecondFactor(r *http.Request, f *login.Flo
 	return s.PopulateMethod(r, f)
 }
 
+func (s *Strategy) PopulateLoginMethodSecondFactorRefresh(r *http.Request, f *login.Flow) error {
+	return s.PopulateMethod(r, f)
+}
+
 func (s *Strategy) PopulateLoginMethodIdentifierFirstCredentials(r *http.Request, f *login.Flow, _ ...login.FormHydratorModifier) error {
 	if s.deps.Config().SelfServiceCodeStrategy(r.Context()).PasswordlessEnabled {
 		f.GetUI().Nodes.Append(

diff --git a/selfservice/strategy/code/strategy_login_test.go b/selfservice/strategy/code/strategy_login_test.go
@@ -831,84 +831,69 @@ func TestFormHydration(t *testing.T) {
 	})
 
 	t.Run("method=PopulateLoginMethodFirstFactor", func(t *testing.T) {
-		t.Run("case=aal1", func(t *testing.T) {
-			t.Run("case=code is used for 2fa but request is 1fa", func(t *testing.T) {
-				r, f := newFlow(mfaEnabled, t)
-				f.RequestedAAL = identity.AuthenticatorAssuranceLevel1
-				require.NoError(t, fh.PopulateLoginMethodFirstFactor(r, f))
-				toSnapshot(t, f)
-			})
-
-			t.Run("case=code is used for passwordless login and request is 1fa", func(t *testing.T) {
-				r, f := newFlow(passwordlessEnabled, t)
-				f.RequestedAAL = identity.AuthenticatorAssuranceLevel1
-				require.NoError(t, fh.PopulateLoginMethodFirstFactor(r, f))
-				toSnapshot(t, f)
-			})
-
-			t.Run("case=code is used for passwordless login and request is 1fa with refresh", func(t *testing.T) {
-				r, f := newFlow(passwordlessEnabled, t)
-				f.RequestedAAL = identity.AuthenticatorAssuranceLevel1
-				f.Refresh = true
-				require.NoError(t, fh.PopulateLoginMethodFirstFactor(r, f))
-				toSnapshot(t, f)
-			})
-
-			t.Run("case=code is used for 2fa and request is 1fa with refresh", func(t *testing.T) {
-				r, f := newFlow(mfaEnabled, t)
-				f.RequestedAAL = identity.AuthenticatorAssuranceLevel1
-				f.Refresh = true
-				require.NoError(t, fh.PopulateLoginMethodFirstFactor(r, f))
-				toSnapshot(t, f)
-			})
+		t.Run("case=code is used for 2fa but request is 1fa", func(t *testing.T) {
+			r, f := newFlow(mfaEnabled, t)
+			f.RequestedAAL = identity.AuthenticatorAssuranceLevel1
+			require.NoError(t, fh.PopulateLoginMethodFirstFactor(r, f))
+			toSnapshot(t, f)
 		})
 
-		t.Run("case=aal2", func(t *testing.T) {
-			t.Run("case=code is used for 2fa and request is 2fa", func(t *testing.T) {
-				r, f := newFlow(mfaEnabled, t)
-				toMFARequest(r, f)
-				require.NoError(t, fh.PopulateLoginMethodFirstFactor(r, f))
-				toSnapshot(t, f)
-			})
+		t.Run("case=code is used for passwordless login and request is 1fa", func(t *testing.T) {
+			r, f := newFlow(passwordlessEnabled, t)
+			f.RequestedAAL = identity.AuthenticatorAssuranceLevel1
+			require.NoError(t, fh.PopulateLoginMethodFirstFactor(r, f))
+			toSnapshot(t, f)
+		})
+	})
 
-			t.Run("case=code is used for passwordless login and request is 2fa", func(t *testing.T) {
-				r, f := newFlow(passwordlessEnabled, t)
-				toMFARequest(r, f)
-				require.NoError(t, fh.PopulateLoginMethodFirstFactor(r, f))
-				toSnapshot(t, f)
-			})
+	t.Run("method=PopulateLoginMethodFirstFactorRefresh", func(t *testing.T) {
+		t.Run("case=code is used for passwordless login and request is 1fa with refresh", func(t *testing.T) {
+			r, f := newFlow(passwordlessEnabled, t)
+			f.RequestedAAL = identity.AuthenticatorAssuranceLevel1
+			f.Refresh = true
+			require.NoError(t, fh.PopulateLoginMethodFirstFactorRefresh(r, f))
+			toSnapshot(t, f)
+		})
 
-			t.Run("case=code is used for 2fa and request is 2fa with refresh", func(t *testing.T) {
-				r, f := newFlow(mfaEnabled, t)
-				toMFARequest(r, f)
-				f.Refresh = true
-				require.NoError(t, fh.PopulateLoginMethodFirstFactor(r, f))
-				toSnapshot(t, f)
-			})
+		t.Run("case=code is used for 2fa and request is 1fa with refresh", func(t *testing.T) {
+			r, f := newFlow(mfaEnabled, t)
+			f.RequestedAAL = identity.AuthenticatorAssuranceLevel1
+			f.Refresh = true
+			require.NoError(t, fh.PopulateLoginMethodFirstFactorRefresh(r, f))
+			toSnapshot(t, f)
+		})
+	})
 
-			t.Run("case=code is used for passwordless login and request is 2fa with refresh", func(t *testing.T) {
-				r, f := newFlow(passwordlessEnabled, t)
-				toMFARequest(r, f)
-				f.Refresh = true
-				require.NoError(t, fh.PopulateLoginMethodFirstFactor(r, f))
-				toSnapshot(t, f)
-			})
+	t.Run("method=PopulateLoginMethodSecondFactor", func(t *testing.T) {
+		t.Run("case=code is used for 2fa and request is 2fa", func(t *testing.T) {
+			r, f := newFlow(mfaEnabled, t)
+			toMFARequest(r, f)
+			require.NoError(t, fh.PopulateLoginMethodSecondFactor(r, f))
+			toSnapshot(t, f)
 		})
 
+		t.Run("case=code is used for passwordless login and request is 2fa", func(t *testing.T) {
+			r, f := newFlow(passwordlessEnabled, t)
+			toMFARequest(r, f)
+			require.NoError(t, fh.PopulateLoginMethodSecondFactor(r, f))
+			toSnapshot(t, f)
+		})
 	})
 
-	t.Run("method=PopulateLoginMethodRefresh", func(t *testing.T) {
-		t.Run("case=code is used for 2fa", func(t *testing.T) {
+	t.Run("method=PopulateLoginMethodSecondFactorRefresh", func(t *testing.T) {
+		t.Run("case=code is used for 2fa and request is 2fa with refresh", func(t *testing.T) {
 			r, f := newFlow(mfaEnabled, t)
+			toMFARequest(r, f)
 			f.Refresh = true
-			require.NoError(t, fh.PopulateLoginMethodFirstFactor(r, f))
+			require.NoError(t, fh.PopulateLoginMethodSecondFactorRefresh(r, f))
 			toSnapshot(t, f)
 		})
 
-		t.Run("case=code is used for passwordless login", func(t *testing.T) {
+		t.Run("case=code is used for passwordless login and request is 2fa with refresh", func(t *testing.T) {
 			r, f := newFlow(passwordlessEnabled, t)
+			toMFARequest(r, f)
 			f.Refresh = true
-			require.NoError(t, fh.PopulateLoginMethodFirstFactor(r, f))
+			require.NoError(t, fh.PopulateLoginMethodSecondFactorRefresh(r, f))
 			toSnapshot(t, f)
 		})
 	})

diff --git a/...hodRefresh-case=code_is_used_for_2fa.json → ...opulateLoginMethodFirstFactorRefresh.json b/...hodRefresh-case=code_is_used_for_2fa.json → ...opulateLoginMethodFirstFactorRefresh.json
diff --git a/selfservice/strategy/idfirst/strategy_login.go b/selfservice/strategy/idfirst/strategy_login.go
@@ -115,7 +115,7 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow,
 	return nil, flow.ErrCompletedByStrategy
 }
 
-func (s *Strategy) PopulateLoginMethodRefresh(r *http.Request, sr *login.Flow) error {
+func (s *Strategy) PopulateLoginMethodFirstFactorRefresh(r *http.Request, sr *login.Flow) error {
 	return nil
 }
 
@@ -127,6 +127,10 @@ func (s *Strategy) PopulateLoginMethodSecondFactor(r *http.Request, sr *login.Fl
 	return nil
 }
 
+func (s *Strategy) PopulateLoginMethodSecondFactorRefresh(r *http.Request, sr *login.Flow) error {
+	return nil
+}
+
 func (s *Strategy) PopulateLoginMethodIdentifierFirstIdentification(r *http.Request, f *login.Flow) error {
 	f.UI.SetCSRF(s.d.GenerateCSRFToken(r))
 

diff --git a/selfservice/strategy/idfirst/strategy_login_test.go b/selfservice/strategy/idfirst/strategy_login_test.go
@@ -436,9 +436,15 @@ func TestFormHydration(t *testing.T) {
 		toSnapshot(t, f)
 	})
 
+	t.Run("method=PopulateLoginMethodFirstFactorRefresh", func(t *testing.T) {
+		r, f := newFlow(ctx, t)
+		require.NoError(t, fh.PopulateLoginMethodFirstFactorRefresh(r, f))
+		toSnapshot(t, f)
+	})
+
 	t.Run("method=PopulateLoginMethodRefresh", func(t *testing.T) {
 		r, f := newFlow(ctx, t)
-		require.NoError(t, fh.PopulateLoginMethodRefresh(r, f))
+		require.NoError(t, fh.PopulateLoginMethodSecondFactorRefresh(r, f))
 		toSnapshot(t, f)
 	})
 

diff --git a/...ategy/oidc/.snapshots/TestFormHydration-method=PopulateLoginMethodFirstFactorRefresh.json b/...ategy/oidc/.snapshots/TestFormHydration-method=PopulateLoginMethodFirstFactorRefresh.json
@@ -0,0 +1,37 @@
+[
+  {
+    "type": "input",
+    "group": "default",
+    "attributes": {
+      "name": "csrf_token",
+      "type": "hidden",
+      "required": true,
+      "disabled": false,
+      "node_type": "input"
+    },
+    "messages": [],
+    "meta": {}
+  },
+  {
+    "type": "input",
+    "group": "oidc",
+    "attributes": {
+      "name": "provider",
+      "type": "submit",
+      "value": "test-provider",
+      "disabled": false,
+      "node_type": "input"
+    },
+    "messages": [],
+    "meta": {
+      "label": {
+        "id": 1010002,
+        "text": "Sign in with test-provider",
+        "type": "info",
+        "context": {
+          "provider": "test-provider"
+        }
+      }
+    }
+  }
+]
diff --git a/...tegy/oidc/.snapshots/TestFormHydration-method=PopulateLoginMethodSecondFactorRefresh.json b/...tegy/oidc/.snapshots/TestFormHydration-method=PopulateLoginMethodSecondFactorRefresh.json
@@ -0,0 +1,15 @@
+[
+  {
+    "type": "input",
+    "group": "default",
+    "attributes": {
+      "name": "csrf_token",
+      "type": "hidden",
+      "required": true,
+      "disabled": false,
+      "node_type": "input"
+    },
+    "messages": [],
+    "meta": {}
+  }
+]
diff --git a/selfservice/strategy/oidc/strategy_login.go b/selfservice/strategy/oidc/strategy_login.go
@@ -289,7 +289,7 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow,
 	return nil, errors.WithStack(flow.ErrCompletedByStrategy)
 }
 
-func (s *Strategy) PopulateLoginMethodRefresh(r *http.Request, lf *login.Flow) error {
+func (s *Strategy) PopulateLoginMethodFirstFactorRefresh(r *http.Request, lf *login.Flow) error {
 	conf, err := s.Config(r.Context())
 	if err != nil {
 		return err
@@ -331,6 +331,10 @@ func (s *Strategy) PopulateLoginMethodSecondFactor(r *http.Request, sr *login.Fl
 	return nil
 }
 
+func (s *Strategy) PopulateLoginMethodSecondFactorRefresh(r *http.Request, sr *login.Flow) error {
+	return nil
+}
+
 func (s *Strategy) PopulateLoginMethodIdentifierFirstCredentials(r *http.Request, f *login.Flow, mods ...login.FormHydratorModifier) error {
 	if f.Type != flow.TypeBrowser {
 		return nil

diff --git a/selfservice/strategy/oidc/strategy_login_test.go b/selfservice/strategy/oidc/strategy_login_test.go
@@ -95,14 +95,20 @@ func TestFormHydration(t *testing.T) {
 		toSnapshot(t, f)
 	})
 
-	t.Run("method=PopulateLoginMethodRefresh", func(t *testing.T) {
+	t.Run("method=PopulateLoginMethodFirstFactorRefresh", func(t *testing.T) {
 		r, f := newFlow(ctx, t)
 
 		id := createIdentity(t, ctx, reg, x.NewUUID(), providerID)
 		r.Header = testhelpers.NewHTTPClientWithIdentitySessionToken(t, ctx, reg, id).Transport.(*testhelpers.TransportWithHeader).GetHeader()
 		f.Refresh = true
 
-		require.NoError(t, fh.PopulateLoginMethodRefresh(r, f))
+		require.NoError(t, fh.PopulateLoginMethodFirstFactorRefresh(r, f))
+		toSnapshot(t, f)
+	})
+
+	t.Run("method=PopulateLoginMethodSecondFactorRefresh", func(t *testing.T) {
+		r, f := newFlow(ctx, t)
+		require.NoError(t, fh.PopulateLoginMethodFirstFactorRefresh(r, f))
 		toSnapshot(t, f)
 	})
 

diff --git a/.../strategy/passkey/.snapshots/TestFormHydration-method=PopulateLoginMethodFirstFactor.json b/.../strategy/passkey/.snapshots/TestFormHydration-method=PopulateLoginMethodFirstFactor.json
@@ -49,7 +49,6 @@
     "type": "script",
     "group": "webauthn",
     "attributes": {
-      "src": "http://ORY-MYC0XYW5H3:4433/.well-known/ory/webauthn.js",
       "async": true,
       "referrerpolicy": "no-referrer",
       "crossorigin": "anonymous",

diff --git a/...on-method=PopulateLoginMethodRefresh.json → ...opulateLoginMethodFirstFactorRefresh.json b/...on-method=PopulateLoginMethodRefresh.json → ...opulateLoginMethodFirstFactorRefresh.json
@@ -15,7 +15,6 @@
     "type": "script",
     "group": "webauthn",
     "attributes": {
-      "src": "http://ORY-MYC0XYW5H3:4433/.well-known/ory/webauthn.js",
       "async": true,
       "referrerpolicy": "no-referrer",
       "crossorigin": "anonymous",