diff --git a/selfservice/flow/settings/handler.go b/selfservice/flow/settings/handler.go index 7287e6a67668..f347a9449441 100644 --- a/selfservice/flow/settings/handler.go +++ b/selfservice/flow/settings/handler.go @@ -4,15 +4,14 @@ import ( "net/http" "time" - "github.com/ory/kratos/text" - - "github.com/ory/kratos/ui/node" - "github.com/ory/x/sqlcon" - "github.com/julienschmidt/httprouter" "github.com/pkg/errors" "github.com/ory/herodot" + "github.com/ory/nosurf" + "github.com/ory/x/sqlcon" + "github.com/ory/x/urlx" + "github.com/ory/kratos/continuity" "github.com/ory/kratos/driver/config" "github.com/ory/kratos/identity" @@ -20,9 +19,9 @@ import ( "github.com/ory/kratos/selfservice/errorx" "github.com/ory/kratos/selfservice/flow" "github.com/ory/kratos/session" + "github.com/ory/kratos/text" + "github.com/ory/kratos/ui/node" "github.com/ory/kratos/x" - "github.com/ory/nosurf" - "github.com/ory/x/urlx" ) const ( @@ -537,12 +536,18 @@ func (h *Handler) submitSettingsFlow(w http.ResponseWriter, r *http.Request, ps } if updateContext == nil { - c := &UpdateContext{Session: ss, Flow: f} - h.d.SettingsFlowErrorHandler().WriteFlowError(w, r, node.DefaultGroup, f, c.GetIdentityToUpdate(), errors.WithStack(schema.NewNoSettingsStrategyResponsible())) + h.d.SettingsFlowErrorHandler().WriteFlowError(w, r, node.DefaultGroup, f, ss.Identity, errors.WithStack(schema.NewNoSettingsStrategyResponsible())) + return + } + + i, err := updateContext.GetIdentityToUpdate() + if err != nil { + // An identity to update must always be present. + h.d.SettingsFlowErrorHandler().WriteFlowError(w, r, node.DefaultGroup, f, ss.Identity, err) return } - if err := h.d.SettingsHookExecutor().PostSettingsHook(w, r, s, updateContext, updateContext.GetIdentityToUpdate()); err != nil { + if err := h.d.SettingsHookExecutor().PostSettingsHook(w, r, s, updateContext, i); err != nil { h.d.SettingsFlowErrorHandler().WriteFlowError(w, r, node.DefaultGroup, f, ss.Identity, err) return } diff --git a/selfservice/flow/settings/hook.go b/selfservice/flow/settings/hook.go index d23cfdf60819..86bcacc5e70f 100644 --- a/selfservice/flow/settings/hook.go +++ b/selfservice/flow/settings/hook.go @@ -9,9 +9,10 @@ import ( "github.com/ory/kratos/text" "github.com/ory/kratos/ui/node" - "github.com/ory/kratos/schema" "github.com/ory/x/sqlcon" + "github.com/ory/kratos/schema" + "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -223,6 +224,6 @@ func (e *HookExecutor) PostSettingsHook(w http.ResponseWriter, r *http.Request, return nil } - x.ContentNegotiationRedirection(w, r, ctxUpdate.GetIdentityToUpdate().CopyWithoutCredentials(), e.d.Writer(), returnTo.String()) + x.ContentNegotiationRedirection(w, r, i.CopyWithoutCredentials(), e.d.Writer(), returnTo.String()) return nil } diff --git a/selfservice/flow/settings/strategy_helper.go b/selfservice/flow/settings/strategy_helper.go index fa358e767218..bb6809b4c6fa 100644 --- a/selfservice/flow/settings/strategy_helper.go +++ b/selfservice/flow/settings/strategy_helper.go @@ -38,12 +38,11 @@ func (c *UpdateContext) UpdateIdentity(i *identity.Identity) { c.toUpdate = i } -func (c *UpdateContext) GetIdentityToUpdate() *identity.Identity { +func (c *UpdateContext) GetIdentityToUpdate() (*identity.Identity, error) { if c.toUpdate == nil { - return c.GetSessionIdentity() + return nil, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Could not find a identity to update.")) } - - return c.toUpdate + return c.toUpdate, nil } func (c UpdateContext) GetSessionIdentity() *identity.Identity { diff --git a/selfservice/flow/settings/strategy_helper_test.go b/selfservice/flow/settings/strategy_helper_test.go new file mode 100644 index 000000000000..5fd8fac32c5c --- /dev/null +++ b/selfservice/flow/settings/strategy_helper_test.go @@ -0,0 +1,21 @@ +package settings + +import ( + "github.com/ory/kratos/identity" + "github.com/ory/kratos/x" + "github.com/stretchr/testify/require" + "testing" +) + +func TestGetIdentityToUpdate(t *testing.T) { + c := new(UpdateContext) + _, err := c.GetIdentityToUpdate() + require.Error(t, err) + + expected := &identity.Identity{ID: x.NewUUID()} + c.UpdateIdentity(expected) + + actual, err := c.GetIdentityToUpdate() + require.NoError(t, err) + require.Equal(t, expected, actual) +} diff --git a/selfservice/strategy/webauthn/settings.go b/selfservice/strategy/webauthn/settings.go index 43e6cfac235b..f1b652f8465f 100644 --- a/selfservice/strategy/webauthn/settings.go +++ b/selfservice/strategy/webauthn/settings.go @@ -192,6 +192,7 @@ func (s *Strategy) continueSettingsFlowRemove(w http.ResponseWriter, r *http.Req } if len(updated) == 0 { i.DeleteCredentialsType(identity.CredentialsTypeWebAuthn) + ctxUpdate.UpdateIdentity(i) return nil } diff --git a/selfservice/strategy/webauthn/settings_test.go b/selfservice/strategy/webauthn/settings_test.go index f06e6c1f4889..cb5192047b68 100644 --- a/selfservice/strategy/webauthn/settings_test.go +++ b/selfservice/strategy/webauthn/settings_test.go @@ -375,6 +375,9 @@ func TestCompleteSettings(t *testing.T) { require.NoError(t, err) _, ok = actual.GetCredentials(identity.CredentialsTypeWebAuthn) assert.False(t, ok) + // Check not to remove other credentials with webauthn + _, ok = actual.GetCredentials(identity.CredentialsTypePassword) + assert.True(t, ok) } t.Run("type=browser", func(t *testing.T) {