search

amkirwan / ember-oauth2

JavaScript library for using OAuth 2.0 Implicit Grant flow (Client-Side Flow) with Ember.js
MIT License
133 stars 22 forks source link

checkState() problems #16

Closed stephen closed 9 years ago

stephen commented 9 years ago

Is the checkState() function doing the right thing here? https://github.com/amkirwan/ember-oauth2/blob/master/lib/ember-oauth2.js#L307

It seems to be taking the state stored i local storage (by associatively using the google hash) and checking it against the local state property via this.get('state')

Wouldn't checking against the state in local storage be enough to verify the state? The current code doesn't seem to work if you use a separate OAuth2 object for the callback page, since the this.get('state') that would have been set on the page calling authorize() would not exist on the separate callback page.

Currently, I'm getting the error that the local state doesn't match up with the returned state. This isn't actually true (manually verified), but the this.get('state') call on that line returns undefined because it was never set in the callback route's instance.

stephen commented 9 years ago

Oof. Figured out what was going wrong (was using window.App instead of window.opener.App). I think my issue still applies, but at least isn't breaking.

amkirwan commented 9 years ago

No problem, glad you figured it out.

Yes if you have two separate OAuth2 objects the states generated will be different as they should be. Every OAuth2 request should have a unique pseudorandom state to verify the request coming back from the server.

I am not sure what you mean by, "Wouldn't checking against the state in local storage be enough to verify the state? "

Currently the checkState() function checks if the state returned from the server equals the state in the original request. Unless I am missing something I'm not sure what other way there is to verify the state.

stephen commented 9 years ago

Currently the checkState() function checks if the state returned from the server equals the state in the original request.

@amkirwan - what I meant to say was that you're checking the "state in the original request" by using this.get('state'), but I assumed that checking against the localStorage copy of the state would be sufficient. I suppose the difference is that the localStorage copy isn't coupled to a specific request.