diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 89656ce44cf..1e186bfff9f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1532,7 +1532,7 @@ class ApplicationController < ActionController::Base end def in_app? - @pseudonym_session && !@pseudonym_session.used_basic_auth? + @pseudonym_session end def json_as_text? diff --git a/app/models/pseudonym_session.rb b/app/models/pseudonym_session.rb index 62991acbe85..c1ea6d6fe1e 100644 --- a/app/models/pseudonym_session.rb +++ b/app/models/pseudonym_session.rb @@ -22,27 +22,10 @@ class PseudonymSession < Authlogic::Session::Base login_field :unique_id find_by_login_method :custom_find_by_unique_id remember_me_for 2.weeks + allow_http_basic_auth false attr_accessor :remote_ip, :too_many_attempts - # we need to know if the session came from http basic auth, so we override - # authlogic's method here to add a flag that we can check - def persist_by_http_auth - controller.authenticate_with_http_basic do |login, password| - if !login.blank? && !password.blank? - send("#{login_field}=", login) - send("#{password_field}=", password) - @valid_basic_auth = valid? - return @valid_basic_auth - end - end - - false - end - def used_basic_auth? - @valid_basic_auth - end - # In authlogic 3.2.0, it tries to parse the last part of the cookie (delimited by '::') # as a timestamp to verify whether the cookie is stale. # This conflicts with the uuid that we use instead in that place, diff --git a/lib/authentication_methods.rb b/lib/authentication_methods.rb index 214124236ba..90f509d6189 100644 --- a/lib/authentication_methods.rb +++ b/lib/authentication_methods.rb @@ -146,14 +146,7 @@ module AuthenticationMethods @current_user = @current_pseudonym && @current_pseudonym.user if api_request? - # only allow api_key to be used if basic auth was sent, not if they're - # just using an app session - # this basic auth support is deprecated and marked for removal in 2012 - if @pseudonym_session.try(:used_basic_auth?) && params[:api_key].present? - Shard.birth.activate { @developer_key = DeveloperKey.where(api_key: params[:api_key]).first } - end - @developer_key || - request.get? || + request.get? || !allow_forgery_protection || CanvasBreachMitigation::MaskingSecrets.valid_authenticity_token?(session, cookies, form_authenticity_param) || CanvasBreachMitigation::MaskingSecrets.valid_authenticity_token?(session, cookies, request.headers['X-CSRF-Token']) || diff --git a/spec/apis/auth_spec.rb b/spec/apis/auth_spec.rb index 610423e203d..7b9ba9eb5fe 100644 --- a/spec/apis/auth_spec.rb +++ b/spec/apis/auth_spec.rb @@ -36,31 +36,6 @@ describe "API Authentication", type: :request do consider_all_requests_local(true) end - context "sharding" do - specs_require_sharding - - it "should use developer key + basic auth access on the default shard from a different shard" do - @shard1.activate do - @account = Account.create! - # this will continue to be supported until we notify api users and explicitly phase it out - user_with_pseudonym(:active_user => true, :username => 'test1@example.com', :password => 'test123', :account => @account) - course_with_teacher(:user => @user, :account => @account) - end - LoadAccount.stubs(:default_domain_root_account).returns(@account) - - get "/api/v1/courses.json" - expect(response.response_code).to eq 401 - get "/api/v1/courses.json?api_key=#{@key.api_key}" - expect(response.response_code).to eq 401 - get "/api/v1/courses.json?api_key=#{@key.api_key}", {}, { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials('test1@example.com', 'failboat') } - expect(response.response_code).to eq 401 - get "/api/v1/courses.json", {}, { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials('test1@example.com', 'test123') } - expect(response).to be_success - get "/api/v1/courses.json?api_key=#{@key.api_key}", {}, { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials('test1@example.com', 'test123') } - expect(response).to be_success - end - end - if Canvas.redis_enabled? # eventually we're going to have to just require redis to run the specs it "should require a valid client id" do get "/login/oauth2/auth", :response_type => 'code', :redirect_uri => 'urn:ietf:wg:oauth:2.0:oob' @@ -138,39 +113,10 @@ describe "API Authentication", type: :request do course_with_teacher(:user => @user) end - it "should allow basic auth" do - get "/api/v1/courses.json?api_key=#{@key.api_key}", {}, - { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials('test1@example.com', 'failboat') } - expect(response.response_code).to eq 401 - get "/api/v1/courses.json", {}, - { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials('test1@example.com', 'test123') } - expect(response).to be_success - end - - it "should allow basic auth with api key" do + it "should not allow basic auth with api key" do get "/api/v1/courses.json?api_key=#{@key.api_key}", {}, { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials('test1@example.com', 'test123') } - expect(response).to be_success - end - - it "should fail without api key" do - post "/api/v1/courses/#{@course.id}/assignments.json", - { :assignment => { :name => 'test assignment', :points_possible => '5.3', :grading_type => 'points' } } expect(response.response_code).to eq 401 - post "/api/v1/courses/#{@course.id}/assignments.json", - { :assignment => { :name => 'test assignment', :points_possible => '5.3', :grading_type => 'points' } }, - { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials('test1@example.com', 'test123') } - expect(response.response_code).to eq 401 - end - - it "should allow post with api key and basic auth" do - post "/api/v1/courses/#{@course.id}/assignments.json?api_key=#{@key.api_key}", - { :assignment => { :name => 'test assignment', :points_possible => '5.3', :grading_type => 'points' } }, - { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials('test1@example.com', 'test123') } - expect(response).to be_success - expect(@course.assignments.count).to eq 1 - expect(@course.assignments.first.title).to eq 'test assignment' - expect(@course.assignments.first.points_possible).to eq 5.3 end end @@ -803,16 +749,6 @@ describe "API Authentication", type: :request do expect(json['id']).to eq @user.id end - it "should not prepend the CSRF protection to HTTP Basic API requests" do - user_with_pseudonym(:active_user => true, :username => 'test1@example.com', :password => 'test123') - get "/api/v1/users/self/profile", {}, { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials('test1@example.com', 'test123') } - expect(response).to be_success - raw_json = response.body - expect(raw_json).not_to match(%r{^while\(1\);}) - json = JSON.parse(raw_json) - expect(json['id']).to eq @user.id - end - it "should prepend the CSRF protection for API endpoints, when session auth is used" do user_with_pseudonym(:active_user => true, :username => 'test1@example.com', :password => 'test123') Account.any_instance.stubs(:trusted_referer?).returns(true) diff --git a/spec/selenium/common.rb b/spec/selenium/common.rb index fe7297b61d0..226e335b10c 100644 --- a/spec/selenium/common.rb +++ b/spec/selenium/common.rb @@ -486,7 +486,6 @@ shared_examples_for "all selenium tests" do else PseudonymSession.any_instance.stubs(:session_credentials).returns([]) PseudonymSession.any_instance.stubs(:record).returns { pseudonym.reload } - PseudonymSession.any_instance.stubs(:used_basic_auth?).returns(false) # PseudonymSession.stubs(:find).returns(@pseudonym_session) end end @@ -505,7 +504,6 @@ shared_examples_for "all selenium tests" do else PseudonymSession.any_instance.unstub :session_credentials PseudonymSession.any_instance.unstub :record - PseudonymSession.any_instance.unstub :used_basic_auth? end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 554a8ac7aa0..559af7b9f75 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -796,7 +796,7 @@ RSpec.configure do |config| pseudonym.stubs(:id).returns(pseudonym.object_id) end - session = stub('PseudonymSession', :record => pseudonym, :session_credentials => nil, :used_basic_auth? => false) + session = stub('PseudonymSession', :record => pseudonym, :session_credentials => nil) PseudonymSession.stubs(:find).returns(session) end