refactor: improve selfservice method tests

cmd/jsonnet_format.go

@@ -50,7 +50,7 @@ Use -w or --write to write output back to files instead of stdout.
 				cmdx.Must(err, `JSONNet file "%s" could not be formatted: %s`, file, err)
 
 				if shouldWrite {
-					err := ioutil.WriteFile(file, []byte(output), 0644)
+					err := ioutil.WriteFile(file, []byte(output), 0644) // #nosec
 					cmdx.Must(err, `Could not write to file "%s" because: %s`, file, err)
 				} else {
 					fmt.Println(output)

courier/persistence.go

@@ -45,10 +45,9 @@ func TestPersister(p Persister) func(t *testing.T) {
 
 		messages := make([]Message, 5)
 		t.Run("case=add messages to the queue", func(t *testing.T) {
-			for k, m := range messages {
-				require.NoError(t, faker.FakeData(&m))
-				require.NoError(t, p.AddMessage(context.Background(), &m))
-				messages[k] = m
+			for k := range messages {
+				require.NoError(t, faker.FakeData(&messages[k]))
+				require.NoError(t, p.AddMessage(context.Background(), &messages[k]))
 				time.Sleep(time.Second) // wait a bit so that the timestamp ordering works in MySQL.
 			}
 		})

courier/template/load_template.go

@@ -12,7 +12,7 @@ import (
 	"github.com/pkg/errors"
 )
 
-var templates = pkger.Dir("/courier/template/templates")
+var _ = pkger.Dir("/courier/template/templates")
 
 var cache, _ = lru.New(16)
 

internal/testhelpers/selfservice_login.go

@@ -35,6 +35,15 @@ func NewLoginUIFlowEchoServer(t *testing.T, reg driver.Registry) *httptest.Serve
 	return ts
 }
 
+func NewLoginUIWith401Response(t *testing.T) *httptest.Server {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusUnauthorized)
+	}))
+	viper.Set(configuration.ViperKeySelfServiceLoginUI, ts.URL+"/login-ts")
+	t.Cleanup(ts.Close)
+	return ts
+}
+
 func InitializeLoginFlowViaBrowser(t *testing.T, client *http.Client, ts *httptest.Server, forced bool) *common.GetSelfServiceLoginFlowOK {
 	publicClient := NewSDKClient(ts)
 

internal/testhelpers/selfservice_settings.go

@@ -4,7 +4,6 @@ package testhelpers
 import (
 	"bytes"
 	"context"
-	"encoding/json"
 	"net/http"
 	"net/url"
 	"strings"
@@ -30,8 +29,6 @@ import (
 	"github.com/ory/kratos/internal/httpclient/client/common"
 	"github.com/ory/kratos/internal/httpclient/models"
 	"github.com/ory/kratos/selfservice/flow/settings"
-	"github.com/ory/kratos/selfservice/strategy/password"
-	"github.com/ory/kratos/selfservice/strategy/profile"
 	"github.com/ory/kratos/x"
 )
 
@@ -251,10 +248,18 @@ func SubmitSettingsForm(
 	isAPI bool,
 	hc *http.Client,
 	publicTS *httptest.Server,
-	withValues func(v url.Values) url.Values,
+	withValues func(v url.Values),
 	method string,
 	expectedStatusCode int,
+	expectedURL string,
 ) string {
+	if hc == nil {
+		hc = new(http.Client)
+		if !isAPI {
+			hc = NewClientWithCookies(t)
+		}
+	}
+
 	hc.Transport = NewTransportWithLogger(hc.Transport, t)
 	var payload *models.SettingsFlow
 	if isAPI {
@@ -263,38 +268,15 @@ func SubmitSettingsForm(
 		payload = InitializeSettingsFlowViaBrowser(t, hc, publicTS).Payload
 	}
 
-	time.Sleep(time.Millisecond) // add a bit of delay to allow `1ns` to time out.
+	time.Sleep(time.Millisecond * 10) // add a bit of delay to allow `1ns` to time out.
 
 	config := GetSettingsFlowMethodConfig(t, payload, method)
+	values := SDKFormFieldsToURLValues(config.Fields)
+	withValues(values)
 
-	b, res := SettingsMakeRequest(t, isAPI, config, hc, EncodeFormAsJSON(t, isAPI,
-		withValues(SDKFormFieldsToURLValues(config.Fields))))
+	b, res := SettingsMakeRequest(t, isAPI, config, hc, EncodeFormAsJSON(t, isAPI, values))
 	assert.EqualValues(t, expectedStatusCode, res.StatusCode, "%s", b)
+	assert.Contains(t, res.Request.URL.String(), expectedURL, "%+v\n\t%s", res.Request, b)
 
-	expectURL := viper.GetString(configuration.ViperKeySelfServiceSettingsURL)
-	if isAPI {
-		switch method {
-		case string(identity.CredentialsTypePassword):
-			expectURL = password.RouteSettings
-		case settings.StrategyProfile:
-			expectURL = profile.RouteSettings
-		default:
-			t.Logf("Expected method to be profile ior password but got: %s", method)
-			t.FailNow()
-		}
-	}
-
-	assert.Contains(t, res.Request.URL.String(), expectURL, "%+v\n\t%s", res.Request, b)
-
-	if isAPI {
-		return b
-	}
-
-	rs, err := NewSDKClientFromURL(viper.GetString(configuration.ViperKeyPublicBaseURL)).Common.GetSelfServiceSettingsFlow(
-		common.NewGetSelfServiceSettingsFlowParams().WithHTTPClient(hc).WithID(res.Request.URL.Query().Get("flow")))
-	require.NoError(t, err)
-
-	body, err := json.Marshal(rs.Payload)
-	require.NoError(t, err)
-	return string(body)
+	return b
 }

persistence/sql/migratest/migration_refresh_test.go

@@ -3,16 +3,16 @@
 package migratest
 
 import (
-	"testing"
 	"encoding/json"
 	"io/ioutil"
+	"testing"
 
 	"github.com/stretchr/testify/require"
 )
 
 func writeFixtureOnError(t *testing.T, err error, actual interface{}, location string) {
-		content, err := json.MarshalIndent(actual, "", "  ")
-		require.NoError(t, err)
-		require.NoError(t, ioutil.WriteFile(location, content, 0666))
+	content, err := json.MarshalIndent(actual, "", "  ")
+	require.NoError(t, err)
+	require.NoError(t, ioutil.WriteFile(location, content, 0666))
 
 }

persistence/sql/persister_identity.go

@@ -96,7 +96,8 @@ func findOrCreateIdentityCredentialsType(_ context.Context, tx *pop.Connection,
 }
 
 func createIdentityCredentials(ctx context.Context, tx *pop.Connection, i *identity.Identity) error {
-	for k, cred := range i.Credentials {
+	for k := range i.Credentials {
+		cred := i.Credentials[k]
 		cred.IdentityID = i.ID
 		if len(cred.Config) == 0 {
 			cred.Config = sqlxx.JSONRawMessage("{}")

selfservice/flow/recovery/error.go

@@ -7,6 +7,8 @@ import (
 
 	"github.com/pkg/errors"
 
+	"github.com/ory/x/sqlxx"
+
 	"github.com/ory/herodot"
 	"github.com/ory/x/urlx"
 
@@ -119,6 +121,7 @@ func (s *ErrorHandler) WriteFlowError(
 		return
 	}
 
+	f.Active = sqlxx.NullString(methodName)
 	if err := s.d.RecoveryFlowPersister().UpdateRecoveryFlow(r.Context(), f); err != nil {
 		s.forward(w, r, f, err)
 		return

selfservice/flow/recovery/flow.go

@@ -118,10 +118,6 @@ func (f Flow) TableName() string {
 	return "selfservice_recovery_flows"
 }
 
-func (f *Flow) URL(recoveryURL *url.URL) *url.URL {
-	return urlx.CopyWithQuery(recoveryURL, url.Values{"request": {f.ID.String()}})
-}
-
 func (f *Flow) GetID() uuid.UUID {
 	return f.ID
 }

selfservice/flow/settings/hook.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 
 	"github.com/ory/kratos/driver/configuration"
 	"github.com/ory/kratos/identity"
@@ -64,6 +65,14 @@ func PostHookPostPersistExecutorNames(e []PostHookPostPersistExecutor) []string
 	return names
 }
 
+func PostHookPrePersistExecutorNames(e []PostHookPrePersistExecutor) []string {
+	names := make([]string, len(e))
+	for k, ee := range e {
+		names[k] = fmt.Sprintf("%T", ee)
+	}
+	return names
+}
+
 func NewHookExecutor(
 	d executorDependencies,
 	c configuration.Provider,
@@ -99,28 +108,24 @@ func (e *HookExecutor) PostSettingsHook(w http.ResponseWriter, r *http.Request,
 	}
 
 	for k, executor := range e.d.PostSettingsPrePersistHooks(settingsType) {
+		logFields := logrus.Fields{
+			"executor":          fmt.Sprintf("%T", executor),
+			"executor_position": k,
+			"executors":         PostHookPrePersistExecutorNames(e.d.PostSettingsPrePersistHooks(settingsType)),
+			"identity_id":       i.ID,
+			"flow_method":       settingsType,
+		}
+
 		if err := executor.ExecuteSettingsPrePersistHook(w, r, ctxUpdate.Flow, i); err != nil {
 			if errors.Is(err, ErrHookAbortRequest) {
-				e.d.Logger().
-					WithRequest(r).
-					WithField("executor", fmt.Sprintf("%T", executor)).
-					WithField("executor_position", k).
-					WithField("executors", PostHookPostPersistExecutorNames(e.d.PostSettingsPostPersistHooks(settingsType))).
-					WithField("identity_id", i.ID).
-					WithField("flow_method", settingsType).
+				e.d.Logger().WithRequest(r).WithFields(logFields).
 					Debug("A ExecuteSettingsPrePersistHook hook aborted early.")
 				return nil
 			}
 			return err
 		}
 
-		e.d.Logger().WithRequest(r).
-			WithField("executor", fmt.Sprintf("%T", executor)).
-			WithField("executor_position", k).
-			WithField("executors", PostHookPostPersistExecutorNames(e.d.PostSettingsPostPersistHooks(settingsType))).
-			WithField("identity_id", i.ID).
-			WithField("flow_method", settingsType).
-			Debug("ExecuteSettingsPrePersistHook completed successfully.")
+		e.d.Logger().WithRequest(r).WithFields(logFields).Debug("ExecuteSettingsPrePersistHook completed successfully.")
 	}
 
 	options := []identity.ManagerOption{identity.ManagerExposeValidationErrorsForInternalTypeAssertion}

selfservice/form/html_form.go

@@ -162,8 +162,8 @@ func (c *HTMLForm) ParseError(err error) error {
 		return err
 	} else if e := new(schema.ValidationError); errors.As(err, &e) {
 		pointer, _ := jsonschemax.JSONPointerToDotNotation(e.InstancePtr)
-		for _, message := range e.Messages {
-			c.AddMessage(&message, pointer)
+		for i := range e.Messages {
+			c.AddMessage(&e.Messages[i], pointer)
 		}
 		return nil
 	} else if e := new(jsonschema.ValidationError); errors.As(err, &e) {

selfservice/strategy/oidc/provider_config.go

@@ -66,7 +66,8 @@ type ConfigurationCollection struct {
 }
 
 func (c ConfigurationCollection) Provider(id string, public *url.URL) (Provider, error) {
-	for _, p := range c.Providers {
+	for k := range c.Providers {
+		p := c.Providers[k]
 		if p.ID == id {
 			switch p.Provider {
 			case "generic":

selfservice/strategy/password/login.go

@@ -89,7 +89,7 @@ type completeSelfServiceLoginFlowWithPasswordMethod struct {
 //     Responses:
 //       200: loginViaApiResponse
 //       302: emptyResponse
-//       400: genericError
+//       400: loginFlow
 //       500: genericError
 func (s *Strategy) handleLogin(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
 	rid := x.ParseUUID(r.URL.Query().Get("flow"))

selfservice/strategy/password/registration.go

@@ -74,28 +74,20 @@ func (s *Strategy) handleRegistrationError(w http.ResponseWriter, r *http.Reques
 }
 
 func (s *Strategy) decode(p *RegistrationFormPayload, r *http.Request) error {
-	option, err := s.decoderRegistration()
-	if err != nil {
-		return err
-	}
-
-	return s.hd.Decode(r, p, option, decoderx.HTTPDecoderSetValidatePayloads(false), decoderx.HTTPDecoderJSONFollowsFormFormat())
-}
-
-func (s *Strategy) decoderRegistration() (decoderx.HTTPDecoderOption, error) {
 	raw, err := sjson.SetBytes(registrationSchema, "properties.traits.$ref", s.c.DefaultIdentityTraitsSchemaURL().String()+"#/properties/traits")
 	if err != nil {
-		return nil, errors.WithStack(err)
+		return errors.WithStack(err)
 	}
 
-	o, err := decoderx.HTTPRawJSONSchemaCompiler(raw)
+	compiler, err := decoderx.HTTPRawJSONSchemaCompiler(raw)
 	if err != nil {
-		return nil, errors.WithStack(err)
+		return errors.WithStack(err)
 	}
 
-	return o, nil
+	return s.hd.Decode(r, p, compiler, decoderx.HTTPDecoderSetValidatePayloads(false), decoderx.HTTPDecoderJSONFollowsFormFormat())
 }
 
+// nolint:deadcode,unused
 // swagger:parameters completeSelfServiceRegistrationFlowWithPasswordMethod
 type completeSelfServiceRegistrationFlowWithPasswordMethod struct {
 	// Flow is flow ID.
@@ -114,7 +106,7 @@ type completeSelfServiceRegistrationFlowWithPasswordMethod struct {
 // Use this endpoint to complete a registration flow by sending an identity's traits and password. This endpoint
 // behaves differently for API and browser flows.
 //
-// API flows expect `application/json` to be sent in the body and responds with
+// API flows expect `application/json` to be sent in the body and respond with
 //   - HTTP 200 and a application/json body with the created identity success - if the session hook is configured the
 //     `session` and `session_token` will also be included;
 //   - HTTP 302 redirect to a fresh registration flow if the original flow expired with the appropriate error messages set;
@@ -138,7 +130,7 @@ type completeSelfServiceRegistrationFlowWithPasswordMethod struct {
 //     Responses:
 //       200: registrationViaApiResponse
 //       302: emptyResponse
-//       400: genericError
+//       400: registrationFlow
 //       500: genericError
 func (s *Strategy) handleRegistration(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
 	rid := x.ParseUUID(r.URL.Query().Get("flow"))

selfservice/strategy/password/settings.go

@@ -36,6 +36,7 @@ func (s *Strategy) SettingsStrategyID() string {
 	return identity.CredentialsTypePassword.String()
 }
 
+// nolint:deadcode,unused
 // swagger:parameters completeSelfServiceSettingsFlowWithPasswordMethod
 type completeSelfServiceSettingsFlowWithPasswordMethod struct {
 	// in: body
@@ -75,23 +76,44 @@ func (p *SettingsFlowPayload) SetFlowID(rid uuid.UUID) {
 
 // swagger:route POST /self-service/settings/methods/password public completeSelfServiceSettingsFlowWithPasswordMethod
 //
-// Complete the browser-based settings flow for the password strategy
+// Complete Settings Flow with Username/Email Password Method
 //
-// This endpoint completes a browser-based settings flow. This is usually achieved by POSTing data to this
-// endpoint.
+// Use this endpoint to complete a settings flow by sending an identity's updated password. This endpoint
+// behaves differently for API and browser flows.
 //
-// > This endpoint is NOT INTENDED for API clients and only works with browsers (Chrome, Firefox, ...) and HTML Forms.
+// API-initiated flows expect `application/json` to be sent in the body and respond with
+//   - HTTP 200 and an application/json body with the session token on success;
+//   - HTTP 302 redirect to a fresh settings flow if the original flow expired with the appropriate error messages set;
+//   - HTTP 400 on form validation errors.
+//   - HTTP 401 when the endpoint is called without a valid session token.
+//   - HTTP 403 when `selfservice.flows.settings.privileged_session_max_age` was reached.
+//     Implies that the user needs to re-authenticate.
+//
+// Browser flows expect `application/x-www-form-urlencoded` to be sent in the body and responds with
+//   - a HTTP 302 redirect to the post/after settings URL or the `return_to` value if it was set and if the flow succeeded;
+//   - a HTTP 302 redirect to the Settings UI URL with the flow ID containing the validation errors otherwise.
+//   - a HTTP 302 redirect to the login endpoint when `selfservice.flows.settings.privileged_session_max_age` was reached.
 //
 // More information can be found at [ORY Kratos User Settings & Profile Management Documentation](../self-service/flows/user-settings).
 //
 //     Consumes:
 //     - application/json
 //     - application/x-www-form-urlencoded
 //
+//     Produces:
+//     - application/json
+//
+//     Security:
+//     - sessionToken
+//
 //     Schemes: http, https
 //
 //     Responses:
+//       200: settingsViaApiResponse
 //       302: emptyResponse
+//       400: settingsFlow
+//       401: genericError
+//       403: genericError
 //       500: genericError
 func (s *Strategy) submitSettingsFlow(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
 	var p SettingsFlowPayload