CRM-21707 use metadata to validate 'parents' field on group.parents
authoreileen <emcnaughton@wikimedia.org>
Thu, 25 Jan 2018 22:59:23 +0000 (11:59 +1300)
committereileen <emcnaughton@wikimedia.org>
Thu, 25 Jan 2018 22:59:23 +0000 (11:59 +1300)
commit1678a63b1b854729d440de47b7305b5f50c66ce8
tree94df183ad888fdff54561ee9cd21929340dc574b
parent30208fab037dfe47c014dec4cec4429b0aeaf1d8
CRM-21707 use metadata to validate 'parents' field on group.parents

There was a test attempting to pass

[1 => 'Test Group'] which was getting through to the BAO but I feel should
have been blocked at the api level as Test Group did not exist. We have existing code to validate against pseudoconstants but this was not kicking in
because
a) there was no pseudoconstant defined for the field
b) we were giving TEXT fields less validation than STRING fields
(the assumption looks like it was that they were html fields but that is
not consistent & IMHO non of the additional checks are harmful.)
c) the getoptions was failing because it was passing 'validate' in
as a param to the pseudoconstant once set up, I handled that in
the pseudoconstant function.

I also altered the test to see [1 => 'non existent group'] as a fail
(rather than converting it to group = 1 & succeed.

There are a couple of follow on thoughts I have
1) I think we should xss check keys as well as values - will add that as a commit
2) there may be a very very hypothetical hole in _civicrm_api3_api_match_pseudoconstant_value - if  is empty it returns early. I think that used
to be valid as some psuedoconstants were not resolveable but now
I think if options is empty it means there are no valid options &
it should be rejected. Note that these strings are still xss checked.
CRM/Contact/DAO/Group.php
CRM/Core/PseudoConstant.php
api/v3/utils.php
tests/phpunit/api/v3/GroupTest.php
xml/schema/Contact/Group.xml