Skip to content
This repository has been archived by the owner on Feb 28, 2022. It is now read-only.

Previewing markdown #292

Closed
rofe opened this issue Apr 26, 2019 · 21 comments
Closed

Previewing markdown #292

rofe opened this issue Apr 26, 2019 · 21 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@rofe
Copy link
Contributor

rofe commented Apr 26, 2019

(spin-off from #251)

Add either a dedicated pipe or extend the existing html pipe to take markdown from the POSTed request.body, proxy the html pipe, and wrap the generated output in a JSON object:

{
  "content": "<!doctype html>... "
}

This would be a safe way to support live (read: "as you type") previewing of markdown using Helix (for example using a browser extension).

The simplest and most transparent solution would be to register a new pipeline to a .preview.json selector/extension combo, and have editing tools POST there.

A more sophisticated solution would be to use a vendor specific mime-type like application/vnd.helix.preview+json in the Accept header, so the POST could be made to the original (.html) path, and pipe resolution would be handled by Fastly based on the header.

@rofe rofe added help wanted Extra attention is needed enhancement New feature or request labels Apr 26, 2019
@trieloff
Copy link
Contributor

Taking the POST body seems fine, switching the content type from HTML to JSON is something I'd want to avoid.

@rofe
Copy link
Contributor Author

rofe commented Apr 26, 2019

So you would prefer the simple solution, i.e. posting to /bla.preview.json?

@tripodsan
Copy link
Contributor

we need to post to the action that is actually doing the transformation. I don't see the html action as something that needs to respond with text/html. if the requested media type is json, we should give json.

I think this should be a generic feature of the pipeline, to suppress the emit step of the default media type, and respond with an alternative.

@tripodsan
Copy link
Contributor

So you would prefer the simple solution, i.e. posting to /bla.preview.json?

I wouldn't do this, otherwise you can never request a preview with a different selector or extension.

@trieloff
Copy link
Contributor

If we do this, we need to be careful on the Fastly side. Making the first request with an Accept header can poison the cache, unless we vary on Accept (which reduces cache efficiency). We can handle it in VCL, just need to be aware of it.

@tripodsan
Copy link
Contributor

also, this issue is about how the pipeline should handle the request, and not how this is mapped in fastly :-)

@trieloff
Copy link
Contributor

@rofe I think you can start with the POST reading-part and we can decide if JSON responses are really needed later.

@tripodsan
Copy link
Contributor

I think this is all wrong :-) operations like this, should actually be made against a (yet to be defined) helix-content API.

@tripodsan
Copy link
Contributor

tripodsan commented Apr 26, 2019

I think you can start with the POST reading-part and we can decide if JSON responses are really needed later.

we already have the POST part,. it works when we POST directly to the action. the problem is:

  1. how do we access this through fastly?
  2. how do we respond with JSON

@trieloff
Copy link
Contributor

  1. how do we access this through fastly?

What's so bad about calling the action directly?

  1. how do we respond with JSON

What's so bad about responding HTML?

@rofe
Copy link
Contributor Author

rofe commented Apr 26, 2019

we already have the POST part

correct, see adobe/helix-simulator#179 and #235

@rofe
Copy link
Contributor Author

rofe commented Apr 26, 2019

What's so bad about calling the action directly?

how does the editing tool (or worse, the end user) know the action URL?

What's so bad about responding HTML?

CORS.

@trieloff
Copy link
Contributor

CORS

We can set the Access-Control-Allow-Origin header from within the action.

how does the editing tool (or worse, the end user) know the action URL?

Ask Fastly? Maybe through a .hlx_edit URL extension that simply redirects to the correct action URL?

@trieloff
Copy link
Contributor

The things that I'd look at for editing-optimized HTML output is:

  • mapping hints for position in HTML to position in Markdown
  • cursor indicators
  • select range indicators

@rofe
Copy link
Contributor Author

rofe commented Apr 26, 2019

for completeness: we don't have to set the Access-Control-Allow-Origin header in the action, Bladerunner/OpenWhisk does that by default. So posting to the action directly would eliminate the CORS issue.

@rofe
Copy link
Contributor Author

rofe commented Apr 26, 2019

Ask Fastly?

Not sure if we really want to keep it that easy to find out the backend URL...

@lkrapf
Copy link

lkrapf commented Apr 26, 2019

@trieloff:

What's so bad about responding HTML?

Because by making your browser post to the preview action, an attacker can get it to render arbitrary HTML (potentially even JS), served from a trustworthy domain. If the response is JSON, which gets only rendered by the extension this would not work.

@trieloff
Copy link
Contributor

@lkrapf I don't share the assumption that the editor is running on the same domain as the site and I'd also assume that the preview function itself requires authentication.

@rofe
Copy link
Contributor Author

rofe commented Apr 26, 2019

Ah, so CORS is a different issue, independent of the content-type. Sorry for the confusion.

@lkrapf
Copy link

lkrapf commented Apr 26, 2019

@trieloff

I don't share the assumption that the editor is running on the same domain as the site

Neither do I. The attack works as follows:

  1. The attacker makes you visit attacker.com
  2. Your browser will automatically POST some attacker controlled content to trusteddomain.com/preview.json

If the response is HTML you will get it rendered in the browser served from (even signed by) trusteddomain.com, with no indication where the content is originally coming from.

and I'd also assume that the preview function itself requires authentication.

Then this attack turns into CSRF - which is yet another issue.

I agree with @tripodsan. The best option would be to turn this into an API.

@kptdobe
Copy link
Contributor

kptdobe commented Aug 28, 2019

Closed during backlog grooming session during hackathon. Reopen if needed or open a new one.

@kptdobe kptdobe closed this as completed Aug 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants