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

access.xml - Add support default value for permission #7517

Closed
wants to merge 2 commits into from

Conversation

ThongTran-Dev
Copy link

@ThongTran-Dev ThongTran-Dev commented Jul 22, 2015

For now, when use ACL permission (by access.xml file for define rule), it's not support to set an default value for permission.

This PR for add an attribute "default" into an rule in access.xml file to set it default is "allow" or "denied", if default not specific, default rule will be "Inherit" as normal use.

@laoneo
Copy link
Member

laoneo commented Aug 11, 2015

Tested it and it worked. The only thing which is missing when you create a new item and set the default value to allowed is that the "Calculated Setting" still shows not allowed. See my screenshot:

access-allowed

@brianteeman
Copy link
Contributor

The calculated field is not updated until you save (read the notes).

As your screenshot shows you haven't entered a title (which is required)
you can't have saved yet.
On 11 Aug 2015 6:32 am, "laoneo" notifications@github.com wrote:

Tested it and it worked. The only thing which is missing when you create a
new item and set the default value to allowed is that the "Calculated
Setting" still shows not allowed. See my screenshot.
[image: access-allowed]
https://cloud.githubusercontent.com/assets/251072/9190882/269bf602-3ffb-11e5-879a-1ff7ee01d325.png


Reply to this email directly or view it on GitHub
#7517 (comment).

@laoneo
Copy link
Member

laoneo commented Aug 11, 2015

That's exactly what I wanted to show. If I set the default value to "Allowed" on a fresh new item, then IMO the calculated setting should reflect the actual state which is "Allowed" in that case.
Additionally if you set, for example the edit.state permission to "Allowed" in your component permissions for the "Registered" group. And you create a new item, then the "Registered" group has the "Allowed" setting and the calculated settings shows "Allowed" too. Despite the item was never saved. So the behavior should be the same I think.

@brianteeman
Copy link
Contributor

As the notes state the calculated setting is ONLY updated on save. Logical
or not that is not related to this issue.
On 11 Aug 2015 6:59 am, "laoneo" notifications@github.com wrote:

That's exactly what I wanted to show. If I set the default value to
"Allowed" on a fresh new item, then IMO the calculated setting should
reflect the actual state which is "Allowed" in that case.
Additionally if you set, for example the edit.state permission to
"Allowed" in your component permissions for the "Registered" group. And you
create a new item, then the "Registered" group has the "Allowed" setting
and the calculated settings shows "Allowed" too. Despite the item was never
saved. So the behavior should be the same I think.


Reply to this email directly or view it on GitHub
#7517 (comment).

@Bakual
Copy link
Contributor

Bakual commented Aug 11, 2015

I'm not yet sure what the purpose of the default is.
First, it would be hardcoded by the developer in the XML, which imho makes no sense for an ACL.
Second, we already have kind of defaults implemented in the system. It's called inheritance, meaning by default it will take the values from either the parent or global level.
So for items, it doesn't make any sense at all, it should be inherit and take the permissions from the component level if not set otherwise.
For component ACL, there is a small usecase when the component uses an ACL rule which isn't available in the global config and for some reason that action should be allowed by default. Which usually doesn't make much sense.

@laoneo
Copy link
Member

laoneo commented Aug 11, 2015

I have rules in my new custom fields component which are not available in the global config. This rules should be by default "Allowed" for all groups. If an admin then wants to restrict permission for my custom rule then it can be done in the permissions.

@laoneo
Copy link
Member

laoneo commented Aug 11, 2015

@brianteeman but then all the fields should be on Calculated setting initially on "Not allowed". IMO it should be the same for all, or not?

@Bakual
Copy link
Contributor

Bakual commented Aug 11, 2015

@brianteeman but then all the fields should be on Calculated setting initially on "Not allowed". IMO it should be the same for all, or not?

In the case you have shown in the screenshot, all other actions are not allowed for registered users. Thus the calculated settings are actually correct.

@laoneo
Copy link
Member

laoneo commented Aug 11, 2015

In that screenshot yes, but here not:

