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

Helpers for accessing the API from themes #5942

Closed
ErisDS opened this issue Oct 13, 2015 · 22 comments
Closed

Helpers for accessing the API from themes #5942

ErisDS opened this issue Oct 13, 2015 · 22 comments
Labels
affects:api Affects the Ghost API
Milestone

Comments

@ErisDS
Copy link
Member

ErisDS commented Oct 13, 2015

With the changes coming in #5848 it will be possible to make ajax requests to the Ghost API from inside a theme. The API urls will need to be absolute* and to include a client id & secret in order to work correctly.

This means that building a URL will look something like:

<protocol>://<full-path-to-blog>/<api-path-inc-version>/<endpoint>?<query-params>&client_id=<client_id>&client_secret=<client_secret>

Or for example

https://blog.ghost.org/ghost/api/v0.1/posts?limit=3&client_id=ghost_frontend&client_secret=a1b2c3d4e5f6

That's quite a pain to build, especially multiple times, so lets make this super easy by allowing theme developers to do something like (assuming they're using jQuery to do ajax):

$.ajax(window.ghost.apiUrl('/posts/?limit=3'))

The key idea here being, provide a function (via a script included in {{ghost_foot}}), which has no dependencies, and which takes just the endpoint & query params, or rather the guts of the API query, and wraps it with the full URL & adds the client details. The resulting string can be passed to any ajax library.

* It is common to serve the admin and API over HTTPS, whilst keeping the frontend of the blog served over HTTP, in this case, the URL must be absolute to avoid redirects which in some browsers will interfere with allowing cross-origin requests.

@ErisDS ErisDS added themes affects:api Affects the Ghost API labels Oct 13, 2015
@ErisDS ErisDS added this to the Public API v1 milestone Oct 13, 2015
@vadimdemedes
Copy link

There's a PR (#4181) for removing authentication on public endpoints, so I guess there is no need to include client_id and client_secret query parameters in this helper anymore?

I also suggest including these improvements:

  1. Accept an object of query parameters as a second argument
  2. Automatically handle missing leading/trailing slashes in the url
window.ghost.apiUrl('posts'); // => /posts/
window.ghost.apiUrl('/posts'); // => /posts/
window.ghost.apiUrl('posts', { limit: 3 }); // => /posts/?limit=3

@acburdine
Copy link
Member

I can pick this one up if no one's got it

@ErisDS
Copy link
Member Author

ErisDS commented Oct 20, 2015

There's a PR (#4181) for removing authentication on public endpoints, so I guess there is no need to include client_id and client_secret query parameters in this helper anymore?

@vdemedes The PR removes user authentication from the public endpoints, and replaces it with client authentication. This means that rather than needing to exchange an email & password for a token, a client gets an id and a secret that they supply instead. This is 100% required, otherwise the ajax requests won't work 😉

@acburdine Yep by all means pick it up but you might want to wait until the filter param is merged and test with it.

I was also thinking along the lines of what @vdemedes suggested, by using a syntax like window.ghost.apiUrl('posts', { limit: 3 }); this helper will more closely resemble the get helper.

This would also make it easy for us to pass the filter parameter through encodeURIComponent, and probably the same for 'order', so that the user doesn't have to do it.

@ErisDS
Copy link
Member Author

ErisDS commented Oct 22, 2015

Another thought I had on this when I was testing #5848 is that at some point, it would also be useful to also expose our url-building utils.

This got me to thinking, do we really want to be including heaps of JS for every theme, as for most it'll be unnecessary overhead, much like jQuery was.

So, what are potential solutions for optionally including it?

  1. Have a flag that you add via code injection
  2. Have a flag that you add directly to {{ghost_foot}} e.g. {{ghost_foot include="ajax_helpers"}}
  3. Providing a js file via /shared/ that you can simply include as you would any other library
  4. ?? other ideas ??

So far I'm leaning towards the middle one for two reasons:

  • The theme can determine for itself if it wants to use these features and choose to include the scripts
  • Using a code-flag to cause the include means that we can do things like add more options in future, and minify them, include them in our own pipeline.

The only downside is that it's an extra 'optional' thing...

@acburdine
Copy link
Member

why not both 2 & 3? I was thinking about how to implement this exactly and short of hardcoding the actual JS into a string and then appending it to the {{ghost_foot}} helper, the only other way was including a file via /shared/ or some other publicly accessible file endpoint. Although, it might be confusing to have both available.

Also, I wouldn't think the JS would be that complex for this, even if we expose the url building stuff. Especially if it's minified.

@kevinansfield
Copy link
Contributor

I think both 2 and 3 would be very useful. 2 for theme developers and 3 for anyone wanting to integrate into something else?

@vadimdemedes
Copy link

I think option 3 is the best one, because it leaves the choice to theme developer whether to include it or not. If we go with option 3, option 2 becomes obsolete, since developers can just use script tag to include the helper. Plus, it won't add a new parameter to ghost_foot, which means less pain the the future with backwards compatibility.

@ErisDS
Copy link
Member Author

ErisDS commented Oct 23, 2015

I agree it would make sense to have a script loaded via /shared/ using a param in {{ghost_foot}} rather than inlining JS, but I'm not sure that getting theme developers to include it directly provides any benefits in terms of either backwards compatibility or future proofing:

If we encourage users to include a script directly, we have to keep that script around indefinitely. We can't move, rename, or really even change that file. If we add a second script in future for some other purpose, they have to choose to include one, the other, or both. That's ok for a short while but including lots of separate small JS files doesn't scale.

The benefit that I see in using something like {{ghost_foot include="ajax_helpers"}}, is that it leaves the implementation up to us.

We can inline a single function for this version. Next version we can change it to include /shared/js/ajax_helper.js via a script tag. In a later version we can change the file in /shared/ to be a build tool, something like like /shared/js/scripts.js?include=ajax_helpers,url_helpers. All the while the theme code doesn't need to change.

@vadimdemedes
Copy link

@ErisDS You are right, I was thinking from the angle of implementation & usage simplicity, which turned out wrong. With possible situations you pointed out, include parameter makes sense.

Also, I'd change ajax_helpers to api_helpers, seems easier to memorize, imo.

@sebgie
Copy link
Contributor

sebgie commented Oct 23, 2015

I'm leaning towards option 3 as it sounds like a first step in the direction of a Ghost-Client-SDK. The script would need to be dynamic since we want to use values that aren't known during the build process (config.url and client_secret). The script would also need to be customized for different clients. We could do that using query params (/api.js?client-id=ghost-frontend) which would be similar to what @ErisDS describes. In the future we could also include methods to handle more complex oAuth flows.

Including the script in ghost_foot could be done automatically if the client.status parameter is set to enabled. That means if the ghost-frontend client is enabled, the script is automatically included. From a developer perspective it would be quite convenient to know that if the client is enabled the script is automatically included?

@kevinansfield
Copy link
Contributor

The script would need to be dynamic since we want to use values that aren't known during the build process (config.url and client_secret)

This sounds like a different direction to most JS SDK libraries which expect those values to be passed in during an initialisation step? I'd definitely advise against outputting credentials in any files that have the possibility of being shared.

@sebgie
Copy link
Contributor

sebgie commented Oct 23, 2015

@kevinansfield making config.url and client_id easily usable by a developer is the goal of this issue. These values don't grant any special permissions at the moment and need to be shared with the client side JavaScript (similar to Google Maps API). Can you think of a way to share these values with a client without exposing them to every visitor to the site?

@acburdine
Copy link
Member

imo client_id is fine to share, but client_secret isn't. @sebgie you mentioned client_secret earlier, which may be where the objection came from

@kevinansfield
Copy link
Contributor

@sebgie Ah, I don't remember seeing too many JS SDKs that work that way. In any case if used in a browser your API key is always publicly available to every visitor to the site, you can try to restrict unauthorised usage by limiting to specific referrers but that's really simple to spoof.

@sebgie
Copy link
Contributor

sebgie commented Oct 23, 2015

@acburdine: client_secret is probably a bad name for the value. It comes from the RFC but the value is not secret and doesn't grant any permissions. Maybe client_key would be better.

The main purpose of having client_secret is to prevent random access (bots, ...) and being able to change it if you need to. Another reason is to have a differentiation between Ghost installs. client_id is the same, client_secret isn't.

If we deliver Ghost-Client-SDK without client_id, config.url and client_secret there is still the question of how to provide an easy way to get these values when writing JS? Initialize the SDK with helpers {{api.client_id}}, {{api.url}} and {{api.client_secret}}? And let 3rd party sites initialize the values manually?

@ErisDS
Copy link
Member Author

ErisDS commented Oct 23, 2015

Is client_secret being a misnomer something we ought to fix? Should it be client_key or client_token? Also, is providing the client_id strictly necessary> Is that's another point to think about maybe?

As I understand this, the client_id and client_secret together make up the 'api key' that you can use to access the Ghost API. There is nothing private about either of these two pieces of data, they are, at least in this case restricted in where the request can come from based on the Origin header, which is part of the browser implementation.

Taking the facebook SDK as an example, to include it you get given a snippet like this:

<script>
  window.fbAsyncInit = function() {
    FB.init({
      appId      : 'your-app-id',
      xfbml      : true,
      version    : 'v2.4'
    });
  };

  (function(d, s, id){
     var js, fjs = d.getElementsByTagName(s)[0];
     if (d.getElementById(id)) {return;}
     js = d.createElement(s); js.id = id;
     js.src = "//connect.facebook.net/en_US/sdk.js";
     fjs.parentNode.insertBefore(js, fjs);
   }(document, 'script', 'facebook-jssdk'));
</script>

Here, the app_id is not secret (unlike a twitter consumer_secret for example), but is restricted in where the request is allowed to come from. You have to have registered an app with that information inside of facebook in order to get a key.

Additionally, this snippet of code is provided to you to include on a 3rd party website, rather than on facebook itself, and you need to explicitly provide the api key to the 3rd party.

Whilst, long term, we definitely intend to provide an SDK for including on a 3rd party site. For now we just need to make it easy for developers to make API requests to and from their own blog. Whilst this might well be the very start of a Ghost SDK, I think that on your own blog, it ought to be self-initialising?

Including the script in ghost_foot could be done automatically if the client.status parameter is set to enabled.

This is an interesting approach - only thing I am concerned about is automatically including extra stuff on people's blogs that they don't want again, like we had with jQuery, although at least this way there would be a way to switch it off.

@sebgie
Copy link
Contributor

sebgie commented Oct 23, 2015

Is client_secret being a misnomer something we ought to fix?

It is called client_secret because of the specification. Right now we don't treat it as as secret (for clients that can't keep secrets anyway) but it is possible that we will have use cases in the future where it will be secret (for clients that can keep secrets). So if we change the name now it could be confusing in the future? One way out of this problem would be hashing to generate a client_token and use only one value?

including extra stuff on people's blogs that they don't want again, like we had with jQuery

This would be different because if the user doesn't use public API access on the frontend it can be disabled. Access from other websites will have different client_ids and can be enabled without adding anything to the frontend.

@kevinansfield
Copy link
Contributor

This would be different because if the user doesn't use public API access on the frontend it can be disabled. Access from other websites will have different client_ids and can be enabled without adding anything to the front-end.

What about the use case where they want to access the public API from their main website (e.g., pull in latest posts) but not necessarily use it from within the blog itself?

Sorry, just re-read your response. I had thought we were talking about auto-including the script if the user had selected the "Enable public api access" checkbox in labs - my point was that access may be wanted not necessarily for use in the blog itself but on external sites in which case we don't want to inject anything?

@vadimdemedes
Copy link

As for automatically outputting client_id and client_token/client_secret in some dynamic javascript file, I agree with @kevinansfield, this is not a good practice and I haven't seen anything like this implemented in the wild. This approach wouldn't protect against bots, as they could request that file and parse the credentials from there.

When it comes to client-side, there's not much you can do to protect your data. If it's on the client-side, it can be easily read by everyone.

If we need to make some server data accessible from the client, why don't we expose client_id and client_token in tags, just like in Ghost's dashboard?

<meta name="env-clientId" content="ghost-admin" />
<meta name="env-clientSecret" content="6e5816927c41" />

It can be easily read by scripts and we won't have to serve dynamically generated scripts.

If we want to display blog's data on the different origin (different website), we just get another pair of credentials.

@vadimdemedes
Copy link

We could take a step further and provide ultimate compatibility for API endpoints between Ghost versions:

ghost.api.posts.create // => /ghost/api/v0.1/posts/
ghost.api.posts.read(14) // => /ghost/api/v0.1/posts/14/
ghost.api.posts.read('some-slug') // => /ghost/api/v0.1/posts/?slug=some-slug

The idea is provide something like rails url helpers, which ensure that API endpoint changes don't break the whole codebase, which is perfect.

I believe this contributes to smoother API changes and stability across releases. With implemented Stripe-like versioned API in Ghost, we will have a bulletproof API, which never breaks.

What do you think?

@ErisDS ErisDS mentioned this issue Nov 2, 2015
24 tasks
@ErisDS
Copy link
Member Author

ErisDS commented Nov 2, 2015

I'm not sure whether it makes sense to support the full set of BREAD api methods as shown by @vdemedes, or whether it makes more sense to have a single utility for talking to the API treating it as a URL builder, more like the get helper:

ghost.api.url('posts', {limit: 5});

The difficulty with supporting the full BREAD 'api' as it were, is that it looks the same as the code that would usually make the request and return a response, whereas in this case the plan was to create a URL to call with the ajax library of your choice.

To make the api.post.read style completely consistent with the server-side version of the API (which would certainly be nice) we'd have to include some sort of ajax library to do the calls - and then we're back to including jQuery or something which is additional to jQuery - and we only just got rid of all that.

Providing code to help build URLs makes sense because urls to call the API are not the only URLs I can see theme developers wanting tools for. Once I've fetched a bunch of tags, I might reasonably also want to generate the URLs for their pages, or to link to posts or author pages.

So.. with that in mind, perhaps it makes sense to have something structured like:

ghost.url('api', 'posts', {limit: 3}); -> http://myblog.com/mysubdir/ghost/api/v0.1/posts/?limit=3

Which could then be extended in future to allow building more urls like:

ghost.url('post', postData); -> http://myblog.com/mysubdir/my-post-slug/ (or whatever permalink structure is in place)
ghost.url('tag' tagData) -> http://myblog.com/mysubdir/tags/my-tag-slug/

I'm certainly in two minds as to whether this is the most useful thing we could provide. Perhaps it would be more useful to provide a full SDK/ajax api here rather than just url building? However, I'm not sure locking ourselves into providing an ajax library again is a sensible step right now.

As I see it, a url builder is the smallest unit of useful functionality that we can provide without including extra dependencies. It is useful beyond just calling the API, and it doesn't preclude us from adding bread-style api functions later if there is demand?

ErisDS added a commit to ErisDS/Ghost that referenced this issue Nov 4, 2015
refs TryGhost#5942

- refactor ghost_head to use Promise.props (settle is going away and this is easier)
- add a new call to fetch the frontend client, if it exists
- add meta tags for the client_id and client_secret on all pages
- don't include the meta tags if the client is not enabled, or if the labs flag is not set
@ErisDS
Copy link
Member Author

ErisDS commented Nov 4, 2015

@acburdine I've done a PR for the first part of this - adding the meta tags - see #6045

That should give you a framework to look at adding a url helper, if that's something you've got time for.

Doing this made me realise that the script should probably be injected in {{ghost_head}} rather than {{ghost_foot}}? Makes sense to do it here along with the meta data.

panezhang pushed a commit to panezhang/Ghost that referenced this issue Nov 9, 2015
refs TryGhost#5942

- refactor ghost_head to use Promise.props (settle is going away and this is easier)
- add a new call to fetch the frontend client, if it exists
- add meta tags for the client_id and client_secret on all pages
- don't include the meta tags if the client is not enabled, or if the labs flag is not set
@ErisDS ErisDS closed this as completed in 250edf2 Nov 20, 2015
ErisDS added a commit to ErisDS/Ghost that referenced this issue Dec 15, 2015
refs TryGhost#5942, TryGhost#6150

There were a few key problems I was looking to solve with this:

- Introduce a single point of truth for what the URL for accessing the API should be
- Provide a simple way to configure the utility (much like a true SDK)

As of this commit, this utility is still automatically available in a Ghost theme.
To use it on an external site, the code would look like:

```
<script type="text/javascript" src="http://my-ghost-blog.com/shared/ghost-url.min.js"></script>
<script type="text/javascript">
ghost.init({
   clientId: "<your-client-id>",
   clientSecret: "<your-client-secret>"
});
</script>
```

To achieve this, there have been a number of changes:

- A new `apiUrl` function has been added to config, which calculates the correct URL. This needs to be unified with the other url generation functions as a separate piece of work.
- The serveSharedFile middleware has been updated, so that it can serve files from / or /shared and to substitute `{{api-url}}` as it does `{{blog-url}}`.
- ghost-url.js and ghost-url.min.js have been updated to be served via the serveSharedFile middleware
- ghost-url.js has been changed slightly, to take the url from an inline variable which is substituted the first time it is served
- `{{ghost_head}}` has been updated, removing the api url handling which is now in config/url.js and removing the configuration of the utility in favour of calling `init()` after the script is required
- `{{ghost_head}}` has also had the meta tags for client id and secret removed
- tests have been updated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects:api Affects the Ghost API
Projects
None yet
Development

No branches or pull requests

5 participants