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

API: Public endpoint permission handling (public access rules) #5614

Closed
1 of 8 tasks
ErisDS opened this issue Jul 29, 2015 · 0 comments
Closed
1 of 8 tasks

API: Public endpoint permission handling (public access rules) #5614

ErisDS opened this issue Jul 29, 2015 · 0 comments
Assignees
Labels
affects:api Affects the Ghost API

Comments

@ErisDS
Copy link
Member

ErisDS commented Jul 29, 2015

Ghost's API has a number of endpoints which should be available for what we refer to as 'public' access. Essentially, any data which can be accessed legitimately from the frontend of a blog, ought also to be accessible through other clients. As described in #4004, public access will still require a client ID, so that this access can be revoked.

The frontend controller code which handles outputting data via a theme is a client of the API, so is the {{#get}} helper (#4439), and also javascript code in the theme which may make requests via AJAX. Our plan is to have default clients which automatically get access, just like frontend controller/theme code.

Similarly to the frontend controller, the {{#get}} helper uses the internal method-call version of the API, so it doesn't need a client id. Each blog will get a default client id which is made available to any JavaScript in a theme (issue #4184) so that it can make ajax calls, and eventually it will be possible to register custom clients as well.

The endpoints that will be public in the first iteration, will be the browse & read endpoints of the post, tags and user resources. Several of the authentication flow endpoints are already public as they happen before auth is available. Every other endpoint should remain behind user auth for the time being. Settings and config will need consideration for public access shortly after, and the other endpoints will become available with the introduction of full oAuth 2.0 flows for clients.

The Rules

Several of the endpoints which are public have very strict rules around what information that can be accessed from that endpoint when the request is public.

  • Posts: - only published posts may be accessed, unless a draft is specifically requested via the UUID (which cannot be looked up via the public API).
  • Users: - only active users may be accessed, email addresses and passwords must not be returned
  • Tags: - no rules yet, but private/hidden tags may change this in future

Tasks / Checklist

The following tasks are required to complete this work:

  • make the posts, tags, and users browse & read endpoints accessible for public requests
  • ensure that draft posts and inactive users cannot be accessed by public requests
  • ensure that the rules remain in place when making requests across relations, e.g. tag.posts or tag.posts_count
  • ensure that the rules remain in place when the filterparameter is used (API: Filter parameter (GQL filter queries spec) #5604)
  • ensure that user email addresses and passwords are not returned for public requests
  • ensure that no other endpoints can be accessed
  • determine what the public access behaviour should be for blogs which are marked as private (all HTTP requests to the api should require the same cookie?)

A note on the settings endpoint: when made publicly accessible, settings will have the rule that only settings with type blog and theme can be accessed publicly. The private blog isPrivate and password settings are currently incorrectly classified as blog settings and would need to be changed before the settings endpoint can be made public.

  • Fix settings endpoint
@ErisDS ErisDS added the affects:api Affects the Ghost API label Jul 29, 2015
@ErisDS ErisDS added this to the Current Backlog milestone Jul 29, 2015
ErisDS added a commit to ErisDS/Ghost that referenced this issue Aug 3, 2015
refs TryGhost#4004, TryGhost#5614

- added new public permission handling functions to permissions
- added a new util to handle either public permissions or normal permissions
- updated posts, tags and users endpoints to use the new util
- added test coverage for the new code
ErisDS referenced this issue in sebgie/Ghost Aug 23, 2015
…hen migrating

closes TryGhost#5298
- remove all harcoded instances of jQuery throughout the front-end of the blog
- add migration function to add cdn link to ghost_foot code injection when migrating up from version 003
- migration version bump
@ErisDS ErisDS mentioned this issue Aug 26, 2015
31 tasks
sebgie pushed a commit to sebgie/Ghost that referenced this issue Aug 31, 2015
refs TryGhost#5614 and TryGhost#5503

- update private blog type, including update to settings.edit
- switch order of populate settings & update fixtures + populate all settings

Private blog settings should not be returned by public endpoints
therefore they need a type which is not `blog` or `theme`.
`core` doesn't suit either, as those settings don't usually have UI
To resolve this, I created a new type `private` which can be used
for any setting which has a UI but should not be public data
ErisDS added a commit to ErisDS/Ghost that referenced this issue Sep 1, 2015
refs TryGhost#5614 and TryGhost#5503

- update private blog type, including update to settings.edit
- switch order of populate settings & update fixtures + populate all settings

Private blog settings should not be returned by public endpoints
therefore they need a type which is not `blog` or `theme`.
`core` doesn't suit either, as those settings don't usually have UI
To resolve this, I created a new type `private` which can be used
for any setting which has a UI but should not be public data
ErisDS added a commit to ErisDS/Ghost that referenced this issue Sep 2, 2015
refs TryGhost#5614 and TryGhost#5503

- update private blog type, including update to settings.edit
- switch order of populate settings & update fixtures + populate all settings

Private blog settings should not be returned by public endpoints
therefore they need a type which is not `blog` or `theme`.
`core` doesn't suit either, as those settings don't usually have UI
To resolve this, I created a new type `private` which can be used
for any setting which has a UI but should not be public data
@ErisDS ErisDS modified the milestone: Current Backlog Oct 9, 2015
@ErisDS ErisDS added this to the Public API v1 milestone Oct 13, 2015
@ErisDS ErisDS mentioned this issue Oct 20, 2015
24 tasks
@ErisDS ErisDS assigned ErisDS and unassigned sebgie Nov 11, 2015
@ErisDS
Copy link
Member Author

ErisDS commented Nov 11, 2015

This issue is a catch-all for the work of checking and fixing any issues with permissions now that public api access is possible.

At the moment, two of the checks listed in the original issue are definitely failing:

  • ensure that the rules remain in place when making requests across relations, e.g. tag.posts or tag.posts_count
  • ensure that the rules remain in place when the filter parameter is used

I'm working on a PR to fix the latter of these two first, as it requires a new approach to permissions, and I believe once I've cracked that then handling counts will become easier, and it will be worthwhile adding tests for the other checks.

To ensure the PR makes some sense when it lands, I wanted to outline this new approach in detail.

TLDR;

With complex API queries which may perform subqueries, joins or complex filters we need to apply access rules for any resource that is requested as part of a query. E.g. if we fetch tags, and order by post count, we need to make sure the post count requested correctly applies the access rules.

By using the filtering syntax we already have access to, we can combine access rules, defaults and custom filters into a set of where statements which will correctly apply the rules by using groups:

(enforced filter expression) and (custom filter expression)

Making sure it's easy to get hold of the access rules & defaults for each resource, combine them, and apply them, will provide a way for us to ensure that even very complex queries result in the right data being fetched.


Checking access rules

Currently, the majority of our permissions handling takes the form of a 'check':

Does the request in progress have the rights to perform the actions it is attempting?

In terms of get requests, that is checking if a user or client is allowed to fetch the data it is asking for based on whether the request is public or private (authenticated) and if it is public, using the public access rules.

The problem we have with checks, is making them granular enough to cover the complexity of API browse requests. It's easy to say 'can this user fetch any posts?' it's hard to check 'does this query do anything that would cause drafts to be returned because that's not allowed'.

In the older version of the API it wasn't too hard to see if a request for posts will result in drafts being returned because we only have to check the status parameter. With the filter parameter, we have to be able to check all kinds of queries, E.g. status:draft,status:published or status:[draft,published] - this is quite tricky.

Applying access rules to a query.

To ensure that we always return data which conforms to the rules, the current implementation has the concept of 'default' clauses which get added to the query. So for posts, we add a where status = 'published' clause to the query. This, rather than a 'check' is applying the rules to the query to enforce them. We just have to figure out when to apply the rules and when not to.

Having a simple, easy way to always 'apply' the rules to any query, will result in us being able to do all kinds of complex joins and filters and always be able to ensure the data returned is permitted.

However, the current version can be easily tricked by using an 'OR'

E.g: filter="status:published,status:draft"

Resulting SQL: where status="published" and status="published" or status="draft"

Any combination of filter query which includes 'or status is draft' will override the first clause.

To solve this, we need to use groups in SQL to enforce that the first set of clauses must always be true. E.g. where (status="published") and (custom filter goes here)

Additionally important here, is that as well as the 'public access rules' we also have, for posts, a concept of a default which can be overridden in the form of page:false to ensure we don't return pages by default.

Adding all this together and we have 3 different types of filter that we might want to apply:

Types of filter statement

We have the following filter statement types we need to combine:

  1. enforced filters (access rules, and other business logic)
  2. default filters (overridable default behaviour)
  3. custom, user provided filters

Enforced filters must be applied to the query no matter what. This is achieved by doing:

(enforced filter expression) and (custom filter expression)

Default filters should be applied to the query, unless they are overridden by the custom filter expression. Overridden filter statements are removed until the default filter expression is reduced down to only those which are not set by the custom filter expression.

Default filters and Enforced filters are all simple single clauses, without groups, combined with 'and', which allows us to do:

(enforced filter expression and reduced default filter expression) and (custom filter expression)

We don't have to put defaults in the first group, but this helps when debugging or reading back queries, the first group is from the 'system', and the second is user-provided.

Posts example:

enforced filters: public request ? status:published : none;
default filters: public request ? page:false : page:false+status:published;

public request ?filter=page:true -> (status:published)+(page:true) -> all published pages
public request ?filter=status:draft -> (status:published+page:false)+(status:draft) -> no posts
private request ?filter=page:true -> (status:published)+(page:true) -> all published pages
private request ?filter=status:draft -> (page:false)+(status:draft) -> all draft posts
private request ?filter=status:[draft,published] -> (page:false)+(status:[draft, published]) -> all posts

One consideration is whether or not outputting 'no posts' for a public API request that tries to get draft posts is correct or whether this should throw an error. Either should be possible, throwing an error requires a bit of extra processing.

Combining filters to apply

The key idea in all of this is boiling down the rules for our queries into a set of easily applicable 'filter' rules: enforced, default, and any custom filters.

Adding the ability to easily fetch & combine these filters for the current request no matter what resource we're requesting and then apply them to a query for a resource means we can always apply our rules to nested queries, joins, filters etc just as easily as we can to a simple request.

This is what I'm aiming for 😁

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

- change isPublicContext to detectPublicContext
  - behaviour now expands the context object out
  - this is a bit of a sideeffect, but this is the simplest change
    that makes it possible to use the context in the model layer without
    significant wider changes
- add new access rules plugin
  - takes a context object as part of `forge()` & caches it on the model instance
  - provides helper functions for testing access rules later on
@ErisDS ErisDS mentioned this issue Nov 16, 2015
ErisDS added a commit to ErisDS/Ghost that referenced this issue Nov 17, 2015
refs TryGhost#5614, TryGhost#5943

- adds a new 'filter' bookshelf plugin which extends the model
- the filter plugin provides handling for merging/combining various filters (enforced, defaults and custom/user-provided)
- the filter plugin also handles the calls to gql
- post processing is also moved to the plugin, to be further refactored/removed in future
- adds tests showing how filter could be abused prior to this commit
ErisDS added a commit to ErisDS/Ghost that referenced this issue Nov 18, 2015
refs TryGhost#6009, TryGhost#5614

- Use the new isPublicContext method to detect whether to add extra clauses to the count
- Add count to users
@ErisDS ErisDS modified the milestone: Public API v1 Dec 3, 2015
@ErisDS ErisDS added the later [triage] Things we intend to work but are not immediate priority label Sep 20, 2016
@ErisDS
Copy link
Member Author

ErisDS commented Sep 20, 2016

I'm closing most API issues temporarily with the later label.

JSON API Overhaul & OAuth access are currently scheduled next on the roadmap

@ErisDS ErisDS closed this as completed Sep 20, 2016
@ErisDS ErisDS mentioned this issue Sep 22, 2017
18 tasks
@ErisDS ErisDS removed later [triage] Things we intend to work but are not immediate priority labels Jan 23, 2019
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

2 participants