Skip to content
This repository has been archived by the owner on May 10, 2019. It is now read-only.

Submitting wildly invalid assertions to /verify causes a 500 server error #598

Closed
ianb opened this issue Nov 17, 2011 · 6 comments
Closed

Comments

@ianb
Copy link

ianb commented Nov 17, 2011

I accidentally pointed my mock assertions at BrowserID (things like the literal string "test@example.com", which isn't base64, JSON, JWT) and I get a 500 error. It should return a proper error message.

@ghost ghost assigned lloyd Nov 17, 2011
@lloyd
Copy link
Contributor

lloyd commented Nov 17, 2011

this is lame. in general the verifier needs hardening and love. it should return extremely helpful errors to end users, given this will affect our cost of supporting it significantly.

I say 4 stars.

lloyd added a commit that referenced this issue Nov 23, 2011
@lloyd lloyd closed this as completed in d25f4df Nov 23, 2011
lloyd added a commit that referenced this issue Nov 23, 2011
…leak out of JWCrypto and cause 500 errors rather than 200 failure responses.
@jbonacci
Copy link
Contributor

jbonacci commented Dec 6, 2011

Ran backend unit tests on local install. Did not see anything unusual.

@ianb
Copy link
Author

ianb commented Dec 6, 2011

Do the unit tests submit something that isn't valid JWT, or once
JWT-decoded is not valid JSON?
On Dec 5, 2011 8:07 PM, "jbonacci" <
reply@reply.github.com>
wrote:

Ran backend unit tests on local install. Did not see anything unusual.


Reply to this email directly or view it on GitHub:
#598 (comment)

@lloyd
Copy link
Contributor

lloyd commented Dec 6, 2011

Both.

--lloyd

On Dec 5, 2011, at 8:44 PM, Ian Bickingreply@reply.github.com wrote:

Do the unit tests submit something that isn't valid JWT, or once
JWT-decoded is not valid JSON?
On Dec 5, 2011 8:07 PM, "jbonacci" <
reply@reply.github.com>
wrote:

Ran backend unit tests on local install. Did not see anything unusual.


Reply to this email directly or view it on GitHub:
#598 (comment)


Reply to this email directly or view it on GitHub:
#598 (comment)

@ianb
Copy link
Author

ianb commented Dec 6, 2011

This breaks on browserid but works on diresworb:

curl https://browserid.org/verify -d 'assertion=xxxx&audience=localhost'

I'm guessing that means it is fixed.

@lloyd
Copy link
Contributor

lloyd commented Dec 7, 2011

Will roll into prod on thurs

--lloyd

On Dec 6, 2011, at 2:35 PM, Ian Bickingreply@reply.github.com wrote:

This breaks on browserid but works on diresworb:

curl https://browserid.org/verify -d 'assertion=xxxx&audience=localhost'

I'm guessing that means it is fixed.


Reply to this email directly or view it on GitHub:
#598 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants