silverstripe / silverstripe-restfulserver

RestfulServer module for Silverstripe CMS
http://www.silverstripe.org/restfulserver-module/
BSD 3-Clause "New" or "Revised" License
45 stars 48 forks source link

Inconsistent Authentication Behaviour #104

Open phptek opened 1 year ago

phptek commented 1 year ago

Silverstripe 4.11 / PHP 7.4 / Module 2.5.0

Having configured the module to return a Basic Auth prompt by manipulating Page::canView() appropriately, I have observed inconsistencies with this module's behaviour:

Observed Behaviour

Scenario #1: When accessing a specific resource such as api/v1/page/1 and being not logged-in via Basic Auth, I am prompted for username and password.

Scenario #2: When accessing a generic resource for a collection of pages such as /api/v1/page/?start=10&limit=100 and being not logged-in via Basic Auth, I simply see {"totalSize":0,"items":[]} with an HTTP 200.

Expected Behaviour

Scenario #1: As above Scenario #2: I should either be prompted to login or see the above JSON snippet with an HTTP 401, not 200.

phptek commented 1 year ago

Modifying RestfulServer::getSearchQuery() as follows, seems to fix it (it's not very performant, there may be other pre-existing iteration routines where this logic is better applied)

    protected function getSearchQuery(
        $className,
        $params = null,
        $sort = null,
        $limit = null,
        $existingQuery = null
    ) {
        if (singleton($className)->hasMethod('getRestfulSearchContext')) {
            $searchContext = singleton($className)->{'getRestfulSearchContext'}();
        } else {
            $searchContext = singleton($className)->getDefaultSearchContext();
        }
        //return $searchContext->getQuery($params, $sort, $limit, $existingQuery);
        $list = $searchContext->getQuery($params, $sort, $limit, $existingQuery);

        foreach ($list as $item) {
            if (!$item->canView()) {
                $this->permissionFailure();
                break;
            }
        }

        return $list;
    }
phptek commented 1 year ago

Related and fixable at the same time is the following (un-modified code):

This is OK:

#> curl -k -H "Authorization: Basic YXBpc0Bjb25zdW1lci5jb203Li4vC" https://localhost:8080/api/v1/page/1

....blob of JSON... + HTTP 200

This is not:

#> curl -k -H "Authorization: Basic YXBpc0Bjb25zdW1lci5jb203Li4vC" https://localhost:8080/api/v1/page/?start=10&limit=100

{"totalSize":0,"items":[]} + HTTP 200

It gets worse:

This is OK:

#> curl -k -H "Authorization: Basic BADPASSWORD" https://localhost:8080/api/v1/page/1

"You don't have access to this item through the API" + HTTP 401

This is not:

#> curl -k -H "Authorization: Basic BADPASSWORD" https://localhost:8080/api/v1/page/?start=10&limit=100

{"totalSize":0,"items":[]} + HTTP 200
mlewis-everley commented 1 year ago

So I have also bumped into this issue. It appears RestfulServer only checks view permissions on each record in the list (rather than on the endpoint itself). I am guessing this is because GET requests can either be made to view as list or a single item?

I was able to circumvent this issue by creating my own separate version of RestfulServer and re-routing the SilverStripe URL's to this instead. My custom version added additional permissions for the GET endpoint (based on ClassName):

use SilverStripe\Security\Permission;
use SilverStripe\RestfulServer\RestfulServer;
use SilverStripe\Security\PermissionProvider;

class CustomRestfulServer extends RestfulServer implements PermissionProvider
{
    private static $endpoint_get_permissions = [
        MyDataObject::class => ['API_VIEW_OBJECTS']
    ];

    protected function getHandler($className, $id, $relationName)
    {
        $global_permissions = $this->config()->endpoint_get_permissions;
        $member = $this->getMember();

        // If permissions required and member not logged in, fail
        if (empty($member) && array_key_exists($className, $global_permissions)) {
            return $this->permissionFailure();
        }

        $result = Permission::checkMember(
            $member,
            $global_permissions[$className]
        );

        if ($result === false) {
            return $this->permissionFailure();
        }

        return parent::getHandler($className, $id, $relationName);
    }

    public function providePermissions()
    {
        return [
            "API_VIEW_OBJECTS => [
                'name' => 'API View All Objects
                'category' => 'API',
            ]
        ];
    }
}

Then, in my routes.yml

---
Name: approutes
After: '#restfulserverroutes'
---
SilverStripe\Control\Director:
  rules:
    'api/v1': 'CustomRestfulServer'

It might be quite useful if the endpoints themselves could have specific permissions mapped to them (Similar to how I believe allowed_actions works currently)?

If not though, the solution above seems to work quite well for me...