Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Raven doesn't work in Chrome App #157

Closed
fanzeyi opened this issue Nov 12, 2013 · 22 comments
Closed

Raven doesn't work in Chrome App #157

fanzeyi opened this issue Nov 12, 2013 · 22 comments

Comments

@fanzeyi
Copy link

fanzeyi commented Nov 12, 2013

Chrome blocked the image requesting in Chrome App for improving security. So the function makeRequest doesn't work in Chrome App.

I patched raven-js for using it in Chrome App here

But there are two problems with my patch:

  1. HTTP GET requesting doesn't have an Origin or a Referer header. So I have to use client secret here to pass the authorization check.
  2. Sentry doesn't allow to set Allowed domain into Chrome App URL like chrome-extension://*/. So I have to use a single wildcard here.

Any better solutions? Thanks!

@mattrobenolt
Copy link
Contributor

Awesome. So step one, I'm going to add support for adding chrome-extension:// as an acceptable url in Sentry.

Then I'm going to look into the issues here. Do you have an example app that I can check out that fails?

@mattrobenolt
Copy link
Contributor

When publishing your chrome app, do you know the app id? Like, could you use: chrome-extension://{{ my id }}/ instead of a wildcard? Just trying to figure out how I should patch this into Sentry.

@fanzeyi
Copy link
Author

fanzeyi commented Nov 12, 2013

@mattrobenolt Of source I can know the app id, and it would not change. ^^

Thank you.

@mattrobenolt
Copy link
Contributor

If Sentry validates the origin header correctly for the chrome-extension protocol, will that resolve the other issue with CSP and not need to make an XHR request? Or will that still be an issue?

@fanzeyi
Copy link
Author

fanzeyi commented Nov 12, 2013

Nope. It's still an issue.

Chrome's CSP doesn't allow to write something like this:

img = new Image();
img.src = "http://example.com";

@mattrobenolt
Copy link
Contributor

Interesting. I’ll loop in some others that understand CSP far more than I do and see what they have to say.

Matt Robenolt
@mattrobenolt

On Tuesday, November 12, 2013 at 8:49 AM, Zeray Rice wrote:

Nope. It's still an issue.
Chrome's CSP doesn't allow to write something like this:
img = new Image(); img.src = "http://example.com";


Reply to this email directly or view it on GitHub (#157 (comment)).

@fanzeyi
Copy link
Author

fanzeyi commented Nov 12, 2013

I wrote an example here

@mattrobenolt
Copy link
Contributor

Oh, nice! Thank you! I can’t guarantee when this will be resolved, but it’s definitely on our mind with a bunch of things around extensions as well. So this will be good information to use to solve it all at once. :)

Matt Robenolt
@mattrobenolt

On Tuesday, November 12, 2013 at 8:55 AM, Zeray Rice wrote:

I wrote an example here (https://github.com/fanzeyi/raven-example-app)


Reply to this email directly or view it on GitHub (#157 (comment)).

@frekw
Copy link

frekw commented Jan 21, 2014

I'm having a very similar problem as well.

I'm trying to get Raven (running on the hosted version of Sentry) working in an application running Chromium Embedded Framework where apps are run as file:// URI:s (but with a custom URI scheme).

The image approach won't work because Chromium won't set Origin or Referer header for GET requests when running from file://.

I tried to patch makeRequest to honour an useXHR option using X-Sentry-Auth headers for validation, but it blows up because of sentry_secret missing (which seems to be the same issue @Shyru was having with PhoneGap; #29). Relevant gist can be found here: https://gist.github.com/frekw/8540890

I think an option to disable validation for posting log entries in the backend on a per project basis would be the way to go.

While it would be possible to patch parseDSN to allow sentry_secret is useXHR is set, leveraging the sentry_secret for auth in raven-js seems like a bad idea since many of these sandboxed apps (e.g Chrome extensions or the type of app I'm working on) can easily be disassembled. They are usually zip-files containing the static assets, but with a custom extension (e.g .crx instead of .zip).

I'll be happy to give any assistance I can to get this working, because me and my team would really like to use Sentry for error reporting.

@mattrobenolt
Copy link
Contributor

The first thing I'll address is auth: at that point, if you're going to be POSTing data and the file is packaged in an extension, you might as well just use the private key. I'd argue that it's less of a hack than allowing an option to disable authentication on the server since ultimately the key can be revoked if there was abuse. Our secret/private key isn't really that private. The only thing that can be done with it is report new errors. It's not an API key or anything to retrieve any other data from Sentry.

Now to address the transport issue: I'm thinking it may make sense to make the transports pluggable. So we can have one for browsers that does the GET method or a separate one for POST that supports CORS.

The reason why I don't support a POST right now is because of cross browser capabilities. It's a real pain to make it work correctly everywhere, and GET works the best. So if we were to provide a transport that only supported, say, V8. This could cover the cases of Chrome apps, etc.

This would work in a similar fashion to the way transports work in the python client, if you're familiar with that. We would encode the transport into the DSN likely and you can register which transport you want to use.

For example:

Raven.registerTransport('https+post', function(data) { /* Send the data with POST */ });
Raven.config('https+post://....').install()

Does that make sense?

@frekw
Copy link

frekw commented Jan 21, 2014

Oh, I should have read the docs better. I just assumed full API access as soon as a public and private key are being used. Then I don't see any problems with distributing the private key in the bundle. :)

I'm unfamiliar with the python library but I think it sounds like a good idea being able to plugging in new transports and choosing via the DSN.

If you want to, I can see if I can find some time to have a go at implementing a plugin API for transport tomorrow?

I don't think it should be too difficult to get going. If we use today's method (let's call it http[s]+get) as the default unless explicitly set, it should be fairly simple to retain backwards compatibility as well and it should only be a matter to add some tests for the new functionality.

@frekw
Copy link

frekw commented Jan 22, 2014

Ok @mattrobenolt, I've added a pull-request introducing registerTransport.

Some changes were needed in addition to your suggestion:

Transports are split up into setup and send since each transport may need different configuration to be passed into Raven.configure, and we want to validate that config during the Raven.config step instead of when we try to send data. The setup method can also choose to accept triggerEvent as a second parameter, since that was the best way to get a hold of the private triggerEvent method Raven uses.

I'm not too happy about passing in triggerEvent to each transport registered, but that was the only way to accomplish this change without adding triggerEvent to the public API (not sure if that's a good idea or not).

I'd love your thoughts on this!

@julien-duponchelle
Copy link

Sentry make a change and it's work for us with cordova:
getsentry/sentry@9e5ecfe

@mattrobenolt
Copy link
Contributor

Whoa, you caught that quick. :) I'm glad this solves the problem. I was actually trying to figure out what other problems this would solve.

@nrako
Copy link

nrako commented Jul 3, 2014

Great! I need that sentry fix getsentry/sentry@9e5ecfe for node-webkit which like cordova doesn't send any referer. @mattrobenolt do you know when this will be part of an official sentry release?

@wildermuthn
Copy link

I too would love to get this working for node-webkit

@mattrobenolt
Copy link
Contributor

@nrako The commit you referenced is being used on app.getsentry.com. We haven't cut an "official" release in a while. We'd just recommend running Sentry from HEAD, which is what we do most of the time.

Also, fwiw, I plan on looking at this closer this week, along with #183 which should solve the rest of the issues.

@nrako
Copy link

nrako commented Jul 14, 2014

Thanks for the infos @mattrobenolt. We end up patching the latest release with what we needed. I don't feel very confident with using HEAD, but since there CI tests we might, we'll see. But hopefully the release will be back? I guess for a v7?

@mattrobenolt
Copy link
Contributor

We're definitely holding out for a v7, but we literally run HEAD ourselves for the hosted service. So if you have problems with that, we're definitely more on top of it and more receptive. If you have an issue with the latest tag, we'll likely tell you to run HEAD. :)

@nrako
Copy link

nrako commented Jul 14, 2014

That should surely increase our confidence to run HEAD thanks :)

@wearhere
Copy link

Hi everyone, I can't quite follow what is expected/not expected to currently work, but I just wanted to report my findings from including raven-js 1.1.16 in a Chrome extension:

  1. I did not have to patch Raven at all i.e. I didn't run into the transport issues initially described by @fanzeyi.

  2. I was able to use the public DSN seemingly despite the comments above from @mattrobenolt et al. about having to include the private/secret keys.

  3. I reproduced @fanzeyi's initial report that Sentry doesn't allow you to add an 'Allowed domain' of chrome-extension://*/ or even chrome-extension://{{ your id }}. So, I had to allow all origin URLs using *. :(

  4. Despite the above, I was able to whitelist and include Chrome extension URLs. My extension modifies Gmail, so I configured Raven like this:

    Raven.config(clientDSN, {
      whitelistUrls: [/^chrome-extension:\/\//, /mail.google.com/],
      includePaths: [/^chrome-extension:\/\//, /mail.google.com/]
    });

    That is, the first match pattern of each array whitelists/includes the background scripts, the second the content scripts.

So, from my perspective it looks like the the only bug is that we can't restrict the origin URLs (#2 above). Any chance you could help with that @mattrobenolt? :)

@mattrobenolt
Copy link
Contributor

I believe this should all be good now with Sentry 8.0 paired with raven-js 2.0.0, but we don't explicitly test against this, so not 100% sure.

Please reopen if there is more we need to do here.

kamilogorek pushed a commit that referenced this issue Jun 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants