This allows weird mixes of client_secret + AUTH_USER -- let's do something like this instead (provided the code actually works)?

$client_phid = $request->getStr('client_id');
$client_secret = $request->getStr('client_secret');
if (!strlen($client_phid) && !strlen($client_secret)) {
  $client_phid = idx($_SERVER, 'PHP_AUTH_USER');
  $client_secret = idx($_SERVER, 'PHP_AUTH_PW');
}

That is:

• If either parameter is present, use the HTTP parameters only. This makes requests with an "obvious" client_secret or client_id always do the more obvious thing, so no one will ever report a bug where they run curl ...&client_secret=x and get some unrelated/bizarre failure because they have a weird ~/.curlrc or something.
• Require both parameters to come from a single source, so callers can't mix things, which I think would just be weird/confusing/unhelpful in all cases.

I explicitly want to prefer the parameters, not the auth header.

A scenario I can imagine where both are present might be something like this:

user: "I ran curl http://example.phabricator.com/api/whatever?client_id=x&client_secret=y but it says my client_id and client_secret are invalid! I'm sure they're right! Help! Your software is super broken!"
me: <I spend hours debugging>
me: Turns out you configured your environment to send an "Authorization" header and didn't tell me about it. Delete that config.
user: wow that worked
me: ...

This specific scenario may never occur, but I can't imagine any scenario where both are present and the "Authorization" header is explicit while the parameters are implicit. In contrast, I can easily imagine scenarios were the "Authorization" header is implicit (via a mechanism like ~/.netrc). Preferring parameters guarantees I'll never end up debugging this kind of config mess.

The use of strlen() is also important. In PHP, the string "0" is falsey:

$ php -r 'echo "0" ? "truthy\n" : "falsey\n";'
falsey

This means that ?client_id=0&client_secret=0 will be incorrectly ignored without the strlen() tests. Although these are probably not valid IDs/secrets, they can lead to a misleading error message (e.g., "client_secret not present", when the correct error is "client_secret present but not valid").

(Similar issues have occurred in the wild in the past with user @0, who is a real user, also has user account https://github.com/0, and has contributed to Phabricator.)

If the Go API sends client_id as both a request parameter and an "Authorization" header, we should require that they agree. Something in this vein:

$client_id_parameter = $request->getStr('client_id');
$client_id_header = idx($_SERVER, 'PHP_AUTH_USER');
if (strlen($client_id_parameter) && strlen($client_id_header)) {
  if ($client_id_parameter !== $client_id_header) {
    throw new Exception(
      pht(
        'Request included a client_id parameter and an "Authorization" header with '.
        'a username, but the values ("%s" and "%s") disagree. The values must match.',
        $client_id_parameter,
        $client_id_header));
  }
}

$client_secret_parameter = $request->getStr('client_secret');
$client_secret_header = idx($_SERVER, 'PHP_AUTH_PW');
if (strlen($client_secret_parameter)) {
  // If the `client_secret` parameter is present, prefer parameters.
  $client_id = $client_id_parameter;
  $client_secret = $client_secret_parameter;
} else {
  // Otherwise, read values from the "Authorization"  header.
  $client_id = $client_id_header;
  $client_secret = $client_secret_header;
}