Problem/Motivation

When a user is required to set up TFA, the page displays a message:

You are required to setup two-factor authentication here. You have 2 attempts left. After this you will be unable to login.

This message is displayed regardless of the user's access to the TFA overview page. As a result, users may be served an 'Access denied' page when clicking 'here'.

Proposed resolution

Display a different message for users without the 'setup own tfa' permission.

Remaining tasks

  1. Write a patch
  2. Review
  3. Commit

User interface changes

Users that have no access to setup their TFA are not directed to the account TFA overview page.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#9 3089931-directed-to-TFA-overview-without-permission.patch-1x-9.patch2.6 KBcmlara
#8 3089931-directed-to-TFA-overview-without-permission.patch-8.patch2.39 KBcmlara
#8 interdiff-3089931-7-8.txt744 bytescmlara
#7 3089931-directed-to-TFA-overview-without-permission.patch2.42 KBcmlara
#7 interdiff-3089931-3-7.txt1.12 KBcmlara
#3 Adds_different_message-3089931-3.patch1.96 KBvatsalkhanna
#2 Adds_different_message_if_user_doesnt_have_permission_to_setup_TFA-3089931-2.patch%0A;%0Agit diff > Adds_different_message_if_user_doesnt_have_permission_to_setup_TFA-3089931-2.patch1.96 KBvatsalkhanna

Comments

idebr created an issue. See original summary.

vatsalkhanna’s picture

vatsalkhanna commented
StatusFileSize
new Adds_different_message_if_user_doesnt_have_permission_to_setup_TFA-3089931-2.patch%0A;%0Agit diff > Adds_different_message_if_user_doesnt_have_permission_to_setup_TFA-3089931-2.patch1.96 KB

Please check the patch attached.

vatsalkhanna’s picture

vatsalkhanna commented
StatusFileSize
new Adds_different_message-3089931-3.patch1.96 KB

Please ignore the above one. Check this patch

vatsalkhanna’s picture

vatsalkhanna commented
Status: Active » Needs review
jcnventura’s picture

jcnventura
Primary language English
commented
Version: 8.x-1.x-dev » 2.x-dev

Moving all open issues to the 2.x branch.

cmlara’s picture

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

Needs rebase to 2.x

Also the message "You do not have permission to setup TFA. Please contact administrator" would likely be better along the lines of:

"You are required to setup two-factor authentication however your account does not have the necessary permissions. Please contact an administrator. You have @remaining attempt(s) left. After this you will be unable to login." Run through formatPlural() so that users know the risk of not taking action.

cmlara’s picture

cmlara
he/him
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-3089931-3-7.txt1.12 KB
new 3089931-directed-to-TFA-overview-without-permission.patch2.42 KB
cmlara’s picture

cmlara
he/him
commented
StatusFileSize
new interdiff-3089931-7-8.txt744 bytes
new 3089931-directed-to-TFA-overview-without-permission.patch-8.patch2.39 KB

Critical change: the hasSkipped() needs to be outside the if/else check as it applies to both those user with and without permission to setup their TFA tokens.

cmlara’s picture

cmlara
he/him
commented
StatusFileSize
new 3089931-directed-to-TFA-overview-without-permission.patch-1x-9.patch2.6 KB

Here is the 1.x patch

The change to TfaPasswordResetTest.php is due to #3387681: TfaPasswordResetTest does not directly test TFA prompts. testing for the messages instead of validating the login forms.

  • cmlara committed a8ff1870 on 2.x 
    Issue #3089931 by vatsalkhanna, cmlara: Users are directed to TFA...

  • cmlara committed a5edf54d on 8.x-1.x 
    Issue #3089931 by vatsalkhanna, cmlara: Users are directed to TFA...
cmlara’s picture

cmlara
he/him
commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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