Skip to content

Conversation

@lauryndbrown
Copy link
Contributor

Add a link to the sms two-factor authentication page to resend code for the case the user does not have their phone immediately handy before the time limit is up.

@lauryndbrown
Copy link
Contributor Author

screen shot 2018-01-05 at 5 17 48 pm

@lauryndbrown lauryndbrown changed the title feat(2FA): Added a resend code link to sms 2fa page feat(2fa): Added a resend code link to sms 2fa page Jan 6, 2018
@benvinegar
Copy link
Contributor

Is there a way to do this without inline JavaScript? Like, could we just link to href="" (I can't remember if that works or not) or even do a <form> post?

Reason I ask is that, for security reasons (see: CSP), we may come back and eliminate all inline JavaScript code.

@mattrobenolt
Copy link
Contributor

Oh. Derp. Not sure why I didn’t think of that. Yeah, we can just use a link to “” should be fine to trigger a refresh.

@lauryndbrown
Copy link
Contributor Author

lauryndbrown commented Jan 8, 2018

@mattrobenolt and @benvinegar so my concern with href="" is that it will resort to cached information from the browser and not generate a new code. Is there a way to do that without letting it used saved data?

Looks like it does grab a new code after the last expires. So if you click the link too quickly it will give the same code. But at least on my browser it seems to be ok. Do there happen to be any docs on href="" behavior?

@mattrobenolt
Copy link
Contributor

I don’t think our cache-control headers would allow that to actually happen. If we think this might actually happen for some reason, we could easily just use a cache busting token. Like, append a random querystring value.

@lauryndbrown
Copy link
Contributor Author

Ah! Ok. Gotcha. Sounds like this will work just fine then.

@benvinegar
Copy link
Contributor

benvinegar commented Jan 8, 2018

Aside: just GET-ing a page triggers an SMS? Shouldn't that be a POST or PUT or something else.

@mattrobenolt
Copy link
Contributor

Yeah, when this page is loaded, we send the SMS. This is current behavior, but we can address changing it somehow later if we want.

@benvinegar
Copy link
Contributor

Let's address afterwards. I'd expect a <form> POST to trigger the SMS followed by a redirect to this view (so people can't accidentally refresh). Clicking "resend" should initiate the POST/redirect flow again.

@lauryndbrown
Copy link
Contributor Author

@mattrobenolt is this ok to merge or are there more changes needed?

Copy link
Contributor

@mattrobenolt mattrobenolt left a comment

Choose a reason for hiding this comment

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

🎉

@lauryndbrown lauryndbrown merged commit 52e72f8 into master Jan 9, 2018
@lauryndbrown lauryndbrown deleted the resend-sms-2fa branch January 9, 2018 00:24
@github-actions github-actions bot locked and limited conversation to collaborators Dec 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants