(dev/core#2077) AuthX - Allow optional "guards"

[totten]

Overview

AuthX (#19590) provides additional ways to authenticate HTTP requests in CiviCRM. It is intended to provide an upgrade/replacement for certain backend scripts (eg extern/rest.php can be replaced by civicrm/ajax/rest). However, the new style currently lacks a function that the old style had -- the ability to guard backend requests with a semi-secret "site key".

This re-creates that functionality, and it aims to do it better.

(Note: I originally wrote/tested this patch in tandem with #19727 - one may or may not prefer to test/review them together. However, I believe they can work independently.)

ping @seamuslee001 @MikeyMJCO

Discussion

Let us first consider why a backend script (extern/rest.php) accepts the parameters that it does. There are two different credentials:

1. The user's API key (civicrm_contact.api_key aka ?api_key=...) demonstrates the identity of the caller.
2. The site key (CIVICRM_SITE_KEY aka ?key=...) demonstrates the sysadmin trusts this user enough to call the scripts.

The api_key is manifestly appropriate. The site_key is more debatable, but I think we should concede it is useful to allow a sysadmin to limit which users can authenticate to extern/rest.php. For example, they may have a policy to allow service-principals but prohibit regular user-principals. However, there are some issues:

• If you have a desktop/mobile app, it is onerous (for admins+users) to distribute the CIVICRM_SITE_KEY. At the same time, it must be shared widely -- which reduces the secrecy and security-benefit. If you change the key, then you must update all deployments/references.
• The extern/rest.php script already does/should/must enforce permissions based on the users role/access-level/system-policy. In many cases, with or without this backend interface, the user can still script equivalent actions (via browser-console, GreaseMonkey, screenscraper, etc) -- because (notwithstanding hypothetical bugs) it's the same access-level.

From a certain POV, we put a burden on users/admins that feels redundant.

I believe there is a better way for the syadmin to indicate their trust in the user -- by assigning roles/permissions. If the sysadmin grants permission AuthX: authenticate with password or AuthX: authenticate with api key, then that should be enough - without involving the CIVICRM_SITE_KEY.

Of course, we do have existing code/integrations/practices which rely on CIVICRM_SITE_KEY, so removing it would be problematic. This PR allows the sysadmin to choose a mix of approaches.

Before

Service requests made to extern/rest.php are required to submit CIVICRM_SITE_KEY.

Service requests made to civicrm/ajax/rest are not.

After

extern/rest.php remains the same as before, but civicrm/ajax/rest is more configurable. The setting authx_guards determines what is required to authenticate. It allows for these cases:

• If there are no guards (authx_guards=[]), then any user is allowed to authenticate via authx.
• The site-key can be used as a guard (authx_guards=["site_key"]), in which case users must submit the extra credential. The site key can be submitted via:
  • Header (X-Civi-Key: ...)
  • Parameter (?_authxSiteKey=...)
  • Legacy parameter (?key=..., but only for civicrm/ajax/rest's legacy/compat mode).
• User permissions can be used as a guard (authx_guards=["perm"]). In this case, the user must have permission authentciate with password or authenticate with api key.
• If both guards are enabled (authx_guards=["site_key","perm"]), then either is sufficient.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[homotechsual]

Heck yeah! Gimme now!

Removing the need for site_key makes API key based authentication useful. We could extend this then I presume to allow the oauth style of pre-authorised applications.

So set a custom guard for app_id requiring the application calling the API to match a pre-defined application ID.

[seamuslee001]

@totten this should be E::ts

[seamuslee001]

@totten this looks good to me I just have 1 ts issue and also should this go against 5.36 instead given that is where Authx is or go with master?

[eileenmcnaughton]

@totten - just noting that only 2 minor things need attending to by you here per ^^

[eileenmcnaughton]

@totten @seamuslee001 noting we agreed this would be rebased against 5.36 & merged (with the extra ts)

[eileenmcnaughton]

@seamuslee001 @totten I rebased this against 5.36 (only one commit needed removing) & fixed the ts - should be good to merge

[eileenmcnaughton]

test this please