From 506eeaa8d0a3f30ac263f2868c9fc4ad333d157b Mon Sep 17 00:00:00 2001 From: Daniel Gal Date: Sat, 8 Feb 2020 14:45:11 +0100 Subject: [PATCH 1/6] Stdout print, redundant mkdir, code copy, cross-platform permissions --- Dockerfile.dev | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Dockerfile.dev b/Dockerfile.dev index 5a69edf..8d89237 100644 --- a/Dockerfile.dev +++ b/Dockerfile.dev @@ -1,15 +1,17 @@ FROM python:3.6-alpine +ENV PYTHONUNBUFFERED=1 ENV CONFIG_FILE=/code/config.yaml ENV PIP_NO_CACHE_DIR=false RUN apk add gcc python3-dev musl-dev postgresql-dev zlib-dev jpeg-dev libpq &&\ pip install pipenv -RUN mkdir /code WORKDIR /code COPY Pipfile Pipfile.lock /code/ RUN pipenv install --system --dev -ENTRYPOINT ["./docker-entrypoint.sh"] +COPY . /code/ + +ENTRYPOINT ["sh", "docker-entrypoint.sh"] From f9e268324584855be97d01b02d09bfc33e139128 Mon Sep 17 00:00:00 2001 From: Daniel Gal Date: Sun, 9 Feb 2020 21:50:17 +0100 Subject: [PATCH 2/6] First working version --- src/account/auth_pipeline.py | 15 +++++++++++++-- .../migrations/0005_profile_display_name.py | 19 +++++++++++++++++++ src/account/models.py | 1 + src/account/urls.py | 3 ++- src/account/views.py | 13 +++++++++++++ 5 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 src/account/migrations/0005_profile_display_name.py diff --git a/src/account/auth_pipeline.py b/src/account/auth_pipeline.py index 09bf423..e3775b6 100644 --- a/src/account/auth_pipeline.py +++ b/src/account/auth_pipeline.py @@ -1,11 +1,22 @@ from django.core import exceptions +from django.shortcuts import redirect +from social_core.pipeline.partial import partial +from rest_framework.response import Response from . import models -def create_profile(backend, user, response, *args, **kwargs): +@partial +def create_profile(strategy, backend, request, details, user, *args, **kwargs): if backend.name == "authsch": + print(user) + print(kwargs) try: user.profile except exceptions.ObjectDoesNotExist: - models.Profile.objects.create(user=user) + display_name = strategy.session_get("displayName", None) + if not display_name: + print("Redirecting to /first-login ...") + return redirect("/first-login") + print(f"Creating profile with display name {display_name} ...") + models.Profile.objects.create(user=user, display_name=display_name) diff --git a/src/account/migrations/0005_profile_display_name.py b/src/account/migrations/0005_profile_display_name.py new file mode 100644 index 0000000..f3c4346 --- /dev/null +++ b/src/account/migrations/0005_profile_display_name.py @@ -0,0 +1,19 @@ +# Generated by Django 2.2.9 on 2020-02-09 18:49 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('account', '0004_auto_20190315_1033'), + ] + + operations = [ + migrations.AddField( + model_name='profile', + name='display_name', + field=models.CharField(default='', max_length=50), + preserve_default=False, + ), + ] diff --git a/src/account/models.py b/src/account/models.py index c1485a3..4994113 100644 --- a/src/account/models.py +++ b/src/account/models.py @@ -9,6 +9,7 @@ class Profile(SoftDeleteModel): about_me = models.TextField(blank=True) is_score_visible = models.BooleanField(default=False) ranked = models.BooleanField(default=False) + display_name = models.CharField(max_length=50) @property def full_name(self): diff --git a/src/account/urls.py b/src/account/urls.py index 9d8ddcc..c8e767b 100644 --- a/src/account/urls.py +++ b/src/account/urls.py @@ -9,5 +9,6 @@ router.register(r"avatar", views.AvatarViewSet) urlpatterns = router.urls + [ - path("logout/", LogoutView.as_view(next_page="/"), name="logout") + path("logout/", LogoutView.as_view(next_page="/"), name="logout"), + path("display-name/", views.set_display_name), ] diff --git a/src/account/views.py b/src/account/views.py index 65c4cac..63e03fe 100644 --- a/src/account/views.py +++ b/src/account/views.py @@ -3,6 +3,12 @@ from rest_framework import permissions from rest_framework.decorators import action from rest_framework.response import Response +from django.shortcuts import redirect +from rest_framework.decorators import ( + api_view, + permission_classes, + authentication_classes, +) from common.mixins import RelativeURLFieldMixin from .permissions import IsSafeMethodOrIsOwnOrIsAdmin @@ -10,6 +16,13 @@ from . import serializers +@api_view(["POST"]) +@permission_classes([permissions.AllowAny]) +def set_display_name(request): + request.session["displayName"] = request.data["displayName"] + return redirect("/api/v1/complete/authsch") + + class ProfileViewSet( RelativeURLFieldMixin, generics.ListAPIView, From 6045b55a6d789a99b7406d31095e1d4222cb57d4 Mon Sep 17 00:00:00 2001 From: Daniel Gal Date: Sun, 9 Feb 2020 22:58:00 +0100 Subject: [PATCH 3/6] Refactors & fixes --- src/account/auth_pipeline.py | 5 ----- src/account/urls.py | 2 +- src/account/views.py | 1 - src/vikoverflow/middlewares.py | 1 + 4 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/account/auth_pipeline.py b/src/account/auth_pipeline.py index e3775b6..4905ce9 100644 --- a/src/account/auth_pipeline.py +++ b/src/account/auth_pipeline.py @@ -1,7 +1,6 @@ from django.core import exceptions from django.shortcuts import redirect from social_core.pipeline.partial import partial -from rest_framework.response import Response from . import models @@ -9,14 +8,10 @@ @partial def create_profile(strategy, backend, request, details, user, *args, **kwargs): if backend.name == "authsch": - print(user) - print(kwargs) try: user.profile except exceptions.ObjectDoesNotExist: display_name = strategy.session_get("displayName", None) if not display_name: - print("Redirecting to /first-login ...") return redirect("/first-login") - print(f"Creating profile with display name {display_name} ...") models.Profile.objects.create(user=user, display_name=display_name) diff --git a/src/account/urls.py b/src/account/urls.py index c8e767b..c9ded82 100644 --- a/src/account/urls.py +++ b/src/account/urls.py @@ -10,5 +10,5 @@ urlpatterns = router.urls + [ path("logout/", LogoutView.as_view(next_page="/"), name="logout"), - path("display-name/", views.set_display_name), + path("auth/display-name/", views.set_display_name), ] diff --git a/src/account/views.py b/src/account/views.py index 63e03fe..eece6f9 100644 --- a/src/account/views.py +++ b/src/account/views.py @@ -2,7 +2,6 @@ from rest_framework import generics from rest_framework import permissions from rest_framework.decorators import action -from rest_framework.response import Response from django.shortcuts import redirect from rest_framework.decorators import ( api_view, diff --git a/src/vikoverflow/middlewares.py b/src/vikoverflow/middlewares.py index f06dc6a..9d50531 100644 --- a/src/vikoverflow/middlewares.py +++ b/src/vikoverflow/middlewares.py @@ -12,6 +12,7 @@ def __call__(self, request): if ( "/api/v1/login" in request.path or "/api/v1/complete" in request.path + or "/api/v1/auth" in request.path or base.ALLOWED_USERS == ["*"] ): return self.get_response(request) From bb01bcc875522935227a181d02e0e301f74f7d31 Mon Sep 17 00:00:00 2001 From: Daniel Gal Date: Mon, 10 Feb 2020 00:48:45 +0100 Subject: [PATCH 4/6] 401 on no-context /display-name call --- src/account/views.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/account/views.py b/src/account/views.py index eece6f9..3fc8bc2 100644 --- a/src/account/views.py +++ b/src/account/views.py @@ -2,12 +2,9 @@ from rest_framework import generics from rest_framework import permissions from rest_framework.decorators import action +from rest_framework.response import Response from django.shortcuts import redirect -from rest_framework.decorators import ( - api_view, - permission_classes, - authentication_classes, -) +from rest_framework.decorators import api_view from common.mixins import RelativeURLFieldMixin from .permissions import IsSafeMethodOrIsOwnOrIsAdmin @@ -16,8 +13,9 @@ @api_view(["POST"]) -@permission_classes([permissions.AllowAny]) def set_display_name(request): + if len(request.session.values()) == 0: + return Response(status=401) request.session["displayName"] = request.data["displayName"] return redirect("/api/v1/complete/authsch") From 5985870e5fef3c5708d3d476adbdb721cb268303 Mon Sep 17 00:00:00 2001 From: Daniel Gal Date: Mon, 10 Feb 2020 12:08:52 +0100 Subject: [PATCH 5/6] Display name set before create user --- src/account/auth_pipeline.py | 24 ++++++++++++++++-------- src/vikoverflow/settings/base.py | 1 + 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/account/auth_pipeline.py b/src/account/auth_pipeline.py index 4905ce9..d292e1b 100644 --- a/src/account/auth_pipeline.py +++ b/src/account/auth_pipeline.py @@ -6,12 +6,20 @@ @partial +def set_display_name(strategy, backend, is_new=False, *args, **kwargs): + if not backend.name == "authsch" and not is_new: + return + display_name = strategy.session_get("displayName", None) + if not display_name: + return redirect("/first-login") + + def create_profile(strategy, backend, request, details, user, *args, **kwargs): - if backend.name == "authsch": - try: - user.profile - except exceptions.ObjectDoesNotExist: - display_name = strategy.session_get("displayName", None) - if not display_name: - return redirect("/first-login") - models.Profile.objects.create(user=user, display_name=display_name) + if not backend.name == "authsch": + return + try: + user.profile + except exceptions.ObjectDoesNotExist: + display_name = strategy.session_get("displayName", None) + models.Profile.objects.create(user=user, display_name=display_name) + diff --git a/src/vikoverflow/settings/base.py b/src/vikoverflow/settings/base.py index 3b477df..2219b8e 100644 --- a/src/vikoverflow/settings/base.py +++ b/src/vikoverflow/settings/base.py @@ -129,6 +129,7 @@ "social_core.pipeline.social_auth.auth_allowed", "social_core.pipeline.social_auth.social_user", "social_core.pipeline.user.get_username", + "account.auth_pipeline.set_display_name", "social_core.pipeline.user.create_user", "account.auth_pipeline.create_profile", "social_core.pipeline.social_auth.associate_user", From e914b903a69b19f652aafd72735a9540ad2eed28 Mon Sep 17 00:00:00 2001 From: Daniel Gal Date: Mon, 10 Feb 2020 12:18:47 +0100 Subject: [PATCH 6/6] Fixed & refactored guard conditions --- src/account/auth_pipeline.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/account/auth_pipeline.py b/src/account/auth_pipeline.py index d292e1b..5c2f674 100644 --- a/src/account/auth_pipeline.py +++ b/src/account/auth_pipeline.py @@ -7,19 +7,17 @@ @partial def set_display_name(strategy, backend, is_new=False, *args, **kwargs): - if not backend.name == "authsch" and not is_new: - return - display_name = strategy.session_get("displayName", None) - if not display_name: - return redirect("/first-login") + if backend.name == "authsch" and is_new: + display_name = strategy.session_get("displayName", None) + if not display_name: + return redirect("/first-login") def create_profile(strategy, backend, request, details, user, *args, **kwargs): - if not backend.name == "authsch": - return - try: - user.profile - except exceptions.ObjectDoesNotExist: - display_name = strategy.session_get("displayName", None) - models.Profile.objects.create(user=user, display_name=display_name) + if backend.name == "authsch": + try: + user.profile + except exceptions.ObjectDoesNotExist: + display_name = strategy.session_get("displayName", None) + models.Profile.objects.create(user=user, display_name=display_name)