selfservice/password: Remove request field and ensure method is set (#…

…183)

cmd/serve.go

@@ -15,10 +15,11 @@
 package cmd
 
 import (
-	"github.com/ory/x/flagx"
 	"os"
 	"strconv"
 
+	"github.com/ory/x/flagx"
+
 	"github.com/spf13/cobra"
 
 	"github.com/ory/kratos/cmd/daemon"

driver/configuration/provider_viper.go

@@ -24,8 +24,8 @@ import (
 )
 
 type ViperProvider struct {
-	l  logrus.FieldLogger
-	ss [][]byte
+	l   logrus.FieldLogger
+	ss  [][]byte
 	dev bool
 }
 
@@ -74,7 +74,7 @@ const (
 
 func NewViperProvider(l logrus.FieldLogger, dev bool) *ViperProvider {
 	return &ViperProvider{
-		l: l,
+		l:   l,
 		dev: dev,
 	}
 }

go.mod

@@ -10,6 +10,7 @@ require (
 	github.com/cenkalti/backoff v2.1.1+incompatible
 	github.com/coreos/go-oidc v2.1.0+incompatible
 	github.com/fsnotify/fsnotify v1.4.7
+	github.com/ghodss/yaml v1.0.0
 	github.com/go-errors/errors v1.0.1
 	github.com/go-openapi/errors v0.19.2
 	github.com/go-openapi/runtime v0.19.5

selfservice/flow/login/request_method.go

@@ -75,7 +75,7 @@ func (c *RequestMethodConfig) Value() (driver.Value, error) {
 }
 
 func (c *RequestMethodConfig) UnmarshalJSON(data []byte) error {
-	c.RequestMethodConfigurator = new(form.HTMLForm)
+	c.RequestMethodConfigurator = form.NewHTMLForm("")
 	return json.Unmarshal(data, c.RequestMethodConfigurator)
 }
 

selfservice/flow/profile/request.go

@@ -80,7 +80,7 @@ func NewRequest(exp time.Duration, r *http.Request, s *session.Session) *Request
 		RequestURL: source.String(),
 		IdentityID: s.Identity.ID,
 		Identity:   s.Identity,
-		Form:       new(form.HTMLForm),
+		Form:       form.NewHTMLForm(""),
 	}
 }
 

selfservice/flow/registration/request_method.go

@@ -76,7 +76,7 @@ func (c *RequestMethodConfig) Value() (driver.Value, error) {
 }
 
 func (c *RequestMethodConfig) UnmarshalJSON(data []byte) error {
-	c.RequestMethodConfigurator = new(form.HTMLForm)
+	c.RequestMethodConfigurator = form.NewHTMLForm("")
 	return json.Unmarshal(data, c.RequestMethodConfigurator)
 }
 

selfservice/form/fields.go

@@ -5,7 +5,7 @@ import (
 	"fmt"
 )
 
-// Fields contains multiple fields asdfasdf
+// Fields contains multiple fields
 //
 // swagger:model formFields
 type Fields map[string]Field
@@ -15,18 +15,12 @@ type Fields map[string]Field
 // swagger:model formField
 type Field struct {
 	// Name is the equivalent of <input name="{{.Name}}">
-	//
-	// Extensions:
-	// ---
-	// x-go-name: Asdffsdafsad
-	// x-go-asdffasd: Asdffsdafsad
-	// ---
 	Name string `json:"name"`
-	// Name is the equivalent of <input type="{{.Type}}">
+	// Type is the equivalent of <input type="{{.Type}}">
 	Type string `json:"type,omitempty"`
-	// Name is the equivalent of <input required="{{.Required}}">
+	// Required is the equivalent of <input required="{{.Required}}">
 	Required bool `json:"required,omitempty"`
-	// Name is the equivalent of <input value="{{.Value}}">
+	// Value is the equivalent of <input value="{{.Value}}">
 	Value interface{} `json:"value,omitempty" faker:"name"`
 	// Errors contains all validation errors this particular field has caused.
 	Errors []Error `json:"errors,omitempty"`

selfservice/strategy/password/login.go

@@ -35,7 +35,6 @@ func (s *Strategy) handleLoginError(w http.ResponseWriter, r *http.Request, rr *
 		if method, ok := rr.Methods[identity.CredentialsTypePassword]; ok {
 			method.Config.Reset()
 			method.Config.SetValue("identifier", r.PostForm.Get("identifier"))
-			method.Config.SetValue("request", r.PostForm.Get("request"))
 			method.Config.SetCSRF(s.cg(r))
 			rr.Methods[identity.CredentialsTypePassword] = method
 		}
@@ -118,6 +117,7 @@ func (s *Strategy) PopulateLoginMethod(r *http.Request, sr *login.Request) error
 
 	f := &form.HTMLForm{
 		Action: action.String(),
+		Method: "POST",
 		Fields: form.Fields{
 			"identifier": {
 				Name:     "identifier",

selfservice/strategy/password/login_test.go

@@ -44,6 +44,7 @@ func nlr(exp time.Duration) *login.Request {
 				Method: identity.CredentialsTypePassword,
 				Config: &login.RequestMethodConfig{
 					RequestMethodConfigurator: &form.HTMLForm{
+						Method: "POST",
 						Action: "/action",
 						Fields: form.Fields{
 							"identifier": {
@@ -62,12 +63,6 @@ func nlr(exp time.Duration) *login.Request {
 								Required: true,
 								Value:    "anti-rf-token",
 							},
-							"request": {
-								Name:     "request",
-								Type:     "hidden",
-								Required: true,
-								Value:    id.String(),
-							},
 						},
 					},
 				},
@@ -84,12 +79,9 @@ type loginStrategyDependencies interface {
 
 func TestLogin(t *testing.T) {
 	var ensureFieldsExist = func(t *testing.T, body []byte) {
-		for _, k := range []string{"identifier",
+		checkFormContent(t, body, "identifier",
 			"password",
-			"csrf_token",
-		} {
-			assert.Equal(t, k, gjson.GetBytes(body, fmt.Sprintf("methods.password.config.fields.%s.name", k)).String())
-		}
+			"csrf_token")
 	}
 
 	type testCase struct {
@@ -304,6 +296,7 @@ func TestLogin(t *testing.T) {
 						Config: &login.RequestMethodConfig{
 							RequestMethodConfigurator: &password.RequestMethod{
 								HTMLForm: &form.HTMLForm{
+									Method: "POST",
 									Action: "/action",
 									Errors: []form.Error{{Message: "some error"}},
 									Fields: form.Fields{

selfservice/strategy/password/registration.go

@@ -68,14 +68,7 @@ func (s *Strategy) handleRegistrationError(w http.ResponseWriter, r *http.Reques
 				}
 			}
 
-			method.Config.SetField("request", form.Field{
-				Name:     "request",
-				Type:     "hidden",
-				Required: true,
-				Value:    r.PostForm.Get("request"),
-			})
 			method.Config.SetCSRF(s.cg(r))
-
 			rr.Methods[identity.CredentialsTypePassword] = method
 		}
 	}

selfservice/strategy/password/registration_test.go

@@ -30,14 +30,31 @@ import (
 	"github.com/ory/kratos/x"
 )
 
+func checkFormContent(t *testing.T, body []byte, requiredFields ...string) {
+	fieldNameSet(t, body, requiredFields)
+	outdatedFieldsDoNotExist(t, body)
+	formMethodIsPOST(t, body)
+}
+
 // fieldNameSet checks if the fields have the right "name" set.
-func fieldNameSet(t *testing.T, body []byte, fields ...string) {
+func fieldNameSet(t *testing.T, body []byte, fields []string) {
 	for _, f := range fields {
 		fieldid := strings.Replace(f, ".", "\\.", -1) // we need to escape this because otherwise json path will interpret this as a nested object (it is not).
 		assert.Equal(t, gjson.GetBytes(body, fmt.Sprintf("methods.password.config.fields.%s.name", fieldid)).String(), f, "%s", body)
 	}
 }
 
+// checks if some keys are not set, this should be used to catch regression issues
+func outdatedFieldsDoNotExist(t *testing.T, body []byte) {
+	for _, k := range []string{"request"} {
+		assert.Equal(t, false, gjson.GetBytes(body, fmt.Sprintf("methods.password.config.fields.%s", k)).Exists())
+	}
+}
+
+func formMethodIsPOST(t *testing.T, body []byte) {
+	assert.Equal(t, "POST", gjson.GetBytes(body, "methods.password.config.method").String())
+}
+
 func TestRegistration(t *testing.T) {
 	t.Run("case=registration", func(t *testing.T) {
 		_, reg := internal.NewRegistryDefault(t)
@@ -77,6 +94,7 @@ func TestRegistration(t *testing.T) {
 						Config: &registration.RequestMethodConfig{
 							RequestMethodConfigurator: password.RequestMethod{
 								HTMLForm: &form.HTMLForm{
+									Method: "POST",
 									Action: "/action",
 									Fields: form.Fields{
 										"password":   {Name: "password", Type: "password", Required: true},
@@ -141,7 +159,7 @@ func TestRegistration(t *testing.T) {
 			assert.Contains(t, res.Request.URL.Path, "signup-ts")
 			assert.Equal(t, rr.ID.String(), gjson.GetBytes(body, "id").String(), "%s", body)
 			assert.Equal(t, "/action", gjson.GetBytes(body, "methods.password.config.action").String(), "%s", body)
-			fieldNameSet(t, body, "password", "csrf_token", "traits.username", "traits.foobar")
+			checkFormContent(t, body, "password", "csrf_token", "traits.username", "traits.foobar")
 			assert.Contains(t, gjson.GetBytes(body, "methods.password.config.fields.password.errors.0").String(), "data breaches and must no longer be used.", "%s", body)
 		})
 
@@ -154,7 +172,7 @@ func TestRegistration(t *testing.T) {
 			assert.Contains(t, res.Request.URL.Path, "signup-ts")
 			assert.Equal(t, rr.ID.String(), gjson.GetBytes(body, "id").String(), "%s", body)
 			assert.Equal(t, "/action", gjson.GetBytes(body, "methods.password.config.action").String(), "%s", body)
-			fieldNameSet(t, body, "password", "csrf_token", "traits.username", "traits.foobar")
+			checkFormContent(t, body, "password", "csrf_token", "traits.username", "traits.foobar")
 			assert.Contains(t, gjson.GetBytes(body, "methods.password.config.fields.traits\\.foobar.errors.0").String(), "foobar is required", "%s", body)
 		})
 
@@ -222,6 +240,7 @@ func TestRegistration(t *testing.T) {
 						Config: &registration.RequestMethodConfig{
 							RequestMethodConfigurator: &password.RequestMethod{
 								HTMLForm: &form.HTMLForm{
+									Method: "POST",
 									Action: "/action",
 									Errors: []form.Error{{Message: "some error"}},
 									Fields: form.Fields{
@@ -246,7 +265,7 @@ func TestRegistration(t *testing.T) {
 			assert.Contains(t, res.Request.URL.Path, "signup-ts")
 			assert.Equal(t, rr.ID.String(), gjson.GetBytes(body, "id").String(), "%s", body)
 			assert.Equal(t, "/action", gjson.GetBytes(body, "methods.password.config.action").String(), "%s", body)
-			fieldNameSet(t, body, "password", "csrf_token", "traits.username")
+			checkFormContent(t, body, "password", "csrf_token", "traits.username")
 
 			assert.Empty(t, gjson.GetBytes(body, "methods.password.config.fields.traits\\.foo.value"), "%s", body)
 			assert.Empty(t, gjson.GetBytes(body, "methods.password.config.fields.traits\\.foo.error"))