diff --git a/dashboard/app/controllers/levels_controller.rb b/dashboard/app/controllers/levels_controller.rb
index 06cbdf33806d8..d847b2d4b8834 100644
--- a/dashboard/app/controllers/levels_controller.rb
+++ b/dashboard/app/controllers/levels_controller.rb
@@ -113,7 +113,7 @@ def update
# POST /levels
# POST /levels.json
def create
- authorize! :create, :level
+ authorize! :create, Level
type_class = level_params[:type].constantize
# Set some defaults.
@@ -153,7 +153,7 @@ def destroy
end
def new
- authorize! :create, :level
+ authorize! :create, Level
if params.key? :type
@type_class = params[:type].constantize
if @type_class == Artist
diff --git a/dashboard/app/models/ability.rb b/dashboard/app/models/ability.rb
index d8a36642a450f..342adbdbe38f6 100644
--- a/dashboard/app/models/ability.rb
+++ b/dashboard/app/models/ability.rb
@@ -171,6 +171,7 @@ def initialize(user)
# permissions.
if user.persisted? && user.permission?(UserPermission::LEVELBUILDER)
can :manage, [
+ Game,
Level,
Script,
ScriptLevel
@@ -185,10 +186,15 @@ def initialize(user)
if user.admin?
can :manage, :all
- # Only custom levels are editable
- cannot [:update, :destroy], Level do |level|
- !level.custom?
- end
+ cannot :manage, [
+ Activity,
+ Game,
+ Level,
+ Script,
+ ScriptLevel,
+ UserLevel,
+ UserScript
+ ]
end
end
end
diff --git a/dashboard/test/controllers/levels_controller_test.rb b/dashboard/test/controllers/levels_controller_test.rb
index c9371ae70c74a..839fb2bae7f02 100644
--- a/dashboard/test/controllers/levels_controller_test.rb
+++ b/dashboard/test/controllers/levels_controller_test.rb
@@ -8,11 +8,11 @@ class LevelsControllerTest < ActionController::TestCase
Level.any_instance.stubs(:write_to_file?).returns(false) # don't write to level files
@level = create(:level)
- @user = create(:admin)
- sign_in(@user)
- @program = ''
-
+ @admin = create(:admin)
@not_admin = create(:user)
+ @levelbuilder = create(:levelbuilder)
+ sign_in(@levelbuilder)
+ @program = ''
end
test "should get index" do
@@ -48,22 +48,24 @@ class LevelsControllerTest < ActionController::TestCase
end
test "should alphanumeric order custom levels on new" do
- Level.where(user_id: @user.id).map(&:destroy)
- level_1 = create(:level, user: @user, name: "BBBB")
- level_2 = create(:level, user: @user, name: "AAAA")
- level_3 = create(:level, user: @user, name: "Z1")
- level_4 = create(:level, user: @user, name: "Z10")
- level_5 = create(:level, user: @user, name: "Z2")
+ Level.where(user_id: @levelbuilder.id).map(&:destroy)
+ level_1 = create(:level, user: @levelbuilder, name: "BBBB")
+ level_2 = create(:level, user: @levelbuilder, name: "AAAA")
+ level_3 = create(:level, user: @levelbuilder, name: "Z1")
+ level_4 = create(:level, user: @levelbuilder, name: "Z10")
+ level_5 = create(:level, user: @levelbuilder, name: "Z2")
get :new, game_id: @level.game
assert_equal [level_2, level_1, level_3, level_5, level_4], assigns(:levels)
end
- test "should not get builder if not admin" do
- sign_in @not_admin
- get :new, game_id: @level.game
- assert_response :forbidden
+ test "should not get builder if not levelbuilder" do
+ [@not_admin, @admin].each do |user|
+ sign_in user
+ get :new, game_id: @level.game
+ assert_response :forbidden
+ end
end
test "should create maze level" do
@@ -136,7 +138,7 @@ class LevelsControllerTest < ActionController::TestCase
assert assigns(:level)
assert assigns(:level).game
- assert_equal @user, assigns(:level).user
+ assert_equal @levelbuilder, assigns(:level).user
assert_equal edit_level_path(assigns(:level)), JSON.parse(@response.body)["redirect"]
end
@@ -203,28 +205,32 @@ class LevelsControllerTest < ActionController::TestCase
assert_equal level.properties[:toolbox_blocks.to_s], @program
end
- test "should not update blocks if not admin" do
- sign_in @not_admin
- post :update_blocks, :level_id => @level.id, :game_id => @level.game.id, :type => 'toolbox_blocks', :program => @program
- assert_response :forbidden
+ test "should not update blocks if not levelbuilder" do
+ [@not_admin, @admin].each do |user|
+ sign_in user
+ post :update_blocks, :level_id => @level.id, :game_id => @level.game.id, :type => 'toolbox_blocks', :program => @program
+ assert_response :forbidden
+ end
end
test "should not edit level if not custom level" do
level = Script.twenty_hour_script.levels.first
- can_edit = Ability.new(@user).can? :edit, level
+ can_edit = Ability.new(@levelbuilder).can? :edit, level
assert_equal false, can_edit
post :update_blocks, :level_id => level.id, :game_id => level.game.id, :type => 'toolbox_blocks', :program => @program
assert_response :forbidden
end
- test "should not create level if not admin" do
- sign_in @not_admin
- assert_no_difference('Level.count') do
- post :create, :name => "NewCustomLevel", :program => @program
- end
+ test "should not create level if not levelbuilder" do
+ [@not_admin, @admin].each do |user|
+ sign_in user
+ assert_no_difference('Level.count') do
+ post :create, :name => "NewCustomLevel", :program => @program
+ end
- assert_response :forbidden
+ assert_response :forbidden
+ end
end
# This should represent the behavior on production.
@@ -415,7 +421,7 @@ class LevelsControllerTest < ActionController::TestCase
test 'should hide legacy unplugged pdf download button for students' do
level = create :unplugged, name: 'OldUnplugged', type: 'Unplugged'
teacher = create(:teacher)
- sign_out(@user)
+ sign_out(@levelbuilder)
sign_in(teacher)
get :show, id: level, game_id: level.game
assert_select '.pdf-button'
@@ -439,7 +445,7 @@ class LevelsControllerTest < ActionController::TestCase
test 'should hide unplugged pdf download section for students' do
level = create :unplugged, name: 'NewUnplugged', type: 'Unplugged'
teacher = create(:teacher)
- sign_out(@user)
+ sign_out(@levelbuilder)
sign_in(teacher)
get :show, id: level, game_id: level.game
assert_select '.pdf-button'
@@ -520,7 +526,7 @@ class LevelsControllerTest < ActionController::TestCase
set_env :test
level = create :artist
- sign_out @user
+ sign_out @levelbuilder
get :edit, id: level
assert_response :redirect
@@ -533,7 +539,7 @@ class LevelsControllerTest < ActionController::TestCase
set_env :test
level = create :artist
- sign_out @user
+ sign_out @levelbuilder
get :embed_level, level_id: level
assert_response :success
@@ -543,7 +549,7 @@ class LevelsControllerTest < ActionController::TestCase
set_env :test
level = create :artist
- sign_out @user
+ sign_out @levelbuilder
get :embed_blocks, level_id: level, block_type: :solution_blocks
assert_response :success
diff --git a/dashboard/test/controllers/script_levels_controller_test.rb b/dashboard/test/controllers/script_levels_controller_test.rb
index 0afbccd8d74ce..76aec7f65a1f5 100644
--- a/dashboard/test/controllers/script_levels_controller_test.rb
+++ b/dashboard/test/controllers/script_levels_controller_test.rb
@@ -753,23 +753,23 @@ def assert_caching_enabled(cache_control_header, max_age, proxy_max_age)
end
test 'includes makerlab javascript dependencies when makerlab level' do
- sign_in @admin
+ sign_in @teacher
level = create :makerlab
script_level = create :script_level, levels: [level]
- get :show, script_id: script_level.script, stage_id: script_level.stage, id: script_level.position, user_id: @admin.id
+ get :show, script_id: script_level.script, stage_id: script_level.stage, id: script_level.position, user_id: @teacher.id
assert_select 'script[src=?]', ActionController::Base.helpers.javascript_path('js/makerlab')
end
test 'excludes makerlab javascript dependencies when applab level' do
- sign_in @admin
+ sign_in @teacher
level = create :applab
script_level = create :script_level, levels: [level]
- get :show, script_id: script_level.script, stage_id: script_level.stage, id: script_level.position, user_id: @admin.id
+ get :show, script_id: script_level.script, stage_id: script_level.stage, id: script_level.position, user_id: @teacher.id
assert_select 'script[src=?]', ActionController::Base.helpers.javascript_path('js/makerlab'), false
end
@@ -962,12 +962,15 @@ def create_admin_script
assert_response :forbidden
end
- test "should get show of admin script if signed in as admin" do
+ # TODO(asher): Consolidate the tests when the user is an admin. In particular,
+ # these scripts should no longer exist, should be accessible to noone, and
+ # will go away.
+ test "should not get show of admin script if signed in as admin" do
admin_script = create_admin_script
sign_in create(:admin)
get :show, script_id: admin_script.name, stage_id: 1, id: 1
- assert_response :success
+ assert_response :forbidden
end
def create_student_of_admin_script
@@ -999,12 +1002,12 @@ def create_student_of_admin_script
assert_response :success
end
- test "should get show of student_of_admin script if signed in as admin" do
+ test "should not get show of student_of_admin script if signed in as admin" do
script = create_student_of_admin_script
sign_in create(:admin)
get :show, script_id: script.name, stage_id: 1, id: 1
- assert_response :success
+ assert_response :forbidden
end
test "should have milestone posting disabled if Milestone is set" do
diff --git a/dashboard/test/controllers/scripts_controller_test.rb b/dashboard/test/controllers/scripts_controller_test.rb
index e2e2f9958a769..f0993e4d113fd 100644
--- a/dashboard/test/controllers/scripts_controller_test.rb
+++ b/dashboard/test/controllers/scripts_controller_test.rb
@@ -7,6 +7,7 @@ class ScriptsControllerTest < ActionController::TestCase
setup do
@admin = create(:admin)
@not_admin = create(:user)
+ @levelbuilder = create(:levelbuilder)
Rails.application.config.stubs(:levelbuilder_mode).returns false
end
@@ -14,7 +15,7 @@ class ScriptsControllerTest < ActionController::TestCase
test "should get index" do
Rails.application.config.stubs(:levelbuilder_mode).returns true
- sign_in(@admin)
+ sign_in(@levelbuilder)
get :index
assert_response :success
assert_not_nil assigns(:scripts)
@@ -27,12 +28,14 @@ class ScriptsControllerTest < ActionController::TestCase
assert_redirected_to_sign_in
end
- test "should not get index if not admin" do
- sign_in @not_admin
+ test "should not get index if not levelbuilder" do
+ [@admin, @not_admin].each do |user|
+ sign_in user
- get :index
+ get :index
- assert_response :forbidden
+ assert_response :forbidden
+ end
end
test "should get show of hoc" do
@@ -125,6 +128,12 @@ class ScriptsControllerTest < ActionController::TestCase
assert_response :success
end
+ test 'should not get show if admin' do
+ sign_in @admin
+ get :show, id: Script::FLAPPY_NAME
+ assert_response :forbidden
+ end
+
test "should use script name as param where script name is words but looks like a number" do
script = create(:script, name: '15-16')
get :show, id: "15-16"
@@ -153,9 +162,9 @@ class ScriptsControllerTest < ActionController::TestCase
end
end
- test "should not get edit if not levelbuilder" do
+ test "should not get edit if not levelbuilder mode" do
Rails.application.config.stubs(:levelbuilder_mode).returns false
- sign_in @admin
+ sign_in @levelbuilder
get :edit, id: 'course1'
assert_response :forbidden
@@ -168,17 +177,19 @@ class ScriptsControllerTest < ActionController::TestCase
assert_redirected_to_sign_in
end
- test "should not get edit if not admin" do
+ test "should not get edit if not levelbuilder" do
Rails.application.config.stubs(:levelbuilder_mode).returns true
- sign_in @not_admin
- get :edit, id: 'course1'
+ [@not_admin, @admin].each do |user|
+ sign_in user
+ get :edit, id: 'course1'
- assert_response :forbidden
+ assert_response :forbidden
+ end
end
test "edit" do
Rails.application.config.stubs(:levelbuilder_mode).returns true
- sign_in @admin
+ sign_in @levelbuilder
script = Script.find_by_name('course1')
get :edit, id: script.name
@@ -200,7 +211,7 @@ class ScriptsControllerTest < ActionController::TestCase
end
test "edit forbidden if not on levelbuilder" do
- sign_in @admin
+ sign_in @levelbuilder
get :edit, id: 'course1'
assert_response :forbidden
end
@@ -224,11 +235,11 @@ def create_admin_script
assert_response :forbidden
end
- test "should get show of admin script if signed in as admin" do
+ test "should not get show of admin script if signed in as admin" do
admin_script = create_admin_script
sign_in @admin
get :show, id: admin_script.name
- assert_response :success
+ assert_response :forbidden
end
end
diff --git a/dashboard/test/factories.rb b/dashboard/test/factories.rb
index 547b079e53b94..706b897ed7f1b 100644
--- a/dashboard/test/factories.rb
+++ b/dashboard/test/factories.rb
@@ -13,6 +13,13 @@
admin true
end
+ factory :levelbuilder do
+ after(:create) do |levelbuilder|
+ levelbuilder.permission = UserPermission::LEVELBUILDER
+ levelbuilder.save
+ end
+ end
+
factory :teacher do
user_type User::TYPE_TEACHER
birthday Date.new(1980, 03, 14)
diff --git a/dashboard/test/helpers/levels_helper_test.rb b/dashboard/test/helpers/levels_helper_test.rb
index 323aa5c4ce35b..6ba57d37f1b18 100644
--- a/dashboard/test/helpers/levels_helper_test.rb
+++ b/dashboard/test/helpers/levels_helper_test.rb
@@ -311,7 +311,7 @@ def stub_country(code)
assert_not can_view_solution?
sign_out user
- user = create :admin
+ user = create :levelbuilder
sign_in user
assert can_view_solution?
@@ -333,7 +333,7 @@ def stub_country(code)
@script = create(:script)
@script_level = create(:script_level, level: @level, script: @script)
- user = create :admin
+ user = create :levelbuilder
sign_in user
assert can_view_solution?
diff --git a/dashboard/test/models/ability_test.rb b/dashboard/test/models/ability_test.rb
index 5d1a78888a6ae..7f14886c18c61 100644
--- a/dashboard/test/models/ability_test.rb
+++ b/dashboard/test/models/ability_test.rb
@@ -103,27 +103,30 @@ class AbilityTest < ActiveSupport::TestCase
test "as admin" do
ability = Ability.new(create(:admin))
- assert ability.can?(:read, Game)
- assert ability.can?(:read, Level)
- assert ability.can?(:read, Activity)
-
- assert ability.can?(:destroy, Game)
- # Can only destroy custom levels
- assert ability.can?(:destroy, Level.where.not(user_id: nil).first)
- assert ability.can?(:destroy, Activity)
-
- assert ability.can?(:read, Script.find_by_name('ECSPD'))
- assert ability.can?(:read, Script.find_by_name('flappy'))
-
- assert ability.can?(:read, @public_script)
- assert ability.can?(:read, @login_required_script)
- assert ability.can?(:read, @student_of_admin_script)
- assert ability.can?(:read, @admin_script)
-
- assert ability.can?(:read, @public_script_level)
- assert ability.can?(:read, @login_required_script_level)
- assert ability.can?(:read, @student_of_admin_script_level)
- assert ability.can?(:read, @admin_script_level)
+ assert ability.cannot?(:read, Activity)
+ assert ability.cannot?(:read, Game)
+ assert ability.cannot?(:read, Level)
+ assert ability.cannot?(:read, Script)
+ assert ability.cannot?(:read, ScriptLevel)
+ assert ability.cannot?(:read, UserLevel)
+ assert ability.cannot?(:read, UserScript)
+
+ assert ability.cannot?(:destroy, Game)
+ assert ability.cannot?(:destroy, Level)
+ assert ability.cannot?(:destroy, Activity)
+
+ assert ability.cannot?(:read, Script.find_by_name('ECSPD'))
+ assert ability.cannot?(:read, Script.find_by_name('flappy'))
+
+ assert ability.cannot?(:read, @public_script)
+ assert ability.cannot?(:read, @login_required_script)
+ assert ability.cannot?(:read, @student_of_admin_script)
+ assert ability.cannot?(:read, @admin_script)
+
+ assert ability.cannot?(:read, @public_script_level)
+ assert ability.cannot?(:read, @login_required_script_level)
+ assert ability.cannot?(:read, @student_of_admin_script_level)
+ assert ability.cannot?(:read, @admin_script_level)
end
test 'with hint_access manage LevelSourceHint and FrequentUnsuccessfulLevelSource' do
@@ -183,6 +186,7 @@ class AbilityTest < ActiveSupport::TestCase
user_id: user.id, permission: UserPermission::LEVELBUILDER)
ability = Ability.new user
+ assert ability.can?(:manage, Game)
assert ability.can?(:manage, Level)
assert ability.can?(:manage, Script)
assert ability.can?(:manage, ScriptLevel)