User with edit_own permission should be able to load Versions for the item concerned #10836

infograf768 · 2016-06-16T13:14:28Z

Pull Request for Issue #10813

Testing Instructions

create a group "authors team" with parent "registered".
administrator interface access allowed:

following permissions for articles:
Configure ACL & Options - Not Allowed.
Configure Options Only - Not Allowed.
Access Administration Interface - Allowed
Create - Allowed
Delete - Not Allowed.
Edit - Not Allowed.
Edit State - Allowed
Edit Own - Allowed

Logging in as a member of "authors team"
Go to articles manager and open an existing article
Create a new article and save
Open the same article.

Before patch, the user has no access to the Versions button and permission is not granted in the com_contenthistory models.

Patch and test on banner, banner client, contact, article, newsfeed

zero-24 · 2016-06-16T13:23:35Z

administrator/components/com_contenthistory/models/preview.php

-		$result->save_date = $table->save_date;
-		$result->version_note = $table->version_note;
-		$result->data = ContenthistoryHelper::prepareData($table);
+		if ($return = true)


Is This correct? I gues you mean == or === true?

brianteeman · 2016-06-16T13:35:04Z

After applying the patch I can load the versions BUT when I restore a version I am only able to Saveascopy is that correct?

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10836.}

infograf768 · 2016-06-16T13:35:46Z

@zero-24
= chnaged to ==

infograf768 · 2016-06-16T13:36:40Z

After applying the patch I can load the versions BUT when I restore a version I am only able to Saveascopy is that correct?

Hmm, nope. We need something more here.

brianteeman · 2016-06-16T13:41:51Z

I have tested this item 🔴 unsuccessfully on 4b41620

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10836.}

infograf768 · 2016-06-16T14:46:49Z

@brianteeman
It looks lile I do not get the same results. It works here.
It breaks (ir rather does not show the Save button) if the user tries to restore a version which has been made by another user, i.e I am user jms and I try to restore a version made by Super User (see below).
Can you test again?

brianteeman · 2016-06-16T14:47:56Z

Ah that could be true - testing now

On 16 June 2016 at 15:46, infograf768 notifications@github.com wrote:

@brianteeman https://github.com/brianteeman
It looks lile I do not get the same results. It works here.
It breaks (ir rather does not show the Save button) if the user tries to
restore a version which has been made by another user, i.e I am user jms
and I try to restore a version made by Super User (see below).
Can you test again?
[image: screen shot 2016-06-16 at 16 45 21]
https://cloud.githubusercontent.com/assets/869724/16121004/ccb19a1c-33e1-11e6-81a1-da5a2c2d81b5.png

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#10836 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABPH8bhe7hlb1bbGuPDrR9IPprbA9oSzks5qMWHcgaJpZM4I3Xy0
.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

infograf768 · 2016-06-16T14:50:27Z

We anyway have a bug here, as any new version gets its author from the last user who modified it... not from the creator of the original version

brianteeman · 2016-06-16T14:53:02Z

Changed my test to success as the issue I found was when restoring a version from a different author so was the correct behaviour

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10836.}

joomla-cms-bot · 2016-06-16T15:35:34Z

This PR has received new commits.

CC: @brianteeman

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10836.}

ggppdk · 2016-06-16T16:21:52Z

@infograf768
3 comments

the "canDo" is calculated by:
JHelperContent::getActions()

which does not include check "owner check":

$user       = JFactory::getUser();
$userId     = $user->id;
...
 && $this->item->created_by == $userId

we need to add it along with the ACL check like this:

if ($canDo->get('core.edit') || ($canDo->get('core.edit.own') && $this->item->created_by == $userId))

example of existing code:
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_content/views/article/view.html.php#L108

Also records that have an "owner" / creator
are:

category
article
contact
tag

Thus 'core.edit.own' check should be only be added for them

Finally, the edit - ACL check is not really needed at all inside the "display" of the view.html.php

that is both and edit and edit.own were checked by the controller before we reach the display() of the view, if the user does not have create / edit then form will not open

https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_content/controller.php#L42
... but then it only checks for "layout==edit" ... e.g. it will not check ... "layout==edit2"

i don't say to remove the ACL check, just say it is redudant when using edit.php layout

[EDIT]
When an edit form succeds in opening because of 'core.edit.own' (not having core.edit on the asset)
then the check:
$this->item->created_by == $userId
has been already made

... that explains why your code will work even if you do not add is-owner check, it is a side-effect of the fact that the check was already made ...

infograf768 · 2016-06-17T09:21:30Z

Also records that have an "owner" / creator
are:

category
article
contact
tag

Thus 'core.edit.own' check should be only be added for them

Indeed, banners should be taken off.
But Newsfeeds has an Edit Own Permission

I have to think over the rest of your comments.

joomla-cms-bot · 2016-06-17T15:57:06Z

This PR has received new commits.

CC: @brianteeman

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10836.}

