Skip to content

Commit

Permalink
Allow anonymous access to Users API but exclude details
Browse files Browse the repository at this point in the history
Summary:
- Restore allowed_methods to post and layers API
- Use CRUD privs not REST methods for for allowed* response
- Allow read access to all users, but strip most details in formatter

Test Plan:
- bin/behat
- Check API responses in browser console
  - allow_privileges should now get returned for posts
  - Should see read/create/update/delete, instead of REST verbs
  - Access users api with client creds should see only username and realname

Reviewers: vladimir, aMoniker

Reviewed By: aMoniker

Differential Revision: https://phabricator.ushahidi.com/D666
  • Loading branch information
rjmackay committed Feb 19, 2015
1 parent 5c8be4b commit 14682cc
Show file tree
Hide file tree
Showing 9 changed files with 80 additions and 8 deletions.
2 changes: 2 additions & 0 deletions application/classes/Ushahidi/Core.php
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,10 @@ public static function init()
'form',
'form_attribute',
'form_group',
'layer',
'media',
'message',
'post',
'tag',
'user',
'set',
Expand Down
4 changes: 4 additions & 0 deletions application/classes/Ushahidi/Formatter/Layer.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,12 @@
* @license https://www.gnu.org/licenses/agpl-3.0.html GNU Affero General Public License Version 3 (AGPL3)
*/

use Ushahidi\Core\Traits\FormatterAuthorizerMetadata;

class Ushahidi_Formatter_Layer extends Ushahidi_Formatter_API
{
use FormatterAuthorizerMetadata;

protected function get_field_name($field)
{
$remap = [
Expand Down
2 changes: 1 addition & 1 deletion application/classes/Ushahidi/Formatter/Media.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ protected function add_metadata(Array $data, Entity $media)
'thumbnail_height' => $thumbnail_height,

// Add the allowed HTTP methods
'allowed_methods' => $this->getAllowedMethods($media),
'allowed_methods' => $this->getAllowedPrivs($media),
];
}

Expand Down
3 changes: 3 additions & 0 deletions application/classes/Ushahidi/Formatter/Post.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@
*/

use Ushahidi\Core\Tool\Formatter;
use Ushahidi\Core\Traits\FormatterAuthorizerMetadata;

class Ushahidi_Formatter_Post extends Ushahidi_Formatter_API
{
use FormatterAuthorizerMetadata;

protected function get_field_name($field)
{
$remap = [
Expand Down
8 changes: 7 additions & 1 deletion application/classes/Ushahidi/Formatter/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class Ushahidi_Formatter_User extends Ushahidi_Formatter_API
{
use FormatterAuthorizerMetadata;

public function __invoke($user)
public function __invoke($user)
{ // prefer doing it here untill we implement parent method for filtering results - mixing and matching with metadata is just plain ugly
$data = parent::__invoke($user);

Expand All @@ -26,6 +26,12 @@ public function __invoke($user)
unset($data['password']);
}

if (!in_array('read_full', $data['allowed_privileges']))
{
// Remove sensitive fields
$data = array_intersect_key($data, array_fill_keys(['id', 'url', 'username', 'realname', 'allowed_privileges'], TRUE));
}

return $data;
}
}
42 changes: 42 additions & 0 deletions application/tests/features/api.users.feature
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,48 @@ Feature: Testing the Users API
Then the response is JSON
And the response has a "id" property
And the type of the "id" property is "numeric"
And the "username" property equals "robbie"
Then the guzzle status code should be 200

Scenario: Finding a User as admin gives full details
Given that I want to find a "User"
And that its "id" is "3"
And that the request "Authorization" header is "Bearer defaulttoken"
When I request "/users"
Then the response is JSON
And the response has a "id" property
And the type of the "id" property is "numeric"
And the "username" property equals "test"
And the "email" property equals "[email protected]"
Then the guzzle status code should be 200

Scenario: Loading own user gives full details
Given that I want to find a "User"
And that its "id" is "me"
And that the request "Authorization" header is "Bearer testbasicuser"
When I request "/users"
Then the response is JSON
And the response has a "id" property
And the type of the "id" property is "numeric"
And the "username" property equals "robbie"
And the "email" property equals "[email protected]"
Then the guzzle status code should be 200

Scenario: Finding a User as anonymous user gives partial details
Given that I want to find a "User"
And that its "id" is "1"
And that the request "Authorization" header is "Bearer testanon"
When I request "/users"
Then the response is JSON
And the response has a "id" property
And the type of the "id" property is "numeric"
And the response has a "realname" property
And the response has a "username" property
And the response does not have a "email" property
And the response does not have a "logins" property
And the response does not have a "failed_attempts" property
And the response does not have a "last_login" property
And the response does not have a "last_attempt" property
Then the guzzle status code should be 200

Scenario: Finding a non-existent user
Expand Down
19 changes: 17 additions & 2 deletions src/Core/Tool/Authorizer/UserAuthorizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@ class UserAuthorizer implements Authorizer
// It uses `PrivAccess` to provide the `getAllowedPrivs` method.
use PrivAccess;

/**
* Get a list of all possible privilges.
* By default, returns standard HTTP REST methods.
* @return Array
*/
protected function getAllPrivs()
{
return ['read', 'create', 'update', 'delete', 'read_full'];
}

/* Authorizer */
public function isAllowed(Entity $entity, $privilege)
{
Expand All @@ -51,8 +61,13 @@ public function isAllowed(Entity $entity, $privilege)
return false;
}

// Regular user should be able to update and read only self
if ($this->isUserSelf($entity) && in_array($privilege, ['read', 'update'])) {
// Regular user should be able to update and read_full only self
if ($this->isUserSelf($entity) && in_array($privilege, ['update', 'read_full'])) {
return true;
}

// Regular user can always read
if ($privilege === 'read') {
return true;
}

Expand Down
6 changes: 3 additions & 3 deletions src/Core/Traits/FormatterAuthorizerMetadata.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
/**
* Ushahidi Formatter + Authorizer Trait
*
* Injects "allowed_methods" into formatted data using an Authorizer.
* Injects "allowed_privileges" into formatted data using an Authorizer.
*
* @author Ushahidi Team <[email protected]>
* @package Ushahidi\Platform
Expand All @@ -26,7 +26,7 @@ public function setAuth(Authorizer $auth)
return $this;
}

protected function getAllowedMethods(Entity $entity)
protected function getAllowedPrivs(Entity $entity)
{
if (!$this->auth) {
throw new \LogicException('Authorizer must be defined by calling setAuth');
Expand All @@ -39,7 +39,7 @@ protected function getAllowedMethods(Entity $entity)
protected function add_metadata(Array $data, Entity $entity)
{
return $data + [
'allowed_methods' => $this->getAllowedMethods($entity),
'allowed_privileges' => $this->getAllowedPrivs($entity),
];
}
}
2 changes: 1 addition & 1 deletion src/Core/Traits/PrivAccess.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ trait PrivAccess
*/
protected function getAllPrivs()
{
return ['get', 'post', 'put', 'delete'];
return ['read', 'create', 'update', 'delete'];
}

// Authorizer
Expand Down

0 comments on commit 14682cc

Please sign in to comment.