Social Login

Hi,
sorry you have to create a sandbox project and commit your module by git to the project's repository.
(go to http://drupal.org/project/user to do so)
You'll find more documentation about this here: http://drupal.org/node/1011196
Information about the review process: http://drupal.org/node/1068952

After you did that you should post the project page's link here and switch the status back.

regards and a happy new year!

You need to set the status to "needs review" if you want to get a review.

Currently we are approaching 5 weeks. You can speed up the process by reviewing other projects.

You are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.

README.txt is missing, see the guidelines for in-project documentation.

You only committed a "social_login_for_drupal.info" file and an empty subdirectory.
Please read the documentation (version control tab on your project page) on how to use git.

Wrong branch name, "7.x-1.0" should be "7.x-1.x".

Get a review bonus and we will come back to your application sooner.

HI,

Can you please let me know drupal 6 plugin etc has been installed and buttons etc show in Logintoboggan form etc
Just after logging in using any of the providers it goes to homepage not to the current page.
What part of code do I change.

Regards
Rajiv

Hi,

I did visit the page and installed as per this but only issue is the redirect to current page
It always goes back to homepage and not current page after logging in.

Need it to go to current page from where login happens just dont know what part of .module file to edit.

hii,
i did use it , it's work fine for me. no redirection issue. you can use this download from site
http://www.loginradius.com/loginradius/plugins.aspx
i have to use it.

Fixed means that git permission was given to him as you're not a git admin you can't do that.
If you think this module is ready; switch to RTBC so a git admin will take a hopefully last check and if he things its good he'll promote it.

It works fine just say if i login from page site.com/xxx it does not redirect to site.com/xxx after login but goes to site.com

Any advice ?

it's working

Unfortunately there are so many errors that I can't attach the full report. Please use the Coder module and review your code with minor settings. Also see Creating Drupal 7.x modules for more information on module development.

• Empty the master branch, see Moving from a master to a major version branch
• Remove unnecessary files and folders like "1387552" and "social_login".
• Remove "version" from the info file, it will be added by drupal.org packaging automatically.
• Remove "datestamp" from the info file, it will be added by drupal.org packaging automatically.
• The "?>" PHP delimiter at the end of files is discouraged, see http://drupal.org/node/318#phptags
  

  ./LoginRadiusSDK.php
  

• ./LoginRadius.admin.inc: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
  

  function loginradius_admin_settings($form, &$form_state) {
  function loginradius_admin_settings_validate($form, &$form_state) {
  function loginradius_admin_settings_submit($form, &$form_state) {
  function loginradius_admin_reset_confirm($form, &$form_state) {
  function loginradius_admin_reset_confirm_submit($form, &$form_state) {
  

• ./LoginRadius.module: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
  

  function return_url($returnUrl = array()) {
  function popup($FullName, $ProfileName, $fname, $lname, $id, $Provider, $msg) {?>
  

• ./LoginRadius.pages.inc: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
  

  function loginradius_return_url($account) {
  function loginradius_user_identities($account) {
  function loginradius_user_delete_form($form, $form_state, $account, $aid = 0) {
  function loginradius_user_delete_form_submit($form, &$form_state) {
  

• Bad line endings were found, always use unix style terminators. See http://drupal.org/coding-standards#indenting
  

  ./LoginRadius.css;              ASCII text, with CRLF line terminators
  ./LoginRadius.info;             ASCII text, with CRLF line terminators
  ./README.txt;                   ASCII English text, with CRLF line terminators
  ./social_login_for_drupal.info; ASCII text, with CRLF line terminators
  
  

• Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards). See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Get a review bonus and we will come back to your application sooner.

See the full report attached.

Hi,

I checked the new revised plugin after loggin in the site still goes back to homepage only with /?destination=node%....
attached to url .
It does not go back to the original page logged in from.
Please have a look.

Rajiv

Ok just rechecked it works only if Devel module redirect option is switched on

That was a mistake devel setting no longer works

i have to cheked this plugin but there is a redirection issue that was my mistake in check. it redirect same page from comment section but not on all page .
but after installing update plugin recently. now it's work fine.

This still needs work, since the mentioned issues are not fixed yet.

I got redirection to work changed

$query = drupal_get_destination();
drupal_goto('',$query);

to

$query = $_GET['q'];
drupal_goto($query);

drupal_redirect_form($form, $redirect = NULL);
working fine with your module.pls try this

Hi,

Can you post a version 6 also as I am using that.
One more thing if used in conjunction with logintoboggan how would i place
the social login buttons in top of box above regular login

Rajiv

Thats ok got it changed weight

One more thing buttons show in Chrome not in Firefix please do check if its problem on your side also

$query = drupal_get_destination();
drupal_goto('',$query);

No longer works for 6
Now again goes back to homepage only please debug and upload changed / revised 6 plugin.

Ok Sorry my mistake reverted to Getq

Now will debug Drupal 6 firefox and let you know

Hey can someone let me know why the icons have suddenly vanished from Firefox ie, chrome, opera are ok.
Any advice would be good .
Rest all the module is ok.

reviewed again both versions working fine.

please see http://drupal.org/node/532400

Ok it works fine ...just replace code on goto to getq

tested both works fine....

Where do you put this drupal_redirect_form($form, $redirect = NULL); ?

Sorry, need to set this back to 'needs work', I have some issues, some regarding security and some about none Drupal standards way.

Filter variables (security risk)

You have some $_GET-variables that you do not filter, you should sanitize values incoming values. See: http://api.drupal.org/api/drupal/includes!common.inc/group/sanitization/7

Hard coded css-link

In the function sociallogin_popup you hard-code the path to the css to '/sites/all/modules/sociallogin/sociallogin.css'. You should not do that, a module can be installed in many different places, and you should the modules path to get the right directory, like drupal_get_path('module', $module_name). Also you should separate your modules functions from theming, see http://api.drupal.org/api/drupal/includes!theme.inc/function/theme/7

Lowercase in paths

Drupal uses lowercase for links, so you should not use links like admin/config/people/SocialLogin/settings, it should be admin/config/people/sociallogin/settings

Forms (could have security issues)

It seems that you are using the Drupal forms in a non standard way, why?

Changing Drupal default behaviour (security risk)

With the function sociallogin_menu_site_status_alter you change the way Drupal is behaving when it
is in off-line mode, why? If really needed, I think that

Curl is needed

I see that in your file LoginRadiusSDK.php that you use curl, it should say somewhere that you need to have curl installed to use the module. And why is there functions in LoginRadiusSDK.php instead of in the module code?

I have more issues, but I think the most important ones is listed above. Also, it seems that Your user account @LoginRadius is a company account, is that correct?

don't fix your own issue
have a look at the application workflow
http://drupal.org/node/532400

reviewed again,security parameters fixed and it looks fine.

$lookup = variable_get('sociallogin_lookup_results');
  variable_del('sociallogin_lookup_results');
  $api_secret = variable_get('sociallogin_lookupsecret_results');
  variable_del('sociallogin_lookupsecret_results');
  variable_set('sociallogin_apikey', $lookup['apiKey']);
  variable_set('sociallogin_apisecret', $lookup['apiSecret']);
  form_state_values_clean($form_state);
  foreach ($form_state['values'] as $key => $value) {
    if (is_array($value) && isset($form_state['values']['array_filter'])) {
      $value = array_keys(array_filter($value));
    }
    variable_set($key, $value);
  }
  drupal_set_message(t('Configuration options sucessfully saved.'));
  // Clear the cached pages and blocks.
  cache_clear_all();
  menu_rebuild();

And there is more, this is not ready yet.

I raised a couple of questions, could you please answer them.

I have reviewed the module files, all issues posted by MiSc are fixed in latest commit.
I didn’t find any more security issues in code and its functioning properly.
So this module is in good shape and ready to move now.

Thanks
Sudhanshu

Sorry, this is not ready for RTBC.

Manual review:

social_login.module

At the function sociallogin_form_user_login_alter You comment says Implements hook_login_form_alter(). There are no hook_login_form_alter.

Also, there is no hook_user_login_form_alter, or popup. You have to comment the code in a way that makes the functions understandable. If your module implements a hook it means that it is using a hook defined in a another module.

_sociallogin_user_login_form_alter is an internal function for the module, but still your comment says it implements the non existing hook_user_login_form_alter

sociallogin_user_register_submit, here you are getting a variable, $secret, and ad a empty string as value, why? You are doing this in a couple of places.

In the same function you use module_invoke_all('sociallogin', $id, $account). Where is that hook defined?

_sociallogin_user_login_form_alter, here you seem to remove stuff that belongs to the openid module, why? On the project page for the Openid module it says there are no Drupal 7 release.

And there a lots more. I am missing comments in the code, you do a lot of stuff, but almost never documented.

Manual review:

Function: sociallogin_popup

Here you have a css link which is out commented - why? (also, the link to the css is wrong, it is hardcoded)

Also you have a form which is hard coded and not uses form api, why?

Curl is needed

You do not check somewhere that the user have curl installed (I think that you should do a check for curl in .install), and you do not tell on the project page that curl is needed.

Name

In you application you call the project LoginRadius - Redirect, in the sandbox Social Login. Your module is a module for loginradius - should not the module name reflect this?

Function _sociallogin_user_login_form_alter

You name it after a hook, but the function is internal for the module, this could be confusing I think.

Why do you not use internal Drupal functions to construct the urls in the $loc vaiable (like $base_url)? And why do you do a URL encode on the variable, I do not think you have to do that here.
http://api.drupal.org/api/drupal/developer!globals.php/global/base_url/7

You do not seem to filter the $loc-variable, but you use it to construct a url, is it checked for security on https://hub.loginradius.com?
http://api.drupal.org/api/drupal/includes!common.inc/group/sanitization/7

reviewed again all the above issues posted are fixed. not use form api but code is following drupal standards. looks and working great

Sorry for the delay, but you have not listed any reviews of other project applications in your issue summary as strongly recommended in the application documentation.

Review of the 7.x-1.x branch:

• Drupal Code Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Get a review bonus and we will come back to your application sooner.

manual review:

1. sociallogin_help(): all user facing text in the item list should run through t() for translation. Use l() to generate link markup. Use theme('item_list', ...) to create list markup.
2. sociallogin_form_user_login_block_alter(): this is a hook implementation and should be documented as such, see http://drupal.org/node/1354#hookimpl Also for sociallogin_form_user_login_alter() and sociallogin_permission() and elsewhere.
3. t("<p style='color:red;'>This email already registered or invalid. Please choose another one.<p>"): HTML tags should be outside of t() where possible.
4. sociallogin_return_url(): doc block is useless. What URL?
5. "public function construct($ApiSecrete) {": shouldn't that be __construct()? http://php.net/manual/en/language.oop5.decon.php
6. sociallogin_popup(): why can't you use the form API here?
7. "$msg = "<p>Please enter your email address to proceed.</p>";" all user facing text must run through t() for translation.
8. "user_access('manage own identities')": where is that permission defined?
9. _sociallogin_own_identities_access(): where is that function used?
10. "define('SOCIALLOGIN_LABEL_STRING', 'Please Login With');": maybe that should be configurable with the admin settings instead of being a constant?
11. sociallogin_user_login_form_alter(): $options is always the empty string ''?

I am add change due to fapi requirements, so mark it as reviewed and tested by community...

My drupal projects files are under "bvdrupaldev.ir/usecod/" directory.

I am using loginradius module, but it is not working.
500 error is coming after login, instead if sitepage.
Login page:
bvdrupaldev.ir/usecod/?q=user

Note: "bvdrupaldev.ir" domain name is restricted it is accesseble outside of the ogranization.

Please tell me the configuration of loginradius under subdirectory.

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 :-)

added sandbox link

Added the review bonus tag because of reviews done of the applicant.

manual review:
You should implemtent hook_uninstall() to remove all variables that your module defines.

But that is just a minor issue, so ...

Thanks for your contribution, loginradius! Welcome to the community of project contributors on drupal.org.

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

As you continue to work on your module, keep in mind: Commit messages - providing history and credit and Release naming conventions.

Thanks to the dedicated reviewer(s) as well.

My issue is that once the user gets login(through the drupal login module and not using the loginradius login) using their fb or twitter mailid the users role is changing to admin and gets redirected to the admin page!!!But wen the user logs in using the login radius fbconnect the user is directly goung to his user page!!!
Pls help me!!
Thanks in advance!!!

this issue is closed, please create a bug/support issue in the module's issue queue