Expose Email as available two factor authentication method

An email provider is already written in the example api code, it'd just be a matter of elevating it to the module code. Let me think on this some more.

I think https://drupal.org/project/email_confirm solves the problem described in the original post.

I agree that having email as a real provider would be helpful though it shouldn't be the "default."

+1 on this one. When I started working on TFA one of my first few questions to Ben was whether this functionality is implemented somewhere. Steam uses this for new devices btw.

I don't know where that example code is, but if I'm directed to it I'm happy to provide a patch.

I think ben's comment about email being in the example code is no longer relevant because the code has been rewritten.

I'd say that this is probably most similar to the the sms code.

Okay, thanks Greg!
I'm working on it.

This was pretty useful as I got more familiar with the setup process of TFA Basic.
The texts could probably be better.

TFA Email setup with no other TFA options set does not work. I can log directly into the site after requiring TFA for my account.

This would be a really nice feature.

Hi,

I have deployed TFA module on my localhost machine and also integrate this email patch, but in setup configuration, when I add security code which I received on email, it asks me to enter admin password and then it again redirects to same page for entering verification code, it is not confirming the email setup account, also I received verification code again. Can anyone help me to sort out this issue.

Thanks
Aneel

I am attempting to develop my own patch (for D7) in response to subsequent comments regarding the https://www.drupal.org/files/issues/2090033-8-tfa-basic-email.patch file on the https://www.drupal.org/node/2090033 issue.

What I notice is that the ready() function, when invoked, does not recognize isset($data['plugins']) or isset($data['data']['plugins']) as being TRUE, even when manually adding tfa_basic_email to the JSON stored in the appropriate row in the tfa_user_settings table, which causes WSOD. The tfa_basic_setup_save_data() function is never invoked when utilizing the E-mail authentication method provided by the patch. If ready() returns FALSE, then tfa_user_login() won't be invoked with conditions for a drupal_goto('system/tfa/' . $account->uid . '/' . $login_hash, array('query' => $query)) if tfa_user_login() is invoked at all.

How may I invoke tfa_user_login() in the tfa_basic_email plugin?

+1 patch

I made a module "tfa_email" in 2015 and it's been working fine for me since then. It was developed standalone (outside of tfa_basic). Since there is some interest and this issue is "Needs Work", I'll leave a .patch here since it may be useful.

Basically, I just created a "modules" directory in tfa_basic, moved my module into it, and did a git diff to create a patch. It might be worth more tightly integrating it with tfa_basic (rather than having it as a separate module).

I realized that I messed up the directories in the patch I previously uploaded. So I'm uploading a new patch. The module files should now be under "modules/tfa_email" instead of just "modules". I also fixed whitespace warnings.

