Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xss and csrf #145

Merged
merged 12 commits into from
Apr 1, 2016
2 changes: 1 addition & 1 deletion application/config/autoload.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
| $autoload['helper'] = array('url', 'file');
*/

$autoload['helper'] = array('url', 'assets', 'language', 'array', 'string');
$autoload['helper'] = array('url', 'assets', 'language', 'array', 'string', 'form');

/*
| -------------------------------------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions application/config/config.php
Original file line number Diff line number Diff line change
Expand Up @@ -431,11 +431,11 @@
| 'csrf_regenerate' = Regenerate token on every submission
| 'csrf_exclude_uris' = Array of URIs which ignore CSRF checks
*/
$config['csrf_protection'] = FALSE;
$config['csrf_protection'] = TRUE;
$config['csrf_token_name'] = 'csrf_test_name';
$config['csrf_cookie_name'] = 'csrf_cookie_name';
$config['csrf_expire'] = 7200;
$config['csrf_regenerate'] = TRUE;
$config['csrf_regenerate'] = FALSE;
$config['csrf_exclude_uris'] = array();

/*
Expand Down
40 changes: 11 additions & 29 deletions application/controllers/Ajax.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ function checkEmail() {
if ($this->expectsPost(array('email'))) {
$result = array();

if (!$this->user->checkUserEmail($this->input->post('email'))) {
if (!$this->user->checkUserEmail($this->email)) {
$result['success'] = true;
} else {
$result['success'] = false;
Expand Down Expand Up @@ -133,13 +133,6 @@ function facebookSignup() {
if ($this->expectsPost(array('email', 'last_name',
'firstname'))) {

/**
* Getting all the posts
*/
$email = $this->input->post('email');
$name = $this->input->post('last_name');
$firstname = $this->input->post('firstname');
$country = $this->input->post('country');
/**
* For fb user, we don't have their fb password (obviously).
* Yet, having a password is mandatory in tw and I don't feel
Expand All @@ -153,24 +146,24 @@ function facebookSignup() {
$password = "FB_"+$this->input->post('id');

// If the email doesn't exists yet
if (!$this->user->checkUserEmail($email)) {
if (!$this->user->checkUserEmail($this->email)) {

/**
* Signup attempt
* TODO: Can this fail ? If so, under which circonstances ? If not,
* remove the if, if yes, provide a else with a dedicated response
* code.
*/
if ($this->user->signup($email, $password, $name, $firstname, $country)) {
if ($this->user->signup($this->email, $password, $this->firstname, $this->last_name, "")) {

$result['success'] = "signup";
$this->user->login($email, $password);
$this->user->login($this->email, $password);

}

// The email was already in the db, so we try to log the user
// using a potentially existing account
} else if ($this->user->login($email, $password)) {
} else if ($this->user->login($this->email, $password)) {

$result['success'] = "signin";

Expand Down Expand Up @@ -205,24 +198,18 @@ function signup() {

$result = array();

$email = $this->input->post('email');
$password = $this->input->post('password');
$name = $this->input->post('name');
$firstname = $this->input->post('firstname');
$country = $this->input->post('country');

//If the email isn't already in used
if (!$this->user->checkUserEmail($email)) {
if (!$this->user->checkUserEmail($this->email)) {

// Create the account
if ($this->user->signup(
$email, $password, $name, $firstname,
$country)) {
$this->email, $this->password, $this->name, $this->firstname,
$this->country)) {

$result['success'] = true;

//Log the user will create his session and so on
$this->user->login($email, $password);
$this->user->login($this->email, $this->password);

}

Expand All @@ -246,14 +233,12 @@ function askResetPassword() {

if ($this->expectsPost(array('email'))) {

$email = $this->input->post('email');

$result = array();

//We don't send the token over the network, we just
//make sure that a token has been generated.
//The token will be transfered in an email.
$resetToken = $this->user->askResetPassword($email);
$resetToken = $this->user->askResetPassword($this->email);

if ($resetToken) {

Expand Down Expand Up @@ -281,12 +266,9 @@ function resetPassword() {

$result = array();

$resetToken = $this->input->post('resetToken');
$password = $this->input->post('password');

//Attempting to reset the password given the token and the
//new password
if ($this->user->resetPassword($resetToken, $password)) {
if ($this->user->resetPassword($this->resetToken, $this->password)) {

$result['success'] = true;
} else {
Expand Down
8 changes: 0 additions & 8 deletions application/controllers/Measures.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,6 @@ public function delete_measure(){

if($this->expectsPost(array('deleteMeasures'))){


var_dump($this->session->userdata('userId'));
var_dump($this->deleteMeasures);
var_dump($this->measure->isOwnedBy(
$this->deleteMeasures,
$this->session->userdata('userId')));

if (
$this->measure->isOwnedBy(
$this->deleteMeasures,
Expand Down Expand Up @@ -220,7 +213,6 @@ public function edit_watch(){

$this->constructMeasurePage();
}
echo "not all posts";
}

/**
Expand Down
4 changes: 2 additions & 2 deletions application/core/MY_Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public function __construct() {
$this->_headerData['userIsLoggedIn'] = $this->user->isLoggedIn();
$this->_headerData['styleSheets'] = array('main');
$this->_headerData['javaScripts'] = array('jquery.min', 'bootstrap.min', 'application', 'MediaElement/mediaelement-and-player.min',
'facebook');
'facebook', "js.cookie");
$this->_headerData['headerClass'] = '';

if ($this->_needLoggedIn && !$this->user->isLoggedIn()) {
Expand Down Expand Up @@ -51,7 +51,7 @@ protected function expectsPost($postNames){
}

//Add the variable in $this
$this->{$postName} = $this->input->post($postName);
$this->{$postName} = $this->security->xss_clean($this->input->post($postName));
}

return true;
Expand Down
21 changes: 20 additions & 1 deletion application/models/Measure.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,26 @@ public function computeAccuracy($watchMeasure) {
{
$userDelta = $watchMeasure->accuracyUserTime-$watchMeasure->measureUserTime;
$refDelta = $watchMeasure->accuracyReferenceTime-$watchMeasure->measureReferenceTime;
$accuracy = ($userDelta*86400/$refDelta)-86400;

/*
Until 1.3.0, users were asked to enter the time
displayed on their timepiece after a 5 secs countdown.

Since 1.3.0 users are asked to click when their Watch
display a given time. This reverses the accuracy formulae...

This side effect of the new measure system (https:github.com/MathieuNls/tw/issues/58)
was reported (#136 and #137) and ignored on the basis that the test harness would have caught it.

The following testes if the measure was taken before 1.3 - 15 fev 2016 (epoch 1455537600) (commit d861c8e436b5ea8909cd1949f86fd20a14b272b4) and adapts the formulae.
*/
if($watchMeasure->accuracyReferenceTime < 1455537600){

$accuracy = ($userDelta*86400/$refDelta)-86400;
}else{
$accuracy = ($refDelta*86400/$userDelta)-86400;
}

$accuracy = sprintf("%.1f", $accuracy);
$watchMeasure->accuracy = $accuracy;

Expand Down
2 changes: 1 addition & 1 deletion application/tests/controllers/Home_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public function test_about() {
public function test_contact() {
$output = $this->request('GET', ['Home', 'contact']);
$this->assertContains('<title>Toolwatch • Contact</title>', $output);
$this->assertContains('<form class="form-horizontal" name="contact">', $output);
$this->assertContains('name="contact" class="form-horizontal"', $output);
}

public function test_resetPassword() {
Expand Down
4 changes: 2 additions & 2 deletions application/tests/controllers/Modal_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public function test_login(){
]
);

$this->assertContains('<form method="post" name="login">', $output);
$this->assertContains('name="login" method="post"', $output);
}

public function test_loginFail(){
Expand All @@ -45,7 +45,7 @@ public function test_signUp(){
]
);

$this->assertContains('<form method="post" name="signup">', $output);
$this->assertContains('name="signup" method="post"', $output);
}

public function test_signUpFail(){
Expand Down
71 changes: 57 additions & 14 deletions application/tests/models/Measure_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ class Measure_test extends TestCase {
private static $userId;
private static $watchId;
private static $watchId2;
private static $watchId3;
private static $measureId;
private static $watch;
private static $watchMeasure;
Expand Down Expand Up @@ -36,7 +37,7 @@ public static function setUpBeforeClass() {
self::$watchId = $CI->Watch->addWatch(
self::$userId,
'brand',
'name',
'name1',
2015,
28,
014
Expand All @@ -45,7 +46,16 @@ public static function setUpBeforeClass() {
self::$watchId2 = $CI->Watch->addWatch(
self::$userId,
'brand',
'name',
'name2',
2015,
28,
014
);

self::$watchId3 = $CI->Watch->addWatch(
self::$userId,
'brand',
'name3',
2015,
28,
014
Expand Down Expand Up @@ -155,7 +165,8 @@ public function test_getMeasuresByUser2() {
mesure : 20:52:30
start countdown : 20:52:30 (+3 days)
mesure : 20:52:36
spd : +2 sec per day
spd : -2 sec per day
@before 1.3
*/
public function test_addBaseMesure1() {

Expand Down Expand Up @@ -209,6 +220,7 @@ public function test_getMeasuresByUser4() {
start countdown : 17:30:20 (+1.5 days)
mesure : 17:31:26
spd : +4 sec per day
@before 1.3
*/
public function test_addBaseMesure2() {

Expand All @@ -233,6 +245,37 @@ public function test_addAccuracyMesure2() {
$this->assertEquals(2, $watchMeasure->statusId);
}

/*
start countdown : 09:08:01
mesure : 09:08:01
start countdown : 09:08:00 (+1 day)
mesure : 09:07:55
spd : +5 sec per day
@after 1.3
*/
public function test_addBaseMesure3() {

self::$measureId = $this->obj->addBaseMesure(
self::$watchId3,
1458634081,
1458634081
);

$this->assertEquals(true, is_numeric(self::$measureId));
}

public function test_addAccuracyMesure3() {
$watchMeasure = $this->obj->addAccuracyMesure(
self::$measureId,
1458720480,
1458720475
);

$this->assertEquals(self::$watchId3, $watchMeasure->watchId);
$this->assertEquals(5.0, $watchMeasure->accuracy, 'it should be 5.0');
$this->assertEquals(2, $watchMeasure->statusId);
}

public function test_getMeasuresByUser6() {

$measures = $this->obj->getMeasuresByUser(
Expand Down Expand Up @@ -292,25 +335,25 @@ public function test_deleteMeasure() {

public function test_getMeasuresCountByWatchBrand() {
$count = $this->obj->getMeasuresCountByWatchBrand('brand');
$this->assertEquals(7, $count);
$this->assertEquals(8, $count);
}

public function test_computePercentileAccuracy(){

$this->assertEquals(67, $this->obj->computePercentileAccuracy(1.5));
$this->assertEquals(75, $this->obj->computePercentileAccuracy(1.5));
$this->assertEquals(0, $this->obj->computePercentileAccuracy(7));

}

public static function tearDownAfterClass() {
$CI = &get_instance();
$CI->load->model('User');
$CI->load->model('Watch');
$CI->load->model('Measure');
$CI->watch->delete_where(array("watchId >=" => "0"));
$CI->User->delete_where(array("userId >=" => "0"));
$CI->Measure->delete_where(array("id >=" => "0"));
}
// public static function tearDownAfterClass() {
// $CI = &get_instance();
// $CI->load->model('User');
// $CI->load->model('Watch');
// $CI->load->model('Measure');
// $CI->watch->delete_where(array("watchId >=" => "0"));
// $CI->User->delete_where(array("userId >=" => "0"));
// $CI->Measure->delete_where(array("id >=" => "0"));
// }

}

Expand Down
Loading