ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

clarify/fix outcome API permissions 

fixes #11504

Change-Id: If7c480bfac9b0339f9ce9f0ec6d5705dafbcb037
test-plan:
 - global outcomes should be visible to:
   - any logged in user
 - outcomes defined in an account should be visible to:
   - admins in subaccounts
   - enrollees in the account's courses
   - enrollees in the account's subaccounts' courses
 - outcomes defined in a course should be visible to:
   - any user that can view the course
 - global outcomes should not be visible without being logged in
 - managing outcomes should require the same permissions as before
Reviewed-on: https://gerrit.instructure.com/14910
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Zach Pendleton <zachp@instructure.com>
This commit is contained in:
Jacob Fugal 2012-10-31 14:28:33 -06:00
parent cb6661ea4b
commit b68592b602
10 changed files with 216 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
app/controllers/outcome_groups_api_controller.rb
View File

@ -22,8 +22,8 @@
class OutcomeGroupsApiController < ApplicationController
include Api::V1::Outcome
before_filter :get_context, :except => :global_redirect
before_filter :require_context, :only => :context_redirect
before_filter :require_user
before_filter :get_context
# @API Redirect for global outcomes
# Convenience redirect to find the root outcome group for global outcomes.
@ -239,10 +239,9 @@ class OutcomeGroupsApiController < ApplicationController
def can_read_outcomes
if @context
authorized_action(@context, @current_user, :read)
authorized_action(@context, @current_user, :read_outcomes)
else
# anyone (that's logged in) can read global outcomes
true
authorized_action(Account.site_admin, @current_user, :read_global_outcomes)
end
end

22
app/controllers/outcomes_api_controller.rb
View File

@ -22,18 +22,19 @@
class OutcomesApiController < ApplicationController
include Api::V1::Outcome
before_filter :require_user
before_filter :get_outcome
# @API Retrieve an outcome's details.
def show
if can_read_outcome
if authorized_action(@outcome, @current_user, :read)
render :json => outcome_json(@outcome, @current_user, session)
end
end
# @API Update an outcome.
def update
if can_manage_outcome
if authorized_action(@outcome, @current_user, :update)
@outcome.update_attributes(params.slice(:title, :description))
if params[:mastery_points] || params[:ratings]
criterion = @outcome.data && @outcome.data[:rubric_criterion]
@ -61,21 +62,4 @@ class OutcomesApiController < ApplicationController
def get_outcome
@outcome = LearningOutcome.active.find(params[:id])
end
def can_read_outcome
if @outcome.context_id
authorized_action(@outcome.context, @current_user, :read)
else
# anyone (that's logged in) can read global outcomes
true
end
end
def can_manage_outcome
if @outcome.context_id
authorized_action(@outcome.context, @current_user, :manage_outcomes)
else
authorized_action(Account.site_admin, @current_user, :manage_global_outcomes)
end
end
end

10
app/models/account.rb
View File

@ -586,7 +586,7 @@ class Account < ActiveRecord::Base
end
given { |user| !self.account_users_for(user).empty? }
can :read and can :manage and can :update and can :delete
can :read and can :manage and can :update and can :delete and can :read_outcomes
given { |user|
root_account = self.root_account
@ -609,6 +609,14 @@ class Account < ActiveRecord::Base
result
}
can :create_courses
# any logged in user can read global outcomes, but must be checked against the site admin
given{ |user,session| self.site_admin? && user }
can :read_global_outcomes
# any user with an association to this account can read the outcomes in the account
given{ |user,session| user && self.user_account_associations.find_by_user_id(user.id) }
can :read_outcomes
end
alias_method :destroy!, :destroy

22
app/models/course.rb
View File

@ -887,7 +887,7 @@ class Course < ActiveRecord::Base
set_policy do
given { |user| self.available? && self.is_public }
can :read
can :read and can :read_outcomes
RoleOverride.permissions.each do |permission, details|
given {|user, session| (self.enrollment_allows(user, session, permission) || self.account_membership_allows(user, session, permission)) && (!details[:if] || send(details[:if])) }
@ -895,14 +895,14 @@ class Course < ActiveRecord::Base
end
given { |user, session| session && session[:enrollment_uuid] && (hash = Enrollment.course_user_state(self, session[:enrollment_uuid]) || {}) && (hash[:enrollment_state] == "invited" || hash[:enrollment_state] == "active" && hash[:user_state] == "pre_registered") && (self.available? || self.completed? || self.claimed? && hash[:is_admin]) }
can :read
can :read and can :read_outcomes
given { |user| (self.available? || self.completed?) && user && user.cached_current_enrollments.any?{|e| e.course_id == self.id && [:active, :invited, :completed].include?(e.state_based_on_date) } }
can :read
can :read and can :read_outcomes
# Active students
given { |user| self.available? && user && user.cached_current_enrollments.any?{|e| e.course_id == self.id && e.participating_student? } }
can :read and can :participate_as_student and can :read_grades
can :read and can :participate_as_student and can :read_grades and can :read_outcomes
given { |user| (self.available? || self.completed?) && user && user.cached_not_ended_enrollments.any?{|e| e.course_id == self.id && e.participating_observer? && e.associated_user_id} }
can :read_grades
@ -912,7 +912,7 @@ class Course < ActiveRecord::Base
# Active admins (Teacher/TA/Designer)
given { |user, session| (self.available? || self.created? || self.claimed?) && user && user.cached_not_ended_enrollments.any?{|e| e.course_id == self.id && e.participating_admin? } && (!session || !session["role_course_#{self.id}"]) }
can :read_as_admin and can :read and can :manage and can :update and can :use_student_view
can :read_as_admin and can :read and can :manage and can :update and can :use_student_view and can :read_outcomes
# Teachers and Designers can delete/reset, but not TAs
given { |user, session| !self.deleted? && !self.sis_source_id && user && user.cached_not_ended_enrollments.any?{|e| e.course_id == self.id && e.participating_content_admin? } && (!session || !session["role_course_#{self.id}"]) }
@ -922,15 +922,15 @@ class Course < ActiveRecord::Base
# Student view student
given { |user| user && user.fake_student? && user.cached_not_ended_enrollments.any?{ |e| e.course_id == self.id } }
can :read and can :participate_as_student and can :read_grades
can :read and can :participate_as_student and can :read_grades and can :read_outcomes
# Prior users
given { |user| (self.available? || self.completed?) && user && self.prior_enrollments.map(&:user_id).include?(user.id) }
can :read
can :read and can :read_outcomes
# Admin (Teacher/TA/Designer) of a concluded course
given { |user| !self.deleted? && user && (self.prior_enrollments.select{|e| e.admin? }.map(&:user_id).include?(user.id) || user.cached_not_ended_enrollments.any? { |e| e.course_id == self.id && e.admin? && e.completed? }) }
can :read and can :read_as_admin and can :read_roster and can :read_prior_roster and can :read_forum and can :use_student_view
can :read and can :read_as_admin and can :read_roster and can :read_prior_roster and can :read_forum and can :use_student_view and can :read_outcomes
given { |user| !self.deleted? && user && (self.prior_enrollments.select{|e| e.instructor? }.map(&:user_id).include?(user.id) || user.cached_not_ended_enrollments.any? { |e| e.course_id == self.id && e.instructor? && e.completed? }) }
can :read_user_notes and can :view_all_grades
@ -941,11 +941,11 @@ class Course < ActiveRecord::Base
# Student of a concluded course
given { |user| (self.available? || self.completed?) && user && (self.prior_enrollments.select{|e| e.student? || e.assigned_observer? }.map(&:user_id).include?(user.id) || user.cached_not_ended_enrollments.any? { |e| e.course_id == self.id && (e.student? || e.assigned_observer?) && e.state_based_on_date == :completed }) }
can :read and can :read_grades and can :read_forum
can :read and can :read_grades and can :read_forum and can :read_outcomes
# Viewing as different role type
given { |user, session| session && session["role_course_#{self.id}"] }
can :read
can :read and can :read_outcomes
# Admin
given { |user, session| self.account_membership_allows(user, session) }
@ -955,7 +955,7 @@ class Course < ActiveRecord::Base
can :read_as_admin and can :manage and can :update and can :delete and can :use_student_view and can :reset_content
given { |user, session| self.account_membership_allows(user, session, :read_course_content) }
can :read
can :read and can :read_outcomes
given { |user, session| !self.deleted? && self.sis_source_id && self.account_membership_allows(user, session, :manage_sis) }
can :delete

15
app/models/learning_outcome.rb
View File

@ -29,8 +29,21 @@ class LearningOutcome < ActiveRecord::Base
sanitize_field :description, Instructure::SanitizeField::SANITIZE
set_policy do
given {|user, session| self.cached_context_grants_right?(user, session, :manage_outcomes) }
# managing a contextual outcome requires manage_outcomes on the outcome's context
given {|user, session| self.context_id && self.cached_context_grants_right?(user, session, :manage_outcomes) }
can :create and can :read and can :update and can :delete
# reading a contextual outcome is also allowed by read_outcomes on the outcome's context
given {|user, session| self.context_id && self.cached_context_grants_right?(user, session, :read_outcomes) }
can :read
# managing a global outcome requires manage_global_outcomes on the site_admin
given {|user, session| self.context_id.nil? && Account.site_admin.grants_right?(user, session, :manage_global_outcomes) }
can :create and can :read and can :update and can :delete
# reading a global outcome is also allowed by just being logged in
given {|user, session| self.context_id.nil? && user }
can :read
end
def infer_defaults

22
spec/apis/v1/outcome_groups_api_spec.rb
View File

@ -51,6 +51,15 @@ describe "Outcome Groups API", :type => :integration do
response.status.to_i.should == 302
end
it "should require a user" do
@user = nil
raw_api_call(:get, "/api/v1/global/root_outcome_group",
:controller => 'outcome_groups_api',
:action => 'redirect',
:format => 'json')
response.status.to_i.should == 401
end
it "should redirect to the root global group" do
root = LearningOutcomeGroup.global_root_outcome_group
raw_api_call(:get, "/api/v1/global/root_outcome_group",
@ -80,7 +89,7 @@ describe "Outcome Groups API", :type => :integration do
@account_user = @user.account_users.create(:account => @account)
end
it "should NOT require permission to read" do
it "should not require manage permission to read" do
revoke_permission(@account_user, :manage_outcomes)
raw_api_call(:get, "/api/v1/accounts/#{@account.id}/root_outcome_group",
:controller => 'outcome_groups_api',
@ -90,6 +99,17 @@ describe "Outcome Groups API", :type => :integration do
response.status.to_i.should == 302
end
it "should require read permission to read" do
# new user, doesn't have a tie to the account
user_with_pseudonym(:account => Account.create!, :active_all => true)
raw_api_call(:get, "/api/v1/accounts/#{@account.id}/root_outcome_group",
:controller => 'outcome_groups_api',
:action => 'redirect',
:account_id => @account.id.to_s,
:format => 'json')
response.status.to_i.should == 401
end
it "should redirect to the root group" do
root = @account.root_outcome_group
raw_api_call(:get, "/api/v1/accounts/#{@account.id}/root_outcome_group",

37
spec/apis/v1/outcomes_api_spec.rb
View File

@ -38,7 +38,7 @@ describe "Outcomes API", :type => :integration do
end
describe "show" do
it "should not require permission" do
it "should not require manage permission" do
revoke_permission(@account_user, :manage_outcomes)
raw_api_call(:get, "/api/v1/outcomes/#{@outcome.id}",
:controller => 'outcomes_api',
@ -48,6 +48,39 @@ describe "Outcomes API", :type => :integration do
response.status.to_i.should == 200
end
it "should require read permission" do
# new user, doesn't have a tie to the account
user_with_pseudonym(:account => Account.create!, :active_all => true)
raw_api_call(:get, "/api/v1/outcomes/#{@outcome.id}",
:controller => 'outcomes_api',
:action => 'show',
:id => @outcome.id.to_s,
:format => 'json')
response.status.to_i.should == 401
end
it "should not require any permission for global outcomes" do
user_with_pseudonym(:account => Account.create!, :active_all => true)
@outcome = LearningOutcome.create!(:title => "My Outcome")
raw_api_call(:get, "/api/v1/outcomes/#{@outcome.id}",
:controller => 'outcomes_api',
:action => 'show',
:id => @outcome.id.to_s,
:format => 'json')
response.status.to_i.should == 200
end
it "should still require a user for global outcomes" do
@outcome = LearningOutcome.create!(:title => "My Outcome")
@user = nil
raw_api_call(:get, "/api/v1/outcomes/#{@outcome.id}",
:controller => 'outcomes_api',
:action => 'show',
:id => @outcome.id.to_s,
:format => 'json')
response.status.to_i.should == 401
end
it "should 404 for deleted outcomes" do
@outcome.destroy
raw_api_call(:get, "/api/v1/outcomes/#{@outcome.id}",
@ -113,7 +146,7 @@ describe "Outcomes API", :type => :integration do
end
describe "update" do
it "should require permission" do
it "should require manage permission" do
revoke_permission(@account_user, :manage_outcomes)
raw_api_call(:put, "/api/v1/outcomes/#{@outcome.id}",
:controller => 'outcomes_api',

64
spec/models/account_spec.rb
View File

@ -384,21 +384,21 @@ describe Account do
hash[k][:user] = user
end
limited_access = [ :read, :manage, :update, :delete ]
limited_access = [ :read, :manage, :update, :delete, :read_outcomes ]
full_access = RoleOverride.permissions.map { |k, v| k } + limited_access
index = full_access.index(:manage_courses)
full_access = full_access[0..index] + [:create_courses] + full_access[index+1..-1]
# site admin has access to everything everywhere
hash.each do |k, v|
account = v[:account]
account.check_policy(hash[:site_admin][:admin]).should == full_access
account.check_policy(hash[:site_admin][:user]).should == limited_access
account.check_policy(hash[:site_admin][:admin]).should == full_access + (k == :site_admin ? [:read_global_outcomes] : [])
account.check_policy(hash[:site_admin][:user]).should == limited_access + (k == :site_admin ? [:read_global_outcomes] : [])
end
# root admin has access to everything except site admin
account = hash[:site_admin][:account]
account.check_policy(hash[:root][:admin]).should == []
account.check_policy(hash[:root][:user]).should == []
account.check_policy(hash[:root][:admin]).should == [:read_global_outcomes]
account.check_policy(hash[:root][:user]).should == [:read_global_outcomes]
hash.each do |k, v|
next if k == :site_admin
account = v[:account]
@ -410,8 +410,8 @@ describe Account do
hash.each do |k, v|
next unless k == :site_admin || k == :root
account = v[:account]
account.check_policy(hash[:sub][:admin]).should == []
account.check_policy(hash[:sub][:user]).should == []
account.check_policy(hash[:sub][:admin]).should == (k == :site_admin ? [:read_global_outcomes] : [:read_outcomes])
account.check_policy(hash[:sub][:user]).should == (k == :site_admin ? [:read_global_outcomes] : [:read_outcomes])
end
hash.each do |k, v|
next if k == :site_admin || k == :root
@ -429,13 +429,13 @@ describe Account do
RoleOverride.clear_cached_contexts
hash.each do |k, v|
account = v[:account]
account.check_policy(hash[:site_admin][:admin]).should == full_access
account.check_policy(hash[:site_admin][:user]).should == some_access
account.check_policy(hash[:site_admin][:admin]).should == full_access + (k == :site_admin ? [:read_global_outcomes] : [])
account.check_policy(hash[:site_admin][:user]).should == some_access + (k == :site_admin ? [:read_global_outcomes] : [])
end
account = hash[:site_admin][:account]
account.check_policy(hash[:root][:admin]).should == []
account.check_policy(hash[:root][:user]).should == []
account.check_policy(hash[:root][:admin]).should == [:read_global_outcomes]
account.check_policy(hash[:root][:user]).should == [:read_global_outcomes]
hash.each do |k, v|
next if k == :site_admin
account = v[:account]
@ -447,8 +447,8 @@ describe Account do
hash.each do |k, v|
next unless k == :site_admin || k == :root
account = v[:account]
account.check_policy(hash[:sub][:admin]).should == []
account.check_policy(hash[:sub][:user]).should == []
account.check_policy(hash[:sub][:admin]).should == (k == :site_admin ? [:read_global_outcomes] : [:read_outcomes])
account.check_policy(hash[:sub][:user]).should == (k == :site_admin ? [:read_global_outcomes] : [:read_outcomes])
end
hash.each do |k, v|
next if k == :site_admin || k == :root
@ -737,6 +737,44 @@ describe Account do
@course.enroll_teacher(@user).accept!
Account.default.grants_right?(@user, :read_sis).should be_true
end
it "should grant :read_global_outcomes to any user iff site_admin" do
@site_admin = Account.site_admin
@site_admin.grants_right?(User.new, :read_global_outcomes).should be_true
@subaccount = @site_admin.sub_accounts.create!
@subaccount.grants_right?(User.new, :read_global_outcomes).should be_false
end
it "should not grant :read_outcomes to user's outside the account" do
Account.default.grants_right?(User.new, :read_outcomes).should be_false
end
it "should grant :read_outcomes to account admins" do
account_admin_user(:account => Account.default)
Account.default.grants_right?(@admin, :read_outcomes).should be_true
end
it "should grant :read_outcomes to subaccount admins" do
account_admin_user(:account => Account.default.sub_accounts.create!)
Account.default.grants_right?(@admin, :read_outcomes).should be_true
end
it "should grant :read_outcomes to enrollees in account courses" do
course(:account => Account.default)
teacher_in_course
student_in_course
Account.default.grants_right?(@teacher, :read_outcomes).should be_true
Account.default.grants_right?(@student, :read_outcomes).should be_true
end
it "should grant :read_outcomes to enrollees in subaccount courses" do
course(:account => Account.default.sub_accounts.create!)
teacher_in_course
student_in_course
Account.default.grants_right?(@teacher, :read_outcomes).should be_true
Account.default.grants_right?(@student, :read_outcomes).should be_true
end
end
describe "canvas_authentication?" do

16
spec/models/course_spec.rb
View File

@ -292,6 +292,22 @@ describe Course do
@enrollment.state_based_on_date.should == :inactive
@course.grants_right?(:read, @teacher).should be_false
end
it "should grant :read_outcomes to teachers in the course" do
course_with_teacher(:active_all => 1)
@course.grants_right?(@teacher, :read_outcomes).should be_true
end
it "should grant :read_outcomes to students in the course" do
course_with_student(:active_all => 1)
@course.grants_right?(@student, :read_outcomes).should be_true
end
it "should grant :read_outcomes to account admins" do
course(:active_all => 1)
account_admin_user(:account => @course.account)
@course.grants_right?(@admin, :read_outcomes).should be_true
end
end
it "should clear content when resetting" do

52
spec/models/learning_outcome_spec.rb
View File

@ -420,4 +420,56 @@ describe LearningOutcome do
@result.mastery.should eql(true)
end
end
describe "permissions" do
context "global outcome" do
before :each do
@outcome = LearningOutcome.create!(:title => 'global outcome')
end
it "should grant :read to any user" do
@outcome.grants_right?(User.new, :read).should be_true
end
it "should not grant :read without a user" do
@outcome.grants_right?(nil, :read).should be_false
end
it "should grant :update iff the site admin grants :manage_global_outcomes" do
@admin = stub
Account.site_admin.expects(:grants_right?).with(@admin, nil, :manage_global_outcomes).returns(true)
@outcome.grants_right?(@admin, :update).should be_true
Account.site_admin.expects(:grants_right?).with(@admin, nil, :manage_global_outcomes).returns(false)
@outcome.grants_right?(@admin, :update).should be_false
end
end
context "non-global outcome" do
before :each do
course(:active_course => 1)
@outcome = @course.created_learning_outcomes.create!(:title => 'non-global outcome')
end
it "should grant :read to users with :read_outcomes on the context" do
student_in_course(:active_enrollment => 1)
@outcome.grants_right?(@user, :read).should be_true
end
it "should not grant :read to users without :read_outcomes on the context" do
@outcome.grants_right?(User.new, :read).should be_false
end
it "should grant :update to users with :manage_outcomes on the context" do
teacher_in_course(:active_enrollment => 1)
@outcome.grants_right?(@user, :update).should be_true
end
it "should not grant :read to users without :read_outcomes on the context" do
student_in_course(:active_enrollment => 1)
@outcome.grants_right?(User.new, :update).should be_false
end
end
end
end