Skip to content

Conversation

@yild
Copy link

@yild yild commented May 28, 2016

Pull Request for Issue #10636 .

Summary of Changes

Allow user/group (that is not descendant of 'Editor' group) with core.edit.own permissions to add article link via XTD button.

Testing Instructions

  1. Create user or user+group (add user to created group).
  2. Go to Global Configuration -> Permissions, set 'Edit Own' permissions to 'Allow' for user/group.
  3. Login to frontend;
  4. Edit article content - use 'Article' XTD Button, this should bring form with articles list, create link;
  5. Further testing for Edit and Edit Own permissions (Denied or Inheritance that gives Denied result):
Edit Edit Own Result
Denied Denied form with JERROR_ALERTNOAUTHOR message for both permissions
Allow Denied form with articles for Edit, form with JERROR_ALERTNOAUTHOR for Edit Own
Denied Allow form with JERROR_ALERTNOAUTHOR for Edit, form with articles for Edit Own
Allow Allow form with articles for both permissions;

Allow user/group with core.edit.own permission to add article link via XTD button.
@brianteeman brianteeman changed the title Update content.php core.edit.own permission doesn't allow to insert article link with a XTD "Article" button May 28, 2016
@hardiktailored
Copy link

I have tested this item ✅ successfully on 711661c

Before applying patch clicking on Article button it was showing permission error; After applying patch it allows to add any article link.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10653.

@tomartailored
Copy link

I have tested this item ✅ successfully on 711661c


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10653.

@ggppdk
Copy link
Contributor

ggppdk commented Jul 30, 2016

Do not assign core.edit.own for usergroup A

  • neither to global perms
  • nor to com_content component perms,

(aka soft deny)

then grant core.edit.own for some sub-category C for usergroup A,
then get a user of user-group A that is also owner of the article of sub-category C to edit it,

and it will not work,

even the existing check for 'core.edit' is wrong, for same reason

  • both checks are needed to be done on the specific article asset
  • and core.edit.own needs to check if current user is owner of the article too

@ggppdk
Copy link
Contributor

ggppdk commented Jul 30, 2016

I have tested this item 🔴 unsuccessfully on 711661c

See my answer above

and i mean check the asset of the article being editted not of the article link being inserted
and this patch is not only that it allows cases that should not be allowed, someone could say that it is ok to let the link be created, but it also fails to allow cases that should be allowed too, so failure is dual


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10653.

@yild
Copy link
Author

yild commented Aug 4, 2016

Sorry but Im not that deep in Joomla to resolve issues regarding permissions inheritance :(

@ggppdk
Copy link
Contributor

ggppdk commented Aug 4, 2016

I will make a PR against the branch of this PR

@andrepereiradasilva
Copy link
Contributor

@ggppdk Does this problem still exists in latest staging after your PR's?

@ggppdk
Copy link
Contributor

ggppdk commented Aug 21, 2016

I did not make a PR for this

First i want to remind that these layouts (pagebreak and modal) are "proxied" to the backend models / views
https://github.com/joomla/joomla-cms/blob/staging/components/com_content/controller.php#L32-L42

Studing this, my syggestion is remove these ACL checks from the file:
components/com_content/content.php

Firstly, for backend users, we do not make any such ACL checks see here:

https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_content/content.php
e.g. a backend user may not have edit or edit.own at all but user can still access these layouts

In more details:

  • It makes no sense for pagebreak insertion (layout=pagebreak) to make some ACL check, since this must be usable when editing any article,

so what are we going to do check the assets of all articles ?
we could pass a "current" article id , but that would be a B/C break ?, but anyway we could , still page break is just a small form to shape some user-typed text

  • i say just remove the ACL check for it (also currently session token is not passed and thus is not checked, but i do not see any reason to add this)

Now about the topic of this PR:
About listing articles in modal layout to insert article links, this is not related to editing any specific article ... again we could pass currently edited article id and check can-edit on its asset, but i say not needed because:

  • for this layout currently session token is checked for frontend, AND since user's view access levels are used by the backend model, the user is shown a list that only includes its viewable articles

https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_content/models/articles.php#L217-L222

My suggestion remove the irrelevant ACL checks from there completely, that wrongly prevent access to the layouts,
and make it similar to the backend,

only the view access levels are relevant and they are already enforced, plus session token is checked for frontend users

@infograf768
Copy link
Member

See PR here:
#12353

@brianteeman
Copy link
Contributor

Closed as #12353 addressed this and it has been merged


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/10653.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants