Skip to content

feat: Social Logins #719

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

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
a774cc8
google login done
yugantarjain Aug 10, 2020
fb6408f
requirements updated
yugantarjain Aug 11, 2020
9b8cedf
sign up using google added (new user is created if old user not found…
yugantarjain Aug 11, 2020
4cea797
apple sign in done
yugantarjain Aug 12, 2020
6340c57
creating and returning access tokens code refactored
yugantarjain Aug 13, 2020
dd9216a
social sign in table created and used
yugantarjain Aug 13, 2020
ba8528d
social sign in implementation finalized
yugantarjain Aug 13, 2020
c6b1aea
code documentation and logic improved
yugantarjain Aug 13, 2020
470d572
messages cosntants fixed. API code refactored and re-used.
yugantarjain Aug 13, 2020
f78e925
tests added and logic improved
yugantarjain Aug 14, 2020
29708e3
social sign in documentation added in user_authentication.md
yugantarjain Aug 14, 2020
a68ee39
code documentation improved
yugantarjain Aug 18, 2020
bb8c558
error unwrapping done to make code clearer. google auth client id mad…
yugantarjain Aug 19, 2020
a9fc3d9
google auth client id used from .env now
yugantarjain Aug 19, 2020
e488923
unnecessary if removed
yugantarjain Aug 21, 2020
191f51f
Update docs/user_authentication.md
yugantarjain Aug 24, 2020
665fb6c
Update docs/user_authentication.md
yugantarjain Aug 24, 2020
a0eda7b
Update docs/user_authentication.md
yugantarjain Aug 24, 2020
db33773
Update docs/user_authentication.md
yugantarjain Aug 24, 2020
d6d4aa3
assertEqual used in tests. doc improved.
yugantarjain Aug 24, 2020
ddc7a54
Merge branch 'develop' into social-sign-in
yugantarjain Aug 29, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions app/api/dao/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,30 @@ def create_user(data: Dict[str, str]):

return messages.USER_WAS_CREATED_SUCCESSFULLY, HTTPStatus.CREATED

@staticmethod
def create_user_using_social_login(data: Dict[str, str], apple_auth_id: str=None):
"""
Creates a new user using Google Auth.

Arguments:
data: A list containing the user's name and email

Returns:
The new user created
"""

name = data["name"]
username = None
password = None
email = data["email"]
terms_and_conditions_checked = True
social_login = True

user = UserModel(name, username, password, email, terms_and_conditions_checked, social_login, apple_auth_id)
user.save_to_db()

return user

@staticmethod
@email_verification_required
def delete_user(user_id: int):
Expand Down Expand Up @@ -377,6 +401,37 @@ def authenticate(username_or_email: str, password: str):

return None

@staticmethod
def get_user_for_social_login(email: str, apple_auth_id: str=None):
"""Returns user for google login

Checks email of the user to find an existing user.
If found, the user is returned.
Else, a new user is created and returned

Arguments:
email: email of the user, used to check for existing user

Returns:
Returns authenticated user
"""

# If apple auth id given, first try to find existing user using it
if apple_auth_id:
user = UserModel.find_by_apple_auth_id(apple_auth_id)
# if user not found, try finding using email
if not user:
user = UserModel.find_by_email(email)

# If apple auth id not given, try finding using email
else:
user = UserModel.find_by_email(email)

if user:
return user
else:
return None

@staticmethod
@email_verification_required
def get_achievements(user_id: int):
Expand Down
14 changes: 12 additions & 2 deletions app/api/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ def add_models_to_namespace(api_namespace):
update_user_request_body_model.name
] = update_user_request_body_model
api_namespace.models[login_request_body_model.name] = login_request_body_model
api_namespace.models[social_auth_body_model.name] = social_auth_body_model
api_namespace.models[login_response_body_model.name] = login_response_body_model
api_namespace.models[refresh_response_body_model.name] = refresh_response_body_model
api_namespace.models[
Expand Down Expand Up @@ -119,8 +120,8 @@ def add_models_to_namespace(api_namespace):
"User registration model",
{
"name": fields.String(required=True, description="User name"),
"username": fields.String(required=True, description="User username"),
"password": fields.String(required=True, description="User password"),
"username": fields.String(required=False, description="User username"),
"password": fields.String(required=False, description="User password"),
Comment on lines +123 to +124
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to change? as far as I see you did not change anything to the POST /register API, you created new endpoints that follow a different request model.

Copy link
Author

@yugantarjain yugantarjain Aug 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Actually, when a user signs up using social login, we first try to find them in our database. If a pre-existing user is not found, we create a new one. To facilitate the creation of a new user, these fields were made optional. I did some testing and it works fine (because we have validation checks for them in any case) and all the unit tests passed.

If there is a better solution to this, please let me know. We also have an ongoing discussion about this on zulip mentorship ios channel.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good @yugantarjain thank you for your explanation!

"email": fields.String(required=True, description="User email"),
"terms_and_conditions_checked": fields.Boolean(
required=True, description="User check Terms and Conditions value"
Expand Down Expand Up @@ -152,6 +153,15 @@ def add_models_to_namespace(api_namespace):
},
)

social_auth_body_model = Model(
"Google authentication data model",
{
"id_token": fields.String(required=True, description="User's idToken given by Google auth"),
"name": fields.String(required=True, description="User's name given by Google auth"),
"email": fields.String(required=True, description="User's email given by Google auth"),
},
)

# TODO: Remove 'expiry' after the android app refactoring.
login_response_body_model = Model(
"Login response data model",
Expand Down
114 changes: 114 additions & 0 deletions app/api/resources/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
from app.api.models.user import *
from app.api.dao.user import UserDAO
from app.api.resources.common import auth_header_parser
from google.oauth2 import id_token
from google.auth.transport import requests

users_ns = Namespace("Users", description="Operations related to users")
add_models_to_namespace(users_ns)
Expand Down Expand Up @@ -390,6 +392,118 @@ def post(cls):
)


@users_ns.route("apple/auth/callback")
class AppleAuth(Resource):
@classmethod
@users_ns.doc("apple-auth callback")
@users_ns.response(HTTPStatus.OK, "Successful login", login_response_body_model)
@users_ns.expect(social_auth_body_model)
def post(cls):
"""
Login/Sign-in user using Apple Sign-In.

The Apple user id (id_token) is recieved which becomes the primary basis for user identification.
If not found then that means user is using apple sign-in for first time. In this case, email is checked.
If email found, that account is used. Else, a new account is created.
"""

apple_auth_id = request.json.get("id_token")
email = request.json.get("email")

# get existing user
user = DAO.get_user_for_social_login(email, apple_auth_id)

# if user not found, create a new user
if not user:
data = request.json
user = DAO.create_user_using_social_login(data, apple_auth_id)
# else, set apple auth id to later identify the user
else:
user.apple_auth_id = apple_auth_id
user.save_to_db()

# create tokens and expiry timestamps
access_token = create_access_token(identity=user.id)
refresh_token = create_refresh_token(identity=user.id)

from run import application
access_expiry = datetime.utcnow() + application.config.get(
"JWT_ACCESS_TOKEN_EXPIRES"
)
refresh_expiry = datetime.utcnow() + application.config.get(
"JWT_REFRESH_TOKEN_EXPIRES"
)

# return data
return (
{
"access_token": access_token,
"access_expiry": access_expiry.timestamp(),
"refresh_token": refresh_token,
"refresh_expiry": refresh_expiry.timestamp(),
},
HTTPStatus.OK,
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can go to a private method in this file or somewhere else, as long as you reuse this logic since you use it in at least 2 different places.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


@users_ns.route("google/auth/callback")
class GoogleAuth(Resource):
@classmethod
@users_ns.doc("google-auth callback")
@users_ns.response(HTTPStatus.OK, "Successful login", login_response_body_model)
@users_ns.response(HTTPStatus.UNAUTHORIZED, f"{messages.GOOGLE_AUTH_TOKEN_VERIFICATION_FAILED}")
@users_ns.expect(social_auth_body_model)
def post(cls):
"""
Login/Sign-in user using Google Sign-In.

The Google user idToken is received from the client side, which is then verified for its authenticity.
On verification, the user is either logged-in or sign-up depending upon wheter it is an existing
or a new user.
"""

token = request.json.get("id_token")
email = request.json.get("email")
client_id = "992237180107-lsibe891591qcubpbd8qom4fts74i5in.apps.googleusercontent.com"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this a secret variable? can you please put this in a constant, either here or probably better in Config.py

Copy link
Author

@yugantarjain yugantarjain Aug 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. I checked the config.py file and am not sure how exactly to add this constant there... Should I just do it the way it is done in messages.py?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yugantarjain Read the 4th Point here in Setup and Run part of Docs https://github.com/anitab-org/mentorship-backend. Config.py reads the os.env variables for config. As part of setup of this app, someone sets the env variables. So you just need to add the variable and read from there.
I am not sure how you can get the exact value of the config into production instance. @isabelcosta you can help clarify this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Made a constant in config.py.


# Verify google auth id token
try:
idinfo = id_token.verify_oauth2_token(token, requests.Request(), client_id)

# id_token is valid. Get user.
user = DAO.get_user_for_social_login(email)

if not user:
# create a new user
data = request.json
user = DAO.create_user_using_social_login(data)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will the logic here work fine if this is not a gmail email? I never did anything like this so I am really curious

Copy link
Author

@yugantarjain yugantarjain Aug 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it will work fine. In-fact, when I tested with Sign in with Apple, I hid my email (the service then returns a private relay email) and it worked perfectly.

# create tokens and expiry timestamps
access_token = create_access_token(identity=user.id)
refresh_token = create_refresh_token(identity=user.id)

from run import application
access_expiry = datetime.utcnow() + application.config.get(
"JWT_ACCESS_TOKEN_EXPIRES"
)
refresh_expiry = datetime.utcnow() + application.config.get(
"JWT_REFRESH_TOKEN_EXPIRES"
)

# return data
return (
{
"access_token": access_token,
"access_expiry": access_expiry.timestamp(),
"refresh_token": refresh_token,
"refresh_expiry": refresh_expiry.timestamp(),
},
HTTPStatus.OK,
)

except ValueError:
return messages.GOOGLE_AUTH_TOKEN_VERIFICATION_FAILED, HTTPStatus.UNAUTHORIZED


@users_ns.route("login")
class LoginUser(Resource):
@classmethod
Expand Down
20 changes: 17 additions & 3 deletions app/database/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class UserModel(db.Model):

# security
password_hash = db.Column(db.String(100))
apple_auth_id = db.Column(db.String(100))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for this to work I think you need to create a migration script to add this field to dev database. Can you do this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I did create a migration script and ran it locally. Do I need to perform any additional steps?


# registration
registration_date = db.Column(db.Float)
Expand Down Expand Up @@ -59,7 +60,7 @@ class UserModel(db.Model):
need_mentoring = db.Column(db.Boolean)
available_to_mentor = db.Column(db.Boolean)

def __init__(self, name, username, password, email, terms_and_conditions_checked):
def __init__(self, name, username, password, email, terms_and_conditions_checked, social_login=False, apple_auth_id=None):
"""Initialises userModel class with name, username, password, email, and terms_and_conditions_checked. """
## required fields

Expand All @@ -69,11 +70,19 @@ def __init__(self, name, username, password, email, terms_and_conditions_checked
self.terms_and_conditions_checked = terms_and_conditions_checked

# saving hash instead of saving password in plain text
self.set_password(password)
if not social_login:
self.set_password(password)

# save auth id for sign in with apple (if present)
if apple_auth_id:
self.apple_auth_id = apple_auth_id

# default values
self.is_admin = True if self.is_empty() else False # first user is admin
self.is_email_verified = False
if social_login:
self.is_email_verified = True
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not verify here? Any special usecase?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. For normal login, the email is not verified and the user specifically has to do that from a link they receive.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got you, I was thinking something different, not applicable here though

Copy link
Author

@yugantarjain yugantarjain Aug 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated this to - self.is_email_verified = social_login
That removes unnecessary if statements.

else:
self.is_email_verified = False
self.registration_date = time.time()

## optional fields
Expand Down Expand Up @@ -129,6 +138,11 @@ def find_by_id(cls, _id: int) -> 'UserModel':
"""Returns the user that has the id we searched for. """
return cls.query.filter_by(id=_id).first()

@classmethod
def find_by_apple_auth_id(cls, _id: str) -> 'UserModel':
"""Returns the user that has apple auth id we searched for."""
return cls.query.filter_by(apple_auth_id=_id).first()

@classmethod
def get_all_admins(cls, is_admin=True):
"""Returns all the admins. """
Expand Down
1 change: 1 addition & 0 deletions app/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@
EMAIL_VERIFICATION_MESSAGE = {
"message": "Check your email, a new verification" " email was sent."
}
GOOGLE_AUTH_TOKEN_VERIFICATION_FAILED = "Google auth token verification failed"

# Success messages
TASK_WAS_ALREADY_ACHIEVED = {"message": "Task was already achieved."}
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ Flask-SQLAlchemy==2.3.2
Flask-Testing==0.7.1
future==0.16.0
gunicorn==20.0.4
google-auth==1.20.1
idna==2.6
itsdangerous==0.24
Jinja2==2.10
Expand Down