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.

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

add admin role/login feature #1784

Merged
merged 2 commits into from
Sep 9, 2017
Merged

add admin role/login feature #1784

merged 2 commits into from
Sep 9, 2017

Conversation

imwhatiam
Copy link
Member

No description provided.

@imwhatiam imwhatiam force-pushed the admin-role-log branch 4 times, most recently from e12edda to b618b45 Compare September 5, 2017 06:30
@@ -101,6 +105,14 @@ def get(self, email=None, id=None):
user.source = emailuser.source
user.role = emailuser.role

try:
Copy link
Contributor

Choose a reason for hiding this comment

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

普通用户需要设置admin role 字段?

@@ -408,6 +451,14 @@ def get_user_with_import(self, username):
user.source = emailuser.source
user.role = emailuser.role

try:
Copy link
Contributor

Choose a reason for hiding this comment

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

同上

@@ -4,74 +4,108 @@
<a href="#" title="{% trans "Close" %}" aria-label="{% trans "Close" %}" class="sf2-icon-x1 sf-popover-close op-icon hidden-md-up js-close-side-nav fright"></a>
<h3 class="hd">{% trans "System Admin" %}</h3>
<ul class="side-tabnav-tabs">

{% if admin_permissions.can_view_system_info %}
Copy link
Contributor

Choose a reason for hiding this comment

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

用 request.user.admin_permission..., 这样就不需要context processor

return Response(result)

def post(self, request):
""" Add an role for an admin user.
Copy link
Member

Choose a reason for hiding this comment

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

'an role' --> 'a role'

return Response(result)

def put(self, request):
""" Update an role for an admin user.
Copy link
Member

Choose a reason for hiding this comment

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

'an role' --> 'a role'

<div class="hd ovhd">
<ul class="tab-tabs-nav fleft">
<li class="tab <% if (cur_tab == 'admin_logs') { %> ui-state-active <% } %>">
<a href="#admin-logs/" class="a">{% trans "Admin Logs" %}</a>
Copy link
Member

Choose a reason for hiding this comment

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

"Admin Logs" --> "Admin Operation Logs"

<div class="hd ovhd">
<ul class="tab-tabs-nav fleft">
<li class="tab <% if (cur_tab == 'admin_logs') { %> ui-state-active <% } %>">
<a href="#admin-logs/" class="a">{% trans "Admin Logs" %}</a>
Copy link
Member

Choose a reason for hiding this comment

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

"Admin Logs" --> "Admin Operation Logs"

Copy link
Member

Choose a reason for hiding this comment

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

#admin-logs/

<script type="text/template" id="admin-login-logs-tmpl">
<div class="hd ovhd">
<ul class="tab-tabs-nav fleft">
<li class="tab <% if (cur_tab == 'admin_logs') { %> ui-state-active <% } %>">
Copy link
Member

@llj llj Sep 7, 2017

Choose a reason for hiding this comment

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

是否 cur_tab 是确定的。

<thead>
<tr>
<th width="35%">{% trans "Email" %}</th>
<th width="25%">{% trans "IP" %}</th>
Copy link
Member

Choose a reason for hiding this comment

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

需要 trans ?

<td>{% trans "Success" %}</td>
<% } else { %>
<td>{% trans "Failed" %}</td>
<% } %>
Copy link
Member

Choose a reason for hiding this comment

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

mark

@imwhatiam imwhatiam force-pushed the admin-role-log branch 2 times, most recently from 80f52bf to 949af15 Compare September 7, 2017 11:05
<li class="tab">
<a href="{{ SITE_ROOT }}sys/trafficadmin/"><span class="sf2-icon-histogram"></span>{% trans "Traffic" %}</a>
</li>
{% endif %}
Copy link
Member

Choose a reason for hiding this comment

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

'Traffic' is moved into 'Statistic'. Remove it.

<li class="tab {% block cur_traffic %}{% endblock %}">
<a href="{{ SITE_ROOT }}sys/trafficadmin/"><span class="sf2-icon-histogram"></span>{% trans "Traffic" %}</a>
</li>
{% endif %}
Copy link
Member

Choose a reason for hiding this comment

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

'Traffic' is moved into 'Statistic'. Remove it.

