How to limit object access to $has_one Member in Silverstripe?

Question

I'm working on a Silverstripe 4.3.1 project which has an object with an owner member attached via $has_one:

class Object extends DataObject
{
    private static $has_one = [
        'Member' => Member::class,
    ];

We want to limit the ability to view/ edit the object to Admins & the owner member.

Here is the code we've used:

public function canView($member = null)
{
    return Permission::check('ADMIN') or
        $this->Member()->ID === Security::getCurrentUser()->ID or
        $this->Member()->ID === $member->ID;
}

public function canEdit($member = null)
{
    return Permission::check('ADMIN') or
        $this->Member()->ID === Security::getCurrentUser()->ID or
        $this->Member()->ID === $member->ID;
}

From what I can tell this used to work, but recent framework upgrades or code changes have broken it.

We are currently getting the following PHP error:

Trying to get property of non-object on the lines containing $this->Member()->ID

Can anyone point me in the right direction for how to fix these errors?

score 2 · Accepted Answer · answered Feb 27 '19 at 01:18

It may be that some Object instances do no have a Member set. In those cases calling this->Member()->ID will error as Member() returns null.

First we should check if $this->Member() is for the Object. If it is not we can return false.

public function canView($member = null)
{
    if (Permission::check('ADMIN')) {
        return true;
    }

    if (!$this || !$this->exists()) {
        return false;
    }

    if (!$this->Member() || !$this->Member()->exists()) {
        return false;
    }

    if ($this->Member()->ID === $member->ID) {
        return true;
    }

    if ($this->Member()->ID === Security::getCurrentUser()->ID) {
        return true;
    }

    return false;
}

public function canEdit($member = null)
{
    if (Permission::check('ADMIN')) {
        return true;
    }

    if (!$this || !$this->exists()) {
        return false;
    }

    if (!$this->Member() || !$this->Member()->exists()) {
        return false;
    }

    if ($this->Member()->ID === $member->ID) {
        return true;
    }

    if ($this->Member()->ID === Security::getCurrentUser()->ID) {
        return true;
    }

    return false;
}

As a side note, it's usually good practice to return false when you know it shouldn't be allowed, but otherwise return null to allow components higher up to perform checks as well. If you return true then you remove any higher logic to run. Example: `Permission::check('ADMIN')` is done in `DataObject::canView()` so you could do something like `if (!$this->Member()->exists()) { return false } else return parent::canView($member)` or something (haven't looked that closely at the actual question example) — scrowler, Feb 27 '19 at 04:16

How to limit object access to $has_one Member in Silverstripe?

1 Answers1