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

Add public API endpoint permission handling #5496

Merged
merged 1 commit into from
Aug 4, 2015

Conversation

ErisDS
Copy link
Member

@ErisDS ErisDS commented Jun 28, 2015

This PR moves the rule handling for what data public access can retrieve into a singlular place (inside of the permissions system), and ensure that anyone attempting to get access to data they shouldn't gets a permissions error (rather than having their query modified).

With this PR, the get helper would be able to make GET requests to the Post, User & Tag endpoints only. Once we have the client auth in place, we'll be able to use this to provide read only access to the API over HTTP, and to explicitly hard-code in a client ID for the frontend controller & GET helper if we want (useful for logging where requests come from?).

It probably needs a bit more work to refactor out the handlePermissions functions into a util so the code isn't duplicated 6 times, but it's nearly there :)

refs #4004, #5614

  • added new public permission handling functions to permissions
  • updated posts, tags and users endpoints to handle public permissions or normal permissions
  • added test coverage for the new code

it('No-auth CANNOT browse non-active users', function (done) {
UserAPI.browse({status: 'invited'}).then(function () {
done(new Error('Browse non-active users is not denied without authentication.'));
}, function (err) {

This comment was marked as abuse.

@ErisDS ErisDS changed the title [WPI] Add public API endpoint permission handling [WIP] Add public API endpoint permission handling Jun 29, 2015
@ErisDS ErisDS mentioned this pull request Jun 30, 2015
@ErisDS ErisDS force-pushed the api-public-perms branch 2 times, most recently from b095572 to 684fab5 Compare July 29, 2015 22:19
@ErisDS ErisDS mentioned this pull request Jul 31, 2015
@sebgie
Copy link
Contributor

sebgie commented Aug 2, 2015

@ErisDS I would like to get this PR into master. Do you know what is wrong with the tests? What is needed to remove the WIP tag?

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 ErisDS changed the title [WIP] Add public API endpoint permission handling Add public API endpoint permission handling Aug 3, 2015
@ErisDS
Copy link
Member Author

ErisDS commented Aug 3, 2015

This has been updated to properly handle the uuid behaviour for reads. The handlePublicPermission method has been factored out into a single instance, and the tests have been updated to ensure good coverage.

Note: this now throws a permissions error where a 404 would have occurred in certain places, tests have been updated accordingly.

sebgie added a commit that referenced this pull request Aug 4, 2015
Add public API endpoint permission handling
@sebgie sebgie merged commit cfce197 into TryGhost:master Aug 4, 2015
@ErisDS ErisDS deleted the api-public-perms branch August 26, 2015 14:49
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