Code review comment for lp:~jamestait/canonical-identity-provider/lp1648775-email-reminder-of-paper-exhaustion

Revision history for this message
James Tait (jamestait) wrote :

IRC conversation log:

2017-05-08T18:47:23+0100 <@roadmr> jayteeuk: so about the e-mail reminder thing...
2017-05-08T18:48:36+0100 < jayteeuk> roadmr, yeah. What do you think about the AuthToken?
2017-05-08T18:48:40+0100 <@roadmr> jayteeuk: authtokens are tricky because the token by design is lost once the e-mail is sent, so testing them is a bit messy. Would just directing the user to his devices page (instead of an individual link per almost-expired device) obviate the need for an extra token?
2017-05-08T18:51:24+0100 <@roadmr> jayteeuk: example, https://login.ubuntu.com/device-list is *your* device list, so there's nothing secret to tokenize there
2017-05-08T18:51:59+0100 <@roadmr> jayteeuk: if we clearly flag the about-to-expire devices there (may already be done in the other branch?) then we shouldn't need any trickery to protect the link
2017-05-08T18:52:10+0100 < jayteeuk> roadmr, I'm thinking about the case where the user has logged in, taken a device over the reminder threshold, logged out and then seen the email.
2017-05-08T18:52:47+0100 < jayteeuk> roadmr, right - because device-list is just the list for whichever user happens to then login.
2017-05-08T18:53:21+0100 <@roadmr> jayteeuk: I see.. so how about the "safety factor" idea? give them warning when there are n codes left (n > 1)
2017-05-08T18:53:45+0100 < jayteeuk> But are they able to login if their codes are exhausted? The e-mail should only have been sent if they have another set of paper codes they can use, so they ought to have at least one other OTP device.
2017-05-08T18:54:19+0100 < jayteeuk> roadmr, the exhaustion limit is configurable.
2017-05-08T18:54:23+0100 <@roadmr> jayteeuk: ah awesome :)
2017-05-08T18:54:45+0100 <@roadmr> jayteeuk: well if we know by design they have extra codes to use, then just the link should suffice, I guess
2017-05-08T18:54:53+0100 < jayteeuk> So for tests it's 1, but I think it's 3 in production.
2017-05-08T18:58:11+0100 < jayteeuk> roadmr, it's a tricky little area, because we don't want to make it possible to hijack the account, but nor do we want to lock them out of the account in the case where they happen to have two printed lists associated with the account because they lost the first and just created a new one.
2017-05-08T18:59:05+0100 <@roadmr> hm, interesting scenario
2017-05-08T18:59:30+0100 < jayteeuk> Maybe I'm over-thinking it (which is not unusual).
2017-05-08T19:01:07+0100 <@roadmr> hehe :) that's how you catch corner cases
2017-05-08T19:15:49+0100 < jayteeuk> roadmr, so WDYT? Should we go with the AuthToken approach to ensure we keep the account secure without risking locking the user out?
2017-05-08T19:18:11+0100 <@roadmr> jayteeuk: when would you send an authtoken'd link? if I had one of my paper devices close to exhaustion?
2017-05-08T19:18:30+0100 <@roadmr> 1- I use a paper code and go under the threshold (potentially meaning I have no more codes to use)
2017-05-08T19:18:44+0100 <@roadmr> 2- I get a link with which I can go into the account and... what?
2017-05-08T19:19:53+0100 < jayteeuk> roadmr: If the account only has one paper device and that device drops below the threshold (i.e. the user just used it), we force renewal before allowing them to proceed to the RP - that's the previous branch.
2017-05-08T19:20:07+0100 <@roadmr> jayteeuk: ok...
2017-05-08T19:21:08+0100 < jayteeuk> roadmr, If the account has more than one paper device and one drops below the threshold (i.e. the user just used it to login) but at least one other still has codes available, we just email the warning saying "You need to refresh this/these device(s)".
2017-05-08T19:21:35+0100 <@roadmr> aha, but not forcing him to do the renewal...
2017-05-08T19:21:57+0100 <@roadmr> but if the other paper device was lost then the user is effectively screwed...
2017-05-08T19:22:26+0100 < jayteeuk> Right. That's the corner case I'm thinking about.
2017-05-08T19:22:26+0100 <@roadmr> if he doesn't check the e-mail before logging out...
2017-05-08T19:22:55+0100 <@roadmr> but if you send him a link with which he can bypass 2fa to add a new device...
2017-05-08T19:23:02+0100 < jayteeuk> I can imagine users just creating a new printed list, rather than regenerating an existing one, for example.
2017-05-08T19:23:12+0100 <@roadmr> and someone has already compromised the account... (see now I'm finding a corner to the corner)
2017-05-08T19:23:21+0100 < jayteeuk> Right...
2017-05-08T19:23:23+0100 < jayteeuk> Inception!
2017-05-08T19:23:28+0100 <@roadmr> jayteeuk: in the above scenario the authtoken'd link would be counter productive :(
2017-05-08T19:23:40+0100 < jayteeuk> Damn, security is hard. 😝
2017-05-08T19:23:51+0100 <@roadmr> jayteeuk: funny you should mention the very scenario this [SSO 2FA recovery via SMS] spec is meant to solve: [redacted]
2017-05-08T19:24:22+0100 < jayteeuk> Wow.
2017-05-08T19:24:51+0100 <@roadmr> jayteeuk: it's only in the design phase at this point...
2017-05-08T19:26:22+0100 < jayteeuk> OK, so at this point at least I'm happier that I'm not just over-thinking it.
2017-05-08T19:26:34+0100 <@roadmr> jayteeuk: but for your scenario, I would say someone who exhausted his paper devices should go to the usual "I got locked out" procedure...
2017-05-08T19:26:46+0100 <@roadmr> jayteeuk: which right now involves going to IS, and later will use this SMS recovery thing
2017-05-08T19:27:09+0100 <@roadmr> (which *still* has a possibility of entire lockout - but between the paper code warning stuff and SMS recovery, it should be reduced to infinitesimum)
2017-05-08T19:29:08+0100 < jayteeuk> OK. So I think that means go with what we have, though probably still pass the email into `send_printed_codes...` rather than blow up if the account has no preferredemail.

« Back to merge proposal