Selaa lähdekoodia

HUE-615. Basic group management

Create, modify membership, and delete groups. Group membership doesn't do
anything yet. But it will!
Jon Natkins 13 vuotta sitten
vanhempi
commit
afbd78c39d

+ 62 - 0
apps/useradmin/src/useradmin/templates/edit_group.mako

@@ -0,0 +1,62 @@
+## Licensed to Cloudera, Inc. under one
+## or more contributor license agreements.  See the NOTICE file
+## distributed with this work for additional information
+## regarding copyright ownership.  Cloudera, Inc. licenses this file
+## to you under the Apache License, Version 2.0 (the
+## "License"); you may not use this file except in compliance
+## with the License.  You may obtain a copy of the License at
+##
+##     http://www.apache.org/licenses/LICENSE-2.0
+##
+## Unless required by applicable law or agreed to in writing, software
+## distributed under the License is distributed on an "AS IS" BASIS,
+## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+## See the License for the specific language governing permissions and
+## limitations under the License.
+<%!
+from desktop.views import commonheader, commonfooter
+%>
+<% import urllib %>
+
+<%namespace name="layout" file="layout.mako" />
+${layout.menubar(section='groups')}
+
+<div class="container-fluid">
+  % if name:
+	${commonheader('Edit Group: ' + name + ' -- Hue Groups', "useradmin", "100px")}
+	<h1>Edit Group: ${name} -- Hue Groups</h1>
+  % else:
+    ${commonheader('Create Group -- Hue Groups', "useradmin", "100px")}
+    <h1>Create Group -- Hue Groups</h1>
+  % endif
+	<form action="${urllib.quote(action)}" method="POST" class="jframe_padded">
+		<fieldset>
+			<legend>
+			  % if name:
+		        Edit Group: ${name}
+		      % else:
+		        Create Group
+		      % endif
+			</legend>
+        <%def name="render_field(field)">
+			<div class="clearfix">
+				${field.label_tag() | n}
+				<div class="input">
+					${unicode(field) | n}
+				</div>
+				% if len(field.errors):
+					${unicode(field.errors) | n}
+				% endif
+			</div>
+		</%def>
+
+		% for field in form:
+			${render_field(field)}
+		% endfor
+        </fieldset>
+		<div class="actions">
+			<input type="submit" value="Save" class="btn primary"/>
+		</div>
+	</form>
+</div>
+${commonfooter()}

+ 6 - 2
apps/useradmin/src/useradmin/templates/edit_user.mako

@@ -17,12 +17,16 @@
 from desktop.views import commonheader, commonfooter
 %>
 <% import urllib %>
+
+<%namespace name="layout" file="layout.mako" />
+${layout.menubar(section='users')}
+
 <div class="container-fluid">
   % if username:
-	${commonheader('Edit User: ' + username + ' -- Hue Users', "useradmin")}
+	${commonheader('Edit User: ' + username + ' -- Hue Users', "useradmin", "100px")}
 	<h1>Edit User: ${username} -- Hue Users</h1>
   % else:
-    ${commonheader('Create User -- Hue Users', "useradmin")}
+    ${commonheader('Create User -- Hue Users', "useradmin", "100px")}
     <h1>Create User -- Hue Users</h1>
   % endif
 	<form action="${urllib.quote(action)}" method="POST" class="jframe_padded">

+ 40 - 0
apps/useradmin/src/useradmin/templates/layout.mako

@@ -0,0 +1,40 @@
+## Licensed to Cloudera, Inc. under one
+## or more contributor license agreements.  See the NOTICE file
+## distributed with this work for additional information
+## regarding copyright ownership.  Cloudera, Inc. licenses this file
+## to you under the Apache License, Version 2.0 (the
+## "License"); you may not use this file except in compliance
+## with the License.  You may obtain a copy of the License at
+##
+##     http://www.apache.org/licenses/LICENSE-2.0
+##
+## Unless required by applicable law or agreed to in writing, software
+## distributed under the License is distributed on an "AS IS" BASIS,
+## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+## See the License for the specific language governing permissions and
+## limitations under the License.
+##
+##
+## no spaces in this method please; we're declaring a CSS class, and ART uses this value for stuff, and it splits on spaces, and 
+## multiple spaces and line breaks cause issues
+<%!
+def is_selected(section, matcher):
+  if section == matcher:
+    return "selected"
+  else:
+    return ""
+%>
+
+<%def name="menubar(section='')">
+	<div class="menubar">
+		<div class="menubar-inner">
+			<div class="container-fluid">
+				<ul class="nav">
+					<li><a href="/useradmin/users" class="${is_selected(section, 'users')}">Users</a></li>
+					<li><a href="/useradmin/groups" class="${is_selected(section, 'groups')}">Groups</a></li>
+				</ul>
+			</div>
+		</div>
+	</div>
+</%def>
+

+ 123 - 0
apps/useradmin/src/useradmin/templates/list_groups.mako

@@ -0,0 +1,123 @@
+## Licensed to Cloudera, Inc. under one
+## or more contributor license agreements.  See the NOTICE file
+## distributed with this work for additional information
+## regarding copyright ownership.  Cloudera, Inc. licenses this file
+## to you under the Apache License, Version 2.0 (the
+## "License"); you may not use this file except in compliance
+## with the License.  You may obtain a copy of the License at
+##
+##     http://www.apache.org/licenses/LICENSE-2.0
+##
+## Unless required by applicable law or agreed to in writing, software
+## distributed under the License is distributed on an "AS IS" BASIS,
+## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+## See the License for the specific language governing permissions and
+## limitations under the License.
+<%!
+from desktop.views import commonheader, commonfooter
+%>
+<% import urllib %>
+<% from django.utils.translation import ugettext, ungettext, get_language, activate %>
+<% _ = ugettext %>
+
+<%namespace name="layout" file="layout.mako" />
+${commonheader("Hue Groups", "useradmin", "100px")}
+${layout.menubar(section='groups')}
+
+<div class="container-fluid">
+	<h1>Hue Groups</h1>
+	<div class="well">
+			Filter by name: <input id="filterInput"/> <a href="#" id="clearFilterBtn" class="btn">Clear</a>
+			<p class="pull-right">
+				<a href="${ url('useradmin.views.edit_group') }" class="btn">Add group</a>
+			</p>
+	</div>
+      <table class="datatables">
+        <thead>
+          <tr>
+            <th>${_('Group Name')}</th>
+            <th>${_('Members')}</th>
+			<th>&nbsp;</th>
+          </tr>
+        </head>
+        <tbody>
+        % for group in groups:
+          <tr class="groupRow" data-search="${group.name}${', '.join([user.username for user in group.user_set.all()])}">
+            <td>${group.name}</td>
+            <td>${', '.join([user.username for user in group.user_set.all()])}</td>
+            <td>
+              <a title="Edit ${group.name}" class="btn small" href="${ url('useradmin.views.edit_group', name=urllib.quote(group.name)) }">Edit</a>
+              <a title="Delete ${group.name}" class="btn small confirmationModal" alt="Are you sure you want to delete ${group.name}?" href="javascript:void(0)" data-confirmation-url="${ url('useradmin.views.delete_group', name=urllib.quote_plus(group.name)) }">Delete</a>
+            </td>
+          </tr>
+        % endfor
+        </tbody>
+      </table>
+
+
+
+<div id="deleteGroup" class="modal hide fade">
+	<form id="deleteGroupForm" action="" method="POST">
+	<div class="modal-header">
+		<a href="#" class="close">&times;</a>
+		<h3 id="deleteGroupMessage">Confirm action</h3>
+	</div>
+	<div class="modal-footer">
+		<input type="submit" class="btn primary" value="Yes"/>
+		<a href="#" class="btn secondary hideModal">No</a>
+	</div>
+	</form>
+</div>
+</div>   
+
+	<script type="text/javascript" charset="utf-8">
+		$(document).ready(function(){
+			$(".datatables").dataTable({
+				"bPaginate": false,
+			    "bLengthChange": false,
+				"bInfo": false,
+				"bFilter": false
+			});
+			$(".dataTables_wrapper").css("min-height","0");
+			$(".dataTables_filter").hide();
+
+			$("#deleteGroup").modal({
+				backdrop: "static",
+				keyboard: true
+			});
+			$(".confirmationModal").click(function(){
+				var _this = $(this);
+				$.getJSON(_this.attr("data-confirmation-url"), function(data){
+					$("#deleteGroupForm").attr("action", data.path);
+					$("#deleteGroupMessage").text(_this.attr("alt"));
+				});
+				$("#deleteGroup").modal("show");
+			});
+			$(".hideModal").click(function(){
+				$("#deleteGroup").modal("hide");
+			});
+			
+			$("#filterInput").keyup(function(){
+		        $.each($(".groupRow"), function(index, value) {
+
+		          if($(value).attr("data-search").toLowerCase().indexOf($("#filterInput").val().toLowerCase()) == -1 && $("#filterInput").val() != ""){
+		            $(value).hide(250);
+		          }else{
+		            $(value).show(250);
+		          }
+		        });
+
+		    });
+
+		    $("#clearFilterBtn").click(function(){
+		        $("#filterInput").val("");
+		        $.each($(".file-row"), function(index, value) {
+		            $(value).show(250);
+		        });
+		    });
+		   
+
+		});
+	</script>
+
+${commonfooter()}

+ 4 - 1
apps/useradmin/src/useradmin/templates/list_users.mako

@@ -20,7 +20,10 @@ from desktop.views import commonheader, commonfooter
 <% from django.utils.translation import ugettext, ungettext, get_language, activate %>
 <% _ = ugettext %>
 
-${commonheader("Hue Users", "useradmin")}
+<%namespace name="layout" file="layout.mako" />
+${commonheader("Hue Users", "useradmin", "100px")}
+${layout.menubar(section='users')}
+
 <div class="container-fluid">
 	<h1>Hue Users</h1>
 	<div class="well">

+ 67 - 22
apps/useradmin/src/useradmin/tests.py

@@ -26,7 +26,7 @@ import urllib
 from nose.tools import assert_true, assert_equal
 
 from desktop.lib.django_test_util import make_logged_in_client
-from django.contrib.auth.models import User
+from django.contrib.auth.models import User, Group
 from django.utils.encoding import smart_unicode
 
 def reset_all_users():
@@ -34,6 +34,10 @@ def reset_all_users():
   for user in User.objects.all():
     user.delete()
 
+def reset_all_groups():
+  """Reset to a clean state by deleting all users"""
+  for grp in Group.objects.all():
+    grp.delete()
 
 def test_invalid_username():
   BAD_NAMES = ('-foo', 'foo:o', 'foo o', ' foo')
@@ -41,16 +45,57 @@ def test_invalid_username():
   c = make_logged_in_client(username="test", is_superuser=True)
 
   for bad_name in BAD_NAMES:
-    assert_true(c.get('/useradmin/new'))
-    response = c.post('/useradmin/new', dict(username=bad_name, password1="test", password2="test"))
+    assert_true(c.get('/useradmin/users/new'))
+    response = c.post('/useradmin/users/new', dict(username=bad_name, password1="test", password2="test"))
     assert_true('not allowed' in response.context["form"].errors['username'][0])
 