@@ -86,7 +86,9 @@ <h3 class="hd">{% trans "Add admins"%}</h3>
},
error: function(xhr) {
var error_msg;
if (xhr.responseText) {
if (xhr.status === 403) {
error_msg = 'Permission denied.'
Copy link
Member

Choose a reason for hiding this comment

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

translate.

"can_manage_user": '{{ user.admin_permissions.can_manage_user }}' === 'True',
"can_manage_group": '{{ user.admin_permissions.can_manage_group }}' === 'True',
"can_view_user_log": '{{ user.admin_permissions.can_view_user_log }}' === 'True',
"can_view_admin_log": '{{ user.admin_permissions.can_view_admin_log }}' === 'True'
Copy link
Member

Choose a reason for hiding this comment

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

mark.

@@ -86,6 +86,37 @@
});
});

$('.admin-role-select').change(function() {
var select = $(this),
Copy link
Member

Choose a reason for hiding this comment

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

$select

@@ -86,6 +86,37 @@
});
});

$('.admin-role-select').change(function() {
var select = $(this),
select_val = select.val(),
Copy link
Member

Choose a reason for hiding this comment

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

select_val --> role

beforeSend: prepareCSRFToken,
success: function(data) {
feedback("{% trans "Edit succeeded" %}", 'success');
$('.admin-role-cur-value', $select_prev).html(select.children('option[value="' +select.val() + '"]').text());
Copy link
Member

Choose a reason for hiding this comment

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

+select.val() + 后加空格。

@@ -86,6 +86,37 @@
});
});

$('.admin-role-select').change(function() {
Copy link
Member

Choose a reason for hiding this comment

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

put it into 'admin' page.

<th width="33%">{% trans "Email" %} / {% trans "Name" %} / {% trans "Contact Email" %}</th>
<th width="12%">{% trans "Status" %}</th>
<th width="34%">{% trans "Email" %} / {% trans "Name" %} / {% trans "Contact Email" %}</th>
<th width="14%">{% trans "Status" %}</th>
Copy link
Member

Choose a reason for hiding this comment

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

上面的 width 值,原来总值是 45%, 你改完变成 48% 了。原来是对的。

{% elif user.admin_role == daily_admin %}
<span class="admin-role-cur-value">{% trans "Daily Administrator" %}</span>
{% elif user.admin_role == audit_admin %}
<span class="admin-role-cur-value">{% trans "Audit Administrator" %}</span>
Copy link
Member

Choose a reason for hiding this comment

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

'Administrator' --> 'Admin'. add 'context' for 'trans'.

Copy link
Member

Choose a reason for hiding this comment

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

{% trans "Default Admin" context "Default Administrator" %}

<option value={{ daily_admin }}{% if user.admin_role == daily_admin %} selected="selected"{% endif %}>{% trans "Daily Administrator" %}</option>
<option value={{ audit_admin }}{% if user.admin_role == audit_admin %} selected="selected"{% endif %}>{% trans "Audit Administrator" %}</option>
{% for role in extra_admin_roles %}
<option value={{ role }}{% if user.admin_role == role %} selected="selected"{% endif %}>{{ role }}</option>
Copy link
Member

Choose a reason for hiding this comment

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

value={{ ... }} --> value="{{ ... }}". 引号加上。上面5行。

user.admin_role = DEFAULT_ADMIN

extra_admin_roles = [x for x in get_available_admin_roles()
if x not in get_basic_admin_roles()]
Copy link
Member

Choose a reason for hiding this comment

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

mark. be simpler?

ldpa_imported_users = seaserv.get_emailusers('LDAPImport', -1, -1)

db_users = ccnet_api.get_emailusers('DB', -1, -1)
ldpa_imported_users = ccnet_api.get_emailusers('LDAPImport', -1, -1)
Copy link
Member

Choose a reason for hiding this comment

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

ldpa_... --> ldap_...

define([
'underscore',
'backbone',
'common',
Copy link
Member

Choose a reason for hiding this comment

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

逗号删了。

}
},

addOne: function(login) {
Copy link
Member

Choose a reason for hiding this comment

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

‘login'?


Permission checking:
1. only admin can perform this action.
2. email(from argument): must be a admin user.
Copy link
Member

Choose a reason for hiding this comment

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

an

## 2. email(from argument): must be a admin user.
if not user.is_staff:
error_msg = "%s must be an administrator." % email
return api_error(status.HTTP_403_FORBIDDEN, error_msg)
Copy link
Member

Choose a reason for hiding this comment

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

##1 & ##2 顺序换下。


Permission checking:
1. only admin with `default_admin` role can perform this action.
2. email(from argument): must be a admin user.
Copy link
Member

Choose a reason for hiding this comment

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

an

## 2. email(from argument): must be a admin user.
if not user.is_staff:
error_msg = "%s must be an administrator." % email
return api_error(status.HTTP_403_FORBIDDEN, error_msg)
Copy link
Member

Choose a reason for hiding this comment

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

##1 & ##2 顺序换下


class LoginLogs(APIView):

authentication_classes = (TokenAuthentication, SessionAuthentication )
Copy link
Member

Choose a reason for hiding this comment

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

) 前有个空格。

from seahub.api2.utils import api_error

from seahub.api2.endpoints.utils import get_user_name_dict, \
get_user_contact_email_dict
Copy link
Member

Choose a reason for hiding this comment

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

这两行可以和上面的 line 13 合并。


permission_url = {
'can_view_system_info': [
'sys/settings',
Copy link
Member

Choose a reason for hiding this comment

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

mark

'api/v2.1/admin/favicon',
'api/v2.1/admin/license',
'api/v2.1/admin/login-background-image',
],
Copy link
Member

Choose a reason for hiding this comment

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

mark.

return amdin_role

def get_admin_role(self, username):
""" Get admin role of an user.
Copy link
Member

@llj llj Sep 9, 2017

Choose a reason for hiding this comment

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

'a user'

'can_view_user_log': True,
'can_view_admin_log': True,
},
# SYSTEM_ADMIN can ONLY view system-info(without upload licence), settings pages.
Copy link
Member

Choose a reason for hiding this comment

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

mark.
'upload licence'

"can_manage_user": {% if user.admin_permissions.can_manage_user %} true {% else %} false {% endif %},
"can_manage_group": {% if user.admin_permissions.can_manage_group %} true {% else %} false {% endif %},
"can_view_user_log": {% if user.admin_permissions.can_view_user_log %} true {% else %} false {% endif %},
"can_view_admin_log": {% if user.admin_permissions.can_view_admin_log %} true {% else %} false {% endif %},
Copy link
Member

Choose a reason for hiding this comment

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

逗号删了。

<option value={{ daily_admin }}{% if user.admin_role == daily_admin %} selected="selected"{% endif %}>{% trans "Daily Admin" context "Daily Administrator" %}</option>
<option value={{ audit_admin }}{% if user.admin_role == audit_admin %} selected="selected"{% endif %}>{% trans "Audit Admin" context "Audit Administrator" %}</option>
{% for role in extra_admin_roles %}
<option value={{ role }}{% if user.admin_role == role %} selected="selected"{% endif %}>{{ role }}</option>
Copy link
Member

Choose a reason for hiding this comment

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

上面 5 行, value={{...}} --> value="{{...}}", 加引号

seahub/urls.py Outdated
@@ -313,7 +313,10 @@

## admin::shares
url(r'^api/v2.1/admin/shares/$', AdminShares.as_view(), name='api-v2.1-admin-shares'),
url(r'^api/v2.1/admin/admin-logs/$', AdminLogs.as_view(), name='api-v2.1-admin-admin-logs'),

## admin::admin operation logs
Copy link
Member

Choose a reason for hiding this comment

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

rm 'operation'

{% else %}
<span class="admin-role-cur-value">{{user.admin_role}}</span>
{% endif %}
<span title="{% trans "Edit"%}" class="admin-role-edit-icon sf2-icon-edit op-icon vh"></span>
Copy link
Member

Choose a reason for hiding this comment

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

这里要加上权限判断:default_admin 才可以编辑。

Copy link
Member

Choose a reason for hiding this comment

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

相应的 js 部分也加上吧。

@llj llj merged commit 45fb407 into master Sep 9, 2017
@imwhatiam imwhatiam deleted the admin-role-log branch February 1, 2018 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants