Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Authorization #17

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
tmp/
.idea
*.iml
*.iml
script/log/
1 change: 0 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ source 'https://rubygems.org'

gem "rails", "2.3.17"
gem "mysql"
gem "authorization", github: "asalant/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"
Expand Down
7 changes: 0 additions & 7 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
GIT
remote: git://github.com/asalant/rails-authorization-plugin.git
revision: 505bd47addf2ef5e3b5d98a7ea8d1352c2aa4ee6
specs:
authorization (1.0.12)

GEM
remote: https://rubygems.org/
specs:
Expand Down Expand Up @@ -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)
Expand Down
11 changes: 11 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion app/controllers/notes_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
class NotesController < ApplicationController

permit "admin or (manager of :organization)"
before_filter :authorize_admin_or_manager

# GET /notes
# GET /notes.xml
Expand Down
9 changes: 5 additions & 4 deletions app/controllers/organizations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:destroy?


# GET /organizations
# GET /organizations.xml
def index
Expand Down Expand Up @@ -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 }
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/people_controller.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/reports_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

class ReportsController < ApplicationController

permit "admin or (manager of :organization)"
before_filter :authorize_admin_or_manager

def index
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/services_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
class ServicesController < ApplicationController

permit "admin or (manager of :organization)"
before_filter :authorize_admin_or_manager

# GET /services
# GET /services.xml
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/taggings_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class TaggingsController < ApplicationController
permit "admin or (manager of :organization)"
before_filter :authorize_admin_or_manager

def create
@person.tag_list << params[:id]
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/tags_controller.rb
Original file line number Diff line number Diff line change
@@ -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])
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/visits_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
class VisitsController < ApplicationController

permit "admin or (manager of :organization)"
before_filter :authorize_admin_or_manager

# GET /visits
# GET /visits.xml
Expand Down
11 changes: 11 additions & 0 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 2 additions & 3 deletions app/models/organization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -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)'
Expand Down Expand Up @@ -71,4 +70,4 @@ def validate_timezone
errors.add :timezone if !@timezone_names.include?(self.timezone)
end
end
end
end
38 changes: 25 additions & 13 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
2 changes: 1 addition & 1 deletion app/views/layouts/application.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion app/views/organizations/index.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion app/views/users/show.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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)

19 changes: 11 additions & 8 deletions test/unit/role_test.rb
Original file line number Diff line number Diff line change
@@ -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

13 changes: 13 additions & 0 deletions test/unit/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
4 changes: 2 additions & 2 deletions test/unit/visit_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down