Skip to content

Commit a931ce1

Browse files
chubes4chubes4
authored
fix: store credential auth config fields byte-exact (#2890)
1 parent 8f297d2 commit a931ce1

3 files changed

Lines changed: 143 additions & 4 deletions

File tree

‎inc/Abilities/AuthAbilities.php‎

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -778,7 +778,7 @@ public function executeSaveAuthConfig( array $input ): array {
778778
}
779779

780780
foreach ( $config_fields as $field_name => $field_config ) {
781-
$value = sanitize_text_field( $config_input[ $field_name ] ?? '' );
781+
$value = self::sanitizeConfigValue( $config_input[ $field_name ] ?? '', $field_config );
782782

783783
if ( ( $field_config['required'] ?? false ) && empty( $value ) && empty( $existing_config[ $field_name ] ?? '' ) ) {
784784
return array(
@@ -953,11 +953,11 @@ public function executeSetAuthToken( array $input ): array {
953953
);
954954
}
955955

956-
// Sanitize string values in account data.
956+
// Account data can contain opaque credentials, including percent-encoded cookies.
957957
$sanitized = array();
958958
foreach ( $account_data as $key => $value ) {
959959
if ( is_string( $value ) ) {
960-
$sanitized[ $key ] = sanitize_text_field( $value );
960+
$sanitized[ $key ] = self::sanitizeOpaqueCredentialValue( $value );
961961
} elseif ( is_int( $value ) || is_float( $value ) || is_bool( $value ) || is_null( $value ) ) {
962962
$sanitized[ $key ] = $value;
963963
} elseif ( is_array( $value ) ) {
@@ -1108,4 +1108,39 @@ private function getPrincipalContext( array $input ): array {
11081108

11091109
return array_filter( $context );
11101110
}
1111+
1112+
/**
1113+
* Sanitize an auth config value according to its provider-declared field type.
1114+
*
1115+
* Password and textarea fields can carry opaque credentials, so they must not
1116+
* pass through sanitize_text_field(), which removes percent-encoded octets.
1117+
*
1118+
* @param mixed $value Submitted field value.
1119+
* @param array $field_config Provider field declaration.
1120+
* @return string Sanitized value.
1121+
*/
1122+
private static function sanitizeConfigValue( $value, array $field_config ): string {
1123+
if ( ! is_scalar( $value ) ) {
1124+
return '';
1125+
}
1126+
1127+
$value = (string) $value;
1128+
$type = $field_config['type'] ?? 'text';
1129+
1130+
if ( in_array( $type, array( 'password', 'textarea' ), true ) ) {
1131+
return self::sanitizeOpaqueCredentialValue( $value );
1132+
}
1133+
1134+
return sanitize_text_field( $value );
1135+
}
1136+
1137+
/**
1138+
* Preserve opaque credential payloads while rejecting control characters.
1139+
*
1140+
* @param string $value Submitted credential value.
1141+
* @return string Credential-safe value.
1142+
*/
1143+
private static function sanitizeOpaqueCredentialValue( string $value ): string {
1144+
return trim( preg_replace( '/[\x00-\x1F\x7F]/', '', $value ) );
1145+
}
11111146
}

‎inc/Cli/Commands/AuthCommand.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -829,7 +829,7 @@ private function connectDirect( string $handler_slug, object $provider, array $a
829829
$missing[] = sprintf( '--%s', $cli_key );
830830
}
831831

832-
$config_data[ $field_name ] = sanitize_text_field( $value );
832+
$config_data[ $field_name ] = $value;
833833
}
834834

835835
if ( ! empty( $missing ) ) {

‎tests/Unit/Abilities/AuthAbilitiesTest.php‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,31 @@ public function is_authenticated(): bool {
2525
}
2626
}
2727

28+
class AuthAbilitiesConfigProvider extends BaseAuthProvider {
29+
30+
public function get_config_fields(): array {
31+
return array(
32+
'session_cookie' => array(
33+
'label' => 'Session Cookie',
34+
'type' => 'password',
35+
'required' => true,
36+
),
37+
'cookie_jar' => array(
38+
'label' => 'Cookie Jar',
39+
'type' => 'textarea',
40+
),
41+
'label' => array(
42+
'label' => 'Label',
43+
'type' => 'text',
44+
),
45+
);
46+
}
47+
48+
public function is_authenticated(): bool {
49+
return false;
50+
}
51+
}
52+
2853
class AuthAbilitiesTest extends WP_UnitTestCase {
2954

3055
private AuthAbilities $auth_abilities;
@@ -135,6 +160,61 @@ public function test_save_auth_config_returns_error_for_unknown_handler(): void
135160
$this->assertArrayHasKey( 'error', $result );
136161
}
137162

163+
public function test_save_auth_config_preserves_opaque_credential_fields(): void {
164+
$provider = new AuthAbilitiesConfigProvider( 'config_provider' );
165+
$this->registerConfigProvider( $provider );
166+
$cookie = 'tk_or=%22https%3A%2F%2Fwww.google.com%2F%22;wporg_sec=abc%7Cdef$o3$g0';
167+
168+
$result = $this->auth_abilities->executeSaveAuthConfig(
169+
array(
170+
'handler_slug' => 'config_handler',
171+
'config' => array(
172+
'session_cookie' => $cookie,
173+
'cookie_jar' => $cookie,
174+
),
175+
)
176+
);
177+
178+
$this->assertTrue( $result['success'] );
179+
$this->assertSame( $cookie, $provider->get_config()['session_cookie'] );
180+
$this->assertSame( $cookie, $provider->get_config()['cookie_jar'] );
181+
}
182+
183+
public function test_save_auth_config_sanitizes_text_fields(): void {
184+
$provider = new AuthAbilitiesConfigProvider( 'config_provider' );
185+
$this->registerConfigProvider( $provider );
186+
$value = " <strong>Example</strong>\n";
187+
188+
$result = $this->auth_abilities->executeSaveAuthConfig(
189+
array(
190+
'handler_slug' => 'config_handler',
191+
'config' => array(
192+
'session_cookie' => 'required-cookie',
193+
'label' => $value,
194+
),
195+
)
196+
);
197+
198+
$this->assertTrue( $result['success'] );
199+
$this->assertSame( sanitize_text_field( $value ), $provider->get_config()['label'] );
200+
}
201+
202+
public function test_set_auth_token_preserves_opaque_token_value(): void {
203+
$provider = new AuthAbilitiesAccountScopeProvider( 'scope_provider' );
204+
$this->registerAccountScopeProvider( $provider );
205+
$token = 'tk_or=%22https%3A%2F%2Fwww.google.com%2F%22%7C$o3$g0';
206+
207+
$result = $this->auth_abilities->executeSetAuthToken(
208+
array(
209+
'handler_slug' => 'scope_handler',
210+
'account_data' => array( 'access_token' => $token ),
211+
)
212+
);
213+
214+
$this->assertTrue( $result['success'] );
215+
$this->assertSame( $token, $provider->get_site_account()['access_token'] );
216+
}
217+
138218
public function test_set_auth_token_saves_user_account_without_site_fallback(): void {
139219
$provider = new AuthAbilitiesAccountScopeProvider( 'scope_provider' );
140220
$this->registerAccountScopeProvider( $provider );
@@ -196,6 +276,30 @@ function ( array $handlers ): array {
196276
HandlerAbilities::clearCache();
197277
}
198278

279+
private function registerConfigProvider( AuthAbilitiesConfigProvider $provider ): void {
280+
add_filter(
281+
'datamachine_auth_providers',
282+
function ( array $providers ) use ( $provider ): array {
283+
$providers['config_provider'] = $provider;
284+
return $providers;
285+
}
286+
);
287+
add_filter(
288+
'datamachine_handlers',
289+
function ( array $handlers ): array {
290+
$handlers['config_handler'] = array(
291+
'slug' => 'config_handler',
292+
'requires_auth' => true,
293+
'auth_provider_key' => 'config_provider',
294+
);
295+
return $handlers;
296+
}
297+
);
298+
299+
AuthAbilities::clearCache();
300+
HandlerAbilities::clearCache();
301+
}
302+
199303
public function test_permission_callback(): void {
200304
wp_set_current_user( 0 );
201305
add_filter( 'datamachine_cli_bypass_permissions', '__return_false' );

0 commit comments

Comments
 (0)