Should we check for $pk==0 ? I think not, as article model is not used for new article creation (form model is used in that case)

Why is AppVeyor failing with php7.1?

What wonders me if the $pk variable is always set. Because when not set , then this checks will always return false for none super admin users as there is no asset com_content..

#17725 (comment)

It is wrong to check 'com_content.article.' . $pk for new records, but possibly harmless because the record (the article) does not exist yet in the DB

[EDIT]
i meant by looking where filter_published state setting is being used , and it is being used inside getItem

Yes, $pk can be unset, but this is not related to the present problem with authorisation.

In fact you can always browse a malformed url, lacking id in query, and you'll get an error page: index.php?option=com_content&view=article&Itemid=101 won't work.

But this can be the chance to improve this too: I think that the component should work anyway, also when input from user is not correct, if there is a univocal intended behaviour.
In this case, the explicit article id in the url seems redundant: if the user passes a url with a correct Itemid but without the article id, it is quite sure he wants the article contained in the menuitem query string. So we can retrieve the article id from the menuitem.

I added this input check and improvement to present PR.

I think this was a step into the wrong direction. I would rather do like:

$asset = 'com_content';
if ($pk)
{
    $asset = 'com_content.article.' . $pk;
}
if ((!$user->authorise('core.edit.state', $asset))

Why is that direction wrong?
It makes the component more robust, resilent to erroneous user input.

Because the model should not be aware about any menu. Imagine you use the model in a plugin which gets triggered when displaying a contact, then you will have a menu with a totally wrong id.

@laoneo: You are right. Thanks. Reverted.

The second checks needs the same logic, or not?

I think not, as $pk is already 'sanitized' in getItem at row 79, calling getState('article.id') that default-returns null and is converted to (int) 0.

I ended up here tonight, searching the issues regarding filtering the article model when there is no authorised user. I am working on a plugin and I need to get info for all the articles, disregarding if the user has access to it. Currently it's impossible to get an unpublished or trashed article, as it will result to a 404.

Does this PR fixes that?

I'm not sure I understand your question.
With this PR the model will return all and just the articles the user has permissions to edit.
So if the user has permissions on a specifit article to edit it or to edit its state then the model will return that article if it is published, archived, suspended, expired or pending.

@LivioCavallo

In my system plugin, I am looping over an array of articles id's - trying to gather their data (e.g. catid) by using the model. But if I request an article that is unpublished from the model with getItem($id) - then the model will throw a 404error. So it's impossible to get such information through the model for unpublished or trashed items.

@LivioCavallo is this PR on for Test?

Yes, this PR is ready for Test!
And it solves the problem.

I have tested this item ✅ successfully on 31a121a

Works as expected, test on unpublished, expired and pending.

I have tested this item ✅ successfully on 31a121a

Ready to Commit after two successful tests.