feat(recovery): allow invalidation of existing sessions

You can now use the `revoke_active_sessions` hook in the recovery flow. It invalidates all of an identity's sessions on successful account recovery. Closes #1077
ory · Mar 7, 2022 · 5029884 · 5029884
1 parent bf10868
commit 5029884
Show file tree

Hide file tree

Showing 8 changed files with 191 additions and 28 deletions.
diff --git a/embedx/config.schema.json b/embedx/config.schema.json
@@ -550,6 +550,21 @@
       "uniqueItems": true,
       "additionalItems": false
     },
+    "selfServiceAfterRecoveryHooks": {
+      "type": "array",
+      "items": {
+        "anyOf": [
+          {
+            "$ref": "#/definitions/selfServiceWebHook"
+          },
+          {
+            "$ref": "#/definitions/selfServiceSessionRevokerHook"
+          }
+        ]
+      },
+      "uniqueItems": true,
+      "additionalItems": false
+    },
     "selfServiceAfterSettingsMethod": {
       "type": "object",
       "additionalProperties": false,
@@ -742,7 +757,7 @@
           "$ref": "#/definitions/defaultReturnTo"
         },
         "hooks": {
-          "$ref": "#/definitions/selfServiceHooks"
+          "$ref": "#/definitions/selfServiceAfterRecoveryHooks"
         }
       }
     },

diff --git a/selfservice/hook/session_destroyer.go b/selfservice/hook/session_destroyer.go
@@ -1,13 +1,15 @@
 package hook
 
 import (
+	"github.com/ory/kratos/selfservice/flow/recovery"
 	"net/http"
 
 	"github.com/ory/kratos/selfservice/flow/login"
 	"github.com/ory/kratos/session"
 )
 
 var _ login.PostHookExecutor = new(SessionDestroyer)
