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

[5.3] Control access to component preferences individually #41496

Open
wants to merge 36 commits into
base: 5.3-dev
Choose a base branch
from

Conversation

MarkRS-UK
Copy link
Contributor

@MarkRS-UK MarkRS-UK commented Aug 28, 2023

Pull Request for Issue # .

Summary of Changes

Modified code in administrator com_config view for component to allow fine control of preferences to be presented to users.

Currently full access is granted to any user having admin access to a component.
This change allows access to be granted individually to fieldsets in a component's access.xml

Testing Instructions

Create a custom component and set up access actions in its access.xml corresponding to fieldsets in its config.xml,
eg, in access xml create more than one fields of the form

<action name="core.options.pref1" title="Preference 1" description="Show preference 1" />

corresponding to fields in config.xml like

<fieldset
	name="pref1"
	label="Preference 1"
>
    <field
        name='something'
        type='text'
        label='Label'
        class='inputbox'
    />
</fieldset>

Create a view in the component with a toolbar with the options button in it with the usual access control.
Install the component and go to its permissions tab in preferences.
Allow admin access to this new component for a non-superuser.
Log in with this user and see that the component's preference are available to the user, but only the permissions tab.
Looking at the permissions tab notice the new options actions at the bottom of the list. Enable one and redisplay the options for the selected user and see that selected tabs are now available.

Change and save an option on a (non-permissions) tab available to a non-superuser and then check that preference values on tabs not available to that user remain intact.

Actual result BEFORE applying this Pull Request

Access to component preferences cannot be controlled individually.

Expected result AFTER applying this Pull Request

Access to component preferences can be controlled individually.

Link to documentations

I'm happy to write documentation for this change should it be accepted.

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@MarkRS-UK
Copy link
Contributor Author

Errm, this is my first pull request and it seems to have gone slightly wrong.

I'm only wanting to make the changes in 298dae1.

I didn't intend all those others :(

@richard67
Copy link
Member

Errm, this is my first pull request and it seems to have gone slightly wrong.

I'm only wanting to make the changes in 298dae1.

I didn't intend all those others :(

Maybe you have chosen to make the pull request against a different base branch than the branch based on which you have created the branch for your PR? This pull request currently is against the 4.3-dev branch. You can change the branch on GitHub by going to the pull request and then using the edit button right beside the title, then there is a way to change the base branch.

@MarkRS-UK
Copy link
Contributor Author

MarkRS-UK commented Aug 28, 2023

I'm fine (as far as I know) with the 4.3 branch, but the three checks listed, one of which has failed, are nothing to do with me, are they?

@richard67
Copy link
Member

richard67 commented Aug 28, 2023

I'm fine (as far as I know) with the 4.3 branch, but the three checks listed, one of which has failed, are nothing to do with me, are they?

They are PHP code style errors reported in one of these tools, and I think they are related to your PR: https://ci.joomla.org/joomla/joomla-cms/68979/1/8

Follow the above link and then click on PHPCS to see the log.

@MarkRS-UK
Copy link
Contributor Author

Ah, ok, I see. Yes, there are code style errors there.
How do I fix that for this PR?

@richard67
Copy link
Member

Ah, ok, I see. Yes, there are code style errors there.
How do I fix that for this PR?

Edit the file and use spaces and not tabulators for indentation. 4 spaces for each level of indentation. When working on a local clone, commit the changes and then push them to remote. Or edit directly on GitHub.

@HLeithner
Copy link
Member

Whilst that sounds nice @HLeithner, does that mean it's not going to go into the 5.0 release?

yes, all feature pull requests have been rebased on 5.1 and all bugfix pull requests are rebased to 4.4

@brianteeman
Copy link
Contributor

Whilst that sounds nice @HLeithner, does that mean it's not going to go into the 5.0 release?

It still needs testing and as its a feature the maintainers have to decide if it will be accepted

@MarkRS-UK
Copy link
Contributor Author

Ok, thanks both. I should'a said "is it not being considered for the 5.0 release". However, point taken.

@HLeithner HLeithner changed the base branch from 5.1-dev to 5.2-dev April 24, 2024 09:08
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.2-dev.

@HLeithner HLeithner changed the title Control access to component preferences individually [5.2] Control access to component preferences individually Apr 24, 2024
@rdeutz
Copy link
Contributor

rdeutz commented May 9, 2024

We discussed this yesterday in the weekly maintainers meeting. Honestly, we don't see the use case or the need for something like this.

I moved it to the next meeting, because we thought the implementation only hides it and doesn't take care about save. So at least I want us to discuss it with all information and the correct state.

Stay tuned.

@MarkRS-UK
Copy link
Contributor Author

MarkRS-UK commented May 9, 2024

Thanks for considering it. Here's my use case.

I'm developing an extension for a volunteer group organising music a festival.
There are many people involved with many different areas of responsibility.
Various groups require component level data; for example ticket prices, marketing information, information distribution (by e- and snail-mail).
This is why I want to allow different groups access to different preferences. We don't want the accountants accessing the marketing preferences, for example.

If I'm right about the code required, this is a tiny change for significant increased functionality.

I would guess this is a step away from "normal" CMS (not just Joomla) use of "many clients one administrator". I have many administrators. Here we have many people working on projects. I think it facilitates building component that move a website away from something that helps with one part of a business to a core part of a business within a single component.

What do you think now? Is this something that came into your considering?

@brianteeman
Copy link
Contributor

Reading your example this does sound like the approach taken by one of the joomla component builders where everything has its own permission. So it is achievable now for your own components just not for core. Having seen components generated with separate permissions for everything it really makes things very complex to use.

@MarkRS-UK
Copy link
Contributor Author

Thanks for that @brianteeman.
They managed to do it using the standard J! preference system, or they built their own?

@brianteeman
Copy link
Contributor

Sorry I dont know. I just rememember that when I looked at a component built with it the permissions page was enormous

@MarkRS-UK
Copy link
Contributor Author

MarkRS-UK commented May 9, 2024

Thanks for helping me with this @brianteeman, but I'm sorry to say I don't follow.

It's not that there would be a huge number of permissions, but that the visibility of permissions is controllable.
The existing permissions code allows, even encourages, grouping of permissions into related types, each group is displayed in a tab.
I envisage (& intended to code) a situation where access to those tabs is a function of the user's group.

The (eg) accountants shouldn't be able to see preferences that are on the marketing tabs, and vice versa, or whatever.

It allows the extension developer to add a line to access.xml for each permission tab. I s'pose that could be a lot of lines, though that wouldn't be very sensible.

Do you remember what the extension was that you saw?

@HLeithner
Copy link
Member

Today meeting decision is to merge this PR if we have a test case (maybe update a core component?) and proper documentation maybe you can update https://manual.joomla.org/docs/next/general-concepts/acl/ with the full access.xml documentation?

thanks

@brianteeman
Copy link
Contributor

Just remembered this #24299

Maybe that would be a good example to create and document?

@MarkRS-UK
Copy link
Contributor Author

Thank you for the attention @HLeithner and the suggestion @brianteeman.
Yes, I'm up for doing those things. I'll post back here when I've finished or if I get stuck.

@MarkRS-UK
Copy link
Contributor Author

@MarkRS-UK
Copy link
Contributor Author

@brianteeman , I now understand better your concern about "everything having its own permissions". That isn't the case with this PR. It should perhaps have been called "Control access to GROUPS OF component preferences individually", as it is controlling preference tabs, not individual preferences.

However, there seems to be some problem with this as Harald tells me the team would rather control individual preferences. I agree with you on this, that that would be too clunky.

@HLeithner HLeithner changed the base branch from 5.2-dev to 5.3-dev September 2, 2024 08:52
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.3-dev.

@HLeithner HLeithner changed the title [5.2] Control access to component preferences individually [5.3] Control access to component preferences individually Sep 2, 2024
@Hackwar Hackwar removed the PR-5.2-dev label Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants