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

/admin/api/publish really should require POST #1006

Open
dairiki opened this issue Feb 25, 2022 · 0 comments
Open

/admin/api/publish really should require POST #1006

dairiki opened this issue Feb 25, 2022 · 0 comments

Comments

@dairiki
Copy link
Contributor

dairiki commented Feb 25, 2022

The /publish endpoint of the admin API is used to publish (deploy) the site to a configured server.

Currently, the frontend uses GET when triggering a deployment. As it is highly dubious (albeit arguable) that publishing the site counts as idempotent, really, a POST should be required here. There may be CORS-related security advantages to requiring POST here, too. In any case, a GET sure feels wrong, in this case.

The reason the fix is not totally trivial is that /publish returns an SSE stream. The EventSource API, which we currently use to handle the SSE output, appears (for no good reason that I've figured out) to not offer any way to open an event stream using any method other than GET.

I'm not sure how best to fix this.
Perhaps use a third-party replacement for EventSource like sse.js which would allow for POST?
Or make the API call a two-step process where the POST to /publish would return a "303 See Other" to a URL that would produce (via GET) the SSE stream.

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