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.

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

Auth Related Questions #9

Open
jamshally opened this issue Jul 31, 2018 · 5 comments
Open

Auth Related Questions #9

jamshally opened this issue Jul 31, 2018 · 5 comments

Comments

@jamshally
Copy link

jamshally commented Jul 31, 2018

I'm submitting a...


[ ] Regression 
[ ] Bug report
[ ] Feature request
[ ] Documentation issue or request
[x] Question 

First, thank for your sharing this code. There are very few good examples of auth with NestJS, and so I was grateful to find this one - which seems very nicely implemented.

In looking through the code, I had some questions to help me understand the choices.

Questions

  • Unless I have missed something, there is a lot of custom auth code in auth.service.ts. It looks like this is used instead of calling passport.authenticate('facebook-token'), which is the documented approach. Is this approach taken for a specific reason?
  • What was the reason for using 'passport-facebook-token' strategy/package over the 'passport-facebook' strategy/package published by the author of Passport.js? (same for twitter)
  • Have you considered any code to mitigate CRSF attacks, or is there something about this implementation that already mitigates this?
  • What was the reason for using Express Middleware directly instead of through the Nest Middleware?
  • What was the reason you decided to use custom JOI validation instead of the pipe based NestJs Object Schema Validation

Thanks for any answers.

@bojidaryovchev
Copy link
Owner

Hello there! Here are the answers:

  1. You should check out the auth.module where we have consumer.apply(middlewares).forRoutes(routes) to apply a middleware before a route handler is reached and the strategies are simply middlewares. In a middleware you can manipulate the request and response so a given strategy acquires the profile information from fb/twitter/google, saves it to the db and attaches the user profile to request.user
  2. I remember I watched a video about facebook authentication with passport in which the author recommended using passport-facebook-token so there you have it
  3. I havent really got to this yet
  4. If you're talking about the function middleware, as far as I remember it didnt work using a nest middleware so I just used the function one and left it like this.. You can try using the nest one and see for yourself
  5. The project needs to be updated to use the latest nest features.. At the time I remember there was only class validator examples and I got it working but the error messages were not as descriptive as joi's so basically that was the whole purpose

I hope I've answered sufficiently.. As mentioned, I have to update the project as soon as possible so I shall get onto it as soon as I got time.. You should read further and decide for yourself, I'm not someone with much of experience.. I explored different projects around github and medium articles about the technologies I cared about and found a way to combine them together so if you could improve it just go for it.. Regards

@jamshally
Copy link
Author

jamshally commented Aug 1, 2018

Thanks for the reply! That is very helpful info.

For item 1: Thanks, I had not noticed the authenticate call there - that makes sense now.

Just sharing for your interest... there, i think, still something unusual about the auth.service.ts implementation. In most examples I have seen, the passport.authenticate is for two separate routes on social login. In your case you call it once, and then do a lot of heavy-lifting still in the async facebookSignIn method, that I think passport will do for you.

For an example, see this tutorial - one of the better ones that I have seen: https://scotch.io/tutorials/easy-node-authentication-facebook, and look at the following code:

 // route for facebook authentication and login
 app.get('/auth/facebook', passport.authenticate('facebook', { 
   scope : ['public_profile', 'email']
 }));

 // handle the callback after facebook has authenticated the user
 app.get('/auth/facebook/callback',
     passport.authenticate('facebook', {
         successRedirect : '/profile',
         failureRedirect : '/'
 }));

I hope that is of some use... or if I am understanding it wrong, that would be good to know too.

I am looking forward to seeing this project progress. It seems like some great work.

Thanks

@bojidaryovchev
Copy link
Owner

bojidaryovchev commented Aug 1, 2018

I'm not 100% sure but I think the way you suggest is used with passport-facebook, not passport-facebook-token.. What happens in facebookSignIn is we construct a url to obtain an access token for the user and then manually invoke the passport-facebook-token strategy by sending a request to our local endpoint (passport-facebook-token needs to have access_token provided)

@jamshally
Copy link
Author

OK, thanks for the clarification.

With regards to my initial passport-facebook vs passport-facebook-token question: I did some further reading, and asked (and answered) a Stack Overflow question about it. I beleive that, for your use case, it may be worth switching to the facebook-passport strategy for additional security.

I am going to have a go implementing passport-facebook in my nestjs project, and would be happy to share if I make progress with it, in case of any benefit for you.

Thanks again for taking the time to answer my questions.

@harishreddy-m
Copy link

I wish all the conversations on open-source projects are as polite as this.

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

No branches or pull requests

3 participants