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

protect UI API calls with POST/csrf #299

Open
joefutrelle opened this issue Aug 10, 2020 · 4 comments
Open

protect UI API calls with POST/csrf #299

joefutrelle opened this issue Aug 10, 2020 · 4 comments
Assignees

Comments

@joefutrelle
Copy link
Contributor

Right now we have GET API calls that users can call from non-browser clients, and some of them are expensive. For endpoints that are just used for the UI, they should be POSTs that require a csrf token.

@joefutrelle
Copy link
Contributor Author

TODO: idedntify relevant endpoints

@mike-kaimika
Copy link
Collaborator

@joefutrelle
I think it might be easier to exclude endpoints then include them. The section from urls.py below are all the API ones. Are there any that should NOT be post requests?

path('api/time-series/<slug:metric>', views.generate_time_series, name='generate_time_series'),

Same for the admin area:

path('api/dt/datasets', views.dt_datasets, name='datasets_dt'),

Just a heads up that this will take some time to switch out. I spot checked a few of the dashboard URLs, and some of them are called from multiple locations in the code, and definitely using GET. So while fixing any particular endpoint is trivial, the time consuming part will be tracking down all the uses of that url and testing

@joefutrelle
Copy link
Contributor Author

Agreed that this is not necessary for all endpoints and not essential for many of them. So I will identify the ones to target and we can just do those for now. My criteria are

  1. Does the endpoint perform an expensive and/or asynchronous operation (e.g., mosaic computation)
  2. Does the endpoint expose internal information like database IDs that callers should not rely on

I think this will be a reasonably small subset of the API endpoints. In the next round we would protect all API endpoints that aren't appropriate to be called from an external script but really only support the UI.

@joefutrelle joefutrelle self-assigned this Aug 27, 2020
@mike-kaimika
Copy link
Collaborator

@joefutrelle I finished up the conversion of all the items that start with /api, minus about 4 of them that I think should remain as GETS (image links mostly). All changes are in the below PR. As mentioned in Slack, I've done the conversion AND tried to locate and test all instances where those methods are called, and everything looks good as far as I can tell

Updated Files
#326

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

No branches or pull requests

2 participants