Implement two-step auth with authenticator apps #211
Conversation
|
I think having two fields for twostep under What if we have I think that might be clearer. |
| reply += "the following command:<br />"; | ||
| reply += "<code>/twostep verify [code from your authenticator]</code>"; | ||
| return user.popup(reply); | ||
| } |
There was a problem hiding this comment.
There should be a fallout here telling the user their options are either email or authentication, and maybe sending them the command's syntax
I.e.,
} else {
return this.errorReply("In order to setup two-step authentication, you must specify if you want to use your email or a seperate two-step authentication app, such as Google Authentication. \nTo do this, use EITHER /twostep setup email OR /twostep setup authenticator.");
}
| buff += `<table border="1" cellspacing ="0" cellpadding="3">`; | ||
| buff += `<tr><td colspan=3><center>${(user.codeAttempt && user.codeAttempt.length > 0 ? user.codeAttempt.join('') : ' ')}</center></td></tr>`; | ||
| buff += `<tr><td>${this.generateButton(1)}</td><td>${this.generateButton(2)}</td><td>${this.generateButton(3)}</td></tr>`; | ||
| buff += `<tr><td>${this.generateButton(4)}</td><td>${this.generateButton(5)}</td><td>${this.generateButton(6)}</td></tr>`; | ||
| buff += `<tr><td>${this.generateButton(7)}</td><td>${this.generateButton(8)}</td><td>${this.generateButton(9)}</td></tr>`; | ||
| buff += `<tr><td> </td><td>${this.generateButton(0)}</td><td> </td></tr></table></center>`; | ||
| buff += `<tr><td>${this.generateButton('<-')}</td><td>${this.generateButton(0)}</td><td>${this.generateButton('R')}</td></tr></table></center>`; |
There was a problem hiding this comment.
Does this mean email popups give the R key?
If so, why? Should they? Maybe use a ternary operator checking if authenticator is true or not?
| let qrImg = "https://chart.googleapis.com/chart?chs=166x166&chld=L|0&cht=qr&chl=" + uri; | ||
| let reply = "|modal||html|Please enter the following code into your authenticator application or scan the QR code.<br />"; | ||
| reply += "Key: " + twoAuthData.secret + "<br />"; | ||
| reply += "<img src=\"" + qrImg + "\" height=\"166\" width=\"166\"><br />"; |
| if (verified) { | ||
| Gold.userData[user.userid].email = user.twostepEmail.email; | ||
| Gold.saveData(); | ||
| return this.sendReply("Two-step authentication has been officially setup for your account."); |
There was a problem hiding this comment.
... as opposed to unofficially?
This was shitty wording to begin with; remove "officially"
|
Is there a particular reason as to why we give multiple emergency codes? What is the actual use-case of giving more than just 1? |
|
Also yeah, don't forget to rebase this; I don't know why GitHub is acting like it doesn't have to be |
| @@ -111,41 +131,90 @@ exports.commands = { | |||
| setup: function (target, room, user) { | |||
| if (!user.named) return this.errorReply("You must be logged in to use this command."); | |||
| if (!user.registered) return this.errorReply("You cannot setup two-step authentication on an account that isn't registered."); | |||
| if (Gold.userData[user.userid] && (Gold.userData[user.userid].email || Gold.userData[user.userid].twostepauth)) return this.errorReply("This account already has two-step authentication enabled."); | |||
There was a problem hiding this comment.
I can't think of how Gold.userData[user.userid] here could be undefined...
There are checks above to make sure the user is logged in and not on a guest account, so I don't think that part of the if statement is necessary... Correct me if I'm wrong?
|
How much testing have you done on this latest version? |
|
This is going to have to be rebased (again) |
|
Nevermind, I rebased it for you |
No description provided.