WordPress preview support

@david, I don’t like that we rely on post._links["predecessor-version"] to fetch the preview post.

I analyzed a big REST API response and half of its size was due to the _links so removing that field means half the size of the response. If we rely on _links for this, people would not be able to remove them.

Do you know if there is a case where http://embedded.local/wp-json/wp/v2/posts/1/revisions/?per_page=1 is not good enough?


I also would love to know your opinion on Support for custom headers in Source packages :slightly_smiling_face:

Ok, doing so with /revisions/?per_page=1 seems fine as WordPress seems to be getting the always the last revision.

This is the code that generates the _links["predecessor-version"] link.

$revisions = wp_get_post_revisions( $post->ID, array( 'fields' => 'ids' ) );
$last_revision = array_shift( $revisions );

$links['predecessor-version'] = array(
  'href' => rest_url( trailingslashit( $base ) . $post->ID . '/revisions/' . $last_revision ),
  'id'   => $last_revision,
);

Source: https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php#L1885-L1902

Wow, it is actually the very same as /wp-json/wp/v2/posts/1/revisions/?per_page=1. You can replace it with the other call, of course!

Sure, I’ll give my feedback in the other thread. :+1:

I’ve managed to get a solution that may work to support query handlers in the current version of Source v1, but still requires more research.

This is the (almost) working PR. It also includes a handler for ?p=ID URLs: https://github.com/frontity/frontity/commit/aadd640ae41c96695f2c1c14aa8c9015ee610d66

The solution proposed is to send both route (which is the link without the /page/x part) along with the query to the getMatch function, but only match the query for handlers that have a flag, for example, matchQuery: true.

The only thing I’ve not managed to figure out is how to set isHome. I’ll work on that and I’ll try to research the implications of adding this system to the current version of source.

So, right now the data.isHome flag is set by fetch instead of the handlers. The logic is:

const isHome = route === normalize(state.source.subdirectory || "/");

Code: https://github.com/frontity/frontity/blob/dev/packages/wp-source/src/actions.ts#L71

The problem is that /?p=ID is not the home, but /?utm_campaign=sale is. We need to be able to differentiate between them.

I see two solutions for the isHome problem:

  1. Add a whitelist of queries that are used in URLs to distinguish between.
  2. Move the logic to the handlers.

Sadly, the second one is not backward-compatible because any person who has a custom handler for the home and it’s relying on data.isHome in his/her theme will be affected if we remove it.


Regarding handlers, I’m thinking about these two options:

  1. Add support for preview to the postType handlers and add a new hander for ?p=ID.
  2. Add a new handler for ?preview=true that handles both cases: slug (/some-post) and query (?p=ID).

Both cases have the isHome problem.

I’ve kept working on this. This my last idea:

path-to-regexp patterns are too limited for queries, so I think we can add support for regular expressions if you want to match queries.

  • If you want to match a URL that doesn’t include a query, you use a path-to-regexp pattern:

    pattern: "/category/:slug" to match /category/nature.

  • If you want to match a URL that includes a query, you use a regular expression:

    pattern: "RegExp:(\\?|&)p=\\d+" to match /?p=13 or /?...&=p=13.

Thanks that, we can also differenciate if the URL is from the home or not, maintaining the backward-compatibility:

const isHome =
  !handler.pattern.startsWith("RegExp:") &&
  route === normalize(state.source.subdirectory || "/");
2 Likes

The work in the PR is almost done: https://github.com/frontity/frontity/pull/564

@david is taking care of the last tasks.


As I mentioned in the PR, I think we should change the name of the token. token is too general. It should start at least with frontity_ to avoid collisions and include more information to allow for other types of tokens and versions.

I’m thinking:

  • Starts with frontity because all Frontity related queries should start like that.
  • It’s followed by preview because that its purpose.
  • It’s followed by jwt because that’s the implementation.
  • It ends with a version, in case we need to increase it at some point while maintaining the old implementation.

So something like this: frontity_preview_jwt_token_v1.

Opinions?

1 Like

I like this approach and it seems something we could use for future implementations similar to this one.

We are going to finally use frontity_source_auth as discussed in Support for auth header in Source packages because it won’t only contain the token, but the whole header :slightly_smiling_face:

Just an important note:

The preview plugin uses a Bearer token in the Authentication header, so if you are using Apache as your web server you would need to add the following lines in your .htaccess file in order to support it:

RewriteEngine on
RewriteCond %{HTTP:Authorization} ^(.*)
RewriteRule ^(.*) - [E=HTTP_AUTHORIZATION:%1]
SetEnvIf Authorization "(.*)" HTTP_AUTHORIZATION=$1

We have merged the PR: https://github.com/frontity/frontity-embedded-proof-of-concept/pull/3

We are also going to do a release of Frontity that supports the frontity_source_auth query in the post type handlers later today as discussed in this FD: Auth header in Source packages and Frontity Query Options

Ok, it’s released now :tada:

To get the preview working in Frontity while using the Embedded mode, please:

It should work without any extra work.

Let us know how it goes!! :smile:

4 Likes

If you are using a custom handler for the post types, you need to add support for the preview=true query.

This is an example of the code that is required. Please adapt it to your needs.

// Overwrite properties if is a preview and a token is present in the state.
if (query.preview && state.source.auth) {
  // Fetch the latest revision using the auth token.
  const response = await libraries.source.api.get({
    endpoint: `posts/${data.id}/revisions?per_page=1`,
    params: state.source.params,
    auth: state.source.auth,
  });

  // Get modified props from revision and merge them with the entity.
  const [{ title, content, excerpt }] = await response.json();
  const entity = state.source[data.type][data.id];
  Object.assign(entity, { title, content, excerpt });
}

Preview works fine but images are not getting loaded on the preview. This is what I got in the console.

That’s weird. How are the URLs of the images and how they should be?

URLs are fine but images are not getting loaded, see

You can check it live here
WordPress: https://api.divaksh.com/beginners-guide-google-spreadsheet-using-java/
Frontity: https://divaksh.com/beginners-guide-google-spreadsheet-using-java/

Oh, it’s not the images what is not loaded. It’s the JavaScript. You’re using lazy loading for the images using JavaScript, so that’s the reason the images don’t load.

Once we release the final version of the Embedded mode this will be automatic, but for now you can use the --public-path option to point the correct URL:

npx frontity build --public-path https://divaksh.com/static
2 Likes

Spent some time reviewing and testing this and it works well. Didn’t run into any issues getting it to work. Awesome work!

From a security standpoint, I don’t see any obvious issues but I see some potential for improvements.

Restrict capabilities that can be performed through the jwt token

I see that the approach taken here is very flexible and basically allows the creation of JWT token with any WP capabilities.

However, I wonder if that’s really necessary. If FRONTITY_JWT_AUTH_KEY is leaked, it means that an attacker will be able to craft an arbitrary JWT token with any capability he wants, essentially gaining full control of the WP website through a feature that was built to add preview support. Unless we are trying to build an alternative authentication mechanism to the REST API, I’d suggest we change the approach a bit.

My suggestion here is to take capabilities and allowed_methods out of the JWT payload and replace with a type=preview. Then when reading the token we validate the token and by checking the type we perform the necessary checks for allowed_methods and which capabilities we need to change in order to make that request successful. So even if the auth key is leaked, it would only expose the preview feature.

We should also check if this token was generated by Frontity, either by checking the supported types or adding a “frontity signature” to it. Just to make sure we don’t try to read an token generated with SECURE_AUTH_KEY for other purposes.

Minor: Avoid using glob to load files

In frontity-embedded.php the PoC is using glob to include the files in php-jwt, it’s preferable to just hardcode the files (or use an autoloader).

  $jwt_files = array(
    'BeforeValidException.php',
    'ExpiredException.php',
    'JWK.php',
    'JWT.php',
    'SignatureInvalidException.php',
  );

	// Load php-jwt classes.
	foreach ( $jwt_files as $filename ) {
		require_once plugin_dir_path( __FILE__ ) . '/includes/php-jwt/' . $filename;
  }

Another potential but minor issue is that it is just going to include any files in that folder, which would give attackers a new way of running malicious code if they manage to upload a file to that folder.

2 Likes

Ah, on another note - at first I almost thought that subscribers users would be able preview any posts because we weren’t checking if the user had permission to edit the post when generating the token but as it turns out WordPress will return false in is_preview() if the user logged in has no permission to preview a post.

If we want to be extra careful we could add an extra check to make sure the logged in user can indeed edit/preview a post.

$id = get_the_ID(); 
if ( $id && is_preview() && is_user_logged_in() && current_user_can( 'edit_post', $id ) ) {

Could be an extra layer of security if other plugins are messing with is_preview.

2 Likes

Awesome. Many thanks @nicholasio.oliveira :clap::slightly_smiling_face:

I have opened three issues to fix these problems: