search

knowthen / Episode-9-Ditching-Cookies-for-JSON-Web-Tokens

Source for screencast: Ditching Cookies for JSON Web Tokens
http://knowthen.com/episode-9-ditching-cookies-for-json-web-tokens/
20 stars 2 forks source link

Possible bug #3

Open zwhitchcox opened 8 years ago

zwhitchcox commented 8 years ago

Just looking at your code...You have app.get('/logs', isAuth(), function* logs() {, but the ui router says .state('logs', { url: '/logs',

These seem to be conflicting GET requests to me, and I don't think the state change error code will work properly.

knowthen commented 8 years ago

Hmm, been a while since I looked at this, but I'm not sure I follow what you're saying.

When the state is logs, it will do a GET to /logs. This triggers the isAuth middleware, which either yields or returns a 401...

I'm missing what your saying, can you elaborate?

zwhitchcox commented 8 years ago

Ok, thank you, but what I'm saying, is if you type in the url, '/logs', directly, it's going to send you a 401 error. So, basically, this url will not be directly accessible.

knowthen commented 8 years ago

Ahh, I see what your saying. Yep, that seems to be a bug. Probably should have made it /api/logs.
I'm not sure if I should fix it, as it makes the app inconsistent with the screencast. Maybe I'll just add a note. Thanks

zwhitchcox commented 8 years ago

That would fix that, but there would still be a bug in the code that redirects you to the login page based on the statechangeerror, because the thrown 401 would not be captured by the ui router, but by Restangular.

knowthen commented 8 years ago

I'm not seeing what your suggesting.

I just ran the app again, and found a different bug that I introduced after the screencast several months ago.. I changed to below if (this.user && this.user.userid > 0) { but after I changed that, when I open app, navigate to logs, the error is evaluated in the handler for $stateChangeError and I get redirected to the login page. I login and see the logs etc..

Are you seeing different behavior?

zwhitchcox commented 8 years ago

Ahh, ok. I didn't realize the statechangeerror would intercept errors from restangular, but that all makes sense now.

This is a little new to me because usually I've just seen people test whether they are logged in before they go to the page like here instead of each individual request.

But I guess this would provide a smoother experience, because they wouldn't have to wait for the rest requests to load?