separate :read_profile from :rename rights

test plan:
 - users (teachers and students) should be able to rename
   themselves only if the "user can edit display name" account
   setting is enabled
 - teachers should not be able to rename students
   (unless they are account administrators)
 - account admins should be able to rename students
 - teachers (who are not account admins) should still be able
   to access student profiles via the API endpoint
   /api/v1/users/:user_id/profile

refs CNVS-15410

Change-Id: Ieca773a566ec55da7d28f8a2f647eba99a8ef1ea
Reviewed-on: https://gerrit.instructure.com/46402
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: James Williams  <jamesw@instructure.com>
QA-Review: Clare Strong <clare@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
This commit is contained in:
Jeremy Stanley 2015-01-05 08:23:02 -07:00
parent 61fae89590
commit f02cab90d5
3 changed files with 38 additions and 8 deletions

View File

@ -188,10 +188,8 @@ class ProfileController < ApplicationController
# @returns Profile
def settings
if api_request?
# allow querying this basic profile data for the current user, or any
# user the current user has renaming rights for (their teachers and admins)
@user = api_find(User, params[:user_id])
return unless @user == @current_user || authorized_action(@user, @current_user, :rename)
return unless authorized_action(@user, @current_user, :read_profile)
else
@user = @current_user
@user.dismiss_bouncing_channel_message!

View File

@ -984,19 +984,19 @@ class User < ActiveRecord::Base
set_policy do
given { |user| user == self }
can :read and can :read_as_admin and can :manage and can :manage_content and can :manage_files and can :manage_calendar and can :send_messages and can :update_avatar and can :manage_feature_flags
can :read and can :read_profile and can :read_as_admin and can :manage and can :manage_content and can :manage_files and can :manage_calendar and can :send_messages and can :update_avatar and can :manage_feature_flags
given { |user| user == self && user.user_can_edit_name? }
can :rename
given {|user| self.courses.any?{|c| c.user_is_instructor?(user)}}
can :rename and can :create_user_notes and can :read_user_notes
can :read_profile and can :create_user_notes and can :read_user_notes
# by default this means that the user we are given is an administrator
# of an account of one of the courses that this user is enrolled in, or
# an admin (teacher/ta/designer) in the course
given { |user| self.check_courses_right?(user, :read_reports) }
can :rename and can :remove_avatar and can :read_reports
can :read_profile and can :remove_avatar and can :read_reports
given { |user| self.check_courses_right?(user, :manage_user_notes) }
can :create_user_notes and can :read_user_notes
@ -1026,7 +1026,7 @@ class User < ActiveRecord::Base
self.associated_accounts.any? {|a| a.grants_right?(user, :manage_students) }
)
end
can :manage_user_details and can :update_avatar and can :remove_avatar and can :rename and can :view_statistics and can :read and can :read_reports and can :manage_feature_flags
can :manage_user_details and can :update_avatar and can :remove_avatar and can :rename and can :read_profile and can :view_statistics and can :read and can :read_reports and can :manage_feature_flags
given do |user|
user && (
@ -1044,7 +1044,7 @@ class User < ActiveRecord::Base
self.all_accounts.select(&:root_account?).all? {|a| has_subset_of_account_permissions?(user, a) } )
)
end
can :manage_user_details and can :rename
can :manage_user_details and can :rename and can :read_profile
given{ |user| self.pseudonyms.with_each_shard.any?{ |p| p.grants_right?(user, :update) } }
can :merge

View File

@ -764,6 +764,38 @@ describe "Users API", type: :request do
end
end
context "non-account-admin user" do
before :once do
user_with_pseudonym name: "Earnest Lambert Watkins"
course_with_teacher user: @user, active_all: true
end
context "with users_can_edit_name enabled" do
before :once do
@course.root_account.settings = { users_can_edit_name: true }
@course.root_account.save!
end
it "should allow user to rename self" do
json = api_call(:put, "/api/v1/users/#{@user.id}", @path_options.merge(id: @user.id),
{ user: { name: "Blue Ivy Carter" } })
expect(json["name"]).to eq "Blue Ivy Carter"
end
end
context "with users_can_edit_name disabled" do
before :once do
@course.root_account.settings = { users_can_edit_name: false }
@course.root_account.save!
end
it "should not allow user to rename self" do
api_call(:put, "/api/v1/users/#{@user.id}", @path_options.merge(id: @user.id),
{ user: { name: "Ovaltine Jenkins" } }, {}, { expected_status: 401 })
end
end
end
context "an unauthorized user" do
it "should receive a 401" do
user