Allow overriding of auth provider settings.

Created on 23 December 2021, about 3 years ago
Updated 8 August 2024, 6 months ago

Problem/Motivation

When provider settings are loaded for auth plugins they don't always include overrides.

This code from the drush command to refresh loads overrides:

    if ($provider = SalesforceAuthConfig::load($providerName)) {
      $auth = $provider->getPlugin();

but the way it is loaded on the edit form does not (at /admin/config/salesforce/authorize/edit/{salesforce_auth})

I suspect there are other places where it isn't loading overrides, because when it's loaded through the SalesforceAuthConfig class it lacks overrides

We are using one auth plugin and switching values on the plugin between production and and test environments. Our default config is like this:

uuid: abc123
langcode: en
status: true
dependencies:
  config:
    - key.key.salesforce_key
  module:
    - salesforce_jwt
id: salesforce
label: Salesforce
provider: jwt
provider_settings:
  consumer_key: 'Overridden in settings.php'
  login_user: 'Overridden in settings.php'
  login_url: 'http://overridden-in-settings-php.com'
  encrypt_key: salesforce_key

When we pull the production site down to the test site, we run the drush command to refresh the token, which sometimes works and sometimes doesn't.

I find that when the plugin is loaded through the UI, such as on the editing form, it doesn't show the overrides. I suspect in some other instances, it's being loaded without the overrides.

I think if it loads incorrectly through the UI, it fails to refresh or revoke the token.

When you are editing the provider, due to the temporary values not passing validation, you can't save the edit form, which says it would refresh the token.

So it appears there's no way through the UI to refresh the token. Can't we have a "refresh default token" button in the UI?

I can fix this by entering the values for an environment, saving the provider form and clearing the cache, then the mapping objects show the correct instance URL.

I think loading the overrides from settings.php is an important thing to do, it works for drush, which I believe is right, but it doesn't work other places. I know other configs will show overrides on the config edit form. So I don't think it's going to break anything to display the overrides on the edit form. Resaving an edit form on an environment so that the config matches the overrides doesn't break anything. Having them mismatch is a bigger problem, and I recommend we update the way the settings for the plugins are loaded to include the overrides.

Steps to reproduce

1. Upload the config above and set as default provider
2. Override as follows with valid credentials:

  $config['salesforce.salesforce_auth.salesforce']['provider_settings']['consumer_key'] = 'valid consumer key';
  $config['salesforce.salesforce_auth.salesforce']['provider_settings']['login_user'] = 'it@mysite.com.devfull';
  $config['salesforce.salesforce_auth.salesforce']['provider_settings']['login_url'] = 'https://test.salesforce.com';

3. try save the config form.

Proposed resolution

add this code:

    $this->provider_settings = $this->authManager()
      ->getConfig($this->id())
      ->get('provider_settings');

into SalesforceAuthConfig::getProviderSettings

  /**
   * Wrapper for provider settings to inject instance id, from auth config.
   *
   * @return array
   *   Provider settings.
   */
  public function getProviderSettings() {
    // Get overrides.
    $this->provider_settings = $this->authManager()
      ->getConfig($this->id())
      ->get('provider_settings');
    return $this->provider_settings + ['id' => $this->id()];
  }

This will pull the provider_settings with overrides from settings.php

This will allow resaving the form on an environment, then clearing the cache to fix when the urls are pointing to production.

Ideally, you would be able to edit the original values within the form, but if you are going to validate, you can't use placeholder data as I'm doing.

The alternative would be to have two providers and switch dynamically, but that doesn't work right now either, because the default provider isn't overridable as mentioned in ✨ Add ability to detect config overrides and invalidate token Needs review

Remaining tasks

User interface changes

API changes

Data model changes

✨ Feature request
Status

Needs work

Version

5.0

Component

salesforce.module

Created by

πŸ‡ΊπŸ‡ΈUnited States oknate Greater New York City Area

Live updates comments and jobs are added and updated live.
  • Security improvements

    It makes Drupal less vulnerable to abuse or misuse. Note, this is the preferred tag, though the Security tag has a large body of issues tagged to it. Do NOT publicly disclose security vulnerabilities; contact the security team instead. Anyone (whether security team or not) can apply this tag to security improvements that do not directly present a vulnerability e.g. hardening an API to add filtering to reduce a common mistake in contributed modules.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

    Bumping this in priority and tagging it as it's a security issue to not support overriding the OAuth settings via PHP in e.g. settings.local.php.

  • πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

    I updated the included README.md file in the MR to note the correct variable name and structure.

  • πŸ‡ΊπŸ‡ΈUnited States AaronBauman Philadelphia
  • First commit to issue fork.
  • Pipeline finished with Failed
    5 months ago
    Total: 168s
    #301254
  • Pipeline finished with Success
    5 months ago
    Total: 326s
    #301260
  • πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

    Ran into a symptom of this today: πŸ› Failed authentication should not change default Active

  • πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

    One interesting detail is that if you override $config['salesforce.settings']['salesforce_auth_provider'] in settings.php the admin/config/salesforce/authorize/list page shows the change immediately.

    There's a bug with the proposed change when multiple configurations are defined - the connection details from the default auth provider is shown on the authorize/list page for all connections, not just the one it affects.

    This shows the list page without the patch or any changes to settings.php:

    This shows the list page without the patch and with the $config['salesforce.settings']['salesforce_auth_provider'] and $config['salesforce.salesforce_auth.legacy_oauth']['provider_settings'] defined in settings.php:

    This shows the list page with patch and without changes to the settings.php file:

    This shows the list page with the patch and with the settings.php changes:

    As you can see, with multiple configurations it retains the default settings when it shows all settings items.

    Another issue is that the state value named "salesforce.auth_tokens.IDENTID" is an object, which will be hard to override.

  • πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

    FYI I ended up handling this via an extra Drush command that runs after we run "drush deploy", with this code:

        $config_name = 'salesforce.salesforce_auth.legacy_oauth';
        $old_config = \Drupal::config($config_name);
        if ($old_config->hasOverrides()) {
          $config_factory = \Drupal::configFactory();
          $new_config = $config_factory->getEditable($config_name);
          $new_config->set('provider_settings.consumer_key', $old_config->get('provider_settings.consumer_key'));
          $new_config->set('provider_settings.consumer_secret', $old_config->get('provider_settings.consumer_secret'));
          $new_config->save();
    
          $this->io()->writeln(dt('Salesforce credentials updated.'));
        }
        else {
          $this->io()->writeln(dt('Did not update the Salesforce credentials'));
        }
    

    It loads the configuration of the "legacy_oauth" authentication config, checks to see if it was overridden via settings.php, and if so re-saves the values into the config object.

Production build 0.71.5 2024