New user not pulled from drupal on Moodle login page

MrDaleSmith commented 10 years ago

Very handy little plugin, thank you.

One error: starting at line 125 of auth.php, the plugin tries to pull a user that can't be found in the Moodle database from the Drupal services REST server. It then throws a DB write error.

This is because the apiObj doesn't log in as the $remote_user, and so gets a FALSE return.

The code should also handle the possibility of getting a FALSE return more gracefully, rather than assuming it will always receive a valid return.

netw3rker commented 10 years ago

I'm not sure I'm following your logic here. Could you put together a pull request with the recommended changes so we can review and discuss them?

MrDaleSmith commented 10 years ago

My fix was a bit hacky: duplicated a lot of code you probably would need to. The issue is where the the apiAoj is created on 102 it is declared as

$apiObj = new RemoteAPI($base_url, $endpoint, 1, $session_name, $session_id);

You then connect with:

$ret = $apiObj->Connect();

on 104.

But when you then try to pull the individual user details from the API with

$newuser = $apiObj->Index('muser?uid='.$uid);

you haven't logged in as the remote user, and the Drupal API won't give you any details. In the sync_users function, you log in as the remote user with

$ret = $apiObj->Login($remote_user, $remote_pw, true);

before you attempt to pull user details from the API.

netw3rker commented 10 years ago

