Drupal 7

Drupal is an excellent option for a mobile application server using the Services Module. One problem however is creating a drupal installation that is an application server and allows remote account creation, and authentication but prevents users from creating accounts and logging in through the webpage.

This is particularly important if one is using a mobile application where LOCATION is important. For example, a node that is created reflecting the users location. Allowing login at the webpage using a browser (and not a mobile smartphone) will cause major problems.

This module creates two Services resources that can only be connected to using a smartphone with an appication that provides the correct key and prohibits login from any browser or other application that does not. These Services endpoint actions are 'register' and 'login'.

This forces the mobile application users to create nodes etc ONLY through the mobile application.

Users with the correct role, CAN login to the website through other methods such as desktop browser for administrative purposes.

Similar Modules:
I could find no modules that do anything similar to this and it was recommended I use Rules, and Services Rules endpoints to create roles that change dynamically during login to try and accomplish this. Something that would take a lot more time for users, than utilizing Services endpoints.

Dependencies
Services Module 3.0 for Drupal 7

Sandbox: http://drupal.org/sandbox/tpainton/1857660
Git: http://git.drupal.org/sandbox/tpainton/1857660.git

CommentFileSizeAuthor
#3 android-and-ios.jpg14.75 KBtpainton

Comments

johnnygamba’s picture

johnnygamba commented
Status: Needs review » Needs work

Using the following as a structure for my review http://drupal.org/node/894256 and http://groups.drupal.org/node/184389
Manual review:

README.txt
Please take a moment to make your README.txt follow the guidelines for in-project documentation.
Master Branch
There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732
Review of the 7.x-1.x branch:
Major coding standards / best practice issues
An automated review of your project has found some issues with your code; As coding standards make sure projects are coded in a consistent style we please you to have a look at the report and try to fix them. Anyway, note that issues found are possibly false positives and fixing all issues is not a requirement for getting through the application process.

You can find the results of the automated report at http://ventral.org/pareview/httpgitdrupalorgsandboxtpainton1851382git

services_login_limiter.module
It seems that the function services_login_limiter_access_menu() it is not used at all. (please correct me if i'm wrong, i don't know much about coding for the services module)
services_login_limiter.module
you have an "else" statement that does nothing, it is empty.
services_login_limiter.admin.inc
You can reduce your code using system_settings_form to make admin pages
general

Review your code using coder, the module has many violations to the Drupal coding standards

apparently you have a problem with the way the module is versioned, when i clone the repo another folder inside the module appears with the same name, i think that's way the complaint about the README.txt does not exists.

tpainton’s picture

tpainton commented

Hi, thanks for taking the time to review. I'll address these things. I am using eGit with Eclipse and having some problems with removing the Master branch, I might just start fresh and start a whole new sandbox project and only push the D7 branch.. Will address all the other points and fix and update you when complete. Again, thanks for taking time out to do this.

tpainton’s picture

tpainton commented
StatusFileSize
new android-and-ios.jpg14.75 KB

Okay, I finally got the directory structure fixed. Eclipse was insistent on adding the parent directory so I just deleted the project and started over.

New Sandbox URL is http://drupal.org/sandbox/tpainton/1857660
GIT: tpainton@git.drupal.org:sandbox/tpainton/1857660.git services_login_limiter

I apologize for the lack of coder evaluation. I actually forgot to commit all my changes. The code that was run through coder and ventral.org and passed is now in the original commit. Directory structure is correct and D7 branch without master are in place in repo.

All components are addressed except implementation of system_settings_form which I will get to.. I forgot how, been a while. The function that was not used , services_login_limiter_access_menu() is now implemented as an access call to services endpoint and always returns true much in the same manner as services_access_menu() does in services 3.0.

The empty else statement has been documented to explain why it's empty.. Future development space coming soon... deletion of accounts which registered locally.

johnnygamba’s picture

johnnygamba commented

hi man, i am having troubles cloning the repo, can you try to clone it and tell me if there was no complaint?
thanks.

lolandese’s picture

lolandese commented

http://drupal.org/sandbox/tpainton/1851382 seems to be a dead link now.
http://drupal.org/sandbox/tpainton/1857660 works and has the same name (Services Login Limiter).

Suggest to edit this issue page.

Cloning through git clone http://git.drupal.org/sandbox/tpainton/1857660.git services_login_limiter results in an empty folder:
warning: remote HEAD refers to nonexistent ref, unable to checkout.

Probably working on it? :)

johnnygamba’s picture

johnnygamba commented

https://help.github.com/articles/error-remote-head-refers-to-nonexistent...

tpainton’s picture

tpainton commented

Try again.. it's working for me and I just ran it out through ventral.org and they didn't get an empty folder.. indeed, they gave me lots of whitespace to get rid of.

http://git.drupal.org/sandbox/tpainton/1857660.git

tpainton’s picture

tpainton commented

hmm. well thats odd.. the link I just posted doesn't work.. but it worked fine on http://ventral.org/pareview.. Okay.. trying to figure this out. Bare with me.

tpainton’s picture

tpainton commented

Okay... well, wierd.. It seems to be working now.. I just checked it out into Eclipse, and ventral.org checks it out and runs the code check on it without problems.. Please try again.

tpainton’s picture

tpainton commented
Issue summary: View changes

Updated Description

lolandese’s picture

lolandese commented

Nope! Sorry. Results (using only the Terminal):

$ git clone http://git.drupal.org/sandbox/tpainton/1857660.git services_login_limiter
Cloning into 'services_login_limiter'...
remote: Counting objects: 17, done.
remote: Compressing objects: 100% (17/17), done.
remote: Total 17 (delta 6), reused 0 (delta 0)
Unpacking objects: 100% (17/17), done.
warning: remote HEAD refers to nonexistent ref, unable to checkout.

After which only the .git folder and it's content exist.

:(

johnnygamba’s picture

johnnygamba commented

have you tried to fix it with the link i posted above?

tpainton’s picture

tpainton commented

Ah... Found it. For some reason this error was only affecting you.. I guess since I was the owner.. Okay I think It should be fixed based on the link you provided.

I edited the project and made the default branch, the only branch I committed. 7.x-1.x. I would test it.. but it will work :)

Thanks for being patient, this is my first project submission and I'm used to using Assembla, not drupal's git.. they seem to be different enough to make me look bad.

I am aware of the few code format problems.. they cropped up on the last commit.. It seems that I need to change something in eclipse because it keeps adding whitespace where I don't want it.. I will get to that.

lolandese’s picture

lolandese commented 
$ git clone http://git.drupal.org/sandbox/tpainton/1857660.git services_login_limiter
Cloning into 'services_login_limiter'...
remote: Counting objects: 17, done.
remote: Compressing objects: 100% (17/17), done.
remote: Total 17 (delta 6), reused 0 (delta 0)
Unpacking objects: 100% (17/17), done.

Nice. No errors this time. Every seems to be there (also the includes sub-folder).

After you feel you addressed all other major issues, please put it back to "Needs review" status.

Thanks.

tpainton’s picture

tpainton commented
Status: Needs work » Needs review

Okay, passes ventral.org and coder specs. I also added implementation of hook_uninstall() to clear system variables as well as using system_settings_form to make admin pages as recommended.

tpainton’s picture

tpainton commented
Priority: Normal » Critical

Updating to critical as wait time > 4 weeks.. as per guidelines. Thanks.

heddn’s picture

heddn
Primary language English
Location Nicaragua
commented
Status: Needs review » Needs work

services_login_limiter.info

services_login_limiter.module

  • @file description should start with a single line ending with a period. Then write a simple paragraph in the next lines. What you have documented really should go in a README, not in this Doxygen comment. See: https://drupal.org/node/1354#file
  • Use of the variables table as implemented in this module is strongly discouraged. First, variables should be name spaced with the module name. Second, this module sets way to many variables. Please switch to use a table dedicated to this module. See http://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/varia... for further rational on the second point.
  • I'd review usage of session in relation to Drupal- http://api.drupal.org/api/drupal/includes%21session.inc/7. I don't really think you need to use the $_SESSION super global directly.
  • Comment on line 258 doesn't make sense to me and it probably doesn't because I haven't use services. We're about to register a new user account and #1083242: Fix for argument source handling in REST Server never mentions user accounts. Could you explain what is going on with that so the uninformed also know?
  • line 65/66: These constants get used as variables. They should be namespaced to the module i.e. begin with the module name.
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

heddn’s picture

heddn
Primary language English
Location Nicaragua
commented

Now I'm educated, I take back part of my comment in #16. Ignore my comment about session. See http://api.drupal.org/api/drupal/includes%21session.inc/function/_drupal....

tpainton’s picture

tpainton commented

Thanks for taking the time to start reviewing. I'll make these fixes.

PA robot’s picture

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

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

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

PA robot’s picture

PA robot commented
Issue summary: View changes

updated sandbox uri