search

iaincollins / nextjs-starter

A starter project for Next.js with authentication
https://nextjs-starter.now.sh
ISC License
1.37k stars 423 forks source link

Redirect Error and null email processed #39

Closed eashish93 closed 7 years ago

eashish93 commented 7 years ago

Hi, Good starter template for next.js, but I see there are some problems. First one is that, without entering any email on the input in route /auth/signin it processed to next page with the success message. Second one is redirect doesn't work on the post request via ajax. So I don't see why you're adding this line on the page. https://github.com/iaincollins/nextjs-starter/blob/master/routes/auth.js#L119 There are other issues as well like opening multiple route for getting csrf token. I hope you resolved these issues.

iaincollins commented 7 years ago

Please feel free to raise specific issues as individual issues if you have any questions or comments.

The redirect does not error so I am closing this.

  1. It redirects right away as an immediate response to users, rather than waiting for the SMTP request to complete, which is not actually a guarantee of delivery to a user mailbox either - so there isn't much point in waiting. This is normal practice and is intended behaviour here.

You are free to change this behaviour in your own project if you wish.

I'm not against adding further checks in template for obviously malformed addresses, but I think that's of limited utility - I don't want to over complicate the example without addressing outstanding issues first.

  1. The point of this mechanism is it supports universal rendering and to not assume or require JavaScript in the client and so works even if you don't have JavaScript enabled in the browser (for example on a feature phone or if you have a NoScript utility enabled).

If you only care about JavaScript enabled browsers authentication and session handling gets a lot simpler.

  1. I don't know what you mean by "There are other issues as well like opening multiple route for getting csrf token." but if you don't know I'm happy to explain how it works and why it's like that.

If you are referring to why there is an an Ajax route for CSRF token and what the implications are then this may help: https://medium.com/@iaincollins/csrf-tokens-via-ajax-a885c7305d4a

eashish93 commented 7 years ago

@iaincollins No, redirect doesn't happen when I put blank email address in the input field. It actually send to success page. Here's the steps to reproduce:

  1. Click on Sign In link on header of the page which sends to route /auth/signin
  2. Now click on signin button without entering any email and it will send to you on this route /auth/check-email That's why the line in this link actually doesn't work: https://github.com/iaincollins/nextjs-starter/blob/master/routes/auth.js#L119.

May be because of this: https://github.com/iaincollins/nextjs-starter/blob/master/pages/auth/signin.js#L45.

I hope you'll address this. And regarding the CSRF token, I am not asking about how it work and why it should be there. I'm telling that you've send the token from server on multiple routes. First one is /session route and second one is /csrf. I think, one route is enough for exposing csrf token.

iaincollins commented 7 years ago

Yes, I understand about how the email address is handled. Please see the answer I've provided.

I'm aware the CSRF token is return in session and token the article I linked to explains why it's useful to have an endpoint to return just the token. If you also want to return session data each time you want to get a token that's up to you.