Api4

[ejegg]

Add support for proxying APIv4 requests. The first patch involves some copy-pasting of API3 code, and the second patch extracts some common functions. If this looks like the right approach I can add a patch to update the documentation.

Fixes #66

[ejegg]

@bjendres Does this look like a decent approach to you? Let me know if it needs any changes. We're planning to move forward with this locally, and we would love to avoid running a fork.

[bjendres]

@bjendres Does this look like a decent approach to you?

On a breif look: yes. Let my colleagues have a look at it, and we'll merge if we don't see any backward compatibility issues.

We're planning to move forward with this locally, and we would love to avoid running a fork.

Sure, I understand. It'd be good to have this feature anyway.

[ejegg]

Thanks for all the feedback, @jensschuppe ! I'll try to make those changes soon.

[ejegg]

Thanks again for the review @jensschuppe . I've read up on authx and have extensively edited this pull request so that it supports four different authx flows (header, xheader, legacyrest, and param) both on incoming requests and on the proxied request to CiviCRM. The login and auto-session flows seemed unlikely to be a good fit for CiviProxy.

The new configuration values are explained in config.dist.php. Any or all of the four flows can be allowed on incoming requests, but a single value needs to be set for the internal flow. I've also added support for the site_key guard.

[agileware-justin]

Just a suggestion. Consider using Guzzle instead of Curl.

Curl has been slowly replaced with Guzzle throughout the CiviCRM codebase.

[bjendres]

Just a suggestion. Consider using Guzzle instead of Curl.

Understandable, however we're trying to keep the dependencies to an absolute minimum, both for securlty and and compatibility reasons. Remember, it's supposed to be a minimalistic system to be dropped on any (trusted) webspace.

Or is there security concerns about curl that I don't know about?

In any case, please create a separate ticket if you would like further discuss the proposal.

[ejegg]

Latest version of commit changes name of authx config settings in anticipation of being able to use them for APIv3 in the future.

[ejegg]

Hi @jensschuppe and @bjendres, I hope you have had a good holiday season. I think I have implemented all of the changes requested in the last review. Is there any other change needed before merging this request?

[bjendres]

@jensschuppe let's schedule a review and discuss a target version.

[eileenmcnaughton]

@jensschuppe - it looks like your feedback has been addressed?

We have this running in production

[jensschuppe]

Thanks for your feedback, @eileenmcnaughton - we just didn't get to review this again yet. We'll try to get this released soon …

[eileenmcnaughton]

thanks @jensschuppe

[jensschuppe]

I discussed with @bjendres and we'd like some more people with an eye on security review this before merging, as the extension is all about security. Also, I opened #71 as a follow-up for supporting APIv3 with AuthX.

@bjendres feel free to add to my comment …

[eileenmcnaughton]

@bjendres @jensschuppe just reminding you we discussed the joys of pressing that green button on this PR :-)

[jensschuppe]

We trust you guys :wink: - released with version 0.7.0.

[eileenmcnaughton]

:-)

[ejegg]

Thanks @jensschuppe !