Add account setting to limit parent app web access

refs: MBL-13883
flag = none

Test plan:
* View account settings as an admin that can manage the site settings
* Enable the option to "Limit access to Canvas web from the Canvas Parent
app"
* As an observer, visit `/api/v1/users/self`
* permissions.limit_parent_app_web_access should be true

* Disable the option in settings
* As an observer, visit `/api/v1/users/self` again
* permissions.limit_parent_app_web_access should be false
* View account settings as an admin that can manage the site settings

* View the account settings as an admin that can not manage site settings
* The option to "Limit access to Canvas web from Canvas Parent app" should
not be visible

Change-Id: Ief167618e0ffbc7dac4de97ea553a97150b44c67
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/224790
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Nate Armstrong <narmstrong@instructure.com>
Product-Review: Nate Armstrong <narmstrong@instructure.com>
This commit is contained in:
Nate Armstrong 2020-01-28 16:39:07 -07:00
parent 66f1762c54
commit 2c842eee28
9 changed files with 88 additions and 9 deletions

View File

@ -943,7 +943,8 @@ class AccountsController < ApplicationController
:include_integration_ids_in_gradebook_exports,
:show_scheduler,
:global_includes,
:gmail_domain
:gmail_domain,
:limit_parent_app_web_access,
].each do |key|
params[:account][:settings].try(:delete, key)
end
@ -1472,6 +1473,7 @@ class AccountsController < ApplicationController
:default_group_storage_quota, :default_group_storage_quota_mb,
:default_user_storage_quota, :default_user_storage_quota_mb, :default_time_zone,
:edit_institution_email, :enable_alerts, :enable_eportfolios, :enable_course_catalog,
:limit_parent_app_web_access,
{:enable_offline_web_export => [:value]}.freeze,
:enable_profiles, :enable_gravatar, :enable_turnitin, :equella_endpoint,
:equella_teaser, :external_notification_warning, :global_includes,

View File

@ -1247,7 +1247,8 @@ class UsersController < ApplicationController
# !!!javascript
# "permissions": {
# "can_update_name": true, // Whether the user can update their name.
# "can_update_avatar": false // Whether the user can update their avatar.
# "can_update_avatar": false, // Whether the user can update their avatar.
# "limit_parent_app_web_access": false // Whether the user can interact with Canvas web from the Canvas Parent app.
# }
#
# @argument include[] [String, "uuid"]

View File

@ -252,6 +252,7 @@ class Account < ActiveRecord::Base
add_setting :enable_course_catalog, :boolean => true, :root_only => true, :default => false
add_setting :usage_rights_required, :boolean => true, :default => false, :inheritable => true
add_setting :limit_parent_app_web_access, boolean: true, default: false
def settings=(hash)

View File

@ -113,7 +113,6 @@ class User < ActiveRecord::Base
has_many :pseudonyms, -> { order(:position) }, dependent: :destroy
has_many :active_pseudonyms, -> { where("pseudonyms.workflow_state<>'deleted'") }, class_name: 'Pseudonym'
has_many :pseudonym_accounts, :source => :account, :through => :pseudonyms
has_many :active_pseudonym_accounts, :source => :account, :through => :active_pseudonyms
has_one :pseudonym, -> { where("pseudonyms.workflow_state<>'deleted'").order(:position) }
has_many :attachments, :as => 'context', :dependent => :destroy
has_many :active_images, -> { where("attachments.file_state != ? AND attachments.content_type LIKE 'image%'", 'deleted').order('attachments.display_name').preload(:thumbnail) }, as: :context, inverse_of: :context, class_name: 'Attachment'
@ -2484,7 +2483,13 @@ class User < ActiveRecord::Base
end
def user_can_edit_name?
active_pseudonym_accounts.any? { |a| a.settings[:users_can_edit_name] != false } || active_pseudonym_accounts.empty?
accounts = pseudonyms.shard(self).active.map(&:account)
return true if accounts.empty?
accounts.any? { |a| a.settings[:users_can_edit_name] != false }
end
def limit_parent_app_web_access?
pseudonyms.shard(self).active.map(&:account).any? { |a| a.limit_parent_app_web_access? }
end
def sections_for_course(course)

View File

@ -508,6 +508,10 @@
<%= settings.check_box :enable_profiles, :checked => @account.settings[:enable_profiles] %>
<%= settings.label :enable_profiles, :en => "Enable Profiles" %>
</div>
<div>
<%= settings.check_box :limit_parent_app_web_access, :checked => @account.settings[:limit_parent_app_web_access] %>
<%= settings.label :limit_parent_app_web_access, :en => "Limit access to Canvas web from the Canvas Parent app" %>
</div>
<% end %>
<%# end of :manage_site_settings %>

View File

@ -127,7 +127,8 @@ module Api::V1::User
if includes.include?('permissions')
json[:permissions] = {
:can_update_name => user.user_can_edit_name?,
:can_update_avatar => service_enabled?(:avatars) && !user.avatar_locked?
:can_update_avatar => service_enabled?(:avatars) && !user.avatar_locked?,
:limit_parent_app_web_access => user.limit_parent_app_web_access?,
}
end

View File

@ -787,7 +787,11 @@ describe "Users API", type: :request do
'integration_id' => nil,
'login_id' => @other_user.pseudonym.unique_id,
'locale' => nil,
'permissions' => {'can_update_name' => true, 'can_update_avatar' => false},
'permissions' => {
'can_update_name' => true,
'can_update_avatar' => false,
'limit_parent_app_web_access' => false,
},
'email' => @other_user.email
})
end
@ -804,7 +808,11 @@ describe "Users API", type: :request do
'short_name' => @other_user.short_name,
'locale' => nil,
'effective_locale' => 'en',
'permissions' => {'can_update_name' => true, 'can_update_avatar' => false}
'permissions' => {
'can_update_name' => true,
'can_update_avatar' => false,
'limit_parent_app_web_access' => false,
},
})
end
@ -813,12 +821,29 @@ describe "Users API", type: :request do
Account.default.tap { |a| a.settings[:users_can_edit_name] = false }.save
json = api_call(:get, "/api/v1/users/self",
{ :controller => 'users', :action => 'api_show', :id => 'self', :format => 'json' })
expect(json['permissions']).to eq({'can_update_name' => false, 'can_update_avatar' => false})
expect(json['permissions']).to eq({
'can_update_name' => false,
'can_update_avatar' => false,
'limit_parent_app_web_access' => false,
})
Account.default.tap { |a| a.enable_service(:avatars) }.save
json = api_call(:get, "/api/v1/users/self",
{ :controller => 'users', :action => 'api_show', :id => 'self', :format => 'json' })
expect(json['permissions']).to eq({'can_update_name' => false, 'can_update_avatar' => true})
expect(json['permissions']).to eq({
'can_update_name' => false,
'can_update_avatar' => true,
'limit_parent_app_web_access' => false,
})
Account.default.tap { |a| a.settings[:limit_parent_app_web_access] = true }.save
json = api_call(:get, "/api/v1/users/self",
{ :controller => 'users', :action => 'api_show', :id => 'self', :format => 'json' })
expect(json['permissions']).to eq({
'can_update_name' => false,
'can_update_avatar' => true,
'limit_parent_app_web_access' => true,
})
end
it "should retrieve the right avatar permissions" do

View File

@ -278,6 +278,7 @@ describe AccountsController do
:enable_turnitin => true,
:admins_can_change_passwords => true,
:admins_can_view_notifications => true,
:limit_parent_app_web_access => true,
}}}
@account.reload
expect(@account.global_includes?).to be_falsey
@ -285,6 +286,7 @@ describe AccountsController do
expect(@account.enable_turnitin?).to be_falsey
expect(@account.admins_can_change_passwords?).to be_falsey
expect(@account.admins_can_view_notifications?).to be_falsey
expect(@account.limit_parent_app_web_access?).to be_falsey
end
it "should allow site_admin to update certain settings" do
@ -298,6 +300,7 @@ describe AccountsController do
:enable_turnitin => true,
:admins_can_change_passwords => true,
:admins_can_view_notifications => true,
:limit_parent_app_web_access => true,
}}}
@account.reload
expect(@account.global_includes?).to be_truthy
@ -305,6 +308,7 @@ describe AccountsController do
expect(@account.enable_turnitin?).to be_truthy
expect(@account.admins_can_change_passwords?).to be_truthy
expect(@account.admins_can_view_notifications?).to be_truthy
expect(@account.limit_parent_app_web_access?).to be_truthy
end
it 'does not allow anyone to set unexpected settings' do

View File

@ -3192,6 +3192,42 @@ describe User do
end
end
describe "limit_parent_app_web_access?" do
before(:once) do
user_with_pseudonym
@pseudonym.account.settings[:limit_parent_app_web_access] = nil
@pseudonym.account.save!
end
it "does not limit parent app web access by default" do
expect(@user.limit_parent_app_web_access?).to eq false
end
it "does limit if the pseudonym limits this" do
@pseudonym.account.settings[:limit_parent_app_web_access] = true
@pseudonym.account.save!
expect(@user.limit_parent_app_web_access?).to eq true
end
describe "multiple pseudonyms" do
before(:once) do
@other_account = Account.create :name => 'Other Account'
@other_account.settings[:limit_parent_app_web_access] = true
@other_account.save!
user_with_pseudonym(:user => @user, :account => @other_account)
end
it "limits if one pseudonym's account limits this" do
expect(@user.limit_parent_app_web_access?).to eq true
end
it "doesn't limit if only a deleted pseudonym's account limits this" do
@user.pseudonyms.where(account_id: @other_account).first.destroy
expect(@user.limit_parent_app_web_access?).to eq false
end
end
end
describe 'generate_observer_pairing_code' do
before(:once) do
course_with_student