I wasn't able to delete my old patch, so I hid it from displaying. I've also provided an interdiff between my two patches (which probably isn't that useful).

@nullkernel, thanks for sharing your patch. It works as expected and plays nicely with TFA Basic.

The following code adds email subject and message fields to the TFA Basic UI for both default and fallback states.

This code can be rolled into nullkernel's patched module (with function names prefixed accordingly) or used separately in a custom companion module with a dependency on the tfa_basic_email module.

/**
 * Implements hook_form_FORM_ID_alter().
 */
function custom_form_tfa_admin_settings_alter(&$form, &$form_state, $form_id) {
  $email_states = array(
    'visible' => array(
      array(
        array(':input[name="tfa_fallback[tfa_basic_email][enable]"]' => array('checked' => TRUE)),
        'or',
        array(':input[name="tfa_validate"]' => array('value' => 'tfa_basic_email'))
      )
    )
  );
  $form['tfa_basic_email_subject_text'] = array(
    '#type' => 'textfield',
    '#title' => t('Email subject text'),
    '#default_value' => variable_get('tfa_basic_email_subject_text', '!sitename verification code'),
    '#description' => t('The email subject sent to user.'),
    '#weight' => 1,
    '#states' => $email_states
  );
  $form['tfa_basic_email_message_text'] = array(
    '#type' => 'textfield',
    '#title' => t('Email message text'),
    '#default_value' => variable_get('tfa_basic_email_message_text', 'Verification code: !code'),
    '#description' => t('The email message sent to user.'),
    '#weight' => 1,
    '#states' => $email_states
  );

  $form['#validate'][] = 'custom_form_tfa_admin_settings_validate';
  $form['#submit'][] = 'custom_form_tfa_admin_settings_submit';
}

/**
 * Validate handler for custom_form_tfa_admin_settings_alter().
 */
function custom_form_tfa_admin_settings_validate($form, &$form_state) {
  if (!empty($values['tfa_fallback']) && (!empty($values['tfa_fallback']['tfa_basic_email']['enable']) || $values['tfa_validate'] === 'tfa_basic_email')) {
    if (empty($form_state['values']['tfa_basic_email_subject_text'])) {
      form_set_error('tfa_basic_email_subject_text', t('Email subject required if Email plugin is enabled.'));
    }
    if (empty($form_state['values']['tfa_basic_email_message_text'])) {
      form_set_error('tfa_basic_email_message_text', t('Email message required if Email plugin is enabled.'));
    }
  }
}

/**
 * Submit handler for custom_form_tfa_admin_settings_alter().
 */
function custom_form_tfa_admin_settings_submit($form, &$form_state) {
  if (!empty($form_state['values']['tfa_basic_email_subject_text'])) {
    variable_set('tfa_basic_email_subject_text', $form_state['values']['tfa_basic_email_subject_text']);
  }
  if (!empty($form_state['values']['tfa_basic_email_message_text'])) {
    variable_set('tfa_basic_email_message_text', $form_state['values']['tfa_basic_email_message_text']);
  }
}

I've refactored my standalone module's code so that it is integrated with tfa_basic (instead of existing as a separate module). Please review.

Thanks for the admin form code @johnhanley. The patch I'm attaching includes your code as well.

Anyone who enabled the "tfa_email" module should disable/uninstall/remove it before applying this patch. Otherwise fatal errors will occur due to some functions being defined twice (if the tfa_email module is still enabled).

I noticed a problem after integrating with tfa_basic, and have updated the patch to fix this. Whether or not a user had tfa_basic_email enabled in their configuration didn't matter - email would still be used as a TFA method. This is because TfaBasicEmail::ready() wasn't implemented, and simply used the TfaBasePlugin::ready() method (which always returns TRUE). With this patch, users shouldn't get email challenges unless they have set up tfa_basic_email.

Wow.. thanks for your big improvement. I already used it in my production site and it works as expected. I'm not sure why testing for php5.5 undergoes as a failure but it works fine in my environment using PHP7.0.

Some little improvements such email code expiration, support code case insensitive (useful for code with letters) and configure plugin by config variables.

One small fix for title. Verification Code should be Email Verification Code.

Unassigning - looks like there is three years since banviktor worked on this.

I will also add that I agree that patch in #22 is RTBC.

Thanks for your work on this, everyone! I have some feedback, some of which seems like it should be fixed prior to the commit of this patch, so I'm moving that status to needs work.

The TfaBasicEmail::generate code generation code uses the old Drupal-5-era password generation code that can lead to insecure codes being generated. That was the basis of https://www.drupal.org/sa-contrib-2018-044 The change to match more modern style of generating a random string is in this commit.

The TfaBasicEmail::validate function uses string comparison that can lead to timing attacks. It would be better to use hash_equals for the comparison.

A few functions are missing php documentation.

I believe the string below causes problems for translations and that it's better to use double quotes around the string so that the ' on "account's" does not need to be escaped with a \.

+          '#value' => t('Receive verification code via your account\'s email address (@email_address).', array('@email_address' => $account->mail)),

The code comment in TfaBasicEmailSetup::begin is missing terminating punctuation. Generally it seems like it would be good to review this with the php code style rules.

Changes for comments from #24.

hash_code is not part of (PHP 5 >= 5.6.0).

Added embedded hash_code check from https://github.com/facebook/php-graph-sdk/blob/5.x/src/Facebook/polyfill...

Uploaded wrong patch. :(

Thank you monstrfolk, your patch looks good and worked for me. There are two "@todo"s in it, do you want feedback on those and to address before commit? IMO neither need addressing but curious to know what you think.

Coltrane, completed one TODO. Left second TODO since is same as SMS code. All changed label text to be more consistent with other TFA code. Please send feedback, or commit. I believe there is nothing pressing that must be completed before commit.

Dear monstrfolk, just applied your latest patch at #30 on my production site. Looks really good and working without any single error occur. Run using PHP7.1 and it works smoothly. I suggest it should be committed to dev version.

Hi, +1 for this, I'm using it and it works well.

I'm going to be using this patch in #30, curious when/if it will be included in a release soon.

Can remove:

protected function validate($code) {
  <snip>
}

Because it was added to tfa 7.x-2.x as timingSafeEquals() in #3183248: Use hash_equals instead of ===.