feat: 2FA respect after_update param.

[artsyjian]

https://artsyproduct.atlassian.net/browse/PLATFORM-3340

We are going to redirect partners from CMS to Force to setup their 2FA (if they are required to but haven't yet) like so:

https://www.artsy.net/user/edit?after_update=https%3A%2F%2Fcms.artsy.net%3A%2Fauth%2Fartsy#two-factor-authentication

After they finish setting up 2FA, we would like them redirected to the URL encoded in after_update param (back to https://cms.artsy.net/auth/artsy).

This PR adds a 2FA setup completion modal specifically for these users. On completion, they are informed that they will be redirected to after_update URL.

The URL is sanitized to prevent rogue redirects (such as to www.evilsite.com, ...)

Changes to hokusai/development.yml are to get hokusai dev working.

Screenshots of new modal:

[artsyjian]

I would like to add tests to confirm the redirect is triggered. Are there existing tests I can borrow from?

[damassi]

This is really nice work @artsyjian 💯

Since there's so much new logic, thoughts on adding some tests to cover it?

[joeyAghion]

Great!

Edit: The combination of "after update," "on complete," and "redirect to" labels got a little confusing. Maybe after_update should just become on_complete? I'm open to alternatives too. (Minor.)

[joeyAghion]

Surprising... what if a caller was trying to redirect to the root page?

[artsyjian]

not sure. shall test.

[artsyjian]

tested. if after_update points to /:

http://localhost:5000/user/edit?after_update=%2F#two-factor-authentication

sanitize_redirect returns / which causes afterUpdateRedirect to return empty string, so the old Log back in modal is displayed. I think this is desired behavior?

if after_update is a relative path other than /, say /foo:

http://localhost:5000/user/edit?after_update=%2Ffoo#two-factor-authentication

sanitize_redirect returns /foo which does cause the redirect modal to show which displays no target hostname (b/c /foo is relative), and clicking OK takes one to localhost:5000/foo.

It looks like we should ensure after_update has an absolute URL?

[joeyAghion]

This case is totally fine with the special / meaning. I was just surprised it operated that way generally for artsy-passport's other cases.

Ideally the new after_update param would accept either absolute or relative URLs, but you're right that the modal's copy would need to change in that case. In the case of relative redirects, I'd be fine if it just didn't say the "you'll be redirected..." bit.

[artsyjian]

revised. now, the presence of after_update param always cause the redirect modal to show. if the redir url is absolute, the modal shows You will be redirected to: <host>. if the url is relative (/, /foo, ...), the modal omits You will be redirected... and it will only say You’ve successfully set up two-factor authentication! and clicking OK will still redirect to that relative url.

[joeyAghion]

If I'm following the logic correctly, returning blank here means the if(redirectTo) ... checks above will be false, so the non-redirecting modal will display, right?

[artsyjian]

yes, that's the intent, but..yeah, it's kinda confusing.

[artsyjian]

Since there's so much new logic, thoughts on adding some tests to cover it?
Thanks @damassi. Definitely should include tests. I will holler if I need help.