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

POST to /db/canned-query that returns JSON should be supported (for API clients) #880

Closed
simonw opened this issue Jul 1, 2020 · 11 comments

Comments

@simonw
Copy link
Owner

simonw commented Jul 1, 2020

Now that CSRF is solved for API requests (#835) it would be good to support API requests to the .json extension.

@simonw
Copy link
Owner Author

simonw commented Jul 1, 2020

The response from this will never be a 302 - it will always be a 200 if the response worked or a 400 for bad parameters or a 500 for errors. The body returned will always be in JSON format.

@simonw simonw pinned this issue Jul 1, 2020
simonw added a commit that referenced this issue Jul 1, 2020
@simonw
Copy link
Owner Author

simonw commented Jul 1, 2020

I've been testing the WIP using this in the console:

fetch('/data/add_name.json', {
  method: 'POST',
  body: 'name=XXXfetch',
  credentials: 'omit',
  headers: {'Content-Type': 'application/x-www-form-urlencoded'}
})
.then(response => console.log(response))

Against a canned query configured like this:

databases:
  data:
    queries:
      add_name:
        sql: insert into names (name) values (:name)
        write: true

I haven't got it to work yet. Latest error is this one:

INFO:     Uvicorn running on http://127.0.0.1:8001 (Press CTRL+C to quit)
Traceback (most recent call last):
  File "/Users/simon/Dropbox/Development/datasette/datasette/app.py", line 975, in route_path
    await response.asgi_send(send)
AttributeError: 'tuple' object has no attribute 'asgi_send'
INFO:     127.0.0.1:49938 - "POST /data/add_name.json HTTP/1.1" 500 Internal Server Error

It looks like I'm going to have to rethink how the BaseView code around tables, formats and hashes is structured in order to fix this. That's a big refactoring! I'm moving this to a new milestone for Datasette 0.46.

@simonw simonw modified the milestones: Datasette 0.45, Datasette 0.46 Jul 1, 2020
@simonw simonw modified the milestones: Datasette 0.46, Datasette 0.49 Aug 28, 2020
@simonw
Copy link
Owner Author

simonw commented Sep 12, 2020

What should happen when something does a POST to an extension that was registered by a plugin, e.g. POST /db/table.atom ?

@simonw
Copy link
Owner Author

simonw commented Sep 12, 2020

Maybe POST to .json doesn't actually make sense. I could instead support POST /db/queryname with an optional mechanism for requesting that the response to that POST be in a JSON format.

Could be a Accept: application/json header with an option of including "_accept": "json" as a POST parameter instead.

@simonw
Copy link
Owner Author

simonw commented Sep 12, 2020

Is it safe to skip CSRF checks if the incoming request has Accept: application/json on it?

I'm not sure that matters since asgi-csrf already won't reject requests that either have no cookies or are using a Authorization: Bearer ... header.

@simonw
Copy link
Owner Author

simonw commented Sep 14, 2020

Answer: no, it's not safe to skip CSRF if there's an Accept: application/json header because of a nasty old crossdomain.xml Flash vulnerability: https://blog.appsecco.com/exploiting-csrf-on-json-endpoints-with-flash-and-redirects-681d4ad6b31b?gi=a5ee3d7a8235

@simonw simonw changed the title POST to /db/canned-query.json should be supported POST to /db/canned-query that returns JSON should be supported (for API clients) Sep 14, 2020
@simonw
Copy link
Owner Author

simonw commented Sep 14, 2020

Relevant code section:

if write:
if request.method == "POST":
params = await request.post_vars()
if canned_query:
params_for_query = MagicParameters(params, request, self.ds)
else:
params_for_query = params
try:
cursor = await self.ds.databases[database].execute_write(
sql, params_for_query, block=True
)
message = metadata.get(
"on_success_message"
) or "Query executed, {} row{} affected".format(
cursor.rowcount, "" if cursor.rowcount == 1 else "s"
)
message_type = self.ds.INFO
redirect_url = metadata.get("on_success_redirect")
except Exception as e:
message = metadata.get("on_error_message") or str(e)
message_type = self.ds.ERROR
redirect_url = metadata.get("on_error_redirect")
self.ds.add_message(request, message, message_type)
return self.redirect(request, redirect_url or request.path)

@simonw
Copy link
Owner Author

simonw commented Sep 14, 2020

I'm going to add support for POST content that is sent as a JSON document, in addition to the existing support for key=value encoded POST bodies.

@simonw
Copy link
Owner Author

simonw commented Sep 14, 2020

I'm going to support several ways of indicating that you would like a JSON response instead of getting a HTTP redirect from your writable canned query submission:

  • Use the Accept: application/json request header
  • Include ?_json=1 in the request query string
  • Include "_json": 1 in the form submission (or the JSON body submission)

@simonw
Copy link
Owner Author

simonw commented Sep 14, 2020

The JSON response will look like this:

{
  "ok": true,
  "message": "A message",
  "redirect": "/blah"
}

"ok" will be true if everything went right and false if there was an error.

The "message" and "redirect" will be whatever was configured using the on_success_message - the message shown on_success_message, on_success_redirect, on_error_message and on_error_redirect settings, see https://docs.datasette.io/en/stable/sql_queries.html#writable-canned-queries

@simonw simonw closed this as completed in 72ac2fd Sep 14, 2020
@simonw
Copy link
Owner Author

simonw commented Sep 14, 2020

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

No branches or pull requests

1 participant