Page MenuHomePhabricator

Profile image from Google not being picked up
Closed, WontfixPublic

Description

Steps to reproduce:

  1. Use a new browser window or log out of Phabricator
  2. Go to the Login or Register page of Phabricator: https://secure.phabricator.com/auth/
  3. Register with a Google account, an account that has a profile pic set with Google.
  4. Note the profile pic Phabricator retrieves from Google's API, both on the 'confirm registration' page and after account creation.

Expected result:
The pic from Google is set as the profile pic in Phabricator

Current result:
Phabricator picks a placeholder pic instead of the Google account's pic.

Event Timeline

pritambaral raised the priority of this task from to Needs Triage.
pritambaral updated the task description. (Show Details)
pritambaral changed the edit policy from "All Users" to "Custom Policy".
pritambaral added a project: Bug Report.
pritambaral added a subscriber: pritambaral.

The Google+ API for a person, available for reference at https://developers.google.com/+/web/api/rest/latest/people#resource, does not agree with what libphutil's PhutilGoogleAuthAdapter.php does currently to extract a profile pic.

A working patch is attached:

PS: The default URL Google's API returns is of a 50x50 pic. The size can be changed by simply changing a URL query parameter. Now, there are two ways of doing that:

  • parse the URL, change the param, build the URL again; but using http_build_url would add a dependency on PHP's pecl-http extension; or
  • Use string substitution. This is risky because there is no guarantee Google will return URLs with the same query parameters in the future.
epriestley triaged this task as Normal priority.
epriestley added a project: Auth.

The UI instructions about navigating the dashboard are also a little out of date since that UI has changed, although I was still mostly able to figure out what they meant. I'll leave a note on T5591 to update them when that gets fixed.

This should be fixed in HEAD. Thanks for the exceptionally clear report, and let us know if you run into anything else.

Thanks for the exceptionally clear report

And thank you for the quick turnaround.

This should be fixed in HEAD.
let us know if you run into anything else.

I'll pull and check for anything odd about the Google auth provider. Or did you mean about general bugs? In that case, of course, I know how to raise an issue.

Minor issue still present. On the "confirm registration" page, the image shown is still a blank one. (Inspecting the element in browser shows its src = PHID-FILE...)

From whatever I remember of the code, I think phabricator calls libphutil's getAccountImageURI() only inside its loginOrCreateAccount() and not before.

I can't reproduce that. Here's what I did:

  • Configured a Google OAuth provider.
  • As a logged out user, clicked "Login or Register with Google" from the login screen.
  • Confirmed the login on the Google side.
  • Ended up here:

The image is correct and the image source is also correct.

I don't see any way in the code that it can possibly render src="PHID-FILE-...", am I misunderstanding?

I don't see any way in the code that it can possibly render src="PHID-FILE-...", am I misunderstanding?

Ah, please ignore my comment about the src, it was misinformed.

I did however just recheck, and I have the same blank image on that page. I'll dig deeper and try to find something more concrete. Do I need to clear some caches or something?

There shouldn't be any caches.

One possibility is that the fetch might be working OK, but generating a thumbnail might not. You could look in FilesAll and see if the full size image is being written correctly. If it is, it might just be a thumbnailing issue.

I haven't been able to trace the cause of the issue, but I do have something to add:

The image is blank because the /file/data/.../profile link the /file/xform/profile/.../PHID-FILE.../ (notice the absence of @local here) url redirects to returns a 404.
And after accepting registration, the phid of the working profile image is different from the phid above.

The @local part is because I have cluster.instance configured locally, it's normal for it to not be present on regular installs.

We're fairly aggressive about re-fetching profile images in order to pick up changes, so the re-fetch is probably also expected.