ggppdk · 2016-06-17T20:51:20Z

@infograf768
I know you have not found yet time to look at my other comments, i am posting again just to explain my comment

Here is why the check for creator is needed in the view.html.php

Copy
administrator/components/com_content/views/article/tmpl/edit.php
as
administrator/components/com_content/views/article/tmpl/edit2.php

Then use it with a user that only has edit.own and component access and on a record not owned by the user

of course my example below, reveals a limitation (backend only, this problem does not exist in frontend):

that currently you cannot create custom backend edit layouts and also limit access to them: they are not saveable, but they do open on direct access

infograf768 · 2016-06-18T08:09:43Z

@ggppdk
Can you propose a PR against my branch?

ggppdk · 2016-06-18T09:08:00Z

yes, will do today,
for view.html.php files is little work, for the content history models will need some searching

…ory, and also added category form case

infograf768 · 2016-06-21T07:19:51Z

@ggppdk
Will test asap infograf768#35

… (cathes core.edit.own and works regardless of if owner has been changed)

Fixed checking of core.edit.own in views and in models of contenthistory, and also added category form case

joomla-cms-bot · 2016-06-25T06:49:22Z

This PR has received new commits.

CC: @brianteeman

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10836.}

infograf768 · 2016-06-25T06:50:17Z

Thanks @ggppdk
I merged your PR.

Needs new testers now.

jsubri · 2016-07-04T17:15:59Z

I'm almost on the edge to be able to test it with a downgraded user account as a parent of the Manager group.But the test instructions starting point is from the Registered group.
As a parent of the Registered group, I can't get the top menus and the usual buttons "New, Edit, Save..." displayed for "Registered" nor "authors team", see below.

Any hints is welcome, probably tired! @pieter-groeneweg

infograf768 · 2016-07-06T09:21:13Z

@brianteeman
Can you retest this please?

brianteeman · 2016-07-06T09:46:21Z

Thanks for the reminder - will do it shortly

brianteeman · 2016-07-06T17:38:45Z

@infograf768 are the test instructions correct? I get the same as @jsubri

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10836.}

ggppdk · 2016-07-06T21:45:02Z

Please don't be confused by my previous pictures,
they were there to prove that the ACL check needed fixing, and it should now be fixed

We are supposed to

configure a usergroup that is denied the edit ACL privilege, but is granted edit.own
then edit an article that is owned by the user

Then if you want to test this in more complete way,

re-allow "edit.own" in Global config for the SubManager usergroup
and edit article's category e.g. "Sample Data Articles" as super user, and deny edit.own for the usergroup "Sub Manager1" for this category

then login as submanager1 and edit the owned article and try to use versions button

brianteeman · 2016-07-06T21:47:14Z

but the test instructions says something different to what you say above

create a group "authors team" with parent "registered".

ggppdk · 2016-07-06T22:04:38Z

ok, the important is that the user belongs to proper usergroup(s)

so that user is denied "edit" but has "edit.own" on the record (article, etc)
and also that user "owns" the record

My 3 pictures clearly show such a case

Finally if user does not own the record (and only has 'edit.own' but not 'edit') then user must not be able to use versioning

if you copy paste any contenthistory related URLs in the browser and you modify the 'item_id' then you should get "you are not authorised to view this resource"

(it is this exact check that this PR was failing in the begining)

pieter-groeneweg · 2016-07-07T10:49:20Z

Not being able to code and write awesome PHP scripts like you guys do (big cheer!)..

But I think that as soon as a user takes ownership of an article, he/she should be able to even revert to a version of that same article even when it was created by someone else.
As I see it, the one who became owner as the last one, is the "king".

It may even make things more simple...

ggppdk · 2016-07-07T11:07:22Z

But I think that as soon as a user takes ownership of an article, he/she should be able to even revert to a version of that same article even when it was created by someone else.

This PR does as you said,
the edit.own is calculated on the current record owner

pieter-groeneweg · 2016-07-07T13:29:06Z

@ggppdk Then I must be confused by all previous entries in this thread. I am new to the lingua of coders ;)

I so much appreciate what you guys are doing.. (not only this thread, but all) More than three cheers!!

jsubri · 2016-07-08T11:04:19Z

I have tested this item ✅ successfully on 83bbb35

I've used a downgraded user account as a parent from the Manager group.
I confirm the issue.
The patch solve the "Versions" button not displayed with edit.own, the Versions button appears after the initial save
I've tested other users from the same group, they need to have "Edit" at the Category level to be able to edit that article and Versions still works fine for them.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10836.}

hardiktailored · 2016-07-14T05:49:20Z

I have tested this item ✅ successfully on 83bbb35

Versions button was not displayed before even when sub-manager permission set to Edit: Denied and Edit Own: Allowed; after applying patch, button displays and version change works.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10836.}

brianteeman · 2016-07-14T08:18:46Z

Rtc

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10836.}

User with edit_own permission should be able to load Versions

4b41620

joomla-cms-bot added the PR-staging label Jun 16, 2016

infograf768 mentioned this pull request Jun 16, 2016

Versioning is not available on edit.own #10813

Closed

zero-24 reviewed Jun 16, 2016
View reviewed changes

= => ==

74ffa32

Makig Travis happier

602e526

edit_own does not exist for Banners

a2df687

Fixed checking of core.edit.own in views and in models of contenthist…

5bbbcff

…ory, and also added category form case

ggppdk and others added 2 commits June 22, 2016 11:45

Fixed content history models to check session as fallback for canEdit…

f2cb8e5

… (cathes core.edit.own and works regardless of if owner has been changed)

Merge pull request #35 from ggppdk/version_edit_own

83bbb35

Fixed checking of core.edit.own in views and in models of contenthistory, and also added category form case

infograf768 mentioned this pull request Jul 2, 2016

Version History: Modify author and date only when not toggling Keep Forever #10975

Merged

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 14, 2016

brianteeman added this to the Joomla 3.6.1 milestone Jul 14, 2016

roland-d merged commit c1fc030 into joomla:staging Jul 16, 2016

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 16, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

User with edit_own permission should be able to load Versions for the item concerned #10836

User with edit_own permission should be able to load Versions for the item concerned #10836

infograf768 commented Jun 16, 2016 •

edited by brianteeman

zero-24 Jun 16, 2016

brianteeman commented Jun 16, 2016

infograf768 commented Jun 16, 2016

infograf768 commented Jun 16, 2016

brianteeman commented Jun 16, 2016

infograf768 commented Jun 16, 2016

brianteeman commented Jun 16, 2016

infograf768 commented Jun 16, 2016

brianteeman commented Jun 16, 2016

joomla-cms-bot commented Jun 16, 2016

ggppdk commented Jun 16, 2016 •

edited

infograf768 commented Jun 17, 2016

joomla-cms-bot commented Jun 17, 2016

ggppdk commented Jun 17, 2016 •

edited

infograf768 commented Jun 18, 2016

ggppdk commented Jun 18, 2016

infograf768 commented Jun 21, 2016

joomla-cms-bot commented Jun 25, 2016

infograf768 commented Jun 25, 2016

jsubri commented Jul 4, 2016

infograf768 commented Jul 6, 2016

brianteeman commented Jul 6, 2016

brianteeman commented Jul 6, 2016

ggppdk commented Jul 6, 2016

brianteeman commented Jul 6, 2016

ggppdk commented Jul 6, 2016 •

edited

pieter-groeneweg commented Jul 7, 2016 •

edited

ggppdk commented Jul 7, 2016

pieter-groeneweg commented Jul 7, 2016

jsubri commented Jul 8, 2016

hardiktailored commented Jul 14, 2016

brianteeman commented Jul 14, 2016

User with edit_own permission should be able to load Versions for the item concerned #10836

User with edit_own permission should be able to load Versions for the item concerned #10836

Conversation

infograf768 commented Jun 16, 2016 • edited by brianteeman

Testing Instructions

zero-24 Jun 16, 2016

Choose a reason for hiding this comment

brianteeman commented Jun 16, 2016

infograf768 commented Jun 16, 2016

infograf768 commented Jun 16, 2016

brianteeman commented Jun 16, 2016

infograf768 commented Jun 16, 2016

brianteeman commented Jun 16, 2016

infograf768 commented Jun 16, 2016

brianteeman commented Jun 16, 2016

joomla-cms-bot commented Jun 16, 2016

ggppdk commented Jun 16, 2016 • edited

infograf768 commented Jun 17, 2016

joomla-cms-bot commented Jun 17, 2016

ggppdk commented Jun 17, 2016 • edited

infograf768 commented Jun 18, 2016

ggppdk commented Jun 18, 2016

infograf768 commented Jun 21, 2016

joomla-cms-bot commented Jun 25, 2016

infograf768 commented Jun 25, 2016

jsubri commented Jul 4, 2016

infograf768 commented Jul 6, 2016

brianteeman commented Jul 6, 2016

brianteeman commented Jul 6, 2016

ggppdk commented Jul 6, 2016

brianteeman commented Jul 6, 2016

ggppdk commented Jul 6, 2016 • edited

pieter-groeneweg commented Jul 7, 2016 • edited

ggppdk commented Jul 7, 2016

pieter-groeneweg commented Jul 7, 2016

jsubri commented Jul 8, 2016

hardiktailored commented Jul 14, 2016

brianteeman commented Jul 14, 2016

infograf768 commented Jun 16, 2016 •

edited by brianteeman

ggppdk commented Jun 16, 2016 •

edited

ggppdk commented Jun 17, 2016 •

edited

ggppdk commented Jul 6, 2016 •

edited

pieter-groeneweg commented Jul 7, 2016 •

edited