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.

Sign up for GitHub

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

Adding a fast sign-in with google #66

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Adding a fast sign-in with google #66

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ccf69ff
Adding a fast sign-in with google
maayanhd Apr 13, 2021
9669440
Fixtures and tests for sign in with google feature
maayanhd May 6, 2021
ccdf698
Fixing user_views_tests
maayanhd May 6, 2021
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
2 changes: 2 additions & 0 deletions Pipfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ flake8 = "*"
pytest = "*"
pytest-django = "*"
black = "*"
django-override-settings = "*"

[packages]
django = "*"
django-crispy-forms = "*"
pillow = "*"
django-cleanup = "*"
django-allauth = "*"

[requires]
python_version = "3.8"
173 changes: 163 additions & 10 deletions Pipfile.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions bootstrap.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ cd /vagrant
# Install dependencies with Pipenv
pipenv sync --dev

#installing package for google fast signing
pipenv install django-allauth

# Run database migrations
pipenv run python manage.py migrate

Expand Down
40 changes: 32 additions & 8 deletions picATrip/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
# Build paths inside the project like this: BASE_DIR / 'subdir'.
BASE_DIR = Path(__file__).resolve().parent.parent


# Quick-start development settings - unsuitable for production
# See https://docs.djangoproject.com/en/3.1/howto/deployment/checklist/

Expand All @@ -28,7 +27,6 @@

ALLOWED_HOSTS = []


# Application definition

INSTALLED_APPS = [
Expand All @@ -44,6 +42,11 @@
'django_cleanup.apps.CleanupConfig',
'Post.apps.PostConfig',
'commenting_system.apps.CommentingSystemConfig',
'django.contrib.sites',
'allauth',
'allauth.account',
'allauth.socialaccount',
'allauth.socialaccount.providers.google',
]

MIDDLEWARE = [
Expand All @@ -61,7 +64,8 @@
TEMPLATES = [
{
'BACKEND': 'django.template.backends.django.DjangoTemplates',
'DIRS': [],
# Defining templates location
'DIRS': [str(BASE_DIR.joinpath("templates"))],
'APP_DIRS': True,
'OPTIONS': {
'context_processors': [
Expand All @@ -76,7 +80,6 @@

WSGI_APPLICATION = 'picATrip.wsgi.application'


# Database
# https://docs.djangoproject.com/en/3.1/ref/settings/#databases

Expand All @@ -87,7 +90,6 @@
}
}


# Password validation
# https://docs.djangoproject.com/en/3.1/ref/settings/#auth-password-validators

Expand All @@ -106,7 +108,6 @@
},
]


# Internationalization
# https://docs.djangoproject.com/en/3.1/topics/i18n/

Expand All @@ -120,7 +121,6 @@

USE_TZ = True


# Static files (CSS, JavaScript, Images)
# https://docs.djangoproject.com/en/3.1/howto/static-files/

Expand All @@ -131,7 +131,31 @@

CRISPY_TEMPLATE_PACK = 'bootstrap4'

DEFAULT_AUTO_FIELD = 'django.db.models.AutoField'

# Django all auth settings
AUTHENTICATION_BACKENDS = (
# Needed to login by username in Django admin, regardless of `allauth`
'django.contrib.auth.backends.ModelBackend',
# `allauth` specific authentication methods, such as login by e-mail
'allauth.account.auth_backends.AuthenticationBackend',
)

SITE_ID = 1
# Turns off verification emails.
ACCOUNT_EMAIL_VERIFICATION = 'none'
LOGIN_REDIRECT_URL = 'homepage'
LOGIN_URL = 'login'
# Skips logout page
ACCOUNT_LOGOUT_ON_GET = True

DEFAULT_AUTO_FIELD = 'django.db.models.AutoField'

SOCIALACCOUNT_PROVIDERS = {
'google': {
'SCOPE': [
'profile',
'email',
],
'AUTH_PARAMS': {'access_type': 'online'},
}
}
1 change: 1 addition & 0 deletions picATrip/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
path('profile/', user_views.profile, name='profile'),
path('postList/', include('Post.urls'), name='postList'),
path('createPost/', post_views.CreateNewPost, name='createPost'),
path('accounts/', include('allauth.urls')),
]

if settings.DEBUG:
Expand Down
19 changes: 15 additions & 4 deletions pickATrip_django_apps/admin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,22 @@ def __init__(self, general_model, admin_site):
super(ListAdminMixin, self).__init__(general_model, admin_site)


def is_related_to_social_signing(suspected_model):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a bad solution but I couldn't find any other solution to exclude these models relates to the 'sites' and 'all-auth' models of Django (since there is an auto-registration of those models and then I kept getting the Already registered exception)- please give me your input here - @EdDev @guy9050 @Yarboa

Copy link
Contributor

Choose a reason for hiding this comment

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

can you please elaborate more what the problem is and how does this function solve it?

Copy link
Collaborator Author

@maayanhd maayanhd Apr 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The automation of the model registration suppose to register to admin only models that haven't been registered already, regarding the 4, 5 models provided by Django that are allowing social account logins (like google sign in) are already registered by Django and then the code tries to register it although it already has been registered, and this causes an exception. I couldn't find a solution so I enforced the automation part to avoid registering the 4, 5 problematic models that have been registered by Django to avoid the problem using an ugly if statement that assuring the model that is registered is not one of these problematic ones (it might have something to do with the fact the social account models are registered differently because it happened automatically after adding the new settings to support social login)

return (
suspected_model._meta.model.__name__ == "Site"
or suspected_model._meta.model.__name__ == "EmailAddress"
or suspected_model._meta.model.__name__ == "SocialApp"
or suspected_model._meta.model.__name__ == "SocialToken"
or suspected_model._meta.model.__name__ == "SocialAccount"
)


# Register all other models automatically - should stay last in file.
models = apps.get_models()
for model in models:
admin_class = type('AdminClass', (ListAdminMixin, admin.ModelAdmin), {})
try:
admin.site.register(model, admin_class)
except admin.sites.AlreadyRegistered:
pass
if not is_related_to_social_signing(model):
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not clear to me, Does the app could run both?
with the social auth and without?

Copy link
Collaborator Author

@maayanhd maayanhd Apr 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

no, It's somehow registered automatically, so In order to avoid conflicts of registering twice I used this bad solution, but If it makes too much trouble we can give up on this PR, the other mentors have told me it's not a great idea
(not in these words).
And I followed a tutorial so I can't assume it would work without it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, i was asking misleading question,
In case no social media is set in admin, will it work the same w/o this PR?

Copy link
Collaborator Author

@maayanhd maayanhd Apr 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@Yarboa sorry I'm still not entirely understand your question, what do you mean by 'social media is set in admin'? I wasn't setting anything since Django is registering the social media models related to OAuth2 of google, and these specific models are making trouble with the auto-model registration code since it's trying to register the models again. I don't think I can give up on those settings (and this is what generates these social media models of Django), I followed a tutorial so I believe every setting is crucial, and Django is having a very specific way to do so, as I read from several sources.

Copy link
Contributor

@Yarboa Yarboa Apr 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

There is pip install of django-allauth
And in App/url.py

This is default user, no option to work with non allauth.urls?
path('accounts/', include('allauth.urls'))

Ok looking here https://django-
allauth.readthedocs.io/en/latest/overview.html

Signup of both local and social accounts, did you check that you can login local?

How do you plan to test this feature?

NOTE: I suggest to describe better in the comment what are the capabilities,

try:
admin.site.register(model, admin_class)
except admin.sites.AlreadyRegistered:
pass
Loading