From 60664aaf05dbd6b228f420688d0171e5789246be Mon Sep 17 00:00:00 2001 From: aeneasr <3372410+aeneasr@users.noreply.github.com> Date: Tue, 25 Aug 2020 17:46:09 +0200 Subject: [PATCH] feat: implement API-based tests for password method settings flows --- selfservice/strategy/password/settings.go | 4 +- .../strategy/password/settings_test.go | 268 +++++++++++------- .../strategy/password/strategy_test.go | 4 +- 3 files changed, 174 insertions(+), 102 deletions(-) diff --git a/selfservice/strategy/password/settings.go b/selfservice/strategy/password/settings.go index 9b6bf0d6335..01c8510e1e4 100644 --- a/selfservice/strategy/password/settings.go +++ b/selfservice/strategy/password/settings.go @@ -135,7 +135,7 @@ func (s *Strategy) continueSettingsFlow( } if ctxUpdate.Session.AuthenticatedAt.Add(s.c.SelfServiceFlowSettingsPrivilegedSessionMaxAge()).Before(time.Now()) { - s.handleSettingsError(w, r, ctxUpdate, p, errors.WithStack(settings.ErrRequestNeedsReAuthentication)) + s.handleSettingsError(w, r, ctxUpdate, p, errors.WithStack(settings.NewFlowNeedsReAuth())) return } @@ -198,7 +198,7 @@ func (s *Strategy) PopulateSettingsMethod(r *http.Request, _ *identity.Identity, func (s *Strategy) handleSettingsError(w http.ResponseWriter, r *http.Request, ctxUpdate *settings.UpdateContext, p *SettingsFlowPayload, err error) { // Do not pause flow if the flow type is an API flow as we can't save cookies in those flows. - if errors.Is(err, settings.ErrRequestNeedsReAuthentication) && ctxUpdate.Flow != nil && ctxUpdate.Flow.Type == flow.TypeBrowser { + if e := new(settings.FlowNeedsReAuth); errors.As(err, &e) && ctxUpdate.Flow != nil && ctxUpdate.Flow.Type == flow.TypeBrowser { if err := s.d.ContinuityManager().Pause(r.Context(), w, r, settings.ContinuityKey(s.SettingsStrategyID()), settings.ContinuityOptions(p, ctxUpdate.Session.Identity)...); err != nil { s.d.SettingsFlowErrorHandler().WriteFlowError(w, r, s.SettingsStrategyID(), ctxUpdate.Flow, ctxUpdate.Session.Identity, err) diff --git a/selfservice/strategy/password/settings_test.go b/selfservice/strategy/password/settings_test.go index 4a02a93fcbf..207669870f1 100644 --- a/selfservice/strategy/password/settings_test.go +++ b/selfservice/strategy/password/settings_test.go @@ -2,13 +2,15 @@ package password_test import ( "context" + "encoding/json" "net/http" "net/http/httptest" + "net/url" "testing" "time" - "github.com/google/uuid" "github.com/julienschmidt/httprouter" + "github.com/ory/x/assertx" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/tidwall/gjson" @@ -17,7 +19,6 @@ import ( "github.com/ory/x/randx" "github.com/ory/viper" - "github.com/ory/x/pointerx" "github.com/ory/kratos/driver/configuration" "github.com/ory/kratos/identity" @@ -25,6 +26,7 @@ import ( "github.com/ory/kratos/internal/httpclient/models" "github.com/ory/kratos/internal/testhelpers" "github.com/ory/kratos/selfservice/flow/settings" + "github.com/ory/kratos/selfservice/strategy/password" "github.com/ory/kratos/session" "github.com/ory/kratos/x" ) @@ -44,28 +46,48 @@ func TestSettings(t *testing.T) { _ = testhelpers.NewErrorTestServer(t, reg) viper.Set(configuration.ViperKeySelfServiceSettingsPrivilegedAuthenticationAfter, "1m") - primaryIdentity := &identity.Identity{ + browserIdentity1 := &identity.Identity{ ID: x.NewUUID(), Credentials: map[identity.CredentialsType]identity.Credentials{ - "password": {Type: "password", Identifiers: []string{"john@doe.com"}, Config: + "password": {Type: "password", Identifiers: []string{"john-browser@doe.com"}, Config: []byte(`{"hashed_password":"foo"}`)}}, - Traits: identity.Traits(`{"email":"john@doe.com"}`), SchemaID: configuration.DefaultIdentityTraitsSchemaID} - secondaryIdentity := &identity.Identity{ + Traits: identity.Traits(`{"email":"john-browser@doe.com"}`), SchemaID: configuration.DefaultIdentityTraitsSchemaID} + apiIdentity1 := &identity.Identity{ + ID: x.NewUUID(), Credentials: map[identity.CredentialsType]identity.Credentials{ + "password": {Type: "password", Identifiers: []string{"john-api@doe.com"}, Config: + []byte(`{"hashed_password":"foo"}`)}}, + Traits: identity.Traits(`{"email":"john-api@doe.com"}`), SchemaID: configuration.DefaultIdentityTraitsSchemaID} + browserIdentity2 := &identity.Identity{ + ID: x.NewUUID(), Credentials: map[identity.CredentialsType]identity.Credentials{}, + Traits: identity.Traits(`{}`), SchemaID: configuration.DefaultIdentityTraitsSchemaID} + apiIdentity2 := &identity.Identity{ ID: x.NewUUID(), Credentials: map[identity.CredentialsType]identity.Credentials{}, Traits: identity.Traits(`{}`), SchemaID: configuration.DefaultIdentityTraitsSchemaID} publicTS, adminTS := testhelpers.NewKratosServer(t, reg) - sessionUser1 := session.NewActiveSession(primaryIdentity, testhelpers.NewSessionLifespanProvider(time.Hour), time.Now()) - sessionUser2 := session.NewActiveSession(secondaryIdentity, testhelpers.NewSessionLifespanProvider(time.Hour), time.Now()) - browserUser1 := testhelpers.NewHTTPClientWithSessionCookie(t, reg, sessionUser1) - browserUser2 := testhelpers.NewHTTPClientWithSessionCookie(t, reg, sessionUser2) - apiUser1 := testhelpers.NewHTTPClientWithSessionToken(t, reg, sessionUser1) + browserUser1 := testhelpers.NewHTTPClientWithSessionCookie(t, reg, session.NewActiveSession(browserIdentity1, testhelpers.NewSessionLifespanProvider(time.Hour), time.Now())) + browserUser2 := testhelpers.NewHTTPClientWithSessionCookie(t, reg, session.NewActiveSession(browserIdentity2, testhelpers.NewSessionLifespanProvider(time.Hour), time.Now())) + apiUser1 := testhelpers.NewHTTPClientWithSessionToken(t, reg, session.NewActiveSession(apiIdentity1, testhelpers.NewSessionLifespanProvider(time.Hour), time.Now())) + apiUser2 := testhelpers.NewHTTPClientWithSessionToken(t, reg, session.NewActiveSession(apiIdentity2, testhelpers.NewSessionLifespanProvider(time.Hour), time.Now())) adminClient := testhelpers.NewSDKClient(adminTS) + var encodeForm = func(t *testing.T, isApi bool, values url.Values) (payload string) { + if !isApi { + return values.Encode() + } + payload = "{}" + for k := range values { + var err error + payload, err = sjson.Set(payload, k, values.Get(k)) + require.NoError(t, err) + } + return payload + } + t.Run("description=should update the password and clear errors after input error occurred", func(t *testing.T) { t.Run("description=should fail if password violates policy", func(t *testing.T) { - var run = func(t *testing.T, isAPI bool, client *http.Client) string { + var run = func(t *testing.T, isAPI bool, client *http.Client, ec int) string { var form *models.FlowMethodConfig if isAPI { rs := testhelpers.InitializeSettingsFlowViaAPI(t, client, publicTS) @@ -77,20 +99,8 @@ func TestSettings(t *testing.T) { values := testhelpers.SDKFormFieldsToURLValues(form.Fields) values.Set("password", "123456") - payload := values.Encode() - if isAPI { - payload = `{}` - for k := range values { - var err error - payload, err = sjson.Set(payload, k, values.Get(k)) - require.NoError(t, err) - } - } - - t.Logf("Submitting payload: %s", payload) - actual, _ := testhelpers.SettingsSubmitForm(t, isAPI, form, client, payload, - expectStatusCodeBetter(isAPI, http.StatusBadRequest, http.StatusNoContent)) + actual, _ := testhelpers.SettingsSubmitForm(t, isAPI, form, client, encodeForm(t, isAPI, values), ec) assert.Equal(t, *form.Action, gjson.Get(actual, "methods.password.config.action").String(), "%s", actual) return actual } @@ -99,16 +109,25 @@ func TestSettings(t *testing.T) { t.Run("session=with privileged session", func(t *testing.T) { viper.Set(configuration.ViperKeySelfServiceSettingsPrivilegedAuthenticationAfter, "5m") - actual := run(t, true, apiUser1) + actual := run(t, true, apiUser1, http.StatusBadRequest) assert.Empty(t, gjson.Get(actual, "methods.password.config.fields.#(name==password).value").String(), "%s", actual) assert.NotEmpty(t, gjson.Get(actual, "methods.password.config.fields.#(name==csrf_token).value").String(), "%s", actual) assert.Contains(t, gjson.Get(actual, "methods.password.config.fields.#(name==password).messages.0.text").String(), "password can not be used because", "%s", actual) }) + + t.Run("session=needs reauthentication", func(t *testing.T) { + viper.Set(configuration.ViperKeySelfServiceSettingsPrivilegedAuthenticationAfter, "1ns") + _ = testhelpers.NewSettingsLoginAcceptAPIServer(t, adminClient) + defer viper.Set(configuration.ViperKeySelfServiceSettingsPrivilegedAuthenticationAfter, "5m") + + actual := run(t, true, apiUser1, http.StatusForbidden) + assert.Empty(t, gjson.Get(actual, "methods.password.config.fields.#(name==password).value").String(), "%s", actual) + }) }) t.Run("type=browser", func(t *testing.T) { var runInner = func(t *testing.T) { - actual := run(t, false, browserUser1) + actual := run(t, false, browserUser1, http.StatusNoContent) assert.Empty(t, gjson.Get(actual, "methods.password.config.fields.#(name==password).value").String(), "%s", actual) assert.NotEmpty(t, gjson.Get(actual, "methods.password.config.fields.#(name==csrf_token).value").String(), "%s", actual) assert.Contains(t, gjson.Get(actual, "methods.password.config.fields.#(name==password).messages.0.text").String(), "password can not be used because", "%s", actual) @@ -127,46 +146,115 @@ func TestSettings(t *testing.T) { runInner(t) }) }) + }) + }) - t.Run("description=should update the password and clear errors if everything is ok", func(t *testing.T) { - t.Run("type=browser", func(t *testing.T) { - rs := testhelpers.InitializeSettingsFlowViaBrowser(t, browserUser1, publicTS) - f := rs.Payload.Methods[string(identity.CredentialsTypePassword)].Config - values := testhelpers.SDKFormFieldsToURLValues(f.Fields) - values.Set("password", uuid.New().String()) - actual, _ := testhelpers.SettingsSubmitForm(t, false, f, browserUser1, values.Encode(), http.StatusNoContent) - - assert.Equal(t, "success", gjson.Get(actual, "state").String(), "%s", actual) - assert.Empty(t, gjson.Get(actual, "methods.password.fields.#(name==password).value").String(), "%s", actual) - assert.Empty(t, gjson.Get(actual, "methods.password.config.fields.#(name==password).messages.0.text").String(), actual) - - actualIdentity, err := reg.PrivilegedIdentityPool().GetIdentityConfidential(context.Background(), primaryIdentity.ID) - require.NoError(t, err) - cfg := string(actualIdentity.Credentials[identity.CredentialsTypePassword].Config) - assert.NotContains(t, cfg, "foo") - assert.NotEqual(t, `{"hashed_password":"foo"}`, cfg) - }) - }) + t.Run("description=should not be able to make requests for another user", func(t *testing.T) { + t.Run("type=api", func(t *testing.T) { + rs := testhelpers.InitializeSettingsFlowViaAPI(t, apiUser1, publicTS) + f := rs.Payload.Methods[string(identity.CredentialsTypePassword)].Config + values := testhelpers.SDKFormFieldsToURLValues(f.Fields) + values.Set("password", x.NewUUID().String()) + actual, res := testhelpers.SettingsMakeRequest(t, true, f, apiUser2, encodeForm(t, true, values)) + assert.Equal(t, http.StatusBadRequest, res.StatusCode) + assert.Contains(t, gjson.Get(actual, "error.reason").String(), "initiated by another person", "%s", actual) }) + + t.Run("type=browser", func(t *testing.T) { + rs := testhelpers.InitializeSettingsFlowViaBrowser(t, browserUser1, publicTS) + f := rs.Payload.Methods[string(identity.CredentialsTypePassword)].Config + values := testhelpers.SDKFormFieldsToURLValues(f.Fields) + values.Set("password", x.NewUUID().String()) + actual, res := testhelpers.SettingsMakeRequest(t, false, f, browserUser2, values.Encode()) + assert.Equal(t, http.StatusOK, res.StatusCode) + assert.Contains(t, gjson.Get(actual, "0.reason").String(), "initiated by another person", "%s", actual) + }) + }) + + t.Run("description=should update the password and clear errors if everything is ok", func(t *testing.T) { + var run = func(t *testing.T, f *models.FlowMethodConfig, isAPI bool, c *http.Client, id *identity.Identity) { + values := testhelpers.SDKFormFieldsToURLValues(f.Fields) + values.Set("password", x.NewUUID().String()) + actual, _ := testhelpers.SettingsSubmitForm(t, isAPI, f, c, encodeForm(t, isAPI, values), expectStatusCode(isAPI, http.StatusOK, http.StatusNoContent)) + + assert.Equal(t, "success", gjson.Get(actual, "state").String(), "%s", actual) + assert.Empty(t, gjson.Get(actual, "methods.password.fields.#(name==password).value").String(), "%s", actual) + assert.Empty(t, gjson.Get(actual, "methods.password.config.fields.#(name==password).messages.0.text").String(), actual) + + actualIdentity, err := reg.PrivilegedIdentityPool().GetIdentityConfidential(context.Background(), id.ID) + require.NoError(t, err) + assert.NotEqualValues(t, browserIdentity1.Credentials[identity.CredentialsTypePassword].Config, actualIdentity.Credentials[identity.CredentialsTypePassword].Config) + } + + t.Run("type=api", func(t *testing.T) { + rs := testhelpers.InitializeSettingsFlowViaAPI(t, apiUser1, publicTS) + f := rs.Payload.Methods[string(identity.CredentialsTypePassword)].Config + run(t, f, true, apiUser1, apiIdentity1) + }) + + t.Run("type=browser", func(t *testing.T) { + rs := testhelpers.InitializeSettingsFlowViaBrowser(t, browserUser1, publicTS) + f := rs.Payload.Methods[string(identity.CredentialsTypePassword)].Config + run(t, f, false, browserUser1, browserIdentity1) + }) + }) + + t.Run("case=should fail because of missing CSRF token/type=browser", func(t *testing.T) { + rs := testhelpers.InitializeSettingsFlowViaBrowser(t, browserUser1, publicTS) + f := rs.Payload.Methods[string(identity.CredentialsTypePassword)].Config + values := testhelpers.SDKFormFieldsToURLValues(f.Fields) + values.Set("password", x.NewUUID().String()) + values.Set("csrf_token", "invalid_token") + + actual, res := testhelpers.SettingsMakeRequest(t, false, f, browserUser1, values.Encode()) + assert.Equal(t, http.StatusOK, res.StatusCode) + assert.Contains(t, res.Request.URL.String(), viper.Get(configuration.ViperKeySelfServiceErrorUI)) + + assertx.EqualAsJSON(t, x.ErrInvalidCSRFToken, json.RawMessage(gjson.Get(actual, "0").Raw), "%s", actual) + }) + + t.Run("case=should pass even without CSRF token/type=api", func(t *testing.T) { + rs := testhelpers.InitializeSettingsFlowViaAPI(t, apiUser1, publicTS) + f := rs.Payload.Methods[string(identity.CredentialsTypePassword)].Config + values := testhelpers.SDKFormFieldsToURLValues(f.Fields) + values.Set("password", x.NewUUID().String()) + values.Set("csrf_token", "invalid_token") + actual, res := testhelpers.SettingsMakeRequest(t, true, f, apiUser1, encodeForm(t, true, values)) + assert.Equal(t, http.StatusOK, res.StatusCode) + + assert.Contains(t, res.Request.URL.String(), publicTS.URL+password.RouteSettings) + assert.NotEmpty(t, gjson.Get(actual, "identity.id").String(), "%s", actual) }) t.Run("description=should update the password even if no password was set before", func(t *testing.T) { - rs := testhelpers.InitializeSettingsFlowViaBrowser(t, browserUser2, publicTS) - - form := rs.Payload.Methods[string(identity.CredentialsTypePassword)].Config - values := testhelpers.SDKFormFieldsToURLValues(form.Fields) - values.Set("password", randx.MustString(16, randx.AlphaNum)) - actual, _ := testhelpers.SettingsSubmitForm(t, false, form, browserUser2, values.Encode(), http.StatusNoContent) - - assert.Equal(t, "success", gjson.Get(actual, "state").String(), "%s", actual) - assert.Empty(t, gjson.Get(actual, "methods.password.fields.#(name==password).value").String(), "%s", actual) - - actualIdentity, err := reg.PrivilegedIdentityPool().GetIdentityConfidential(context.Background(), secondaryIdentity.ID) - require.NoError(t, err) - cfg := string(actualIdentity.Credentials[identity.CredentialsTypePassword].Config) - assert.Contains(t, cfg, "hashed_password") - require.Len(t, actualIdentity.Credentials[identity.CredentialsTypePassword].Identifiers, 1) - assert.Contains(t, actualIdentity.Credentials[identity.CredentialsTypePassword].Identifiers[0], "-4") + var run = func(t *testing.T, f *models.FlowMethodConfig, isAPI bool, c *http.Client, id *identity.Identity) { + values := testhelpers.SDKFormFieldsToURLValues(f.Fields) + values.Set("password", randx.MustString(16, randx.AlphaNum)) + actual, _ := testhelpers.SettingsSubmitForm(t, isAPI, f, c, encodeForm(t, isAPI, values), + expectStatusCode(isAPI, http.StatusOK, http.StatusNoContent)) + + assert.Equal(t, "success", gjson.Get(actual, "state").String(), "%s", actual) + assert.Empty(t, gjson.Get(actual, "methods.password.fields.#(name==password).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") + require.Len(t, actualIdentity.Credentials[identity.CredentialsTypePassword].Identifiers, 1) + assert.Contains(t, actualIdentity.Credentials[identity.CredentialsTypePassword].Identifiers[0], "-4") + } + + t.Run("type=api", func(t *testing.T) { + rs := testhelpers.InitializeSettingsFlowViaBrowser(t, apiUser2, publicTS) + f := rs.Payload.Methods[string(identity.CredentialsTypePassword)].Config + run(t, f, true, apiUser2, apiIdentity2) + }) + + t.Run("type=browser", func(t *testing.T) { + rs := testhelpers.InitializeSettingsFlowViaBrowser(t, browserUser2, publicTS) + f := rs.Payload.Methods[string(identity.CredentialsTypePassword)].Config + run(t, f, false, browserUser2, browserIdentity2) + }) }) t.Run("description=should update the password and execute hooks", func(t *testing.T) { @@ -183,45 +271,29 @@ func TestSettings(t *testing.T) { viper.Set(configuration.ViperKeySelfServiceSettingsAfter, nil) }) - rs := testhelpers.InitializeSettingsFlowViaBrowser(t, browserUser1, publicTS) + var run = func(t *testing.T, f *models.FlowMethodConfig, isAPI bool, c *http.Client, id *identity.Identity) { + returned = false - form := rs.Payload.Methods[string(identity.CredentialsTypePassword)].Config - values := testhelpers.SDKFormFieldsToURLValues(form.Fields) - values.Set("password", randx.MustString(16, randx.AlphaNum)) + values := testhelpers.SDKFormFieldsToURLValues(f.Fields) + values.Set("password", randx.MustString(16, randx.AlphaNum)) + _, _ = testhelpers.SettingsMakeRequest(t, isAPI, f, c, encodeForm(t, isAPI, values)) - res, err := browserUser1.PostForm(pointerx.StringR(form.Action), values) - require.NoError(t, err) - defer res.Body.Close() + assert.True(t, returned) + actualIdentity, err := reg.PrivilegedIdentityPool().GetIdentityConfidential(context.Background(), browserIdentity1.ID) + require.NoError(t, err) + cfg := string(actualIdentity.Credentials[identity.CredentialsTypePassword].Config) + assert.NotContains(t, cfg, "foo") + assert.NotEqual(t, `{"hashed_password":"foo"}`, cfg) + } - assert.True(t, returned) + t.Run("type=api", func(t *testing.T) { + t.Skip("Post-registration redirects do not work for API flows and are thus not tested here.") + }) - actualIdentity, err := reg.PrivilegedIdentityPool().GetIdentityConfidential(context.Background(), primaryIdentity.ID) - require.NoError(t, err) - cfg := string(actualIdentity.Credentials[identity.CredentialsTypePassword].Config) - assert.NotContains(t, cfg, "foo") - assert.NotEqual(t, `{"hashed_password":"foo"}`, cfg) + t.Run("type=browser", func(t *testing.T) { + rs := testhelpers.InitializeSettingsFlowViaBrowser(t, browserUser1, publicTS) + form := rs.Payload.Methods[string(identity.CredentialsTypePassword)].Config + run(t, form, false, browserUser1, browserIdentity1) + }) }) - - // t.Run("case=should fail because of missing CSRF token/type=browser", func(t *testing.T) { - // rr := newRegistrationRequest(t, time.Minute, false) - // body, _ := makeRequest(t, rr.ID, false, url.Values{ - // "csrf_token": {"invalid_token"}, - // "traits.username": {"registration-identifier-csrf-browser"}, - // "password": {x.NewUUID().String()}, - // "traits.foobar": {"bar"}, - // }.Encode(), http.StatusOK) - // assertx.EqualAsJSON(t, x.ErrInvalidCSRFToken, - // json.RawMessage(gjson.GetBytes(body, "0").Raw), "%s", body) - // }) - // - // t.Run("case=should pass even without CSRF token/type=api", func(t *testing.T) { - // rr := newRegistrationRequest(t, time.Minute, true) - // body, _ := makeRequest(t, rr.ID, true, `{ - // "csrf_token": "invalid_token", - // "traits.username": "registration-identifier-csrf-api", - // "traits.foobar": "bar", - // "password": "5216f2ef-f14b-4c92-bd91-08c2c2fe1448" - // }`, http.StatusOK) - // assert.NotEmpty(t, gjson.GetBytes(body, "identity.id").Raw, "%s", body) // registration successful - // }) } diff --git a/selfservice/strategy/password/strategy_test.go b/selfservice/strategy/password/strategy_test.go index 742d0d23389..a9eadad9ae5 100644 --- a/selfservice/strategy/password/strategy_test.go +++ b/selfservice/strategy/password/strategy_test.go @@ -20,10 +20,10 @@ import ( ) func expectStatusCodeBrowserOKOr(isAPI bool, apiExpect int) int { - return expectStatusCodeBetter(isAPI, apiExpect, http.StatusOK) + return expectStatusCode(isAPI, apiExpect, http.StatusOK) } -func expectStatusCodeBetter(isAPI bool, api, browser int) int { +func expectStatusCode(isAPI bool, api, browser int) int { if isAPI { return api }