UdacityMobileWebScholarship / guess-quote

This application is a collaborative project made by the Google Udacity Mobile Web Specialist Scholars.
MIT License
22 stars 48 forks source link

Implementing authentication #8

Closed danivijay closed 6 years ago

danivijay commented 6 years ago

Please comment opinions and interest in contribution about implementing authentication with JWT.

sounak07 commented 6 years ago

I would like to contribute jwt anthenticattion if assigned...

sounak07 commented 6 years ago

Can we use mongoose to set up db?

skyshader commented 6 years ago

I think @danivijay is working on the database setup part.

Since you've shown interest towards working on jwt authentication, I'd say go ahead with it :+1:

Just acknowledge with a response if you're taking this up. :)

sounak07 commented 6 years ago

Yes I would take JWT then @skyshader

danivijay commented 6 years ago

Please let us know when will you complete, so that the task can be assigned to you @sounak07

drenther commented 6 years ago

Will there be OAuth based Social auths or just local password based authentication?

skyshader commented 6 years ago

@drenther Just local password based. But we can explore google or facebook login as well. This would certainly help. Shall we look at passport, thoughts?

sounak07 commented 6 years ago

I can do and commit it by tomorrow @danivijay.

skyshader commented 6 years ago

@sounak07 let's settle the above discussion before starting implementation. Do contribute your thoughts too.

drenther commented 6 years ago

Yes. Using passport will help keep the code easy to follow for everyone. I think anyone with node and express experience, have used passport before. And about the JWT should we go with Cookies or LocalStorage with Auth header?

LocalStorage isn't the safest option but it sure is the easiest.

sounak07 commented 6 years ago

I think we should first implement local password based auth then we can extend it to Google, Facebook as alternative auth.

drenther commented 6 years ago

Sounds good. What about the Cookies vs LocalStorage? What is everyone's opinion on that?

skyshader commented 6 years ago

@sounak07 we can also look at only Google/Facebook (any one) in the beginning. @twishasaraiya @danivijay what are your thoughts? Do we want the users to create an account and then login with their username and password. Or do we just want them to get authenticated by maybe login using google or facebook?

skyshader commented 6 years ago

@drenther how is localstorage not a safest option. Any pointers?

drenther commented 6 years ago

Any JWT or info stored in localStorage is vulnerable to XSS. Take a look at this - https://www.rdegges.com/2018/please-stop-using-local-storage/

But given the scope of this project. That shouldn't be the prime concern. And we probably should focus on ease of development for the entire community rather than ultra-tight security. Any thoughts?

twishasaraiya commented 6 years ago

I think we should provide Google/Facebook login in the beginning then later on provide username and password login.

JoyBoy-369 commented 6 years ago

It's not really hard to implement cookies, especially with a ton of libraries already present.

skyshader commented 6 years ago

@drenther So, XSS happens when you allow the user to input malicious code to your application. We are anyway going to sanitize all the inputs we take from user before using it to interact with database (local or remote). This is similar case as it was with SQL injection. But people didn't throw SQL away instead they adhered to certain practices that takes care of such issues.

If we start searching about security issues with cookies we would find a ton of them. But it's how we handle user input at every place is what matters.

In my opinion we should go with localstorage. Do share your thoughts on above points. Would love to hear your point of view. :)

skyshader commented 6 years ago

Alright. For initials we will go with social login. Let's begin with Login with Google as this course is powered by Google :stuck_out_tongue: @sounak07 / @drenther anyone wants to take this up?

sounak07 commented 6 years ago

If we do google auth, we need a google account for Creating the Google App, Client ID, and Client Secret.....do consider that.

skyshader commented 6 years ago

Correct. You can create your own project and use it's api key. However I am concerned that other contributors have to do the same.

@twishasaraiya can we create a separate google account and share it's api key to the contributors? What could be the concerns here?

Or everyone who is running the server code on their local can create their own project and use that api key? This makes sense to me actually but is a overhead for contributors.

Thoughts?

drenther commented 6 years ago

@skyshader I don't mind going with localStorage. Glad we know what we need to do to use localStorage properly. And about the social login how do we go about with API Keys handling?

skyshader commented 6 years ago

@drenther you have any thoughts on this?

sounak07 commented 6 years ago

I think using a common key is better and we can always hide it using environment variables created from the terminal.

drenther commented 6 years ago

@sounak07 But if we have a common key, how do suggest we share it along? Because as you said yourself we won't commit the .env file

Case 1 - Common Keys We need manually share it with every new Contributor via safe means.

Case 2 - Separate Keys Every new contributor needs to setup a seperate project for the keys.

What do we go for?

skyshader commented 6 years ago

@sounak07 and @drenther we can see if we can generate a basic key that would have limited access available. If that is possible, we will share a .env.example file which will contain that basic api key with limited access.

For proper access we suggest you generate your own api key and we will generate a key that we use for the production environment.

sounak07 commented 6 years ago

The moderators can choose whomever they want to give access to the key and DM them to share the key i guess.......but that's my opinion, I would like to hear what all of you guys think about this

drenther commented 6 years ago

Both have their pros and cons (as with everything I guess). Whatever the moderators choose I guess.

Personally, I like @skyshader approach, generate your own key for dev purpose and one common for production that will be handled by moderators.

JoyBoy-369 commented 6 years ago

@skyshader The whole point of using cookies to store JWT is to prevent the token from being accessed by any scripts. With local storage, it's way easier to steal tokens. Also, web storage doesn't really adhere to any secure standard for transportation of tokens. With cookies having set secure: true we make sure that the cookies are always sent over HTTPS. If you're worried about CSRF attacks, you can always use a CSRF token. However, as someone already mentioned above it's not a top priority to get the most secure app right now.

drenther commented 6 years ago

@SevenSinS02 Exactly the point I was making earlier. But, localStorage works for the case here I guess. As it will be way simpler than the "Double Submit Cookie" implementation.

skyshader commented 6 years ago

@SevenSinS02 I do understand your's and @drenther 's point. And yeah it's not top priority to get the most secure app right now. But we will try to manage the 2 main things that could result in XSS: user's input and any such libraries that might access localstorage. Am I missing anything here?

sounak07 commented 6 years ago

One thing that I would like to mention that won't it be good if we atleast have a very basic anth UI before writing the backend....I think that would be helpful

drenther commented 6 years ago

@sounak07 Yes, I guess a basic password based auth is the right way to go for now then. So password based local auth with JWT stored in localStorage and attached as Auth Header for API requests. That's the way to go now? Any thoughts @skyshader ?

JoyBoy-369 commented 6 years ago

@drenther I totally understand. @sounak07 Since it's only a social authentication, it would remain same throughout any app (at least on the front end). There would be a difference only in the way the authentication is handled in the backend; which raises a question as to where the authentication is being handled - on the client side or the server side? Handling on the client side would be way simpler but, if I'm not mistaken would increase the app size as it would require an SDK. Handling on the server side is a little bit trickier but much more secure and keeps the app size relatively smaller.

skyshader commented 6 years ago

Umm. That was my initial thought but since social logins really ease the process for users using our app, hence orientation is towards social login a bit more.

anurag-majumdar commented 6 years ago

I feel its a good idea to implement the local login first. Although OAuth based logins are really popular with Fb and Google logins. I have used all these techniques before in some of my side projects and as sevensins mentioned, the sdk for fb and google is also coming into the picture for loading of the app initally at client side. I have implemented this feature from ground up and will share the flow with you guys. I can contribute to the full authentication part for implementation of local and social logins.

skyshader commented 6 years ago

Guys! Let's limit our conversation on this thread. It gets overwhelming for people to go though the comments here. Let's start with basic JWT auth for now and let's use passport for that so it would be easier to add other strategies in future. Also, let's use localstorage to store the token.

As this is just a backend task for now, we would need to test it via postman only.

anurag-majumdar commented 6 years ago

Good call skyshader, I would like to note one thing by saying that it will be way easier to use just only bcrypt and jwt for normal sign in than passport. For jwt we can use passport-jwt library but for password comparing and setting in db we can directly use bcrypt for that. Code for it will be really simple. I have implemented the same before too.

anurag-majumdar commented 6 years ago

If you guys want to use social logins with google and facebook we need to get the api key and client secret. Which will be handled by the moderator or co-moderators for setup so that we can use the same.

twishasaraiya commented 6 years ago

I think the contributors should generate their own keys for development purpose and we can have a common key for production purpose. Since many people may be new to this I want someone to take up the task of documenting the whole process of setup so that it would be easy for a newbie too or may provide a link to some article that explains it well.

Rohanhacker commented 6 years ago

can I work on this ?

kanlanc commented 6 years ago

Is this up for grabs?

danivijay commented 6 years ago

@sounak07 is currently working on it.

Rohanhacker commented 6 years ago

@danivijay Please assign this issue to him if he is working on it

danivijay commented 6 years ago

I'm afraid that is not possible @Rohanhacker, assignee list is limited for now.

Rohanhacker commented 6 years ago

@danivijay one person will be working on a particular issue at a time right ?? You just have to assign one person ?? who is working on that particular issue

gpalsingh commented 6 years ago

Is this issue solved? I don't see the pull request that fixes it.

danivijay commented 6 years ago
  1. https://github.com/UdacityMobileWebScholarship/guess-quote/pull/13 - merged
  2. https://github.com/UdacityMobileWebScholarship/guess-quote/pull/25 - under progress @gpalsingh