From e75d33439accc97e2464b03606162609efa58920 Mon Sep 17 00:00:00 2001 From: "xiaokong1937@gmail.com" Date: Mon, 5 Sep 2016 21:38:21 +0800 Subject: [PATCH 1/3] fix captcha not valid bug of login view; use django's default login and logout view to enhance robustness --- apps/templates/_header_bar.html | 8 ++- apps/templates/_user_profile.html | 4 +- apps/users/forms.py | 20 +++++--- apps/users/templates/users/login.html | 10 ++-- apps/users/urls.py | 44 +++++++++++++---- apps/users/views.py | 71 +++++---------------------- 6 files changed, 70 insertions(+), 87 deletions(-) diff --git a/apps/templates/_header_bar.html b/apps/templates/_header_bar.html index 66699be53..71bd0e95e 100644 --- a/apps/templates/_header_bar.html +++ b/apps/templates/_header_bar.html @@ -19,9 +19,15 @@
  • + {% if user.is_authenticated %} - Log out + {% trans 'Log out' %} + {% else %} + + {% trans 'Log in' %} + + {% endif %}
    • diff --git a/apps/templates/_user_profile.html b/apps/templates/_user_profile.html index 13d272d90..d47ba6537 100644 --- a/apps/templates/_user_profile.html +++ b/apps/templates/_user_profile.html @@ -8,10 +8,10 @@ - {{ request.user.name }} + {{ user.name }} - {{ request.user.get_role_display | default:"{% trans 'User' %}" }} + {{ user.get_role_display | default:_('User') }} diff --git a/apps/users/forms.py b/apps/users/forms.py index 88b6ccd93..3f2dcfa98 100644 --- a/apps/users/forms.py +++ b/apps/users/forms.py @@ -1,20 +1,24 @@ # ~*~ coding: utf-8 ~*~ -from django.forms import ModelForm from django import forms -from captcha.fields import CaptchaField +from django.contrib.auth.forms import AuthenticationForm from django.utils.translation import gettext_lazy as _ +from captcha.fields import CaptchaField + from .models import User, UserGroup -class UserLoginForm(forms.Form): +class UserLoginForm(AuthenticationForm): username = forms.CharField(label=_('Username'), max_length=100) - password = forms.CharField(label=_('Password'), widget=forms.PasswordInput, max_length=100) + password = forms.CharField( + label=_('Password'), widget=forms.PasswordInput, max_length=100, + strip=False) captcha = CaptchaField() -class UserAddForm(ModelForm): +class UserAddForm(forms.ModelForm): + class Meta: model = User fields = [ @@ -32,7 +36,8 @@ class UserAddForm(ModelForm): } -class UserUpdateForm(ModelForm): +class UserUpdateForm(forms.ModelForm): + class Meta: model = User fields = [ @@ -51,7 +56,8 @@ class UserUpdateForm(ModelForm): } -class UserGroupForm(ModelForm): +class UserGroupForm(forms.ModelForm): + class Meta: model = UserGroup diff --git a/apps/users/templates/users/login.html b/apps/users/templates/users/login.html index b2cc09668..81fd5252e 100644 --- a/apps/users/templates/users/login.html +++ b/apps/users/templates/users/login.html @@ -40,11 +40,10 @@ {% if form.errors %} {% if 'captcha' in form.errors %}

    {% trans 'Captcha invalid' %}

    + {% else %} +

    {{ form.non_field_errors.as_text }}

    {% endif %} {% endif %} - {% if errors %} -

    {{ errors }}

    - {% endif %}
    @@ -57,7 +56,7 @@ - Forgot password? + {% trans 'Forgot password' %}?

    @@ -79,7 +78,4 @@ - - - diff --git a/apps/users/urls.py b/apps/users/urls.py index 27f159f1c..e2b0308e8 100644 --- a/apps/users/urls.py +++ b/apps/users/urls.py @@ -1,37 +1,61 @@ -from django.conf.urls import url, include +from django.conf.urls import url from django.contrib.auth import views as auth_views -from django.urls import reverse_lazy +from django.utils.translation import ugettext as _ import views import api +from users.forms import UserLoginForm + app_name = 'users' urlpatterns = [ - url(r'^login$', views.UserLoginView.as_view(), name='login'), - url(r'^logout$', views.UserLogoutView.as_view(), name='logout'), + url(r'^login$', + auth_views.login, + {'template_name': "users/login.html", + 'authentication_form': UserLoginForm, + 'redirect_authenticated_user': True}, + name='login'), + url(r'^logout$', + auth_views.logout, + { + "template_name": "common/flash_message_standalone.html", + "extra_context": { + 'title': _('Logout success'), + 'messages': _('Logout success, return login page'), + 'redirect_url': '/users/login', + 'auto_redirect': True, + } + }, + name='logout'), url(r'^password/forget$', views.UserForgetPasswordView.as_view(), name='forget-password'), url(r'^password/forget/sendmail-success$', views.UserForgetPasswordSendmailSuccessView.as_view(), name='forget-password-sendmail-success'), url(r'^password/reset$', views.UserResetPasswordView.as_view(), name='reset-password'), - url(r'^password/reset/success$', views.UserResetPasswordSuccessView.as_view(), name='reset-password-success'), + url(r'^password/reset/success$', views.UserResetPasswordSuccessView.as_view(), + name='reset-password-success'), url(r'^user$', views.UserListView.as_view(), name='user-list'), url(r'^user/(?P[0-9]+)$', views.UserDetailView.as_view(), name='user-detail'), url(r'^user/add$', views.UserAddView.as_view(), name='user-add'), url(r'^user/(?P[0-9]+)/edit$', views.UserUpdateView.as_view(), name='user-edit'), url(r'^user/(?P[0-9]+)/delete$', views.UserDeleteView.as_view(), name='user-delete'), url(r'^usergroup$', views.UserGroupListView.as_view(), name='usergroup-list'), - url(r'^usergroup/(?P[0-9]+)$', views.UserGroupDetailView.as_view(), name='usergroup-detail'), + url(r'^usergroup/(?P[0-9]+)$', + views.UserGroupDetailView.as_view(), name='usergroup-detail'), url(r'^usergroup/add/$', views.UserGroupAddView.as_view(), name='usergroup-add'), - url(r'^usergroup/(?P[0-9]+)/edit$', views.UserGroupUpdateView.as_view(), name='usergroup-edit'), - url(r'^usergroup/(?P[0-9]+)/delete$', views.UserGroupDeleteView.as_view(), name='usergroup-delete'), + url(r'^usergroup/(?P[0-9]+)/edit$', + views.UserGroupUpdateView.as_view(), name='usergroup-edit'), + url(r'^usergroup/(?P[0-9]+)/delete$', + views.UserGroupDeleteView.as_view(), name='usergroup-delete'), ] urlpatterns += [ url(r'^v1/users$', api.UserListAddApi.as_view(), name='user-list-api'), - url(r'^v1/users/(?P[0-9]+)$', api.UserDetailDeleteUpdateApi.as_view(), name='user-detail-api'), + url(r'^v1/users/(?P[0-9]+)$', + api.UserDetailDeleteUpdateApi.as_view(), name='user-detail-api'), url(r'^v1/users/(?P[0-9]+)/active$', api.UserActiveApi.as_view(), name='user-active-api'), url(r'^v1/usergroups$', api.UserGroupListAddApi.as_view(), name='usergroup-list-api'), - url(r'^v1/usergroups/(?P[0-9]+)$', api.UserGroupDetailDeleteUpdateApi.as_view(), name='usergroup-detail-api'), + url(r'^v1/usergroups/(?P[0-9]+)$', + api.UserGroupDetailDeleteUpdateApi.as_view(), name='usergroup-detail-api'), ] diff --git a/apps/users/views.py b/apps/users/views.py index 5d61525d8..19245b5f8 100644 --- a/apps/users/views.py +++ b/apps/users/views.py @@ -4,78 +4,28 @@ from __future__ import unicode_literals import logging -from django.shortcuts import get_object_or_404, reverse, render, Http404, redirect +from django.conf import settings +from django.contrib.messages.views import SuccessMessageMixin +from django.db.models import Q +from django.http import HttpResponseRedirect +from django.shortcuts import get_object_or_404, reverse from django.urls import reverse_lazy from django.utils.translation import ugettext as _ -from django.db.models import Q -from django.views.generic.base import View, TemplateView +from django.views.generic.base import TemplateView from django.views.generic.list import ListView -from django.views.generic.edit import CreateView, DeleteView, UpdateView, ProcessFormView, FormView +from django.views.generic.edit import CreateView, DeleteView, UpdateView from django.views.generic.detail import DetailView -from django.contrib.messages.views import SuccessMessageMixin -from django.conf import settings -from django.http import HttpResponseRedirect -from django.contrib.auth import views as auth_view, authenticate, login, logout from common.utils import get_object_or_none from .models import User, UserGroup -from .forms import UserAddForm, UserUpdateForm, UserGroupForm, UserLoginForm -from .utils import AdminUserRequiredMixin, ssh_key_gen, user_add_success_next, send_reset_password_mail +from .forms import UserAddForm, UserUpdateForm, UserGroupForm +from .utils import AdminUserRequiredMixin, user_add_success_next, send_reset_password_mail logger = logging.getLogger('jumpserver.users.views') -class UserLoginView(FormView): - template_name = 'users/login.html' - form_class = UserLoginForm - redirect_field_name = 'next' - - def get(self, request, *args, **kwargs): - if self.request.user.is_staff: - return redirect(request.POST.get(self.redirect_field_name, reverse('index'))) - # Todo: Django have bug, lose context issue: https://github.com/django/django/pull/7202 - # so we jump it and use origin method render_to_response - # return super(UserLoginView, self).get(request, *args, **kwargs) - return self.render_to_response(self.get_context_data(**kwargs)) - - def post(self, request, *args, **kwargs): - form = self.get_form() - if not form.is_valid(): - return self.form_invalid(form) - - username = form['username'].value() - password = form['password'].value() - - user = authenticate(username=username, password=password) - if user is None: - kwargs.update({'errors': _('Username or password invalid')}) - return self.get(request, *args, **kwargs) - - login(request, user) - return redirect(request.GET.get(self.redirect_field_name, reverse('index'))) - - -class UserLogoutView(TemplateView): - template_name = 'common/flash_message_standalone.html' - - def get(self, request, *args, **kwargs): - logout(request) - - return super(UserLogoutView, self).get(request) - - def get_context_data(self, **kwargs): - context = { - 'title': _('Logout success'), - 'messages': _('Logout success, return login page'), - 'redirect_url': reverse('users:login'), - 'auto_redirect': True, - } - kwargs.update(context) - return super(UserLogoutView, self).get_context_data(**kwargs) - - class UserListView(AdminUserRequiredMixin, ListView): model = User paginate_by = settings.CONFIG.DISPLAY_PER_PAGE @@ -166,7 +116,8 @@ class UserDetailView(AdminUserRequiredMixin, DetailView): context_object_name = "user" def get_context_data(self, **kwargs): - groups = [group for group in UserGroup.objects.iterator() if group not in self.object.groups.iterator()] + groups = [ + group for group in UserGroup.objects.iterator() if group not in self.object.groups.iterator()] context = {'app': _('Users'), 'action': _('User detail'), 'groups': groups} kwargs.update(context) return super(UserDetailView, self).get_context_data(**kwargs) From 6aedfb52199ab0dc14ddeb292040cf6dc9ad1928 Mon Sep 17 00:00:00 2001 From: "xiaokong1937@gmail.com" <763691951@qq.com> Date: Mon, 5 Sep 2016 22:20:50 +0800 Subject: [PATCH 2/3] fix small template erros --- apps/users/templates/users/_user.html | 4 ++-- apps/users/templates/users/user_edit.html | 3 ++- apps/users/templates/users/user_list.html | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/apps/users/templates/users/_user.html b/apps/users/templates/users/_user.html index b8acec82c..937b25005 100644 --- a/apps/users/templates/users/_user.html +++ b/apps/users/templates/users/_user.html @@ -14,7 +14,7 @@

    -
    {% trans 'Create user' %}
    +
    {% block user_template_title %}{% trans 'Create user' %}{% endblock %}
    @@ -94,4 +94,4 @@ }); }) -{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/apps/users/templates/users/user_edit.html b/apps/users/templates/users/user_edit.html index fb3725a75..4d7c842d3 100644 --- a/apps/users/templates/users/user_edit.html +++ b/apps/users/templates/users/user_edit.html @@ -1,5 +1,6 @@ {% extends 'users/_user.html' %} {% load i18n %} +{% block user_template_title %}{% trans "Edit user" %}{% endblock %} {% block username %}
    @@ -16,4 +17,4 @@
    -{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/apps/users/templates/users/user_list.html b/apps/users/templates/users/user_list.html index 51fbfa57b..67654b794 100644 --- a/apps/users/templates/users/user_list.html +++ b/apps/users/templates/users/user_list.html @@ -34,7 +34,7 @@ {{ user.groups.all|join_queryset_attr:"name" }} {{ user.name }} - {% if user.is_expired %} + {% if user.is_expired and user.is_active %} {% else %} From 3c6f50b788a5d24953bc5ef8aa54436cf6110db0 Mon Sep 17 00:00:00 2001 From: ibuler Date: Mon, 5 Sep 2016 23:42:10 +0800 Subject: [PATCH 3/3] Modify some bug --- apps/templates/_user_profile.html | 2 +- apps/users/views.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/apps/templates/_user_profile.html b/apps/templates/_user_profile.html index d47ba6537..04a146496 100644 --- a/apps/templates/_user_profile.html +++ b/apps/templates/_user_profile.html @@ -11,7 +11,7 @@ {{ user.name }} - {{ user.get_role_display | default:_('User') }} + {{ user.get_role_display | default:{% trans 'User' %} }} diff --git a/apps/users/views.py b/apps/users/views.py index 19245b5f8..2b2ed441d 100644 --- a/apps/users/views.py +++ b/apps/users/views.py @@ -116,8 +116,7 @@ class UserDetailView(AdminUserRequiredMixin, DetailView): context_object_name = "user" def get_context_data(self, **kwargs): - groups = [ - group for group in UserGroup.objects.iterator() if group not in self.object.groups.iterator()] + groups = [group for group in UserGroup.objects.iterator() if group not in self.object.groups.iterator()] context = {'app': _('Users'), 'action': _('User detail'), 'groups': groups} kwargs.update(context) return super(UserDetailView, self).get_context_data(**kwargs)