Skip to content
This repository was archived by the owner on Sep 20, 2023. It is now read-only.

Implement 2FA reset for T3#4528

Merged
tomlinton merged 7 commits intomasterfrom
tomlinton/t3-2fa-change
Feb 11, 2021
Merged

Implement 2FA reset for T3#4528
tomlinton merged 7 commits intomasterfrom
tomlinton/t3-2fa-change

Conversation

@tomlinton
Copy link
Copy Markdown
Contributor

@tomlinton tomlinton commented Feb 3, 2021

Implements 2FA reset for T3.

Checklist:

@tomlinton tomlinton marked this pull request as ready for review February 4, 2021 01:56
Copy link
Copy Markdown
Collaborator

@shahthepro shahthepro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a quick look, looks good to me. LGTM

Comment thread infra/token-transfer-client/src/actions/otp.js Outdated
oldCodeError: null,
newCode: '',
newCodeError: null,
modalState: 'OTP'
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Probably better to put this as an enum

Copy link
Copy Markdown
Contributor

@franckc franckc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread infra/token-transfer-server/src/controllers/user.js
@franckc
Copy link
Copy Markdown
Contributor

franckc commented Feb 9, 2021

Do we have any sort of alerting on T3? It would be nice to have an alert on unusual high volume of 2FA resets... that could indicate a hack.

@tomlinton
Copy link
Copy Markdown
Contributor Author

We've got Discord webhooks so I'll add a notification 👍

@tomlinton tomlinton merged commit 3981e54 into master Feb 11, 2021
@tomlinton tomlinton deleted the tomlinton/t3-2fa-change branch February 11, 2021 01:18
@franckc
Copy link
Copy Markdown
Contributor

franckc commented Feb 11, 2021

We've got Discord webhooks so I'll add a notification 👍

ah yes, perfect!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants