Start handlers for new media endpoints when media resource configured

[devonh]

This is in response to issue #17473.
Not all the necessary handlers to deal with media requests are started now when configuring synapse to use a media worker as per the example config. The new media endpoints introduced with authenticated media fall under the client & federation handlers in synapse.
This PR starts up handlers for the new media endpoints if a worker has been configured with only the media resource type.

Pull Request Checklist

• Pull request is based on the develop branch
• Pull request includes a changelog file. The entry should:
  • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
  • Use markdown where necessary, mostly for code blocks.
  • End with either a period (.) or an exclamation mark (!).
  • Start with a capital letter.
  • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
• Code style is correct
  (run the linters)

[anoadragon453]

Quickly sanity checking, it's not possible to just move the new client and federation servlets into the media repository resource, registering them in register_servlets_for_media_repo?

[anoadragon453]

It looks like you might want a continue in this if statement's body?

[devonh]

Hmm interesting. We could do that. The warning log is mostly there to assist people who didn't realize that the config option is necessary for things to work properly.
If we continue here, it would be very easy to overlook this log line and not realize that the media endpoints weren't actually setup.

I would prefer to keep it as is, since this follows the "fail fast" approach and forces the admin to rectify the situation right then and there.

[anoadragon453]

Looks like I had misinterpreted what this was trying to do this first time around. Failing fast sounds good!

[devonh]

Quickly sanity checking, it's not possible to just move the new client and federation servlets into the media repository resource, registering them in register_servlets_for_media_repo?

It's definitely possible to approach that way. It depends what we want to do.
They are client/federation endpoints so logically it makes sense for them to be under those handlers. The code would be a lot simpler if they were moved to the media resource though.

The approach in this PR follows the precedent for OIDC which follows the same pattern.

[anoadragon453]

The approach in this PR follows the precedent for OIDC which follows the same pattern.

I see. Let's go with that then, and we can refactor later if needed.

[turt2live]

just fyi that this fix would really help all the Synapse self-hosters who updated to the latest Element Desktop today to find that their media doesn't work.

Can this be prioritized for the next scheduled RC?

[airblag]

just fyi that this fix would really help all the Synapse self-hosters who updated to the latest Element Desktop today to find that their media doesn't work.

I was exaclty in this situation, and change my media_worker configuration like this to make it work with Element 1.11.72

worker_app: synapse.app.media_repository
worker_name: media_worker

worker_listeners:
    - type: http
      port: 8180
      x_forwarded: true
      resources:
        - names: [media, client, federation]

worker_log_config: /etc/matrix-synapse/workers/media_worker-log.yaml

[vincejv]

hello i applied the temporary fix that @airblag posted but the SSO Icon is still not showing/broken will this commit also fix the SSO icon in the login page?

The path is /_matrix/media/v3/thumbnail/authelia.com/cKlrTPsGvlpKxAYeHWJsdVHI?width=24&height=24&method=crop&allow_redirect=true

+1 on @turt2live prioritization so we can test on the RC

[vincejv]

The other media like the avatar and sent pictures, seems to work fine however, just the SSO Icon on the login page.

[turt2live]

That looks like an Element Web bug, nothing to do with the server. Looks like the service worker is too eager.

[vincejv]

Actually that screenshot is from Element Desktop with DevTools on. Have zero issues with Element Web 1.11.72 so far..

All media broke including this with 72 minor vsrsion bump..

[anoadragon453]

@turt2live How urgent is it that this land in the next Synapse release? We just put out v1.113.0rc1 yesterday...

[anoadragon453]

Looks like I had misinterpreted what this was trying to do this first time around. Failing fast sounds good!

[turt2live]

@turt2live How urgent is it that this land in the next Synapse release? We just put out v1.113.0rc1 yesterday...

Not critical, I think. Documentation should get people by for a release cycle.