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

add root account trusted referers 

refs: CNVS-16643

Adds a setting to root accounts for trusted referers.  The field is
setup to allow a comma delimited list of hosts to trust for the account.

Test Plan:

  - Edit and save the account trusted referers.
  - Should show only for root accounts.
  - Should format the referers according to the following rules when it
    is saved.
      - If the scheme is https and the port is provided it will strip
        off the port.
      - If the scheme is http and the port is provided it will strip
        off the port.
      - It will remove the path part of the url.

Change-Id: Ie916339162748cf88259ac566036fc5fa2f5d08e
Reviewed-on: https://gerrit.instructure.com/45779
Tested-by: Jenkins <jenkins@instructure.com>
Product-Review: Blake Simkins <bsimkins@instructure.com>
Reviewed-by: Jacob Fugal <jacob@instructure.com>
QA-Review: August Thornton <august@instructure.com>
This commit is contained in:
Nick Cloward 2014-12-15 11:43:45 -07:00 committed by Matt Fairbourn
parent 457138aadd
commit f047e8b64b
6 changed files with 131 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
app/controllers/accounts_controller.rb
View File

@ -462,6 +462,12 @@ class AccountsController < ApplicationController
end
end
if params[:account][:settings] && params[:account][:settings].has_key?(:trusted_referers)
if trusted_referers = params[:account][:settings].delete(:trusted_referers)
@account.trusted_referers = trusted_referers if @account.root_account?
end
end
if sis_id = params[:account].delete(:sis_source_id)
if !@account.root_account? && sis_id != @account.sis_source_id && @account.root_account.grants_right?(@current_user, session, :manage_sis)
if sis_id == ''

27
app/models/account.rb
View File

@ -216,6 +216,7 @@ class Account < ActiveRecord::Base
add_setting :product_name, root_only: true
add_setting :author_email_in_notifications, boolean: true, root_only: true, default: false
add_setting :include_students_in_global_survey, boolean: true, root_only: true, default: true
add_setting :trusted_referers, root_only: true
def settings=(hash)
if hash.is_a?(Hash)
@ -1526,4 +1527,30 @@ class Account < ActiveRecord::Base
end
Bookmarker = BookmarkedCollection::SimpleBookmarker.new(Account, :name, :id)
def format_referer(referer_url)
begin
referer = URI(referer_url || '')
rescue URI::InvalidURIError
return
end
return unless referer.host
referer_with_port = "#{referer.scheme}://#{referer.host}"
referer_with_port += ":#{referer.port}" unless referer.port == (referer.scheme == 'https' ? 443 : 80)
referer_with_port
end
def trusted_referers=(value)
self.settings[:trusted_referers] = unless value.blank?
value.split(',').map { |referer_url| format_referer(referer_url) }.compact.join(',')
end
end
def trusted_referer?(referer_url)
return if !self.settings.has_key?(:trusted_referers) || self.settings[:trusted_referers].empty?
if referer_with_port = format_referer(referer_url)
self.settings[:trusted_referers].split(',').include?(referer_with_port)
end
end
end

11
app/views/accounts/settings.html.erb
View File

@ -146,6 +146,17 @@ TEXT
</tr>
<% end %>
<tr>
<td><%= settings.blabel :trusted_referers, :en => "Trusted HTTP Referers" %></td>
<td>
<%= settings.text_field :trusted_referers,
:value => @account.settings[:trusted_referers],
:class => 'same-width-as-select',
:placeholder => "https://example.edu" %>
<p style="font-size: 0.9em;"><%= t("This is a comma separated list of URL's to trust. Trusting any URL's in this list will bypass the CSRF token when logging in to Canvas.") %></p>
</td>
</tr>
<tr>
<td colspan="2"><%= settings.check_box :prevent_course_renaming_by_teachers, :checked => @account.settings[:prevent_course_renaming_by_teachers] %>
<%= settings.label :prevent_course_renaming_by_teachers, :en => "Don't let teachers rename their courses" %>

35
spec/controllers/accounts_controller_spec.rb
View File

