Skip to content

Commit

Permalink
Request: Move Basic Auth credential from Url to Request due to preven…
Browse files Browse the repository at this point in the history
…t leak it (#211)
  • Loading branch information
jakubboucek authored Mar 1, 2022
1 parent 04224e7 commit e99694a
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 12 deletions.
20 changes: 20 additions & 0 deletions src/Http/Request.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ class Request implements IRequest
/** @var ?callable */
private $rawBodyCallback;

private ?string $user;

private ?string $password;


public function __construct(
UrlScript $url,
Expand All @@ -63,6 +67,8 @@ public function __construct(
?string $remoteAddress = null,
?string $remoteHost = null,
?callable $rawBodyCallback = null,
?string $user = null,
?string $password = null,
) {
$this->url = $url;
$this->post = (array) $post;
Expand All @@ -73,6 +79,8 @@ public function __construct(
$this->remoteAddress = $remoteAddress;
$this->remoteHost = $remoteHost;
$this->rawBodyCallback = $rawBodyCallback;
$this->user = $user;
$this->password = $password;
}


Expand Down Expand Up @@ -274,6 +282,18 @@ public function getRawBody(): ?string
}


public function getUser(): ?string
{
return $this->user;
}


public function getPassword(): ?string
{
return $this->password;
}


/**
* Returns the most preferred language by browser. Uses the `Accept-Language` header. If no match is reached, it returns `null`.
* @param string[] $langs supported languages
Expand Down
20 changes: 12 additions & 8 deletions src/Http/RequestFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ public function fromGlobals(): Request
$url = new Url;
$this->getServer($url);
$this->getPathAndQuery($url);
$this->getUserAndPassword($url);
[$post, $cookies] = $this->getGetPostCookie($url);
[$remoteAddr, $remoteHost] = $this->getClient($url);
[$user, $password] = $this->getUserAndPassword();

return new Request(
new UrlScript($url, $this->getScriptPath($url)),
Expand All @@ -73,6 +73,8 @@ public function fromGlobals(): Request
$remoteAddr,
$remoteHost,
fn(): string => file_get_contents('php://input'),
$user,
$password,
);
}

Expand Down Expand Up @@ -109,13 +111,6 @@ private function getPathAndQuery(Url $url): void
}


private function getUserAndPassword(Url $url): void
{
$url->setUser($_SERVER['PHP_AUTH_USER'] ?? '');
$url->setPassword($_SERVER['PHP_AUTH_PW'] ?? '');
}


private function getScriptPath(Url $url): string
{
if (PHP_SAPI === 'cli-server') {
Expand Down Expand Up @@ -290,6 +285,15 @@ private function getClient(Url $url): array
}


private function getUserAndPassword(): array
{
$user = $_SERVER['PHP_AUTH_USER'] ?? null;
$password = $_SERVER['PHP_AUTH_PW'] ?? null;

return [$user, $password];
}


private function useForwardedProxy(Url $url, &$remoteAddr, &$remoteHost): void
{
$forwardParams = preg_split('/[,;]/', $_SERVER['HTTP_FORWARDED']);
Expand Down
8 changes: 4 additions & 4 deletions tests/Http/RequestFactory.userAndPassword.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ $_SERVER = [
'PHP_AUTH_PW' => 'password',
];
$factory = new RequestFactory;
Assert::same('user', $factory->fromGlobals()->getUrl()->getUser());
Assert::same('password', $factory->fromGlobals()->getUrl()->getPassword());
Assert::same('user', $factory->fromGlobals()->getUser());
Assert::same('password', $factory->fromGlobals()->getPassword());


$_SERVER = [];
$factory = new RequestFactory;
Assert::same('', $factory->fromGlobals()->getUrl()->getUser());
Assert::same('', $factory->fromGlobals()->getUrl()->getPassword());
Assert::same(null, $factory->fromGlobals()->getUser());
Assert::same(null, $factory->fromGlobals()->getPassword());

0 comments on commit e99694a

Please sign in to comment.