Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

social-login-redirect: Redirection added for social auth users #306

Merged
merged 3 commits into from
Nov 27, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions web/modules/custom/ct_user/ct_user.module
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

use Drupal\Core\Routing\RouteMatchInterface;
use Symfony\Component\HttpFoundation\RedirectResponse;

/**
* Implements hook_help().
Expand All @@ -22,3 +23,47 @@ function ct_user_help($route_name, RouteMatchInterface $route_match) {
default:
}
}

/**
* Implements hook_form_alter().
*/
function ct_user_form_alter(&$form, $form_state, $form_id) {
// Check if the form is the user login form.
if ($form_id == 'user_login_form') {
// Add a custom validation function to the form.
$form['#validate'][] = 'ct_user_user_login_form_validate';
}
}

/**
* Custom validation function for the user login form.
*/
function ct_user_user_login_form_validate(&$form, $form_state) {
// Get the name value from the form.
$name = $form_state->getValue('name');

// Get the id of the user.
$ids = \Drupal::entityQuery('user')
->condition('name', $name)
->range(0, 1)
->accessCheck()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
->accessCheck()
->accessCheck(FALSE)

I think access check should be disabled in this case as well. I know anonymous users can read user info but this will be useful in case that ever changes.

->execute();

if ($ids) {
// Check if there is a social auth profile.
$socialIds = \Drupal::entityQuery("social_auth")
->accessCheck(FALSE)
->condition("user_id", reset($ids))
->execute();
}

// If the user has no social auth profile, allow the login to continue.
if (empty($socialIds)) {
return;
}

// If the user has social auth profile, redirect them to /user/login/google.
$response = new RedirectResponse('/user/login/google');
$response->send();
user_logout();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider the flow:

If no user with that email exists, $ids will be empty. In that case, the first if block never gets executed and $socialIds is never set. This means that the second block won't be entered. Finally, in this case, the user will get redirected to Google login. This might be okay in registration case but it is not very likely that someone will come this way to register. It will be clearer to the user if they are actually sent back. In our case, we just have to return from validation because the login system in Drupal will throw the error.

The root of this problem here is that the second if block is outside of the first if block and there is a chance it never gets set. You won't see errors because you are checking for empty but the problem still remains (as I have described above).

To fix this, you can make it very simple by considering this pseudo-code.

  1. If the user doesn't exists ($ids is empty), then immediately return.
  2. Now we know that the user definitely exists. Check if there is a social auth profile. If not, the immediately exit.
  3. Now we know that the user exists AND has a social auth profile. Redirect to Google auth.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I have updated the code. Please verify

Loading