ss/password: Make form fields an array (#197)

Closes #186

docs/api.swagger.json

@@ -269,6 +269,7 @@
  "parameters": [
  {
  "type": "string",
+ "x-go-name": "Request",
  "description": "Request should be set to the value of the `request` query parameter\nby the profile management UI.",
  "name": "request",
  "in": "query",
@@ -416,8 +417,8 @@
  },
  "formFields": {
  "description": "Fields contains multiple fields",
- "type": "object",
- "additionalProperties": {
+ "type": "array",
+ "items": {
  "$ref": "#/definitions/formField"
  }
  },

selfservice/flow/login/handler_test.go

@@ -107,7 +107,7 @@ func TestLoginHandler(t *testing.T) {
  require.NoError(t, err)
 
  assert.Equal(t, "password", gjson.GetBytes(body, "methods.password.method").String(), "%s", body)
- assert.NotEmpty(t, gjson.GetBytes(body, "methods.password.config.fields.csrf_token.value").String(), "%s", body)
+ assert.NotEmpty(t, gjson.GetBytes(body, "methods.password.config.fields.#(name==csrf_token).value").String(), "%s", body)
  assert.NotEmpty(t, gjson.GetBytes(body, "id").String(), "%s", body)
  assert.Empty(t, gjson.GetBytes(body, "headers").Value(), "%s", body)
  assert.Contains(t, gjson.GetBytes(body, "methods.password.config.action").String(), gjson.GetBytes(body, "id").String(), "%s", body)

selfservice/flow/profile/handler.go

@@ -4,6 +4,7 @@ import (
  "encoding/json"
  "net/http"
  "net/url"
+ "sort"
 
  "github.com/ory/kratos/schema"
 
@@ -198,6 +199,7 @@ func (h *Handler) fetchUpdateProfileRequest(w http.ResponseWriter, r *http.Reque
  Value: rid,
  })
  ar.Form.SetCSRF(nosurf.Token(r))
+ sort.Sort(ar.Form.Fields)
  h.d.Writer().Write(w, r, ar)
 }
 
@@ -341,11 +343,12 @@ func (h *Handler) completeProfileManagementFlow(w http.ResponseWriter, r *http.R
 
  ar.Form.Reset()
  ar.UpdateSuccessful = true
- for name, field := range form.NewHTMLFormFromJSON("", json.RawMessage(i.Traits), "traits").Fields {
- ar.Form.SetField(name, field)
+ for _, field := range form.NewHTMLFormFromJSON("", json.RawMessage(i.Traits), "traits").Fields {
+ ar.Form.SetField(field.Name, field)
  }
  ar.Form.SetValue("request", r.Form.Get("request"))
  ar.Form.SetCSRF(nosurf.Token(r))
+ sort.Sort(ar.Form.Fields)
 
  if err := h.d.ProfileRequestPersister().UpdateProfileRequest(r.Context(), ar); err != nil {
  h.handleProfileManagementError(w, r, ar, i.Traits, err)
@@ -366,12 +369,13 @@ func (h *Handler) handleProfileManagementError(w http.ResponseWriter, r *http.Re
  rr.UpdateSuccessful = false
 
  if traits != nil {
- for name, field := range form.NewHTMLFormFromJSON("", json.RawMessage(traits), "traits").Fields {
- rr.Form.SetField(name, field)
+ for _, field := range form.NewHTMLFormFromJSON("", json.RawMessage(traits), "traits").Fields {
+ rr.Form.SetField(field.Name, field)
  }
  }
  rr.Form.SetValue("request", r.Form.Get("request"))
  rr.Form.SetCSRF(nosurf.Token(r))
+ sort.Sort(rr.Form.Fields)
  }
 
  h.d.ProfileRequestRequestErrorHandler().HandleProfileManagementError(w, r, identity.CredentialsTypePassword, rr, err)

selfservice/flow/profile/handler_test.go

@@ -205,17 +205,26 @@ func TestUpdateProfile(t *testing.T) {
  assert.Equal(t, primaryIdentity.TraitsSchemaID, pr.Payload.Identity.TraitsSchemaID)
  assert.Equal(t, kratos.URL+profile.BrowserProfilePath, pr.Payload.RequestURL)
 
- require.NotEmpty(t, pr.Payload.Form.Fields[form.CSRFTokenName])
- delete(pr.Payload.Form.Fields, form.CSRFTokenName)
+ found := false
+ for i := range pr.Payload.Form.Fields {
+ if pr.Payload.Form.Fields[i].Name == form.CSRFTokenName {
+ found = true
+ require.NotEmpty(t, pr.Payload.Form.Fields[i])
+ pr.Payload.Form.Fields = append(pr.Payload.Form.Fields[:i], pr.Payload.Form.Fields[i+1:]...)
+ break
+ }
+ }
+ require.True(t, found)
+
  assert.Equal(t, &models.Form{
  Action: kratos.URL + profile.BrowserProfilePath,
  Method: "POST",
  Fields: models.FormFields{
- "request": models.FormField{Name: "request", Required: true, Type: "hidden", Value: rid},
- "traits.stringy": models.FormField{Name: "traits.stringy", Required: false, Type: "text", Value: "foobar"},
- "traits.numby": models.FormField{Name: "traits.numby", Required: false, Type: "number", Value: json.Number("2.5")},
- "traits.booly": models.FormField{Name: "traits.booly", Required: false, Type: "checkbox", Value: false},
- "traits.email": models.FormField{Name: "traits.email", Required: false, Type: "text", Value: "[email protected]"},
+ &models.FormField{Name: "request", Required: true, Type: "hidden", Value: rid},
+ &models.FormField{Name: "traits.booly", Required: false, Type: "checkbox", Value: false},
+ &models.FormField{Name: "traits.email", Required: false, Type: "text", Value: "[email protected]"},
+ &models.FormField{Name: "traits.numby", Required: false, Type: "number", Value: json.Number("2.5")},
+ &models.FormField{Name: "traits.stringy", Required: false, Type: "text", Value: "foobar"},
  },
  }, pr.Payload.Form)
  })
@@ -245,11 +254,11 @@ func TestUpdateProfile(t *testing.T) {
  values.Set("traits.stringy", "bazbar") // it should still override new values!
  actual, _ := submitForm(t, rs, values)
 
- assert.NotEmpty(t, "too-short", gjson.Get(actual, "form.fields.csrf_token.value").String(), "%s", actual)
- assert.Equal(t, "too-short", gjson.Get(actual, "form.fields.traits\\.should_long_string.value").String(), "%s", actual)
- assert.Equal(t, "bazbar", gjson.Get(actual, "form.fields.traits\\.stringy.value").String(), "%s", actual)
- assert.Equal(t, "2.5", gjson.Get(actual, "form.fields.traits\\.numby.value").String(), "%s", actual)
- assert.Equal(t, "traits.should_long_string: String length must be greater than or equal to 25", gjson.Get(actual, "form.fields.traits\\.should_long_string.errors.0.message").String(), "%s", actual)
+ assert.NotEmpty(t, gjson.Get(actual, "form.fields.#(name==csrf_token).value").String(), "%s", actual)
+ assert.Equal(t, "too-short", gjson.Get(actual, "form.fields.#(name==traits.should_long_string).value").String(), "%s", actual)
+ assert.Equal(t, "bazbar", gjson.Get(actual, "form.fields.#(name==traits.stringy).value").String(), "%s", actual)
+ assert.Equal(t, "2.5", gjson.Get(actual, "form.fields.#(name==traits.numby).value").String(), "%s", actual)
+ assert.Equal(t, "traits.should_long_string: String length must be greater than or equal to 25", gjson.Get(actual, "form.fields.#(name==traits.should_long_string).errors.0.message").String(), "%s", actual)
  })
 
  t.Run("description=should come back with form errors if trying to update email", func(t *testing.T) {
@@ -276,10 +285,10 @@ func TestUpdateProfile(t *testing.T) {
  actual, response := submitForm(t, rs, values)
  assert.False(t, response.Payload.UpdateSuccessful, "%s", actual)
 
- assert.Equal(t, "1", gjson.Get(actual, "form.fields.traits\\.should_big_number.value").String(), "%s", actual)
- assert.Equal(t, "traits.should_big_number: Must be greater than or equal to 1200", gjson.Get(actual, "form.fields.traits\\.should_big_number.errors.0.message").String(), "%s", actual)
+ assert.Equal(t, "1", gjson.Get(actual, "form.fields.#(name==traits.should_big_number).value").String(), "%s", actual)
+ assert.Equal(t, "traits.should_big_number: Must be greater than or equal to 1200", gjson.Get(actual, "form.fields.#(name==traits.should_big_number).errors.0.message").String(), "%s", actual)
 
- assert.Equal(t, "foobar", gjson.Get(actual, "form.fields.traits\\.stringy.value").String(), "%s", actual) // sanity check if original payload is still here
+ assert.Equal(t, "foobar", gjson.Get(actual, "form.fields.#(name==traits.stringy).value").String(), "%s", actual) // sanity check if original payload is still here
  })
 
  t.Run("flow=fail second update", func(t *testing.T) {
@@ -291,16 +300,16 @@ func TestUpdateProfile(t *testing.T) {
  actual, response := submitForm(t, rs, values)
  assert.False(t, response.Payload.UpdateSuccessful, "%s", actual)
 
- assert.Empty(t, gjson.Get(actual, "form.fields.traits\\.should_big_number.errors.0.message").String(), "%s", actual)
- assert.Empty(t, gjson.Get(actual, "form.fields.traits\\.should_big_number.value").String(), "%s", actual)
+ assert.Empty(t, gjson.Get(actual, "form.fields.#(name==traits.should_big_number).errors.0.message").String(), "%s", actual)
+ assert.Empty(t, gjson.Get(actual, "form.fields.#(name==traits.should_big_number).value").String(), "%s", actual)
 
- assert.Equal(t, "short", gjson.Get(actual, "form.fields.traits\\.should_long_string.value").String(), "%s", actual)
- assert.Equal(t, "traits.should_long_string: String length must be greater than or equal to 25", gjson.Get(actual, "form.fields.traits\\.should_long_string.errors.0.message").String(), "%s", actual)
+ assert.Equal(t, "short", gjson.Get(actual, "form.fields.#(name==traits.should_long_string).value").String(), "%s", actual)
+ assert.Equal(t, "traits.should_long_string: String length must be greater than or equal to 25", gjson.Get(actual, "form.fields.#(name==traits.should_long_string).errors.0.message").String(), "%s", actual)
 
- assert.Equal(t, "this-is-not-a-number", gjson.Get(actual, "form.fields.traits\\.numby.value").String(), "%s", actual)
- assert.Equal(t, "traits.numby: Invalid type. Expected: number, given: string", gjson.Get(actual, "form.fields.traits\\.numby.errors.0.message").String(), "%s", actual)
+ assert.Equal(t, "this-is-not-a-number", gjson.Get(actual, "form.fields.#(name==traits.numby).value").String(), "%s", actual)
+ assert.Equal(t, "traits.numby: Invalid type. Expected: number, given: string", gjson.Get(actual, "form.fields.#(name==traits.numby).errors.0.message").String(), "%s", actual)
 
- assert.Equal(t, "foobar", gjson.Get(actual, "form.fields.traits\\.stringy.value").String(), "%s", actual) // sanity check if original payload is still here
+ assert.Equal(t, "foobar", gjson.Get(actual, "form.fields.#(name==traits.stringy).value").String(), "%s", actual) // sanity check if original payload is still here
  })
 
  t.Run("flow=succeed with final request", func(t *testing.T) {
@@ -313,15 +322,15 @@ func TestUpdateProfile(t *testing.T) {
  actual, response := submitForm(t, rs, values)
  assert.True(t, response.Payload.UpdateSuccessful, "%s", actual)
 
- assert.Empty(t, gjson.Get(actual, "form.fields.traits\\.numby.errors").Value(), "%s", actual)
- assert.Empty(t, gjson.Get(actual, "form.fields.traits\\.should_big_number.errors").Value(), "%s", actual)
- assert.Empty(t, gjson.Get(actual, "form.fields.traits\\.should_long_string.errors").Value(), "%s", actual)
+ assert.Empty(t, gjson.Get(actual, "form.fields.#(name==traits.numby).errors").Value(), "%s", actual)
+ assert.Empty(t, gjson.Get(actual, "form.fields.#(name==traits.should_big_number).errors").Value(), "%s", actual)
+ assert.Empty(t, gjson.Get(actual, "form.fields.#(name==traits.should_long_string).errors").Value(), "%s", actual)
 
- assert.Equal(t, 15.0, gjson.Get(actual, "form.fields.traits\\.numby.value").Value(), "%s", actual)
- assert.Equal(t, 9001.0, gjson.Get(actual, "form.fields.traits\\.should_big_number.value").Value(), "%s", actual)
- assert.Equal(t, "this is such a long string, amazing stuff!", gjson.Get(actual, "form.fields.traits\\.should_long_string.value").Value(), "%s", actual)
+ assert.Equal(t, 15.0, gjson.Get(actual, "form.fields.#(name==traits.numby).value").Value(), "%s", actual)
+ assert.Equal(t, 9001.0, gjson.Get(actual, "form.fields.#(name==traits.should_big_number).value").Value(), "%s", actual)
+ assert.Equal(t, "this is such a long string, amazing stuff!", gjson.Get(actual, "form.fields.#(name==traits.should_long_string).value").Value(), "%s", actual)
 
- assert.Equal(t, "foobar", gjson.Get(actual, "form.fields.traits\\.stringy.value").String(), "%s", actual) // sanity check if original payload is still here
+ assert.Equal(t, "foobar", gjson.Get(actual, "form.fields.#(name==traits.stringy).value").String(), "%s", actual) // sanity check if original payload is still here
  })
 
  t.Run("flow=try another update with invalid data", func(t *testing.T) {

selfservice/flow/registration/handler_test.go

@@ -109,7 +109,7 @@ func TestRegistrationHandler(t *testing.T) {
  require.NoError(t, err)
 
  assert.Equal(t, "password", gjson.GetBytes(body, "methods.password.method").String(), "%s", body)
- assert.NotEmpty(t, gjson.GetBytes(body, "methods.password.config.fields.csrf_token.value").String(), "%s", body)
+ assert.NotEmpty(t, gjson.GetBytes(body, "methods.password.config.fields.#(name==csrf_token).value").String(), "%s", body)
  assert.NotEmpty(t, gjson.GetBytes(body, "id").String(), "%s", body)
  assert.Empty(t, gjson.GetBytes(body, "headers").Value(), "%s", body)
  assert.Contains(t, gjson.GetBytes(body, "methods.password.config.action").String(), gjson.GetBytes(body, "id").String(), "%s", body)

selfservice/form/fields.go

@@ -8,7 +8,7 @@ import (
 // Fields contains multiple fields
 //
 // swagger:model formFields
-type Fields map[string]Field
+type Fields []Field
 
 // Field represents a HTML Form Field
 //
@@ -32,6 +32,18 @@ func (f *Field) Reset() {
  f.Value = nil
 }
 
+func (ff Fields) Len() int {
+ return len(ff)
+}
+
+func (ff Fields) Swap(i, j int) {
+ ff[i], ff[j] = ff[j], ff[i]
+}
+
+func (ff Fields) Less(i, j int) bool {
+ return ff[i].Name < ff[j].Name
+}
+
 func toFormType(n string, i interface{}) string {
  switch n {
  case CSRFTokenName: