Skip to content

Commit

Permalink
Authorize update/create on a per-class basis
Browse files Browse the repository at this point in the history
Previously it was checking access to create ActiveFedora::Base
  • Loading branch information
jcoyne committed Jan 1, 2014
1 parent c8f2b6b commit 78e6429
Show file tree
Hide file tree
Showing 12 changed files with 134 additions and 74 deletions.
2 changes: 2 additions & 0 deletions .rspec
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
--colour

3 changes: 1 addition & 2 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ task :setup do
Dir.chdir(dummy) do
puts "Generating test app ..."
sh "rails generate test_app"
#sh "rm spec/models/user_spec.rb" # generated by devise
sh "touch public/test.html" # for spec/features/record_editing_spec.rb
sh "touch public/test.html" # for spec/features/record_editing_spec.rb
end
end
end
Expand Down
83 changes: 44 additions & 39 deletions app/controllers/concerns/records_controller_behavior.rb
Original file line number Diff line number Diff line change
@@ -1,18 +1,24 @@
module RecordsControllerBehavior
extend ActiveSupport::Concern

included do
before_filter :load_and_authorize_record, only: [:edit, :update]
end
load_and_authorize_resource only: [:new, :edit, :update, :create], instance_name: resource_instance_name

def new
authorize! :create, ActiveFedora::Base
unless has_valid_type?
rescue_from HydraEditor::InvalidType do
render 'records/choose_type'
return
end
end

@record = params[:type].constantize.new
module ClassMethods
def cancan_resource_class
HydraEditor::ControllerResource
end
def resource_instance_name
'record'
end
end

def new
initialize_fields
render 'records/new'
end
Expand All @@ -23,23 +29,15 @@ def edit
end

def create
authorize! :create, ActiveFedora::Base
unless has_valid_type?
redirect_to(respond_to?(:hydra_editor) ? hydra_editor.new_record_path : new_record_path, flash: {error: "Lost the type"})
return
end
@record = params[:type].constantize.new
set_attributes

respond_to do |format|
if @record.save
if resource.save
format.html { redirect_to redirect_after_create, notice: 'Object was successfully created.' }
# ActiveFedora::Base#to_json causes a circular reference. Do something easy
data = @record.terms_for_editing.inject({}) { |h,term| h[term] = @record[term]; h }
format.json { render json: data, status: :created, location: redirect_after_create }
format.json { render json: object_as_json, status: :created, location: redirect_after_create }
else
format.html { render action: "new" }
format.json { render json: @record.errors, status: :unprocessable_entity }
format.json { render json: resource.errors, status: :unprocessable_entity }
end
end

Expand All @@ -48,60 +46,67 @@ def create
def update
set_attributes
respond_to do |format|
if @record.save
if resource.save
format.html { redirect_to redirect_after_update, notice: 'Object was successfully updated.' }
format.json { head :no_content }
else
format.html { render action: "edit" }
format.json { render json: @record.errors, status: :unprocessable_entity }
format.json { render json: resource.errors, status: :unprocessable_entity }
end
end
end

protected

def load_and_authorize_record
load_record
authorize_record!
end

def load_record
@record = ActiveFedora::Base.find(params[:id], cast: true)
end

def authorize_record!
authorize! params[:action].to_sym, @record
def object_as_json
# ActiveFedora::Base#to_json causes a circular reference (before 7.0). Do something easy
resource.terms_for_editing.each_with_object({}) { |term, h| h[term] = resource[term] }
end

# Override this method if you want to set different metadata on the object
def set_attributes
@record.attributes = collect_form_attributes
resource.attributes = collect_form_attributes
end

def collect_form_attributes
attributes = params[ActiveModel::Naming.singular(@record)]
attributes = params[ActiveModel::Naming.singular(resource)]
# removes attributes that were only changed by initialize_fields
attributes.reject { |key, value| @record[key].empty? and value == [""] }
attributes.reject { |key, value| resource[key].empty? and value == [""] }
end

# Override to redirect to an alternate location after create
def redirect_after_create
main_app.catalog_path @record
main_app.catalog_path resource
end

# Override to redirect to an alternate location after update
def redirect_after_update
main_app.catalog_path @record
main_app.catalog_path resource
end

def has_valid_type?
HydraEditor.models.include? params[:type]
end

def initialize_fields
@record.terms_for_editing.each do |key|
resource.terms_for_editing.each do |key|
# if value is empty, we create an one element array to loop over for output
@record[key] = [''] if @record[key].empty?
resource[key] = [''] if resource[key].empty?
end
end

def resource
get_resource_ivar
end

# Get resource ivar based on the current resource controller.
#
def get_resource_ivar #:nodoc:
instance_variable_get("@#{resource_instance_name}")
end

def resource_instance_name
self.class.resource_instance_name
end

end
1 change: 0 additions & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
HydraEditor::Engine.routes.draw do
resources :records, only: [:new, :edit, :create, :update], :constraints => { :id => /[a-zA-Z0-9_.:]+/ }

end
2 changes: 1 addition & 1 deletion gemfiles/gemfile.rails3
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
source "http://rubygems.org"
source "https://rubygems.org"

gemspec :path=>"../"

Expand Down
2 changes: 1 addition & 1 deletion gemfiles/gemfile.rails4
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
source "http://rubygems.org"
source "https://rubygems.org"

gemspec :path=>"../"

Expand Down
12 changes: 12 additions & 0 deletions lib/hydra-editor.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,24 @@
require "cancan"
require "hydra_editor/engine"
require "bootstrap_forms"

module HydraEditor

class InvalidType < RuntimeError; end

extend ActiveSupport::Autoload

autoload :ControllerResource

def self.models= val
@models = val
end

def self.models
@models ||= []
end

def self.valid_model?(type)
models.include? type
end
end
18 changes: 18 additions & 0 deletions lib/hydra_editor/controller_resource.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
class HydraEditor::ControllerResource < CanCan::ControllerResource
def find_resource
ActiveFedora::Base.find(id_param, cast: true)
end

def resource_class
raise HydraEditor::InvalidType, "Lost the type" unless has_valid_type?
type_param.constantize
end

def has_valid_type?
HydraEditor.valid_model? type_param
end

def type_param
@params[:type]
end
end
8 changes: 5 additions & 3 deletions lib/hydra_editor/engine.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
module HydraEditor
class Engine < ::Rails::Engine
engine_name 'hydra_editor'
config.paths.add "app/helpers/concerns", eager_load: true
config.paths.add "app/controllers/concerns", eager_load: true
config.paths.add "app/models/concerns", eager_load: true
config.autoload_paths += %W(
#{config.root}/app/helpers/concerns
#{config.root}/app/controllers/concerns
#{config.root}/app/models/concerns
)
end
end
65 changes: 45 additions & 20 deletions spec/controllers/records_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -1,20 +1,23 @@
require 'spec_helper'

describe RecordsController do
routes { HydraEditor::Engine.routes }
before do
HydraEditor.models = ['Audio', 'Pdf']
end
describe "an admin" do
let(:user) { FactoryGirl.create(:admin) }
before do
HydraEditor.models = ['Audio', 'Pdf']
@user = FactoryGirl.create(:admin)
sign_in @user
sign_in user
end
describe "who goes to the new page" do
it "should be successful" do
get :new, use_route: 'hydra_editor'
get :new
response.should be_successful
response.should render_template(:choose_type)
end
it "should be successful" do
get :new, :type=>'Audio', :use_route=>'hydra_editor'
get :new, :type=>'Audio'
response.should be_successful
response.should render_template(:new)
end
Expand All @@ -28,14 +31,26 @@
stub_audio.should_receive(:save).and_return(true)
end
it "should be successful" do
post :create, :type=>'Audio', :audio=>{:title=>"My title"}, :use_route=>'hydra_editor'
post :create, :type=>'Audio', :audio=>{:title=>"My title"}
response.should redirect_to("/catalog/#{assigns[:record].id}")
assigns[:record].title.should == ['My title']
end
it "should be successful with json" do
post :create, :type=>'Audio', :audio=>{:title=>"My title"}, :format=>:json, :use_route=>'hydra_editor'
post :create, :type=>'Audio', :audio=>{:title=>"My title"}, :format=>:json
response.status.should == 201
end

describe "when the user has access to create only some classes" do
before do
controller.current_ability.cannot :create, ActiveFedora::Base
controller.current_ability.can :create, Audio
end
it "should be successful" do
post :create, :type=>'Audio', :audio=>{:title=>"My title"}
response.should redirect_to("/catalog/#{assigns[:record].id}")
assigns[:record].title.should == ['My title']
end
end
describe "when set_attributes is overloaded" do
controller(RecordsController) do
def set_attributes
Expand All @@ -49,11 +64,21 @@ def set_attributes
assigns[:record].creator.should == ["Fleece Vest"]
end
end
describe "when object_as_json is overloaded" do
before do
controller.stub(:object_as_json).and_return({message: 'it works'} )
end
it "should run object_as_json" do
post :create, :type=>'Audio', :audio=>{:title=>"My title"}, format: 'json'
expect(JSON.parse(response.body)).to eq({"message" => "it works"})
expect(response.code).to eq '201'
end
end
describe "when redirect_after_create is overridden" do
it "should redirect to the alternate location" do
controller.stub(:redirect_after_create).and_return(root_url)
post :create, :type=>'Audio', :audio=>{:title=>"My title"}, :use_route=>'hydra_editor'
response.should redirect_to(root_url)
controller.stub(:redirect_after_create).and_return('/')
post :create, :type=>'Audio', :audio=>{:title=>"My title"}
response.should redirect_to('/')
end
end
end
Expand All @@ -65,7 +90,7 @@ def set_attributes
controller.should_receive(:authorize!).with(:edit, @audio)
end
it "should be successful" do
get :edit, :id=>@audio.pid, :use_route=>'hydra_editor'
get :edit, :id=>@audio.pid
response.should be_successful
assigns[:record].title.should == ['My title2']
end
Expand All @@ -80,33 +105,33 @@ def set_attributes
controller.should_receive(:authorize!).with(:update, @audio)
end
it "should be successful" do
put :update, :id=>@audio, :audio=>{:title=>"My title 3"}, :use_route=>'hydra_editor'
put :update, :id=>@audio, :audio=>{:title=>"My title 3"}
response.should redirect_to("/catalog/#{assigns[:record].id}")
assigns[:record].title.should == ['My title 3']
end
it "should be successful with json" do
put :update, :id=>@audio.pid, :audio=>{:title=>"My title"}, :format=>:json, :use_route=>'hydra_editor'
put :update, :id=>@audio.pid, :audio=>{:title=>"My title"}, :format=>:json
response.status.should == 204
end
describe "when redirect_after_update is overridden" do
it "should redirect to the alternate location" do
controller.stub(:redirect_after_update).and_return(root_url)
put :update, :id=>@audio, :audio=>{:title=>"My title 3"}, :use_route=>'hydra_editor'
response.should redirect_to(root_url)
controller.stub(:redirect_after_update).and_return('/')
put :update, :id=>@audio, :audio=>{:title=>"My title 3"}
response.should redirect_to('/')
end
end
end
end

describe "a user without create ability" do
let(:user) { FactoryGirl.create(:user) }
before do
@user = FactoryGirl.create(:user)
sign_in @user
controller.current_ability.cannot :create, ActiveFedora::Base
sign_in user
controller.current_ability.cannot :create, Audio
end
describe "who goes to the new page" do
it "should not be allowed" do
lambda { get :new, :use_route=>'hydra_editor' }.should raise_error CanCan::AccessDenied
lambda { get :new, type: 'Audio' }.should raise_error CanCan::AccessDenied
end
end
end
Expand Down
8 changes: 4 additions & 4 deletions spec/support/audio.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
# A class just for testing
class Audio < ActiveFedora::Base
include Hydra::ModelMixins::RightsMetadata
include Hydra::AccessControls::Permissions

# Uses the Hydra Rights Metadata Schema for tracking access permissions & copyright
has_metadata "rightsMetadata", type: Hydra::Datastream::RightsMetadata

# Tufts specific needed metadata streams
has_metadata "descMetadata", :type => ActiveFedora::QualifiedDublinCoreDatastream
has_metadata "descMetadata", type: ActiveFedora::QualifiedDublinCoreDatastream

validates_presence_of :title#, :creator, :description
validates_presence_of :title

delegate_to "descMetadata", [:title, :creator, :description, :subject]
has_attributes :title, :creator, :description, :subject, datastream: "descMetadata", multiple: true

def terms_for_editing
[:title, :creator, :description, :subject]
Expand Down
Loading

0 comments on commit 78e6429

Please sign in to comment.