I see where your confusion is. In the loginpage_hook() method, there is no remote username/password authentication (and there shouldn't be). Inside of this hook, the session of the already drupal-authenticated user is used to make a backend request to drupal from moodle. This means that the following criteria can be guaranteed: 1) there is a drupal user entry that can be imported 2) the user is logged in and session is valid

Things are a bit different Inside the sync script. There is no already authenticated user session, so a login is required in order to access the necessary resources.

You should make sure that the view is able to return a specific user, and is accessible to all users, not just the service-account. If you want, you can always create 2 view displays, one that only returns the details of the logged in user, and one that returns the details of any user (for just the service user). set those displays with the appropriate permissions, and views will do the rest.

MrDaleSmith commented 10 years ago

Ah, fair enough: I prefer having the API locked down to solely those who actually need it, so I'll keep logging in as remote, I think.

netw3rker commented 10 years ago

the problem is, you can't know who is logged in unless you actually connect as them using their session. It doesn't make a lot of sense to connect, get some data, connect again, log in, get some more data when really just one or two connections is all ya need.

here's an example view that I'd recommend you use to keep things locked down the way you want*, and still work (I should probably create this in a gist, but give it a shot & don't forget to look at the access permissions per display.. thats the important part):

there is a context filter to filter by logged in user. this is because I couldn't find the normal filter for authenticated user. find and use that to maximize the data security.

$view = new view();
$view->name = 'moodle_user_listings';
$view->description = '';
$view->tag = 'default';
$view->base_table = 'users';
$view->human_name = 'Moodle User Listings';
$view->core = 7;
$view->api_version = '3.0';
$view->disabled = FALSE; /* Edit this to true to make a default view disabled initially */

/* Display: Master */
$handler = $view->new_display('default', 'Master', 'default');
$handler->display->display_options['title'] = 'Moodle User Listings';
$handler->display->display_options['use_more_always'] = FALSE;
$handler->display->display_options['access']['type'] = 'role';
$handler->display->display_options['access']['role'] = array(
  3 => '3',
  4 => '4',
);
$handler->display->display_options['cache']['type'] = 'none';
$handler->display->display_options['query']['type'] = 'views_query';
$handler->display->display_options['exposed_form']['type'] = 'basic';
$handler->display->display_options['pager']['type'] = 'full';
$handler->display->display_options['pager']['options']['items_per_page'] = '10';
$handler->display->display_options['style_plugin'] = 'default';
$handler->display->display_options['row_plugin'] = 'fields';
/* Field: User: Name */
$handler->display->display_options['fields']['name']['id'] = 'name';
$handler->display->display_options['fields']['name']['table'] = 'users';
$handler->display->display_options['fields']['name']['field'] = 'name';
$handler->display->display_options['fields']['name']['label'] = 'name';
$handler->display->display_options['fields']['name']['alter']['word_boundary'] = FALSE;
$handler->display->display_options['fields']['name']['alter']['ellipsis'] = FALSE;
/* Field: User: Active */
$handler->display->display_options['fields']['status']['id'] = 'status';
$handler->display->display_options['fields']['status']['table'] = 'users';
$handler->display->display_options['fields']['status']['field'] = 'status';
$handler->display->display_options['fields']['status']['label'] = 'status';
$handler->display->display_options['fields']['status']['type'] = 'boolean';
$handler->display->display_options['fields']['status']['not'] = 0;
/* Field: User: E-mail */
$handler->display->display_options['fields']['mail']['id'] = 'mail';
$handler->display->display_options['fields']['mail']['table'] = 'users';
$handler->display->display_options['fields']['mail']['field'] = 'mail';
$handler->display->display_options['fields']['mail']['label'] = 'email';
/* Field: User: First Name */
$handler->display->display_options['fields']['field_first_name']['id'] = 'field_first_name';
$handler->display->display_options['fields']['field_first_name']['table'] = 'field_data_field_first_name';
$handler->display->display_options['fields']['field_first_name']['field'] = 'field_first_name';
$handler->display->display_options['fields']['field_first_name']['label'] = 'firstname';
/* Field: User: Last Name */
$handler->display->display_options['fields']['field_last_name']['id'] = 'field_last_name';
$handler->display->display_options['fields']['field_last_name']['table'] = 'field_data_field_last_name';
$handler->display->display_options['fields']['field_last_name']['field'] = 'field_last_name';
$handler->display->display_options['fields']['field_last_name']['label'] = 'lastname';
/* Field: User: Uid */
$handler->display->display_options['fields']['uid']['id'] = 'uid';
$handler->display->display_options['fields']['uid']['table'] = 'users';
$handler->display->display_options['fields']['uid']['field'] = 'uid';
$handler->display->display_options['fields']['uid']['label'] = 'uid';
/* Field: User: City */
$handler->display->display_options['fields']['field_city']['id'] = 'field_city';
$handler->display->display_options['fields']['field_city']['table'] = 'field_data_field_city';
$handler->display->display_options['fields']['field_city']['field'] = 'field_city';
$handler->display->display_options['fields']['field_city']['label'] = 'city';
/* Field: User: Country */
$handler->display->display_options['fields']['field_country']['id'] = 'field_country';
$handler->display->display_options['fields']['field_country']['table'] = 'field_data_field_country';
$handler->display->display_options['fields']['field_country']['field'] = 'field_country';
$handler->display->display_options['fields']['field_country']['label'] = 'country';
/* Field: User revision: Vid */
$handler->display->display_options['fields']['vid']['id'] = 'vid';
$handler->display->display_options['fields']['vid']['table'] = 'user_revision';
$handler->display->display_options['fields']['vid']['field'] = 'vid';
$handler->display->display_options['fields']['vid']['label'] = 'change_id';
/* Sort criterion: User revision: Vid */
$handler->display->display_options['sorts']['vid']['id'] = 'vid';
$handler->display->display_options['sorts']['vid']['table'] = 'user_revision';
$handler->display->display_options['sorts']['vid']['field'] = 'vid';
$handler->display->display_options['sorts']['vid']['order'] = 'DESC';
/* Filter criterion: User: Active */
$handler->display->display_options['filters']['status']['id'] = 'status';
$handler->display->display_options['filters']['status']['table'] = 'users';
$handler->display->display_options['filters']['status']['field'] = 'status';
$handler->display->display_options['filters']['status']['value'] = '1';
$handler->display->display_options['filters']['status']['group'] = 1;
$handler->display->display_options['filters']['status']['expose']['operator'] = FALSE;
/* Filter criterion: User: The user ID */
$handler->display->display_options['filters']['uid_raw']['id'] = 'uid_raw';
$handler->display->display_options['filters']['uid_raw']['table'] = 'users';
$handler->display->display_options['filters']['uid_raw']['field'] = 'uid_raw';
$handler->display->display_options['filters']['uid_raw']['expose']['operator_id'] = 'uid_raw_op';
$handler->display->display_options['filters']['uid_raw']['expose']['label'] = 'The user ID';
$handler->display->display_options['filters']['uid_raw']['expose']['operator'] = 'uid_raw_op';
$handler->display->display_options['filters']['uid_raw']['expose']['identifier'] = 'uid';
$handler->display->display_options['filters']['uid_raw']['expose']['remember_roles'] = array(
  2 => '2',
  1 => 0,
  3 => 0,
  4 => 0,
  5 => 0,
  6 => 0,
);
/* Filter criterion: User revision: Vid */
$handler->display->display_options['filters']['vid']['id'] = 'vid';
$handler->display->display_options['filters']['vid']['table'] = 'user_revision';
$handler->display->display_options['filters']['vid']['field'] = 'vid';
$handler->display->display_options['filters']['vid']['operator'] = '>';
$handler->display->display_options['filters']['vid']['exposed'] = TRUE;
$handler->display->display_options['filters']['vid']['expose']['operator_id'] = 'vid_op';
$handler->display->display_options['filters']['vid']['expose']['label'] = 'Change ID';
$handler->display->display_options['filters']['vid']['expose']['operator'] = 'vid_op';
$handler->display->display_options['filters']['vid']['expose']['identifier'] = 'change_id';
$handler->display->display_options['filters']['vid']['expose']['remember_roles'] = array(
  2 => '2',
  1 => 0,
  3 => 0,
  4 => 0,
  5 => 0,
  6 => 0,
);

/* Display: Page */
$handler = $view->new_display('page', 'Page', 'page');
$handler->display->display_options['path'] = 'moodle-user-listings';

/* Display: Services - Moodle Service */
$handler = $view->new_display('services', 'Services - Moodle Service', 'services_1');
$handler->display->display_options['display_description'] = 'Shows all users. Should be accessed only by the moodle service user';
$handler->display->display_options['defaults']['access'] = FALSE;
$handler->display->display_options['access']['type'] = 'role';
$handler->display->display_options['access']['role'] = array(
  3 => '3',
  4 => '4',
);
$handler->display->display_options['path'] = 'muser';

/* Display: Services - user specific */
$handler = $view->new_display('services', 'Services - user specific', 'services_2');
$handler->display->display_options['display_description'] = 'Returns the results of just one user. This should be accessible to any user';
$handler->display->display_options['defaults']['access'] = FALSE;
$handler->display->display_options['access']['type'] = 'role';
$handler->display->display_options['access']['role'] = array(
  2 => '2',
);
$handler->display->display_options['defaults']['arguments'] = FALSE;
/* Contextual filter: User: Uid */
$handler->display->display_options['arguments']['uid']['id'] = 'uid';
$handler->display->display_options['arguments']['uid']['table'] = 'users';
$handler->display->display_options['arguments']['uid']['field'] = 'uid';
$handler->display->display_options['arguments']['uid']['default_action'] = 'default';
$handler->display->display_options['arguments']['uid']['default_argument_type'] = 'current_user';
$handler->display->display_options['arguments']['uid']['summary']['number_of_records'] = '0';
$handler->display->display_options['arguments']['uid']['summary']['format'] = 'default_summary';
$handler->display->display_options['arguments']['uid']['summary_options']['items_per_page'] = '25';
$handler->display->display_options['defaults']['filter_groups'] = FALSE;
$handler->display->display_options['defaults']['filters'] = FALSE;
/* Filter criterion: User: Active */
$handler->display->display_options['filters']['status']['id'] = 'status';
$handler->display->display_options['filters']['status']['table'] = 'users';
$handler->display->display_options['filters']['status']['field'] = 'status';
$handler->display->display_options['filters']['status']['value'] = '1';
$handler->display->display_options['filters']['status']['group'] = 1;
$handler->display->display_options['filters']['status']['expose']['operator'] = FALSE;
/* Filter criterion: User: The user ID */
$handler->display->display_options['filters']['uid_raw']['id'] = 'uid_raw';
$handler->display->display_options['filters']['uid_raw']['table'] = 'users';
$handler->display->display_options['filters']['uid_raw']['field'] = 'uid_raw';
$handler->display->display_options['filters']['uid_raw']['expose']['operator_id'] = 'uid_raw_op';
$handler->display->display_options['filters']['uid_raw']['expose']['label'] = 'The user ID';
$handler->display->display_options['filters']['uid_raw']['expose']['operator'] = 'uid_raw_op';
$handler->display->display_options['filters']['uid_raw']['expose']['identifier'] = 'uid';
$handler->display->display_options['filters']['uid_raw']['expose']['remember_roles'] = array(
  2 => '2',
  1 => 0,
  3 => 0,
  4 => 0,
  5 => 0,
  6 => 0,
);
/* Filter criterion: User revision: Vid */
$handler->display->display_options['filters']['vid']['id'] = 'vid';
$handler->display->display_options['filters']['vid']['table'] = 'user_revision';
$handler->display->display_options['filters']['vid']['field'] = 'vid';
$handler->display->display_options['filters']['vid']['operator'] = '>';
$handler->display->display_options['filters']['vid']['exposed'] = TRUE;
$handler->display->display_options['filters']['vid']['expose']['operator_id'] = 'vid_op';
$handler->display->display_options['filters']['vid']['expose']['label'] = 'Change ID';
$handler->display->display_options['filters']['vid']['expose']['operator'] = 'vid_op';
$handler->display->display_options['filters']['vid']['expose']['identifier'] = 'change_id';
$handler->display->display_options['filters']['vid']['expose']['remember_roles'] = array(
  2 => '2',
  1 => 0,
  3 => 0,
  4 => 0,
  5 => 0,
  6 => 0,
);
$handler->display->display_options['path'] = 'muser';

MrDaleSmith commented 10 years ago

Cheers: I'll give that a go.

netw3rker commented 10 years ago

n/p. Since things are working as they should, I'm going to close this issue. If you want to contribute the alternate method as a feature/setting (something like a setting that gives admins the option of relying on the session exclusively, or using it and then logging in as the service user for more data) I'll accept it if you submit it in a pull request.

cannod / moodle-drupalservices

New user not pulled from drupal on Moodle login page #29