diff --git a/.gitignore b/.gitignore index 154182a3..4686914b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ tmp/ .idea -*.iml \ No newline at end of file +*.iml +script/log/ diff --git a/Gemfile b/Gemfile index 2843f17a..e32414c4 100644 --- a/Gemfile +++ b/Gemfile @@ -2,7 +2,6 @@ source 'https://rubygems.org' gem "rails", "2.3.17" gem "mysql" -gem "authorization", github: "alex-frost/rails-authorization-plugin" gem 'json', '1.7.7' # (CVE-2013-026) Can remove once rails depends on > 1.7.6 gem 'haml', "3.0.25" gem 'googlecharts', "1.6.0" diff --git a/Gemfile.lock b/Gemfile.lock index 35704042..a0ed8fda 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,9 +1,3 @@ -GIT - remote: git://github.com/alex-frost/rails-authorization-plugin.git - revision: 505bd47addf2ef5e3b5d98a7ea8d1352c2aa4ee6 - specs: - authorization (1.0.12) - GEM remote: https://rubygems.org/ specs: @@ -58,7 +52,6 @@ PLATFORMS DEPENDENCIES acts-as-taggable-on (= 2.0.6) annotate - authorization! calendar_date_select (= 1.16.1) debugger googlecharts (= 1.6.0) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 585d5e84..11becc33 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -46,4 +46,15 @@ def date_from_params(params) Date.new params[:year].to_i, params[:month].to_i, params[:day].to_i end + def authorize_admin_or_manager + redirect_unauthrized unless user_is_admin? || user_is_manager? + end + + def authorize_admin + redirect_unauthrized unless user_is_admin? + end + + def redirect_unauthrized + redirect_to({ :controller => 'sessions', :action => 'new' }) + end end diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb index 1a088018..2f05c03e 100644 --- a/app/controllers/notes_controller.rb +++ b/app/controllers/notes_controller.rb @@ -1,6 +1,6 @@ class NotesController < ApplicationController - permit "admin or (manager of :organization)" + before_filter :authorize_admin_or_manager # GET /notes # GET /notes.xml diff --git a/app/controllers/organizations_controller.rb b/app/controllers/organizations_controller.rb index 0cd339f9..792bbd42 100644 --- a/app/controllers/organizations_controller.rb +++ b/app/controllers/organizations_controller.rb @@ -3,9 +3,9 @@ class OrganizationsController < ApplicationController skip_before_filter :login_required, :only => [:index, :show, :new, :create] before_filter :assign_id_param, :resolve_organization_by_id, :except => [ :index, :new, :create ] - permit "admin", :only => [ :destroy ] - permit "admin or (manager of :organization)", :only => [ :show, :edit, :update ] - + before_filter :authorize_admin_or_manager, :only => [ :show, :edit, :update ] + before_filter :authorize_admin, :only => [ :destory ] + # GET /organizations # GET /organizations.xml def index @@ -49,7 +49,8 @@ def create respond_to do |format| if @organization.valid? && @user.valid? && @organization.save && @user.save - @user.has_role 'manager', @organization + role = Role.create( :name => 'manager', :authorizable => @organization ) + @user.roles << role if role and not @user.roles.exists?( role.id ) self.current_user = @user flash[:notice] = 'Organization was successfully created.' format.html { redirect_to @organization } diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 4059cee8..dd011736 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -1,6 +1,6 @@ class PeopleController < ApplicationController - permit "admin or (manager of :organization)" + before_filter :authorize_admin_or_manager # GET /people/1 # GET /people/1.xml diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index ee49ea14..a1053082 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -2,7 +2,7 @@ class ReportsController < ApplicationController - permit "admin or (manager of :organization)" + before_filter :authorize_admin_or_manager def index end diff --git a/app/controllers/services_controller.rb b/app/controllers/services_controller.rb index 69dcde3a..772363dc 100644 --- a/app/controllers/services_controller.rb +++ b/app/controllers/services_controller.rb @@ -1,6 +1,6 @@ class ServicesController < ApplicationController - permit "admin or (manager of :organization)" + before_filter :authorize_admin_or_manager # GET /services # GET /services.xml diff --git a/app/controllers/taggings_controller.rb b/app/controllers/taggings_controller.rb index bd1c67e9..d9157409 100644 --- a/app/controllers/taggings_controller.rb +++ b/app/controllers/taggings_controller.rb @@ -1,10 +1,11 @@ class TaggingsController < ApplicationController - permit "admin or (manager of :organization)" + before_filter :authorize_admin_or_manager def create @person.tag_list << params[:id] @person.save! if request.xhr? + @person.taggings.reload index else redirect_to_person_path diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index a6eb77bc..6004736e 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -1,6 +1,6 @@ class TagsController < ApplicationController - permit "admin or (manager of :organization)" + before_filter :authorize_admin_or_manager def show @tag = ActsAsTaggableOn::Tag.find(params[:id]) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index f2eafd91..e9ef183a 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -3,8 +3,8 @@ class UsersController < ApplicationController before_filter :resolve_user_by_id skip_before_filter :login_from_cookie, :login_required, :only => [:new, :create, :activate, :reset, :forgot] - permit "admin", :only => [:index] - permit "admin or (owner of :user)", :only => [:edit, :update, :destroy] + before_filter :authorize_admin, :only => [:index] + before_filter :authorize_admin_or_manager, :only => [:edit, :update, :destroy] # render new.rhtml def new diff --git a/app/controllers/visits_controller.rb b/app/controllers/visits_controller.rb index 8e867ec4..113d0702 100644 --- a/app/controllers/visits_controller.rb +++ b/app/controllers/visits_controller.rb @@ -1,6 +1,6 @@ class VisitsController < ApplicationController - permit "admin or (manager of :organization)" + before_filter :authorize_admin_or_manager # GET /visits # GET /visits.xml diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index ac1ff377..c679239d 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -64,4 +64,15 @@ def labeled_value(label_value, value) ).render(self, :label_value => label_value, :value => value) end + def user_is_manager? + wrapped_current_user.try(:is_manager_of?, @organization) + end + + def user_is_admin? + wrapped_current_user.try(:is_admin?) + end + + def wrapped_current_user + User.current_user if User.current_user != :false + end end diff --git a/app/helpers/people_helper.rb b/app/helpers/people_helper.rb index e78d908d..2ae6dfa6 100644 --- a/app/helpers/people_helper.rb +++ b/app/helpers/people_helper.rb @@ -5,11 +5,13 @@ def auto_complete_result_with_add_person(entries, field, phrase = nil) return unless entries items = entries.map do |entry| content_tag("li", phrase ? highlight(entry[field], phrase) : h(entry[field]), + :class => 'person', :url => person_path(:organization_key => @organization.key, :id => entry.id)) end items << content_tag("li", content_tag("b", 'Add Person'), + :class => 'add', :url => new_person_path(:organization_key => @organization.key)) - content_tag("ul", items.uniq) + content_tag("ul", items.uniq.join('')) end end diff --git a/app/models/organization.rb b/app/models/organization.rb index 4282a223..773b416f 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -14,6 +14,7 @@ class Organization < ActiveRecord::Base has_many :people, :dependent => :destroy + has_many :accepted_roles, :as => :authorizable, :class_name => 'Role' validates_presence_of :name, :key, :timezone validates_length_of :name, :within => 3..40, :unless => proc { |organization| organization.errors.on :name } @@ -22,8 +23,6 @@ class Organization < ActiveRecord::Base validates_format_of :key, :with => /\A\w+\Z/i, :unless => proc { |organization| organization.errors.on :key } validate :validate_timezone - acts_as_authorizable - def initialize(attributes=nil) super(attributes) self[:timezone] ||= 'Pacific Time (US & Canada)' @@ -71,4 +70,4 @@ def validate_timezone errors.add :timezone if !@timezone_names.include?(self.timezone) end end -end \ No newline at end of file +end diff --git a/app/models/user.rb b/app/models/user.rb index c46bb029..3fa9b6e7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -21,13 +21,12 @@ class User < ActiveRecord::Base belongs_to :organization - + # Authenticated user cattr_accessor :current_user - # Authorization plugin - acts_as_authorized_user - + has_and_belongs_to_many :roles + def accepts_role?(role, user) 'owner' == role && self == user end @@ -45,7 +44,7 @@ def accepts_role?(role, user) validates_email_format_of :email, :check_mx=> true, :unless => proc { |user| user.errors.on :email } validates_uniqueness_of :login, :email, :case_sensitive => false before_save :encrypt_password - before_create :make_activation_code + before_create :make_activation_code # prevents a user from submitting a crafted form that bypasses activation # anything else you want your user to change should be added here. attr_accessible :name, :login, :email, :password, :password_confirmation @@ -90,7 +89,7 @@ def recently_forgot_password? def recently_reset_password? @reset_password - end + end # Authenticates a user by their login name and unencrypted password. Returns the user or nil. def self.authenticate(login, password) @@ -113,7 +112,7 @@ def authenticated?(password) end def remember_token? - remember_token_expires_at && Time.now.utc < remember_token_expires_at + remember_token_expires_at && Time.now.utc < remember_token_expires_at end # These create and unset the fields required for remembering users between browser closes @@ -138,24 +137,37 @@ def forget_me end def organization - @organization ||= self.is_manager_for_what.first + @organization ||= is_manager_for_what + end + + def is_manager_of?(other_organization) + return false if !other_organization || !organization + organization.id == other_organization.id && roles.first.name == 'manager' + end + + def is_admin? + roles.first.name == 'admin' end protected - # before filter + # before filter def encrypt_password return if password.blank? self.salt = Digest::SHA1.hexdigest("--#{Time.now.to_s}--#{login}--") if new_record? self.crypted_password = encrypt(password) end - + def password_required? crypted_password.blank? || !password.blank? || reset_code end - - def make_activation_code + def make_activation_code self.activation_code = Digest::SHA1.hexdigest( Time.now.to_s.split(//).sort_by {rand}.join ) end - + + def is_manager_for_what + role = roles.find_all_by_name("manager").first + role.authorizable if role + end + end diff --git a/app/models/visit.rb b/app/models/visit.rb index 6376a279..974ac006 100644 --- a/app/models/visit.rb +++ b/app/models/visit.rb @@ -52,11 +52,11 @@ def initialize(params={}) :self => %w{arrived_at staff member volunteer note} } def self.csv_header - CSV.generate_line(CSV_FIELDS[:person] + CSV_FIELDS[:self]) + CSV.generate_line(['person_id'] + CSV_FIELDS[:person] + CSV_FIELDS[:self]) end def to_csv - values = person.attributes.values_at(*CSV_FIELDS[:person]) + values = [person.id].concat person.attributes.values_at(*CSV_FIELDS[:person]) values << (arrived_at.nil? ? nil : arrived_at.strftime("%Y-%m-%d %H:%M")) values << staff? values << member? diff --git a/app/views/layouts/application.html.haml b/app/views/layouts/application.html.haml index 725da9f1..8bb0f596 100644 --- a/app/views/layouts/application.html.haml +++ b/app/views/layouts/application.html.haml @@ -37,7 +37,7 @@ = tab_item('Home', organization_path(@organization)) = tab_item('Visits', today_visits_path(:organization_key => @organization.key)) = tab_item('Reports', report_path(:action => 'index', :organization_key => @organization.key)) - -if permit? "admin or (manager of :organization)" + -if user_is_manager? || user_is_admin? =tab_item('Settings', edit_organization_path(@organization)) -else =tab_item('Home', root_path) diff --git a/app/views/organizations/index.html.haml b/app/views/organizations/index.html.haml index 94aa01da..c78f509c 100644 --- a/app/views/organizations/index.html.haml +++ b/app/views/organizations/index.html.haml @@ -65,5 +65,5 @@ %br -if organization.member_count > 9 = "#{organization.member_count} members" - -if permit? 'admin' + -if user_is_admin? %td= link_to 'Remove', organization, :confirm => 'Are you sure?', :method => :delete diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml index 4ac57085..95ef2b70 100644 --- a/app/views/users/show.html.haml +++ b/app/views/users/show.html.haml @@ -4,6 +4,6 @@ =labeled_value 'Email', @user.email =labeled_value 'Login', @user.login =labeled_value 'Activated at', @user.activated_at.nil? ? 'Not yet activated' : datetime_long(@user.activated_at) --if permit?('owner of :user') +-if user_is_manager? =link_to 'Edit', edit_user_path(@user) diff --git a/config/ey.yml b/config/ey.yml new file mode 100644 index 00000000..6fcb5cef --- /dev/null +++ b/config/ey.yml @@ -0,0 +1,175 @@ +# Engine Yard Cloud Deploy Options +# +#################################################################### +# IMPORTANT +# Commit this file into your git repository. +# These options are loaded on the server during deploy. +#################################################################### +# +# Valid locations: +# * REPOSITORY_ROOT/config/ey.yml. +# * REPOSITORY_ROOT/ey.yml +# +# Further information available here: +# https://support.cloud.engineyard.com/entries/20996661-customize-your-deployment-on-engine-yard-cloud +# +# For advanced usage, see the source that loads this configuration: +# https://github.com/engineyard/engineyard-serverside/blob/master/lib/engineyard-serverside/configuration.rb +# +defaults: + # Run migrations during deploy by default. + # + # When set to true, runs the migration_command (below) during deploy. + # + # This setting can be overridden for individual deployments using + # the command line options --migrate or --no-migrate. + # + migrate: true + + # Default migration command to run when migrations are enabled. + # + migration_command: rake db:migrate --trace + + # Enables rails assets precompilation always and halts when the task fails. + # + # By default, assets are detected using app/assets and config/application.rb. + # + # If you use rails assets, set this to true. + # + # For more control over assets, set precompile_assets: false and + # run your precompile task in the deploy/before_compile_assets.rb deploy hook. + # + precompile_assets: false + + # Override the assets:precompile rake task. This option will be used instead + # of assets:precompile in the `rake assets:precompile` command. + # + #precompile_assets_task: assets:precompile + + # Asset strategies affect the way assets are stored on the server. + # + # * private + # Store assets directly in public/assets for each deployment. + # Previous assets are symlinked for continuity. + # When assets are reused, they are copied using rsync. + # + # * shifting + # Assets are kept in a shared directory on each server. + # When new assets are compiled, old assets are shifted to a shared + # last_assets directory. This has always been the default behavior. + # + # * shared + # Assets are kept in a shared directory on each server. + # When new assets are compiled, the same directory is used. + # Assets will accumulate in this mode if a cleaning script is not run. + # Use this strategy if you want to write your own asset cleaning script. + # + # * cleaning + # Like shared, but a cleaning script is run before each new compile. + # The script attempts to remove all files not mentioned by the old + # manifest.yml, before it is replaced by the new manifest (leaving 2 + # deployments worth of assets in the directory) + # + # "private" is recommended because it is the least error prone. + # If you prefer faster compilation, "shared" can be quicker, but will require + # custom scripting and will cause problems when rollbacks are used. + # "shifting" is the default behavior. + # + #asset_strategy: private + + # This list of repository relative paths is checked for changes during + # each deployment (when change detection is not disabled). If `git diff` + # detects changes since the last deployment, fresh assets will be compiled. + # + # This option overrides the default list, so include the following + # defaults if you need them. + # + #asset_dependencies: + #- app/assets # default + #- lib/assets # default + #- vendor/assets # default + #- Gemfile.lock # default + #- config/application.rb # default + #- config/routes.rb # default + #- config/requirejs.yml # example of a custom asset dependency + + # When true, precompiles assets even if no changes would be detected by + # running git diff with the asset_dependencies above. + # + # Default is false (always check git diff before asset compilation) + # + #precompile_unchanged_assets: false + + # Choose which servers should compile assets. + # + # Default behavior is to exclude util instances. + # Specify :all to compile on all servers including util servers. + # + #asset_roles: :all + + # Bundle without different bundler groups: + # Ex: bundle install --without '[bundle_without]' + # + # Default is "". + # Leave blank to remove --without from the bundle install command. + # + #bundle_without: + + # Add extra options to the bundle install command line. + # Does not override bundle_without, if specified. + # + # If the application's gems are vendored in the + # repository, setting --local can speed up bundle. + # + #bundle_options: + + # Enable maintenance page during migrate action (default) + # Setting this to false, disables maintenance page during migrations. + # + # CAUTION! No-downtime migrations requires careful migration + # planning. Migrations must be non-destructive. The *previous* + # deployment might serve pages during a partially migrated state. + # For example, if you rename a column, all traffic served during + # that migration will be broken until the new code is deployed. + # + #maintenance_on_migrate: true + + # Enable maintanence page during every deploy. + # Unicorn and Passenger support no-downtime deploys, so the default + # for these servers is false. Mongrel and some other servers default + # to true to avoid downtime during server restarting. + # + #maintenance_on_restart: + + # If true, always run deployments in verbose mode. + # + #verbose: false + + # Hide the warning shown when the Gemfile does not contain a recognized + # database adapter (mongodb for example) + # + # This warning is here to help new customers that accidentally have no adapter. + # You may safely set this to true if you aren't using a common database. + # + #ignore_database_adapter_warning: false + + # You can add custom keys that will be available in your deploy hooks. + # Custom keys will be available using config.key or config[:key] + # + #your_own_custom_key: custom info + + +#################################################################### +# Environment specific options. +# +# The options you specify here will only apply to a single environment +# that exactly matches the environment name key. +# +# Environment options will override the default options above. +# +environments: + + # These options will only apply to the EXAMPLE_ENVIRONMENT environment. + #EXAMPLE_ENVIRONMENT: + #precompile_unchanged_assets: true + diff --git a/test/functional/people_controller_test.rb b/test/functional/people_controller_test.rb index 2fc755e2..84505b1e 100644 --- a/test/functional/people_controller_test.rb +++ b/test/functional/people_controller_test.rb @@ -70,11 +70,27 @@ def test_requires_manager assert_redirected_to new_session_path end - def test_auto_complete - get :auto_complete_for_person_full_name, :organization_key => 'sfbk', :person => { :full_name => 'memb' } - assert_response :success - assert_not_nil assigns(:items) - assert_equal 1, assigns(:items).size + context "Auto-complete" do + + setup do + get :auto_complete_for_person_full_name, :organization_key => 'sfbk', :person => { :full_name => 'memb' } + end + + should respond_with :success + should assign_to :items + + should 'find matching items' do + assert_equal 1, assigns(:items).size + end + + should 'render list of people' do + assert_select 'ul li.person', 1 + end + + should 'render add link' do + assert_select 'ul li.add', 1 + end + end context "Update person" do diff --git a/test/functional/taggings_controller_test.rb b/test/functional/taggings_controller_test.rb index 857a8e08..0974bf50 100644 --- a/test/functional/taggings_controller_test.rb +++ b/test/functional/taggings_controller_test.rb @@ -68,6 +68,10 @@ class TaggingsControllerTest < ActionController::TestCase should 'have the new tag' do assert_equal ['mechanic', 'mom', 'three'], people(:mary).tag_list end + + should 'render links to delete tags' do + assert_select '.tags_control .edit .tag a.delete', 3 + end end end diff --git a/test/unit/role_test.rb b/test/unit/role_test.rb index c552c6fb..3e5a6547 100644 --- a/test/unit/role_test.rb +++ b/test/unit/role_test.rb @@ -1,17 +1,20 @@ require 'test_helper' -class RoleTest < ActiveSupport::TestCase +class RolesTest < ActiveSupport::TestCase + include ApplicationHelper def test_admin_role - assert users(:admin).is_admin? - assert !users(:sfbk).is_admin? + User.current_user = users(:admin) + assert user_is_admin? + User.current_user = users(:greeter) + assert !user_is_admin? end def test_manager_role - assert users(:sfbk).is_manager? - assert users(:sfbk).is_manager_of?(organizations(:sfbk)) - assert !users(:sfbk).is_manager_of?(organizations(:scbc)) + User.current_user = users(:sfbk) + @organization = organizations(:sfbk) + assert user_is_manager? + User.current_user = users(:scbc) + assert !user_is_manager? end - end - diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index 02bc1d09..1524979d 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -106,6 +106,19 @@ def test_reset assert User.authenticate('sfbk', 'new_password') end + def test_is_magager_of + assert users(:sfbk).is_manager_of?(organizations(:sfbk)) + assert !users(:sfbk).is_manager_of?(nil) + assert !users(:sfbk).is_manager_of?(organizations(:scbc)) + assert !create_user.is_manager_of?(organizations(:scbc)) + end + + def test_is_magager_of + assert users(:admin).is_admin? + assert !users(:sfbk).is_admin? + end + + protected def create_user(options = {}) User.create({ :name => 'Quire', :login => 'quire', :email => 'quire@example.com', :password => 'quire', :password_confirmation => 'quire' }.merge(options)) diff --git a/test/unit/visit_test.rb b/test/unit/visit_test.rb index 526312e7..5e1e14bd 100644 --- a/test/unit/visit_test.rb +++ b/test/unit/visit_test.rb @@ -34,11 +34,11 @@ def test_chain_finders end def test_to_csv - assert_match /^Mary,Member,mary@example.com,false,415 123-1234,95105,2007-02-01 10:01,false,true,false,Mary.+/, visits(:mary_1).to_csv + assert_match /^365790011,Mary,Member,mary@example.com,false,415 123-1234,95105,2007-02-01 10:01,false,true,false,Mary.+/, visits(:mary_1).to_csv end def test_csv_header - assert_equal "first_name,last_name,email,email_opt_out,phone,postal_code,arrived_at,staff,member,volunteer,note\n", Visit.csv_header + assert_equal "person_id,first_name,last_name,email,email_opt_out,phone,postal_code,arrived_at,staff,member,volunteer,note\n", Visit.csv_header end def test_create_defaults