authx - Accept API keys by default #20081
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Overview -------- This slightly relaxes the default settings so that it is easier to use `authx` as a replacement for `extern/rest.php` (which uses the `api_key` for authentication). Before ------ `extern/rest.php` accepts `api_key`s. `authx` can accept `api_key`s, but you have to change some settings to allow it. After ----- Both `extern/rest.php` and `authx` accept `api_key`s by default. Comments ----------------- The defaults in authx were designed to be a bit paranoid. The basic goal -- don't let anyone open up access accidentally. However, the current protections are a bit of overkill. Suppose you've created an `api_key` (sufficient for `extern/rest.php`) and you want to switch to using `civicrm/ajax/*`. Here are the sysadmin tasks: 1. Enable `authx` (*It's inert otherwise.*) 2. Grant the user permission `authenticate via api key`, or convey the `SITE_KEY` to the user, or disable all `authx_guards` 3. Update the setting `authx_header_cred` or `authx_xheader_cred` or `authx_param_cred` to allow `api_key` Notably, when this default was first chosen during drafting, the `authx_guards` (step 2) didn't exist. But now they do, and we have even more steps to go through. This change relaxes the defaults so that step `civicrm#3` is not necessary. This arrangement is still fairly paranoid (ie we still have 1+2 -- compared to `extern/rest.php`, there's still an extra opt-in hoop).
|
(Standard links)
|
|
So I think this is ok in my book but would appreciate @MikeyMJCO 's view on this matter as well |
|
@seamuslee001 I'm inclined to merge as I think @MikeyMJCO has had enough time to comment if he wants to |
|
This is good with me :-) |
|
thanks for confirming @MikeyMJCO |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This slightly relaxes the default settings so that it is easier to use
authxas a replacement forextern/rest.php(whichuses the
api_keyfor authentication).Before
extern/rest.phpacceptsapi_keys.authxcan acceptapi_keys, but you have to change some settings to allow it.After
Both
extern/rest.phpandauthxacceptapi_keys by default.Comments
The defaults in authx were designed to be a bit paranoid. The basic goal -- don't let anyone open up access accidentally. However, the current protections are a bit of overkill. Suppose you're setting up an API key for use with
civicrm/ajax/*. Here are the sysadmin tasks:civicrm_contact.api_key). (Doing this requiresedit api keysoredit own api keys.)authx(It's inert otherwise.)authenticate via api key, or convey theSITE_KEYto the user, or disable allauthx_guardsauthx_header_credorauthx_xheader_credorauthx_param_credto allowapi_keyNotably, the default in
#4was chosen during earlier drafting... beforeauthx_guardsexisted. But now we have the guards, and we have even more steps to go through.This change relaxes the defaults so that step
#4is not necessary. This arrangement is still fairly paranoid (ie we still have 1+2+3). Compared toextern/rest.php, there's still an extra opt-in hoop.