Skip to content

Commit

Permalink
removed excess comments, cleaned up code, added test to ensure we sti…
Browse files Browse the repository at this point in the history
…ll work with depreciated use of "**"
  • Loading branch information
fideloper committed Jan 18, 2018
1 parent 890f573 commit 9618897
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 11 deletions.
15 changes: 6 additions & 9 deletions src/TrustProxies.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,23 +66,20 @@ protected function setTrustedProxyIpAddresses(Request $request)
{
$trustedIps = $this->proxies ?: $this->config->get('trustedproxy.proxies');

// We only trust specific IP addresses
// Only trust specific IP addresses
if (is_array($trustedIps)) {
return $this->setTrustedProxyIpAddressesToSpecificIps($request, $trustedIps);
}

// We trust any IP address that calls us, but not proxies further
// up the forwarding chain.
// TODO: Determine if this should only trust the first IP address
// Currently it trusts the entire chain (array of IPs),
// potentially making the "**" convention redundant.
if ($trustedIps === '*') {
// Trust any IP address that calls us
// `**` for backwards compatibility, but is depreciated
if ($trustedIps === '*' || $trustedIps === '**') {
return $this->setTrustedProxyIpAddressesToTheCallingIp($request);
}
}

/**
* We specify the IP addresses to trust explicitly.
* Specify the IP addresses to trust explicitly.
*
* @param \Illuminate\Http\Request $request
* @param array $trustedIps
Expand All @@ -93,7 +90,7 @@ private function setTrustedProxyIpAddressesToSpecificIps(Request $request, $trus
}

/**
* We set the trusted proxy to be the first IP addresses received.
* Set the trusted proxy to be the IP address calling this servers
*
* @param \Illuminate\Http\Request $request
*/
Expand Down
18 changes: 16 additions & 2 deletions tests/TrustedProxyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public function test_request_does_not_trust()
public function test_does_trust_trusted_proxy()
{
$req = $this->createProxiedRequest();
$req->setTrustedProxies(['192.168.10.10'], Request::HEADER_X_FORWARDED_FOR);
$req->setTrustedProxies(['192.168.10.10'], Request::HEADER_X_FORWARDED_ALL);

$this->assertEquals('173.174.200.38', $req->getClientIp(), 'Assert trusted proxy x-forwarded-for header used');
$this->assertEquals('https', $req->getScheme(), 'Assert trusted proxy x-forwarded-proto header used');
Expand All @@ -56,6 +56,20 @@ public function test_trusted_proxy_sets_trusted_proxies_with_wildcard()
});
}

/**
* Test the next most typical usage of TrustedProxies:
* Trusted X-Forwarded-For header, wilcard for TrustedProxies
*/
public function test_trusted_proxy_sets_trusted_proxies_with_double_wildcard_for_backwards_compat()
{
$trustedProxy = $this->createTrustedProxy(Request::HEADER_X_FORWARDED_ALL, '**');
$request = $this->createProxiedRequest();

$trustedProxy->handle($request, function ($request) {
$this->assertEquals('173.174.200.38', $request->getClientIp(), 'Assert trusted proxy x-forwarded-for header used with wildcard proxy setting');
});
}



/**
Expand Down Expand Up @@ -197,7 +211,7 @@ protected function createProxiedRequest($serverOverRides = [])
// which is likely something like this:
$request = Request::create('http://localhost:8888/tag/proxy', 'GET', [], [], [], $serverOverRides, null);
// Need to make sure these haven't already been set
$request->setTrustedProxies([], Request::HEADER_X_FORWARDED_FOR);
$request->setTrustedProxies([], Request::HEADER_X_FORWARDED_ALL);

return $request;
}
Expand Down

0 comments on commit 9618897

Please sign in to comment.