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

Offset accepts string as valid input #1997

Closed
1 of 3 tasks
sigalsax opened this issue Jan 19, 2021 · 1 comment · Fixed by #1998
Closed
1 of 3 tasks

Offset accepts string as valid input #1997

sigalsax opened this issue Jan 19, 2021 · 1 comment · Fixed by #1998

Comments

@sigalsax
Copy link
Contributor

Summary

Our REST API for list, accepts a string for offset as valid data when it should only accept a numeric value. When we run offset=somestring, we get the JSON data from the Conjur server without any constraints applied to it as if we were to run conjur list

For example, the returned data would resemble the following

      {
        "version": 2166,
        "created_at": "2021-01-14T09:31:31.605+00:00",
        "policy_text": "- !layer somelayer\n- !group somegroup\n- !host anotherhost\n- !user someuser\n- !variable 
...
        "client_ip": "54.236.50.115",
        "id": "cucumber:policy:root",
        "role": "cucumber:user:admin"
      },

Steps to Reproduce

Steps to reproduce the behavior:

  1. Run
curl -k -H "$(conjur authn authenticate -H)" \
   'https://conjur-server/resources/cucumber?offset=somestring' \
     | jq .

Expected Results

A non-numeric value for offset should return a 500 internal server error like limit does

Actual Results (including error logs, if applicable)

...
      {
        "version": 2166,
        "created_at": "2021-01-14T09:31:31.605+00:00",
...
        "finished_at": "2021-01-14T09:31:31.747+00:00",
        "client_ip": "54.236.50.115",
        "id": "cucumber:policy:root",
        "role": "cucumber:user:admin"
      },
...

Reproducible

  • Always
  • Sometimes
  • Non-Reproducible

Version/Tag number

All versions

Environment setup

DAP server in AWS, Conjur CLI on my local machine

Additional Information

NA

@micahlee
Copy link
Contributor

Thanks for writing this up @sigalsax !

I would suggest that rather than

A non-numeric value for offset should return a 500 internal server error like limit does

an invalid value for either offset or limit should result in 422 Unprocessable Entity. The description for this is:

The HyperText Transfer Protocol (HTTP) 422 Unprocessable Entity response status code indicates that the server understands the content type of the request entity, and the syntax of the request entity is correct, but it was unable to process the contained instructions.

A more generic alternative could be just 400 Bad Request. However, that is really intended for when something is wrong in the HTTP protocol itself, rather than in the application-level API expectations. So, I would lean against going that route.

Conjur should really only return a 500 internal server error if it encounters an error we haven't thought to handle yet (see ref), and not one we ever intentionally return.

CC: @liavyona

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants