Skip to content

Commit 1056657

Browse files
committed
enrollments API requires pseudonym on course's root account
in order to enroll a user in a course via the API, the user being enrolled must have an active pseudonym that works with the course's root account. test plan: - an account administrator or site administrator should receive a user-not-found (status 404) error when trying to use the API to enroll a user in a course where the user does not have a login in the course's root account (i.e., the user "belongs to a different institution"), AND the course's root account is _not_ the default account (since the default account trusts everybody; this allows anyone to enroll in FFT courses) - you should be able to create an Observer via the course's people page and link that observer to students in the course before creating a login for the observer. fixes CNVS-12020 Change-Id: Ib058d9b78830347e6fadfdc86bf4325aa557c325 Reviewed-on: https://gerrit.instructure.com/32374 Reviewed-by: Brian Palmer <brianp@instructure.com> Reviewed-by: Rob Orton <rob@instructure.com> Tested-by: Jenkins <jenkins@instructure.com> QA-Review: Matt Fairbourn <mfairbourn@instructure.com> Product-Review: Jeremy Stanley <jeremy@instructure.com>
1 parent 5aae89b commit 1056657

5 files changed

Lines changed: 47 additions & 6 deletions

File tree

app/controllers/enrollments_api_controller.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,9 @@ def create
325325
if params[:enrollment][:course_section_id].present?
326326
params[:enrollment][:section] = @context.course_sections.active.find params[:enrollment].delete(:course_section_id)
327327
end
328-
user = api_find(User, params[:enrollment].delete(:user_id))
328+
api_user_id = params[:enrollment].delete(:user_id)
329+
user = api_find(User, api_user_id)
330+
raise(ActiveRecord::RecordNotFound, "Couldn't find User with API id '#{api_user_id}'") unless user.can_be_enrolled_in_course?(@context)
329331
@enrollment = @context.enroll_user(user, type, params[:enrollment].merge(:allow_multiple_enrollments => true))
330332
@enrollment.valid? ?
331333
render(:json => enrollment_json(@enrollment, @current_user, session)) :

app/models/user.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2314,6 +2314,11 @@ def can_create_enrollment_for?(course, session, type)
23142314
can_add
23152315
end
23162316

2317+
def can_be_enrolled_in_course?(course)
2318+
!!find_pseudonym_for_account(course.root_account, true) ||
2319+
(self.creation_pending? && self.enrollments.where(course_id: course).exists?)
2320+
end
2321+
23172322
def group_member_json(context)
23182323
h = { :user_id => self.id, :name => self.last_name_first, :display_name => self.short_name }
23192324
if context && context.is_a?(Course)

spec/apis/v1/enrollments_api_spec.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
describe "enrollment creation" do
2424
context "an admin user" do
2525
before do
26-
site_admin_user(:active_all => true)
26+
account_admin_user(:active_all => true)
2727
course(:active_course => true)
2828
@unenrolled_user = user_with_pseudonym
2929
@section = @course.course_sections.create!
@@ -242,6 +242,12 @@
242242
JSON.parse(response.body)['message'].should eql 'Can\'t add an enrollment to a concluded course.'
243243
end
244244

245+
it "should not enroll a user lacking a pseudonym on the course's account" do
246+
foreign_user = user
247+
api_call_as_user @admin, :post, @path, @path_options, { :enrollment => { :user_id => foreign_user.id } }, {},
248+
{ expected_status: 404 }
249+
end
250+
245251
context "custom course-level roles" do
246252
before :each do
247253
@course_role = @course.root_account.roles.build(:name => 'newrole')

spec/models/user_spec.rb

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1449,6 +1449,34 @@ def search_messageable_users(viewing_user, *args)
14491449
end
14501450
end
14511451

1452+
describe "can_be_enrolled_in_course?" do
1453+
before do
1454+
course active_all: true
1455+
end
1456+
1457+
it "should allow a user with a pseudonym in the course's root account" do
1458+
user_with_pseudonym account: @course.root_account, active_all: true
1459+
@user.can_be_enrolled_in_course?(@course).should be_true
1460+
end
1461+
1462+
it "should allow a temporary user with an existing enrollment but no pseudonym" do
1463+
@user = User.create! { |u| u.workflow_state = 'creation_pending' }
1464+
@course.enroll_student(@user)
1465+
@user.can_be_enrolled_in_course?(@course).should be_true
1466+
end
1467+
1468+
it "should not allow a registered user with an existing enrollment but no pseudonym" do
1469+
user active_all: true
1470+
@course.enroll_student(@user)
1471+
@user.can_be_enrolled_in_course?(@course).should be_false
1472+
end
1473+
1474+
it "should not allow a user with neither an enrollment nor a pseudonym" do
1475+
user active_all: true
1476+
@user.can_be_enrolled_in_course?(@course).should be_false
1477+
end
1478+
end
1479+
14521480
describe "email_channel" do
14531481
it "should not return retired channels" do
14541482
u = User.create!

spec/selenium/people_settings_spec.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ def remove_user(user, role = nil)
9090
end
9191

9292
def add_user_to_second_section(role_name=nil)
93-
student_in_course(:role_name => role_name)
93+
student_in_course(:user => user_with_pseudonym, :role_name => role_name)
9494
section_name = 'Another Section'
9595
add_section(section_name)
9696
# open tab
@@ -150,7 +150,7 @@ def use_edit_sections_dialog(user, role = nil)
150150

151151
it "should deal with observers linked to multiple students" do
152152
students = []
153-
obs = user_model(:name => "The Observer")
153+
obs = user_with_pseudonym(:name => "The Observer")
154154
2.times do |i|
155155
student_in_course(:name => "Student #{i}")
156156
students << @student
@@ -211,7 +211,7 @@ def use_edit_sections_dialog(user, role = nil)
211211
send "custom_#{et}_role", "custom"
212212
send "course_with_#{et}", :course => @course, :active_all => true, :custom_role => 'custom'
213213
user_session @user
214-
student_in_course :course => @course, :role_name => 'custom stu'
214+
student_in_course :user => user_with_pseudonym, :course => @course, :role_name => 'custom stu'
215215

216216
go_to_people_page
217217

@@ -232,7 +232,7 @@ def use_edit_sections_dialog(user, role = nil)
232232
context "multiple enrollments" do
233233
it "should link an observer enrollment when other enrollment types exist" do
234234
course_with_student :course => @course, :active_all => true, :name => 'teh student'
235-
course_with_ta :course => @course, :active_all => true
235+
course_with_ta :user => user_with_pseudonym, :course => @course, :active_all => true
236236
course_with_observer :course => @course, :active_all => true, :user => @ta
237237

238238
go_to_people_page

0 commit comments

Comments
 (0)