selfservice: Explicitly whitelist form parser keys (#105)

Previously the form parser would try to detect the field type by
asserting types for the whole form. That caused passwords
containing only numbers to fail to unmarshal into a string
value.

This patch resolves that issue by introducing a prefix
option to the BodyParser

Closes #98

selfservice/body_decoder.go

@@ -20,6 +20,10 @@ import (
 
 type BodyDecoder struct{}
 
+type BodyDecoderOptions struct {
+	AssertTypesForPrefix string
+}
+
 func NewBodyDecoder() *BodyDecoder {
 	return &BodyDecoder{}
 }
@@ -33,8 +37,8 @@ func (d BodyDecoder) json(r *http.Request) (json.RawMessage, error) {
 	return p, nil
 }
 
-func (d *BodyDecoder) DecodeForm(form url.Values, o interface{}) (err error) {
-	payload, err := d.form(form)
+func (d *BodyDecoder) DecodeForm(form url.Values, o interface{}, opts BodyDecoderOptions) (err error) {
+	payload, err := d.form(form, opts)
 	if err != nil {
 		return err
 	}
@@ -83,12 +87,16 @@ func (d *BodyDecoder) ParseFormField(values []string) (typed interface{}, err er
 	return typed, err
 }
 
-func (d *BodyDecoder) form(form url.Values) (json.RawMessage, error) {
+func (d *BodyDecoder) form(form url.Values, opts BodyDecoderOptions) (json.RawMessage, error) {
+	var err error
 	payload := []byte("{}")
 	for k := range form {
-		typed, err := d.ParseFormField(form[k])
-		if err != nil {
-			return nil, err
+		var typed interface{} = form.Get(k)
+
+		if len(opts.AssertTypesForPrefix) == 0 || strings.HasPrefix(k, opts.AssertTypesForPrefix) {
+			if typed, err = d.ParseFormField(form[k]); err != nil {
+				return nil, err
+			}
 		}
 
 		payload, err = sjson.SetBytes(payload, k, typed)
@@ -100,7 +108,7 @@ func (d *BodyDecoder) form(form url.Values) (json.RawMessage, error) {
 	return payload, nil
 }
 
-func (d *BodyDecoder) Decode(r *http.Request, o interface{}) (err error) {
+func (d *BodyDecoder) Decode(r *http.Request, o interface{}, opts BodyDecoderOptions) (err error) {
 	ct, _, err := mime.ParseMediaType(r.Header.Get("Content-Type"))
 	if err != nil {
 		return errors.WithStack(herodot.ErrBadRequest.WithDebug(err.Error()).WithReasonf("Unable to parse HTTP request content type: %s", err.Error()))
@@ -113,7 +121,7 @@ func (d *BodyDecoder) Decode(r *http.Request, o interface{}) (err error) {
 		if err := r.ParseForm(); err != nil {
 			return errors.WithStack(herodot.ErrBadRequest.WithDebug(err.Error()).WithReasonf("Unable to parse HTTP form request: %s", err.Error()))
 		}
-		p, err = d.form(r.PostForm)
+		p, err = d.form(r.PostForm, opts)
 	}
 
 	if err != nil {

selfservice/body_decoder_test.go

@@ -22,6 +22,7 @@ func TestNewBodyDecoder(t *testing.T) {
 			payload url.Values
 			raw     string
 			result  string
+			opt     BodyDecoderOptions
 		}{
 			{
 				d: "should work with nested keys",
@@ -31,6 +32,15 @@ func TestNewBodyDecoder(t *testing.T) {
 				},
 				result: `{"request":"bar","traits":{"foo":"bar"}}`,
 			},
+			{
+				d: "should ignore unless prefix is set",
+				payload: url.Values{
+					"traits.foo": {"12345"},
+					"password":   {"12345"},
+				},
+				opt:    BodyDecoderOptions{AssertTypesForPrefix: "traits."},
+				result: `{"password":"12345","traits":{"foo":12345}}`,
+			},
 			{
 				d:      "should work with true and false",
 				raw:    "traits.consent.newsletter=false&traits.consent.newsletter=true&traits.consent.tos=false",
@@ -40,7 +50,7 @@ func TestNewBodyDecoder(t *testing.T) {
 			t.Run(fmt.Sprintf("case=%d/description=%s", k, tc.d), func(t *testing.T) {
 				ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 					var result json.RawMessage
-					require.NoError(t, dec.Decode(r, &result))
+					require.NoError(t, dec.Decode(r, &result, tc.opt))
 					require.JSONEq(t, tc.result, string(result), "%s", result)
 				}))
 				defer ts.Close()
@@ -75,7 +85,7 @@ func TestNewBodyDecoder(t *testing.T) {
 			t.Run(fmt.Sprintf("case=%d/description=%s", k, tc.d), func(t *testing.T) {
 				ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 					var result json.RawMessage
-					require.NoError(t, dec.Decode(r, &result))
+					require.NoError(t, dec.Decode(r, &result, BodyDecoderOptions{}))
 					require.JSONEq(t, tc.result, string(result), "%s", result)
 				}))
 				defer ts.Close()

selfservice/oidc/form.go

@@ -90,7 +90,9 @@ func merge(dc *selfservice.BodyDecoder, form string, traits json.RawMessage) (js
 		Traits map[string]interface{} `json:"traits"`
 	}
 
-	if err := dc.DecodeForm(q, &decodedForm); err != nil {
+	if err := dc.DecodeForm(q, &decodedForm, selfservice.BodyDecoderOptions{
+		AssertTypesForPrefix: "traits.",
+	}); err != nil {
 		return nil, err
 	}
 

selfservice/password/registration.go

@@ -65,7 +65,7 @@ func (s *Strategy) handleRegistration(w http.ResponseWriter, r *http.Request, _
 	}
 
 	var p RegistrationFormPayload
-	if err := s.dec.Decode(r, &p); err != nil {
+	if err := s.dec.Decode(r, &p, selfservice.BodyDecoderOptions{AssertTypesForPrefix: "traits."}); err != nil {
 		s.handleRegistrationError(w, r, ar, err)
 		return
 	}

selfservice/password/registration_test.go

@@ -241,6 +241,18 @@ func TestRegistration(t *testing.T) {
 			assert.Empty(t, gjson.GetBytes(body, "methods.password.config.error"))
 			assert.Contains(t, gjson.GetBytes(body, "methods.password.config.fields.traits\\.foobar.error").String(), "foobar is required", "%s", body)
 		})
+
+		t.Run("case=should work even if password is just numbers", func(t *testing.T) {
+			viper.Set(configuration.ViperKeyDefaultIdentityTraitsSchemaURL, "file://./stub/registration.schema.json")
+			rr := newRegistrationRequest(t, time.Minute)
+			body, res := makeRequest(t, rr.ID, url.Values{
+				"traits.username": {"registration-identifier-10"},
+				"password":        {"93172388957812344432"},
+				"traits.foobar":   {"bar"},
+			}.Encode(), http.StatusOK)
+			assert.Contains(t, res.Request.URL.Path, "return-ts")
+			assert.Equal(t, `registration-identifier-10`, gjson.GetBytes(body, "identity.traits.username").String(), "%s", body)
+		})
 	})
 
 	t.Run("method=PopulateSignUpMethod", func(t *testing.T) {