diff options
| -rw-r--r-- | app/controllers/controller_extension/errors.rb | 11 | ||||
| -rw-r--r-- | app/controllers/controller_extension/fetch_user.rb | 20 | ||||
| -rw-r--r-- | app/models/account.rb | 8 | ||||
| -rw-r--r-- | app/models/anonymous_user.rb | 2 | ||||
| -rw-r--r-- | app/models/api_user.rb | 4 | ||||
| -rw-r--r-- | app/models/temporary_user.rb | 23 | ||||
| -rw-r--r-- | app/models/user.rb | 2 | ||||
| -rw-r--r-- | config/defaults.yml | 8 | ||||
| -rw-r--r-- | test/factories.rb | 4 | ||||
| -rw-r--r-- | test/integration/api/tmp_user_test.rb | 2 | ||||
| -rw-r--r-- | test/unit/api_token_test.rb | 8 | ||||
| -rw-r--r-- | test/unit/tmp_user_test.rb | 4 | 
12 files changed, 66 insertions, 30 deletions
diff --git a/app/controllers/controller_extension/errors.rb b/app/controllers/controller_extension/errors.rb index 8f8edde..2b68955 100644 --- a/app/controllers/controller_extension/errors.rb +++ b/app/controllers/controller_extension/errors.rb @@ -4,21 +4,22 @@ module ControllerExtension::Errors    protected    def access_denied -    respond_to_error :not_authorized, :forbidden, home_url +    render_error :not_authorized, :forbidden, home_url    end    def login_required      # Warden will intercept the 401 response and call      # SessionController#unauthenticated instead. -    respond_to_error :not_authorized_login, :unauthorized, login_url +    render_error :not_authorized_login, :unauthorized, login_url    end -  def not_found -    respond_to_error :not_found, :not_found, home_url +  def not_found(msg=nil, url=nil) +    render_error(msg || :not_found, :not_found, url || home_url)    end +  private -  def respond_to_error(message, status=nil, redirect=nil) +  def render_error(message, status=nil, redirect=nil)      error = message      message = t(message) if message.is_a?(Symbol)      respond_to do |format| diff --git a/app/controllers/controller_extension/fetch_user.rb b/app/controllers/controller_extension/fetch_user.rb index 695d723..97f92fa 100644 --- a/app/controllers/controller_extension/fetch_user.rb +++ b/app/controllers/controller_extension/fetch_user.rb @@ -8,11 +8,25 @@ module ControllerExtension::FetchUser    protected +  # +  # fetch @user from params, but enforce permissions: +  # +  # * admins may fetch any user +  # * monitors may fetch test users +  # * users may fetch themselves +  # +  # these permissions matter, it is what protects +  # users from being updated or deleted by other users. +  #    def fetch_user      @user = User.find(params[:user_id] || params[:id]) -    if !@user && admin? -      redirect_to users_url, :alert => t(:no_such_thing, :thing => 'user') -    elsif !admin? && @user != current_user +    if current_user.is_admin? || current_user.is_monitor? +      if @user.nil? +        not_found(t(:no_such_thing, :thing => 'user'), users_url) +      elsif current_user.is_monitor? +        access_denied unless @user.is_test? +      end +    elsif @user != current_user        access_denied      end    end diff --git a/app/models/account.rb b/app/models/account.rb index abde89d..a85e56c 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -21,7 +21,7 @@ class Account      user = User.new(attrs)      user.save -    if !user.tmp? && user.persisted? +    if !user.is_tmp? && user.persisted?        identity = user.identity        identity.user_id = user.id        identity.save @@ -59,7 +59,7 @@ class Account    def destroy(destroy_identity=false)      return unless @user -    if !@user.tmp? +    if !@user.is_tmp?        if destroy_identity == false          @user.identities.each do |id|            id.orphan! @@ -77,7 +77,7 @@ class Account    # in place, but the user should not be able to send email or    # create new authentication certificates.    def disable -    if @user && !@user.tmp? +    if @user && !@user.is_tmp?        @user.enabled = false        @user.save        @user.identities.each do |id| @@ -119,7 +119,7 @@ class Account    def self.creation_problem?(user, identity)      return true if user.nil? || !user.persisted? || user.errors.any? -    if !user.tmp? +    if !user.is_tmp?        return true if identity.nil? || !identity.persisted? || identity.errors.any?      end      return false diff --git a/app/models/anonymous_user.rb b/app/models/anonymous_user.rb index e1f7a0e..79a5f32 100644 --- a/app/models/anonymous_user.rb +++ b/app/models/anonymous_user.rb @@ -9,7 +9,7 @@ class AnonymousUser < Object      false    end -  def is_test? +  def is_monitor?      false    end diff --git a/app/models/api_user.rb b/app/models/api_user.rb index d80a4d1..2efe1cb 100644 --- a/app/models/api_user.rb +++ b/app/models/api_user.rb @@ -7,8 +7,8 @@ end  # for running monitor tests against a live production  # installation.  # -class ApiTestUser < ApiUser -  def is_test? +class ApiMonitorUser < ApiUser +  def is_monitor?      true    end  end diff --git a/app/models/temporary_user.rb b/app/models/temporary_user.rb index 87800ed..2afae15 100644 --- a/app/models/temporary_user.rb +++ b/app/models/temporary_user.rb @@ -16,7 +16,8 @@ module TemporaryUser    USER_DB     = 'users'    TMP_USER_DB = 'tmp_users' -  TMP_LOGIN   = 'test_user' +  TMP_LOGIN   = 'tmp_user'  # created and deleted frequently +  TEST_LOGIN  = 'test_user' # created, rarely deleted    included do      use_database_method :db_name @@ -26,7 +27,7 @@ module TemporaryUser      # override it.      instance_eval <<-EOS, __FILE__, __LINE__ + 1        def find_by_login(*args) -        if args.grep(/#{TMP_LOGIN}/).any? +        if args.grep(/^#{TMP_LOGIN}/).any?            by_login.database(tmp_database).key(*args).first()          else            by_login.key(*args).first() @@ -60,6 +61,14 @@ module TemporaryUser      def create_tmp_database!        design_doc.sync!(tmp_database.tap{|db|db.create!})      end + +    def is_tmp?(login) +      !login.nil? && login =~ /^#{TMP_LOGIN}/ +    end + +    def is_test?(login) +      !login.nil? && (login =~ /^#{TMP_LOGIN}/ || login =~ /^#{TEST_LOGIN}/) +    end    end    # @@ -71,8 +80,14 @@ module TemporaryUser    end    # returns true if this User instance is stored in tmp db. -  def tmp? -    !login.nil? && login.include?(TMP_LOGIN) +  def is_tmp? +    self.class.is_tmp?(self.login) +  end + +  # returns true if this user is used for testing purposes +  # (either a temporary or long lived) +  def is_test? +    self.class.is_test?(self.login)    end  end diff --git a/app/models/user.rb b/app/models/user.rb index 1aeea1c..1c54f0c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -107,7 +107,7 @@ class User < CouchRest::Model::Base      false    end -  def is_test? +  def is_monitor?      false    end diff --git a/config/defaults.yml b/config/defaults.yml index 84ee4c9..844adaa 100644 --- a/config/defaults.yml +++ b/config/defaults.yml @@ -118,7 +118,7 @@ development:    <<: *service_levels    admins: [blue, red, staff]    api_tokens: -    test: nil +    monitor: nil      admin: nil    domain: example.org    secret_token: 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx' @@ -132,8 +132,10 @@ test:    <<: *service_levels    admins: [admin, admin2]    api_tokens: -    test: "212da28a59dcaca487365309dc93aa09" +    monitor: "212da28a59dcaca487365309dc93aa09"      admin: nil +    allowed_ips: +      - 0.0.0.0    domain: test.me    secret_token: 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'    reraise_errors: true @@ -149,7 +151,7 @@ production:    <<: *common    admins: []    api_tokens: -    test: nil +    monitor: nil      admin: nil    domain: example.net    engines: diff --git a/test/factories.rb b/test/factories.rb index b6e1475..5d49729 100644 --- a/test/factories.rb +++ b/test/factories.rb @@ -26,6 +26,10 @@ FactoryGirl.define do        end      end +    factory :test_user do +      login {"test_user_" + Faker::Internet.user_name + '_' + SecureRandom.hex(4)} +    end +      factory :premium_user do        effective_service_level_code 2      end diff --git a/test/integration/api/tmp_user_test.rb b/test/integration/api/tmp_user_test.rb index 4c1e659..bf5f99d 100644 --- a/test/integration/api/tmp_user_test.rb +++ b/test/integration/api/tmp_user_test.rb @@ -4,7 +4,7 @@ require_relative 'srp_test'  class TmpUserTest < SrpTest    setup do -    register_user('test_user_'+SecureRandom.hex(5)) +    register_user('tmp_user_'+SecureRandom.hex(5))    end    test "login with srp" do diff --git a/test/unit/api_token_test.rb b/test/unit/api_token_test.rb index 55d7507..266a370 100644 --- a/test/unit/api_token_test.rb +++ b/test/unit/api_token_test.rb @@ -6,15 +6,15 @@ class ApiTokenTest < ActiveSupport::TestCase    end    test "api token only authenticates ApiUser" do -    token_string = APP_CONFIG['api_tokens']['test'] -    assert !token_string.nil? +    token_string = APP_CONFIG['api_tokens']['monitor'] +    assert !token_string.nil?, 'monitor token should be configured'      assert !token_string.empty?      token = ApiToken.find_by_token(token_string)      user = token.authenticate      assert user, 'api token should authenticate'      assert user.is_a?(ApiUser), 'api token should return api user' -    assert user.is_test?, 'api test token should return test user' -    assert !user.is_admin?, 'api test token should not return admin user' +    assert user.is_monitor?, 'api monitor token should return monitor user' +    assert !user.is_admin?, 'api monitor token should not return admin user'    end    test "invalid api tokens can't authenticate" do diff --git a/test/unit/tmp_user_test.rb b/test/unit/tmp_user_test.rb index 9494377..1dea5f9 100644 --- a/test/unit/tmp_user_test.rb +++ b/test/unit/tmp_user_test.rb @@ -6,7 +6,7 @@ class TmpUserTest < ActiveSupport::TestCase      InviteCodeValidator.any_instance.stubs(:validate)    end -  test "test_user saved to tmp_users" do +  test "tmp_user saved to tmp_users" do      begin        assert User.ancestors.include?(TemporaryUser) @@ -17,7 +17,7 @@ class TmpUserTest < ActiveSupport::TestCase        end        assert_difference('User.tmp_database.info["doc_count"]') do -        tmp_user = User.create!(:login => 'test_user_'+SecureRandom.hex(5).downcase, +        tmp_user = User.create!(:login => 'tmp_user_'+SecureRandom.hex(5).downcase,            :password_verifier => 'ABCDEF0010101', :password_salt => 'ABCDEF')          assert tmp_user.database.to_s.include?('tmp')        end  | 
