« Back Issue #237
  • 2014-05-26 14:34 Reported by sreenidhipm
  • 2014-06-16 16:09 Marked fixed
Status: Marked fixed
Priority: Medium

Captcha settings for Yioop - text and graphical

sreenidhipm 2014-05-26 14:34

The captcha settings for Yioop are either text, graphical or hash captcha. The user(admin) can select one among these three at any point of time; these changes are reflected in the Yioop system in the 'create account' and 'suggest-a-url' page. The user can also add text captcha questions and recovery questions to the database and in turn, delete the selected ones. There is an option to edit the selected questions as well, which is updated in the database.

Steps to reproduce: The following are the steps to reproduce:

1. Login as root 2. Select 'Captcha settings' under 'System components' 3. Select the captcha mode- text, graphical or hash(not yet implemented). 4.If text captcha is selected, add/edit/delete questions to and from the database 5. Check in create account and suggest-a-url page according to the captcha mode selected

administrator 2014-05-27 07:55

Hi Sreenidhi,

Please get me a patch that follows the Yioop coding guidelines: http://www.seekquarry.com/?c=main&p=coding In particular, there seem to be lots of lines over 80 characters, white space issues (there should be no tabs characters, trailing white space, ...), commented out code in your patch, CSS name and variable naming conventions not followed, etc. Once the patch is in the proper form I will look at it seriously.

Best, Chris

administrator 2014-05-30 20:14

Sreenidhi,

That's a pretty huge patch (obviously a lot of work), so it will take some time to digest all of its ramifications. Here are some things that need to be addressed before I am willing to apply it:

In the code fragment below ...

-/** Captcha Mode Possibility*/ -define('TEXT_CAPTCHA', 1); +/** Captcha Mode*/ +define('TEXT_CAPTCHA', 'text_captcha');

-/** Captcha Mode Possibility*/ -define('HASH_CAPTCHA', 2); +/** Captcha Mode*/ +define('HASH_CAPTCHA', 'hash_captcha'); +define('GRAPHICAL_CAPTCHA', 'graphical_captcha'); + +//Captcha Type +define('CAPTCHA', 'captcha'); +define('RECOVERY', 'recovery'); + +//Captcha Possibilities +define('MOST', 'most'); +define('LEAST', 'least');

Where are you defining things to be the same string in lower caps. A line like -define('TEXT_CAPTCHA', 1); is better than +define('TEXT_CAPTCHA', 'text_captcha'); in that comparing strings is slower. The define allows you to have a humanly readable identifier while at the same time allowing PHP to compare things as int's

In the next piece of code:

class ProfileModel extends Model

-{ +{ /* + var $profile_fields = array('API_ACCESS', 'AUTH_KEY', + 'AUTHENTICATION_MODE', 'CACHE_LINK', 'CAPTCHA_MODE','CAPTCHA_TYPE', + 'CAPTCHA_POSSIBILITIES','DEBUG_LEVEL', 'DESCRIPTION_WEIGHT', + 'DB_HOST', 'DBMS', 'DB_NAME','DB_PASSWORD', + */

$profile_fields is used in several places why are you commenting it out?

In the next snippet you are doing a print_r of a string (which is a little weird as print_r is usually used on arrays) and the string is not localized or localizable. + $method_name_choices = str_replace("preferences_choices_least%", + "preferences_choices_least", $method_name_filter_choices); + $method_name_choices = $method_name_choices.$method_index_choices; + + }else{ + print_r("Not the right question type!"); + } + return $method_name_choices; + } +

This next snippet is adding lines from a commit I did earlier today which had me worried that your code was out of sync with the git repository somehow

    }

