fix permission check for TAs refs #5833

also add a checkbox for admins to only search for existing users
(i.e. ignore open registration)

Change-Id: I84d2ba7992339b37506e41a50c336325af0ac73b
testplan:
 * add a student to a course as a TA
 * add a user to an account or a course as an admin, with and without
   open registration, with and without manage_user_logins permission,
   with and without checking "search only existing users" (not all
   combinations applicable, each underlying condition is checked in
   specs)
Reviewed-on: https://gerrit.instructure.com/6969
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>

app/controllers/accounts_controller.rb

@@ -382,7 +382,7 @@ class AccountsController < ApplicationController
   
   def add_account_user
     if authorized_action(@context, @current_user, :manage_account_memberships)
-      list = UserList.new(params[:user_list], @context, @context.grants_right?(@current_user, session, :manage_user_logins))
+      list = UserList.new(params[:user_list], @context, params[:only_search_existing_users] ? false : @context.open_registration_for?(@current_user, session))
       users = list.users
       account_users = users.map do |user|
         account_user = @context.add_user(user, params[:membership_type])

app/controllers/courses_controller.rb

@@ -762,7 +762,7 @@ class CoursesController < ApplicationController
         if params[:auto_accept] && @context.account.grants_right?(@current_user, session, :manage_admin_users)
           @enrollment_state = 'active'
         end
