MSC4148: Permitting HTTP(S) URLs for SSO IdP icons

[turt2live]

Rendered

In line with matrix-org/matrix-spec#1700, the following disclosure applies:

I am Director of Standards Development at The Matrix.org Foundation C.I.C., Matrix Spec Core Team (SCT) member, employed by Element, and operate the t2bot.io service. This proposal is written and published as a Trust & Safety team member allocated in full to the Foundation.

FCP tickyboxes

[turt2live]

Implementation requirements:

• None required, in my opinion. This MSC is logically possible to implement considering a client's use of the media download and thumbnail APIs.

[uhoreg]

I don't want to bikeshed too much on an MSC that's only meant as a temporary measure, but couldn't we stick the HTTP(S) URI in a different property, maybe something stupid like icon_location? This would allow both old and new clients to work properly.

[turt2live]

I'm not sure it'd materially improve the experience, honestly. Clients which don't update will still get broken icons.

[deepbluev7]

I concur that this change will break clients. Clients currently expect mxc urls and actively prevent loading https urls. Changing the same property to https will break existing clients and prevent even the fallback to allow loading old media using the old endpoints. Meanwhile I see no goo reason not to use a different property name for these urls. Clients which don't update won't get broken icons immediately without this MSC. With this MSC they will. And servers can even provide a quality of implementation "feature" and allow uploading media for unauthenticated access for the idp urls. So this should really be a separate property.

[turt2live]

Using a separate property would lead to clients not showing avatars, which it sounds like would also happen on clients not prepared for non-mxc URLs in the existing field. While it's a technically breaking change, the effective behaviour is the same regardless of the approach.

Note: clients which crash because of non-mxc URLs being present should be fixed regardless of this MSC's direction.

[clokep]

I think the idea is if you provide both it allows clients to migrate more gracefully by a server giving both an MXC and HTTPS URL?

[dbkr]

This thread should probably be resolved one way or the other, so I'll add as concern for it. For what it's worth, having it as a separate field seems sensible to me because servers can send both as a transition period. Since the HTTP(S) icon will only support a single size, you could even call it icon_64.

[uhoreg]

My preference would be a separate field, but I don't think it's a huge issue either way, given it's a temporary measure, and just aesthetic.

[richvdh]

If we use a separate field here, I wonder what the implementation would look like. Currently, Synapse passes the idp_icon setting from its config straight though (see doc, which says "must be an MXC URI"). To take advantage of separate fields here, we'd need to support separate fields in Synapse's config, which seems like it is complicating matters for the average Synapse admin.

I also see @turt2live's point that it probably won't help much in practice. Either: your server allows the mxc: uri, in which case send that; else it doesn't, in which case what's the point of a separate field. The only reason to send both is to help you keep track of the number of old clients that aren't honoring the new field (and thus presumably don't support https: uris).

IMHO the downsides of a separate field outweigh the upsides.

-0.5.

[KitsuneRal]

An extra field means extra maintenance, on both (client and server) sides. Given that it's mainly an aesthetic issue, I'm fine with temporary limited breakage as long as we get straight to the to-be state. We have more than enough legacy cruft in the protocol already, let's not add to it.

[turt2live]

In theory OIDC is expected this year, but I'm not tracking that work. @hughns do you have insight here?

[clokep]

This seems reasonable to me.

@mscbot fcp merge

[mscbot]

Team member @mscbot has proposed to merge this. The next step is review by the rest of the tagged people:

• @clokep
• @dbkr
• @uhoreg
• @turt2live
• @ara4n
• @anoadragon453
• @richvdh
• @tulir
• @erikjohnston
• @KitsuneRal

Concerns:

• decide on rename vs separate field
• drop the overspecified 64x64 resolution requirement

Once at least 75% of reviewers approve (and there are no outstanding concerns), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for information about what commands tagged team members can give me.

[dbkr]

@mscbot concern decide on rename vs separate field

[erikjohnston]

It's probably worth noting that mxc URLs also give the ability for clients to request different sized thumbnails, and this MSC effectively hard codes the quality of the thumbnail to be used.

[ara4n]

bleurgh. it seems crazy to mandate 64x64 for an icon - that's tiny. macOS icons are 512x512 for instance (or 1024x1024 at retina). can we just delete this in favour of "Icons should be high enough resolution to render pleasantly on a wide variety of clients" or just leave it unspecified.

[ara4n]

(other than that, i'm fine with the msc tho)

[ara4n]

@mscbot concern drop the overspecified 64x64 resolution requirement