- if (referenceNode) { + if(referenceNode) {

        var corrected_spell = elt("spell-check");

+ var thesaurus_results = elt("thesaurus-results"); + /* corrected_spell might not be present if WORD_SUGGEST off + If there are already thesaurus results we don't want to + clutter the top area so also don't suggest + */ + if(!corrected_spell || thesaurus_results) {return; }

        var csrf_token = elt("csrf-token").value;
+                    <form id = "text-captcha-container-add" 

+ class = "text-captcha-container-add" + method = "post" action = "">

In the next snippet you are directly including a javascript into a document.

Please use the $data["INCLUDE_SCRIPT"] mechanism for doing that. +</div><!-- End of div: current-activity --> + +<script type = "text/javascript" src = "./scripts/captchasettings.js" ></script> +

In the next snippet you are using inline CSS. Make a class selector and pull the styles out into search.css + ob_end_clean (); + $image_data_base64 = base64_encode($image_data); + e('<div style="width:260px;padding-top:10px;">'); + e('<img src="data:image/png;base64,'.$image_data_base64.'" + width="260" height="70" border="1" alt="CAPTCHA">');

In managelocales_element.php you seem to be making lots of changes and I wasn't sure why? Can you briefly explain? diff --git a/views/elements/managelocales_element.php b/views/elements/managelocales_element.php index e2b48e8..35e9dd8 100644 --- a/views/elements/managelocales_element.php +++ b/views/elements/managelocales_element.php

Finally, you left the dead code below in your patch... + // echo('<pre>'); + // print_r($_REQUEST."
"); + // echo('</pre>'); + // echo("
SESSION
"); + // echo('<pre>'); + // print_r($_SESSION); + // echo('</pre>'); + // echo("
DATA
"); + // echo('<pre>'); + // print_r($data); + // echo('</pre>');

Please address the above issues and make a new patch. I will look at it promptly.

Best, Chris

administrator 2014-05-31 00:36

Your patch is getting pretty close to done now... Hopefully, one more iteration after some sleep...

In config.php...

Why is wiki being deleted?

        "resultsEditor", "searchSources"),

- "social" => array("manageGroups", "groupFeeds", "mixCrawls", "wiki"), + "social" => array("manageGroups", "groupFeeds", "mixCrawls"),

WORK_DIRECTORY should stay blank. Use a local_config.php file to set the WORK_DIRECTORY for development purposes. -define('WORK_DIRECTORY', ''); +define('WORK_DIRECTORY', '/Library/WebServer/Documents/yioop_data200');

I still noticed a bunch of inline styles in captchasettings_element.php

Also, please don't use heading levels like h4 for a smaller size heading -- correctly nest headings according to WAI accessibility guidelines, and just style them to the size you want.

sreenidhipm 2014-05-31 11:36

Hello Professor,

The patch contains the changes needed, now.

administrator 2014-05-31 13:07

Hey Sreenidhi,

It doesnt look like you addressed the config.php issues.

Chris

sreenidhipm 2014-05-31 13:19

Hey professor,

If you scroll down a little bit, it will contain the changes made. That will be almost towards the end of the patch file.

Thanks, Sreenidhi

administrator 2014-05-31 14:25

Hi Sreenidhi,

Sorry about that. I just noticed though in verifying the above that captchasettings_element.php has lots of non-localized strings can you fix this? Also, text input tags need to have associated labels.

Best, Chris

sreenidhipm 2014-05-31 16:19

Hey Professor,

captchasettings_element now has all the strings localized.

Thanks, Sreenidhi

administrator 2014-05-31 22:04

Hey Sreenidhi,

I've applied your patch on my local machine and everything seems to be working. It will probably take me a couple days to move stuff around to the way I like before I push your stuff live . These kind of changes are stuff it would be impossible to get you to do unless you are into mind reading. Otherwise, it looks good. I will leave this issue open until the process is complete in case I have questions. Thanks for your hard work.

Best, Chris

sreenidhipm 2014-05-31 22:12

Awesome! Thank you so much, Professor! It was a pleasure, working on this project!

administrator 2014-06-16 16:09

I kept the image captcha and backed out a fair bit of the other code to make something that was stable and worked with all target DBMSs rather than only sqlite. A representative commit after my changes is 8f54f15cd. I need to rethink the UI on this before deciding how the word captcha should work. Closing this issue for now.

sreenidhipm 2014-06-16 16:09
X