Page MenuHomePhabricator

Google auth fails and throws exception about missing client state parameter
Closed, ResolvedPublic

Description

Google auth is broken on current stable. Best guess is due to recent PhutilURI changes in rPHU.

Looking at network requests, I see:

  1. https://phabricator.[CENSORED].com/ 302 GET

Location: https://accounts.google.com/o/oauth2/auth?client_id=[CENSORED].apps.googleusercontent.com&scope=email%20profile&redirect_uri=https%3A%2F%2Fphabricator.[CENSORED].com%2Foauth%2Fgoogle%2Flogin%2F&state=[CENSORED]&response_type=code

  1. https://accounts.google.com/o/oauth2/auth?client_id=[CENSORED].apps.googleusercontent.com&scope=email%20profile&redirect_uri=https%3A%2F%2Fphabricator.[CENSORED].com%2Foauth%2Fgoogle%2Flogin%2F&state=[CENSORED]&response_type=code 302 GET

Location: https://phabricator.[CENSORED].com/oauth/google/login/?state=[CENSORED]&code=[CENSORED]&scope=email+profile+https://www.googleapis.com/auth/userinfo.profile+https://www.googleapis.com/auth/userinfo.email

  1. https://phabricator.[CENSORED].com/oauth/google/login/?state=[CENSORED]&code=[CENSORED]&scope=email+profile+https://www.googleapis.com/auth/userinfo.profile+https://www.googleapis.com/auth/userinfo.email 302 GET

Location: https://phabricator.[CENSORED].com/auth/login/google:google.com/

  1. https://phabricator.[CENSORED].com/auth/login/google:google.com/ 500 GET

Unhandled Exception ("Exception")
The authentication provider did not return a client state parameter in its response, but one was expected. If this problem persists, you may need to clear your cookies.

Depth Library File Where
5 phabricator applications/auth/provider/PhabricatorOAuth2AuthProvider.php : 64 PhabricatorAuthProvider::verifyAuthCSRFCode()
4 phabricator applications/auth/controller/PhabricatorAuthLoginController.php : 42 PhabricatorOAuth2AuthProvider::processLoginRequest()
3 phabricator aphront/configuration/AphrontApplicationConfiguration.php : 286 PhabricatorAuthLoginController::handleRequest()
2 phabricator aphront/configuration/AphrontApplicationConfiguration.php : 209 AphrontApplicationConfiguration::processRequest()
1 webroot/index.php : 35 AphrontApplicationConfiguration::runHTTPRequest()

Version information from https://phabricator.example.com/config/version/:

Event Timeline

I thought rPHU4d21f105b17f250aaaeb8937142ae4cbc978389f was meant to fix this, but apparently not.

I can't immediately reproduce this: I can log into my local development install and into secure.phabricator.com using Google OAuth without any issues right now.

Google OAuth is also fairly popular (including on admin.phacility.com) and I'd expect we'd be receiving many reports if it was broken in general. admin.phacility.com shows what looks like a normal rate of successful Google auth logins.

(This is not likely to be related to the API changes to PhutilURI.)

The redirect from (3) to (4) is slightly suspicious.

If you visit http://.../oauth/google/login/?a=b, you should be redirected to http://.../auth/login/google:google.com/?a=b, i.e. with request parameters preserved.

If you are not redirected with the parameters preserved, the culprit might be D20147, which changed how we read request parameters from using $_GET to using $_SERVER['REQUEST_URI'] so that we can faithfully read URIs like /x/?a=1&a=2, where a parameter is repeated. I would normally expect both values to be populated and agree, except that REQUEST_URI preserves a=1&a=2 while $_GET will not.

You can test this by by writing this file to phabricator/support/debug.php:

phabricator/support/debug.php
<?php

var_dump(
  array(
    'GET' => $_GET,
    'REQUEST_URI' => $_SERVER['REQUEST_URI'],
  ));

Then, visit https://.../debug/?a=b. Expected output is:

array(2) {
  ["GET"]=>
  array(2) {
    ["__path__"]=>
    string(7) "/debug/"
    ["a"]=>
    string(1) "b"
  }
  ["REQUEST_URI"]=>
  string(11) "/debug/?a=b"
}

If your output does not match (particularly, REQUEST_URI is empty or missing parameters), there's a problem with constructing REQUEST_URI.

A possible source of the problem is that nginx may require fastcgi_param REQUEST_URI $request_uri;. It appears I may have overlooked that when implementing D20147. If adding that line to your nginx configuration fixes the problem, I'll update the documentation and add a setup warning, or switch from REQUEST_URI to QUERY_STRING (which we already document should be passed) and add a setup warning for that.

https://phabricator.example.com/debug/?a=b returns:

array(2) {
  ["GET"]=>
  array(2) {
    ["__path__"]=>
    string(7) "/debug/"
    ["a"]=>
    string(1) "b"
  }
  ["REQUEST_URI"]=>
  NULL
}

After adding fastcgi_param REQUEST_URI $request_uri; to the nginx vhost config, I now see the following:

array(2) {
  ["GET"]=>
  array(2) {
    ["__path__"]=>
    string(7) "/debug/"
    ["a"]=>
    string(1) "b"
  }
  ["REQUEST_URI"]=>
  string(11) "/debug/?a=b"
}

With that change, can confirm Google OAuth now works!

Great! Thanks for the report, and for your help tracking this down by going through the diagnostic steps.

I'll update the documentation/setup guidance in the upstream.

(You can delete support/debug.php if you haven't already.)

epriestley triaged this task as Normal priority.

I'll update the documentation/setup guidance in the upstream.

D20227 should fix this. It just switches us to use QUERY_STRING instead of REQUEST_URI. We can use either QUERY_STRING or REQUEST_URI to get the information we need, but QUERY_STRING already has documentation and a setup check.