This majorly worries me. This turns the access level system (which is a bonafide view ACL action) into a full ACL decision matrix.

Could you please post the contents from your site here. It is essential for archival purposes so that in the future people can read the information even if it is no longer present on your website. Thanks

Could you please post the contents from your site here. It is essential for archival purposes so that in the future people can read the information even if it is no longer present on your website. Thanks

Good call (again) :) will copy paste as much as possible into the first post.

This majorly worries me. This turns the access level system (which is a bonafide view ACL action) into a full ACL decision matrix.

I hope your worries can be taken away, how can we best discuss them?

PS: previous discussion of several approaches that led to this solution: https://issues.joomla.org/tracker/joomla-cms/19473

Current Limitations

This RFC proposes to extend the configuration of com_fields to better control the displaying of Fields on (component edit / create) Forms. This functionality will greatly extend the use cases for com_fields in ‘real-life’ implementations.

Use Case com_fields and com_content:

I run a large blog site where the blogs are ‘enriched’ with custom fields to add credits, opengraph image / title / description, twitter card image / title / description to the blogs. The access to these field values must be set to public in order for visitors / facebook / twitter etc. to ‘see’ the given values.

These filling of these fields must be limited to the publisher role to avoid mis usage and to comply to site standards (for images).

The fields should be invisible to authors in order to avoid ‘support calls’: what are these fields for, why cannot I edit them, etc.

With the current configuration settings this is not possible:

• Setting the Access to publisher will prevent the fields from displaying on the article editing / creation form, but will also only display the values set in these fields to publisher only: public / guest will not see the values set.
• using show on administrator will partly work, but the publishers need to be able to login via the back-end. The show on parameter is ACL independent (simple toggle). Publishers do not have access to the back-end but need access to these fields via the front-end.
• setting the ACL to only allow publishers to Edit Custom Field Value, will display the fields on everybody’s article edit / creation form (including authors) as read-only except for the groups that have the correct permissions. This will clutter the article edit / creation form with a lot of read-only fields with the possibility that in the long list of fields some fields are read-only and some are editable.

Use Case com_fields and com_users:

I run a membership site where users subscribe to a service. Depending on their subscription, values can be set in their user profile via com_fields. The amount of fields for all subscriptions total up to 50 fields.

The values set in these fields should be visible on the front-end (via a special view that reads the user’s set value (via com_fields api)), the access to these fields must be set to public in order for public and guests to see the values for a specific user.

Specific fields should only be visible on the user profile edit form for the groups the user is member of and where these fields should be assigned to.

• Setting access to a field to the specific group that the fields belongs to does not work as this will also make the values set in these fields inaccessible to public. The values can only be seen by the users that have group membership the field access is set to
• using show on administrator will now work, because it is a generic toggle that is independent of group ACL membership: They are either all shown or all invisible. Furthermore no users have access to the back-end.
• Setting the ACL to only have specific user groups Edit Custom Field Value rights will display all fields on the user profile edit form: most read-only and some where values can be set. Only the fields where values can be set should be shown on the user edit profile form.

Furthermore: because the fields access is set to public, when a new user registers on the site, all (!) 50 fields are shown in the registration form as read-only, where the registering user needs to scroll down past all the read-only fields to reach the actual register button. Only the fields that are necessary in the form should be displayed.

Proposed Change

The current limitation lies in the fact that the access field is used for both the form AND for the values.

I propose to extend com_fields with an additional access view level: Access Form

The Access Form holds the ACL group that a user needs to be member of in order to be able to see the field on the form.

The Access Form can be set on field level AND on group level, making the maintenance of large field sets easier as not every field needs to be modified when the Access Form value changes (can be set on fields group level).

Use Case com_fields and com_content:

With the proposed change, the following configuration can be set for the com_content fields:

1. Access = Public (as the values set need to be displayed to visitors)
2. Show on: both (as the fields should be BOTH editable via the back-end AND front-end)
3. ACL permission: set Edit Custom Field Value permission to publisher (only members of the publisher acl can set and edit the field values)
4. (proposed setting) Access Form: set to publisher (only publishers will have the fields on the article edit / creation form.

Users that do not have sufficient Access rights to edit the fields will also not see these fields on the article edit / creation form. This will make the edit / create form very UX friendly as only relevant fields are displayed on the form!

Use Case com_fields and com_users:

With the proposed change, the following configuration can be set for the com_users fields:

1. Access = Public (as the values set need to be displayed to visitors)
2. Show on: both (as the fields should be BOTH editable via the back-end AND front-end)
3. ACL permission: set Edit Custom Field Value permission to public (only members of the public acl can set and edit the field values)
4. (proposed setting) Access Form: set to ‘subscription group’ (only subscribers to the ‘subscription group’ will have the fields on their user profile edit form.

The benefit here is that users that do not have sufficient Access rights to edit the fields will also not see these fields on their user profile edit form. This will make the edit form very UX friendly as only relevant fields are displayed on the form!

I get the use case, I get the issues being addressed, I get the concerns from the previous PR and why that might or might not be a good idea.

The access level "system" isn't intended to be an edit ACL decision maker for items (i.e. a form), it is a (debatably poor to decent) display ACL decision maker. This PR makes access levels an edit ACL decision maker. That just feels wrong to me. Not to mention that from a technical level, saving field values would now have to check this access and not just the core ACL system (what happens if someone manipulates the DOM or POST and includes a field that their access level doesn't have this "Access Form" ability?), and at a quick glance I'm not seeing that in this PR.

Granted, I don't work with CCKs. I don't work with system implementations where I need per-field ACL rules. So maybe I'm off base here. But view, create, and update are three distinctive ACL actions in most systems and a create/update action shouldn't be reliant on a system which decides whether the view action is allowed.

I get the use case, I get the issues being addressed, I get the concerns from the previous PR and why that might or might not be a good idea.

Cool :)

The access level "system" isn't intended to be an edit ACL decision maker for items (i.e. a form), it is a (debatably poor to decent) display ACL decision maker.

IMO the edit decision maker was and still is configured in the permissions tab for the field / group (when it comes to values: Edit Custom Field Value). The added access_form is (again) IMO a display ACL decision maker: display the field on the form (or not) regardless of edit permissions (when no edit permissions, display as read-only). Just like the 'Show on' setting but then more fine grained as it is not a site / administrator toggle but ACL based.

Not to mention that from a technical level, saving field values would now have to check this access and not just the core ACL system (what happens if someone manipulates the DOM or POST and includes a field that their access level doesn't have this "Access Form" ability?), and at a quick glance I'm not seeing that in this PR.

This is the same as before this PR, if they can do it after this PR, they can do it now as well.

Not to mention that from a technical level, saving field values would now have to check this access and not just the core ACL system (what happens if someone manipulates the DOM or POST and includes a field that their access level doesn't have this "Access Form" ability?), and at a quick glance I'm not seeing that in this PR.

This is the same as before this PR, if they can do it after this PR, they can do it now as well.

This PR makes it worse. You're saying that a user has edit permission to a field via the ACL system, yet using access levels to not display the field (effectively revoking edit permission). So this PR is just manipulating the UI and isn't actually doing any of the actual checks that I personally would expect an action restricting edit access to do.

The added access_form is (again) IMO a display ACL decision maker: display the field on the form (or not) regardless of edit permissions (when no edit permissions, display as read-only). Just like the 'Show on' setting but then more fine grained as it is not a site / administrator toggle but ACL based.

Show on is different. It is entirely DOM manipulation and has nothing to do with the user's ability to see/edit the data (with JavaScript turned off, it has no effect). This PR would effectively remove the field in full from the DOM. And because this PR changes what gets rendered on the page (the DOM), it does effectively become a second level edit ACL check because it overrules the ACL's edit action to make a decision to not display a form field.

You're saying that a user has edit permission to a field via the ACL system, yet using access levels to not display the field (effectively revoking edit permission)

Would it make a difference if the check was done on group membership instead of on ACL membership?

So this PR is just manipulating the UI

Effectively it doesn't change the UI, but changes the fields model to provide the field items (or not)
What I am trying to achieve is not to restrict the editing of the field's values (that is done via the permissions tab), but the displaying the field on the form. A toggle (show on form yes / no) is to limited as this should be 'toggled' on user group membership: the approach (after discussions) here is to use the ACL's for this.

What I am trying to achieve is not to restrict the editing of the field's values (that is done via the permissions tab), but the displaying the field on the form.

I get that. But it still in effect acts as a decision maker for whether the user can edit a field because the model will filter out items where this access level setting doesn't match the active user's groups. Which to me means if this is really the approach to be taken that the save action needs to account for this condition as well otherwise it can be argued (probably in favor of based on how the system behaves) that there is a write access bypass in com_fields allowing users to save changes to fields which have been filtered out based on this condition.

Yes, technically your approach is only affecting the display of pages. But under the hood this level of filtering acts as a second level edit ACL check. Go to your "Use Case com_fields and com_content", in this scenario if I configure fields to be editable by public users but set the "Access Form" setting to "Registered", this will remove fields which should be publicly editable from a form but if I either manipulate the DOM or craft the correct format POST request to save that form, I could still edit that field as a public user; this is presumably an undesired thing.

It is as @mbabker said
it is a probably a bad mixing with ACL ...

Arguably you could (probably ?) patch
setFieldValue() in com_fields model to

		// Don't save the value when the user is not authorized to change it
		if (!$field || !FieldsHelper::canEditFieldValue($field))
		{
			return false;
		}
		
		// ADD access_form check here ...
		if (!$user_does_not_have_access_form)
		{
			return false;
		}

Maybe a cleaner ? implementation that would make sense
is like this:

• access level for viewing non-editable values in record forms

Thus first (editfieldvalue) ACL is considered to find if a field is editable and only then

• for non editable fields, you use this access level to decide if their value is viewable in item form

So i would call it
non-editable Viewing Access

for non editable fields, you use this access level to decide if their value is viewable in item form

Except at that point why shouldn't it just use the already existing access, as in what's the need for a second access level configuration if you go with that approach?

Except at that point why shouldn't it just use the already existing access, as in what's the need for a second access level configuration if you go with that approach?

good question
it is the usage scenario that @Ruud68 described above

some fields in user profile should have a public access level
so that users can view these data about other users

but for every user in their own profile form (or for some admins), these fields are not editable and we want them to be hidden (that is hidden in the form)

Which comes back around full circle. You have ACL granting edit permissions and you're using a display oriented permission check to effectively revoke the granted edit permission.

If that's really how CCKs and these type of usage scenarios have to work (again, no idea, I avoid that type of stuff), then so be it I guess. I don't like it, everything about the implementation raises a red flag to me, but such is life. But, that still means there has to be a check in the save actions exactly as I pointed out before because in this usage scenario it is no longer good enough to check whether the user has the core.edit.value permission; the edit permission checks used in save actions must also account for this permission being revoked through the viewing access level filter.

I have to agree that it sounds very odd to me.

I am struggling to see what you cant achieve with the existing viewlevels and access levels

I am struggling to see what you cant achieve with the existing viewlevels and access levels

If I read things right, apparently there's a configuration where the user ends up seeing the value of a field that they cannot edit as either plain text or a readonly field and there's a desire to remove those fields from the UI versus showing those readonly fields. Which is a valid workflow I'd suggest, and one I don't think you could do entirely with layout overrides.

I am struggling to see what you cant achieve with the existing viewlevels and access levels

If I read things right, apparently there's a configuration where the user ends up seeing the value of a field that they cannot edit as either plain text or a readonly field and there's a desire to remove those fields from the UI versus showing those readonly fields. Which is a valid workflow I'd suggest, and one I don't think you could do entirely with layout overrides.

Yes that is the usage case,

It is about the viewing case, of whether the readonly value of non-editable fields is shown in record form,
anyway i was just thinking of cleaner solution than what this PR suggests, was just a thought

but some things can be done with a layout override, like skipping the readonly value of a non-editable field (that must be viewable outside the form)
just it involves extra work to create the override every time and you need some hardcoding of configuration into the layout !

some fields in user profile should have a public access level
so that users can view these data about other users
but for every user in their own profile form (or for some admins), these fields are not editable and we want them to be hidden (that is hidden in the form)

correct

whether the user has the core.edit.value permission;

This was actually one of my earlier approaches: add an additional permission: core.display.field
the check was then on this ACL to check if the field should be displayed on the form or not for a specific group of users.

the edit permission checks used in save actions must also account for this permission being revoked through the viewing access level filter.

But this permission is always checked and has priority over the access_form 'permission'
suppose custom field a has access level of publisher. On an article this field will be displayed read-only to authors. When setting the access_form level to superuser, the field will not display to publishers, but the actual permission (access level publishers) is always respected when saving. So when somebody is able to add the field to their form and save the form, the save function will always check to see if the user has publisher permissions.

I am struggling to see what you cant achieve with the existing viewlevels and access levels

The simplest 'use case' would be to have fields that registered users can fill in and of which you want the data to be shown to visitors.
So you set the access level (which determines the access to the field values) to public,
You set the permission Edit Custom Field Value to registerd

Now when a new user registers all fields are displayed on the registration form as read-only making the registration form cluttered as you only want the username / password / email fields.
Setting the access level to registered removes the fields form the form so they only show on the form of registered users, BUT they also prohibit the displaying of the values to registered users.

In one of my real-life use cases: authors can set an avatars for displaying above their articles. I want the avatar field only to show up on the form of authors. The only way to do this now is to set the access level to author, but in that case the avatars above the blogs are only shown when an author logs in, not to visitors.

Just to rule out it is about semantics. Would it make a difference if the field was not called Access Form, but e.g. Display on Form? And that the input for this field is not the ACL list, but the administrator can select one or more user groups?

Would it make a difference if the field was not called Access Form, but e.g. Display on Form?

No. It's still the same functionality just under a different label.

And that the input for this field is not the ACL list, but the administrator can select one or more user groups?

Same answer.

But this permission is always checked and has priority over the access_form 'permission'
suppose custom field a has access level of publisher. On an article this field will be displayed read-only to authors. When setting the access_form level to superuser, the field will not display to publishers, but the actual permission (access level publishers) is always respected when saving. So when somebody is able to add the field to their form and save the form, the save function will always check to see if the user has publisher permissions.

This is the part I'm not following. What happens if I submit a POST request (not going through the Joomla UI now, just straight PHP) where my POST body contains this form field that has been set to access_form level of super users only? Personally, I would expect that an ACL check to determine if the user can edit the field would fail, even if the user has been granted the proper core.edit(.*) permissions, because you are now saying that access to this form field is restricted to super users. So even if a lower level user group has edit permissions, that is to me superseded by your access_form configuration applying a more restrictive setting. It sounds like you are saying that the code handling the save action would still allow an authenticated user in the publisher group to edit the field's value (because the ACL core.edit(.*) permission has been granted to that group), and that to me is a write access security violation. This is why, based on my review of the code and proposed functionality, I suggest that this change introduces a second level edit ACL check and that it must be accounted for in other places (i.e. save) where edit permissions for a field are checked.

In one of my real-life use cases: authors can set an avatars for displaying above their articles. I want the avatar field only to show up on the form of authors. The only way to do this now is to set the access level to author, but in that case the avatars above the blogs are only shown when an author logs in, not to visitors.

The use case and the problem being addressed is clear. How this is to be fixed in the code IMO is not. This approach just does not feel right. I'd almost settle for a core behavior/option that lets you toggle whether to show a field as readonly or remove it in full in these types of conditions, but the security side of me again says that if you're removing it then when in a write context the ACL needs to be aware of this removal to prevent what could be considered a write access ACL violation.

I'd almost settle for a core behavior/option that lets you toggle whether to show a field as readonly or remove it in full in these types of conditions, but the security side of me again says that if you're removing it then when in a write context the ACL needs to be aware of this removal to prevent what could be considered a write access ACL violation.

That was actually also one of the approaches, use a parameter to show / not show when read-only.
just for my understanding: when setting a field to show on administrator only, are we then not also vulnerable to a write access violation?

Maybe good to summarize here what options / approaches have been worked on already before coming up with this approach.

1. add 'display on form field' via field / group parameters: https://issues.joomla.org/tracker/joomla-cms/19473#event-340353
2. adding a display when read-only yes / no toggle via field / group parameters (does what is needed, but limits 'future' use cases)
3. adding a ACL permission: core.create.value https://issues.joomla.org/tracker/joomla-cms/19473#event-339971
4. this PR adding a Access Form field via db change

   ---

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

... just for my understanding: when setting a field to show on administrator only, are we then not also vulnerable to a write access violation?

no,
because field is not added as part of JForm definition,
JForm validation in controller save task, will clear it as non present inside form definition,

but then the fields plugin code will maintain the old value,
https://github.com/joomla/joomla-cms/pull/19884/files#diff-6c82fc812577c688a3915acf799ac420R125

probably this PR , will also benefit from same effect
(i have not tested, nor did much of code review of this PR yet)

This PR and similar PR would have best to have had, before the core.edit.value ACL permission was ever implemented, now you get 2 ways to control editing of fields in forms !

So i still think that 'access_form' should not be used as ACL edit of field value, because it gets weird as @mbabker explained

but rather (as i suggested above) only be used to decide if a readonly value is viewable inside form when a user cannot edit a field value !

no, because field is not added as part of JForm definition, JForm validation in controller save task, will clear it as non present inside form definition,

The check for 'show on' is done in public function onCustomFieldsPrepareDom($field, DOMElement $parent, JForm $form) (file ./administrator/components/com_fields/libraries/fieldsplugin.php) which is 'triggered' from public static function prepareForm($context, JForm $form, $data)

When the 'show on' condition is met then this is the function that adds the field to the DOMElement.

		// Detect if the field should be shown at all
		if ($field->params->get('show_on') == 1 && $app->isClient('administrator'))
		{
			return;
		}
		elseif ($field->params->get('show_on') == 2 && $app->isClient('site'))
		{
			return null;
		}

When a field is NOT in the item list then this function will not be executed for that field so that field is NOT added to the DOMElement. That is why I asked what the difference was security wise in 'removing' a field from the form via the 'show on' parameter and via this PR.

So i still think that 'access_form' should not be used as ACL edit of field value, because it gets weird as @mbabker explained

So what would be your preferred approach? #19996 (comment)

Sorry, thought you were talking about the other thing we have that's called "showon" (because that's not confusing at all).

I guess the show on field becomes obsolete with this pr as it is really a simple toggle if the field should be shown on the site or admin application or on both places.

I was not taking of showon hiding via js , did i mention such a thing ? , i clearly spoke of show on Administrator side, or of SITE side feature, please see field configurations for the feature

I writing on mobile, sorry, i see now that you were speaking of same feature i mentioned, but what i said is true, please test to confirm, add it in form manually and try to save it

Okay, needed to figure out how to manipulate POST data (learning something new every day :))

test case 1:

field is on form > manipulate post data (value of field) and send this new data: field gets updated with NEW value = okay :)

test case 2:

set field to only display on administrator (via show_on parameter) > send manipulated post data: field does NOT get updated = okay :)

test case 3:

set field to only display to super users via access_form (logged in user is author) > send manipulated post data: field does NOT get updated - okay :)

test case 4 (just to be sure):

remove access_form restriction > send manipulated post data: field gets update with NEW value.

So what I conclude from these tests is that it is NOT possible to 'inject' or manipulate fields via the POST when field is not displayed via the access_form setting (or show_on parameter).

I guess the show on field becomes obsolete with this pr as it is really a simple toggle if the field should be shown on the site or admin application or on both places.

This PR can do the same as the show_on parameter when configured correct, being obsolete resulting in removing the show_on would have impact for end-users.

Hi, reading back this discussion I read a lot of 'red-flag' feelings. One of them I tackled (the writing of fields via POST). But I think that the 'red-flags' are stopping the discussion around the functionality.
I have previously worked on 4 (!) different approaches.

If this approach via the access_form view level is a 'no-go' I want to work on a different approach.
I have asked here: #19996 (comment) what is the most feasible scenario that can make it to implementation. Unfortunately nobody commented on this.

What I propose is that I work out scenario 1: Adding this functionality not via a view level, but via a new field / group parameter. The way this parameter works is the same as the currently implemented show_on (site // administrator / both) parameter. The difference is that the value set is one or more user groups > show only if the logged in user is member of one of the configured groups.

But although I am very keen to contribute to the Joomla project I am not so keen on 'running in the wrong direction': like for everybody else, also my time is limited :(

So @mbabker @ggppdk @laoneo what do you think? go / no-go / other?

hhmm
core.edit.value is given based on usergroups,
and then a usergroup selection setting (or an access level) for showing it or not showing it ?

what about this

1. if field editable then always show
2. if field non-editable, then 2 configuration settings

Parameter 1:
Field non-editable (Desc: Behaviour when field cannot be editted)

• hide
• show as readonly
• show no-access message
• no-access message + show readonly

Parameter 2:
Custom no-access message (Desc: Leave blank for default message)
(Text input language filtered)

To avoid confusion can we perhaps start calling this as "display on"

On Fri, 30 Mar 2018, 09:33 Georgios Papadakis, notifications@github.com
wrote:

hhmm
core.edit.value is given based on usergroups,
and then a usergroup selection setting (or an access level) for showing it
or not showing it ?

what about this

1. if field editable then always show
2. if field non-editable, then 2 configuration settings

Parameter 1:
Field non-editable (Desc: Behaviour when field cannot be editted)

• hide
• show as readonly
• show no-access message
• no-access message + show readonly

Parameter 2:
Custom no-access message (Desc: Leave blank for default message)
(Text input language filtered)

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#19996 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8fGiYMdxmsy6YQWBfqZh_ZAA6x1Rks5tje3UgaJpZM4S7H1i
.

But i do understand that what i suggested above
has no potentital to do this (inside the form)

• display readonly value to some users
• hide the readonly value for some others

@ggppdk what you propose looks similar to option 2:

adding a display when read-only yes / no toggle via field / group parameters (does what is needed, but limits 'future' use cases)

but then without the (custom) no-access message: so just a toggle to display / not display when read-only.

But i do understand that what i suggested above
has no potentital to do this (inside the form)

• display readonly value to some users
• hide the readonly value for some others

What I am proposing is not a toggle, but a user group selection to display / not display the (read-only) field. This facilitates more (future) use cases (like displaying for some users and not displaying for other users: so group membership triggered and not write-access triggered).

This is doable (as I already did it in a 'poc mock-up')

@Ruud68
Yes i understand that what you proposed facilitates more use cases
and i understant that a no access custom message can added independetely later

so what do you do when it rains on a saturday... #20039

I hope this clears all the 'red flags' from @mbabker :)

• configuration not via (secondary) access level, but via parameter in field / fieldgroup
• display / no display logic same as for the already implemented show_on (site / administrator / both) parameter
• no database changes required

Set to "closed" on behalf of @Quy by The JTracker Application at issues.joomla.org/joomla-cms/19996

Closing in favor for #20068