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

[WIP] Remove authentication for public endpoints #4251

Closed
wants to merge 3 commits into from
Closed

[WIP] Remove authentication for public endpoints #4251

wants to merge 3 commits into from

Conversation

recursivefunk
Copy link

WIP solution for #4181

Feedback welcome!

Description:

There is a file core/server/api/publicEndpoints.js that defines all of the publicly available endpoints. These are represented as an array of objects. Each object contains a regex which matches the endpoint, an array allowable verbs (just GET for now - probably forever?), and optionally an array of objects which represent the allowable get queries.

In the middleware, before authentication occurs, the request path is passed along to the publicEndpoints module. The module tries to exist ASAP. If it doesn't match a public regex, false is immediately returned. Otherwise a check on the request method (GET) is performed, if it is anything other than the whitelisted verb, the test fails. Finally, get parameters are checked, if any extra or unexpected queries are sent along the entire test fails. If all the tests pass, then the module returns true and the middleware bypasses authentication sending an empty object instead of a user. This breaks a couple of the operations because a user model is expected. A potential solution is to have a built in Public API user which is used only for public facing API calls.

This seemed like a good approach as it makes it simple to add public calls (just add an object with the appropriate regex for the route, allowed verbs and allowed queries) and removing a public route just means removing the corresponding object.

Right now, the regexes are hard coded which isn't that big of a deal with the exception of the API version. This should be dynamically built based on a configuration so that when the API version updates, this code doesn't need to be changed.

@ErisDS
Copy link
Member

ErisDS commented Oct 8, 2014

Hi @jrayaustin, would you mind please updating your PR with a few details about your proposed solution and why you chose it? It's really helpful to have a bit of prose to go with code and without it we've no idea what the thought process was. Also, worth reviewing the contribution guidelines before you de-WIP this.

Also, I feel like my comment on the issue may have been misinterpreted. We really are looking for people to get stuck in and help us ship the updated API to the public and this issue is one of them - but there are a number of dependencies in terms of the API security and finishing the oAuth work that means this PR is going to sit around for a while before it can be merged. I'm not sure if you realised that?

@sebgie
Copy link
Contributor

sebgie commented Oct 8, 2014

Hi @jrayaustin and welcome to Ghost!

I know that public access to the API is one of the most wanted features at the moment. We aim to open the API in a controllable way and use OAuth and API keys to give the operator of a blog the tools to allow or deny access to private and public endpoints (see #4004).

This PR implements the very last part of a chain of other issue that are needed to make this happen:

Preferred, but not required provided none of the currently unsecured API endpoints are opened

  • Permissions Migration
  • Missing API permissions

Required

Bonus:

  • Ghost frontend client

@recursivefunk
Copy link
Author

@ErisDS I totally understand this is will sit for a while but I figured if I can help move things along that would be great. I'll update the description some point today to describe what's happening and yes, I totally need to get all the guideline stuff in here too.

@sebgie thanks for the welcome! Looking forward to contributing in any way I can.

@recursivefunk
Copy link
Author

@ErisDS New description.

@recursivefunk
Copy link
Author

Can someone help me get a rough handle on what's required in order to get an initial user created without any user input? @ErisDS

@ErisDS
Copy link
Member

ErisDS commented Oct 16, 2014

I'm sorry @jrayaustin I don't understand what you mean by this? I am also not sure how this PR fits in with the planned OAuth work.

Requests to the API require a client_id - as @sebgie explained there are a number of open issues around the process of making this happen.

@recursivefunk
Copy link
Author

Hi @ErisDS this is in response to #4181. People want to be able to access certain endpoints without any type of authentication (maybe except for domain restrictions) that's what I was hoping to accomplish.

@ErisDS
Copy link
Member

ErisDS commented Oct 16, 2014

Endpoints which can be accessed without authentication still require authorization - the client_id is still needed to give blog owners control over who can access their content via the API.

Perhaps it would help to have a read through #4004 and the related child issues so that you understand what the end goal is here? #4181 is not a standalone issue - it is dependent on the work to add the client ID being completed first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants