Only create backup codes when user initiated #296
Conversation
adamwoodnz
force-pushed
the
update/250-explicit-backup-code-generation
branch
from
325ff31 to
f87ee1d
Compare
adamwoodnz
force-pushed
the
update/250-explicit-backup-code-generation
branch
from
f87ee1d to
dd49e23
Compare
| ! backupCodesEnabled || | ||
| backupCodesRemaining === 0 || | ||
| regenerating || | ||
| ! backupCodesVerified |
There was a problem hiding this comment.
These are the conditions which led to the codes being generated on load
There was a problem hiding this comment.
We currently auto navigate to the backup code component when a user sets up a one time password. We'll also do that in the onboarding flow and add that support for Webauthn. Now that this doesn't auto trigger, do you think we should export the "Setup" component and handle that in those flows? Alternatively we could pass a prop into this component. Thoughts?
There was a problem hiding this comment.
Ah I wasn't aware that they need to be auto generated in those cases. Yeah passing a prop seems reasonable.
There was a problem hiding this comment.
Sorry I'm confused about this. We can still auto navigate to backup codes, but my understanding is that we still should not auto generate. Is it a requirement that codes are generated if one of the 2FA methods has been activated? If it is then auto generation still makes sense.
There was a problem hiding this comment.
It's not a requirement per se, but we want to limit the number of accounts that don't have recovery codes as much as possible seeing that regaining access to a locked out account can be troublesome for users.
There was a problem hiding this comment.
Yeah I guess it's a balance between acting on their account without their action vs ensuring they don't get locked out. I'll add the auto-generation so we can see what it looks like.
There was a problem hiding this comment.
Reading through #507, and in particular @dd32's comment, I don't think auto generating is the best path here. If we're going to auto generate it feels like we should only display the codes, and require a second action to save. I've updated the button text in this PR to be more explicit about the action taken if they do choose to generate. What do you think?
There was a problem hiding this comment.
I think that's fair. I just tested this. It does help to establish context.
We can monitor whether users bail on this view and adjust as necessary.
| <ButtonGroup> | ||
| <CopyToClipboardButton codes={ backupCodes } /> | ||
| <PrintButton /> | ||
| <DownloadButton codes={ backupCodes } /> | ||
| </ButtonGroup> |
There was a problem hiding this comment.
Moved to the CodeList and conditionally rendered
adamwoodnz
changed the title
adamwoodnz
force-pushed
the
update/250-explicit-backup-code-generation
branch
3 times, most recently
from
b85ea72 to
985f7a5
Compare
| @@ -20,7 +20,11 @@ import { GlobalContext } from '../script'; | |||
| * Render the correct component based on the URL. | |||
| */ | |||
| export default function Settings() { | |||
| const { backupCodesEnabled, navigateToScreen, screen } = useContext( GlobalContext ); | |||
| const { | |||
| user: { backupCodesEnabled }, | |||
There was a problem hiding this comment.
This was undefined. Sometimes I wish we had TS for things like this.
| <Button isSecondary onClick={ () => setGenerating( true ) }> | ||
| { backupCodesEnabled | ||
| ? 'Regenerate and save backup codes' | ||
| : 'Generate and save backup codes' } |
There was a problem hiding this comment.
I understand why we want to add "Generate and save backup codes" because that's what technically happens but I don't know that first time users will understand what save means here. I could be wrong.
adamwoodnz
force-pushed
the
update/250-explicit-backup-code-generation
branch
from
c889ef6 to
e126858
Compare
adamwoodnz
force-pushed
the
update/250-explicit-backup-code-generation
branch
from
e126858 to
f5a9c25
Compare
Fixes #250
Changes the backup codes screen to not generate codes on load. Instead the
Generate codesbutton must be activated. Once backup codes have been enabled the button text changes toRegenerate codesas before.Improves the UI by not rendering the action buttons until the codes are available.