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.

Sign up for GitHub

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

Jump to bottom

Better non-signing implementation #2

Closed
dcramer opened this issue Feb 3, 2012 · 3 comments
Closed

Better non-signing implementation #2

dcramer opened this issue Feb 3, 2012 · 3 comments

Comments

@dcramer
Copy link
Member

dcramer commented Feb 3, 2012

We need to implement some behavior in Sentry (so this ticket applies to both raven-js and Sentry) that removes the need for Raven to send a signed request. The signed request in JS doesn't actually provide any benefit, as it exposes the signing key (secret key).

Two things we should do:

  1. Implement ProjectDomain (model exists in Sentry, but its not implemented yet) to check for whitelisted trusted domains.
  2. Fully support non-signed requests officially. This should probably just be public key to allow it, and should not be enabled in Sentry by default. Possibly make it a per-project option that says "allow public errors" which dont require the secret key or a signed request.
@bkonkle
Copy link
Contributor

bkonkle commented Feb 3, 2012

I could also add an option to omit the secretKey and send the message to the server for signing. It would be a very simple view to write, and someone could probably write a dead-simple Nginx module to avoid the app server entirely. Whitelisting is definitely the easier choice for now, though.

@bkonkle
Copy link
Contributor

bkonkle commented Feb 4, 2012

I went ahead and added a signatureUrl option to allow it to operate without a secretKey and hit a url to get the signature. I also added the option to configure it with a base64 string to obfuscate it. I added a note to the Readme explaining that this option was still insecure, but less obvious.

Next week I'll try to send a pull request for Sentry to add whitelisting.

@dcramer
Copy link
Member Author

dcramer commented May 8, 2012

This is now fixed

@dcramer dcramer closed this as completed May 8, 2012
@wearhere wearhere mentioned this issue Jan 13, 2015
Closed
benvinegar added a commit that referenced this issue Dec 17, 2015
@benvinegar
2.0.0-rc2 attempt #2 (w/ version fix)
7c80674
benvinegar added a commit that referenced this issue Dec 17, 2015
@benvinegar
2.0.0-rc2 attempt #2 (w/ version fix)
d85d1ed
benvinegar added a commit that referenced this issue Dec 17, 2015
@benvinegar
2.0.0-rc2 attempt #2 (w/ version fix)
99724d9
benvinegar added a commit that referenced this issue Dec 18, 2015
@benvinegar
2.0.0-rc2 attempt #2 (w/ version fix)
acb7790
@benvinegar benvinegar mentioned this issue Mar 16, 2016
Closed
@benvinegar benvinegar mentioned this issue Jun 20, 2016
Merged
@benvinegar benvinegar mentioned this issue Feb 9, 2017
Closed
@wwwouter wwwouter mentioned this issue Mar 12, 2018
Closed
kamilogorek pushed a commit that referenced this issue Jun 12, 2018
@mattrobenolt
Safer error handling when parsing stack traces to prevent the entire …
73015b4 
…process from taking a shit, should fix #2
kamilogorek pushed a commit that referenced this issue Jun 12, 2018
@mattrobenolt
Bumping 0.2.3, fixes #2
b319f4d
@lobsterkatie lobsterkatie mentioned this issue Mar 12, 2021
Closed
@1999 1999 mentioned this issue May 29, 2021
Merged
@User9684 User9684 mentioned this issue Nov 18, 2022
Closed
3 tasks
@lesliewu1 lesliewu1 mentioned this issue Dec 22, 2022
Closed
3 tasks
@billyvg billyvg mentioned this issue Mar 8, 2023
Closed
@JustDoItSascha JustDoItSascha mentioned this issue Jul 13, 2023
Closed
3 tasks
@billyvg billyvg mentioned this issue Jul 28, 2023
Closed
@kamilogorek kamilogorek mentioned this issue Aug 2, 2023
Merged
kamilogorek added a commit that referenced this issue Aug 2, 2023
@kamilogorek
ref: Allow withSentryConfig to accept async config function (#8721)
ef5cb5f 
Modified one template to use the async function instead, as adding an
additional variant to an already bloated integration test runner felt
meh.

Note that type from
#7444 is incorrect.
It should be:

```js
const nextConfig = async () => {
  /** @type {import('next').NextConfig} */
  return {};
}
```

and not:

```js
/** @type {import('next').NextConfig} */
const nextConfig = async () => {
  return {};
}
```

Note #2: async function handling works for `next >= 12.1` only:
https://nextjs.org/docs/app/api-reference/next-config-js/pageExtensions

Verified the behavior locally using provided sample config from the
original issue.

Fixes #7444
@Lms24 Lms24 mentioned this issue Aug 8, 2023
Closed
@Lms24 Lms24 mentioned this issue Jul 30, 2024
Merged
This was referenced Aug 17, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dcramer @bkonkle