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

Revert "add :read and :read_sis permissions on pseudonym" 

This reverts commit 9d49603aa1.

fixes CNVS-17290

Change-Id: I50502bd8569294232c3c2f24e7fee8af1b9ec543
Reviewed-on: https://gerrit.instructure.com/45388
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Jacob Fugal <jacob@instructure.com>
QA-Review: Caleb Guanzon <cguanzon@instructure.com>
Product-Review: Matt Fairbourn <mfairbourn@instructure.com>
This commit is contained in:
Jeremy Stanley 2014-12-07 17:12:52 -07:00 committed by Rob Orton
parent 006bd091c5
commit bfbca511b8
6 changed files with 11 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
app/models/pseudonym.rb
View File

@ -234,12 +234,6 @@ class Pseudonym < ActiveRecord::Base
end
set_policy do
given do |user|
user.try(:id) == self.user_id ||
self.account.grants_any_right?(user, :manage_user_logins, :manage_students)
end
can :read
# an admin can only create and update pseudonyms when they have
# :manage_user_logins permission on the pseudonym's account, :read
# permission on the pseudonym's owner, and a superset of hte pseudonym's
@ -287,16 +281,7 @@ class Pseudonym < ActiveRecord::Base
self.account.grants_right?(user, :manage_sis) &&
self.grants_right?(user, :update)
end
can :manage_sis and can :read_sis
# an admin can read a pseudonym's SIS ID when they have :read_sis
# permission on the pseudonym's account and the pseudonym's owner is
# visible to them
given do |user|
self.account.grants_right?(user, :read_sis) &&
self.grants_right?(user, :read)
end
can :read_sis
can :manage_sis
# an admin can delete any non-SIS pseudonym that they can update
given do |user|

4
app/views/users/_logins.html.erb
View File

@ -4,12 +4,12 @@
<fieldset id="login_information">
<legend><%= t('titles.logins', 'Login Information') %></legend>
<table>
<% pseudonyms = @user.all_active_pseudonyms.select{ |p| can_do(p, @current_user, :read) } + [nil] %>
<% pseudonyms = @user.all_active_pseudonyms + [nil] %>
<% pseudonyms.each do |pseudonym| %>
<tr class='login <%= 'blank' unless pseudonym %>' style="<%= hidden unless pseudonym %>">
<th scope='row'>
<b class='unique_id'><%= pseudonym && pseudonym.unique_id %></b>
<% if pseudonym && ((pseudonym.sis_user_id && can_do(pseudonym, @current_user, :read_sis)) || can_do(pseudonym, @current_user, :manage_sis)) %>
<% if pseudonym && ((pseudonym.sis_user_id && can_do(pseudonym.account, @current_user, :read_sis)) || can_do(pseudonym, @current_user, :manage_sis)) %>
<div style="font-size: 0.8em; padding-left: 20px;">
<%= before_label('sis_id', 'SIS ID') %> <span id="sis_user_id_<%= pseudonym.id %>" class="sis_user_id"><%= pseudonym.sis_user_id %></span>
</div>

2
lib/api/v1/pseudonym.rb
View File

@ -5,7 +5,7 @@ module Api::V1::Pseudonym
def pseudonym_json(pseudonym, current_user, session)
opts = API_PSEUDONYM_JSON_OPTS
opts = opts.reject { |opt| opt == :sis_user_id } unless pseudonym.grants_right?(current_user, :read_sis)
opts = opts.reject { |opt| opt == :sis_user_id } unless pseudonym.account.grants_any_right?(current_user, :read_sis, :manage_sis)
api_json(pseudonym, current_user, session, :only => opts)
end

4
lib/api/v1/user.rb
View File

@ -49,8 +49,8 @@ module Api::V1::User
json.merge! :sis_user_id => sis_pseudonym.sis_user_id,
:integration_id => sis_pseudonym.integration_id,
# TODO: don't send sis_login_id; it's garbage data
:sis_login_id => sis_pseudonym.unique_id if sis_pseudonym.grants_right?(current_user, :read_sis)
json[:sis_import_id] = sis_pseudonym.sis_batch_id if sis_pseudonym.grants_right?(current_user, :manage_sis)
:sis_login_id => sis_pseudonym.unique_id if @domain_root_account.grants_any_right?(current_user, :read_sis, :manage_sis)
json[:sis_import_id] = sis_pseudonym.sis_batch_id if @domain_root_account.grants_right?(current_user, session, :manage_sis)
json[:root_account] = HostUrl.context_host(sis_pseudonym.account) if include_root_account
end
if pseudonym = (sis_pseudonym || user.find_pseudonym_for_account(@domain_root_account))

8
spec/apis/v1/courses_api_spec.rb
View File

@ -1365,8 +1365,8 @@ describe CoursesController, type: :request do
expect(json.map { |u| u['login_id'] }.sort).to eq ["nobody@example.com", "nobody2@example.com"].sort
end
it "should include user sis id and login id if can manage_sis in the account" do
account_admin_user(user: @me)
it "should include user sis id and login id if can manage_students in the course" do
expect(@course1.grants_right?(@me, :manage_students)).to be_truthy
first_student = user_with_pseudonym(:name => 'Zombo', :username => 'nobody2@example.com')
@course1.enroll_student(first_student).accept!
first_student.pseudonym.update_attribute(:sis_user_id, 'user2')
@ -1756,8 +1756,8 @@ describe CoursesController, type: :request do
expect(json.map { |u| u['login_id'] }.sort).to eq ["nobody@example.com", "nobody2@example.com"].sort
end
it "should include user sis id and login id if can manage_sis in the account" do
account_admin_user(user: @me)
it "should include user sis id and login id if can manage_students in the course" do
expect(@course1.grants_right?(@me, :manage_students)).to be_truthy
first_student = user_with_pseudonym(:name => 'Zombo', :username => 'nobody2@example.com')
@course1.enroll_student(first_student).accept!
first_student.pseudonym.update_attribute(:sis_user_id, 'user2')

4
spec/apis/v1/users_api_spec.rb
View File

@ -104,13 +104,11 @@ describe Api::V1::User do
it 'should use an sis pseudonym from another account if necessary' do
@user = User.create!(:name => 'User')
@account2 = Account.create!
sis_pseudonym = @user.pseudonyms.create!(:unique_id => 'abc', :account => @account2) { |p| p.sis_user_id = 'a'}
@user.pseudonyms.create!(:unique_id => 'abc', :account => @account2) { |p| p.sis_user_id = 'a'}
Account.default.any_instantiation.stubs(:trust_exists?).returns(true)
Account.default.any_instantiation.stubs(:trusted_account_ids).returns([@account2.id])
HostUrl.expects(:context_host).with(@account2).returns('school1')
@user.stubs(:find_pseudonym_for_account).with(Account.default).returns(@pseudonym)
sis_pseudonym.any_instantiation.stubs(:grants_right?).with(@admin, :read_sis).returns(true)
sis_pseudonym.any_instantiation.stubs(:grants_right?).with(@admin, :manage_sis).returns(true)
expect(@test_api.user_json(@user, @admin, {}, [], Account.default)).to eq({
'name' => 'User',
'sortable_name' => 'User',