-        if (@enrollments = EnrollmentsFromUserList.process(UserList.new(params[:user_list], @context.root_account), @context, :course_section_id => params[:course_section_id], :enrollment_type => params[:enrollment_type], :limit_priveleges_to_course_section => params[:limit_priveleges_to_course_section] == '1', :enrollment_state => @enrollment_state))
+        if (@enrollments = EnrollmentsFromUserList.process(UserList.new(params[:user_list], @context.root_account, params[:only_search_existing_users] ? false : @context.open_registration_for?(@current_user, session)), @context, :course_section_id => params[:course_section_id], :enrollment_type => params[:enrollment_type], :limit_priveleges_to_course_section => params[:limit_priveleges_to_course_section] == '1', :enrollment_state => @enrollment_state))
           format.json do
             Enrollment.send(:preload_associations, @enrollments, [:course_section, {:user => [:communication_channel, :pseudonym]}])
             json = @enrollments.map { |e|

app/controllers/user_lists_controller.rb

@@ -23,11 +23,11 @@ class UserListsController < ApplicationController
   # POST /courses/:course_id/user_lists.json
   # POST /accounts/:account_id/user_lists.json
   def create
-    return unless authorized_action(@context, @current_user, @context.is_a?(Course) ? :manage_admin_users : :manage_account_memberships)
+    return unless authorized_action(@context, @current_user, @context.is_a?(Course) ? [:manage_students, :manage_admin_users] : :manage_account_memberships)
+    root_account = @context.root_account || @context
     respond_to do |format|
-      format.json { render :json => UserList.new(params[:user_list],
-          @context.root_account || @context,
-          @context.is_a?(Account) && @context.grants_right?(@current_user, session, :manage_user_logins)) }
+      format.json { render :json => UserList.new(params[:user_list], root_account,
+          params[:only_search_existing_users] ? false : @context.open_registration_for?(@current_user)) }
     end
   end
 end

app/models/account.rb

@@ -1048,6 +1048,12 @@ class Account < ActiveRecord::Base
     (self.root_account || self).sub_accounts.find_or_create_by_name(t('#account.manually_created_courses', "Manually-Created Courses"))
   end
 
+  def open_registration_for?(user, session = nil)
+    root_account = self.root_account || self
+    return true if root_account.open_registration?
+    root_account.grants_right?(user, session, :manage_user_logins)
+  end
+
   named_scope :sis_sub_accounts, lambda{|account, *sub_account_source_ids|
     {:conditions => {:root_account_id => account.id, :sis_source_id => sub_account_source_ids}, :order => :sis_source_id}
   }

app/models/course.rb

@@ -2497,4 +2497,8 @@ class Course < ActiveRecord::Base
       Course.find(new_course.id)
     end
   end
+
+  def open_registration_for?(user, session = nil)
+    root_account.open_registration_for?(user, session)
+  end
 end

app/views/shared/_user_lists.html.erb

@@ -1,8 +1,16 @@
+<% root_account = @context.root_account || @context %>
+<% account = @context.respond_to?(:account) ? @context.account : @context %>
 <div id="user_list_boxes">
   <a id="user_lists_path" style="display:none;" href="<%= polymorphic_path([@context, :user_lists], :format => :json) %>"></a>
   <div id="user_list_textarea_container" style="z-index: 2;">
     <input type="hidden" name="enrollment_type" value="StudentEnrollment" />
-    <span style="font-size: 0.8em;"><%= (@context.root_account || @context).login_handle_name_is_customized? ? t(:copy_and_paste_notice_with_login_handle, "Copy and paste a list of users. You can use their email address or %{login_handle_name}.", :login_handle_name => (@context.root_account || @context).login_handle_name) : t(:copy_and_paste_notice_just_email, "Copy and paste a list of email addresses to add users.") %></span>
+    <% if @context.open_registration_for?(@current_user, session) && can_do(account, @current_user, :read) %>
+      <div style="white-space: nowrap;">
+        <input type='checkbox' name='only_search_existing_users' value="1" />
+        <%= label_tag :only_search_existing_users, :en => 'only search existing users' %>
+      </div>
+    <% end %>
+    <span style="font-size: 0.8em;"><%= root_account.login_handle_name_is_customized? ? t(:copy_and_paste_notice_with_login_handle, "Copy and paste a list of users. You can use their email address or %{login_handle_name}.", :login_handle_name => root_account.login_handle_name) : t(:copy_and_paste_notice_just_email, "Copy and paste a list of email addresses to add users.") %></span>
     <textarea name="user_list" class="user_list"><% if @students && @students.empty? %><%= t :example_user_list, '"Example Student" <student@example.com>
 "Other Student" <otherstudent@example.com>
 "Lastname, Firstname" <firstlast@example.com>

lib/user_list.rb

@@ -18,12 +18,14 @@
 #
 
 class UserList
+  # open_registration is true, false, or nil. if nil, it defaults to root_account.open_registration?
   def initialize(string, root_account = nil, open_registration = nil)
     @addresses = []
     @errors = []
     @duplicate_addresses = []
     @root_account = root_account || Account.default
-    @open_registration = open_registration || @root_account.open_registration?
+    @open_registration = open_registration
+    @open_registration = @root_account.open_registration? if @open_registration.nil?
     parse_list(string)
     resolve
   end

public/javascripts/user_lists.js

@@ -46,8 +46,7 @@ I18n.scoped('user_lists', function(I18n) {
         .click(function(e){
           e.preventDefault();
           UL.showProcessing();
-          var params = {user_list: $("#user_list_textarea_container textarea").val() };
-          $.ajaxJSON(user_lists_path, 'POST', params, UL.showResults);
+          $.ajaxJSON(user_lists_path, 'POST', $form.getFormData(), UL.showResults);
         })
       .end()
       .submit(function(event) {

spec/controllers/user_lists_controller_spec.rb

@@ -0,0 +1,40 @@
+#
+# Copyright (C) 2011 Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
+
+describe UserListsController do
+  it "should not fail for permission to add students" do
+    course
+    account_admin_user_with_role_changes(:membership_type => 'myadmin', :role_changes => { :manage_students => true })
+    user_session(@user)
+
+    post 'create', :course_id => @course.id, :user_list => ''
+    response.should be_success
+  end
+
+  it "should allow bypassing the effects of open registration" do
+    course
+    user
+    Account.default.add_user(@user)
+    user_session(@user)
+
+    UserList.any_instance.expects(:initialize).with('', Account.default, false)
+    post 'create', :course_id => @course.id, :user_list => '', :only_search_existing_users => true
+  end
+end

spec/models/account_spec.rb

@@ -596,4 +596,22 @@ describe Account do
       Account.default.fast_all_users.should == [@jtolds, @johnstclair]
     end
   end
+
+  describe "open_registration_for?" do
+    it "should be true for anyone if open registration is turned on" do
+      account = Account.default
+      account.settings = { :open_registration => true }
+      account.open_registration_for?(nil).should be_true
+      account.open_registration_for?(user).should be_true
+    end
+
+    it "should be true for account admins" do
+      account = Account.default
+      account.open_registration_for?(nil).should be_false
+      account.open_registration_for?(user).should be_false
+      user
+      account.add_user(@user)
+      account.open_registration_for?(@user).should be_true
+    end
+  end
 end