Skip to content
This repository was archived by the owner on Apr 12, 2023. It is now read-only.

Added Feature for Login using OAuth - #48

Open
saivittalb wants to merge 3 commits into
JBossOutreachArchive:masterfrom
saivittalb:OAuth
Open

Added Feature for Login using OAuth#48
saivittalb wants to merge 3 commits into
JBossOutreachArchive:masterfrom
saivittalb:OAuth

Conversation

@saivittalb

Copy link
Copy Markdown

Implemented Feature for Login using OAuth2 🎉

If any change is required, please let me know! 😃

@jatinkatyal13 jatinkatyal13 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Checkout the comments !

Comment thread api/social_oauth.py Outdated
Comment thread api/social_oauth.py Outdated
Comment thread api/views.py Outdated
Comment thread certificate_generator/settings.py Outdated
Comment thread api/views.py Outdated
@jatinkatyal13

jatinkatyal13 commented Nov 23, 2018

Copy link
Copy Markdown
Collaborator

Let's talk about the flow now!
Whenever the user signs up using any of the OAuth backend we'll create a new user account for them afterwards we'll be providing them with JWT token whenever they sign-in with the similar backend! We'll retrieve their information like name, email only when they sign-up or sign-in not with every request!
@aashutoshrathi Views?

@aashutoshrathi aashutoshrathi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi, @saivittalb
I will review this tomorrow properly.
But for now I have left a comment on the line, I'm confused with.
And also make sure you PEP8ify all your changes 😄
Thanks that's a lot of nice work in one PR. 🚀

Comment thread certificate_generator/settings.py Outdated
@saivittalb

Copy link
Copy Markdown
Author

I have corrected my code in reference to @aashutoshrathi's comment and also used PEP8 to test locally with the above environment!

Comment thread api/serializers.py
Comment thread api/serializers.py
Comment thread api/tests.py Outdated
Comment thread api/tests.py Outdated
Comment thread api/tests.py Outdated

@aashutoshrathi aashutoshrathi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Check comments that I have left.

@aashutoshrathi aashutoshrathi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good to go, we just have to run it properly.

@aashutoshrathi aashutoshrathi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Use decouple

Comment thread certificate_generator/settings.py Outdated
'FACEBOOK_SECRET',
'TWITTER_KEY',
'TWITTER_SECRET']:
exec("{key} = os.environ.get('{key}', '')".format(key=key))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

instead os.environ.get use config

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

config is already imported in the class from decouple
I changed the os.environ.get to config

@aashutoshrathi aashutoshrathi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Update env vars and add social_django to requirements and Pipfile

@aashutoshrathi aashutoshrathi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Apart from changes requested, do an AutoPEP8 operation

Comment thread .env.sample Outdated
TWITTER_SECRET=YOUR-SECRET-KEY
ALLOWED_HOSTS=.localhost, .herokuapp.com
DEBUG=
DEBUG= No newline at end of file

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Add a new line

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done ✅

Comment thread requirements.txt Outdated
pytz==2018.5
whitenoise==4.1
psycopg2-binary
social_django

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Change this to social-auth-app-django

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done ✅ Also did an AutoPEP8 operation 👍

@aashutoshrathi aashutoshrathi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Check comments

Comment thread .travis.yml
@@ -1,25 +1,31 @@
language: python
python:
- 3.6

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Revert indentation changes in this file

Comment thread requirements.txt
pytz == 2018.5
whitenoise == 4.1
psycopg2 - binary
social-auth-app-django

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Revert indentation changes here too.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants