From 11d8f431a5c83c569c1028462aceee5f40ac3266 Mon Sep 17 00:00:00 2001 From: ADmad Date: Tue, 20 Oct 2026 21:51:29 +0530 Subject: [PATCH 1/2] Protect sensitive data from leaking in debug output/logs. --- composer.json | 2 +- src/AuthenticationService.php | 33 ++ src/Authenticator/CookieAuthenticator.php | 31 +- src/Authenticator/HttpDigestAuthenticator.php | 38 ++- src/Authenticator/JwtAuthenticator.php | 25 +- src/Authenticator/TokenAuthenticator.php | 3 +- src/Identifier/CallbackIdentifier.php | 3 +- src/Identifier/IdentifierInterface.php | 3 +- src/Identifier/Ldap/AdapterInterface.php | 4 +- src/Identifier/Ldap/ExtensionAdapter.php | 3 +- src/Identifier/LdapIdentifier.php | 9 +- src/Identifier/PasswordIdentifier.php | 9 +- src/Identifier/TokenIdentifier.php | 3 +- src/PasswordHasher/AbstractPasswordHasher.php | 3 +- src/PasswordHasher/DefaultPasswordHasher.php | 12 +- src/PasswordHasher/FallbackPasswordHasher.php | 12 +- src/PasswordHasher/LegacyPasswordHasher.php | 9 +- .../PasswordHasherInterface.php | 11 +- tests/TestCase/DebugRedactionTest.php | 128 ++++++++ tests/TestCase/SensitiveParameterTest.php | 301 ++++++++++++++++++ 20 files changed, 604 insertions(+), 38 deletions(-) create mode 100644 tests/TestCase/DebugRedactionTest.php create mode 100644 tests/TestCase/SensitiveParameterTest.php diff --git a/composer.json b/composer.json index d409d212..b7e56f4b 100644 --- a/composer.json +++ b/composer.json @@ -23,7 +23,7 @@ "docs": "https://book.cakephp.org/authentication/4/en/" }, "require": { - "php": ">=8.1", + "php": ">=8.2", "cakephp/http": "^5.0", "cakephp/utility": "^5.0", "laminas/laminas-diactoros": "^3.0", diff --git a/src/AuthenticationService.php b/src/AuthenticationService.php index 11476f8f..8a4b2332 100644 --- a/src/AuthenticationService.php +++ b/src/AuthenticationService.php @@ -555,4 +555,37 @@ protected function getImpersonationProvider(): ImpersonationInterface return $provider; } + + /** + * Debug info. + * + * Nested authenticator configuration can contain secrets such as the + * JWT `secretKey`, the digest `secret` or a string `salt` HMAC key. + * Those keys are redacted so debug output never contains them. + * + * @return array + */ + public function __debugInfo(): array + { + $config = $this->_config; + if (isset($config['authenticators']) && is_array($config['authenticators'])) { + foreach ($config['authenticators'] as $name => $authenticatorConfig) { + if (!is_array($authenticatorConfig)) { + continue; + } + foreach (['secretKey', 'secret', 'salt'] as $secretKey) { + if (isset($authenticatorConfig[$secretKey])) { + $config['authenticators'][$name][$secretKey] = '*****'; + } + } + } + } + + return [ + 'config' => $config, + 'authenticators' => $this->_authenticators, + 'successfulAuthenticator' => $this->_successfulAuthenticator, + 'result' => $this->_result, + ]; + } } diff --git a/src/Authenticator/CookieAuthenticator.php b/src/Authenticator/CookieAuthenticator.php index bcf53270..765d41f4 100644 --- a/src/Authenticator/CookieAuthenticator.php +++ b/src/Authenticator/CookieAuthenticator.php @@ -31,6 +31,7 @@ use InvalidArgumentException; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; +use SensitiveParameter; use UnexpectedValueException; /** @@ -151,7 +152,7 @@ public function authenticate(ServerRequestInterface $request): ResultInterface * @param array $token The decoded token parts. * @return \Authentication\Authenticator\ResultInterface */ - protected function _authenticateToken(array $token): ResultInterface + protected function _authenticateToken(#[SensitiveParameter] array $token): ResultInterface { [$username, $expires, $tokenHash] = $token; if (!is_string($username) || !is_numeric($expires) || !is_string($tokenHash)) { @@ -198,7 +199,7 @@ protected function _authenticateToken(array $token): ResultInterface * @param array $token The decoded token parts. * @return \Authentication\Authenticator\ResultInterface */ - protected function _authenticateLegacyToken(array $token): ResultInterface + protected function _authenticateLegacyToken(#[SensitiveParameter] array $token): ResultInterface { [$username, $tokenHash] = $token; if (!is_string($username) || !is_string($tokenHash)) { @@ -267,7 +268,7 @@ protected function _legacyHashWithinLimits(array $info): bool * @param \ArrayAccess|array $identity Identity data. * @return string */ - protected function _createLegacyPlainToken(ArrayAccess|array $identity): string + protected function _createLegacyPlainToken(#[SensitiveParameter] ArrayAccess|array $identity): string { $usernameField = $this->getConfig('fields.username'); $passwordField = $this->getConfig('fields.password'); @@ -348,7 +349,7 @@ public function persistIdentity( * @return string * @throws \JsonException */ - protected function _createToken(ArrayAccess|array $identity): string + protected function _createToken(#[SensitiveParameter] ArrayAccess|array $identity): string { $usernameField = $this->getConfig('fields.username'); $passwordField = $this->getConfig('fields.password'); @@ -448,4 +449,26 @@ protected function _createCookie(mixed $value): CookieInterface $options, ); } + + /** + * Debug info. + * + * Redacts the `salt` configuration value when it is a string as a + * string salt is used as the HMAC key for remember-me tokens. + * + * @return array + */ + public function __debugInfo(): array + { + $config = $this->_config; + if (isset($config['salt']) && is_string($config['salt'])) { + $config['salt'] = '*****'; + } + + return [ + 'config' => $config, + 'identifier' => $this->_identifier, + 'passwordHasher' => $this->_passwordHasher, + ]; + } } diff --git a/src/Authenticator/HttpDigestAuthenticator.php b/src/Authenticator/HttpDigestAuthenticator.php index 42f6c327..d657a70a 100644 --- a/src/Authenticator/HttpDigestAuthenticator.php +++ b/src/Authenticator/HttpDigestAuthenticator.php @@ -20,6 +20,7 @@ use Cake\Utility\Security; use InvalidArgumentException; use Psr\Http\Message\ServerRequestInterface; +use SensitiveParameter; /** * HttpDigest Authenticator @@ -149,7 +150,7 @@ protected function _getDigest(ServerRequestInterface $request): ?array * @param string $digest The raw digest authentication headers. * @return array|null An array of digest authentication headers */ - public function parseAuthData(string $digest): ?array + public function parseAuthData(#[SensitiveParameter] string $digest): ?array { if (str_starts_with($digest, 'Digest ')) { $digest = substr($digest, 7); @@ -178,8 +179,11 @@ public function parseAuthData(string $digest): ?array * @param string $method Request method * @return string Response hash */ - public function generateResponseHash(array $digest, string $password, string $method): string - { + public function generateResponseHash( + #[SensitiveParameter] array $digest, + #[SensitiveParameter] string $password, + string $method, + ): string { return md5( $password . ':' . $digest['nonce'] . ':' . $digest['nc'] . ':' . $digest['cnonce'] . ':' . $digest['qop'] . ':' . @@ -195,8 +199,11 @@ public function generateResponseHash(array $digest, string $password, string $me * @param string $realm The realm the password is for. * @return string the hashed password that can later be used with Digest authentication. */ - public static function password(string $username, string $password, string $realm): string - { + public static function password( + string $username, + #[SensitiveParameter] string $password, + string $realm, + ): string { return md5($username . ':' . $realm . ':' . $password); } @@ -277,4 +284,25 @@ protected function validNonce(string $nonce): bool return hash_equals($check, $checksum); } + + /** + * Debug info. + * + * Redacts the `secret` configuration value so that debug output + * never contains the nonce signing secret. + * + * @return array + */ + public function __debugInfo(): array + { + $config = $this->_config; + if (isset($config['secret'])) { + $config['secret'] = '*****'; + } + + return [ + 'config' => $config, + 'identifier' => $this->_identifier, + ]; + } } diff --git a/src/Authenticator/JwtAuthenticator.php b/src/Authenticator/JwtAuthenticator.php index b96b3f4f..ea4f04dc 100644 --- a/src/Authenticator/JwtAuthenticator.php +++ b/src/Authenticator/JwtAuthenticator.php @@ -27,6 +27,7 @@ use Firebase\JWT\Key; use Psr\Http\Message\ServerRequestInterface; use RuntimeException; +use SensitiveParameter; use stdClass; class JwtAuthenticator extends TokenAuthenticator @@ -158,7 +159,7 @@ public function getPayload(?ServerRequestInterface $request = null): ?object * @param string $token JWT token to decode. * @return object|null The JWT's payload as a PHP object, null on failure. */ - protected function decodeToken(string $token): ?object + protected function decodeToken(#[SensitiveParameter] string $token): ?object { $jsonWebKeySet = $this->getConfig('jwks'); if ($jsonWebKeySet) { @@ -174,4 +175,26 @@ protected function decodeToken(string $token): ?object return JWT::decode($token, $key); } + + /** + * Debug info. + * + * Redacts the `secretKey` configuration value so that debug output + * never contains the signing secret. + * + * @return array + */ + public function __debugInfo(): array + { + $config = $this->_config; + if (isset($config['secretKey'])) { + $config['secretKey'] = '*****'; + } + + return [ + 'config' => $config, + 'identifier' => $this->_identifier, + 'payload' => $this->payload, + ]; + } } diff --git a/src/Authenticator/TokenAuthenticator.php b/src/Authenticator/TokenAuthenticator.php index b2afbc36..c556f934 100644 --- a/src/Authenticator/TokenAuthenticator.php +++ b/src/Authenticator/TokenAuthenticator.php @@ -20,6 +20,7 @@ use Authentication\Identifier\IdentifierInterface; use Authentication\Identifier\TokenIdentifier; use Psr\Http\Message\ServerRequestInterface; +use SensitiveParameter; /** * Token Authenticator @@ -75,7 +76,7 @@ protected function getToken(ServerRequestInterface $request): ?string * @param string $prefix Prefix to strip * @return string */ - protected function stripTokenPrefix(string $token, string $prefix): string + protected function stripTokenPrefix(#[SensitiveParameter] string $token, string $prefix): string { $prefixLength = mb_strlen($prefix); if (mb_substr(mb_strtolower($token), 0, $prefixLength) === mb_strtolower($prefix)) { diff --git a/src/Identifier/CallbackIdentifier.php b/src/Identifier/CallbackIdentifier.php index d795696e..a4f42145 100644 --- a/src/Identifier/CallbackIdentifier.php +++ b/src/Identifier/CallbackIdentifier.php @@ -20,6 +20,7 @@ use Authentication\Authenticator\Result; use InvalidArgumentException; use RuntimeException; +use SensitiveParameter; /** * Callback Identifier @@ -66,7 +67,7 @@ protected function checkCallable(): void /** * @inheritDoc */ - public function identify(array $credentials): ArrayAccess|array|null + public function identify(#[SensitiveParameter] array $credentials): ArrayAccess|array|null { $callback = $this->getConfig('callback'); diff --git a/src/Identifier/IdentifierInterface.php b/src/Identifier/IdentifierInterface.php index 1d17d4cd..9a3d6ffc 100644 --- a/src/Identifier/IdentifierInterface.php +++ b/src/Identifier/IdentifierInterface.php @@ -17,6 +17,7 @@ namespace Authentication\Identifier; use ArrayAccess; +use SensitiveParameter; interface IdentifierInterface { @@ -26,7 +27,7 @@ interface IdentifierInterface * @param array $credentials Authentication credentials * @return \ArrayAccess|array|null */ - public function identify(array $credentials): ArrayAccess|array|null; + public function identify(#[SensitiveParameter] array $credentials): ArrayAccess|array|null; /** * Gets a list of errors happened in the identification process diff --git a/src/Identifier/Ldap/AdapterInterface.php b/src/Identifier/Ldap/AdapterInterface.php index 2ad03810..239feccc 100644 --- a/src/Identifier/Ldap/AdapterInterface.php +++ b/src/Identifier/Ldap/AdapterInterface.php @@ -16,6 +16,8 @@ */ namespace Authentication\Identifier\Ldap; +use SensitiveParameter; + interface AdapterInterface { /** @@ -25,7 +27,7 @@ interface AdapterInterface * @param string $password Bind password * @return bool */ - public function bind(string $bind, string $password): bool; + public function bind(string $bind, #[SensitiveParameter] string $password): bool; /** * Connect to an LDAP server diff --git a/src/Identifier/Ldap/ExtensionAdapter.php b/src/Identifier/Ldap/ExtensionAdapter.php index 963f86a7..8621dae3 100644 --- a/src/Identifier/Ldap/ExtensionAdapter.php +++ b/src/Identifier/Ldap/ExtensionAdapter.php @@ -19,6 +19,7 @@ use ErrorException; use LDAP\Connection; use RuntimeException; +use SensitiveParameter; /** * Provides a very thin OOP wrapper around the ldap_* functions. @@ -58,7 +59,7 @@ public function __construct() * @param string $password Bind password * @return bool */ - public function bind(string $bind, string $password): bool + public function bind(string $bind, #[SensitiveParameter] string $password): bool { $this->_setErrorHandler(); $result = ldap_bind($this->getConnection(), $bind, $password); diff --git a/src/Identifier/LdapIdentifier.php b/src/Identifier/LdapIdentifier.php index 5ae3a19d..3f893049 100644 --- a/src/Identifier/LdapIdentifier.php +++ b/src/Identifier/LdapIdentifier.php @@ -24,6 +24,7 @@ use ErrorException; use InvalidArgumentException; use RuntimeException; +use SensitiveParameter; /** * LDAP Identifier @@ -145,7 +146,7 @@ protected function _buildLdapObject(): void /** * @inheritDoc */ - public function identify(array $credentials): ArrayAccess|array|null + public function identify(#[SensitiveParameter] array $credentials): ArrayAccess|array|null { $this->_connectLdap(); $fields = $this->getConfig('fields'); @@ -195,8 +196,10 @@ protected function _connectLdap(): void * @param string $password The password * @return \ArrayAccess|null */ - protected function _bindUser(string $username, string $password): ?ArrayAccess - { + protected function _bindUser( + string $username, + #[SensitiveParameter] string $password, + ): ?ArrayAccess { $config = $this->getConfig(); try { $ldapBind = $this->_ldap->bind($config['bindDN']($username), $password); diff --git a/src/Identifier/PasswordIdentifier.php b/src/Identifier/PasswordIdentifier.php index 18d82451..cd97cadf 100644 --- a/src/Identifier/PasswordIdentifier.php +++ b/src/Identifier/PasswordIdentifier.php @@ -22,6 +22,7 @@ use Authentication\PasswordHasher\PasswordHasherFactory; use Authentication\PasswordHasher\PasswordHasherInterface; use Authentication\PasswordHasher\PasswordHasherTrait; +use SensitiveParameter; /** * Password Identifier @@ -95,7 +96,7 @@ public function getPasswordHasher(): PasswordHasherInterface /** * @inheritDoc */ - public function identify(array $credentials): ArrayAccess|array|null + public function identify(#[SensitiveParameter] array $credentials): ArrayAccess|array|null { if (!isset($credentials[self::CREDENTIAL_USERNAME])) { return null; @@ -121,8 +122,10 @@ public function identify(array $credentials): ArrayAccess|array|null * @param string|null $password The password. * @return bool */ - protected function _checkPassword(ArrayAccess|array|null $identity, ?string $password): bool - { + protected function _checkPassword( + #[SensitiveParameter] ArrayAccess|array|null $identity, + #[SensitiveParameter] ?string $password, + ): bool { $passwordField = $this->getConfig('fields.' . self::CREDENTIAL_PASSWORD); $identity ??= [ diff --git a/src/Identifier/TokenIdentifier.php b/src/Identifier/TokenIdentifier.php index 47ed1d68..fa5f99fa 100644 --- a/src/Identifier/TokenIdentifier.php +++ b/src/Identifier/TokenIdentifier.php @@ -19,6 +19,7 @@ use ArrayAccess; use Authentication\Identifier\Resolver\ResolverAwareTrait; use Cake\Utility\Security; +use SensitiveParameter; /** * Token Identifier @@ -44,7 +45,7 @@ class TokenIdentifier extends AbstractIdentifier /** * @inheritDoc */ - public function identify(array $credentials): ArrayAccess|array|null + public function identify(#[SensitiveParameter] array $credentials): ArrayAccess|array|null { $dataField = $this->getConfig('dataField'); if (!isset($credentials[$dataField])) { diff --git a/src/PasswordHasher/AbstractPasswordHasher.php b/src/PasswordHasher/AbstractPasswordHasher.php index 4dd4272b..80bb18ef 100644 --- a/src/PasswordHasher/AbstractPasswordHasher.php +++ b/src/PasswordHasher/AbstractPasswordHasher.php @@ -16,6 +16,7 @@ namespace Authentication\PasswordHasher; use Cake\Core\InstanceConfigTrait; +use SensitiveParameter; /** * Abstract password hashing class @@ -51,7 +52,7 @@ public function __construct(array $config = []) * @param string $password The password to verify * @return bool */ - public function needsRehash(string $password): bool + public function needsRehash(#[SensitiveParameter] string $password): bool { return password_needs_rehash($password, PASSWORD_DEFAULT); } diff --git a/src/PasswordHasher/DefaultPasswordHasher.php b/src/PasswordHasher/DefaultPasswordHasher.php index 973d2233..32ead99c 100644 --- a/src/PasswordHasher/DefaultPasswordHasher.php +++ b/src/PasswordHasher/DefaultPasswordHasher.php @@ -15,6 +15,8 @@ */ namespace Authentication\PasswordHasher; +use SensitiveParameter; + /** * Default password hashing class. */ @@ -41,7 +43,7 @@ class DefaultPasswordHasher extends AbstractPasswordHasher * @param string $password Plain text password to hash. * @return string Password hash or false on failure. */ - public function hash(string $password): string + public function hash(#[SensitiveParameter] string $password): string { return password_hash( $password, @@ -57,8 +59,10 @@ public function hash(string $password): string * @param string $hashedPassword Existing hashed password. * @return bool True if hashes match else false. */ - public function check(string $password, string $hashedPassword): bool - { + public function check( + #[SensitiveParameter] string $password, + #[SensitiveParameter] string $hashedPassword, + ): bool { return password_verify($password, $hashedPassword); } @@ -69,7 +73,7 @@ public function check(string $password, string $hashedPassword): bool * @param string $password The password to verify * @return bool */ - public function needsRehash(string $password): bool + public function needsRehash(#[SensitiveParameter] string $password): bool { return password_needs_rehash($password, $this->_config['hashType'], $this->_config['hashOptions']); } diff --git a/src/PasswordHasher/FallbackPasswordHasher.php b/src/PasswordHasher/FallbackPasswordHasher.php index dbfd4d6d..b8dd1e8c 100644 --- a/src/PasswordHasher/FallbackPasswordHasher.php +++ b/src/PasswordHasher/FallbackPasswordHasher.php @@ -15,6 +15,8 @@ */ namespace Authentication\PasswordHasher; +use SensitiveParameter; + /** * A password hasher that can use multiple different hashes where only * one is the preferred one. This is useful when trying to migrate an @@ -60,7 +62,7 @@ public function __construct(array $config = []) * @param string $password Plain text password to hash. * @return string Password hash */ - public function hash(string $password): string + public function hash(#[SensitiveParameter] string $password): string { return $this->_hashers[0]->hash($password); } @@ -75,8 +77,10 @@ public function hash(string $password): string * @param string $hashedPassword Existing hashed password. * @return bool True if hashes match else false. */ - public function check(string $password, string $hashedPassword): bool - { + public function check( + #[SensitiveParameter] string $password, + #[SensitiveParameter] string $hashedPassword, + ): bool { foreach ($this->_hashers as $hasher) { if ($hasher->check($password, $hashedPassword)) { return true; @@ -93,7 +97,7 @@ public function check(string $password, string $hashedPassword): bool * @param string $password The password to verify * @return bool */ - public function needsRehash(string $password): bool + public function needsRehash(#[SensitiveParameter] string $password): bool { return $this->_hashers[0]->needsRehash($password); } diff --git a/src/PasswordHasher/LegacyPasswordHasher.php b/src/PasswordHasher/LegacyPasswordHasher.php index 718d713c..315062dc 100644 --- a/src/PasswordHasher/LegacyPasswordHasher.php +++ b/src/PasswordHasher/LegacyPasswordHasher.php @@ -19,6 +19,7 @@ use Cake\Error\Debugger; use Cake\Utility\Security; use RuntimeException; +use SensitiveParameter; /** * Password hashing class that use weak hashing algorithms. This class is @@ -57,7 +58,7 @@ public function __construct(array $config = []) * @param string $password Plain text password to hash. * @return string Password hash */ - public function hash(string $password): string + public function hash(#[SensitiveParameter] string $password): string { return Security::hash($password, $this->_config['hashType'], $this->_config['salt']); } @@ -69,8 +70,10 @@ public function hash(string $password): string * @param string $hashedPassword Existing hashed password. * @return bool True if hashes match else false. */ - public function check(string $password, string $hashedPassword): bool - { + public function check( + #[SensitiveParameter] string $password, + #[SensitiveParameter] string $hashedPassword, + ): bool { return $hashedPassword === $this->hash($password); } } diff --git a/src/PasswordHasher/PasswordHasherInterface.php b/src/PasswordHasher/PasswordHasherInterface.php index a310310d..051dd61a 100644 --- a/src/PasswordHasher/PasswordHasherInterface.php +++ b/src/PasswordHasher/PasswordHasherInterface.php @@ -15,6 +15,8 @@ */ namespace Authentication\PasswordHasher; +use SensitiveParameter; + /** * PasswordHasherInterface */ @@ -26,7 +28,7 @@ interface PasswordHasherInterface * @param string $password Plain text password to hash. * @return string Password hash */ - public function hash(string $password): string; + public function hash(#[SensitiveParameter] string $password): string; /** * Check hash. Generate hash from user provided password string or data array @@ -36,7 +38,10 @@ public function hash(string $password): string; * @param string $hashedPassword Existing hashed password. * @return bool True if hashes match else false. */ - public function check(string $password, string $hashedPassword): bool; + public function check( + #[SensitiveParameter] string $password, + #[SensitiveParameter] string $hashedPassword, + ): bool; /** * Returns true if the password need to be rehashed, due to the password being @@ -48,5 +53,5 @@ public function check(string $password, string $hashedPassword): bool; * @param string $password The password to verify * @return bool */ - public function needsRehash(string $password): bool; + public function needsRehash(#[SensitiveParameter] string $password): bool; } diff --git a/tests/TestCase/DebugRedactionTest.php b/tests/TestCase/DebugRedactionTest.php new file mode 100644 index 00000000..9214334e --- /dev/null +++ b/tests/TestCase/DebugRedactionTest.php @@ -0,0 +1,128 @@ + 'planted-jwt-secret', + 'algorithm' => 'HS256', + ]); + + $output = $this->debugOutput($authenticator); + + $this->assertStringNotContainsString('planted-jwt-secret', $output); + $this->assertSame('*****', $authenticator->__debugInfo()['config']['secretKey']); + $this->assertStringContainsString('HS256', $output, 'Non sensitive config keys must stay visible.'); + } + + public function testHttpDigestAuthenticatorRedactsSecret(): void + { + $authenticator = new HttpDigestAuthenticator(null, [ + 'secret' => 'planted-digest-secret', + 'qop' => 'auth', + ]); + + $output = $this->debugOutput($authenticator); + + $this->assertStringNotContainsString('planted-digest-secret', $output); + $this->assertSame('*****', $authenticator->__debugInfo()['config']['secret']); + $this->assertStringContainsString('qop', $output, 'Non sensitive config keys must stay visible.'); + } + + public function testCookieAuthenticatorRedactsStringSalt(): void + { + $authenticator = new CookieAuthenticator(null, [ + 'salt' => 'planted-hmac-salt', + 'rememberMeField' => 'remember_me', + ]); + + $output = $this->debugOutput($authenticator); + + $this->assertStringNotContainsString('planted-hmac-salt', $output); + $this->assertSame('*****', $authenticator->__debugInfo()['config']['salt']); + $this->assertStringContainsString('remember_me', $output, 'Non sensitive config keys must stay visible.'); + } + + public function testCookieAuthenticatorKeepsBooleanSalt(): void + { + $authenticator = new CookieAuthenticator(null, ['salt' => true]); + + $this->assertTrue($authenticator->__debugInfo()['config']['salt'], 'A boolean salt flag is not a secret.'); + } + + public function testAuthenticationServiceRedactsNestedAuthenticatorSecrets(): void + { + $service = new AuthenticationService([ + 'authenticators' => [ + 'Authentication.Jwt' => [ + 'secretKey' => 'planted-service-jwt-secret', + ], + 'Authentication.HttpDigest' => [ + 'secret' => 'planted-service-digest-secret', + ], + 'Authentication.Cookie' => [ + 'salt' => 'planted-service-cookie-salt', + 'rememberMeField' => 'remember_me', + ], + ], + ]); + + $output = $this->debugOutput($service); + + $this->assertStringNotContainsString('planted-service-jwt-secret', $output); + $this->assertStringNotContainsString('planted-service-digest-secret', $output); + $this->assertStringNotContainsString('planted-service-cookie-salt', $output); + + $nested = $service->__debugInfo()['config']['authenticators']; + $this->assertSame('*****', $nested['Authentication.Jwt']['secretKey']); + $this->assertSame('*****', $nested['Authentication.HttpDigest']['secret']); + $this->assertSame('*****', $nested['Authentication.Cookie']['salt']); + $this->assertSame('remember_me', $nested['Authentication.Cookie']['rememberMeField'], 'Non sensitive config keys must stay visible.'); + + $this->assertStringContainsString('remember_me', $output, 'Non sensitive config keys must stay visible.'); + $this->assertStringContainsString('identityClass', $output, 'Top level config keys must stay visible.'); + } +} diff --git a/tests/TestCase/SensitiveParameterTest.php b/tests/TestCase/SensitiveParameterTest.php new file mode 100644 index 00000000..6b1d2399 --- /dev/null +++ b/tests/TestCase/SensitiveParameterTest.php @@ -0,0 +1,301 @@ +}> + */ + public static function sensitiveParametersProvider(): array + { + return [ + 'PasswordHasherInterface::hash' => [ + PasswordHasherInterface::class, + 'hash', + ['password'], + ], + 'PasswordHasherInterface::check' => [ + PasswordHasherInterface::class, + 'check', + ['password', 'hashedPassword'], + ], + 'PasswordHasherInterface::needsRehash' => [ + PasswordHasherInterface::class, + 'needsRehash', + ['password'], + ], + 'AbstractPasswordHasher::needsRehash' => [ + AbstractPasswordHasher::class, + 'needsRehash', + ['password'], + ], + 'DefaultPasswordHasher::hash' => [ + DefaultPasswordHasher::class, + 'hash', + ['password'], + ], + 'DefaultPasswordHasher::check' => [ + DefaultPasswordHasher::class, + 'check', + ['password', 'hashedPassword'], + ], + 'DefaultPasswordHasher::needsRehash' => [ + DefaultPasswordHasher::class, + 'needsRehash', + ['password'], + ], + 'LegacyPasswordHasher::hash' => [ + LegacyPasswordHasher::class, + 'hash', + ['password'], + ], + 'LegacyPasswordHasher::check' => [ + LegacyPasswordHasher::class, + 'check', + ['password', 'hashedPassword'], + ], + 'FallbackPasswordHasher::hash' => [ + FallbackPasswordHasher::class, + 'hash', + ['password'], + ], + 'FallbackPasswordHasher::check' => [ + FallbackPasswordHasher::class, + 'check', + ['password', 'hashedPassword'], + ], + 'FallbackPasswordHasher::needsRehash' => [ + FallbackPasswordHasher::class, + 'needsRehash', + ['password'], + ], + 'IdentifierInterface::identify' => [ + IdentifierInterface::class, + 'identify', + ['credentials'], + ], + 'PasswordIdentifier::identify' => [ + PasswordIdentifier::class, + 'identify', + ['credentials'], + ], + 'PasswordIdentifier::_checkPassword' => [ + PasswordIdentifier::class, + '_checkPassword', + ['identity', 'password'], + ], + 'TokenIdentifier::identify' => [ + TokenIdentifier::class, + 'identify', + ['credentials'], + ], + 'LdapIdentifier::identify' => [ + LdapIdentifier::class, + 'identify', + ['credentials'], + ], + 'LdapIdentifier::_bindUser' => [ + LdapIdentifier::class, + '_bindUser', + ['password'], + ], + 'CallbackIdentifier::identify' => [ + CallbackIdentifier::class, + 'identify', + ['credentials'], + ], + 'Ldap AdapterInterface::bind' => [ + AdapterInterface::class, + 'bind', + ['password'], + ], + 'ExtensionAdapter::bind' => [ + ExtensionAdapter::class, + 'bind', + ['password'], + ], + 'HttpDigestAuthenticator::parseAuthData' => [ + HttpDigestAuthenticator::class, + 'parseAuthData', + ['digest'], + ], + 'HttpDigestAuthenticator::generateResponseHash' => [ + HttpDigestAuthenticator::class, + 'generateResponseHash', + ['digest', 'password'], + ], + 'HttpDigestAuthenticator::password' => [ + HttpDigestAuthenticator::class, + 'password', + ['password'], + ], + 'TokenAuthenticator::stripTokenPrefix' => [ + TokenAuthenticator::class, + 'stripTokenPrefix', + ['token'], + ], + 'JwtAuthenticator::decodeToken' => [ + JwtAuthenticator::class, + 'decodeToken', + ['token'], + ], + 'CookieAuthenticator::_authenticateToken' => [ + CookieAuthenticator::class, + '_authenticateToken', + ['token'], + ], + 'CookieAuthenticator::_authenticateLegacyToken' => [ + CookieAuthenticator::class, + '_authenticateLegacyToken', + ['token'], + ], + 'CookieAuthenticator::_createLegacyPlainToken' => [ + CookieAuthenticator::class, + '_createLegacyPlainToken', + ['identity'], + ], + 'CookieAuthenticator::_createToken' => [ + CookieAuthenticator::class, + '_createToken', + ['identity'], + ], + ]; + } + + /** + * Parameters that must stay unmarked as they are not secret. + * + * @return array}> + */ + public static function plainParametersProvider(): array + { + return [ + 'LdapIdentifier::_bindUser username' => [ + LdapIdentifier::class, + '_bindUser', + ['username'], + ], + 'HttpDigestAuthenticator::password username' => [ + HttpDigestAuthenticator::class, + 'password', + ['username'], + ], + 'HttpDigestAuthenticator::password realm' => [ + HttpDigestAuthenticator::class, + 'password', + ['realm'], + ], + 'HttpDigestAuthenticator::generateResponseHash method' => [ + HttpDigestAuthenticator::class, + 'generateResponseHash', + ['method'], + ], + 'TokenAuthenticator::stripTokenPrefix prefix' => [ + TokenAuthenticator::class, + 'stripTokenPrefix', + ['prefix'], + ], + ]; + } + + /** + * @param class-string $class Class name + * @param string $method Method name + * @param array $parameters Parameter names expected to be sensitive + * @return void + */ + #[DataProvider('sensitiveParametersProvider')] + public function testParameterIsMarkedSensitive(string $class, string $method, array $parameters): void + { + $reflection = new ReflectionMethod($class, $method); + foreach ($parameters as $name) { + $parameter = $this->parameterNamed($reflection, $name); + $this->assertNotNull($parameter, sprintf('Parameter `%s` not found on `%s::%s()`.', $name, $class, $method)); + $this->assertNotEmpty( + $parameter->getAttributes(SensitiveParameter::class), + sprintf('Parameter `$%s` of `%s::%s()` must be marked with #[SensitiveParameter].', $name, $class, $method), + ); + } + } + + /** + * @param class-string $class Class name + * @param string $method Method name + * @param array $parameters Parameter names expected to stay unmarked + * @return void + */ + #[DataProvider('plainParametersProvider')] + public function testPlainParameterIsNotMarkedSensitive(string $class, string $method, array $parameters): void + { + $reflection = new ReflectionMethod($class, $method); + foreach ($parameters as $name) { + $parameter = $this->parameterNamed($reflection, $name); + $this->assertNotNull($parameter, sprintf('Parameter `%s` not found on `%s::%s()`.', $name, $class, $method)); + $this->assertEmpty( + $parameter->getAttributes(SensitiveParameter::class), + sprintf('Parameter `$%s` of `%s::%s()` is not secret and must not be marked sensitive.', $name, $class, $method), + ); + } + } + + /** + * Finds a parameter by name. + * + * @param \ReflectionMethod $method Method to inspect + * @param string $name Parameter name + * @return \ReflectionParameter|null The parameter or null when not found + */ + protected function parameterNamed(ReflectionMethod $method, string $name): ?ReflectionParameter + { + foreach ($method->getParameters() as $parameter) { + if ($parameter->getName() === $name) { + return $parameter; + } + } + + return null; + } +} From 8ebbf3e6472108b9ad41ac34e62dd8e31461a44f Mon Sep 17 00:00:00 2001 From: ADmad Date: Wed, 21 Oct 2026 13:18:23 +0530 Subject: [PATCH 2/2] Keep last 4 bytes of secrets visible in redacted debug output. Allows developers to verify that the correct secret/key is configured without exposing the full value. Secrets of 8 bytes or less are masked entirely. The shared masking rule lives in RedactionTrait. Also only redact string values in AuthenticationService's nested config, so boolean flags like CookieAuthenticator's salt stay visible. --- src/AuthenticationService.php | 6 +- src/Authenticator/AbstractAuthenticator.php | 2 + src/Authenticator/CookieAuthenticator.php | 6 +- src/Authenticator/HttpDigestAuthenticator.php | 7 +- src/Authenticator/JwtAuthenticator.php | 7 +- src/RedactionTrait.php | 44 ++++++++++++ tests/TestCase/DebugRedactionTest.php | 67 +++++++++++++++---- 7 files changed, 116 insertions(+), 23 deletions(-) create mode 100644 src/RedactionTrait.php diff --git a/src/AuthenticationService.php b/src/AuthenticationService.php index 8a4b2332..1216d5f6 100644 --- a/src/AuthenticationService.php +++ b/src/AuthenticationService.php @@ -38,6 +38,7 @@ class AuthenticationService implements AuthenticationServiceInterface, ImpersonationInterface { use InstanceConfigTrait; + use RedactionTrait; /** * Authenticator collection @@ -574,8 +575,9 @@ public function __debugInfo(): array continue; } foreach (['secretKey', 'secret', 'salt'] as $secretKey) { - if (isset($authenticatorConfig[$secretKey])) { - $config['authenticators'][$name][$secretKey] = '*****'; + $value = $authenticatorConfig[$secretKey] ?? null; + if (is_string($value)) { + $config['authenticators'][$name][$secretKey] = static::redactSecret($value); } } } diff --git a/src/Authenticator/AbstractAuthenticator.php b/src/Authenticator/AbstractAuthenticator.php index 8fec2c86..0237ccb8 100644 --- a/src/Authenticator/AbstractAuthenticator.php +++ b/src/Authenticator/AbstractAuthenticator.php @@ -18,12 +18,14 @@ use Authentication\Identifier\IdentifierInterface; use Authentication\Identifier\PasswordIdentifier; +use Authentication\RedactionTrait; use Cake\Core\InstanceConfigTrait; use Psr\Http\Message\ServerRequestInterface; abstract class AbstractAuthenticator implements AuthenticatorInterface { use InstanceConfigTrait; + use RedactionTrait; /** * Default config for this object. diff --git a/src/Authenticator/CookieAuthenticator.php b/src/Authenticator/CookieAuthenticator.php index 765d41f4..dbac5dbf 100644 --- a/src/Authenticator/CookieAuthenticator.php +++ b/src/Authenticator/CookieAuthenticator.php @@ -454,7 +454,9 @@ protected function _createCookie(mixed $value): CookieInterface * Debug info. * * Redacts the `salt` configuration value when it is a string as a - * string salt is used as the HMAC key for remember-me tokens. + * string salt is used as the HMAC key for remember-me tokens. The + * last 4 bytes are kept visible so the configured salt can be + * verified. * * @return array */ @@ -462,7 +464,7 @@ public function __debugInfo(): array { $config = $this->_config; if (isset($config['salt']) && is_string($config['salt'])) { - $config['salt'] = '*****'; + $config['salt'] = static::redactSecret($config['salt']); } return [ diff --git a/src/Authenticator/HttpDigestAuthenticator.php b/src/Authenticator/HttpDigestAuthenticator.php index d657a70a..90df1199 100644 --- a/src/Authenticator/HttpDigestAuthenticator.php +++ b/src/Authenticator/HttpDigestAuthenticator.php @@ -289,15 +289,16 @@ protected function validNonce(string $nonce): bool * Debug info. * * Redacts the `secret` configuration value so that debug output - * never contains the nonce signing secret. + * never contains the full nonce signing secret. The last 4 bytes are + * kept visible so the configured secret can be verified. * * @return array */ public function __debugInfo(): array { $config = $this->_config; - if (isset($config['secret'])) { - $config['secret'] = '*****'; + if (isset($config['secret']) && is_string($config['secret'])) { + $config['secret'] = static::redactSecret($config['secret']); } return [ diff --git a/src/Authenticator/JwtAuthenticator.php b/src/Authenticator/JwtAuthenticator.php index ea4f04dc..1ec987dd 100644 --- a/src/Authenticator/JwtAuthenticator.php +++ b/src/Authenticator/JwtAuthenticator.php @@ -180,15 +180,16 @@ protected function decodeToken(#[SensitiveParameter] string $token): ?object * Debug info. * * Redacts the `secretKey` configuration value so that debug output - * never contains the signing secret. + * never contains the full signing secret. The last 4 bytes are kept + * visible so the configured key can be verified. * * @return array */ public function __debugInfo(): array { $config = $this->_config; - if (isset($config['secretKey'])) { - $config['secretKey'] = '*****'; + if (isset($config['secretKey']) && is_string($config['secretKey'])) { + $config['secretKey'] = static::redactSecret($config['secretKey']); } return [ diff --git a/src/RedactionTrait.php b/src/RedactionTrait.php new file mode 100644 index 00000000..1a1f6dc8 --- /dev/null +++ b/src/RedactionTrait.php @@ -0,0 +1,44 @@ + + */ + public static function secretProvider(): array + { + return [ + 'long secret keeps last 4 bytes' => ['planted-hmac-salt-2f9c', '*****2f9c'], + 'secret of 9 bytes keeps last 4 bytes' => ['123456789', '*****6789'], + 'secret of 8 bytes is fully masked' => ['12345678', '*****'], + 'secret of 4 bytes is fully masked' => ['1234', '*****'], + 'empty secret' => ['', '*****'], + ]; + } + + /** + * @param string $secret Configured secret + * @param string $expected Expected redacted value + * @return void + */ + #[DataProvider('secretProvider')] + public function testRedactedSecretFormat(string $secret, string $expected): void + { + $authenticator = new CookieAuthenticator(null, ['salt' => $secret]); + + $this->assertSame($expected, $authenticator->__debugInfo()['config']['salt']); + } + public function testJwtAuthenticatorRedactsSecretKey(): void { $authenticator = new JwtAuthenticator(null, [ @@ -54,21 +88,21 @@ public function testJwtAuthenticatorRedactsSecretKey(): void $output = $this->debugOutput($authenticator); $this->assertStringNotContainsString('planted-jwt-secret', $output); - $this->assertSame('*****', $authenticator->__debugInfo()['config']['secretKey']); + $this->assertSame('*****cret', $authenticator->__debugInfo()['config']['secretKey']); $this->assertStringContainsString('HS256', $output, 'Non sensitive config keys must stay visible.'); } public function testHttpDigestAuthenticatorRedactsSecret(): void { $authenticator = new HttpDigestAuthenticator(null, [ - 'secret' => 'planted-digest-secret', + 'secret' => 'planted-digest-nonce-4f2e', 'qop' => 'auth', ]); $output = $this->debugOutput($authenticator); - $this->assertStringNotContainsString('planted-digest-secret', $output); - $this->assertSame('*****', $authenticator->__debugInfo()['config']['secret']); + $this->assertStringNotContainsString('planted-digest-nonce-4f2e', $output); + $this->assertSame('*****4f2e', $authenticator->__debugInfo()['config']['secret']); $this->assertStringContainsString('qop', $output, 'Non sensitive config keys must stay visible.'); } @@ -82,7 +116,7 @@ public function testCookieAuthenticatorRedactsStringSalt(): void $output = $this->debugOutput($authenticator); $this->assertStringNotContainsString('planted-hmac-salt', $output); - $this->assertSame('*****', $authenticator->__debugInfo()['config']['salt']); + $this->assertSame('*****salt', $authenticator->__debugInfo()['config']['salt']); $this->assertStringContainsString('remember_me', $output, 'Non sensitive config keys must stay visible.'); } @@ -98,29 +132,36 @@ public function testAuthenticationServiceRedactsNestedAuthenticatorSecrets(): vo $service = new AuthenticationService([ 'authenticators' => [ 'Authentication.Jwt' => [ - 'secretKey' => 'planted-service-jwt-secret', + 'secretKey' => 'planted-service-jwt-key-a1b2', ], 'Authentication.HttpDigest' => [ - 'secret' => 'planted-service-digest-secret', + 'secret' => 'planted-service-digest-c3d4', ], 'Authentication.Cookie' => [ 'salt' => 'planted-service-cookie-salt', 'rememberMeField' => 'remember_me', ], + 'Authentication.Session' => [ + 'salt' => true, + ], ], ]); $output = $this->debugOutput($service); - $this->assertStringNotContainsString('planted-service-jwt-secret', $output); - $this->assertStringNotContainsString('planted-service-digest-secret', $output); + $this->assertStringNotContainsString('planted-service-jwt-key-a1b2', $output); + $this->assertStringNotContainsString('planted-service-digest-c3d4', $output); $this->assertStringNotContainsString('planted-service-cookie-salt', $output); $nested = $service->__debugInfo()['config']['authenticators']; - $this->assertSame('*****', $nested['Authentication.Jwt']['secretKey']); - $this->assertSame('*****', $nested['Authentication.HttpDigest']['secret']); - $this->assertSame('*****', $nested['Authentication.Cookie']['salt']); + $this->assertSame('*****a1b2', $nested['Authentication.Jwt']['secretKey']); + $this->assertSame('*****c3d4', $nested['Authentication.HttpDigest']['secret']); + $this->assertSame('*****salt', $nested['Authentication.Cookie']['salt']); $this->assertSame('remember_me', $nested['Authentication.Cookie']['rememberMeField'], 'Non sensitive config keys must stay visible.'); + $this->assertTrue( + $nested['Authentication.Session']['salt'], + 'A boolean salt flag is not a secret and must stay visible.', + ); $this->assertStringContainsString('remember_me', $output, 'Non sensitive config keys must stay visible.'); $this->assertStringContainsString('identityClass', $output, 'Top level config keys must stay visible.');