Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: hook to revoke sessions after password changed #3514

Merged
merged 1 commit into from
Sep 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions embedx/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,9 @@
"anyOf": [
{
"$ref": "#/definitions/selfServiceWebHook"
},
{
"$ref": "#/definitions/selfServiceSessionRevokerHook"
}
]
},
Expand Down
10 changes: 5 additions & 5 deletions selfservice/flow/settings/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ type (
PostHookPrePersistExecutorFunc func(w http.ResponseWriter, r *http.Request, a *Flow, s *identity.Identity) error

PostHookPostPersistExecutor interface {
ExecuteSettingsPostPersistHook(w http.ResponseWriter, r *http.Request, a *Flow, s *identity.Identity) error
ExecuteSettingsPostPersistHook(w http.ResponseWriter, r *http.Request, a *Flow, id *identity.Identity, s *session.Session) error
}
PostHookPostPersistExecutorFunc func(w http.ResponseWriter, r *http.Request, a *Flow, s *identity.Identity) error
PostHookPostPersistExecutorFunc func(w http.ResponseWriter, r *http.Request, a *Flow, id *identity.Identity, s *session.Session) error

HooksProvider interface {
PreSettingsHooks(ctx context.Context) []PreHookExecutor
Expand Down Expand Up @@ -84,8 +84,8 @@ func (f PostHookPrePersistExecutorFunc) ExecuteSettingsPrePersistHook(w http.Res
return f(w, r, a, s)
}

func (f PostHookPostPersistExecutorFunc) ExecuteSettingsPostPersistHook(w http.ResponseWriter, r *http.Request, a *Flow, s *identity.Identity) error {
return f(w, r, a, s)
func (f PostHookPostPersistExecutorFunc) ExecuteSettingsPostPersistHook(w http.ResponseWriter, r *http.Request, a *Flow, id *identity.Identity, s *session.Session) error {
return f(w, r, a, id, s)
}

func PostHookPostPersistExecutorNames(e []PostHookPostPersistExecutor) []string {
Expand Down Expand Up @@ -252,7 +252,7 @@ func (e *HookExecutor) PostSettingsHook(w http.ResponseWriter, r *http.Request,
}

for k, executor := range e.d.PostSettingsPostPersistHooks(r.Context(), settingsType) {
if err := executor.ExecuteSettingsPostPersistHook(w, r, ctxUpdate.Flow, i); err != nil {
if err := executor.ExecuteSettingsPostPersistHook(w, r, ctxUpdate.Flow, i, ctxUpdate.Session); err != nil {
if errors.Is(err, ErrHookAbortFlow) {
e.d.Logger().
WithRequest(r).
Expand Down
2 changes: 1 addition & 1 deletion selfservice/hook/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (e Error) ExecuteSettingsPrePersistHook(w http.ResponseWriter, r *http.Requ
return e.err("ExecuteSettingsPrePersistHook", settings.ErrHookAbortFlow)
}

func (e Error) ExecuteSettingsPostPersistHook(w http.ResponseWriter, r *http.Request, a *settings.Flow, s *identity.Identity) error {
func (e Error) ExecuteSettingsPostPersistHook(w http.ResponseWriter, r *http.Request, a *settings.Flow, id *identity.Identity, s *session.Session) error {
return e.err("ExecuteSettingsPostPersistHook", settings.ErrHookAbortFlow)
}

Expand Down
12 changes: 12 additions & 0 deletions selfservice/hook/session_destroyer.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,18 @@ import (
"context"
"net/http"

"github.com/ory/kratos/identity"
"github.com/ory/kratos/selfservice/flow/login"
"github.com/ory/kratos/selfservice/flow/recovery"
"github.com/ory/kratos/selfservice/flow/settings"
"github.com/ory/kratos/session"
"github.com/ory/kratos/ui/node"
"github.com/ory/x/otelx"
)

var _ login.PostHookExecutor = new(SessionDestroyer)
var _ recovery.PostHookExecutor = new(SessionDestroyer)
var _ settings.PostHookPostPersistExecutor = new(SessionDestroyer)

type (
sessionDestroyerDependencies interface {
Expand Down Expand Up @@ -48,3 +51,12 @@ func (e *SessionDestroyer) ExecutePostRecoveryHook(_ http.ResponseWriter, r *htt
return nil
})
}

func (e *SessionDestroyer) ExecuteSettingsPostPersistHook(_ http.ResponseWriter, r *http.Request, _ *settings.Flow, i *identity.Identity, s *session.Session) error {
return otelx.WithSpan(r.Context(), "selfservice.hook.SessionDestroyer.ExecuteSettingsPostPersistHook", func(ctx context.Context) error {
if _, err := e.r.SessionPersister().RevokeSessionsIdentityExcept(ctx, i.ID, s.ID); err != nil {
return err
}
return nil
})
}
12 changes: 12 additions & 0 deletions selfservice/hook/session_destroyer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,18 @@ func TestSessionDestroyer(t *testing.T) {
)
},
},
{
name: "ExecuteSettingsPostPersistHook",
hook: func(i *identity.Identity) error {
return h.ExecuteSettingsPostPersistHook(
httptest.NewRecorder(),
new(http.Request),
nil,
i,
&session.Session{Identity: i},
)
},
},
} {
t.Run(tc.name, func(t *testing.T) {
var i identity.Identity
Expand Down
2 changes: 1 addition & 1 deletion selfservice/hook/verification.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (e *Verifier) ExecutePostRegistrationPostPersistHook(w http.ResponseWriter,
})
}

func (e *Verifier) ExecuteSettingsPostPersistHook(w http.ResponseWriter, r *http.Request, f *settings.Flow, i *identity.Identity) error {
func (e *Verifier) ExecuteSettingsPostPersistHook(w http.ResponseWriter, r *http.Request, f *settings.Flow, i *identity.Identity, _ *session.Session) error {
return otelx.WithSpan(r.Context(), "selfservice.hook.Verifier.ExecuteSettingsPostPersistHook", func(ctx context.Context) error {
return e.do(w, r.WithContext(ctx), i, f, nil)
})
Expand Down
2 changes: 1 addition & 1 deletion selfservice/hook/verification_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func TestVerifier(t *testing.T) {
for k, hf := range map[string]func(*hook.Verifier, *identity.Identity, flow.Flow) error{
"settings": func(h *hook.Verifier, i *identity.Identity, f flow.Flow) error {
return h.ExecuteSettingsPostPersistHook(
httptest.NewRecorder(), u, f.(*settings.Flow), i)
httptest.NewRecorder(), u, f.(*settings.Flow), i, &session.Session{ID: x.NewUUID(), Identity: i})
},
"register": func(h *hook.Verifier, i *identity.Identity, f flow.Flow) error {
return h.ExecutePostRegistrationPostPersistHook(
Expand Down
2 changes: 1 addition & 1 deletion selfservice/hook/web_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ func (e *WebHook) ExecuteSettingsPreHook(_ http.ResponseWriter, req *http.Reques
})
}

func (e *WebHook) ExecuteSettingsPostPersistHook(_ http.ResponseWriter, req *http.Request, flow *settings.Flow, id *identity.Identity) error {
func (e *WebHook) ExecuteSettingsPostPersistHook(_ http.ResponseWriter, req *http.Request, flow *settings.Flow, id *identity.Identity, _ *session.Session) error {
if gjson.GetBytes(e.conf, "can_interrupt").Bool() || gjson.GetBytes(e.conf, "response.parse").Bool() {
return nil
}
Expand Down
9 changes: 5 additions & 4 deletions selfservice/hook/web_hook_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ func TestWebHooks(t *testing.T) {
uc: "Post Settings Hook",
createFlow: func() flow.Flow { return &settings.Flow{ID: x.NewUUID()} },
callWebHook: func(wh *hook.WebHook, req *http.Request, f flow.Flow, s *session.Session) error {
return wh.ExecuteSettingsPostPersistHook(nil, req, f.(*settings.Flow), s.Identity)
return wh.ExecuteSettingsPostPersistHook(nil, req, f.(*settings.Flow), s.Identity, s)
},
expectedBody: func(req *http.Request, f flow.Flow, s *session.Session) string {
return bodyWithFlowAndIdentity(req, f, s)
Expand Down Expand Up @@ -568,7 +568,7 @@ func TestWebHooks(t *testing.T) {
uc: "Post Settings Hook - no block",
createFlow: func() flow.Flow { return &settings.Flow{ID: x.NewUUID()} },
callWebHook: func(wh *hook.WebHook, req *http.Request, f flow.Flow, s *session.Session) error {
return wh.ExecuteSettingsPostPersistHook(nil, req, f.(*settings.Flow), s.Identity)
return wh.ExecuteSettingsPostPersistHook(nil, req, f.(*settings.Flow), s.Identity, s)
},
webHookResponse: func() (int, []byte) {
return http.StatusOK, []byte{}
Expand All @@ -590,7 +590,7 @@ func TestWebHooks(t *testing.T) {
uc: "Post Settings Hook Post Persist - block has no effect",
createFlow: func() flow.Flow { return &settings.Flow{ID: x.NewUUID()} },
callWebHook: func(wh *hook.WebHook, req *http.Request, f flow.Flow, s *session.Session) error {
return wh.ExecuteSettingsPostPersistHook(nil, req, f.(*settings.Flow), s.Identity)
return wh.ExecuteSettingsPostPersistHook(nil, req, f.(*settings.Flow), s.Identity, s)
},
webHookResponse: func() (int, []byte) {
return http.StatusBadRequest, webHookResponse
Expand Down Expand Up @@ -789,8 +789,9 @@ func TestWebHooks(t *testing.T) {
wh := hook.NewWebHook(&whDeps, conf)
uuid := x.NewUUID()
in := &identity.Identity{ID: uuid}
s := &session.Session{ID: x.NewUUID(), Identity: in}

postPersistErr := wh.ExecuteSettingsPostPersistHook(nil, req, f, in)
postPersistErr := wh.ExecuteSettingsPostPersistHook(nil, req, f, in, s)
assert.NoError(t, postPersistErr)
assert.Equal(t, in, &identity.Identity{ID: uuid})

Expand Down
68 changes: 68 additions & 0 deletions selfservice/strategy/password/settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,74 @@ func TestSettings(t *testing.T) {
})
})

t.Run("description=should update the password and revoke other user sessions", func(t *testing.T) {
conf.MustSet(ctx, config.HookStrategyKey(config.ViperKeySelfServiceSettingsAfter, "password"), []config.SelfServiceHook{{Name: "revoke_active_sessions"}})
t.Cleanup(func() {
conf.MustSet(ctx, config.ViperKeySelfServiceSettingsAfter, nil)
})

var check = func(t *testing.T, actual string, id *identity.Identity) {
assert.Equal(t, "success", gjson.Get(actual, "state").String(), "%s", actual)
assert.Empty(t, gjson.Get(actual, "ui.nodes.#(name==password).attributes.value").String(), "%s", actual)

actualIdentity, err := reg.PrivilegedIdentityPool().GetIdentityConfidential(context.Background(), id.ID)
require.NoError(t, err)
cfg := string(actualIdentity.Credentials[identity.CredentialsTypePassword].Config)
assert.Contains(t, cfg, "hashed_password", "%+v", actualIdentity.Credentials)
require.Len(t, actualIdentity.Credentials[identity.CredentialsTypePassword].Identifiers, 1)
assert.Contains(t, actualIdentity.Credentials[identity.CredentialsTypePassword].Identifiers[0], "-4")
}

var initClients = func(isAPI, isSPA bool, id *identity.Identity) (client1, client2 *http.Client) {
if isAPI {
client1 = testhelpers.NewHTTPClientWithIdentitySessionToken(t, reg, id)
client2 = testhelpers.NewHTTPClientWithIdentitySessionToken(t, reg, id)
return client1, client2
}
client1 = testhelpers.NewHTTPClientWithIdentitySessionCookie(t, reg, id)
client2 = testhelpers.NewHTTPClientWithIdentitySessionCookie(t, reg, id)

return client1, client2
}

var run = func(t *testing.T, isAPI, isSPA bool, id *identity.Identity) {
var payload = func(v url.Values) {
v.Set("method", "password")
v.Set("password", randx.MustString(16, randx.AlphaNum))
}

user1, user2 := initClients(isAPI, isSPA, id)

actual := expectSuccess(t, isAPI, isSPA, user1, payload)
check(t, actual, id)

// second client should be logged out
res, err := user2.Do(httpx.MustNewRequest("POST", publicTS.URL+settings.RouteSubmitFlow, strings.NewReader(url.Values{"foo": {"bar"}}.Encode()), "application/json"))
require.NoError(t, err)
res.Body.Close()
assert.EqualValues(t, http.StatusUnauthorized, res.StatusCode, "%+v", res.Request)

// again change password via first client
actual = expectSuccess(t, isAPI, isSPA, user1, payload)
check(t, actual, id)
}

t.Run("type=api", func(t *testing.T) {
id := newIdentityWithoutCredentials(x.NewUUID().String() + "@ory.sh")
run(t, true, false, id)
})

t.Run("type=spa", func(t *testing.T) {
id := newIdentityWithoutCredentials(x.NewUUID().String() + "@ory.sh")
run(t, false, true, id)
})

t.Run("type=browser", func(t *testing.T) {
id := newIdentityWithoutCredentials(x.NewUUID().String() + "@ory.sh")
run(t, false, false, id)
})
})

t.Run("case=should fail if no identifier was set in the schema", func(t *testing.T) {
testhelpers.SetDefaultIdentitySchema(conf, "file://stub/missing-identifier.schema.json")

Expand Down
2 changes: 1 addition & 1 deletion test/e2e/cypress/support/config.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ export interface SelfServiceAfterSettings {
}
export interface SelfServiceAfterSettingsMethod {
default_browser_return_url?: RedirectBrowsersToSetURLPerDefault
hooks?: SelfServiceWebHook[]
hooks?: (SelfServiceWebHook | SelfServiceSessionRevokerHook)[]
}
export interface SelfServiceWebHook {
hook: "web_hook"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
default_browser_return_url: "#/definitions/defaultReturnTo"
hooks:
- "#/definitions/selfServiceWebHook"
- "#/definitions/selfServiceSessionRevokerHook"
Loading