From 67b5aa4198e0f6ab2cd29767aedcb4bf5b5dc4d9 Mon Sep 17 00:00:00 2001 From: elijah Date: Mon, 28 Mar 2016 15:52:21 -0700 Subject: api tokens - clarify terms: "monitors" are admins that authenticated via api token, "tmp" users are users that exist only in tmp db, "test" users are either tmp users or users named "test_user_x" --- app/controllers/controller_extension/errors.rb | 11 ++++++----- app/controllers/controller_extension/fetch_user.rb | 20 ++++++++++++++++--- app/models/account.rb | 8 ++++---- app/models/anonymous_user.rb | 2 +- app/models/api_user.rb | 4 ++-- app/models/temporary_user.rb | 23 ++++++++++++++++++---- app/models/user.rb | 2 +- config/defaults.yml | 8 +++++--- test/factories.rb | 4 ++++ test/integration/api/tmp_user_test.rb | 2 +- test/unit/api_token_test.rb | 8 ++++---- 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 -- cgit v1.2.3