I think I have this working fairly reasonably, now. There are four major cases:
- Queries
- All Stories
- Search
- Advanced Search
- Transactions
- Transaction Logs
All Stories
Jan 25 2019
In T13231#243710, @epriestley wrote:Another issue here is that Duo doesn't seem to have a way to prevent new user creation. YIf you /preauth a user, they get an enroll link whether they already have an account in Duo or not, and there's no apparent way to distinguish between "this creates a new user" and "this encourages an existing user, who has already been created but has not enrolled a device yet, to enroll".
Another issue is that we apparently (???) only get one shot at learning the user's (internal-to-Duo) User ID, and only if we're allowed to enroll them. The first call to /enroll gives us a user ID, subsequent calls do not and there's no other way to do the lookup. This isn't really a problem, it just means we're forced to use usernames everywhere (which are mutable/aliasable/etc/etc) when we could otherwise use nice stable user IDs.
Jan 24 2019
Another issue here is that Duo doesn't seem to have a way to prevent new user creation. If you /preauth a user, they get an enroll link whether they already have an account in Duo or not, and there's no apparent way to distinguish between "this creates a new user" and "this encourages an existing user, who has already been created but has not enrolled a device yet, to enroll".
Thanks, that's quite helpful!
If we want to synchronize to existing accounts, I think it's not good enough for us to pick the user's email address unless that's also the username the organization already uses -- in your case, is it?
I'm imaging that one use case is that you just want everyone to use MFA, and Duo is a more attractive approach than TOTP (since it's easier to use) or SMS (since it's more secure). In this case, you might just be using Duo as "Better TOTP", and the enrollment/management side isn't as important. For example, if we deployed Duo on secure or in the Phacility cluster as a default option, we'd use it in this mode.
FWIW - most users of this feature (especially after T13229) will be organizations that already have DUO in their environment, and likely already have users defined in DUO. Creating random usernames, or PHID's is probably not going to work for most of those organizations. Phabricator does require email's to be unique (no reuse) so that might be a better choice for initial userid setting?
When you delete a Duo factor from your phone app, you still get prompted to "auth", Duo just reports that you have no enrolled devices which support push.
Not exactly related, but PEAR got compromised: https://news.ycombinator.com/item?id=18987518
Striping should probably "revoke" (disable + can not be re-enabled), vs the user "remove" button becoming a "disable" action.
Duo wants us to <img src="..." /> a QR code.
With a 15-second TTL, the user doesn't vanish or anything, and Duo does not let us do another enroll attempt even after the enroll time expires:
If you try to /enroll the same user PHID twice, the second /enroll fails:
Rolling this mess up again:
Jan 23 2019
I think this is more or less at the end of the line in terms of actionable work that doesn't make more sense being scoped in elsewhere.
This will probably roll forward into a "next iteration", but it would be nice-to-have to show users the numbers we expect to text them from, so they can whitelist/anticipate the messages.
managing contact numbers
Also, managing contact numbers needs to require MFA or evading SMS MFA is trivial. 🐈
Currently, I think attackers can CSRF the MFA enroll endpoint (/mfa/new/?provider=xyz). This could send you an SMS, soon. This is mostly harmless but potentially a little annoying with push factors like SMS/Duo. I'll get a CSRF gate on it.
Also, I think we still have an issue with rate limiting where:
So I don't forget about it: the default settings/ UI is now incorrectly showing Conpherence settings, not date and time settings. Clicking anything works properly.
Jan 21 2019
The need for more UI support on the "MFA Required" flow is making me consider expanding scope here into knocking "Settings" back out to a full-width UI. I think that's not terribly hard. I'm maybe going to start there so it can get more testing as I go through everything else, and I can give up if it's harder than I think.
From D19975, two considerations:
T13227 also needs to happen this week.
Thanks! I get the same behavior locally, I filed this upstream: https://bugs.php.net/bug.php?id=77496
I can't get MYSQLI_OPT_LOCAL_INFILE to work on secure, either. I tried on secure001 and secure004 (where the database is not local). As far as I can tell, this option doesn't do anything, anywhere, ever?
I believe D19969 has now fixed this, although I'm not entirely certain, since it was never reliably reproducible in production so there's no way to really test or verify it.
Jan 20 2019
We're probably done here, but ideally the next steps are:
Reproduction steps are:
Jan 19 2019
D19968 got us closer here, but the link targets aren't actually rendering properly.
The fix for T12921: Link to referenced object in transaction emails (in D19968) is a bit buggy -- the links aren't generating as absolute links with a protocol and domain name. This is likely an easy fix, although I don't expect to get to it tonight.
Jan 18 2019
No one seemed to hate this. I'm not sure it's seeing too much use, but we could grep some access logs eventually.
Nothing really actionable here.
Maybe another point in favor of this claim is that the option does not work is the behavior of this:
I think that maybe mysql_nonapi.c just overrides the conn->options() call? Near line 269 of PHP 7.2.3:
I can't get MYSQLI_OPT_LOCAL_INFILE to work on secure, either. I tried on secure001 and secure004 (where the database is not local). As far as I can tell, this option doesn't do anything, anywhere, ever? I'm going to look at the source and see if I can figure out what's going on, but I'll back it out of D19998 if I can't find some evidence that it's useful.
I'm unable to get the MySQLi option MYSQLI_OPT_LOCAL_INFILE to actually work. Here's the script I'm using:
It looks like we don't need to do anything about mysql on the CLI since this option is, thankfully, not enabled by default:
Jan 17 2019
I ran a query against a subset of instances to see if any instances had configured metamta.*.subject-prefix options.
When the time comes, this should almost certainly also handle contact numbers for non-email media (T920 / D19988).
Hey guys,
Jan 16 2019
D19955 has how I think this likely breaks down, I'm not planning to touch it in this iteration since the badness, while bad, is mostly in a box (hopefully) until the next PHPMailer CVE. All yours!
When I was messing around with SMS sending last week, I put a rough cut of this together that is capable of sending an SMS using SNS, which has almost the same API as SES: D19982. I can tackle this unless you're already in the middle of it.
SMS MFA