+var _ recovery.PostHookExecutor = new(SessionDestroyer)
 
 type (
 	sessionDestroyerDependencies interface {
@@ -24,7 +26,14 @@ func NewSessionDestroyer(r sessionDestroyerDependencies) *SessionDestroyer {
 }
 
 func (e *SessionDestroyer) ExecuteLoginPostHook(_ http.ResponseWriter, r *http.Request, _ *login.Flow, s *session.Session) error {
-	if err := e.r.SessionPersister().DeleteSessionsByIdentity(r.Context(), s.Identity.ID); err != nil {
+	if _, err := e.r.SessionPersister().RevokeSessionsIdentityExcept(r.Context(), s.Identity.ID, s.ID); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (e *SessionDestroyer) ExecutePostRecoveryHook(_ http.ResponseWriter, r *http.Request, _ *recovery.Flow, s *session.Session) error {
+	if _, err := e.r.SessionPersister().RevokeSessionsIdentityExcept(r.Context(), s.Identity.ID, s.ID); err != nil {
 		return err
 	}
 	return nil

diff --git a/selfservice/hook/session_destroyer_test.go b/selfservice/hook/session_destroyer_test.go
@@ -20,7 +20,6 @@ import (
 	"github.com/ory/kratos/internal"
 	"github.com/ory/kratos/selfservice/hook"
 	"github.com/ory/kratos/session"
-	"github.com/ory/x/sqlcon"
 )
 
 func init() {
@@ -35,32 +34,64 @@ func TestSessionDestroyer(t *testing.T) {
 
 	h := hook.NewSessionDestroyer(reg)
 
-	t.Run("method=ExecuteLoginPostHook", func(t *testing.T) {
-		var i identity.Identity
-		require.NoError(t, faker.FakeData(&i))
-		require.NoError(t, reg.PrivilegedIdentityPool().CreateIdentity(context.Background(), &i))
+	for _, tc := range []struct {
+		name string
+		hook func(*identity.Identity) error
+	}{
+		{
+			name: "ExecuteLoginPostHook",
+			hook: func(i *identity.Identity) error {
+				return h.ExecuteLoginPostHook(
+					httptest.NewRecorder(),
+					new(http.Request),
+					nil,
+					&session.Session{Identity: i},
+				)
+			},
+		},
+		{
+			name: "ExecutePostRecoveryHook",
+			hook: func(i *identity.Identity) error {
+				return h.ExecutePostRecoveryHook(
+					httptest.NewRecorder(),
+					new(http.Request),
+					nil,
+					&session.Session{Identity: i},
+				)
+			},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			var i identity.Identity
+			require.NoError(t, faker.FakeData(&i))
+			require.NoError(t, reg.PrivilegedIdentityPool().CreateIdentity(context.Background(), &i))
 
-		sessions := make([]session.Session, 5)
-		for k := range sessions {
-			s := sessions[k] // keep this for pointers' sake ;)
-			require.NoError(t, faker.FakeData(&s))
-			s.IdentityID = uuid.Nil
-			s.Identity = &i
+			sessions := make([]session.Session, 5)
+			for k := range sessions {
+				s := sessions[k] // keep this for pointers' sake ;)
+				require.NoError(t, faker.FakeData(&s))
+				s.IdentityID = uuid.Nil
+				s.Identity = &i
+				s.Active = true
 
-			require.NoError(t, reg.SessionPersister().UpsertSession(context.Background(), &s))
-		}
+				require.NoError(t, reg.SessionPersister().UpsertSession(context.Background(), &s))
+				sessions[k] = s
+			}
 
-		// Should revoke all the sessions.
-		require.NoError(t, h.ExecuteLoginPostHook(
-			httptest.NewRecorder(),
-			new(http.Request),
-			nil,
-			&session.Session{Identity: &i},
-		))
+			for k := range sessions {
+				sess, err := reg.SessionPersister().GetSession(context.Background(), sessions[k].ID)
+				require.NoError(t, err)
+				assert.True(t, sess.IsActive())
+			}
 
-		for k := range sessions {
-			_, err := reg.SessionPersister().GetSession(context.Background(), sessions[k].ID)
-			assert.EqualError(t, err, sqlcon.ErrNoRows.Error())
-		}
-	})
+			// Should revoke all the sessions.
+			require.NoError(t, tc.hook(&i))
+
+			for k := range sessions {
+				sess, err := reg.SessionPersister().GetSession(context.Background(), sessions[k].ID)
+				require.NoError(t, err)
+				assert.False(t, sess.IsActive())
+			}
+		})
+	}
 }
diff --git a/selfservice/strategy/link/strategy_recovery_test.go b/selfservice/strategy/link/strategy_recovery_test.go
@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"strings"
 	"testing"
 	"time"
 
@@ -194,7 +195,7 @@ func TestRecovery(t *testing.T) {
 
 	public, _ := testhelpers.NewKratosServerWithCSRF(t, reg)
 
-	var createIdentityToRecover = func(email string) {
+	var createIdentityToRecover = func(email string) *identity.Identity {
 		var id = &identity.Identity{
 			Credentials: map[identity.CredentialsType]identity.Credentials{
 				"password": {Type: "password", Identifiers: []string{email}, Config: sqlxx.JSONRawMessage(`{"hashed_password":"foo"}`)}},
@@ -208,6 +209,7 @@ func TestRecovery(t *testing.T) {
 		assert.False(t, addr.Verified)
 		assert.Nil(t, addr.VerifiedAt)
 		assert.Equal(t, identity.VerifiableAddressStatusPending, addr.Status)
+		return id
 	}
 
 	var expect = func(t *testing.T, hc *http.Client, isAPI, isSPA bool, values func(url.Values), c int) string {
@@ -512,6 +514,52 @@ func TestRecovery(t *testing.T) {
 		check(t, expectSuccess(t, nil, false, false, values))
 	})
 
+	t.Run("description=should recover and invalidate all other sessions if hook is set", func(t *testing.T) {
+		conf.MustSet(config.HookStrategyKey(config.ViperKeySelfServiceRecoveryAfter, config.HookGlobal), []config.SelfServiceHook{{Name: "revoke_active_sessions"}})
+		t.Cleanup(func() {
+			conf.MustSet(config.HookStrategyKey(config.ViperKeySelfServiceRegistrationAfter, identity.CredentialsTypePassword.String()), nil)
+		})
+
+		recoveryEmail := strings.ToLower(testhelpers.RandomEmail())
+		email := recoveryEmail
+		id := createIdentityToRecover(email)
+
+		sess, err := session.NewActiveSession(id, conf, time.Now(), identity.CredentialsTypePassword, identity.AuthenticatorAssuranceLevel1)
+		require.NoError(t, err)
+		require.NoError(t, reg.SessionPersister().UpsertSession(context.Background(), sess))
+
+		actualSession, err := reg.SessionPersister().GetSession(context.Background(), sess.ID)
+		require.NoError(t, err)
+		assert.True(t, actualSession.IsActive())
+
+		var check = func(t *testing.T, actual string) {
+			message := testhelpers.CourierExpectMessage(t, reg, recoveryEmail, "Recover access to your account")
+			recoveryLink := testhelpers.CourierExpectLinkInMessage(t, message, 1)
+
+			cl := testhelpers.NewClientWithCookies(t)
+			cl.CheckRedirect = func(req *http.Request, via []*http.Request) error {
+				return http.ErrUseLastResponse
+			}
+			res, err := cl.Get(recoveryLink)
+			require.NoError(t, err)
+			require.NoError(t, res.Body.Close())
+			assert.Equal(t, http.StatusSeeOther, res.StatusCode)
+			require.Len(t, cl.Jar.Cookies(urlx.ParseOrPanic(public.URL)), 2)
+			cookies := spew.Sdump(cl.Jar.Cookies(urlx.ParseOrPanic(public.URL)))
+			assert.Contains(t, cookies, "ory_kratos_session")
+
+			actualSession, err := reg.SessionPersister().GetSession(context.Background(), sess.ID)
+			require.NoError(t, err)
+			assert.False(t, actualSession.IsActive())
+		}
+
+		var values = func(v url.Values) {
+			v.Set("email", recoveryEmail)
+		}
+
+		check(t, expectSuccess(t, nil, false, false, values))
+	})
+
 	t.Run("description=should not be able to use an invalid link", func(t *testing.T) {
 		c := testhelpers.NewClientWithCookies(t)
 		f := testhelpers.InitializeRecoveryFlowViaBrowser(t, c, false, public, nil)

diff --git a/...estCompleteLogin-flow=refresh-case=no_webauth_credentials-passwordless=false-browser.json b/...estCompleteLogin-flow=refresh-case=no_webauth_credentials-passwordless=false-browser.json
@@ -0,0 +1,15 @@
+[
+  {
+    "attributes": {
+      "disabled": false,
+      "name": "csrf_token",
+      "node_type": "input",
+      "required": true,
+      "type": "hidden"
+    },
+    "group": "default",
+    "messages": [],
+    "meta": {},
+    "type": "input"
+  }
+]
diff --git a/...ts/TestCompleteLogin-flow=refresh-case=no_webauth_credentials-passwordless=false-spa.json b/...ts/TestCompleteLogin-flow=refresh-case=no_webauth_credentials-passwordless=false-spa.json
@@ -0,0 +1,15 @@
+[
+  {
+    "attributes": {
+      "disabled": false,
+      "name": "csrf_token",
+      "node_type": "input",
+      "required": true,
+      "type": "hidden"
+    },
+    "group": "default",
+    "messages": [],
+    "meta": {},
+    "type": "input"
+  }
+]
diff --git a/...TestCompleteLogin-flow=refresh-case=no_webauth_credentials-passwordless=true-browser.json b/...TestCompleteLogin-flow=refresh-case=no_webauth_credentials-passwordless=true-browser.json
@@ -0,0 +1,15 @@
+[
+  {
+    "attributes": {
+      "disabled": false,
+      "name": "csrf_token",
+      "node_type": "input",
+      "required": true,
+      "type": "hidden"
+    },
+    "group": "default",
+    "messages": [],
+    "meta": {},
+    "type": "input"
+  }
+]
diff --git a/...ots/TestCompleteLogin-flow=refresh-case=no_webauth_credentials-passwordless=true-spa.json b/...ots/TestCompleteLogin-flow=refresh-case=no_webauth_credentials-passwordless=true-spa.json
@@ -0,0 +1,15 @@
+[
+  {
+    "attributes": {
+      "disabled": false,
+      "name": "csrf_token",
+      "node_type": "input",
+      "required": true,
+      "type": "hidden"
+    },
+    "group": "default",
+    "messages": [],
+    "meta": {},
+    "type": "input"
+  }
+]