summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/controllers/controller_extension/errors.rb11
-rw-r--r--app/controllers/controller_extension/fetch_user.rb20
-rw-r--r--app/models/account.rb8
-rw-r--r--app/models/anonymous_user.rb2
-rw-r--r--app/models/api_user.rb4
-rw-r--r--app/models/temporary_user.rb23
-rw-r--r--app/models/user.rb2
-rw-r--r--config/defaults.yml8
-rw-r--r--test/factories.rb4
-rw-r--r--test/integration/api/tmp_user_test.rb2
-rw-r--r--test/unit/api_token_test.rb8
-rw-r--r--test/unit/tmp_user_test.rb4
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