Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tag selector not respecting current users view levels #16173

Closed
wants to merge 4 commits into from

Conversation

ggppdk
Copy link
Contributor

@ggppdk ggppdk commented May 21, 2017

Pull Request for Issue #8569
Part of this code is by @zero-24 from PR: #15467

Summary of Changes

Tag selector respects the user's view levels
while at the same time it maintains the existing values
Please read issue #8569 to know more

Note 1: I have not tested this PR (yet)
Note 2: This PR does not include server side validation of the view access levels (yet, you can suggest place to save me some time, i can do when i get time for it)

Testing Instructions

Create a TAG in the Tags component, set the access to Super Users
Login in as a manager or another user not in Super User group
Create new article and select Tags

Expected result

The tag with view access Super Users is not visible (for non superusers)

Actual result

All tags visible, regardless of access set in Tag component (for non superusers)

Documentation Changes Required

New protected method prepareValues() added to class JFormFieldTag

@stevlam
Copy link

stevlam commented May 21, 2017

hi, please consider a super user should view all tags in any view acess level, even if super user is not in that view acess level.

check this: in joomla 3.7.1 install + this code, create a tag with guest acess level, then try to assign that tag in a article as a super user and save - not saved! - similer issue in categories described in #15960 (comment)

@mbabker
Copy link
Contributor

mbabker commented May 21, 2017

The notion of a super user doesn't exist in the view access levels (weirdly enough). So I wouldn't suggest a patch that forces that ACL config into an incompatible system, and if we're going to make such a change it needs to be looked at as a bigger picture review of the whole system.

@ggppdk
Copy link
Contributor Author

ggppdk commented May 21, 2017

@mbabker

The notion of a super user doesn't exist in the view access levels (weirdly enough). ...

I have missed this.
I have updated the PR to be consistent with this behaviour !

@stevlam
yes i have done a search after mbabker comment,
and indeed i see it now in other code for tags !

@mbabker
Copy link
Contributor

mbabker commented May 21, 2017

That's bypassing the view access level configuration completely. My statement still stands, the notion of a super user doesn't exist in this part of Joomla. Since viewing access levels is not part of the ACL system, the only way to get a "super user should view all tags in any view acess level, even if super user is not in that view acess level" type of behavior is to use the ACL system to determine if viewing access level filters should be bypassed.

The system design has to be changed. Either a "super user" element has to be added to the view access level configuration somehow, or JAccess::getAuthorisedViewLevels() needs to account for users in the super user ACL group when it is fetching the data. Anything else is a bypass and/or hack of the system internals.

@ggppdk
Copy link
Contributor Author

ggppdk commented May 21, 2017

@mbabker

Agreed, but what you suggest cannot be done in this PR.
Or was my last commit wrong ? I am a little confused now

@stevlam
Copy link

stevlam commented May 21, 2017

don't understand much of joomla internals to know best way to do.
just think is a bug a super user create a tag (or category in the other issue) and not able to choose it.

also seem similer problem to other groups if you create a article with tag (or category in the other issue) with acess "Guest" and edit that article later with another group "Administrator" - when you try to save the tag (or category in the other issue) disapeears on save!

@mbabker
Copy link
Contributor

mbabker commented May 21, 2017

Agreed, but what you suggest cannot be done in this PR.

You're right. The only way around this issue right now is to continue to patch core with the view access level bypass/hack that is so liberally used in place of someone trying to address the core design flaw. To meet the user expectation that a super user should have all access to all the things, JAccess::getAuthorisedViewLevels() must be changed so that it always returns all of the access levels when it is being checked for a super user. Anything short of that continues to sweep the issue under the rug in hopes that nobody ever moves the rug and finds it.

@ghost
Copy link

ghost commented May 22, 2017

I have tested this item 🔴 unsuccessfully on 114b2e8

Super-User-Tag is gone after a Non-Super-User saved an Article having a Super-User-Tag (Comment of @mbabker)


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

@ggppdk
Copy link
Contributor Author

ggppdk commented May 22, 2017

hhmm it seems my code to check and keep existing values is incomplete, i will test and update this PR

@brianteeman
Copy link
Contributor

It has been seven months since the last update stating that an update was coming. As that hasnt happened I am closing this. It can always be re-opened if updated

@brianteeman brianteeman closed this Jan 4, 2018
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.

None yet

6 participants