diff options
26 files changed, 432 insertions, 37 deletions
diff --git a/app/controllers/controller_extension/authentication.rb b/app/controllers/controller_extension/authentication.rb index e2b24f0..63b9e5f 100644 --- a/app/controllers/controller_extension/authentication.rb +++ b/app/controllers/controller_extension/authentication.rb @@ -34,6 +34,12 @@ module ControllerExtension::Authentication access_denied unless admin? end + def require_monitor + unless current_user.is_monitor? || current_user.is_admin? + access_denied + end + end + def authentication_errors return unless attempted_login? errors = get_warden_errors diff --git a/app/controllers/controller_extension/errors.rb b/app/controllers/controller_extension/errors.rb index 8f8edde..3d919b0 100644 --- a/app/controllers/controller_extension/errors.rb +++ b/app/controllers/controller_extension/errors.rb @@ -4,21 +4,25 @@ module ControllerExtension::Errors protected def access_denied - respond_to_error :not_authorized, :forbidden, home_url + render_error :not_authorized, :forbidden, home_url end + alias_method :render_access_denied, :access_denied 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 + alias_method :render_login_required, :login_required - 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 + alias_method :render_not_found, :not_found + 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/controllers/controller_extension/token_authentication.rb b/app/controllers/controller_extension/token_authentication.rb index 4ad1977..c41d61b 100644 --- a/app/controllers/controller_extension/token_authentication.rb +++ b/app/controllers/controller_extension/token_authentication.rb @@ -5,7 +5,7 @@ module ControllerExtension::TokenAuthentication def token @token ||= authenticate_with_http_token do |token, options| - Token.find_by_token(token) + Token.find_by_token(token) || ApiToken.find_by_token(token, request.headers['REMOTE_ADDR']) end end diff --git a/app/controllers/v1/identities_controller.rb b/app/controllers/v1/identities_controller.rb new file mode 100644 index 0000000..4efd1f5 --- /dev/null +++ b/app/controllers/v1/identities_controller.rb @@ -0,0 +1,16 @@ +module V1 + class IdentitiesController < ApiController + before_filter :token_authenticate + before_filter :require_monitor + + def show + @identity = Identity.find_by_address(params[:id]) + if @identity + respond_with @identity + else + render_not_found + end + end + + end +end diff --git a/app/controllers/v1/users_controller.rb b/app/controllers/v1/users_controller.rb index 2e840d9..8296eb0 100644 --- a/app/controllers/v1/users_controller.rb +++ b/app/controllers/v1/users_controller.rb @@ -2,10 +2,12 @@ module V1 class UsersController < ApiController include ControllerExtension::FetchUser + # allow optional access to this controller using API auth tokens: + before_filter :token_authenticate + before_filter :fetch_user, :only => [:update, :destroy] - before_filter :require_admin, :only => [:index] + before_filter :require_monitor, :only => [:index, :show] before_filter :require_login, :only => [:index, :update, :destroy] - before_filter :require_registration_allowed, only: :create respond_to :json @@ -19,9 +21,27 @@ module V1 end end + def show + if params[:login] + @user = User.find_by_login(params[:login]) + elsif params[:id] + @user = User.find(params[:id]) + end + if @user + respond_with @user + else + not_found + end + end + def create - @user = Account.create(params[:user]) - respond_with @user # return ID instead? + if current_user.is_monitor? + create_test_account + elsif APP_CONFIG[:allow_registration] + create_account + else + head :forbidden + end end def update @@ -30,19 +50,34 @@ module V1 end def destroy - @user.account.destroy(params[:identities] == "destroy") + destroy_identity = current_user.is_monitor? || params[:identities] == "destroy" + @user.account.destroy(destroy_identity) if @user == current_user logout end render :json => {'success' => 'user deleted'} end - protected + private + + # tester auth can only create test users. + def create_test_account + if User::is_test?(params[:user][:login]) + @user = Account.create(params[:user]) + respond_with @user + else + head :forbidden + end + end - def require_registration_allowed - unless APP_CONFIG[:allow_registration] + def create_account + if APP_CONFIG[:allow_registration] + @user = Account.create(params[:user]) + respond_with @user # return ID instead? + else head :forbidden end end + 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 5745316..79a5f32 100644 --- a/app/models/anonymous_user.rb +++ b/app/models/anonymous_user.rb @@ -9,6 +9,10 @@ class AnonymousUser < Object false end + def is_monitor? + false + end + def id nil end diff --git a/app/models/api_token.rb b/app/models/api_token.rb new file mode 100644 index 0000000..5c7923d --- /dev/null +++ b/app/models/api_token.rb @@ -0,0 +1,89 @@ +# +# Works like a regular authentication Token, but is configured in the conf file for +# use by admins or testing. +# +# This is not actually a model, but it used in the place of the normal Token model +# when appropriate +# + +require 'digest/sha2' + +class ApiToken + + # + # Searches static config to see if there is a matching api token string. + # Return an ApiToken if successful, or nil otherwise. + # + def self.find_by_token(token, ip_address=nil) + if APP_CONFIG["api_tokens"].nil? || APP_CONFIG["api_tokens"].empty? + # no api auth tokens are configured + return nil + elsif ip_address && !ip_allowed?(ip_address) + return nil + elsif !token.is_a?(String) || token.size < 24 + # don't allow obviously invalid token strings + return nil + else + token_digest = Digest::SHA512.hexdigest(token) + username = self.static_auth_tokens[token_digest] + if username + if username == "monitor" + return ApiMonitorToken.new + elsif username == "admin" + # not yet supported + return nil + end + else + return nil + end + end + end + + private + + # + # A static hash to represent the configured api auth tokens, in the form: + # + # { + # "<sha521 of token>" => "<username>" + # } + # + # SHA512 is used here in order to prevent an attacker from discovering + # the value for an auth token by measuring the string comparison time. + # + def self.static_auth_tokens + @static_auth_tokens ||= APP_CONFIG["api_tokens"].inject({}) {|hsh, entry| + if ["monitor", "admin"].include?(entry[0]) + hsh[Digest::SHA512.hexdigest(entry[1])] = entry[0] + end + hsh + }.freeze + end + + def self.ip_allowed?(ip) + ip == "0.0.0.0" || + ip == "127.0.0.1" || ( + APP_CONFIG["api_tokens"] && + APP_CONFIG["api_tokens"]["allowed_ips"].is_a?(Array) && + APP_CONFIG["api_tokens"]["allowed_ips"].include?(ip) + ) + end + +end + +class ApiAdminToken < ApiToken + # not yet supported + #def authenticate + # AdminUser.new + #end +end + +# +# These tokens used by the platform to run regular monitor tests +# of a production infrastructure. +# +class ApiMonitorToken < ApiToken + def authenticate + ApiMonitorUser.new + end +end diff --git a/app/models/api_user.rb b/app/models/api_user.rb new file mode 100644 index 0000000..2efe1cb --- /dev/null +++ b/app/models/api_user.rb @@ -0,0 +1,23 @@ + +class ApiUser < AnonymousUser +end + +# +# A user that has limited admin access, to be used +# for running monitor tests against a live production +# installation. +# +class ApiMonitorUser < ApiUser + def is_monitor? + true + end +end + +# +# Not yet supported: +# +#class ApiAdminUser < ApiUser +# def is_admin? +# true +# end +#end
\ No newline at end of file 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 61793be..51e9279 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -68,8 +68,10 @@ class User < CouchRest::Model::Base def to_json(options={}) { - :login => login, - :ok => valid? + :login => self.login, + :ok => self.valid?, + :id => self.id, + :enabled => self.enabled? }.to_json(options) end @@ -107,6 +109,10 @@ class User < CouchRest::Model::Base false end + def is_monitor? + false + end + def most_recent_tickets(count=3) Ticket.for_user(self).limit(count).all #defaults to having most recent updated first end diff --git a/app/views/errors/not_found.json b/app/views/errors/not_found.json new file mode 100644 index 0000000..2fcc629 --- /dev/null +++ b/app/views/errors/not_found.json @@ -0,0 +1,4 @@ +{ + "error": "not_found", + "message": "Not Found" +}
\ No newline at end of file diff --git a/app/views/errors/server_error.json b/app/views/errors/server_error.json new file mode 100644 index 0000000..d9a1a86 --- /dev/null +++ b/app/views/errors/server_error.json @@ -0,0 +1,4 @@ +{ + "error": "server_error", + "message": "Server Error" +}
\ No newline at end of file diff --git a/config/defaults.yml b/config/defaults.yml index 906b446..844adaa 100644 --- a/config/defaults.yml +++ b/config/defaults.yml @@ -117,6 +117,9 @@ development: <<: *common <<: *service_levels admins: [blue, red, staff] + api_tokens: + monitor: nil + admin: nil domain: example.org secret_token: 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx' reraise_errors: true @@ -128,6 +131,11 @@ test: <<: *common <<: *service_levels admins: [admin, admin2] + api_tokens: + monitor: "212da28a59dcaca487365309dc93aa09" + admin: nil + allowed_ips: + - 0.0.0.0 domain: test.me secret_token: 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx' reraise_errors: true @@ -142,6 +150,9 @@ production: <<: *cert_options <<: *common admins: [] + api_tokens: + monitor: nil + admin: nil domain: example.net engines: - support diff --git a/config/routes.rb b/config/routes.rb index da6edce..e370aa4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -26,16 +26,18 @@ LeapWeb::Application.routes.draw do namespace "api", { module: "v1", path: "/1/", - defaults: {format: 'json'} } do - resources :sessions, :only => [:new, :create, :update], + defaults: {format: 'json'}, :constraints => { :id => /[^\/]+(?=\.json\z)|[^\/]+/ } + } do + resources :sessions, :only => [:new, :create, :update] delete "logout" => "sessions#destroy", :as => "logout" - resources :users, :only => [:create, :update, :destroy, :index] + resources :users, :only => [:create, :update, :destroy, :index, :show] resources :messages, :only => [:index, :update] resource :cert, :only => [:show, :create] resource :smtp_cert, :only => [:create] resource :service, :only => [:show] resources :configs, :only => [:index, :show] + resources :identities, :only => [:show] end scope "(:locale)", :locale => CommonLanguages.match_available do 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/functional/token_auth_test.rb b/test/functional/token_auth_test.rb new file mode 100644 index 0000000..53d5fb3 --- /dev/null +++ b/test/functional/token_auth_test.rb @@ -0,0 +1,40 @@ +# +# tests for authenticating an admin or monitor user +# via static configured tokens. +# + +require_relative '../test_helper' + +class TokenAuthTest < ActionController::TestCase + tests V1::ConfigsController + + def test_login_via_api_token + with_config(:allow_anonymous_certs => false) do + monitor_auth do + get :index + assert assigns(:token), 'should have authenticated via api token' + assert assigns(:token).is_a? ApiToken + assert @controller.send(:current_user).is_a? ApiMonitorUser + end + end + end + + def test_fail_api_auth_when_ip_not_allowed + with_config(:allow_anonymous_certs => false) do + allowed = "99.99.99.99" + new_config = {api_tokens: APP_CONFIG["api_tokens"].merge(allowed_ips: [allowed])} + with_config(new_config) do + monitor_auth do + request.env['REMOTE_ADDR'] = "1.1.1.1" + get :index + assert_nil assigns(:token), "should not be able to auth with api token when ip restriction doesn't allow it" + request.env['REMOTE_ADDR'] = allowed + get :index + assert assigns(:token), "should have authenticated via api token" + end + end + end + end + +end + diff --git a/test/functional/v1/identities_controller_test.rb b/test/functional/v1/identities_controller_test.rb new file mode 100644 index 0000000..6410c44 --- /dev/null +++ b/test/functional/v1/identities_controller_test.rb @@ -0,0 +1,24 @@ +require_relative '../../test_helper' + +class V1::IdentitiesControllerTest < ActionController::TestCase + + test "api monitor can fetch identity" do + monitor_auth do + identity = FactoryGirl.create :identity + get :show, :id => identity.address, :format => 'json' + assert_response :success + assert_equal identity, assigns(:identity) + + get :show, :id => "blahblahblah", :format => 'json' + assert_response :not_found + end + end + + + test "anonymous cannot fetch identity" do + identity = FactoryGirl.create :identity + get :show, :id => identity.address, :format => 'json' + assert_response :forbidden + end + +end diff --git a/test/functional/v1/users_controller_test.rb b/test/functional/v1/users_controller_test.rb index ffe2484..7afbb02 100644 --- a/test/functional/v1/users_controller_test.rb +++ b/test/functional/v1/users_controller_test.rb @@ -1,4 +1,4 @@ -require 'test_helper' +require_relative '../../test_helper' class V1::UsersControllerTest < ActionController::TestCase @@ -81,4 +81,54 @@ class V1::UsersControllerTest < ActionController::TestCase end end + test "admin can show user" do + user = FactoryGirl.create :user + login :is_admin? => true + get :show, :id => 0, :login => user.login, :format => :json + assert_response :success + assert_json_response user + get :show, :id => user.id, :format => :json + assert_response :success + assert_json_response user + get :show, :id => "0", :format => :json + assert_response :not_found + end + + test "normal users cannot show user" do + user = find_record :user + login + get :show, :id => 0, :login => user.login, :format => :json + assert_access_denied + end + + test "api monitor auth can create and destroy test users" do + with_config(allow_registration: false) do + monitor_auth do + user_attribs = record_attributes_for :test_user + post :create, :user => user_attribs, :format => :json + assert_response :success + delete :destroy, :id => assigns(:user).id, :format => :json + assert_response :success + end + end + end + + test "api monitor auth cannot create normal users" do + monitor_auth do + user_attribs = record_attributes_for :user + post :create, :user => user_attribs, :format => :json + assert_response :forbidden + end + end + + test "api monitor auth cannot delete normal users" do + post :create, :user => record_attributes_for(:user), :format => :json + assert_response :success + normal_user_id = assigns(:user).id + monitor_auth do + delete :destroy, :id => normal_user_id, :format => :json + assert_response :forbidden + end + end + end diff --git a/test/integration/api/signup_test.rb b/test/integration/api/signup_test.rb index 236c547..7216496 100644 --- a/test/integration/api/signup_test.rb +++ b/test/integration/api/signup_test.rb @@ -1,4 +1,4 @@ -require 'test_helper' +require_relative '../../test_helper' require_relative 'srp_test' class SignupTest < SrpTest @@ -8,7 +8,7 @@ class SignupTest < SrpTest end test "signup response" do - assert_json_response :login => @login, :ok => true + assert_json_response :login => @login, :ok => true, :id => @user.id, :enabled => true assert last_response.successful? 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/integration/api/token_test.rb b/test/integration/api/token_test.rb index ad3ac22..dafbfb7 100644 --- a/test/integration/api/token_test.rb +++ b/test/integration/api/token_test.rb @@ -1,4 +1,4 @@ -require 'test_helper' +require_relative '../../test_helper' require_relative 'srp_test' class TokenTest < SrpTest @@ -12,4 +12,5 @@ class TokenTest < SrpTest token = server_auth['token'] assert Token.find(Digest::SHA512.hexdigest(token)) end + end diff --git a/test/support/auth_test_helper.rb b/test/support/auth_test_helper.rb index 7af3341..acc6076 100644 --- a/test/support/auth_test_helper.rb +++ b/test/support/auth_test_helper.rb @@ -29,6 +29,21 @@ module AuthTestHelper @token.expects(:destroy) if @token end + # authenticate as the api monitor + def monitor_auth(&block) + token_auth(APP_CONFIG['api_tokens']['monitor'], &block) + end + + # authenticate with a token + def token_auth(token_str) + original = request.env['HTTP_AUTHORIZATION'] + request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Token.encode_credentials(token_str) + if block_given? + yield + request.env['HTTP_AUTHORIZATION'] = original + end + end + protected def header_for_token_auth diff --git a/test/unit/api_token_test.rb b/test/unit/api_token_test.rb new file mode 100644 index 0000000..266a370 --- /dev/null +++ b/test/unit/api_token_test.rb @@ -0,0 +1,28 @@ +require_relative '../test_helper' + +class ApiTokenTest < ActiveSupport::TestCase + + setup do + end + + test "api token only authenticates ApiUser" do + 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_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 + assert_nil ApiToken.find_by_token("not a token") + with_config({"api_tokens" => {"test" => ""}}) do + assert_equal "", APP_CONFIG['api_tokens']['test'] + assert_nil ApiToken.find_by_token("") + end + end + +end
\ No newline at end of file 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 |