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

Remove empty query params #238

Closed
ruscoder opened this issue Jan 23, 2020 · 6 comments
Closed

Remove empty query params #238

ruscoder opened this issue Jan 23, 2020 · 6 comments
Assignees
Labels

Comments

@ruscoder
Copy link

ruscoder commented Jan 23, 2020

Hi! GET /Patient?_id= will create request such as params: {_id: ''} and to handle it correctly we need to check that value != null and value != ''.

Security note
For Aidbox's users it leads to security issues when developers forgot to check minLength in policies, e.g.

    "properties": {
        "request-method": {
            "constant": "get",
        },
        "uri": {"constant": "/Patient"},
        "params": {
            "required": ["patient"],
        },
    }

and for GET /Patient?_id= this policy will allow the query, and the user will have an access to ALL patients in the system.

As workaround we use a bit enhanced policy, but it annoying to check minLength for required properties.

    "properties": {
        "request-method": {
            "constant": "get",
        },
        "uri": {"constant": "/Patient"},
        "params": {
            "required": ["patient"],
            "properties": {
                 "patient": {"minLength": 1}
            }
        },
    }
@kzotoff
Copy link

kzotoff commented Jan 31, 2020

For some fields empty string may have another meaning: no entries to return.

For example, I generate some Patient ID set and want to get these entries so I just do GET /Patient?id=id-1,id-2,.... If set contains no elements, request will return all entries instead of none so I need to additionally check if my search set is not empty. It looks a bit inconsistent.

@ruscoder
Copy link
Author

ruscoder commented Aug 2, 2021

What do you think about it?

@ZaLLin @KGOH

I've updated description, added security risks

@niquola niquola added the bug label Aug 4, 2021
@Nesmeshnoy Nesmeshnoy changed the title Feature request: Remove empty query params Remove empty query params Aug 6, 2021
@Nesmeshnoy Nesmeshnoy added this to the August 2021 - v:2108 milestone Aug 17, 2021
@Nesmeshnoy
Copy link
Contributor

@ruscoder fixed. please check it and give us feedback.

@ruscoder
Copy link
Author

Thanks! I'll test it

@Nesmeshnoy
Copy link
Contributor

Keep us updated.

@ruscoder
Copy link
Author

Works great!

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

No branches or pull requests

5 participants