@ -311,9 +311,28 @@ describe AccountsController do
expect(@account.sis_source_id).to be_nil
end
it "should not allow admins to set the trusted_referers on sub accounts" do
account_with_admin_logged_in
@account = @account.sub_accounts.create!
post 'update', :id => @account.id, :account => { :settings => {
:trusted_referers => 'http://example.com'
} }
@account.reload
expect(@account.settings[:trusted_referers]).to be_nil
end
it "should allow admins to set the trusted_referers on root accounts" do
account_with_admin_logged_in
post 'update', :id => @account.id, :account => { :settings => {
:trusted_referers => 'http://example.com'
} }
@account.reload
expect(@account.settings[:trusted_referers]).to eq 'http://example.com'
end
it "should not allow non-site-admins to update certain settings" do
account_with_admin_logged_in
post 'update', :id => @account.id, :account => { :settings => {
post 'update', :id => @account.id, :account => { :settings => {
:global_includes => true,
:enable_profiles => true,
:admins_can_change_passwords => true,
@ -331,7 +350,7 @@ describe AccountsController do
user_session(@user)
@account = Account.create!
Account.site_admin.account_users.create!(user: @user)
post 'update', :id => @account.id, :account => { :settings => {
post 'update', :id => @account.id, :account => { :settings => {
:global_includes => true,
:enable_profiles => true,
:admins_can_change_passwords => true,
@ -378,7 +397,7 @@ describe AccountsController do
before :each do
user_session(@user)
end
context "with :manage_storage_quotas" do
before :once do
role = custom_account_role 'quota-setter', :account => @account
@ -388,7 +407,7 @@ describe AccountsController do
:role => role
@account.account_users.create!(user: @user, role: role)
end
it "should allow setting default quota (mb)" do
post 'update', :id => @account.id, :account => {
:default_storage_quota_mb => 999,
@ -400,7 +419,7 @@ describe AccountsController do
expect(@account.default_user_storage_quota_mb).to eq 99
expect(@account.default_group_storage_quota_mb).to eq 9999
end
it "should allow setting default quota (bytes)" do
post 'update', :id => @account.id, :account => {
:default_storage_quota => 101.megabytes,
@ -408,7 +427,7 @@ describe AccountsController do
@account.reload
expect(@account.default_storage_quota).to eq 101.megabytes
end
it "should allow setting storage quota" do
post 'update', :id => @account.id, :account => {
:storage_quota => 777.megabytes
@ -417,7 +436,7 @@ describe AccountsController do
expect(@account.storage_quota).to eq 777.megabytes
end
end
context "without :manage_storage_quotas" do
before :once do
role = custom_account_role 'quota-loser', :account => @account
@ -425,7 +444,7 @@ describe AccountsController do
:role => role
@account.account_users.create!(user: @user, role: role)
end
it "should disallow setting default quota (mb)" do
post 'update', :id => @account.id, :account => {
:default_storage_quota => 999,

47
spec/models/account_spec.rb
View File

@ -1107,6 +1107,53 @@ describe Account do
account.ensure_defaults
expect(account.lti_guid).to eq '12345'
end
end
it 'should format a referer url' do
account = Account.new
expect(account.format_referer(nil)).to be_nil
expect(account.format_referer('')).to be_nil
expect(account.format_referer('not a url')).to be_nil
expect(account.format_referer('http://example.com/')).to eq 'http://example.com'
expect(account.format_referer('http://example.com/index.html')).to eq 'http://example.com'
expect(account.format_referer('http://example.com:80')).to eq 'http://example.com'
expect(account.format_referer('https://example.com:443')).to eq 'https://example.com'
expect(account.format_referer('http://example.com:3000')).to eq 'http://example.com:3000'
end
it 'should format trusted referers when set' do
account = Account.new
account.trusted_referers = 'https://example.com/,http://example.com:80,http://example.com:3000'
expect(account.settings[:trusted_referers]).to eq 'https://example.com,http://example.com,http://example.com:3000'
account.trusted_referers = nil
expect(account.settings[:trusted_referers]).to be_nil
account.trusted_referers = ''
expect(account.settings[:trusted_referers]).to be_nil
end
describe 'trusted_referer?' do
let!(:account) do
account = Account.new
account.settings[:trusted_referers] = 'https://example.com,http://example.com,http://example.com:3000'
account
end
it 'should be true when a referer is trusted' do
expect(account.trusted_referer?('http://example.com')).to be_truthy
expect(account.trusted_referer?('http://example.com:3000')).to be_truthy
expect(account.trusted_referer?('http://example.com:80')).to be_truthy
expect(account.trusted_referer?('https://example.com:443')).to be_truthy
end
it 'should be false when a referer is not provided' do
expect(account.trusted_referer?(nil)).to be_falsey
expect(account.trusted_referer?('')).to be_falsey
end
it 'should be false when a referer is not trusted' do
expect(account.trusted_referer?('https://example.com:5000')).to be_falsey
end
end
end

13
spec/selenium/admin/admin_settings_tab_spec.rb
View File

@ -124,6 +124,19 @@ describe "admin settings tab" do
it "should click on 'restrict students from viewing courses before start date'" do
check_box_verifier("#account_settings_restrict_student_future_view", :restrict_student_future_view)
end
it "should set trusted referers for account" do
trusted_referers = 'https://example.com,http://example.com'
set_value f("#account_settings_trusted_referers"), trusted_referers
click_submit
expect(Account.default[:settings][:trusted_referers]).to eq trusted_referers
expect(f("#account_settings_trusted_referers").attribute('value')).to eq trusted_referers
set_value f("#account_settings_trusted_referers"), ''
click_submit
expect(Account.default[:settings][:trusted_referers]).to be_nil
expect(f("#account_settings_trusted_referers").attribute('value')).to eq ''
end
end
context "global includes" do