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

dev/core#4338 - Prevent people from sneakily adding fields to your extension's reserved custom groups by using the Move action #26518

Merged
merged 1 commit into from
Jun 13, 2023

Conversation

demeritcowboy
Copy link
Contributor

Overview

https://lab.civicrm.org/dev/core/-/issues/4338

Before

  1. Make an extension that makes a reserved custom field group.
  2. Someone adds a field to some other non-reserved custom field group.
  3. Then they sneakily use the Move link from the links on the right to move it into your reserved group.
  4. You make some code update that then crashes or mangles the data in that field because it's not expecting it to be there.

After

Reserved groups not listed as an option in the dropdown.

Technical Details

The UI currently goes out of its way to prevent you even seeing that reserved groups exist on the custom group admin screen, and bouncing if you try to hack an edit link to a field. It should also prevent moving a field into the group.

The call to Pseudoconstant::get is discouraged in its docblock so since I was changing it anyway I replaced with api.

Comments

@civibot
Copy link

civibot bot commented Jun 13, 2023

No issue was found matching the number given in the pull request title. Please check the issue number.

@civibot
Copy link

civibot bot commented Jun 13, 2023

(Standard links)

@civibot civibot bot added the master label Jun 13, 2023
@@ -91,7 +91,15 @@ public function preProcess() {
*/
public function buildQuickForm() {

$customGroup = CRM_Core_PseudoConstant::get('CRM_Core_DAO_CustomField', 'custom_group_id');
$customGroup = [];
$groups = \Civi\Api4\CustomGroup::get()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We get bitten by forgetting to pass FALSE here... but I think in this context checking permissions is ok. Anyone who doesn't have permission to see custom groups has no business using this form.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TRUE

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants