Avatars (or all images?) should go through proxy/authentication and be cached

[shiftkey]

No description provided.

[joshaber]

They should also be authenticated for Enterprise instances.

[joshaber]

I'm thinking we could treat this as a stretch goal for M4. I think it should definitely happen before 1.0, but as-is we're on-par with Classic.

Thoughts?

[joshaber]

For posterity, @niik and I talked about this a bit and it should probably be implemented as a service worker.

[joshaber]

From https://developer.mozilla.org/en-US/docs/Web/API/Service_Worker_API:

Service workers only run over HTTPS, for security reasons.

I don't feel like I have enough context to know, but could this be a problem for Enterprise customers?

[shiftkey]

I don't feel like I have enough context to know, but could this be a problem for Enterprise customers?

Technically you can run GHE over HTTP despite it being highly discouraged for all sorts of reasons - there's probably some old threads about it we can find to see how badly this will break for them. These days it's mostly the self-signed certificate hassles (e.g. trialing GHE) that we'd need to handle, and if we're adding support for adding self-signed certificates in the app then that should be covered.

[joshaber]

And I suppose worst case it's just avatars that break.

[shiftkey]

I guess if we can serve up a placeholder image in case of failure then it's okay?

[joshaber]

It looks like this may not be possible right now: https://github.com/github/github/issues/71916

I'll wait and see what response that issue gets, but I imagine we'll have to punt this.

[joshaber]

Based on the initial response, it sounds like this is currently not possible. Avatars won't authenticate with a token.

I'll keep this open, but I'm 👢'ing it from M4.

[NickCraver]

Just hit missing avatars on enterprise instances. I manually authenticated inside electron via window.location = '<url>'; and did 2-factor auth...now I have avatars.

It's a crappy workaround, but visually this is the most visible missing piece in the current UI for enterprise customers, IMO. Aside from that, I'm liking this thing :)

[shiftkey]

I manually authenticated inside electron via window.location = ''; and did 2-factor auth...now I have avatars.

Yeah, inside the renderer process we do have the ability to assign cookies from a valid GitHub session, which would explain how that workaround succeeds, but we've avoided doing the entire auth process completely in there (because security?).

Aside from that, I'm liking this thing :)

💖

[niik]

I manually authenticated inside electron via window.location = ''; and did 2-factor auth...now I have avatars.

[alrz]

I guess if we can serve up a placeholder image in case of failure then it's okay?

Please!

[hparadiz]

This is still an issue. It's a BUG!!!! Not an "enhancement".

Eight months since the issue was reported and still nothing?

[shiftkey]

@hparadiz are you connecting to GitHub or a GitHub Enterprise instance?

[hparadiz]

@shiftkey Enterprise

[shiftkey]

@hparadiz cool.

So an update on this - even with providing a valid token, there's situations where Enterprise won't return the avatar to Desktop. We're talking with the Enterprise team to get this fixed, which will then take time to reach customers in an update. Apologies for the delay.

[icarnaghan]

Subscribing - I have the same issue.

[sergeyspatar]

Version 1.0.4 still has this bug. Avatars from GHE are not displayed.

[joshaber]

Yes, this is currently blocked by some required work on GitHub Enterprise.

[shiftkey]

Bumping this thread as we've not had much movement on this from our end - a few people have suggested re-implementing Gravatar support as a fallback that we had in the classic apps.

@desktop/core unless there's any objections to this I'll throw it on my plate this week to estimate the work involved.

[haacked]

It may seem cosmetic, but displaying a broken image icon is worse than even just showing a placeholder image. It reduces confidence in the application. So I'm 👍 to doing something here.

The gravatar fallback seems like a good start. Gravatar itself has a fallback mechanism. So the fallback sequence would be:

1. GHE avatar
2. Gravatar
3. Placeholder

We should never show a broken image here.

[haacked]

We should never show a broken image here.

Now that I've said that, say hello to my upcoming new profile image.

[ausiddiqui]

on 1.0.9 and the broken image is now a default robot in the commit summary / description box area (for a logged in Enterprise repo), but still a broken image under Preferences > Accounts.

[icarnaghan]

I'm seeing the robot too, but I'm also seeing broken avatar images in my history list.

[say25]

@shiftkey I know the current solution is to use Gravatar for fallback, do you know if the GitHub enterprise team will be able to do the work required to not require Gravatar fall back in the future?

[shiftkey]

@say25 it's an ongoing discussion with the team, and even after a fix lands it would require GitHub Enterprise administrators to update their instances.

I've made a big dent into this in #3513, so please follow along with that PR and we'll see if we can make this experience better in the short-term.

[keithy]

Still not fixed