Skip to content

Commit

Permalink
Mark parameters containing user credentials as sensitive
Browse files Browse the repository at this point in the history
PHP 8.2 introduced the `SensitiveParameter` attribute.

The effect of the attribute is that the value of the parameter is no longer directly shown in stack traces; instead, starting with PHP 8.2, the parameter will be presented as a `SensitiveParameterValue` object.

As the attribute only applies to parameters, it (unfortunately) has no effect on serialization of the object. See: https://3v4l.org/StoQO
Might be an idea to start a discussion about an `SensitiveProperty` attribute on the PHP Internals mailing list, but that's outside the scope of this PR.

For now, this PR marks the `$args` parameter for the `Auth\Basic` class constructor and the `Proxy\Http` constructor as sensitive as both of these are supposed to contain user credentials (user name, password) for accessing a protected URL.

Includes updating the example code for custom authentication to also use the attribute.

**Open question**: the `$options` array passed to a large range of Requests methods can [also contain credentials](https://github.com/WordPress/Requests/blob/ebb9f65855c860bc33005b3d8bccf6444e598fba/src/Requests.php#L395-L399). Should this parameter also be marked as sensitive in all appropriate places ?

Refs:
* https://www.php.net/manual/en/class.sensitiveparameter.php
* https://wiki.php.net/rfc/redact_parameters_in_back_traces
  • Loading branch information
jrfnl committed Jul 8, 2024
1 parent ebb9f65 commit ee3e53a
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 3 deletions.
5 changes: 4 additions & 1 deletion docs/authentication-custom.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ services that do this; perhaps this is a market waiting to be tapped?)
class MySoftware_Auth_Hotdog implements WpOrg\Requests\Auth {
protected $password;

public function __construct($password) {
public function __construct(
#[\SensitiveParameter]
$password
) {
$this->password = $password;
}

Expand Down
6 changes: 5 additions & 1 deletion src/Auth/Basic.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

namespace WpOrg\Requests\Auth;

use SensitiveParameter;
use WpOrg\Requests\Auth;
use WpOrg\Requests\Exception\ArgumentCount;
use WpOrg\Requests\Exception\InvalidArgument;
Expand Down Expand Up @@ -48,7 +49,10 @@ class Basic implements Auth {
* @throws \WpOrg\Requests\Exception\InvalidArgument When the passed argument is not an array or null.
* @throws \WpOrg\Requests\Exception\ArgumentCount On incorrect number of array elements (`authbasicbadargs`).
*/
public function __construct($args = null) {
public function __construct(
#[SensitiveParameter]
$args = null
) {
if (is_array($args)) {
if (count($args) !== 2) {
throw ArgumentCount::create('an array with exactly two elements', count($args), 'authbasicbadargs');
Expand Down
5 changes: 4 additions & 1 deletion src/Proxy/Http.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ final class Http implements Proxy {
* @throws \WpOrg\Requests\Exception\InvalidArgument When the passed argument is not an array, a string or null.
* @throws \WpOrg\Requests\Exception\ArgumentCount On incorrect number of arguments (`proxyhttpbadargs`)
*/
public function __construct($args = null) {
public function __construct(
#[SensitiveParameter]
$args = null
) {
if (is_string($args)) {
$this->proxy = $args;
} elseif (is_array($args)) {
Expand Down

0 comments on commit ee3e53a

Please sign in to comment.