2
\$\begingroup\$

I'm writing a social networking site in Django. I created an account application which is part of site project. I implemented an authentication mechanism in the project. Every user has different profile. A profile class is defined in models.py. In the profile each user can define date of birth and avatar. This actions is defined by edit function in views.py and ProfileEditForm class in forms.py. user_login method also check is given user active and is not banned. Logged users can also reset and change password, follow other users, view dashboard, view user list and view detailed info about another users. I'm not going to deploy this site because I created it as part of Django learning. I'm going to make a few simple Django project and then make something more complex. I'm learning Python together with Java. In Java I'm doing JavaFX and in Python I'm doing Django topic. Could I ask about any reviews below code? What do you think about this implementation of authentication mechanism?

urls.py:

from django.conf.urls import url
import django.contrib.auth.views
from . import views

urlpatterns = [
    # Poprzedni widok logowania.
    # url(r'^login/$', views.user_login, name='login'),

    # Wzorce adresów URL dla widoków logowania i wylogowania.
    url(r'^login/$',
        django.contrib.auth.views.login,
        name='login'),
    url(r'^logout/$',
        django.contrib.auth.views.logout,
        name='logout'),
    url(r'^logout-then-login/$',
        django.contrib.auth.views.logout_then_login,
        name='logout_then_login'),
    url(r'^$', views.dashboard, name='dashboard'),

    # Adresy URL przeznaczone do obsługi zmiany hasła.
    url(r'^password-change/$',
        django.contrib.auth.views.password_change,
        name='password_change'),
    url(r'^password-change/done/$',
        django.contrib.auth.views.password_change_done,
        name='password_change_done'),

    # Adresy URL przeznaczone do obsługi procedury zerowania hasła.
    url(r'^password-reset/$',
        django.contrib.auth.views.password_reset,
        name='password_reset'),
    url(r'^password-reset/done/$',
        django.contrib.auth.views.password_reset_done,
        name='password_reset_done'),
    url(r'^password-reset/confirm/(?P<uidb64>[-\w]+)/(?P<token>[-\w]+)/$',
        django.contrib.auth.views.password_reset_confirm,
        name='password_reset_confirm'),
    url(r'^password-reset/complete/$',
        django.contrib.auth.views.password_reset_complete,
        name='password_reset_complete'),

    # Rejestracja konta użytkownika i jego profil.
    url(r'^register/$', views.register, name='register'),
    url(r'^edit/$', views.edit, name='edit'),
    url(r'^users/$', views.user_list, name='user_list'),
    url(r'^users/follow/$', views.user_follow, name='user_follow'),
    url(r'^users/(?P<username>[-\w]+)/$',
        views.user_detail,
        name='user_detail'),
]

models.py:

from django.db import models
from django.conf import settings
from django.contrib.auth.models import User

class Profile(models.Model):
    user = models.OneToOneField(settings.AUTH_USER_MODEL)
    date_of_birth = models.DateField(blank=True, null=True)
    photo = models.ImageField(upload_to='users/%Y/%m/%d',
                              blank=True)

    def __str__(self):
        return 'Profil użytkownika {}.'.format(self.user.username)

class Contact(models.Model):
    user_from = models.ForeignKey(User,
                                  related_name='rel_from_set')
    user_to = models.ForeignKey(User,
                                related_name='rel_to_set')
    created = models.DateTimeField(auto_now_add=True,
                                   db_index=True)
    class Meta:
        ordering = ('-created',)

    def __str__(self):
        return '{} follows {}'.format(self.user_from,
                                      self.user_to)


# Dynamiczne dodanie poniższej kolumny do modelu User.
User.add_to_class('following',
                  models.ManyToManyField('self',
                                         through=Contact,
                                         related_name='followers',
                                         symmetrical=False))

forms.py:

from django import forms
from django.contrib.auth.models import User
from .models import Profile


class LoginForm(forms.Form):
    username = forms.CharField()
    password = forms.CharField(widget=forms.PasswordInput)


class UserRegistrationForm(forms.ModelForm):
    password = forms.CharField(label='Hasło',
                               widget=forms.PasswordInput)
    password2 = forms.CharField(label='Powtórz hasło',
                                widget=forms.PasswordInput)

    class Meta:
        model = User
        fields = ('username', 'first_name', 'email')

    def clean_password2(self):
        cd = self.cleaned_data
        if cd['password'] != cd['password2']:
            raise forms.ValidationError('Hasła nie są identyczne.')
        return cd['password2']


class UserEditForm(forms.ModelForm):
    class Meta:
        model = User
        fields = ('first_name', 'last_name', 'email')


class ProfileEditForm(forms.ModelForm):
    class Meta:
        model = Profile
        fields = ('date_of_birth', 'photo')

views.py:

from django.http import HttpResponse
from django.shortcuts import render
from django.contrib.auth import authenticate, login
from .forms import LoginForm, UserRegistrationForm, UserEditForm, ProfileEditForm
from django.contrib.auth.decorators import login_required
from .models import Profile
from django.contrib import messages
from django.shortcuts import get_object_or_404
from django.contrib.auth.models import User
from django.http import JsonResponse
from django.views.decorators.http import require_POST
from common.decorators import ajax_required
from .models import Contact
from actions.utils import create_action
from actions.models import Action


def user_login(request):
    if request.method == 'POST':
        form = LoginForm(request.POST)
        if form.is_valid():
            cd = form.cleaned_data
            user = authenticate(username=cd['username'],
                                password=cd['password'])
            if user is not None:
                if user.is_active:
                    login(request, user)
                    return HttpResponse('Uwierzytelnienie zakończyło się sukcesem.')
                else:
                    return HttpResponse('Konto jest zablokowane.')
            else:
                return HttpResponse('Nieprawidłowe dane uwierzytelniające.')
    else:
        form = LoginForm()
    return render(request, 'account/login.html', {'form': form})

@login_required
def dashboard(request):
    # Domyślnie wyświetlane są wszystkie akcje.
    actions = Action.objects.exclude(user=request.user)
    following_ids = request.user.following.values_list('id',
                                                       flat=True)
    if following_ids:
        # Jeżeli użytkownik obserwuje innych, będzie otrzymywał jedynie
        # informacje o podejmowanych przez nich akcjach.
        actions = actions.filter(user_id__in=following_ids).select_related('user', 'user__profile').prefetch_related('target')
    actions = actions[:10]

    return render(request,
                  'account/dashboard.html',
                  {'section': 'dashboard',
                   'actions': actions})


def register(request):
    if request.method == 'POST':
        user_form = UserRegistrationForm(request.POST)
        if user_form.is_valid():
            # Utworzenie nowego obiektu użytkownika,
            # ale jeszcze nie zapisujemy go w bazie danych.
            new_user = user_form.save(commit=False)
            # Ustawienie wybranego hasła.
            new_user.set_password(
                user_form.cleaned_data['password'])
            # Zapisanie obiektu User.
            new_user.save()
            # Utworzenie profilu użytkownika.
            profile = Profile.objects.create(user=new_user)
            create_action(new_user, 'utworzył konto')
            return render(request,
                          'account/register_done.html',
                          {'new_user': new_user})
    else:
        user_form = UserRegistrationForm()
    return render(request,
                  'account/register.html',
                  {'user_form': user_form})

@login_required
def edit(request):
    if request.method == 'POST':
        user_form = UserEditForm(instance=request.user,
                                 data=request.POST)
        profile_form = ProfileEditForm(
                                    instance=request.user.profile,
                                    data=request.POST,
                                    files=request.FILES)
        if user_form.is_valid() and profile_form.is_valid():
            user_form.save()
            profile_form.save()
            messages.success(request, 'Uaktualnienie profilu '\
                                      'zakończyło się sukcesem.')
        else:
            messages.error(request, 'Wystąpił błąd podczas uaktualniania profilu.')
    else:
        user_form = UserEditForm(instance=request.user)
        profile_form = ProfileEditForm(instance=request.user.profile)
    return render(request,
                  'account/edit.html',
                  {'user_form': user_form,
                   'profile_form': profile_form})

@login_required
def user_list(request):
    users = User.objects.filter(is_active=True)
    return render(request,
                  'account/user/list.html',
                  {'section': 'people',
                   'users': users})

@login_required
def user_detail(request, username):
    user = get_object_or_404(User,
                             username=username,
                             is_active=True)
    return render(request,
                  'account/user/detail.html',
                  {'section': 'people',
                   'user': user})

@ajax_required
@require_POST
@login_required
def user_follow(request):
    user_id = request.POST.get('id')
    action = request.POST.get('action')
    if user_id and action:
        try:
            user = User.objects.get(id=user_id)
            if action == 'follow':
                Contact.objects.get_or_create(
                    user_from=request.user,
                    user_to=user)
                create_action(request.user, 'obserwuje', user)
            else:
                Contact.objects.filter(user_from=request.user,
                                       user_to=user).delete()
            return JsonResponse({'status':'ok'})
        except User.DoesNotExist:
            return JsonResponse({'status':'ok'})
    return JsonResponse({'status':'ok'})
\$\endgroup\$
  • \$\begingroup\$ No, please don't translate the code; we want to review your real code in its natural state. Internationalization issues are a matter that is addressable in review. @pgs Just paste the whole thing into Google Translate. \$\endgroup\$ – 200_success May 3 '17 at 16:40
1
\$\begingroup\$
  1. For auth purpose have a look at django-allauth ,It provides huge features which i feel is required in your context . It provides social logins , auth features,setting up multiple emails and many more.
  2. You probably are using older version of django , you should set on_delete on the latest one .
  3. You are creating a overhead to check the models fields by putting blank=True and null=True , In this condition you have to check if the field is blank or not and even if the field is null or not.
  4. related_name seems ugly and long for me. Try making your related_name, name of the field(in singular)
  5. Try placing default image instead of making it blank=True , since this field is for Profile, and you would like to show some default image if the user hasn't uploaded an image .
  6. Making an abstract TimeStamp Model can be fine , because you would required created_at , created_by e.t.c field for many of your models.
  7. Instead of password ,password2 ,you can make confirm_password so as to make your code readable
  8. If you have only few urls which doesn't require login ,then you can place those urls on settings as login_exempt_urls and create middleware to allow unathenticated access to those urls and remaining urls to have authentication required. This method will help you to avoid login_required on each and every view.
  9. For your dashboard view,try avoiding creating a list of following_id and then performing an ORM , this method would be fine for low data ,but trust me it will affect you badly for huge data .Instead of actions = actions[:10] , you should limit your data from ORM. This way you are fetching every data .
  10. You can use CBV if you would like to go for more advaned. Trust me CBV are much easier and keeps your code really clean .
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy