Page MenuHomePhabricator

Beta Cluster points to maps-beta.wmflabs.org instead of maps.wikimedia.org, which CSP blocks
Closed, ResolvedPublic

Description

Yet another special-case.

Details

Related Gerrit Patches:
mediawiki/extensions/Kartographer : masterAutomatically include map server in CSP sources

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 17 2019, 3:37 PM

I wonder if the CSP should contain any prod domains at all. Maybe commons/upload...

Bawolff added a subscriber: Bawolff.Feb 3 2020, 8:14 AM

I wonder if the CSP should contain any prod domains at all. Maybe commons/upload...

Primary reason for CSP containing prod domains is for people to able to load user-scripts cross-wiki, and for those user-scripts to be able to hit the api cross wiki. In principle extensions that need to access resources should be responsible for adjusting the CSP themselves, and not rely on global config for it.

Change 569526 had a related patch set uploaded (by Brian Wolff; owner: Brian Wolff):
[mediawiki/extensions/Kartographer@master] Automatically include map/icon server in CSP sources

https://gerrit.wikimedia.org/r/569526

I wonder if the CSP should contain any prod domains at all. Maybe commons/upload...

Primary reason for CSP containing prod domains is for people to able to load user-scripts cross-wiki, and for those user-scripts to be able to hit the api cross wiki.

That feels like a really weak justification for so much complexity, though.

I wonder if the CSP should contain any prod domains at all. Maybe commons/upload...

Primary reason for CSP containing prod domains is for people to able to load user-scripts cross-wiki, and for those user-scripts to be able to hit the api cross wiki. In principle extensions that need to access resources should be responsible for adjusting the CSP themselves, and not rely on global config for it.

do we really need to load user scripts from prod though?

I meant, that is the justification for doing it on prod. I assume beta is following suite for no reason other than it defaults to prod config.

Yes, so I think my point stands that our CSP should not contain prod's domains - it should be our equivalent domains.

It is kind of useful to not have equiv domains. It makes it easier to find things like maps which should whitelist itself but doesnt.

I'm not sure I follow. Why would it need prod domains for that? Wouldn't it
be a case of whether the beta maps server is whitelisted?

Sorry. I mean to say- i dont think we should replace the prod csp domains with beta ones, as not having beta ones eases detecting bugs. I would be ok with removing the current prod domains from beta so it has no csp domsins predefined at all.

The downside of having no predefined domains of course is it is another way that prod and beta are different.

not having beta ones eases detecting bugs.

The downside of having no predefined domains of course is it is another way that prod and beta are different.

I'm still confused here. How can it be easier to detect bugs if beta deliberately avoids being equivalent to production?

Krinkle added a subscriber: Krinkle.EditedFeb 4 2020, 9:57 PM

@Bawolff MediaWiki extensions that interact with Maps have their values in wmf-config varied by beta and production. The same way that Beta Cluster has its own RESTBase, Parsoid, Mathoid, upload.wm.o, login.wm.o, etc, it also has its own Maps server.

I can see a use case for allowing the easy testing of gadgets and user scripts that may have prod maps server in them (as users testing these on Beta want to test changes in the gadget, not chanages in Maps). Certainly no reason to disallow that afaik.

But that's secondary to the much more prominent use case of using Beta Cluster for testing MediaWiki itself, which refers maps-beta in beta.

While making the maps URLs lazily added might work for some cases, I don't think that would actually work well in practice given that the server-side will not always know whether Maps is used on a given page. Plus, it isn't where the entry comes from in production currently. That comes from wmf-config. Afaik this is just a simple config bug where we forgot to replace or extend the maps server in wmf-config, similar to what we already do there for the other entries we configure there.

Bawolff added a comment.EditedFeb 10 2020, 5:33 AM

I'm more thinking of the general use case - You can use maps extension outside of Wikimedia (In which case it still calls out to maps.wikimedia.org for the backend). In principle, I think one should be able to enable the CSP config in LocalSettings.php, enable an extension like Maps, and have things just work. (Eventually anyways). I think of the $wgCSPHeader config to be more for user scripts specifically and not for extension controlled stuff. I hope in the very long term, that csp being turned on could be the default config of MediaWiki.

In my mind, whether or not beta includes similar CSP rules as production is a separate issue. Even if it did, I still think that Maps (and similar things) should whitelist the stuff they use. That way there isn't a coupling between the manual CSP config and domains needed to make an extension work. If the domain configured for Maps suddenly changed to something else, everything still works without having to worry about adjusting some totally unrelated config.

In that light, having something like beta that doesn't whitelist all those various domains helps find extensions that aren't whitelisting the stuff they use. I don't see this change as a way to fix beta, just that beta helped find something that should be changed anyway, even if beta used its own csp rules like prod does.

While making the maps URLs lazily added might work for some cases, I don't think that would actually work well in practice given that the server-side will not always know whether Maps is used on a given page. Plus, it isn't where the entry comes from in production currently. That comes from wmf-config. Afaik this is just a simple config bug where we forgot to replace or extend the maps server in wmf-config, similar to what we already do there for the other entries we configure there.

Just to clarify, why wouldn't server side know if a map is used on a page?

We also have to figure out some approach for the other external domains used on wikivoyage T211971#4840328.

So reflecting on this - I still think that Maps should manage its own exceptions. Well it is kind of useful for beta cluster to give errors for anywhere that isn't the case, ultimately the point of beta cluster is to test wikipedia's config, so it should mirror that as closely as possible

Change 569526 merged by jenkins-bot:
[mediawiki/extensions/Kartographer@master] Automatically include map server in CSP sources

https://gerrit.wikimedia.org/r/569526

Krinkle removed a subscriber: Krinkle.Fri, Mar 13, 11:12 PM
Bawolff closed this task as Resolved.Mon, Mar 16, 7:29 AM
Bawolff claimed this task.