+def test_group_admin():
+  reset_all_users()
+  reset_all_groups()
+
+  c = make_logged_in_client(username="test", is_superuser=True)
+  response = c.get('/useradmin/groups')
+  # No groups just yet
+  assert_true(len(response.context["groups"]) == 0)
+  assert_true("Hue Groups" in response.content)
+
+  # Create a group
+  response = c.get('/useradmin/groups/new')
+  assert_true("Create Group" in response.content)
+  c.post('/useradmin/groups/new', dict(name="testgroup"))
+
+  # We should have an empty group in the DB now
+  assert_true(len(Group.objects.all()) == 1)
+  assert_true(Group.objects.filter(name="testgroup").exists())
+  assert_true(len(Group.objects.get(name="testgroup").user_set.all()) == 0)
+
+  # And now, just for kicks, let's try adding a user
+  response = c.get('/useradmin/groups/edit/testgroup')
+  assert_true("Edit Group: testgroup" in response.content)
+  response = c.post('/useradmin/groups/edit/testgroup',
+                    dict(name="testgroup",
+                    members=[User.objects.get(username="test").pk],
+                    save="Save"), follow=True)
+  assert_true(len(Group.objects.get(name="testgroup").user_set.all()) == 1)
+  assert_true(Group.objects.get(name="testgroup").user_set.filter(username="test").exists())
+
+  # Test some permissions
+  c2 = make_logged_in_client(username="nonadmin", is_superuser=False)
+  response = c2.get('/useradmin/groups/new')
+  assert_true("You must be a superuser" in response.content)
+  response = c2.get('/useradmin/groups/edit/testgroup')
+  assert_true("You must be a superuser" in response.content)
+
+  response = c.post('/useradmin/groups/delete/testgroup')
+  assert_true(len(Group.objects.all()) == 0)
+
 
 def test_user_admin():
   FUNNY_NAME = '~`!@#$%^&*()_-+={}[]|\;"<>?/,.'
   FUNNY_NAME_QUOTED = urllib.quote(FUNNY_NAME)
 
   reset_all_users()
+  reset_all_groups()
   c = make_logged_in_client(username="test", is_superuser=True)
 
   # Test basic output.
@@ -60,48 +105,48 @@ def test_user_admin():
 
   # Test editing a superuser
   # Just check that this comes back
-  response = c.get('/useradmin/edit/test')
+  response = c.get('/useradmin/users/edit/test')
   # Edit it, to add a first and last name
-  response = c.post('/useradmin/edit/test',
+  response = c.post('/useradmin/users/edit/test',
                     dict(username="test",
                          first_name=u"Inglés",
                          last_name=u"Español",
                          is_superuser="True",
                          is_active="True"))
   # Now make sure that those were materialized
-  response = c.get('/useradmin/edit/test')
+  response = c.get('/useradmin/users/edit/test')
   assert_equal(smart_unicode("Inglés"), response.context["form"].instance.first_name)
   assert_true("Español" in response.content)
   # Shouldn't be able to demote to non-superuser
-  response = c.post('/useradmin/edit/test', dict(username="test",
+  response = c.post('/useradmin/users/edit/test', dict(username="test",
                         first_name=u"Inglés", last_name=u"Español",
                         is_superuser=False, is_active=True))
   assert_true("You cannot remove" in response.content,
               "Shouldn't be able to remove the last superuser")
   # Shouldn't be able to delete the last superuser
-  response = c.post('/useradmin/delete/test', {})
+  response = c.post('/useradmin/users/delete/test', {})
   assert_true("You cannot remove" in response.content,
               "Shouldn't be able to delete the last superuser")
 
   # Let's try changing the password
-  response = c.post('/useradmin/edit/test', dict(username="test", first_name="Tom", last_name="Tester", is_superuser=True, password1="foo", password2="foobar"))
+  response = c.post('/useradmin/users/edit/test', dict(username="test", first_name="Tom", last_name="Tester", is_superuser=True, password1="foo", password2="foobar"))
   assert_equal(["Passwords do not match."], response.context["form"]["password2"].errors, "Should have complained about mismatched password")
-  response = c.post('/useradmin/edit/test', dict(username="test", first_name="Tom", last_name="Tester", password1="foo", password2="foo", is_active=True, is_superuser=True))
+  response = c.post('/useradmin/users/edit/test', dict(username="test", first_name="Tom", last_name="Tester", password1="foo", password2="foo", is_active=True, is_superuser=True))
   assert_true(User.objects.get(username="test").is_superuser)
   assert_true(User.objects.get(username="test").check_password("foo"))
   # Change it back!
-  response = c.post('/useradmin/edit/test', dict(username="test", first_name="Tom", last_name="Tester", password1="test", password2="test", is_active="True", is_superuser="True"))
+  response = c.post('/useradmin/users/edit/test', dict(username="test", first_name="Tom", last_name="Tester", password1="test", password2="test", is_active="True", is_superuser="True"))
   assert_true(User.objects.get(username="test").check_password("test"))
   assert_true(make_logged_in_client(username = "test", password = "test"),
               "Check that we can still login.")
 
   # Create a new regular user (duplicate name)
-  assert_true(c.get('/useradmin/new'))
-  response = c.post('/useradmin/new', dict(username="test", password1="test", password2="test"))
+  assert_true(c.get('/useradmin/users/new'))
+  response = c.post('/useradmin/users/new', dict(username="test", password1="test", password2="test"))
   assert_equal({ 'username': ["User with this Username already exists."]}, response.context["form"].errors)
 
   # Create a new regular user (for real)
-  response = c.post('/useradmin/new', dict(username=FUNNY_NAME,
+  response = c.post('/useradmin/users/new', dict(username=FUNNY_NAME,
                                            password1="test",
                                            password2="test",
                                            is_active="True"))
@@ -113,18 +158,18 @@ def test_user_admin():
   # Check permissions by logging in as the new user
   c_reg = make_logged_in_client(username=FUNNY_NAME, password="test")
   # Regular user should be able to modify oneself
-  response = c_reg.post('/useradmin/edit/%s' % (FUNNY_NAME_QUOTED,),
+  response = c_reg.post('/useradmin/users/edit/%s' % (FUNNY_NAME_QUOTED,),
                         dict(username = FUNNY_NAME,
                              first_name = "Hello",
                              is_active = True))
-  response = c_reg.get('/useradmin/edit/%s' % (FUNNY_NAME_QUOTED,))
+  response = c_reg.get('/useradmin/users/edit/%s' % (FUNNY_NAME_QUOTED,))
   assert_equal("Hello", response.context["form"].instance.first_name)
   # Can't edit other people.
-  response = c_reg.post("/useradmin/delete/test")
+  response = c_reg.post("/useradmin/users/delete/test")
   assert_true("You must be a superuser" in response.content,
               "Regular user can't edit other people")
   # Regular user should not be able to self-promote to superuser
-  response = c_reg.post('/useradmin/edit/%s' % (FUNNY_NAME_QUOTED,),
+  response = c_reg.post('/useradmin/users/edit/%s' % (FUNNY_NAME_QUOTED,),
                         dict(username = FUNNY_NAME,
                              first_name = "OLÁ",
                              is_superuser = True,
@@ -135,18 +180,18 @@ def test_user_admin():
   # Revert to regular "test" user, that has superuser powers.
   c_su = make_logged_in_client()
   # Inactivate FUNNY_NAME
-  c_su.post('/useradmin/edit/%s' % (FUNNY_NAME_QUOTED,),
+  c_su.post('/useradmin/users/edit/%s' % (FUNNY_NAME_QUOTED,),
                         dict(username = FUNNY_NAME,
                              first_name = "Hello",
                              is_active = False))
   # Now make sure FUNNY_NAME can't log back in
-  response = c_reg.get('/useradmin/edit/%s' % (FUNNY_NAME_QUOTED,))
+  response = c_reg.get('/useradmin/users/edit/%s' % (FUNNY_NAME_QUOTED,))
   assert_true(response.status_code == 302 and "login" in response["location"],
               "Inactivated user gets redirected to login page")
 
   # Delete that regular user
-  response = c_su.post('/useradmin/delete/%s' % (FUNNY_NAME_QUOTED,))
+  response = c_su.post('/useradmin/users/delete/%s' % (FUNNY_NAME_QUOTED,))
   assert_true("Hue Users" in response.content)
   # You shouldn't be able to create a user without a password
-  response = c_su.post('/useradmin/new', dict(username="test"))
+  response = c_su.post('/useradmin/users/new', dict(username="test"))
   assert_true("You must specify a password when creating a new user." in response.content)

+ 10 - 4
apps/useradmin/src/useradmin/urls.py

@@ -16,13 +16,19 @@
 # limitations under the License.
 
 from django.conf.urls.defaults import patterns, url
-from desktop.lib.django_util import get_username_re_rule
+from desktop.lib.django_util import get_username_re_rule, get_groupname_re_rule
 
 username_re = get_username_re_rule()
+groupname_re = get_groupname_re_rule()
 
 urlpatterns = patterns('useradmin',
   url(r'^$', 'views.list_users'),
-  url(r'^edit/(?P<username>%s)$' % (username_re,), 'views.edit_user'),
-  url(r'^new$', 'views.edit_user', name="useradmin.new"),
-  url(r'^delete/(?P<username>%s)$' % (username_re,), 'views.delete_user'),
+  url(r'^users$', 'views.list_users'),
+  url(r'^groups$', 'views.list_groups'),
+  url(r'^users/edit/(?P<username>%s)$' % (username_re,), 'views.edit_user'),
+  url(r'^groups/edit/(?P<name>%s)$' % (groupname_re,), 'views.edit_group'),
+  url(r'^users/new$', 'views.edit_user', name="useradmin.new"),
+  url(r'^groups/new$', 'views.edit_group', name="useradmin.new_group"),
+  url(r'^users/delete/(?P<username>%s)$' % (username_re,), 'views.delete_user'),
+  url(r'^groups/delete/(?P<name>%s)$' % (groupname_re,), 'views.delete_group'),
 )

+ 122 - 3
apps/useradmin/src/useradmin/views.py

@@ -17,7 +17,7 @@
 """
 User management application.
 """
-import os
+import re
 import pwd
 import grp
 import logging
@@ -27,8 +27,8 @@ import subprocess
 import django
 import django.contrib.auth.forms
 from django import forms
-from django.contrib.auth.models import User
-from desktop.lib.django_util import get_username_re_rule, render, PopupException
+from django.contrib.auth.models import User, Group
+from desktop.lib.django_util import get_username_re_rule, get_groupname_re_rule, render, PopupException, format_preserving_redirect
 from django.core import urlresolvers
 
 LOG = logging.getLogger(__name__)
@@ -39,6 +39,9 @@ __groups_lock = threading.Lock()
 def list_users(request):
   return render("list_users.mako", request, dict(users=User.objects.all()))
 
+def list_groups(request):
+  return render("list_groups.mako", request, dict(groups=Group.objects.all()))
+
 def delete_user(request, username):
   if not request.user.is_superuser:
     raise PopupException("You must be a superuser to delete users.")
@@ -62,6 +65,28 @@ def delete_user(request, username):
       request,
       dict(path=request.path, title="Delete user?"))
 
+def delete_group(request, name):
+  if not request.user.is_superuser:
+    raise PopupException("You must be a superuser to delete groups.")
+  if request.method == 'POST':
+    try:
+      global groups_lock
+      __groups_lock.acquire()
+      try:
+        group = Group.objects.get(name=name)
+        group.delete()
+      finally:
+        __groups_lock.release()
+
+      # Send a flash message saying "deleted"?
+      return list_groups(request)
+    except Group.DoesNotExist:
+      raise PopupException("Group not found.")
+  else:
+    return render("confirm.mako",
+      request,
+      dict(path=request.path, title="Delete group?"))
+
 class UserChangeForm(django.contrib.auth.forms.UserChangeForm):
   """
   This is similar, but not quite the same as djagno.contrib.auth.forms.UserChangeForm
@@ -158,6 +183,38 @@ def edit_user(request, username=None):
   return render('edit_user.mako', request,
     dict(form=form, action=request.path, username=username))
 
+def edit_group(request, name=None):
+  """
+  edit_group(request, name = None) -> reply
+
+  @type request:        HttpRequest
+  @param request:       The request object
+  @type name:       string
+  @param name:      Default to None, when creating a new group
+
+  Only superusers may create a group
+  """
+  if not request.user.is_superuser:
+    raise PopupException("You must be a superuser to add or edit a group.")
+
+  if name is not None:
+    instance = Group.objects.get(name=name)
+  else:
+    instance = None
+
+  if request.method == 'POST':
+    form = GroupEditForm(request.POST, instance=instance)
+    if form.is_valid():
+      form.save()
+      request.flash.put('Group information updated')
+      url = urlresolvers.reverse(list_groups)
+      return format_preserving_redirect(request, url)
+
+  else:
+    form = GroupEditForm(instance=instance)
+  return render('edit_group.mako', request,
+    dict(form=form, action=request.path, name=name))
+
 
 def _check_remove_last_super(user_obj):
   """Raise an error if we're removing the last superuser"""
@@ -232,3 +289,65 @@ def sync_unix_users_and_groups(min_uid, max_uid, check_shell):
 
   __users_lock.release()
   __groups_lock.release()
+
+class GroupEditForm(forms.ModelForm):
+  """
+  Form to manipulate a group.  This manages the group name and its membership.
+  """
+  class Meta:
+    model = Group
+    fields = ("name",)
+
+  def clean_name(self):
+    # Note that the superclass doesn't have a clean_name method.
+    data = self.cleaned_data["name"]
+    if not re.match(get_groupname_re_rule(), data):
+      raise forms.ValidationError("Group name may only contain letters, " +
+                                  "numbers, hypens or underscores.")
+    return data
+
+  def __init__(self, *args, **kwargs):
+    super(GroupEditForm, self).__init__(*args, **kwargs)
+
+    if self.instance.id:
+      initial_members = User.objects.filter(groups=self.instance).order_by('username')
+    else:
+      initial_members = []
+
+    self.fields["members"] = _make_model_field(initial_members, User.objects.order_by('username'))
+
+  def _compute_diff(self, field_name):
+    """Used for members, but not permissions."""
+    current = set(self.fields[field_name].initial_objs)
+    updated = set(self.cleaned_data[field_name])
+    delete = current.difference(updated)
+    add = updated.difference(current)
+    return delete, add
+
+  def save(self):
+    super(GroupEditForm, self).save()
+    self._save_members()
+
+  def _save_members(self):
+    delete_membership, add_membership = self._compute_diff("members")
+    for user in delete_membership:
+      user.groups.remove(self.instance)
+      user.save()
+    for user in add_membership:
+      user.groups.add(self.instance)
+      user.save()
+
+def _make_model_field(initial, choices, multi=True):
+  """ Creates multiple choice field with given query object as choices. """
+  if multi:
+    field = forms.models.ModelMultipleChoiceField(choices, required=False)
+    field.initial_objs = initial
+    field.initial = [ obj.pk for obj in initial ]
+  else:
+    field = forms.models.ModelChoiceField(choices, required=False)
+    field.initial_obj = initial
+    if initial:
+      field.initial = initial.pk
+  return field
+
+

+ 4 - 0
desktop/core/src/desktop/lib/django_util.py

@@ -44,6 +44,7 @@ MAKO = 'mako'
 
 # This is what Debian allows. See chkname.c in shadow.
 USERNAME_RE_RULE = "[^-:\s][^:\s]*"
+GROUPNAME_RE_RULE = "[\w-]+"
 
 class Encoder(simplejson.JSONEncoder):
   """
@@ -67,6 +68,9 @@ class Encoder(simplejson.JSONEncoder):
 def get_username_re_rule():
   return USERNAME_RE_RULE
 
+def get_groupname_re_rule():
+  return GROUPNAME_RE_RULE
+
 def login_notrequired(func):
   """A decorator for view functions to allow access without login"""
   func.login_notrequired = True