access-allowed-other

@laoneo
Copy link
Member

laoneo commented Aug 11, 2015

I mean I can live with it, that the "Calculated Setting" is calculated on Save and doesn't respect the initial default value. It is only cosmetic.
@test success

@laoneo
Copy link
Member

laoneo commented Aug 11, 2015

Tested it successfully


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

@Bakual
Copy link
Contributor

Bakual commented Aug 11, 2015

This PR does the default value only as a visual thing by pre-selecting the respective option in the select. If you want to have the calculated value as well, then you would have to do a bit more code than that.

Also when I look at the code, I think the default values are not used when calculating the actual value. So if you haven't saved the permissions yet, the default value isn't used at all. Which would be a major flaw in the PR.
Eg you use a default value for a rule in your component permission tab. As long as the component settings aren't saved, the action will be denied for anyone. As soon as you save the settings, the action will be allowed (if default is allowed).

@Bakual
Copy link
Contributor

Bakual commented Aug 11, 2015

I have rules in my new custom fields component which are not available in the global config. This rules should be by default "Allowed" for all groups. If an admin then wants to restrict permission for my custom rule then it can be done in the permissions.

That is the only usecase I can see. But it has a lot of flaws to it imho and is countering how our ACL works:

  • The only default value that makes sense is "allowed", "inherit" would be default anyway and "denied" does exactly the same thing as "inherit" at this stage.
  • If you set the action to "allowed" by default, it means that you basically turn of inheritance of the ACL. All child groups get a dedicated "allowed" as well, when they in fact should instead be inheriting that setting from the parent group.

I think there has to be a better approach to fix your issue with your fields. This one just feels wrong.
Maybe view levels (and setting it to public by default) is a better fit?

@laoneo
Copy link
Member

laoneo commented Aug 11, 2015

The only default value that makes sense is "allowed", "inherit" would be default anyway and "denied" does exactly the same thing as "inherit" at this stage.

Agree.

If you set the action to "allowed" by default, it means that you basically turn of inheritance of the ACL. All child groups get a dedicated "allowed" as well, when they in fact should instead be inheriting that setting from the parent group.

Think about a component which integrates the same way as com_categories (no own menu link in back end). If that component want's to add new rules, there is no way to set it globally. The admin has to go to every item and manually set it to allowed. What would be then the right way to add new rules in that way?

@laoneo
Copy link
Member

laoneo commented Aug 11, 2015

Maybe view levels (and setting it to public by default) is a better fit?

This would be an abuse of view levels as access levels are intended to be used to display stuff and not while editing.

@Bakual
Copy link
Contributor

Bakual commented Aug 11, 2015

What would be then the right way to add new rules in that way?

Maybe consider adding an own menu item for that component which allows setting component wide permissions?

This would be an abuse of view levels as access levels are intended to be used to display stuff and not while editing.

True

@laoneo
Copy link
Member

laoneo commented Aug 11, 2015

For the time being, I made my own rules form field to add the default value for the new rule. I hope that this PR will be merged as there are use cases when a default value makes sense.

@Bakual
Copy link
Contributor

Bakual commented Aug 11, 2015

For the time being, I made my own rules form field to add the default value for the new rule.

That is probably the best approach for this quite usecase specific request.

@laoneo
Copy link
Member

laoneo commented Aug 11, 2015

This request was not made for my specific use case. I've just tried to explain one use case. It was a coincident that @thongredweb made a PR which would solve my use case.

@Biromain
Copy link

Any news about this feature?

@roland-d
Copy link
Contributor

Hello @thongredweb

Thank you for your contribution.

The last comment here was on 28th January 2016. So the question is, Is this issue/pull request still valid?
If no reply is received within 4 weeks we will close this issue.

Thanks for understanding!


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

@laoneo
Copy link
Member

laoneo commented Apr 14, 2016

Guess it didn't found much of acceptance.

@brianteeman brianteeman closed this May 8, 2016
@brianteeman
Copy link
Contributor

Closed as stated above. It can always be re-opened if needed


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

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

7 participants