Diff
Modified: trunk/app/controllers/group_announcements_controller.rb (3498 => 3499)
--- trunk/app/controllers/group_announcements_controller.rb 2013-04-11 12:26:59 UTC (rev 3498)
+++ trunk/app/controllers/group_announcements_controller.rb 2013-04-11 15:00:18 UTC (rev 3499)
@@ -99,17 +99,17 @@
protected
def find_group
- begin
- @group = Network.find(params[:network_id])
- rescue ActiveRecord::RecordNotFound
- error("Group couldn't be found")
+ @group = Network.find_by_id(params[:network_id])
+
+ if @group.nil?
+ render_404("Group not found.")
end
end
def check_admin
unless @group.administrator?(current_user.id)
- error("Only group administrators are allowed to create new announcements")
+ render_401("Only group administrators are allowed to create new announcements.")
end
end
@@ -122,61 +122,36 @@
def find_announcement_auth
- begin
- begin
- # find the announcement first
- @announcement = GroupAnnouncement.find(params[:id])
-
- # announcement found, but check if belongs to the group in URL
- unless @group.announcements.include?(@announcement)
- raise ActiveRecord::RecordNotFound
- end
- rescue ActiveRecord::RecordNotFound
- raise ActiveRecord::RecordNotFound, "Group announcement was not found"
- end
-
+ # find the announcement first
+ @announcement = GroupAnnouncement.find_by_id_and_network_id(params[:id], params[:network_id])
+
+ if @announcement.nil?
+ render_404("Group announcement not found.")
+ else
+
# at this point, group announcement is found and it definitely belongs to the group in URL;
# now go through different actions and check which links are allowed for current user
not_auth = false
case action_name.to_s.downcase
when "show"
# if the announcement is private, show it only to group members
- unless @announcement.public
- not_auth = true unless @group.member?(current_user.id)
+ unless @announcement.public || @group.member?(current_user.id)
+ not_auth = true
end
when "edit","update","destroy"
# only owner of the group can destroy the announcement
- unless ((@announcement.user == current_user) || (@group.owner?(current_user.id)))
- not_auth = true;
- raise ActiveRecord::RecordNotFound, "You don't have permissions to perform this action"
+ unless (@announcement.user == current_user) || (@group.owner?(current_user.id))
+ not_auth = true
end
else
# don't allow anything else, for now
not_auth = true
end
-
-
+
# check if we had any errors
if not_auth
raise ActiveRecord::RecordNotFound, "Group announcement was not found"
end
-
- rescue ActiveRecord::RecordNotFound => exc
- error(exc.message)
end
end
-
-
- private
-
- def error(message)
- flash[:error] = message
- return_to_path = @group.nil? ? networks_path : group_announcements_path(@group)
-
- respond_to do |format|
- format.html { redirect_to return_to_path }
- end
- end
-
-
end
Modified: trunk/app/controllers/group_policies_controller.rb (3498 => 3499)
--- trunk/app/controllers/group_policies_controller.rb 2013-04-11 12:26:59 UTC (rev 3498)
+++ trunk/app/controllers/group_policies_controller.rb 2013-04-11 15:00:18 UTC (rev 3499)
@@ -75,7 +75,10 @@
format.html { redirect_to network_policies_path(@group) }
end
else
- error("This policy is being used by address@hidden resources and may not be deleted.")
+ respond_to do |format|
+ flash[:error] = "This policy is being used by address@hidden resources and may not be deleted."
+ format.html { redirect_to network_policies_path(@group) }
+ end
end
end
@@ -83,38 +86,25 @@
protected
def find_group
- begin
- @group = Network.find(params[:network_id])
- rescue ActiveRecord::RecordNotFound
- error("Group couldn't be found")
+ @group = Network.find_by_id(params[:network_id])
+
+ if @group.nil?
+ render_404("Group not found.")
end
end
def find_policy
- begin
- @policy = Policy.find(params[:id])
- rescue ActiveRecord::RecordNotFound
- error("Policy couldn't be found")
+ @policy = Policy.find_by_id(params[:id])
+
+ if @policy.nil?
+ render_404("Policy not found.")
end
end
def check_admin
unless @group.administrator?(current_user.id)
- error("Only group administrators are allowed to manage policies")
+ render_401("Only group administrators are allowed to manage policies.")
end
end
-
- private
-
- def error(message)
- flash[:error] = message
- return_to_path = @group.nil? ? networks_path : network_policies_path(@group)
-
- respond_to do |format|
- format.html { redirect_to return_to_path }
- end
- end
-
-
end
Modified: trunk/app/controllers/jobs_controller.rb (3498 => 3499)
--- trunk/app/controllers/jobs_controller.rb 2013-04-11 12:26:59 UTC (rev 3498)
+++ trunk/app/controllers/jobs_controller.rb 2013-04-11 15:00:18 UTC (rev 3499)
@@ -9,7 +9,8 @@
before_filter :check_runner_available, : [:new, :update]
- before_filter :find_experiment_auth
+ before_filter :find_experiment
+ before_filter :auth_experiment, :except => [:create, :new]
before_filter :find_jobs, : [:index]
before_filter :find_job_auth, :except => [:index, :new, :create]
@@ -348,8 +349,19 @@
end
end
- def find_experiment_auth
+ def find_experiment
+ return if ["create","new"].include?(action_name) && params[:experiment_id].nil?
+ @experiment = Experiment.find_by_id(params[:experiment_id])
+
+ if @experiment.nil?
+ render_404("Experiment not found.")
+ end
+ end
+
+ def auth_experiment
+ return if ["create","new"].include?(action_name) && params[:experiment_id].nil?
+
action_permissions = {
"create" => "create",
"destroy" => "destroy",
@@ -360,15 +372,8 @@
"update" => "edit"
}
- experiment = Experiment.find(:first, :conditions => ["id = ?", params[:experiment_id]])
-
- if experiment and Authorization.check(action_permissions[action_name], experiment, current_user)
- @experiment = experiment
- else
- # New and Create actions are allowed to run outside of the context of an Experiment
- unless ['new', 'create'].include?(action_name.downcase)
- error("The Experiment that this Job belongs to could not be found or the action is not authorized", "is invalid (not authorized)")
- end
+ unless Authorization.check(action_permissions[action_name], @experiment, current_user)
+ render_401("You are not authorized to access this experiment.")
end
end
@@ -396,27 +401,16 @@
"update" => "edit",
}
- job = Job.find(:first, :conditions => ["id = ?", params[:id]])
+ @job = Job.find_by_id(params[:id])
- if job and job.experiment.id == @experiment.id and Authorization.check(action_permissions[action_name], job, current_user)
- @job = job
- else
- error("Job not found or action not authorized", "is invalid (not authorized)")
+ if @job.nil? || @job.experiment.id != @experiment.id
+ render_404("Job not found.")
+ elsif !Authorization.check(action_permissions[action_name], @job, current_user)
+ render_401("Action not authorized.")
end
end
def check_runnable_supported
# TODO: move all checks for the runnable object here!
end
-
-private
-
- def error(notice, message, attr=:id)
- flash[:error] = notice
- (err = Job.new.errors).add(attr, message)
-
- respond_to do |format|
- format.html { redirect_to experiment_jobs_url(params[:experiment_id]) }
- end
- end
end
Modified: trunk/app/controllers/memberships_controller.rb (3498 => 3499)
--- trunk/app/controllers/memberships_controller.rb 2013-04-11 12:26:59 UTC (rev 3498)
+++ trunk/app/controllers/memberships_controller.rb 2013-04-11 15:00:18 UTC (rev 3499)
@@ -7,8 +7,9 @@
before_filter :login_required
before_filter :check_user_present # only allow actions on memberships as on nested resources
-
- before_filter :find_memberships, : [:index]
+
+ before_filter :find_network, : :new
+ before_filter :find_user_auth, : :index
before_filter :find_membership_auth, : [:show, :accept, :edit, :update, :destroy]
# declare sweepers and which actions should invoke them
@@ -84,14 +85,17 @@
flash[:notice] = 'Membership was successfully accepted.'
format.html { redirect_to network_url(@membership.network_id) }
else
- error("Membership already accepted", "already accepted")
+ flash[:error] = "Membership already accepted."
end
+ format.html { redirect_to network_url(@membership.network_id) }
end
end
# GET /users/1/memberships
# GET /memberships
def index
+ @memberships = @user.memberships
+
respond_to do |format|
format.html # index.rhtml
end
@@ -122,14 +126,8 @@
# GET /users/1/memberships/new
# GET /memberships/new
def new
- if params[:network_id]
- begin
- @network = Network.find(params[:network_id])
-
- @membership = Membership.new(:user_id => current_user.id, :network_id => @network.id)
- rescue ActiveRecord::RecordNotFound
- error("Group not found", "is invalid", :network_id)
- end
+ if @network
+ @membership = Membership.new(:user_id => current_user.id, :network_id => @network.id)
else
@membership = Membership.new(:user_id => current_user.id)
end
@@ -191,7 +189,10 @@
end
end
else
- error("Membership not created (already exists)", "not created, already exists")
+ respond_to do |format|
+ flash[:error] = "Membership not created (already exists)"
+ format.html { render :action ="" "new" }
+ end
end
end
@@ -339,51 +340,30 @@
end
end
- def find_memberships
- if params[:user_id].to_i == current_user.id.to_i
- begin
- @user = User.find(params[:user_id])
-
- @memberships = @user.memberships
- rescue ActiveRecord::RecordNotFound
- error("User not found", "is invalid", :user_id)
- end
- else
- error("You are not authorised to view other users' memberships", "")
+ def find_network
+ @network = Network.find_by_id(params[:network_id])
+
+ if @network.nil? && params[:network_id]
+ render_404("Group not found.")
end
end
- def find_membership
- if params[:user_id]
- begin
- @user = User.find(params[:user_id])
-
- begin
- @membership = Membership.find(params[:id], :conditions => ["user_id = ?", @user.id])
- rescue ActiveRecord::RecordNotFound
- error("Membership not found", "is invalid")
- end
- rescue ActiveRecord::RecordNotFound
- error("User not found", "is invalid", :user_id)
- end
- else
- begin
- @membership = Membership.find(params[:id])
- rescue ActiveRecord::RecordNotFound
- error("Membership not found", "is invalid")
- end
+ def find_user_auth
+ @user = User.find_by_id(params[:user_id])
+
+ if @user.nil?
+ render_404("User not found.")
+ elsif @user != current_user
+ render_401("You are not authorised to view other users' memberships.")
end
end
-
+
def find_membership_auth
- begin
- begin
- # find the membership first
- @membership = Membership.find(params[:id])
- rescue ActiveRecord::RecordNotFound
- raise ActiveRecord::RecordNotFound, "Membership not found"
- end
-
+ @membership = Membership.find_by_id(params[:id])
+
+ if @membership.nil?
+ render_404("Membership not found.")
+ else
# now go through different actions and check which links (including user_id in the link) are allowed
not_auth = false
case action_name.to_s.downcase
@@ -392,34 +372,30 @@
# depending on who initiated it (link is for current user's id only)
if @membership.user_established_at == nil
unless @membership.user_id == current_user.id && params[:user_id].to_i == @membership.user_id
- not_auth = true;
+ not_auth = true
end
elsif @membership.network_established_at == nil
unless @membership.network.administrator?(current_user.id) # TODO: CHECK WHY?! && params[:user_id].to_i == @membership.network.owner.id
- not_auth = true;
+ not_auth = true
end
end
when "show", "destroy", "update"
# Only the owner of the network OR the person who the membership is for can view/delete memberships;
# link - just user to whom the membership belongs
- unless (@membership.network.administrator?(current_user.id) || @membership.user_id == current_user.id) && @membership.user_id == params[:user_id].to_i
+ unless (@membership.network.administrator?(current_user.id) ||
+ @membership.user_id == current_user.id) && @membership.user_id == params[:user_id].to_i
not_auth = true
end
else
# don't allow anything else, for now
not_auth = true
end
-
-
+
# check if we had any errors
if not_auth
- raise ActiveRecord::RecordNotFound, "You are not authorised to view other users' memberships"
+ render_401("You are not authorised to view other users' memberships.")
end
-
- rescue ActiveRecord::RecordNotFound => exc
- error(exc.message, "")
end
-
end
private
@@ -428,14 +404,4 @@
message = Message.new(:from => from_id, :to => to_id, :subject => subject, :body => body, :reply_id => nil, :read_at => nil, :deleted_by_sender => true )
message.save
end
-
- def error(notice, message, attr=:id)
- flash[:error] = notice
- (err = Membership.new.errors).add(attr, message)
-
- respond_to do |format|
- format.html { redirect_to user_memberships_url(current_user.id) }
- end
- end
-
end
Modified: trunk/app/controllers/messages_controller.rb (3498 => 3499)
--- trunk/app/controllers/messages_controller.rb 2013-04-11 12:26:59 UTC (rev 3498)
+++ trunk/app/controllers/messages_controller.rb 2013-04-11 15:00:18 UTC (rev 3499)
@@ -55,10 +55,9 @@
# if current_user is not recipient, they must be the sender
message_folder = ( @message.recipient?(current_user.id) ? "inbox" : "outbox" )
- if (message_folder == "inbox" && @message.deleted_by_recipient == true)
- error("Message not found (id not authorized)", "is invalid (not sender or recipient)")
- elsif (message_folder == "outbox" && @message.deleted_by_sender == true)
- error("Message not found (id not authorized)", "is invalid (not sender or recipient)")
+ if (message_folder == "inbox" && @message.deleted_by_recipient == true) ||
+ (message_folder == "outbox" && @message.deleted_by_sender == true)
+ render_404("Message not found.")
else
# message is found, and is not deleted by current_user -> show the message;
# mark message as read if it is viewed by the receiver
@@ -83,7 +82,6 @@
end
end
end
-
end
@@ -242,19 +240,11 @@
protected
- def find_message_by_to
- begin
- @message = Message.find(params[:id], :conditions => ["`to` = ?", current_user.id])
- rescue ActiveRecord::RecordNotFound
- error("Message not found (id not authorized)", "is invalid (not recipient)")
- end
- end
-
def find_message_by_to_or_from
begin
@message = Message.find(params[:id], :conditions => ["`to` = ? OR `from` = ?", current_user.id, current_user.id])
rescue ActiveRecord::RecordNotFound
- error("Message not found (id not authorized)", "is invalid (not sender or recipient)")
+ render_404("Message not found.")
end
end
@@ -263,7 +253,7 @@
begin
@reply = Message.find(params[:reply_id], :conditions => ["`to` = ?", current_user.id])
rescue ActiveRecord::RecordNotFound
- error("Reply not found (id not authorized)", "is invalid (not recipient)")
+ render_404("Reply not found.")
end
end
end
@@ -303,15 +293,4 @@
return ordering
end
-
-private
-
- def error(notice, message)
- flash[:error] = notice
- (err = Message.new.errors).add(:id, message)
-
- respond_to do |format|
- format.html { redirect_to messages_url }
- end
- end
end
Modified: trunk/app/controllers/networks_controller.rb (3498 => 3499)
--- trunk/app/controllers/networks_controller.rb 2013-04-11 12:26:59 UTC (rev 3498)
+++ trunk/app/controllers/networks_controller.rb 2013-04-11 15:00:18 UTC (rev 3499)
@@ -12,7 +12,9 @@
before_filter :login_required, :except => [:index, :show, :content, :search, :all]
before_filter :find_networks, : [:all]
- before_filter :find_network, : [:membership_request, :show, :tag, :content]
+ before_filter :find_network, : [:membership_request, :show, :tag, :content,
+ :edit, :update, :destroy, :invite, :membership_invite,
+ :membership_invite_external]
before_filter :find_network_auth_admin, : [:invite, :membership_invite, :membership_invite_external]
before_filter :find_network_auth_owner, : [:edit, :update, :destroy]
@@ -406,39 +408,19 @@
:host => base_host,
:id => @network.id
rescue ActiveRecord::RecordNotFound
- error("Group not found", "is invalid (not owner)")
+ render_404("Group not found.")
end
end
def find_network_auth_owner
- begin
- @network = Network.find(params[:id], :include => [ :owner, :memberships ])
- unless @network.owner == current_user || current_user.admin?
- error("Group not found (id not authorized)", "is invalid (not group administrator)")
- end
- rescue ActiveRecord::RecordNotFound
- error("Group not found (id not authorized)", "is invalid (not group administrator)")
+ unless @network.owner == current_user || current_user.admin?
+ render_401("You must be the group owner to perform this action.")
end
end
def find_network_auth_admin
- if @network = Network.find_by_id(params[:id], :include => [ :owner, :memberships ])
- unless @network.administrator?(current_user.id)
- error("You must be a group administrator to invite people","")
- end
- else
- error("Group not found (id not authorized)", "is invalid (not owner)")
+ unless @network.administrator?(current_user.id)
+ render_401("You must be a group administrator to perform this action.")
end
end
-
-private
-
- def error(notice, message)
- flash[:error] = notice
- (err = Network.new.errors).add(:id, message)
-
- respond_to do |format|
- format.html { redirect_to networks_url }
- end
- end
end
Modified: trunk/app/controllers/oauth_controller.rb (3498 => 3499)
--- trunk/app/controllers/oauth_controller.rb 2013-04-11 12:26:59 UTC (rev 3498)
+++ trunk/app/controllers/oauth_controller.rb 2013-04-11 15:00:18 UTC (rev 3499)
@@ -174,12 +174,4 @@
render_404("Client Application not found")
end
end
-
- def error(notice, message, attr=:id)
- flash[:error] = notice
-
- respond_to do |format|
- format.html { redirect_to oauth_url }
- end
- end
end
Modified: trunk/app/controllers/packs_controller.rb (3498 => 3499)
--- trunk/app/controllers/packs_controller.rb 2013-04-11 12:26:59 UTC (rev 3498)
+++ trunk/app/controllers/packs_controller.rb 2013-04-11 15:00:18 UTC (rev 3499)
@@ -292,13 +292,11 @@
end
def edit_item
- if params[:entry_type].blank? or params[:entry_id].blank?
- error("Invalid item entry specified for editing", "")
- else
- @type = params[:entry_type].downcase
- @item_entry = find_entry(@pack.id, params[:entry_type], params[:entry_id])
+ @type = params[:entry_type].downcase
+ @item_entry = find_entry(@pack.id, params[:entry_type], params[:entry_id])
+ if @item_entry.nil?
+ render_404("Invalid item entry specified for editing.")
end
-
# Will render packs/new_item.rhtml
end
@@ -510,16 +508,7 @@
end
private
-
- def error(notice, message, attr=:id)
- flash[:error] = notice
- (err = Pack.new.errors).add(attr, message)
-
- respond_to do |format|
- format.html { redirect_to packs_url }
- end
- end
-
+
# This finds the specified entry within the specified pack (otherwise returns nil).
def find_entry(pack_id, entry_type, entry_id)
case entry_type.downcase
Modified: trunk/test/functional/group_policies_controller_test.rb (3498 => 3499)
--- trunk/test/functional/group_policies_controller_test.rb 2013-04-11 12:26:59 UTC (rev 3498)
+++ trunk/test/functional/group_policies_controller_test.rb 2013-04-11 15:00:18 UTC (rev 3499)
@@ -13,7 +13,7 @@
def test_non_admins_cannot_view
login_as(:jane)
get :index, :network_id => networks(:exclusive_network).id
- assert_response :redirect
+ assert_response :unauthorized
end
def test_can_create