Problem/Motivation

Smartphones that can install TOTP apps are not ubiquitous and TFA Basic does not allow SMS to be the primary TFA method for accounts.

Proposed resolution

Allow other TFA plugins to be set as the primary validation TFA method.

Remaining tasks

  1. Allow tfa_validate plugin to be tfa_basic_sms, tfa_basic_recovery_code, or tfa_basic_help (partially handled in update #8
  2. Support validation checks of sms and help configuration
  3. Modify tfa_basic_overview to check for sms, recovery_code, and help plugins being set in tfa_validate
  4. Modify tfa_basic_overview to support totp plugin in tfa_fallback

User interface changes

  • TFA plugin admin
  • Account TFA setup screens

API changes

None expected.

Original report

In the territory where my site is used, SMS is ubiquitous, whilst smartphones are not necessarily as common and it would be useful to set SMS as the default validation plugin, with TOTP and Recovery Codes as a fall-back.

Examples

CommentFileSizeAuthor
#13 2-step verification.png74.33 KBscor
#12 2335063-tfa-basic-allow-sms-12.patch39.77 KBcoltrane
#11 2335063-tfa-basic-allow-sms-11.patch39.28 KBcoltrane
#10 2335063-tfa-basic-allow-sms-10.patch37.45 KBcoltrane
#6 2335063-tfa-basic-allow-sms-6.patch4.91 KBcoltrane
#4 allow_sms_plugin_as-2335063-4.patch615 bytessubhojit777

Comments

johnkareoke’s picture

johnkareoke commented
Status: Active » Closed (fixed)

Have taken a look at the module code and see that this is relatively easy to change: have commented out line 404 of tfa_basic.module

/**
 * Implements hook_form_FORM_ID_alter().
 */
function tfa_basic_form_tfa_admin_settings_alter(&$form, &$form_state, $form_id) {
  global $cookie_domain;
  // Disallow recovery codes and SMS from being used as default validation.
  unset($form['tfa_validate']['#options']['tfa_basic_recovery_code']);
<strong>  unset($form['tfa_validate']['#options']['tfa_basic_sms']);</strong>
coltrane’s picture

coltrane
he/him
commented
Title: SMS plugin as optional primary TFA validation plugin and option Twilio Mobile Number » Allow SMS plugin as optional primary TFA validation plugin and admin config Twilio Mobile Number
Status: Closed (fixed) » Active

Your approach isn't recommended, commenting out the code will mean that when you upgrade to a new TFA Basic release you'll undo your change and have to manually reset. A better approach is for the module to adapt to support your feature request. This is doable just needs a patch to the admin form and context handling.

subhojit777’s picture

subhojit777
he/him
Primary language Bengali
commented

Is there any reason those values were unset.

subhojit777’s picture

subhojit777
he/him
Primary language Bengali
commented
Status: Active » Needs review
StatusFileSize
new allow_sms_plugin_as-2335063-4.patch615 bytes

I have removed the code as said by @johncarolin in #1, and it is working alright. I mean if Twilio is selected, then TOTP becomes the fallback validation, no issues with that.

realkevinoshea’s picture

realkevinoshea commented

Patch worked fine for me. My use case is the same, we want to verify using SMS only because many of our users do not have smartphones.

coltrane’s picture

coltrane
he/him
commented
StatusFileSize
new 2335063-tfa-basic-allow-sms-6.patch4.91 KB

Thanks for the work here but #4 doesn't modify the #states logic so that Twilio config variables are required. Try this out please.

coltrane’s picture

coltrane
he/him
commented
Status: Needs review » Needs work

Needs work to support the setup flow because TFA Basic assumes TOTP first and doesn't check for SMS or recovery codes as tfa_validate plugin.

coltrane’s picture

coltrane
he/him
commented
Issue summary: View changes

Updated issue summary to clarify the remaining work necessary to achieve the goal.

pjcdawkins’s picture

pjcdawkins commented

Yes, I'd be interested in any way to get the setup stage working via SMS.

coltrane’s picture

coltrane
he/him
commented
Status: Needs work » Needs review
StatusFileSize
new 2335063-tfa-basic-allow-sms-10.patch37.45 KB

Attached patch gets the various plugins working with user setup depending on their configuration (as primary validation or fallback).

Tests pass but could use testing with SMS enabled as primary validation plugin. I was mostly testing using the Recovery codes as the validation plugin.

Note, you'll need to clear Drupal page cache after applying this patch because some menu page arguments have changed.

coltrane’s picture

coltrane
he/him
commented
StatusFileSize
new 2335063-tfa-basic-allow-sms-11.patch39.28 KB

SMS works just needed some minor text updates for being the primary validation plugin.

coltrane’s picture

coltrane
he/him
commented
StatusFileSize
new 2335063-tfa-basic-allow-sms-12.patch39.77 KB

#11 was missing modification to tfa_basic_context_alter for SMS being set as validation.

scor’s picture

scor commented
Issue summary: View changes
StatusFileSize
new 2-step verification.png74.33 KB

Sorry if I missed it, but it's not clear in the summary. What's the intend behavior here? Is it the site admin who is going to choose what the primary plugin (and thus required) is? The ideal behavior IMO is similar to what Google does, which is to let the user choice between SMS and TOTP for example:

coltrane’s picture

coltrane
he/him
commented
Issue summary: View changes

Updated issue summary to more clearly define the problem and proposed solution.

scor’s picture

scor commented
Issue summary: View changes

Thanks Ben. Removed duplicate image from OP.

Allow other TFA plugins to be set as the primary validation TFA method.

Will the primary validation method be required for a user to be able to setup another validation method? Example: If the site admin sets SMS as primary, will the user be able to setup TOTP w/o SMS? and vice versa?

coltrane’s picture

coltrane
he/him
commented

TFA Basic currently (and also with the latest patch) doesn't allow skipping setting up the primary method. It would make sense to allow either to be set but I'll have to think on how to adapt the setup form to handle that. Given that this patch is already kind of big I'm inclined to push that to a separate ticket.

  • coltrane committed a2d1be1 on 7.x-1.x 
    Issue #2335063 by coltrane: Allow any plugin to be main validation...
coltrane’s picture

coltrane
he/him
commented
Status: Needs review » Fixed

Committed #12. Per-user selection of TFA method at #2478307: Allow user to decide which method is primary

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.