Skip to content

Commit 5fda1dc

Browse files
committed
Added more test cases, some logic fixes
1 parent 944236a commit 5fda1dc

File tree

2 files changed

+46
-18
lines changed

2 files changed

+46
-18
lines changed

libs/csrf/csrfprotector.php

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,6 @@ function __construct($cfg) {
6565

6666
class csrfProtector
6767
{
68-
/*
69-
* application/json content type
70-
*/
71-
const JSONCONTENTTYPE = "application/json";
72-
7368
/*
7469
* Variable: $cookieExpiryTime
7570
* expiry time for cookie
@@ -94,10 +89,17 @@ class csrfProtector
9489
/**
9590
* Variable: $cookieConfig
9691
* Array of parameters for the setcookie method
97-
* @var cookieConfig;
92+
* @var array<any>
9893
*/
9994
private static $cookieConfig = null;
10095

96+
/**
97+
* Variable: $tokenHeaderKey
98+
* Key value in header array, which contain the token
99+
* @var string
100+
*/
101+
private static $tokenHeaderKey = null;
102+
101103
/*
102104
* Variable: $requestType
103105
* Varaible to store weather request type is post or get
@@ -192,6 +194,9 @@ public static function init($length = null, $action = null)
192194
if (self::$config['CSRFP_TOKEN'] == '')
193195
self::$config['CSRFP_TOKEN'] = CSRFP_TOKEN;
194196

197+
self::$tokenHeaderKey = 'HTTP_' .strtoupper(self::$config['CSRFP_TOKEN']);
198+
self::$tokenHeaderKey = str_replace('-', '_', self::$tokenHeaderKey);
199+
195200
// load parameters for setcookie method
196201
if (!isset(self::$config['cookieConfig']))
197202
self::$config['cookieConfig'] = array();
@@ -211,8 +216,6 @@ public static function init($length = null, $action = null)
211216
}
212217
}
213218

214-
// TODO: initialize the setcookie params, based on config;
215-
216219
// Authorise the incoming request
217220
self::authorizePost();
218221

@@ -267,7 +270,6 @@ public static function authorizePost()
267270
self::refreshToken(); //refresh token for successfull validation
268271
}
269272
} else if (!static::isURLallowed()) {
270-
271273
//currently for same origin only
272274
if (!(isset($_GET[self::$config['CSRFP_TOKEN']])
273275
&& isset($_SESSION[self::$config['CSRFP_TOKEN']])
@@ -293,7 +295,7 @@ public static function authorizePost()
293295
* any (string / bool) - token retrieved from header or form payload
294296
*/
295297
private static function getTokenFromRequest() {
296-
// look for token in header, then in payload
298+
// look for in $_POST, then header
297299
if (isset($_POST[self::$config['CSRFP_TOKEN']])) {
298300
return $_POST[self::$config['CSRFP_TOKEN']];
299301
}
@@ -302,12 +304,11 @@ private static function getTokenFromRequest() {
302304
if (isset(apache_request_headers()[self::$config['CSRFP_TOKEN']])) {
303305
return apache_request_headers()[self::$config['CSRFP_TOKEN']];
304306
}
305-
} else {
306-
$serverKey = 'HTTP_' .strtoupper(self::$config['CSRFP_TOKEN']);
307-
$serverKey = str_replace('-', '_', $serverKey);
308-
if (isset($_SERVER[$serverKey])) {
309-
return $_SERVER[$serverKey];
310-
}
307+
}
308+
309+
if (self::$tokenHeaderKey === null) return false;
310+
if (isset($_SERVER[self::$tokenHeaderKey])) {
311+
return $_SERVER[self::$tokenHeaderKey];
311312
}
312313

313314
return false;

test/csrfprotector_test.php

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -350,11 +350,10 @@ public function testAuthorisePost_failedAction_6()
350350
}
351351

352352
/**
353-
* test authorise success
353+
* test authorise success with token in $_POST
354354
*/
355355
public function testAuthorisePost_success()
356356
{
357-
358357
$_SERVER['REQUEST_METHOD'] = 'POST';
359358
$_POST[csrfprotector::$config['CSRFP_TOKEN']]
360359
= $_GET[csrfprotector::$config['CSRFP_TOKEN']]
@@ -382,6 +381,34 @@ public function testAuthorisePost_success()
382381
// $this->assertTrue(csrfp_wrapper::checkHeader($_SESSION[csrfprotector::$config['CSRFP_TOKEN']][0])); // Combine these 3 later
383382
}
384383

384+
/**
385+
* test authorise success with token in header
386+
*/
387+
public function testAuthorisePost_success_2()
388+
{
389+
unset($_POST[csrfprotector::$config['CSRFP_TOKEN']]);
390+
$_SERVER['REQUEST_METHOD'] = 'POST';
391+
$serverKey = 'HTTP_' .strtoupper(csrfprotector::$config['CSRFP_TOKEN']);
392+
$serverKey = str_replace('-', '_', $serverKey);
393+
394+
$csrfp = new csrfProtector;
395+
$reflection = new \ReflectionClass(get_class($csrfp));
396+
$property = $reflection->getProperty('tokenHeaderKey');
397+
$property->setAccessible(true);
398+
// change value to false
399+
$property->setValue($csrfp, $serverKey);
400+
401+
$_SERVER[$serverKey] = $_SESSION[csrfprotector::$config['CSRFP_TOKEN']][0];
402+
$temp = $_SESSION[csrfprotector::$config['CSRFP_TOKEN']];
403+
404+
csrfprotector::authorizePost(); //will create new session and cookies
405+
$this->assertFalse($temp == $_SESSION[csrfprotector::$config['CSRFP_TOKEN']][0]);
406+
$this->assertTrue(csrfp_wrapper::checkHeader('Set-Cookie'));
407+
$this->assertTrue(csrfp_wrapper::checkHeader('csrfp_token'));
408+
// $this->assertTrue(csrfp_wrapper::checkHeader($_SESSION[csrfprotector::$config['CSRFP_TOKEN']][0])); // Combine these 3 later
409+
410+
}
411+
385412
/**
386413
* test for generateAuthToken()
387414
*/

0 commit comments

Comments
 (0)