Extends security of drupal websites with 2-step phone code authentication using Android app Drulapp. This is done by splitting login screen in to two pages, one for login credentials and another for submitting code generated on phone. Code generation and Authentication is based on rfc6238 which will be valid for 30 seconds. (with one past "time step window". see the link for why it is needed).
Features

  • Partially works offline. This is done by setting validity in days(just like Drupal core's cache validity). After validity expired Drulapp app must connect to website to get the updated buttons.
  • All device id and user related information are hashed using Sha256 and Hex encoded.
  • more details ....

sandbox project page:
https://www.drupal.org/sandbox/nithinkolekar/2856406
git clone command:
git clone --branch 7.x-1.x https://git.drupal.org/sandbox/nithinkolekar/2856406.git drulapp
code repository :
http://drupalcode.org/sandbox-nithinkolekar-2856406

For reviewers:
addition to the code review you must install above mentioned app(currently only android) to test complete functional task of this module.

Comments

nithinkolekar created an issue. See original summary.

jeetendrakumar’s picture

jeetendrakumar commented
Status: Needs review » Needs work

Please fix following errors:

https://pareview.sh/node/1265

nithinkolekar’s picture

nithinkolekar commented
Status: Needs work » Needs review

Corrected errors and added missing CSS file.

PA robot’s picture

PA robot commented
Status: Needs review » Needs work

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpsgitdrupalorgsandboxnithinkolekar2856406git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

nithinkolekar’s picture

nithinkolekar commented
Status: Needs work » Needs review

All errors and warning codes are corrected. Though some of the warning messages like param type is not necessary for d7, for ex @param $account as per the code in d7 core's user.module.

khurram_awan’s picture

khurram_awan commented
Status: Needs review » Needs work

Hello Mate,
Looks like you readme.txt is empty. please read Readme documentation and add readme.txt accordingly.
Thanks,
Khurram

yogesh kushwaha’s picture

yogesh kushwaha commented

Hi nithinkolekar,

Below are my manual review

  • Please write a descriptive @file comment. There is a dummy comment.
  • Please use configuration form for app related settings instead of defining constants on module file.
  • If you don't have any arguments for page then there is no need to include 'page arguments' => array() line into your hook_menu array.
PA robot’s picture

PA robot commented
Status: Needs work » Closed (won't fix)

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.

nithinkolekar’s picture

nithinkolekar commented
Status: Closed (won't fix) » Needs review
joao sausen’s picture

joao sausen commented

Module looks good, but you don't need the drulapp.api.php file since there is nothing there. Also there is this $data['error_code'] = t("101"); on drulapp.pages.inc, you probably dont want that in a t().

lingros’s picture

lingros commented

Automated Review

Without any issues.

Manual Review

Individual user account
[Yes: Follows] the guidelines for individual user accounts.
No duplication
[No: Causes] module duplication and/or fragmentation.
Master Branch
[Yes: Follows] the guidelines for master branch.
Licensing
[Yes: Follows] the licensing requirements.
3rd party assets/code
[Yes: Follows] the guidelines for 3rd party assets/code.
README.txt/README.md
[No: Does not follow] the guidelines for in-project documentation and/or the README Template. You probably forgot to fill it, your README.txt contains only “TODO” note.
Code long/complex enough for review
[Yes: Follows] the guidelines for project length and complexity.
Secure code
[Yes: Meets the security requirements.]
Coding style & Drupal API usage
  1. You probably don’t need function drulapp_userid_is_blocked() in drulapp.admin.inc, it’s do nothing and has no return value.

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

This review uses the Project Application Review Template.

lucif3rum’s picture

lucif3rum commented

I have manually reviewed this project and see a few duplicate functions and also the readme file could be done better.

nithinkolekar’s picture

nithinkolekar commented

No duplication
[No: Causes] module duplication and/or fragmentation.

I have manually reviewed this project and see a few duplicate functions ...

reviewer doesn't know anything about what that link is for . There are several blindfolded reviews in this project applications section by developers who desperately need to get their own project approved :(.

DA should add another status "Coding standard passed & Manual code review pending" (MCRP). So that only experienced user with security code review can further test the source code for any security vulnerability.

ajaygupta1139’s picture

ajaygupta1139 commented

Manual review :

  1. Please remove the unwanted function drulapp_userid_is_blocked from drulapp.pages.inc
    1. Automated review is fine - https://pareview.sh/node/1295
    2. Please sanitize all $_POST values, ref - https://api.drupal.org/api/drupal/includes%21common.inc/group/sanitization/7.x
    3. Please use form attached js or drupal_add_js to add in JS with form in place of form suffix or prefix in drulapp.module on line 382
    4. Please remove unwanted function drulapp_settings_validate() from drulapp.admin.inc
    5. Unwanted file can also be deleted from repo - drulapp.api.php
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work

I am changing status basing on the last comment. To the reviewers: Please change the status value, when you are reporting something that needs to be corrected.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs work » Closed (won't fix)

If you are still working on this application, you should fix all known problems and set the status to Needs review. (See also the project application workflow.)
Please don't change status of this application if you aren't sure you have time to dedicate to this application, or it will be closed again as won't fix.

I am closing this application due to lack of activity.