Added hash-based captcha and implemented code to toggle between hash-based captcha and text-based captcha

Added test case for sha1

Akash,

Make sure to add me to the monitoring list when you create an issue.

Chris

Hi Akash,

I couldn't get your patch to apply. Grab the latest version of Yioop and make a new patch. Here are some specific comments:

(1) -define('WORK_DIRECTORY', ''); +define('WORK_DIRECTORY', 'C:/xampp1/htdocs/yioop_data'); Your patch should not edit the work directory in config.php

(2) Locale string should be all lower case: serversettings_element_Text_Captcha is a no-no.

(3) You should not call session_start in /controllers/controller.php b/controllers/controller.php displayView

(4) Do not leave commented out code: - $_SESSION['value'] = date('Y-m-d H:i:s', $visitor['END_TIME']); - $url = BASE_URL."?c=static&p=".$visitor['PAGE_NAME']; - header("Location:".$url); - exit(); + //$_SESSION['value'] = date('Y-m-d H:i:s', $visitor['END_TIME']); + //$url = BASE_URL."?c=static&p=".$visitor['PAGE_NAME']; + //header("Location:".$url); + //exit(); If you need to remove this explain why in the notes on your new patch.

(5) $_SESSION["randomString"] = substr( "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ", mt_rand(0, 50) , 1) .substr( md5( time() ), 1); line length is over 80 columns. Also, what is it you are trying to produce with this line?

(6) validateHashCode is not documented

(7) .border-top

{

- border-top:2px solid black; + border-top:2px solid #000000;

    padding-top:5px;
}
Why?

(8) a/models/profile_model.php b/models/profile_model.php - 'USER_AGENT_SHORT', 'WEB_URI', 'WEB_ACCESS', 'WORD_SUGGEST' + 'USER_AGENT_SHORT', 'WEB_URI', 'WEB_ACCESS', 'WORD_SUGGEST', 'CAPTCHA_MODE' list CAPTCHA_MODE in alphabetical order

(9) _safe_add do not name functions with leading underscore (10) _ft give functions meaningful names (11) Why are you using document.getElementById in hashcash.js? Let's assume basic.js will always be loaded., so can use elt.

Hello Dr.Pollett,

I have created second patch and attached. I have incorporated your suggestions.Regarding your question:

$_SESSION["randomString"] = substr( "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ", mt_rand(0, 50) , 1) .substr( md5( time() ), 1);

what is it you are trying to produce with this line? [Akash:] The idea behind this line is to generate the randomString. I have added md5 part to increase the randomness

I am worried these lines:

           $_SESSION["request_time"] = time();

+ $_SESSION["level"] = 2; + $_SESSION["randomString"] = substr( "abcdefghijklmnopqrstuvwxyzAB + CDEFGHIJKLMNOPQRSTUVWXYZ", mt_rand(0, 50) , 1) .substr + ( md5( time() ), 1); have a race condition between the two invocations of time. I think it would be better to tack in a salt in addition to time(), say AUTH_KEY, otherwise it is not very random and might be guessable using trying drifts from network time.

Got your point.So you are suggesting to add salt value in the string on which sha1 is calucalted.

Yes.

ok. I am planning to take: salt = rand(0,100). Is it fine?

Hey Akash,

Thanks for your hard work. I partially committed your patch. I'd like you to continue working on this feature to completely implement it. First, git pull the mods I made after applying your patch. Mainly, I renamed some variables, fixed white-space and long-line issues. Below are the more substantive problems I had: (1) There is a use of $_SESSION['randomString'] I don't understand. If it's not useful please delete it. In any case, field values should be random_string or RANDOM_STRING. (2) I deleted sha1_test.php. This was throwing errors caused I believe because it is not using the existing UnitTest class. Please rewrite it so that it uses this framework. In particular, it shouldn't have to write out a complete web page. If you do output HTML please don't do things like border="1" -- use CSS instead. Also, your code was too complicated. You could have greatly simplified it by writing out Javascript arrays in PHP-land and testing those in a for loop. (3) Your code should also replace the text captcha for account registration. Here is should replace just the captcha not the recover account questions.

Please make a new patch from the current starting point addressing the above.

Best, Chris

Hello Dr.Pollett,

I will fix first 2 problems.

For the third comment, when you are saying "text captcha for account registration" you mean create account module right?

For (3) Yes

In my testing your code seems to break the previous captcha stuff (I get a bunch of notices and warnings now when using the text-based captcha). Also, make sure to change <script> --> <script type="text/javascript" > Please make a new patch (do a git pull first) after testing both the new and old captcha. Also, rename hashcash.js to hashcaptcha.js and update includes, etc.

One question: as you said you were getting bunch of notices and warnings when using text-based captcha, is it on UI or console?

I am not sure what you means. They were PHP generated warnings. Where those go (UI or console) depends on how you have your web server configured.

Actually I am not getting any warnings in PHP log file for xampp so I was wondering which kind of the warnings you are getting.

I was debugging issue with the Text CAPTCHA. I think the issue is, if first time CAPTCHA answers are incorrect then it is not working next time even if right answers are provided in the Suggest URL module. However it works when right answers are provided first time.I am investigating that issue.

Thanks.

Fixed by 535d1c9c2

Attachments:

my6.patch

createAccount_captchaToggle.patch

Sha1_test.patch

my5.patch