Make WordPress Core

Opened 3 months ago

Closed 3 months ago

Last modified 6 weeks ago

#64957 closed defect (bug) (fixed)

Connectors: `WP_Connector_Registry::register()` ignores custom `setting_name`

Reported by: jorgefilipecosta's profile jorgefilipecosta Owned by: jorgefilipecosta's profile jorgefilipecosta
Milestone: 7.0 Priority: high
Severity: normal Version: 7.0
Component: General Keywords: has-patch dev-reviewed has-unit-tests
Focuses: Cc:

Description

WP_Connector_Registry::register() always auto-generates the setting_name for connectors that use api_key authentication:

$connector['authentication']['setting_name'] = 'connectors_ai_' . str_replace( '-', '_', $id ) . '_api_key';

This ignores any setting_name value provided in the $args['authentication'] array, making it impossible for connectors to use an existing WordPress option (e.g. one already registered by their plugin) as their API key setting.

Steps to Reproduce

  1. Register a connector with a custom setting_name:
    $registry->register( 'my-connector', array(
        'name'           => 'My Connector',
        'type'           => 'ai_provider',
        'authentication' => array(
            'method'       => 'api_key',
            'setting_name' => 'my_existing_option',
        ),
    ) );
    
  2. Retrieve the connector: wp_get_connector( 'my-connector' )
  3. Inspect $connector['authentication']['setting_name']

Expected Results

setting_name is my_existing_option (the caller-provided value).

Actual Results

setting_name is connectors_ai_my_connector_api_key (the auto-generated value). The custom value is silently discarded.

Proposed Fix

Check for a non-empty, string setting_name in the provided args before falling back to the auto-generated default:

- $connector['authentication']['setting_name'] = 'connectors_ai_' . str_replace( '-', '_', $id ) . '_api_key';
+ if ( ! empty( $args['authentication']['setting_name'] ) && is_string( $args['authentication']['setting_name'] ) ) {
+     $connector['authentication']['setting_name'] = $args['authentication']['setting_name'];
+ } else {
+     $connector['authentication']['setting_name'] = 'connectors_ai_' . str_replace( '-', '_', $id ) . '_api_key';
+ }

This is fully backwards-compatible: connectors that do not provide a custom setting_name continue to receive the auto-generated value.

PR: https://github.com/WordPress/wordpress-develop/pull/11357

Change History (14)

This ticket was mentioned in PR #11357 on WordPress/wordpress-develop by @jorgefilipecosta.


3 months ago
#1

## Summary

  • WP_Connector_Registry::register() always auto-generates the setting_name for api_key connectors, ignoring any caller-provided value in $args['authentication']['setting_name']
  • This prevents connectors from using existing WordPress options as their API key storage
  • The fix checks for a non-empty setting_name in the provided args before falling back to the auto-generated name

Trac ticket: https://core.trac.wordpress.org/ticket/64957

## Test plan

  • Register a connector with a custom setting_name and verify it is preserved in the registry output
  • Verify existing connectors without a custom setting_name still get the auto-generated value

#2 @gziolo
3 months ago

  • Keywords dev-reviewed added

#3 @gziolo
3 months ago

I'll follow up later with a new test case so this change lands before the next RC.

#4 @jorgefilipecosta
3 months ago

  • Owner set to jorgefilipecosta
  • Resolution set to fixed
  • Status changed from new to closed

In 62116:

Connectors: Respect custom setting_name in connector registration.

WP_Connector_Registry::register() always auto-generates the setting_name
for connectors with api_key authentication, ignoring any caller-provided
value. This prevents connectors from using existing WordPress options as their
API key storage.
This change checks for a non-empty setting_name in the provided args before
falling back to the auto-generated name.

Props jorgefilipecosta, gziolo.
Fixes #64957.

This ticket was mentioned in PR #11378 on WordPress/wordpress-develop by @gziolo.


3 months ago
#5

  • Keywords has-unit-tests added

## Summary

  • Validate setting_name in connector registration — reject invalid values with _doing_it_wrong() instead of silently falling back to auto-generated names.
  • Update PHPDoc to reflect current behavior — widen type from literal 'ai_provider' to non-empty-string, document setting_name in @param.
  • Change auto-generated setting_name pattern from connectors_ai_{$id}_api_key to connectors_{$type}_{$id}_api_key so it works for any connector type. Built-in AI providers infer their names using the existing connectors_ai_{$id}_api_key convention.
  • Add constant_name and env_var_name as optional authentication fields, allowing connectors to declare explicit PHP constant and environment variable names for API key lookup.
  • Refactor _wp_connectors_get_api_key_source() to accept explicit env_var_name and constant_name parameters instead of deriving them from the provider ID.
  • Generalize REST dispatch, settings registration, and script module data to work with all connector types, not just ai_provider.

