Merge pull request #798 from warpc/796-change_api_defender

[refs #796] add api limit for user

Gemfile

@@ -87,4 +87,5 @@ group :test do
   gem 'factory_girl_rails', '~> 4.0.0'
   gem 'rr',                 '~> 1.0.4'
   gem 'shoulda'
+  gem 'mock_redis',         '0.6.2'
 end

Gemfile.lock

@@ -172,6 +172,7 @@ GEM
     meta-tags (1.2.6)
       actionpack
     mime-types (1.19)
+    mock_redis (0.6.2)
     multi_json (1.3.6)
     mustache (0.99.4)
     net-scp (1.0.4)
@@ -386,6 +387,7 @@ DEPENDENCIES
   jquery-rails (~> 2.0.2)
   mailcatcher
   meta-tags (~> 1.2.5)
+  mock_redis (= 0.6.2)
   newrelic_rpm (~> 3.4.1)
   omniauth (~> 1.1.0)
   omniauth-openid (~> 1.0.1)

app/models/user.rb

@@ -87,7 +87,9 @@ class User < Avatar
     def find_for_database_authentication(warden_conditions)
       conditions = warden_conditions.dup
       login = conditions.delete(:login)
-      where(conditions).where(["lower(uname) = :value OR lower(email) = :value", { :value => login.downcase }]).first
+      where(conditions)
+      .where(["lower(uname) = :value OR lower(email) = :value OR authentication_token = :orig_value",
+             { :value => login.downcase, :orig_value => login }]).first
     end
 
     def new_with_session(params, session)
@@ -103,6 +105,11 @@ class User < Avatar
         end
       end
     end
+
+    def auth_by_token_or_login_pass(user, pass)
+      u = User.find_for_database_authentication(:login => user)
+      u if u && !u.access_locked? && (u.authentication_token == user || u.valid_password?(pass))
+    end
   end
 
   # def update_with_password(params={})

lib/api_defender.rb

@@ -9,15 +9,13 @@ class ApiDefender < Rack::Throttle::Hourly
     options = {
       :cache => Redis.new(:thread_safe => true),
       :key_prefix => :throttle,
-      
+      :max => 500 # only 500 request per hour
-      # only 500 request per hour
-      :max => 500
     }
     @app, @options = app, options
   end
 
-  # this method checks if request needs throttling. 
+  # this method checks if request needs throttling.
-  # If so, it increases usage counter and compare it with maximum 
+  # If so, it increases usage counter and compare it with maximum
   # allowed API calls. Returns true if a request can be handled.
   def allowed?(request)
      need_defense?(request) ? cache_incr(request) <= max_per_window : true
@@ -30,8 +28,9 @@ class ApiDefender < Rack::Throttle::Hourly
     # requests remaining does they have
     if need_defense?(request)
       heders['X-RateLimit-Limit']     = max_per_window.to_s
-      heders['X-RateLimit-Remaining'] = ([0, max_per_window - (cache_get(cache_key(request)).to_i rescue 1)].max).to_s
+      heders['X-RateLimit-Remaining'] = ([0, max_per_window - (cache_get(choice_key(request)).to_i rescue 1)].max).to_s
     end
+    @is_authorized = @user = nil
     [status, heders, body]
   end
 
@@ -40,14 +39,34 @@ class ApiDefender < Rack::Throttle::Hourly
     key = cache_key(request)
     count = cache.incr(key)
     cache.expire(key, 1.day) if count == 1
+
+    if @user
+      count = cache.incr(choice_key(request))
+      cache.expire(key, 1.day) if count == 1
+    end
     count
   end
 
   protected
 
-    # only API calls should be throttled
+  # only API calls should be throttled
-    def need_defense?(request)
+  def need_defense?(request)
-      request.env['PATH_INFO'] =~ /^\/api\/v1\//
+    request.env['PATH_INFO'] =~ /^\/api\/v1\// && !system_user?(request)
-    end
+  end
 
-end
+  def authorized?(request)
+    return @is_authorized if @is_authorized
+    auth = Rack::Auth::Basic::Request.new(request.env)
+    @user = User.auth_by_token_or_login_pass(*auth.credentials) if auth.provided? and auth.basic?
+    @is_authorized = true # cache
+  end
+
+  def choice_key request
+    return cache_key(request) unless @user
+    [@options[:key_prefix], @user.uname, Time.now.strftime('%Y-%m-%dT%H')].join(':')
+  end
+
+  def system_user? request
+    authorized?(request) && %w(rosa_system iso_worker_1).include?(@user.try :uname)
+  end
+end

lib/plugins/grack/auth.rb

@@ -12,9 +12,7 @@ module Grack
         return render_not_found if project.blank?
 
         return ::Rack::Auth::Basic.new(@app) do |u, p|
-          user = (User.where(:authentication_token => u).first ||
+          user = User.auth_by_token_or_login_pass(u, p) and
-                      User.find_for_database_authentication(:login => u)) and
-          !user.access_locked? and (user.authentication_token == u or user.valid_password?(p)) and
           ability = ::Ability.new(user) and ability.can?(action, project) # project.members.include?(user)
         end.call(env) unless project.public? and read? # need auth
       end

spec/integration/api_defender_spec.rb

@@ -0,0 +1,122 @@
+require 'spec_helper'
+
+describe ApiDefender do
+  def get_basic_auth user = @user, by_token = false, by_email = false
+    u,pass = if by_token
+               [user.authentication_token, '']
+             elsif by_email
+               [user.email, @password]
+             else
+               [user.uname, @password]
+             end
+    ActionController::HttpAuthentication::Basic.encode_credentials u, pass
+  end
+
+  def get_request auth_user = nil, by_token = false, by_email = false
+    auth = auth_user ? {'HTTP_AUTHORIZATION' => get_basic_auth(auth_user, by_token, by_email)} : {}
+    get "/api/v1/users/#{@user.id}.json", {}, auth
+  end
+
+  def get_request2 auth_user = nil, by_token = false, by_email = false
+    auth_user = FactoryGirl.create(:user) if !auth_user && APP_CONFIG['anonymous_access'] == false
+    get_request auth_user, by_token, by_email
+  end
+
+  before do
+    stub_symlink_methods && stub_redis
+    @redis = Redis.new
+    @password = '123456'
+    @rate_limit = 3 # dont forget change in max_per_window
+
+    ApiDefender.class_eval("def cache; Redis.new; end; def max_per_window; return #{@rate_limit}; end;")
+  end
+
+  before(:each) do
+    @user = FactoryGirl.create :user, :password => @password
+    @system_user = FactoryGirl.create :user, :uname => 'rosa_system'
+  end
+
+  if APP_CONFIG['anonymous_access'] == true
+    context 'for anonymous user' do
+      it "should return the total limit" do
+        get_request
+        response.headers['X-RateLimit-Limit'].should == @rate_limit.to_s
+      end
+
+      it "should return the correct limit usage for anonymous user" do
+        get_request
+        response.headers['X-RateLimit-Remaining'].should == (@rate_limit-1).to_s
+      end
+
+      it "should return the correct limit usage for anonymous user after authenticated access" do
+        get_request @user
+        get_request
+        response.headers['X-RateLimit-Remaining'].should == (@rate_limit-2).to_s
+      end
+
+      it "should forbidden anonymous user after exceeding limit rate" do
+        (@rate_limit+1).times {get_request}
+        response.status.should == 403
+      end
+    end
+  else
+    it "should forbidden anonymous access" do
+      get_request
+      response.status.should == 401
+    end
+  end
+
+  context 'for user' do
+    it "should return the correct limit usage for auth user" do
+      get_request @user
+      response.headers['X-RateLimit-Remaining'].should == (@rate_limit-1).to_s
+    end
+
+    it "should allow auth by uname and password" do
+      (@rate_limit+1).times {get_request2}
+      get_request @user
+      response.headers['X-RateLimit-Remaining'].should == (@rate_limit-1).to_s
+    end
+
+    it "should allow auth by email and password" do
+      (@rate_limit+1).times {get_request2}
+      get_request @user, false, true
+      response.headers['X-RateLimit-Remaining'].should == (@rate_limit-1).to_s
+    end
+
+    it "should allow auth by token" do
+      (@rate_limit+1).times {get_request2}
+      get_request @user, true
+      response.headers['X-RateLimit-Remaining'].should == (@rate_limit-1).to_s
+    end
+
+    it "should return the correct limit usage for auth user after other user" do
+      get_request2
+      get_request @user
+      response.headers['X-RateLimit-Remaining'].should == (@rate_limit-1).to_s
+    end
+
+    it "should forbidden user after exceeding limit rate" do
+      (@rate_limit+1).times {get_request @user}
+      response.status.should == 403
+    end
+
+    it "should not forbidden user after exceeding limit rate of the other user" do
+      (@rate_limit+1).times {get_request2}
+      get_request @user
+      response.status.should == 200
+    end
+  end
+
+  context 'for system user' do
+    it "should not return the limit usage for system user" do
+      get_request @system_user, true
+      response.headers['X-RateLimit-Limit'].should_not == @rate_limit.to_s
+    end
+
+    it "should not forbidden system user" do
+      (@rate_limit+1).times {get_request @system_user, true}
+      response.status.should == 200
+    end
+  end
+end

spec/spec_helper.rb

@@ -58,6 +58,11 @@ Resque.inline = true
 %x(rm -Rf #{APP_CONFIG['git_path']})
 %x(mkdir -p #{APP_CONFIG['git_path']})
 
+def stub_redis
+  redis_instance = MockRedis.new
+  stub(Redis).new { redis_instance }
+end
+
 def fill_project project
   %x(mkdir -p #{project.path} && cp -Rf #{Rails.root}/spec/tests.git/* #{project.path}) # maybe FIXME ?
 end