Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[server] Improve GitHub Enterprise avatars handling #8825

Merged
merged 2 commits into from
Mar 16, 2022

Conversation

jankeromnes
Copy link
Contributor

@jankeromnes jankeromnes commented Mar 15, 2022

Description

GitHub Enterprise doesn't seem to allow cross-origin requests for user avatars, leading to a broken Gitpod UI.

Also, I initially wanted to fetch the avatars, and encode them as data URIs, however GitHub Enterprise also doesn't seem to have an API to download avatars (only browser authentication via cookies seems to work).

Thankfully, GitHub Enterprise does provide a Gravatar ID that we can use. This Pull Request does just that whenever possible, and also improves the display of avatars that cannot be found or loaded.

Related Issue(s)

Fixes #8755
Partially improves #8815

How to test

  1. Connect with GitHub Enterprise
  2. Open the org selector in /new
  3. The avatars should work for all accounts/orgs that use Gravatar
  4. Add a project and start a prebuild for a repo where the owner does not use Gravatar
  5. In the Project Branches & Prebuilds pages, the broken avatar will show the alt text, but it should be truncated such that the rest of the details are still legible

Release Notes

[server] Improve GitHub Enterprise avatars handling

Documentation

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Mar 15, 2022

Hmm, of course, neither our GHE user nor its org have Gravatars, so that doesn't help 😅

Screenshot 2022-03-15 at 15 14 05

Screenshot 2022-03-15 at 15 13 47

But I guess this is the best we can do. I'll also fix the avatar width in the workspaces list and call it done. 😕

@jankeromnes jankeromnes force-pushed the jx/fix-ghe-org-avatars branch from 4b353d7 to aea5dbb Compare March 15, 2022 14:28
@roboquat roboquat added size/S and removed size/XS labels Mar 15, 2022
@jankeromnes jankeromnes changed the title [server] Prefer Gravatar for GitHub Enterprise owner avatars if available [server] Improve GitHub Enterprise avatars handling Mar 15, 2022
@jankeromnes jankeromnes marked this pull request as ready for review March 15, 2022 14:31
@jankeromnes jankeromnes requested a review from a team March 15, 2022 14:31
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Mar 15, 2022
@jankeromnes jankeromnes requested a review from jldec March 15, 2022 14:32
Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was in the neighborhood and left one comment below.
Thanks for tackling this, @jankeromnes!

accountAvatarUrl: r.owner?.avatar_url,
account: r.owner.login,
accountAvatarUrl: r.owner.gravatar_id
? `https://www.gravatar.com/avatar/${r.owner.gravatar_id}?size=128`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: This is still failing to load the avatar when there's no gravatar set, right?

New project Project branches
Screenshot 2022-03-15 at 8 00 55 PM Screenshot 2022-03-15 at 8 00 43 PM

Copy link
Contributor Author

@jankeromnes jankeromnes Mar 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I really tried to find a way to get the avatar from GitHub Enterprise, but it absolutely refuses to serve the avatar unless you're logged in as a user to GHE with a browser cookie (i.e., no API call can actually serve the image, it only gives you its URL, and the avatar URL refuses any sort of token you throw at it and always redirects to interactive login).

Copy link
Contributor

@gtsiolis gtsiolis Mar 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. Still, grabbing the gravatar is a small improvement step.

account: r.owner.login,
accountAvatarUrl: r.owner.gravatar_id
? `https://www.gravatar.com/avatar/${r.owner.gravatar_id}?size=128`
: r.owner.avatar_url,
Copy link
Contributor

@gtsiolis gtsiolis Mar 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: Posting this here in case it helps. GitHub, incl. GHE, allows you to fetch the user avatar by appending the username with .png suffix, like /username.png. But you have to be logged in for GHE to work. 😞

Copy link
Contributor

@jldec jldec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merci @jankeromnes

@roboquat roboquat merged commit 4058da6 into main Mar 16, 2022
@roboquat roboquat deleted the jx/fix-ghe-org-avatars branch March 16, 2022 18:40
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note size/S team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avatar access restriction causes overlaid text in project branch list.
4 participants