Page $owns does not respect asset canPublish

[mark-a-j-adriano]

Error scenario:

• An asset has an Expiry date (we do not own the image and we are only allowed to use the image for a certain timeframe.)
• We need to prevent the image to be published if an ExpiryDate field has not been added by the Content Editor.
• We can still publish the parent page as it is only the asset that has restriction

Expected:

• Page can be published while its image should remain in Draft mode.

Project setup:

• Followed the steps here : https://docs.silverstripe.org/en/4/getting_started/
• Add a file extension -> (simple canPublish to false)
• Add upload field to Page
• Dev build as usual

How to re-create:

• Add a page and insert a Photo
• Upload the photo (as you can see no publish button will show up as I removed it via canPublish)
• Go back to the page
• Hit Publish
• Attached photo is also published.

---

FileExtension:

<?php

namespace App\Extensions;

use SilverStripe\ORM\DataExtension;

class FileExtension extends DataExtension
{
    /**
     * Don't allow a file to be published if there is no Expiry details
     */
    public function canPublish($member = null)
    {
        return false;
    }

    /**
     * Check if this file can be published
     *
     * @param Member $member
     * @return boolean
     */
    public function extendCanPublish($member = null)
    {
        return false;
    }
}

---

Page:

<?php

namespace {

    use SilverStripe\AssetAdmin\Forms\UploadField;
    use SilverStripe\Assets\Image;
    use SilverStripe\CMS\Model\SiteTree;

    class Page extends SiteTree
    {
        private static $db = [];

        private static $has_one = [
            'Photo' => Image::class,
        ];

        private static $owns = [
            'Photo',
        ];


        public function getCMSFields()
        {
            $fields = parent::getCMSFields();

            $fields->removeFieldFromTab('Root.Main', 'ParentURL');

            $fields->addFieldToTab('Root.Attachments', UploadField::create('Photo'));

            return $fields;
        }
    }
}

---

Extension:

---
Name: file-assets
After:
  - "asset-admin/*"
  - "versioned/*"
  - assetadmin
---
SilverStripe\Assets\File:
  extensions:
    - App\Extensions\FileExtension

PRs

• Add canPublish check for ChangeSet #384

[kinglozzer]

I’m not sure of the best approach to fix this - instinctively I think that the changeset API itself should avoid these checks because it should always be possible to publish items programatically and ignore permission checks when doing so, and adding a requirement to either create or find an appropriate user to “act as” feels like a step in the wrong direction to me. We’ve taken steps to avoid that before, essentially saying “permission checks should be performed elsewhere”: #113

Perhaps we need to add an API to handle this? We currently have publishRecursive() which will build a changeset, add all owned items and publish them, regardless of permissions. We could add a second publishRecursive() method (or perhaps a boolean arg) for whether to apply permission checks when adding items to the changeset - so if those checks are enabled and an owned item doesn't have canPublish() permissions, it doesn’t get added to the changeset in the first place?

I think this does have potential BC risk, especially if we alter the behaviour of existing methods/APIs

[GuySartorelli]

I hadn't considered the angle of publishing programatically, that's a good point.
I'd be in favour of a boolean arg on the publish method(s) to determine if permissions should be respected - imo that would be true by default (i.e. do respect permissions) but obviously that would be a breaking change and would have to be done in a major release.

[mark-a-j-adriano]

What if we add a new static boolean $recursive_publish_check Data extension on dataObject and it defaults to true? So before doing the $object->publishSingle on changeSetItem we check if the object has the static variable set and if so then check the objects canPublish. (just writing this down) I will write a new PR when I get the chance. It will still be a BC but users now has more option plus the fact that automated publishing task without a user can still work (I think, maybe incorrect)

[maxime-rainville]

We had a chat about this. We can't fully address this it a minor and it's too late to make the CMS5 cut off.

A solution like what @mark-a-j-adriano could probably be shipped in a minor.