## Test plan

  • [ ] Verify existing AI provider connectors retain their connectors_ai_{id}_api_key setting names
  • [ ] Register a custom non-AI connector type and verify auto-generated setting name uses connectors_{type}_{id}_api_key
  • [ ] Verify constant_name and env_var_name are stored when provided and omitted when not
  • [ ] Verify invalid setting_name, constant_name, and env_var_name values trigger _doing_it_wrong()
  • [ ] Verify API key masking works for non-AI connector types in REST responses
  • [ ] Verify settings registration skips already-registered settings
  • [ ] Run full connector test suite: npm run test:php -- --filter=Tests_Connectors

See also: https://github.com/WordPress/gutenberg/pull/76828

🤖 Generated with Claude Code

#6 @gziolo
3 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#7 @jorgefilipecosta
3 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 62180:

Connectors: Fix and generalize the API for custom connector types.

Validate setting_name, constant_name, and env_var_name in connector
registration — reject invalid values with _doing_it_wrong() instead of
silently falling back.
Change the auto-generated setting_name pattern from
connectors_ai_{$id}_api_key to connectors_{$type}_{$id}_api_key so it
works for any connector type. Built-in AI providers infer their names using
the existing connectors_ai_{$id}_api_key convention, preserving backward
compatibility.
Add constant_name and env_var_name as optional authentication fields,
allowing connectors to declare explicit PHP constant and environment
variable names for API key lookup. AI providers auto-generate these using
the {CONSTANT_CASE_ID}_API_KEY convention.
Refactor _wp_connectors_get_api_key_source() to accept explicit
env_var_name and constant_name parameters instead of deriving them from
the provider ID. Environment variable and constant checks are skipped when
not provided.
Generalize REST dispatch, settings registration, and script module data to
work with all connector types, not just ai_provider. Settings
registration skips already-registered settings. Non-AI connectors determine
isConnected based on key source.
Replace isInstalled with pluginFile in script module data output to fix
plugin entity ID resolution on the frontend.
Update PHPDoc to reflect current behavior — widen type from literal
'ai_provider' to non-empty-string, document new authentication fields,
and use Anthropic examples throughout.

Props gziolo, jorgefilipecosta.
Fixes #64957.

This ticket was mentioned in PR #11399 on WordPress/wordpress-develop by @jorgefilipecosta.


3 months ago
#8

## Summary

Register Akismet Anti-Spam as a built-in non-AI connector with spam_filtering type.

  • Uses the existing wordpress_api_key WordPress option for API key storage
  • Checks the WPCOM_API_KEY PHP constant
  • Skips register_setting() since Akismet already registers wordpress_api_key

See also: https://github.com/WordPress/gutenberg/pull/76828

#9 @gziolo
3 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening to include [62180] in the 7.0 branch.

@bluefuton commented on PR #11399:


3 months ago
#10

One note on resetting API keys - it's not possible for the user to reset their API key on akismet.com, so we should hide this text for Akismet.

https://github.com/user-attachments/assets/580d3af1-0c02-4fd4-b2a7-f3997fe2ff58

@gziolo commented on PR #11399:


3 months ago
#11

@bluefuton, can we change the copy to a more generic version, like:

You can manage it at akismet.com

@bluefuton commented on PR #11399:


3 months ago
#12

@bluefuton, can we change the copy to a more generic version, like:

You can manage it at akismet.com

We'd be happy with a more generic version @gziolo 👍 In Akismet's case you can't really do anything with your API key on akismet.com (other than copy it), so maybe something like this?

_You can manage your account at akismet.com_

#13 @jorgefilipecosta
3 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 62194:

Connectors: Fix and generalize the API for custom connector types.

Validate setting_name, constant_name, and env_var_name in connector
registration — reject invalid values with _doing_it_wrong() instead of
silently falling back.
Change the auto-generated setting_name pattern from
connectors_ai_{$id}_api_key to connectors_{$type}_{$id}_api_key so it
works for any connector type. Built-in AI providers infer their names using
the existing connectors_ai_{$id}_api_key convention, preserving backward
compatibility.
Add constant_name and env_var_name as optional authentication fields,
allowing connectors to declare explicit PHP constant and environment
variable names for API key lookup. AI providers auto-generate these using
the {CONSTANT_CASE_ID}_API_KEY convention.
Refactor _wp_connectors_get_api_key_source() to accept explicit
env_var_name and constant_name parameters instead of deriving them from
the provider ID. Environment variable and constant checks are skipped when
not provided.
Generalize REST dispatch, settings registration, and script module data to
work with all connector types, not just ai_provider. Settings
registration skips already-registered settings. Non-AI connectors determine
isConnected based on key source.
Replace isInstalled with pluginFile in script module data output to fix
plugin entity ID resolution on the frontend.
Update PHPDoc to reflect current behavior — widen type from literal
'ai_provider' to non-empty-string, document new authentication fields,
and use Anthropic examples throughout.

Props gziolo, jorgefilipecosta.
Fixes #64957.

#14 @jorbin
6 weeks ago

@jorgefilipecosta Please remember to follow the commit message guidelines https://make.wordpress.org/core/handbook/best-practices/commit-messages/ so that it's easy to track what has been merged.

Note: See TracTickets for help on using tickets.