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

"Field id is required" when patching /api/v2/firewall/rule #504

Closed
michelsup opened this issue Jul 8, 2024 · 3 comments
Closed

"Field id is required" when patching /api/v2/firewall/rule #504

michelsup opened this issue Jul 8, 2024 · 3 comments
Labels
enhancement Issues or PRs that enhance existing features v2 Issues or PRs that apply to v2

Comments

@michelsup
Copy link

michelsup commented Jul 8, 2024

Describe the bug
When you go to /api/v2/documentation#/FIREWALL/patchFirewallRuleEndpoint, fill an id, and Execute you get a 400 error.

To Reproduce
Go to /api/v2/documentation#/FIREWALL/patchFirewallRuleEndpoint
Select "application/x-www-form-urlencoded"
Enter an existing rule id
Click Execute

Expected behavior
Get a 200 response

Screenshots or Response
curl -X 'PATCH' \ 'https://xxxxx:4443/api/v2/firewall/rule' \ -H 'accept: application/json' \ -H 'Authorization: Basic YWRtaW46UHdldDY0NTch' \ -H 'Content-Type: application/x-www-form-urlencoded' \ -d 'descr=&gateway=&log=&source_port=&floating=&pdnpipe=&ackqueue=&statetype=keep%20state&tcp_flags_set=&quick=&destination=&defaultqueue=&destination_port=&tcp_flags_any=&icmptype=any&sched=&dnpipe=&protocol=&interface=&type=&source=&id=25&disabled=&tcp_flags_out_of=&ipprotocol=&direction=any'

{ "code": 400, "status": "bad request", "response_id": "MODEL_REQUIRES_ID", "message": "Field id is required.", "data": [] }

pfSense Version & Package Version:

  • pfSense Version: 24.03
  • Package Version 2.0

Affected Endpoints:

  • URL: /api/v2/firewall/rule
@jaredhendrickson13
Copy link
Owner

The application/x-www-form-urlencoded content-type expects the parameters to be included the URL itself rather than the request body, for example:

curl -s -k -u admin:pfsense -H "content-type: application/x-www-form-urlencoded" -H "accept: application/json" -X PATCH 'https://localhost/api/v2/firewall/rule?id=0'
{
  "code": 200,
  "status": "ok",
  "response_id": "SUCCESS",
  "message": "",
  "data": {
    "id": 0,
    "type": "pass",
    "interface": [
      "lan"
    ],
    "ipprotocol": "inet",
    "protocol": null,
    "icmptype": null,
    "source": "lan",
    "source_port": null,
    "destination": "any",
    "destination_port": null,
    "descr": "Default allow LAN to any rule",
    "disabled": false,
    "log": false,
    "statetype": "keep state",
    "tcp_flags_any": false,
    "tcp_flags_out_of": null,
    "tcp_flags_set": null,
    "gateway": null,
    "sched": null,
    "dnpipe": null,
    "pdnpipe": null,
    "defaultqueue": null,
    "ackqueue": null,
    "floating": false,
    "quick": null,
    "direction": null,
    "tracker": 100000101,
    "created_time": 1713996066,
    "created_by": "",
    "updated_time": 1713996066,
    "updated_by": "[email protected] (API)"
  }
}

However, application/x-www-form-urlencoded is not recommended for requests other than GET and DELETE requests because it only supports type inference and the API requires strict typing (e.g. there's no way to differentiate "true" from true, or "443" from 443). application/json is the recommended content-type for POST, PATCH and PUT requests.

@michelsup
Copy link
Author

michelsup commented Jul 9, 2024

Thank you for your quick reply. My bad then, I should have miss the info on the documentation.
I would suggest you to remove the ability to use application/x-www-form-urlencoded within the documentation if it's not recommended, and to be more precise, if it plainly doesn't work as expected/suggested in /api/v2/documentation#/FIREWALL/patchFirewallRuleEndpoint.
BTW, thank you for the hint and, overall, your work.

@jaredhendrickson13
Copy link
Owner

After looking into this a little more, I do think this is a valid issue. I'll keep this open as an enhancement since the API should allow URL encoded parameters to be submitted via request body per HTTP standard. It was also supported in v1 so there's not really a valid reason to have that limitation in v2. I'll make sure to update documentation accordingly as well.

Thanks!

@jaredhendrickson13 jaredhendrickson13 added enhancement Issues or PRs that enhance existing features v2 Issues or PRs that apply to v2 labels Jul 9, 2024
@jaredhendrickson13 jaredhendrickson13 moved this from Ready to Done in pfSense REST API v2.1.0 Jul 14, 2024
@jaredhendrickson13 jaredhendrickson13 linked a pull request Aug 28, 2024 that will close this issue
@jaredhendrickson13 jaredhendrickson13 removed a link to a pull request Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues or PRs that enhance existing features v2 Issues or PRs that apply to v2
Projects
No open projects
Status: Done
Development

No branches or pull requests

2 participants