This module provides Single Sign On for Enterprises, Social Login and User/Passwords across Drupal instances. It relies on Auth0 which is a cloud service (https://auth0.com).

https://github.com/auth0/auth0-drupal

git clone --branch 7.x-1.x http://git.drupal.org/sandbox/auth0/2298303.git

Here is the link to the sandbox
https://www.drupal.org/sandbox/auth0/2298303

Comments

gisle’s picture

You should also link to your sandbox project page. And I think you should delete the link to github, it is not relevant here.

Your git clone command contains your username and is for you only. It will not allow others to clone your project.

PA robot’s picture

Status: Needs review » Needs work

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

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.

PA robot’s picture

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.

auth0’s picture

Issue summary: View changes
Status: Closed (won't fix) » Needs review
SolidOpinion’s picture

Status: Needs review » Needs work

1. Remove "version" from the ./auth0.info file, it will be added by drupal.org packaging automatically.

2 ./auth0.module: all functions should be prefixed with your module/theme name to avoid name clashes. See https://www.drupal.org/node/318#naming

for example function auth0_verify_email_page() { instead function verify_email_page()

3. Unused variable $state in auth0.module:102. Remove it, please.

4. DrupalSecure has found some issues with your code (please check the Writing secure core handbook).
FILE: /var/www/drupal-7-pareview/pareview_temp/block--user--login.tpl.php
Printing unsanitized input from variable_get

5. Codespell has found some spelling errors in your code.
./README.txt:58: doesnt ==> doesn't
./auth0.module:26: controler ==> controller
./auth0.module:99: comming ==> coming
./auth0.module:158: asign ==> assign
./auth0.module:159: asign ==> assign

Plase check your code with automated report tool
http://pareview.sh/pareview/httpgitdrupalorgsandboxauth02298303git

auth0’s picture

Thanks SolidOpinion! We will take a look at this.

PA robot’s picture

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.

ronan’s picture

Status: Closed (won't fix) » Needs review

We've cleaned up the code standards and and addressed the issues listed above. Can we have another review on this?

Thanks!

Arun Murugadoss - Drupal Geeks’s picture

Hi,

. The auth0.module file contains the below 'require_once()' statement in line no: 19.
require_once (DRUPAL_ROOT . '/' . AUTH0_PATH . '/vendor/autoload.php');
But the specified resource 'vendor/autoload.php' was not included. So it gives the Fatal error on the module installation. Please fix the issue.
Fatal error: require_once(): Failed opening required '/sites/all/modules/authsinglesignon/vendor/autoload.php'

. 'Package' detail is missing from the auth0.info file.

. Please remove the unused single_sign_on__auth0_.info file.

. Please remove any one of the README file. There are duplicates. It contains both .md and .txt files.

. What is the need of the below 'use' statement in auth0.module Line no: 21?
use Auth0SDK\Auth0;

. The module seems to be providing 3 configuration links such as

admin/config/people/auth0
admin/config/people/auth0/basic
admin/config/people/auth0/advanced

So these configuration links can be added to the auth0.info file as well.

. The hook function auth0_login_form($form, &$form_state) { in the auth0.module file is misleading and it is missing the documentation.

/**
 * Implements hook_form().
 */

hook_form() should follow the module name. So it is supposed to be function auth0_form($form, &$form_state) {

Devaraj johnson’s picture

Automated Review
Go through the issue mentioned in the below url http://pareview.sh/pareview/httpgitdrupalorgsandboxauth02298303git
Manual Review
Individual user account
Yes: Follows
No duplication
Yes: Does not
Master Branch
Yes: Follows the guidelines for master branch.
README.txt/README.md
Yes: Follows

Secure code

Coding style & Drupal API usage
1. Just a recommendation
It good if you implement hook_install()1) It is recommended to always implement hook_install(). Here you can find an example.
Also implement hook_help() its good to implement for a professional developer.

ronan’s picture

We have implemented as many of these suggestions as made sense:

The auth0.module file contains the below 'require_once()' statement

We added a soft dependency on the Composer Manager module and added an INSTALL.txt for instructions on how to download the module's dependencies.

. 'Package' detail is missing from the auth0.info file.

Per the guidelines this should be left blank for this module:

https://www.drupal.org/node/542202#package

Please remove any one of the README file. There are duplicates. It contains both .md and .txt files.

Done.

What is the need of the below 'use' statement in auth0.module Line no: 21?

That's a namespace import. I have removed it as part of the new dependency management.

So these configuration links can be added to the auth0.info file as well

I've added the main one.

The hook function auth0_login_form($form, &$form_state) { in the auth0.module file is misleading and it is missing the documentation.

I think you're confusing a hook implementation with a form callback. This is a form callback and not an implementation of hook_form(). However I have renamed this function as it was misleadingly named.

It good if you implement hook_install()1) It is recommended to always implement hook_install()

This module maintains no database tables and has no initial set up so it does not need an install hook. If you can point me to the recommendation you're referencing I'll add an empty install hook.

PA robot’s picture

Issue summary: View changes

Fixed the git clone URL in the issue summary for non-maintainer users.

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

auth0’s picture

Hi, I would like to move forward and publish this module but didnt get any feedback.
What would we need to change to publish it?
I think we target all the changes you asked, we don't know what would left from our side.
Thanks.

mohaly’s picture

Why this module didn't published till now, I need it.

mlevasseur’s picture

Status: Needs review » Needs work

Some super nitpicky stuff:
auth0.module Line 128: Change catch(Exception$e) { to catch (Exception $e) {
auth0.module Line 160: Change catch(Exception$e) {} to catch (Exception $e) {}
auth0.module Line 162: Remove // var_dump($auth0); die;
auth0.module Lines 195, 202: You're checking if $user_info['email'] is set twice (empty() automatically checks for isset()). I would suggest in setting something like $email_set = !empty($user_info['email'); and using that in your if() statements.
auth0.module Lines 202, 229, 237: Same thing for $user_info['email_verified'].
auth0.module Line 259: Remove return FALSE; since it's never reached.
auth0.module Lines 289, 296, 304, 315: Chain your calls on new lines. Ex:

db_update('table')
  ->fields(array())
  ->condition()
  ->execute();

auth0.module Line 338: change if (isset($user_info['email']) && !empty($user_info['email'])) { to if (!empty($user_info['email'])) { (empty checks for isset already).
auth0.module Line 358: change this to if ($new_user) instead. Currently it will always return true because you're checking an object you're creating yourself, instead of the result of trying to create a user from that object. Also, you're always assuming a user is successfully created by naively returning $new_user->uid. What happens if user_save fails?
auth0.module Lines 437, 439: Change 'signup' to 'sign-up' (437) and 'sign up' (439)
auth0.module Line 568: Missing Implements template_preprocess_HOOK() declaration in header comment.

You need to create a auth0.api.php file containing your drupal_alter hooks.

Finally, consider doing 3 reviews of other projects so you can get into prioritised "review bonus" issue queue (https://www.drupal.org/node/1975228)

PA robot’s picture

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.

josephdpurcell’s picture

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

Setting this back to needs work. This module provides a great value for those using Auth0. Anything I can do to help get this module supported?

mlevasseur’s picture

If the author has made the previously mentioned changes, you can give it a code review yourself and give it a status of RTBC if you don't find any problems with it.

josephdpurcell’s picture

After initial review of the Drupal 7 and Drupal 8 versions of this module on GitHub, https://github.com/auth0/auth0-drupal, I think this module needs a thorough review. If I can spend some time on this I will after contacting the maintainer to ensure that changes are welcome. Particularly, I encourage not using an include statement, but rather follow patterns of other modules for handling composer dependencies.

Regarding code style, here is a comparison of several Drupal 8 modules: https://github.com/josephdpurcell/code-climate-and-drupal/blob/gh-pages/.... It gives a hint at how well the Auth0 module is complying with Drupal code style compared to others.

josephdpurcell’s picture

Update: significant improvements have been made on the code style front for the Drupal 8 version: https://codeclimate.com/github/auth0/auth0-drupal. It's now a 4.0 GPA on Code Climate.

Some comments for suggested changes on the Drupal 8 version:

  • Avoid use of \Drupal::service() and instead use the ::create method or services.yml to inject services (more info)
  • Add @param, @return, and @throws documentation
  • Write validation in BasicAdvancedForm::validateForm, else remove that method
margyly’s picture

I'm also interested in this project. We are looking at using Auth0 for a Drupal 7 project.

PA robot’s picture

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.

josephdpurcell’s picture

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

Setting to "needs work" to complete the few items from #20 on the D8 branch of the module.

PA robot’s picture

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.

josephdpurcell’s picture

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

I still think this is a valuable module to get added to drupal.org. Re-opening.

klausi’s picture

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

If we don't hear back from the original applicant we can't continue here. If you would like to maintain and publish this yourself please open a project application under your own account.

josephdpurcell’s picture

Ok. I've opened a ticket on the GitHub repo for an Auth0 representative to write back: https://github.com/auth0/auth0-drupal/issues/65.

josephdpurcell’s picture

I assume this can be closed? I see now there is a project page: https://www.drupal.org/project/auth0

mostekcm’s picture

Assigned: Unassigned » mostekcm
Status: Closed (won't fix) » Needs review

Hello, I'm taking over maintenance of this drupal plugin for Auth0. Can we open this up for review again? I am an employee of Auth0. I'm just working on getting my profile to reflect that.

Thanks!

mostekcm’s picture

Status: Needs review » Needs work

I'm setting this to needs work until I can review all issues at http://git.drupal.org/sandbox/auth0/2298303.git

PA robot’s picture

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.

apaderno’s picture

Assigned: mostekcm » Unassigned
apaderno’s picture

If you are still interested on being able to opt into security coverage for projects you create, please open a new application using a project for which the only commits (for the time required to set the application's status to Fixed) are from you.
Please don't open a new application if you aren't sure to have time to dedicate to the application, or it will be closed again as won't fix.

I am closing this application due to lack of activity.