mixpanel / mixpanel-js

Official Mixpanel JavaScript Client Library
https://mixpanel.com/help/reference/javascript
Other
886 stars 312 forks source link

2.31 is a breaking change with CSP headers #234

Open DrPhil opened 4 years ago

DrPhil commented 4 years ago

If you have specified a restrictive CSP then 2.31 is a breaking change.

VM105:1 Refused to connect to 'https://api-js.mixpanel.com/decide/?verbose=1&version=1&lib=web&token=redacted' because it violates the following Content Security Policy directive: "connect-src 'self' api.mixpanel.com".

Consider either releasing guidelines for using CSP with Mixpanel or re-release 2.31 as version 3.0.0.

shnere commented 4 years ago

@tdumitrescu please be more careful with releases, both 2.31 and 2.32 contain breaking changes and should have been a major version bump.

tdumitrescu commented 4 years ago

@DrPhil @shnere we're very sorry for any inconvenience this may have caused you. We're curious to know what breaking change affected you in 2.32.0.

By necessity we have to be extremely sparing with major version bumps, and we make an effort not to introduce any breaking changes in the 2.x line. I suspect the problem arising in this issue is around the definition of a breaking change. With regard to 2.31.0: we did change one of our default API hostnames, and while this would affect you if you had set up a CSP restricting requests to the old hostname, this hostname was also never considered part of the library's public API. We absolutely should have communicated that upcoming change proactively to users, and that's on us, but there are a lot of internal elements of the library that we need to be able to update without considering every behavior change a "breaking" change. This includes Mixpanel's HTTP API surface area (IP addresses and hostnames), as well as request-sending methods (i.e. the recent default change from GET to POST, which saw tracking data move from URL params to the request body; this library never made any guarantees about how it's going to send out requests, and you shouldn't be expecting that to remain unchanged - it's an implementation detail and not part of a public API). What we consider the public API is documented at https://developer.mixpanel.com/docs/javascript-full-api-reference.

You don't have to agree with this stance - there are a lot of ways to play the versioning game, and some projects are much more aggressive than others about jumping versions - but I want to make sure people are at least aware of our approach and to know that we're not cavalier about versioning. We make every effort to ensure that any new 2.x release will work seamlessly without any changes at all to existing site implementations. There are further updates in the works for early 2020 which will change elements of the library that are not considered public and shouldn't be relied on - e.g., the number of events sent out per request, and request-retry mechanisms. If you feel uncomfortable automatically upgrading when these types of library updates occur, depending on your integration type there are ways to pin the library to a specific version, and then you can choose exactly when to upgrade explicitly.

tdumitrescu commented 4 years ago

Oh and @DrPhil: great idea about publishing CSP guidelines for Mixpanel users. We'll get working on it.

shnere commented 4 years ago

@tdumitrescu thank you for your response. 2.31 broke us because we didn't have https://api-js.mixpanel.com in our CSP. In the case of 2.32, it broke because we are proxying calls done to the mixpanel API through our backend by changing the api_host variable, which was assuming calls to the API were done using GET and we didn't have a POST handler on our side. This was solved in two separate ways, in one case we simply added a POST handler on our backend and in the other we switched back the api_method to use GET as documented in the release, since it wasn't straight forward to add a POST handler. I understand this may not be the correct or even supported/expected way to use this library, just a series of unfortunate events that ended up breaking our mixpanel integration.

Is there a way to pin the library to a specific version using the CDN? //cdn.mxpnl.com/libs/mixpanel-2-latest.min.js

Thanks!

tdumitrescu commented 4 years ago

So sorry for the disturbance there. If you're using the script tag to load the library asynchronously, you can change the URL to https://cdn.mxpnl.com/libs/mixpanel-2.32.0.min.js to pin it to 2.32.0.

If you don't want to mess with the contents of our JS snippet in the script tag, you can also set the URL via a global var as long as it's set before the snippet runs:

window.MIXPANEL_CUSTOM_LIB_URL = 'https://cdn.mxpnl.com/libs/mixpanel-2.32.0.min.js';
davidbielik commented 4 years ago

great idea about publishing CSP guidelines for Mixpanel users. We'll get working on it.

Hi @tdumitrescu are Mixpanel's required CSP headers documented anywhere? I couldn't find them in the JS docs, security docs, or a google search. Thanks!

rstudner commented 3 years ago

Quick comment.

I'm running v2.39

I do not configure api_host anywhere.

I have one environment I just noticed hitting this new URL. My other enviroment running v2.39, is still hitting api.mixpanel.com.

That's weird.

shaialon commented 3 years ago

By popular demand on this thread, adding a link to unofficial Mixpanel CSP configuration on RapidSec, that is manually curated from many connected site. Hope this helps.

tdumitrescu commented 3 years ago

Thanks @shaialon, what's the mechanism for submitting corrections to that config? It looks like there's a bunch of cruft in there which can be removed for safety.

shaialon commented 3 years ago

@tdumitrescu we intend to open source it soon enough. Until then you can send any suggested changes to support@rapidsec.com, or otherwise comment in this thread.

EDIT: CONTENT BELOW REMOVED

tdumitrescu commented 3 years ago

Thanks for the info @shaialon, we'll do a closer review and get back to you, but before then I would strongly recommend removing all the entries for cdn-mxpnl.com and cr-input.mxpnl.net. These domains are NOT associated with Mixpanel and are likely to be phishing sites (the IP addresses for cdn-mxpnl.com are also associated with a long list of deceptive domains like "cdn-js.net").

shaialon commented 3 years ago

Thanks @tdumitrescu , apologies for the inconvenience. Removed them from our records and and will deploy the changes soon. We have a filter for malicous domains - I don't know how this one got through (will check).

P.S. You may want to report mxpnl.net: https://www.virustotal.com/gui/domain/mxpnl.net/detection

shaialon commented 3 years ago

Updating that the package is now updated: https://rapidsec.com/csp-packages/mixpanel

MH4GF commented 9 months ago

I had to configure CSP to send events to mixpanel, but it worked with "connect-src api-js.mixpanel.com", so I share it. It should be better if it is mentioned in the documentation, so please consider it!