Commit 12ff225f authored by Richard Mansfield's avatar Richard Mansfield
Browse files

Remove username & password from register page & change them after registration instead

parent d4976c32
...@@ -406,7 +406,6 @@ function auth_setup () { ...@@ -406,7 +406,6 @@ function auth_setup () {
} }
} }
$USER->renew(); $USER->renew();
auth_check_password_change();
auth_check_required_fields(); auth_check_required_fields();
} }
else if ($sessionlogouttime > 0) { else if ($sessionlogouttime > 0) {
...@@ -689,18 +688,85 @@ function auth_get_available_auth_types($institution=null) { ...@@ -689,18 +688,85 @@ function auth_get_available_auth_types($institution=null) {
} }
/** /**
* Checks that all the required fields are set, and handles setting them if required. * Checks that all the required fields are set, and handles setting them if required.
*
* Checks whether the current user needs to change their password, and handles
* the password changing if it's required.
*/ */
function auth_check_required_fields() { function auth_check_required_fields() {
global $USER; global $USER, $SESSION;
$changepassword = true;
$elements = array();
if (
!$USER->get('passwordchange') // User doesn't need to change their password
|| ($USER->get('parentuser') && $USER->get('loginanyway')) // User is masquerading and wants to log in anyway
|| defined('NOCHECKPASSWORDCHANGE') // The page wants to skip this hassle
) {
$changepassword = false;
}
// Check if the user wants to log in anyway
if ($USER->get('passwordchange') && $USER->get('parentuser') && isset($_GET['loginanyway'])) {
$USER->loginanyway = true;
$changepassword = false;
}
if ($changepassword) {
$authobj = AuthFactory::create($USER->authinstance);
if ($authobj->changepasswordurl) {
redirect($authobj->changepasswordurl);
exit;
}
if (method_exists($authobj, 'change_password')) {
if ($SESSION->get('resetusername')) {
$elements['username'] = array(
'type' => 'text',
'defaultvalue' => $USER->get('username'),
'title' => get_string('changeusername', 'account'),
'description' => get_string('changeusernamedesc', 'account', hsc(get_config('sitename'))),
);
}
$elements['password1'] = array(
'type' => 'password',
'title' => get_string('newpassword') . ':',
'description' => get_string('yournewpassword'),
'rules' => array(
'required' => true
)
);
$elements['password2'] = array(
'type' => 'password',
'title' => get_string('confirmpassword') . ':',
'description' => get_string('yournewpasswordagain'),
'rules' => array(
'required' => true,
),
);
if (defined('JSON')) { $elements['email'] = array(
'type' => 'text',
'title' => get_string('principalemailaddress', 'artefact.internal'),
'ignore' => (trim($USER->get('email')) != '' && !preg_match('/@example\.org$/', $USER->get('email'))),
'rules' => array(
'required' => true,
'email' => true,
),
);
}
}
else if (defined('JSON')) {
// Don't need to check this for json requests // Don't need to check this for json requests
return; return;
} }
safe_require('artefact', 'internal'); safe_require('artefact', 'internal');
require_once('pieforms/pieform.php'); require_once('pieforms/pieform.php');
$elements = array();
$alwaysmandatoryfields = array_keys(ArtefactTypeProfile::get_always_mandatory_fields()); $alwaysmandatoryfields = array_keys(ArtefactTypeProfile::get_always_mandatory_fields());
foreach(ArtefactTypeProfile::get_mandatory_fields() as $field => $type) { foreach(ArtefactTypeProfile::get_mandatory_fields() as $field => $type) {
...@@ -716,7 +782,10 @@ function auth_check_required_fields() { ...@@ -716,7 +782,10 @@ function auth_check_required_fields() {
} }
if ($field == 'email') { if ($field == 'email') {
// Use a text field for their first e-mail address, not the if (isset($elements['email'])) {
continue;
}
// Use a text field for their first e-mail address, not the
// emaillist element // emaillist element
$type = 'text'; $type = 'text';
} }
...@@ -763,118 +832,22 @@ function auth_check_required_fields() { ...@@ -763,118 +832,22 @@ function auth_check_required_fields() {
)); ));
$smarty = smarty(); $smarty = smarty();
if ($USER->get('parentuser')) {
$smarty->assign('loginasoverridepasswordchange',
get_string('loginasoverridepasswordchange', 'admin',
'<a href="' . get_config('wwwroot') . '?loginanyway">', '</a>'));
}
$smarty->assign('changepassword', $changepassword);
$smarty->assign('form', $form); $smarty->assign('form', $form);
$smarty->display('requiredfields.tpl'); $smarty->display('requiredfields.tpl');
exit; exit;
} }
function requiredfields_submit(Pieform $form, $values) { function requiredfields_validate(Pieform $form, $values) {
global $USER, $SESSION;
foreach ($values as $field => $value) {
if (in_array($field, array('submit', 'sesskey'))) {
continue;
}
set_profile_field($USER->get('id'), $field, $value);
}
$SESSION->add_ok_msg(get_string('requiredfieldsset', 'auth'));
redirect();
}
/**
* Checks whether the current user needs to change their password, and handles
* the password changing if it's required.
*
* This only applies for the internal authentication plugin. Other plugins
* will, in theory, have different data stores, making changing the password
* via the internal form difficult.
*/
function auth_check_password_change() {
global $USER; global $USER;
if ( if (!isset($values['password1'])) {
!$USER->get('passwordchange') // User doesn't need to change their password return true;
|| ($USER->get('parentuser') && $USER->get('loginanyway')) // User is masquerading and wants to log in anyway
|| defined('NOCHECKPASSWORDCHANGE') // The page wants to skip this hassle
) {
return;
}
// Check if the user wants to log in anyway
if ($USER->get('passwordchange') && $USER->get('parentuser') && isset($_GET['loginanyway'])) {
$USER->loginanyway = true;
return;
}
$authobj = AuthFactory::create($USER->authinstance);
if ($authobj->changepasswordurl) {
redirect($authobj->changepasswordurl);
exit;
}
// @todo auth preference for a password change screen for all auth methods other than internal
if (method_exists($authobj, 'change_password')) {
require_once('pieforms/pieform.php');
$form = pieform(array(
'name' => 'change_password',
'method' => 'post',
'plugintype' => 'auth',
'pluginname' => 'internal',
'elements' => array(
'password1' => array(
'type' => 'password',
'title' => get_string('newpassword') . ':',
'description' => get_string('yournewpassword'),
'rules' => array(
'required' => true
)
),
'password2' => array(
'type' => 'password',
'title' => get_string('confirmpassword') . ':',
'description' => get_string('yournewpasswordagain'),
'rules' => array(
'required' => true,
),
),
'email' => array(
'type' => 'text',
'title' => get_string('principalemailaddress', 'artefact.internal'),
'ignore' => (trim($USER->get('email')) != '' && !preg_match('/@example\.org$/', $USER->get('email'))),
'rules' => array(
'required' => true,
'email' => true,
),
),
'submit' => array(
'type' => 'submit',
'value' => get_string('changepassword'),
),
)
));
$smarty = smarty();
$smarty->assign('change_password_form', $form);
if ($USER->get('parentuser')) {
$smarty->assign('loginasoverridepasswordchange',
get_string('loginasoverridepasswordchange', 'admin',
'<a href="' . get_config('wwwroot') . '?loginanyway">', '</a>'));
}
$smarty->display('change_password.tpl');
exit;
} }
}
/**
* Validates the form for changing the password for a user.
*
* Change password will only be if a URL for it exists, or a function exists.
*
* @param Pieform $form The form to check
* @param array $values The values to check
*/
function change_password_validate(Pieform $form, $values) {
global $USER;
// Get the authentication type for the user, and // Get the authentication type for the user, and
// use the information to validate the password // use the information to validate the password
...@@ -888,33 +861,61 @@ function change_password_validate(Pieform $form, $values) { ...@@ -888,33 +861,61 @@ function change_password_validate(Pieform $form, $values) {
&& $authobj->authenticate_user_account($USER, $values['password1'])) { && $authobj->authenticate_user_account($USER, $values['password1'])) {
$form->set_error('password1', get_string('passwordnotchanged')); $form->set_error('password1', get_string('passwordnotchanged'));
} }
if ($authobj->authname == 'internal' && isset($values['username']) && $values['username'] != $USER->get('username')) {
if (!AuthInternal::is_username_valid($values['username'])) {
$form->set_error('username', get_string('usernameinvalidform', 'auth.internal'));
}
if (!$form->get_error('username') && record_exists_select('usr', 'LOWER(username) = ?', strtolower($values['username']))) {
$form->set_error('username', get_string('usernamealreadytaken', 'auth.internal'));
}
}
} }
/** function requiredfields_submit(Pieform $form, $values) {
* Changes the password for a user, given that it is valid.
*
* @param array $values The submitted form values
*/
function change_password_submit(Pieform $form, $values) {
global $USER, $SESSION; global $USER, $SESSION;
$authobj = AuthFactory::create($USER->authinstance); if (isset($values['password1'])) {
$authobj = AuthFactory::create($USER->authinstance);
// This method should exist, because if it did not then the change
// password form would not have been shown.
if ($password = $authobj->change_password($USER, $values['password1'])) {
$SESSION->add_ok_msg(get_string('passwordsaved'));
}
else {
// TODO: Exception is the wrong type here!
throw new Exception('Attempt by "' . $USER->get('username') . '@'
. $USER->get('institution') . 'to change their password failed');
}
}
if (isset($values['username'])) {
$SESSION->set('resetusername', false);
if ($values['username'] != $USER->get('username')) {
$USER->username = $values['username'];
$USER->commit();
$otherfield = true;
}
}
// This method should exist, because if it did not then the change foreach ($values as $field => $value) {
// password form would not have been shown. if (in_array($field, array('submit', 'sesskey', 'password1', 'password2', 'username'))) {
if ($password = $authobj->change_password($USER, $values['password1'])) { continue;
$SESSION->add_ok_msg(get_string('passwordsaved')); }
if (!empty($values['email'])) { if ($field == 'email') {
$USER->email = $values['email']; $USER->email = $values['email'];
$USER->commit(); $USER->commit();
set_profile_field($USER->id, 'email', $values['email']);
} }
redirect(); set_profile_field($USER->get('id'), $field, $value);
$otherfield = true;
} }
// TODO: Exception is the wrong type here! if (isset($otherfield)) {
throw new Exception('Attempt by "' . $USER->get('username') . '@' $SESSION->add_ok_msg(get_string('requiredfieldsset', 'auth'));
. $USER->get('institution') . 'to change their password failed'); }
redirect();
} }
/** /**
...@@ -1260,7 +1261,6 @@ function login_submit(Pieform $form, $values) { ...@@ -1260,7 +1261,6 @@ function login_submit(Pieform $form, $values) {
// User is allowed to log in // User is allowed to log in
//$USER->login($userdata); //$USER->login($userdata);
auth_check_password_change();
auth_check_required_fields(); auth_check_required_fields();
} }
......
...@@ -490,10 +490,7 @@ ...@@ -490,10 +490,7 @@
<TABLE NAME="usr_registration"> <TABLE NAME="usr_registration">
<FIELDS> <FIELDS>
<FIELD NAME="id" TYPE="int" LENGTH="10" SEQUENCE="true" NOTNULL="true" /> <FIELD NAME="id" TYPE="int" LENGTH="10" SEQUENCE="true" NOTNULL="true" />
<FIELD NAME="username" TYPE="char" LENGTH="30" NOTNULL="true"/>
<FIELD NAME="password" TYPE="char" LENGTH="40" NOTNULL="true"/>
<FIELD NAME="institution" TYPE="char" LENGTH="255" NOTNULL="true"/> <FIELD NAME="institution" TYPE="char" LENGTH="255" NOTNULL="true"/>
<FIELD NAME="salt" TYPE="char" LENGTH="8" NOTNULL="true"/>
<FIELD NAME="firstname" TYPE="text" NOTNULL="true"/> <FIELD NAME="firstname" TYPE="text" NOTNULL="true"/>
<FIELD NAME="lastname" TYPE="text" NOTNULL="true"/> <FIELD NAME="lastname" TYPE="text" NOTNULL="true"/>
<FIELD NAME="email" TYPE="text" NOTNULL="true"/> <FIELD NAME="email" TYPE="text" NOTNULL="true"/>
......
...@@ -1177,6 +1177,16 @@ function xmldb_core_upgrade($oldversion=0) { ...@@ -1177,6 +1177,16 @@ function xmldb_core_upgrade($oldversion=0) {
ensure_record_exists('event_type', $event, $event); ensure_record_exists('event_type', $event, $event);
} }
if ($oldversion < 2009082400) {
$table = new XMLDBTable('usr_registration');
$field = new XMLDBField('username');
drop_field($table, $field);
$field = new XMLDBField('salt');
drop_field($table, $field);
$field = new XMLDBField('password');
drop_field($table, $field);
}
return $status; return $status;
} }
......
...@@ -27,7 +27,7 @@ ...@@ -27,7 +27,7 @@
defined('INTERNAL') || die(); defined('INTERNAL') || die();
$config = new StdClass; $config = new StdClass;
$config->version = 2009082101; $config->version = 2009082400;
$config->release = '1.2.0beta2dev'; $config->release = '1.2.0beta2dev';
$config->minupgradefrom = 2008040200; $config->minupgradefrom = 2008040200;
$config->minupgraderelease = '1.0.0 (release tag 1.0.0_RELEASE)'; $config->minupgraderelease = '1.0.0 (release tag 1.0.0_RELEASE)';
......
...@@ -100,15 +100,14 @@ if (isset($key)) { ...@@ -100,15 +100,14 @@ if (isset($key)) {
} }
$user = new User(); $user = new User();
$user->username = $registration->username;
$user->password = $registration->password;
$user->salt = $registration->salt;
$user->passwordchange = 0;
$user->active = 1; $user->active = 1;
$user->authinstance = $authinstance->id; $user->authinstance = $authinstance->id;
$user->firstname = $registration->firstname; $user->firstname = $registration->firstname;
$user->lastname = $registration->lastname; $user->lastname = $registration->lastname;
$user->email = $registration->email; $user->email = $registration->email;
$user->username = get_new_username($user->firstname . $user->lastname);
$user->passwordchange = 1;
$user->salt = substr(md5(rand(1000000, 9999999)), 2, 8);
create_user($user, $profilefields); create_user($user, $profilefields);
...@@ -150,6 +149,7 @@ if (isset($key)) { ...@@ -150,6 +149,7 @@ if (isset($key)) {
else { else {
$SESSION->add_ok_msg(get_string('registrationcomplete', 'mahara', get_config('sitename'))); $SESSION->add_ok_msg(get_string('registrationcomplete', 'mahara', get_config('sitename')));
} }
$SESSION->set('resetusername', true);
redirect(); redirect();
} }
create_registered_user(); create_registered_user();
...@@ -159,30 +159,6 @@ if (isset($key)) { ...@@ -159,30 +159,6 @@ if (isset($key)) {
// Default page - show the registration form // Default page - show the registration form
$elements = array( $elements = array(
'username' => array(
'type' => 'text',
'title' => get_string('username'),
'rules' => array(
'required' => true
),
'help' => true,
),
'password1' => array(
'type' => 'password',
'title' => get_string('password'),
'description' => get_string('passwordformdescription', 'auth.internal'),
'rules' => array(
'required' => true
),
'help' => true,
),
'password2' => array(
'type' => 'password',
'title' => get_string('confirmpassword'),
'rules' => array(
'required' => true
)
),
'firstname' => array( 'firstname' => array(
'type' => 'text', 'type' => 'text',
'title' => get_string('firstname'), 'title' => get_string('firstname'),
...@@ -297,18 +273,6 @@ function register_validate(Pieform $form, $values) { ...@@ -297,18 +273,6 @@ function register_validate(Pieform $form, $values) {
$institution = $values['institution']; $institution = $values['institution'];
safe_require('auth', 'internal'); safe_require('auth', 'internal');
if (!$form->get_error('username') && !AuthInternal::is_username_valid($values['username'])) {
$form->set_error('username', get_string('usernameinvalidform', 'auth.internal'));
}
if (!$form->get_error('username') && record_exists_select('usr', 'LOWER(username) = ?', strtolower($values['username']))) {
$form->set_error('username', get_string('usernamealreadytaken', 'auth.internal'));
}
$user =(object) $values;
$user->authinstance = get_field('auth_instance', 'id', 'authname', 'internal', 'institution', $institution);
password_validate($form, $values, $user);
// First name and last name must contain at least one non whitespace // First name and last name must contain at least one non whitespace
// character, so that there's something to read // character, so that there's something to read
if (!$form->get_error('firstname') && !preg_match('/\S/', $values['firstname'])) { if (!$form->get_error('firstname') && !preg_match('/\S/', $values['firstname'])) {
...@@ -365,8 +329,6 @@ function register_submit(Pieform $form, $values) { ...@@ -365,8 +329,6 @@ function register_submit(Pieform $form, $values) {
// don't die_info, since reloading the page shows the login form. // don't die_info, since reloading the page shows the login form.
// instead, redirect to some other page that says this // instead, redirect to some other page that says this
safe_require('auth', 'internal'); safe_require('auth', 'internal');
$values['salt'] = substr(md5(rand(1000000, 9999999)), 2, 8);
$values['password'] = AuthInternal::encrypt_password($values['password1'], $values['salt']);
$values['key'] = get_random_key(); $values['key'] = get_random_key();
// @todo the expiry date should be configurable // @todo the expiry date should be configurable
$values['expiry'] = db_format_timestamp(time() + 86400); $values['expiry'] = db_format_timestamp(time() + 86400);
......
{include file="header.tpl"} {include file="header.tpl"}
{if $changepassword}
<h1>{str tag="changepassword"}</h1>
<p>{str tag="changepasswordinfo"}</p>
{if $loginasoverridepasswordchange}<div class="message">{$loginasoverridepasswordchange}</div>{/if}
{else}
<h1>{str tag='requiredfields' section='auth'}</h1> <h1>{str tag='requiredfields' section='auth'}</h1>
{/if}
{$form} {$form}
{include file="footer.tpl"} {include file="footer.tpl"}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment