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

Inconsistent Authentication Behaviour #104

Open
phptek opened this issue Feb 1, 2023 · 3 comments
Open

Inconsistent Authentication Behaviour #104

phptek opened this issue Feb 1, 2023 · 3 comments

Comments

@phptek
Copy link
Contributor

phptek commented Feb 1, 2023

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
Copy link
Contributor Author

phptek commented Feb 1, 2023

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
Copy link
Contributor Author

phptek commented Feb 1, 2023

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
Copy link

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...

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

No branches or pull requests

3 participants