Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fuzzing Feature Request - Empty Required Parameter Tests #555

Open
aazizpc opened this issue Jan 29, 2024 · 19 comments
Open

Fuzzing Feature Request - Empty Required Parameter Tests #555

aazizpc opened this issue Jan 29, 2024 · 19 comments
Assignees
Labels
enhancement New feature or request

Comments

@aazizpc
Copy link

aazizpc commented Jan 29, 2024

Original Payload:

{
    "engagement": {
        "crn": "crn{{$timestamp}}",
        ..
}

Assuming "crn" is the mandatory(aka required) field.

Fuzzing already has the feature to have negative tests to exclude mandatory parameters using the config:

"requiredFields": {
                      "enabled": true
                    },

which results in the crn field itself to be excluded

{
    "engagement": {
        ..
}

But, it would be great if we had a feature to allow the parameter to exist (either JSON or Query Parameter) but make it empty.
Expected Payload:

{
    "engagement": {
        "crn": "",
        ..
}

Workaround:
As of now, making minLength: 1 makes this possible by adding a test with no characters in the parameter. But, this doesn't work when the parameter is overwritten with Postman variables such as {{crn}}.

@thim81
Copy link
Collaborator

thim81 commented Jan 30, 2024

hi @aazizpc

That could be a valuable addition to generate a Fuzzing variation for the required field with an empty value.

@thim81
Copy link
Collaborator

thim81 commented Mar 3, 2024

@aazizpc

We got a similar request for "blank" values generated by Fuzzing and looking for more input from the users.

The fuzzy generation is based on the OpenAPI definition.
If the property has minLength option defined in OpenAPI, it will generate a fuzzy variance with a less characters.
If the property is marked as required, Fuzzing will drop the property.

For "blank", we don't have any support yet, but we have received this request already a couple of times.

How would you want configure this?
Would you add a certain x-portman-fuzz-blank: true to your OpenAPI or configure it in the Portman config, and define blanking for certain properties?

@aazizpc
Copy link
Author

aazizpc commented Mar 6, 2024

@thim81
Thanks for the consultation. We would not want to add any new annotation to the OpenAPI spec since we already have the information that the field is mandatory through the existing spec.

We already have the "requiredFields" Fuzzing option too that can be made to generate the two negative tests - one for skipping the field itself, and one for having the empty value, but if we do need an extra option to clarify - We would be satisfied with perhaps a new Fuzzing option in the portman-config.api.json file like "requiredFieldsEmpty" as shown in the config snippet below.

    "variationTests": [
      {
        "openApiOperation": "*::*",
        "variations": [
          {
            "name": "Unprocessable",
            "fuzzing": [
              {
                "requestBody": [
                  {
                    "requiredFields": {
                      "enabled": true
                    },
                    "requiredFieldsEmpty": {
                      "enabled": true
                    },
                    "minimumNumberFields": {
                      "enabled": true
                    },
                    "maximumNumberFields": {
                      "enabled": true
                    },
                    "minLengthFields": {
                      "enabled": true
                    },
                    "maxLengthFields": {
                      "enabled": true
                    }
                  }
                ],

And of course, this should apply to not just requestBody, but also requestQueryParams and requestHeaders.
And, this should also work when we overwrite the value in the overwrites section with anything - including a postman variable such as {{crn}}.
We understand that this will work only with quoted string data in JSON bodies, because with numbers and bool, we can't have double quoted values. But with headers, query parameters, it can apply to all field types.

@thim81
Copy link
Collaborator

thim81 commented Mar 6, 2024

@nicklloyd What do you think of introducing the extra option "requiredFieldsEmpty"?
Or should the empty value test generation be part of the generated "requiredFields"?

@thim81
Copy link
Collaborator

thim81 commented Apr 8, 2024

@aazizpc I'm leaning towards the additional "requiredFieldsEmpty", since it will allow you to control the fuzzing for "required" and "empty" separately.

Pulling in some Portman experts from the community: @savage-alex @danielkocot @tillig here to see what their opinion is on the Fuzzing topic?

Should we introduce an extra option "requiredFieldsEmpty"? Or should the empty value test generation be part of the generated "requiredFields"?

@tillig
Copy link
Contributor

tillig commented Apr 8, 2024

I think for request body separating "missing" from "empty" is interesting. It may be that totally missing or null is treated differently than empty string, in which case that might be valuable to test.

Some thoughts, in no particular order, but which may apply to how this might be considered.

I work primarily in .NET, and missing/null/empty gets handled somewhat differently depending on data type and whether it's in the request body, headers, and query string parameters.

  • If a query string parameter is missing or empty it's the same as if it wasn't passed at all. http://server?foo=1&bar= is treated as though bar wasn't provided. I'm not sure how to pass an empty string or null on a query string, but if you were to pass http://server?foo=1&bar="" then bar would be the literal "" string, including the quotes. We've already run into some weirdness with fuzzing and query string parameters where we have a schema saying "this parameter isn't required, but if it's provided, it has a minLength=1. The test that gets generated is to pass ?param= (a zero-length string) but this comes across as "the parameter wasn't provided, so use the behavior as though the parameter wasn't passed." We usually turn off required parameter fuzzing for query strings.
  • If you omit a parameter in the body, it's treated the same as if you passed null. There's no differentiation once the ASP.NET framework has processed it and bound it to a model, so there's no way to validate missing vs. null.
  • I haven't tried it, but I think if you have an empty request header it's actually invalid HTTP. Which doesn't mean it shouldn't be tested, I guess, but it seems more like testing your HTTP server's protocol adherence than API contract at that point.

Maybe what that boils down to is:

I think required-fields-empty separate from required-fields-being-present is interesting to control, but only for request bodies. For query strings, it's only a maybe depending on your web app framework. For headers it doesn't make sense.

@thim81
Copy link
Collaborator

thim81 commented Apr 9, 2024

@tillig This is exactly the kind of input, I was looking for.

Based on your input, it is clear that the fuzzing feature can be greatly improved to generate the mentioned cases.
It has also become apparent that the body, header & query param handling will become more different.

If I translate your input into expected cases, I have this list:

JSON Request body:

  1. omit properties {"name": "John"}
  2. empty values for properties {"name": "John", "surname": ""}
  3. null values for properties {"name": "John", "surname": null}

Question: Since Portman Fuzzing is based on OpenAPI info, should the (1) (2) cases be applied on "required", with the option to control both cases in the Portman config?

Example OpenAPI required property trigger:

components:
  schemas:
    User:
      type: object
      required:
        - name
        - surname
      properties:
        name:
          type: string
          example: "John"
        surname:
          type: string
          example: "Doe"

Portman Config:

{
  "requiredFields": {
    "enabled": true
  },
  "requiredFieldsEmpty": {
    "enabled": true
  }
}

And for case (3), Would it be based on "nullable: false" in OpenAPI?

Example OpenAPI null fuzzing trigger

components:
  schemas:
    User:
      type: object
      properties:
        name:
          type: string
          example: "John"
        surname:
          type: string
          nullable: false
          example: "Doe"

Portman Config:

{
  "nullFields": {
    "enabled": true
  }
}

Or what would you expect/desire to trigger & control the Fuzzing generation?

@thim81
Copy link
Collaborator

thim81 commented Apr 9, 2024

@tillig For the query params, the empty values are indeed handled depending on the backend framework.

Would it make sense to provide some additional options in Portman to define this?

I detected these cases for query parameters:
Request Query Params:

  1. omit query params http://example.com/api?name=john
  2. empty values for query params http://example.com/api?name=&surname= AND/OR http://example.com/api?name=""&surname="" (which can be defined by a Portman config)
  3. null values for query params http://example.com/api?name=null&surname=null AND/OR http://example.com/api?name=%00&surname=%00 (which can be defined by a Portman config)

Cases (1) (2) could be triggered and configured like this?

Example OpenAPI required query param trigger:

parameters:
  - in: query
    name: name
    schema:
      type: string
    required: true
  - in: query
    name: surname
    schema:
      type: string
    required: true

Portman Config:

{
  "requiredFields": {
    "enabled": true
  },
  "requiredFieldsEmpty": {
    "enabled": true,
    "valueType": "QUOTES" ("QUOTES"/"BLANK"/"ALL")
  }
}

Cases (3) could be triggered and configured like this?

Example OpenAPI null query param trigger:

parameters:
  - in: query
    name: name
    schema:
      type: string
      nullable: true
  - in: query
    name: surname
    schema:
      type: string
      nullable: true

Portman Config:

{
  "nullFields": {
    "enabled": true,
    "valueType": "NULL" ("NULL"/"%00"/"ALL")
  }
}

@tillig
Copy link
Contributor

tillig commented Apr 9, 2024

Let me step back a second and just restate how I understand OpenAPI to ensure I'm on the same page.

  • required in an OpenAPI schema means "present" but has nothing to do with whether a value is null or not.
  • nullable: true (in OAS 3.0.x) or type: ["string", "null"] (type arrays in OAS 3.1) are what indicate whether null is allowed as a value; by default things are not nullable.
  • Neither of these have anything to do with empty string. A string field that is both required and not nullable may correctly be "" empty string and that's valid (hence this discussion about null vs. empty).

Given that, in answer to your questions:

  • Since Portman Fuzzing is based on OpenAPI info, should the (1) (2) cases be applied on "required"...? No. Required isn't the same as empty string, so they should be kept separate.
  • And for case (3), Would it be based on "nullable: false" in OpenAPI? By default things are not nullable in OpenAPI unless specified, so I'd invert this to be based on nullable: true or type: ["string", "null"].

There's also something in my mind that is sticking with me: This is only for string values. The empty string thing - that's only for string objects. Not integer, number, array, boolean, or object. It makes me wonder if this should factor in to type-specific configuration for fuzzing, like

{
  "requiredFields": {
    "enabled": true
  },
  "typeFuzzing": {
    "string": {
      "requiredFieldsEmpty": {
        "enabled": true
      }
    }
  }
}

I'm not glued to that configuration structure, just trying to indicate that perhaps having type-specific configuration options may be something to consider. For example, with numbers, sometimes 0 is the default if a required property is omitted (that's how it binds in ASP.NET) so having requiredFieldsZero for a numeric input may be interesting.

I think for query params the cases are:

  • Omitted: http://example.com/api?name=john
  • Present but without value: http://example.com/api?name=john&surname=
  • Maybe a null string? http://example.com/api?name=john&surname=%00

I don't think surname="" is valid because query strings aren't JSON, so this is the literal string with two double quotes rather than empty string. Same with surname=null - that's the literal string null not the actual value null.

However, I could see that it could be interesting to provide "additional fuzzing parameter values" for things, like:

{
  "requiredFields": {
    "enabled": true
  },
  "typeFuzzing": {
    "string": {
      "requiredFieldsEmpty": {
        "enabled": true,
        "values": ["\"\"", "%00"]
      },
    }
  }
}

Like, possibly be able to say "these are some custom values that my app considers equivalent to empty string."

Again, not superglued to that, just throwing ideas around.

@thim81
Copy link
Collaborator

thim81 commented Apr 9, 2024

At this point, for me the nullable case is getting more and more clear.

About the empty case for request body, how would you define which properties should be blank values?

I was thinking in the direction of the "required" openapi in combination with a Portman specific config, but after reading your very valid feedback, I wonder how you define this?

@tillig
Copy link
Contributor

tillig commented Apr 9, 2024

For request bodies I think empty string tests are for fields with type: string and one or more of...

  • minLength greater than zero.
  • pattern, but where the pattern specified does not match empty string.
  • any format annotation since none of those appear to allow empty string; though per the JSON schema I gather format is less about validation so maybe this one can be ignored.

@thim81
Copy link
Collaborator

thim81 commented Apr 10, 2024

@tillig Looking at the initial starting point of this, there was the request to fuzz "required" properties also with blank values.
The advice here would be that @aazizpc should add minLenght to his engagement property.

Does this mean that there is no use-case anymore for Portman requiredFieldsEmpty config?
Or would you generate requiredFieldsEmpty cases for any string field?

@tillig
Copy link
Contributor

tillig commented Apr 10, 2024

I'd say that if a string field is required and doesn't allow empty, it also needs a minLength 1. While there is value in fuzzing string fields with empty string values, the required flag technically allows empty string as a valid value.

From the original request:

As of now, making minLength: 1 makes this possible by adding a test with no characters in the parameter. But, this doesn't work when the parameter is overwritten with Postman variables such as {{crn}}.

So the logic holds - setting min length fixes it.

The problem seems like it's more about the variable replacement handling and whether the fuzzer will override the {{var}} style replacement.

It might be good to see a minimum repro showing the issue - both a tiny schema and a tiny Portman config. Not snippets, the full thing - just as small as possible.

@thim81
Copy link
Collaborator

thim81 commented Apr 11, 2024

I'll try to find time to setup a POC to show the behaviour and how the config would look like.

@thim81
Copy link
Collaborator

thim81 commented Apr 14, 2024

This is the little POC, I created using the latest Portman version:

OpenAPI

openapi: 3.0.1
info:
  title: Sample API
  description: API description in Markdown.
  version: 1.0.0
servers:
  - url: 'http://api.example.com/v1'
paths:
  /users:
    post:
      summary: Create a new user
      requestBody:
        content:
          application/json:
            schema:
              type: object
              required:
                - name
                - surname
              properties:
                name:
                  type: string
                  description: The user's name.
                surname:
                  type: string
                  description: The user's surname.
                  minLength: 1
      responses:
        '201':
          description: Created

Portman

{
  "version": 1.0,
  "tests": {
    "variationTests": [
      {
        "openApiOperation": "*::/*",
        "variations": [
          {
            "name": "Bad",
            "openApiResponse": "400",
            "fuzzing": [
              {
                "requestBody": [
                  {
                    "requiredFields": {
                      "enabled": true
                    },
                    "minLengthFields": {
                      "enabled": true
                    }
                  }
                ]
              }
            ],
            "tests": {
              "contractTests": [
                {
                  "statusCode": {
                    "enabled": true,
                    "code": 400
                  }
                },
                {
                  "jsonBody": {
                    "enabled": true
                  }
                },
                {
                  "contentType": {
                    "enabled": true
                  }
                },
                {
                  "schemaValidation": {
                    "enabled": true
                  }
                }
              ]
            }
          }
        ]
      }
    ]
  },
  "globals": {
    "stripResponseExamples": true
  }
}

Postman result
image
image
image

``` { "_": { "postman_id": "b9e37556-9698-4a04-b5e0-a6aa842061eb" }, "item": [ { "id": "835d5d08-8435-4a60-b568-cb1ede6e3582", "name": "Create a new user", "request": { "name": "Create a new user", "description": { "type": "text/plain" }, "url": { "path": [ "users" ], "host": [ "{{baseUrl}}" ], "query": [], "variable": [] }, "header": [ { "key": "Content-Type", "value": "application/json" } ], "method": "POST", "body": { "mode": "raw", "raw": "{\n \"name\": \"dolore non in cupidatat\",\n \"surname\": \"qui cillum ullamco et\"\n}", "options": { "raw": { "language": "json" } } } }, "response": [ { "_": { "postman_previewlanguage": "text" }, "id": "6cb67cad-1ec6-449b-b88b-da4ef2e0f236", "name": "Created", "originalRequest": { "url": { "path": [ "users" ], "host": [ "{{baseUrl}}" ], "query": [], "variable": [] }, "method": "POST", "body": { "mode": "raw", "raw": "{\n \"name\": \"dolore non in cupidatat\",\n \"surname\": \"qui cillum ullamco et\"\n}", "options": { "raw": { "language": "json" } } } }, "status": "Created", "code": 201, "header": [ { "key": "Content-Type", "value": "text/plain" } ], "body": "", "cookie": [] } ], "event": [], "protocolProfileBehavior": { "disableBodyPruning": true } }, { "id": "87cbffa2-131e-44ff-9ba7-ed7ac308c214", "name": "Variation Tests", "item": [ { "id": "63de135a-2500-496a-bd7c-4caa670d605f", "name": "555-fuzz-blank Variations", "item": [ { "id": "a9ecc22d-e512-4f8a-8db7-72e25d0370b8", "name": "Create a new user[Bad][required name]", "request": { "name": "Create a new user[Bad][required name]", "description": { "type": "text/plain" }, "url": { "path": [ "users" ], "host": [ "{{baseUrl}}" ], "query": [], "variable": [] }, "header": [ { "key": "Content-Type", "value": "application/json" } ], "method": "POST", "body": { "mode": "raw", "raw": "{\n \"surname\": \"qui cillum ullamco et\"\n}", "options": { "raw": { "language": "json" } } } }, "response": [], "event": [], "protocolProfileBehavior": { "disableBodyPruning": true } }, { "id": "21e8c221-7cbf-4ea1-afd0-3b544355a84e", "name": "Create a new user[Bad][required surname]", "request": { "name": "Create a new user[Bad][required surname]", "description": { "type": "text/plain" }, "url": { "path": [ "users" ], "host": [ "{{baseUrl}}" ], "query": [], "variable": [] }, "header": [ { "key": "Content-Type", "value": "application/json" } ], "method": "POST", "body": { "mode": "raw", "raw": "{\n \"name\": \"dolore non in cupidatat\"\n}", "options": { "raw": { "language": "json" } } } }, "response": [], "event": [], "protocolProfileBehavior": { "disableBodyPruning": true } }, { "id": "84379cfc-023d-4f83-a3cf-4c06ab52544c", "name": "Create a new user[Bad][minimum length surname]", "request": { "name": "Create a new user[Bad][minimum length surname]", "description": { "type": "text/plain" }, "url": { "path": [ "users" ], "host": [ "{{baseUrl}}" ], "query": [], "variable": [] }, "header": [ { "key": "Content-Type", "value": "application/json" } ], "method": "POST", "body": { "mode": "raw", "raw": "{\n \"name\": \"dolore non in cupidatat\",\n \"surname\": \"\"\n}", "options": { "raw": { "language": "json" } } } }, "response": [], "event": [], "protocolProfileBehavior": { "disableBodyPruning": true } } ], "event": [] } ], "event": [] } ], "event": [], "variable": [ { "type": "string", "value": "http://api.example.com/v1", "key": "baseUrl" } ], "info": { "_postman_id": "b9e37556-9698-4a04-b5e0-a6aa842061eb", "name": "555-fuzz-blank", "schema": "https://schema.getpostman.com/json/collection/v2.1.0/collection.json", "description": { "content": "API description in Markdown.", "type": "text/plain" } } } ```

@thim81
Copy link
Collaborator

thim81 commented Apr 14, 2024

About the {{CNR}} handling, this also seems to work as expected.

Same OpenAPI as above

Portman config

{
  "version": 1.0,
  "tests": {
    "variationTests": [
      {
        "openApiOperation": "*::/*",
        "variations": [
          {
            "name": "Bad",
            "openApiResponse": "400",
            "fuzzing": [
              {
                "requestBody": [
                  {
                    "requiredFields": {
                      "enabled": true
                    },
                    "minLengthFields": {
                      "enabled": true
                    }
                  }
                ]
              }
            ],
            "overwrites": [
              {
                "openApiOperation": "*::/*",
                "overwriteRequestBody": [
                  {
                    "key": "surname",
                    "value": "{{CNR}}"
                  }
                ]
              }
            ],
            "tests": {
              "contractTests": [
                {
                  "statusCode": {
                    "enabled": true,
                    "code": 400
                  }
                },
                {
                  "jsonBody": {
                    "enabled": true
                  }
                },
                {
                  "contentType": {
                    "enabled": true
                  }
                },
                {
                  "schemaValidation": {
                    "enabled": true
                  }
                }
              ]
            }
          }
        ]
      }
    ]
  },
  "globals": {
    "stripResponseExamples": true
  }
}

Postman Results
image
image
image

@thim81
Copy link
Collaborator

thim81 commented Apr 14, 2024

It still makes me wonder, if there is a need for fuzzing where blank values are used? Using requiredFieldsEmpty ?

@tillig
Copy link
Contributor

tillig commented Apr 14, 2024

I'm not sure why - required doesn't imply not empty.

@danielkocot
Copy link
Contributor

Summon @miriamgreis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants