apideck-libraries / portman

Port OpenAPI Specs to Postman Collections, inject test suite and run via Newman 👨🏽‍🚀
http://getportman.com/
Apache License 2.0
632 stars 59 forks source link

Fuzzing Feature Request - Empty Required Parameter Tests #555

Open aazizpc opened 7 months ago

aazizpc commented 7 months ago

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 commented 7 months ago

hi @aazizpc

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

thim81 commented 6 months ago

@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 commented 6 months ago

@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 commented 6 months ago

@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 commented 5 months ago

@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 commented 5 months ago

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.

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 commented 5 months ago

@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 commented 5 months ago

@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 commented 5 months ago

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

Given that, in answer to your questions:

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:

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 commented 5 months ago

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 commented 5 months ago

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

thim81 commented 5 months ago

@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 commented 5 months ago

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 commented 5 months ago

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

thim81 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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

tillig commented 5 months ago

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

danielkocot commented 5 months ago

Summon @miriamgreis