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

User Email Addresses for Registraton and Login via $CFG['emailAsSID'] option #201

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xyztextbooks
Copy link
Contributor

Changes in the Pull request enable the use of email addresses for registration and login. Detailed Development notes are here:
https://docs.google.com/document/d/1fAhsDDIyJsvnlR79o8W9PcUa1ks4MD3QxC7AzPRbjJQ/edit?usp=sharing

  • When the new $CFG['emailAsSID'] option is enabled, the SID (username) field is hidden from users, so users register and login with their email address
  • Under the hood, the SID field is still used for registration and login, but it is set equal to the email field's value wherever possible (e.g., registration and login forms, and forms that allow for changing of user email addresses).
  • Requires database modifications to increase length of imas_users.SID field to 100
  • Requires modification of $loginprompt, $longloginprompt, and $loginformat vars in config.php
  • Complete usage notes appears in the new 'Using Email Addresses for Login' of readme.md

Files that were not modified (that are related to usernames):

  • batchcreateinstr.php
  • newinstructor-ipeds.php
  • modltidomaincred case in admin/actions.php

Several of the modified forms ("New Instructor Account Request", "Modify User Profile", "Admin -> New User", "Roster->Enroll a New Student") will allow an email address that is in another account's email field (as long as it is unique to the SID field). This will accommodate legacy situations that previously created multiple accounts with the same email.

- When enabled, the SID (username) field is hidden from users, so users register and login with their email address
- Under the hood, the SID field is still used for registration and login, but it is set equal to the email field's value wherever possible (e.g., registration and login forms, and forms that allow for changing of user email addresses).
- Requires database modifications to increase length of imas_users.SID field to 100
- Requires modification of $loginprompt, $longloginprompt, and $loginformat vars in config.php
- Complete usage notes appears in the new 'Using Email Addresses for Login' of readme.md
@xyztextbooks xyztextbooks changed the title User Email Addresses for Registraton and Login (Add support for $CFG['emailAsSID'] option) User Email Addresses for Registraton and Login via $CFG['emailAsSID'] option Oct 26, 2020
@drlippman
Copy link
Owner

I like the idea of this, but I worry about the confusion that would be caused by prompting for email but expecting a username from people who have legacy usernames. Also, that editing a user's info would seem to change their username to their email without them possibly realizing.

I don't have an idea offhand of how to improve this, given that many systems already will have multiple user accounts with the same email.

@xyztextbooks
Copy link
Contributor Author

I agree that these changes are not ideal for legacy installations. Here at XYZ, we have three iMathAS deployments that already use email for login thanks to changes we made to our iMathAS fork. My main hope for this pull request was to get these changes into the iMathAS master branch and thereby avoid having to manually merge code every time we update iMathAS.

If you are still willing to entertain the idea of merging these changes, I suppose there are a few options:

  • document this option as "for new installations only"
  • document this option as "experimental" with full disclosure of the risks of using it
  • leave the changes undocumented and therefore usable only by those-in-the-know

@drlippman
Copy link
Owner

Fair enough. Being a config option, I suppose figuring out how to transition from the default to emails can wait.

There's a lot of code changes here, so it may take me a while to finish reviewing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants