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

Drupal 7, refuse the possibility to execute the REST API with blocked users #26185

Merged
merged 1 commit into from
May 20, 2023

Conversation

rubofvil
Copy link
Contributor

@rubofvil rubofvil commented May 9, 2023

Refuse the possibility to execute the REST API with blocked users in drupal 7.

Overview

Security issue using the REST API.
User blocked can acces to the API by REST.

Before

A user blocked in drupal 7 accessing by API REST (.../modules/contrib/civicrm/extern/rest.php?entity=Contact&action=get&json={"s..."}&api_key=FIXME_USER_KEY&key=FIXME_SITE_KEY')
can access to info

After

The user is blocked to use the API REST

This block the possibility to execute the rest API with blocked drupal users
@civibot
Copy link

civibot bot commented May 9, 2023

(Standard links)

@civibot civibot bot added the master label May 9, 2023
@demeritcowboy
Copy link
Contributor

I can't remember if authx indirectly calls this but note that the default policy for some authx scenarios is "optional", meaning that the user doesn't have to be a valid user. So we should test if this prevents authx from working in those scenarios - see https://docs.civicrm.org/dev/en/latest/framework/authx/#settings

@rubofvil rubofvil changed the title Drupal 7, avoid load drupal user blocked in bootstrap Drupal 7, refuse the possibility to execute the REST API with blocked users in drupal 7. May 10, 2023
@rubofvil rubofvil changed the title Drupal 7, refuse the possibility to execute the REST API with blocked users in drupal 7. Drupal 7, refuse the possibility to execute the REST API with blocked users May 10, 2023
@demeritcowboy
Copy link
Contributor

I think this is ok. I don't believe the authx setting of optional is meant as an "override" of cms settings, and it doesn't seem to be a problem.

Have put merge-ready.

@demeritcowboy demeritcowboy added the merge ready PR will be merged after a few days if there are no objections label May 16, 2023
@demeritcowboy demeritcowboy merged commit 96337b1 into civicrm:master May 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants