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

APIv4 - Silently ignore non-permissioned fields instead of throwing exceptions #20724

Merged
merged 1 commit into from
Jun 30, 2021

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Jun 29, 2021

Overview

Fixes API calls to lesser-permissioned users to silently ignore fields the user can't see.

Before

Previously the api would throw an exception when the user did not have permission to use a field in the WHERE or HAVING clause.

After

Now the unauthorized fields are silently ignored.

Comments

Replaces #20695

@civibot
Copy link

civibot bot commented Jun 29, 2021

(Standard links)

@civibot civibot bot added the master label Jun 29, 2021
@seamuslee001
Copy link
Contributor

@colemanw this test failure seems to relate

api\v4\Action\ContactApiKeyTest::testGetApiKey
Undefined index: undefined_fields

/home/jenkins/bknix-dfl/build/core-20724-4he1d/web/sites/all/modules/civicrm/tests/phpunit/api/v4/Action/ContactApiKeyTest.php:73
/home/jenkins/bknix-dfl/extern/phpunit8/phpunit8.phar:671

@colemanw colemanw force-pushed the apiFieldPermissions branch from 8d23d60 to ee13304 Compare June 29, 2021 15:46
@colemanw
Copy link
Member Author

@seamuslee001 I've updated the test

…xceptions

Previously the api would throw an exception when the user did not have permission to use a field
in the WHERE or HAVING clause.
@seamuslee001
Copy link
Contributor

Looks good to me @colemanw added merge on pass after I resolved the conflict with a recent merge

@colemanw colemanw merged commit 4fbe460 into civicrm:master Jun 30, 2021
@colemanw colemanw deleted the apiFieldPermissions branch June 30, 2021 14:45
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.

2 participants