myexperiment-hackers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[myexperiment-hackers] [3498] trunk/app: Second pass of adding proper HT


From: noreply
Subject: [myexperiment-hackers] [3498] trunk/app: Second pass of adding proper HTTP error codes
Date: Thu, 11 Apr 2013 12:27:00 +0000 (UTC)

Revision
3498
Author
fbacall
Date
2013-04-11 12:26:59 +0000 (Thu, 11 Apr 2013)

Log Message

Second pass of adding proper HTTP error codes

Modified Paths

Diff

Modified: trunk/app/controllers/blobs_controller.rb (3497 => 3498)


--- trunk/app/controllers/blobs_controller.rb	2013-04-10 14:38:02 UTC (rev 3497)
+++ trunk/app/controllers/blobs_controller.rb	2013-04-11 12:26:59 UTC (rev 3498)
@@ -287,19 +287,17 @@
   
   # POST /files/1;rate
   def rate
-    if @blob.contributor_type == 'User' and @blob.contributor_id == current_user.id
-      error("You cannot rate your own file!", "")
-    else
+    unless @blob.contributor_type == 'User' and @blob.contributor_id == current_user.id
       Rating.delete_all(["rateable_type = ? AND rateable_id = ? AND user_id = ?", @blob.class.to_s, @blob.id, current_user.id])
-      
       Rating.create(:rateable => @blob, :user => current_user, :rating => params[:rating])
       
       respond_to do |format|
-        format.html { 
+        format.html do
           render :update do |page|
             page.replace_html "ratings_inner", :partial => "contributions/ratings_box_inner", :locals => { :contributable => @blob, :controller_name => controller.controller_name }
             page.replace_html "ratings_breakdown", :partial => "contributions/ratings_box_breakdown", :locals => { :contributable => @blob }
-          end }
+          end
+        end
       end
     end
   end
@@ -473,15 +471,4 @@
       render_401("You are not authorised to manage this file.") unless @blob.owner?(current_user)
     end
   end
-  
-  private
-  
-  def error(notice, message, attr=:id)
-    flash[:error] = notice
-     (err = Blob.new.errors).add(attr, message)
-    
-    respond_to do |format|
-      format.html { redirect_to blobs_url }
-    end
-  end
 end

Modified: trunk/app/controllers/bookmarks_controller.rb (3497 => 3498)


--- trunk/app/controllers/bookmarks_controller.rb	2013-04-10 14:38:02 UTC (rev 3497)
+++ trunk/app/controllers/bookmarks_controller.rb	2013-04-11 12:26:59 UTC (rev 3498)
@@ -37,21 +37,8 @@
 protected
 
   def find_bookmark_auth
-    begin
-      @bookmark = Bookmark.find(params[:id], :conditions => ["user_id = ?", current_user.id])
-    rescue ActiveRecord::RecordNotFound
-      error("Bookmark not found", "is invalid")
+    if (@bookmark = Bookmark.find_by_id(params[:id], :conditions => ["user_id = ?", current_user.id])).nil?
+      render_404("Bookmark not found.")
     end
   end
-  
-private
-
-  def error(notice, message, attr=:id)
-    flash[:error] = notice
-    (err = Bookmark.new.errors).add(attr, message)
-    
-    respond_to do |format|
-      format.html { redirect_to bookmarks_url }
-    end
-  end
 end

Modified: trunk/app/controllers/citations_controller.rb (3497 => 3498)


--- trunk/app/controllers/citations_controller.rb	2013-04-10 14:38:02 UTC (rev 3497)
+++ trunk/app/controllers/citations_controller.rb	2013-04-11 12:26:59 UTC (rev 3498)
@@ -6,17 +6,19 @@
 class CitationsController < ApplicationController
   before_filter :login_required, :except => [ :index, :show ]
   
-  before_filter :find_workflow_auth
+  before_filter :find_workflow
+  before_filter :auth_view_workflow, : [:index, :show]
+  before_filter :auth_edit_workflow, : :create
+  before_filter :find_citation, : [:show, :edit, :update, :destroy ]
+  before_filter :auth_citation, : [:edit, :update, :destroy ]
   
-  before_filter :find_citations, : :index
-  before_filter :find_citation, : :show
-  before_filter :find_citation_auth, : [ :edit, :update, :destroy ]
-  
   # declare sweepers and which actions should invoke them
   cache_sweeper :citation_sweeper, : [ :create, :update, :destroy ]
   
   # GET /citations
   def index
+    @citations = @workflow.citations
+
     respond_to do |format|
       format.html # index.rhtml
     end
@@ -79,73 +81,33 @@
   
 protected
 
-  def find_workflow_auth
-    begin
-      # attempt to authenticate the user before you return the workflow
-      login_required if login_available?
-    
-      workflow = Workflow.find(params[:workflow_id])
-      
-      if Authorization.check((["index", "show"].include?(action_name) ? "view" : "edit"), workflow, current_user)
-        @workflow = workflow
-        
-        # remove workflow data from workflow if the user is not authorized for download
-        @workflow.content_blob.data = "" unless Authorization.check("download", @workflow, current_user)
-      else
-        if logged_in?
-          error("Workflow not found (id not authorized)", "is invalid (not authorized)")
-        else
-          find_workflow_auth if login_required
-        end
-      end
-    rescue ActiveRecord::RecordNotFound
-      error("Workflow not found", "is invalid")
+  def find_workflow
+    if (@workflow = Workflow.find_by_id(params[:workflow_id])).nil?
+      render_404("Workflow not found.")
     end
   end
-  
-  def find_citations
-    if @workflow
-      @citations = @workflow.citations
-    else
-      @citations = []
+
+  def auth_view_workflow
+    unless Authorization.check("view", @workflow, current_user)
+      render_401("You are not authorized to view this workflow's citations.")
     end
   end
-  
-  def find_citation
-    if citation = @workflow.citations.find(:first, :conditions => ["id = ?", params[:id]])
-      @citation = citation
-    else
-      error("Citation not found", "is invalid", params[:id])
+
+  def auth_edit_workflow
+    unless Authorization.check("edit", @workflow, current_user)
+      render_401("You are not authorized to manage this workflow's citations.")
     end
   end
-  
-  def find_citation_auth
-    if citation = @workflow.citations.find(:first, :conditions => ["id = ? AND user_id = ?", params[:id], current_user.id])
-      @citation = citation
-    else
-      error("Citation not found (id not authorized)", "is invalid (not authorized)", params[:id])
+
+  def find_citation
+    if (@citation = @workflow.citations.find(:first, :conditions => ["id = ?", params[:id]])).nil?
+      render_404("Citation not found.")
     end
   end
   
-private
-
-  def error(notice, message, attr=nil)
-    flash[:error] = notice
-
-    workflow_id_attr = attr
-    workflow_id_attr = :id if workflow_id_attr.nil?
-
-    (err = Citation.new.errors).add(workflow_id_attr, message)
-
-    respond_to do |format|
-      format.html {
-        if attr
-          redirect_to workflow_citations_url(params[:workflow_id])
-        else
-          redirect_to workflows_url
-        end
-      }
+  def auth_citation
+    unless @citation.user == current_user
+      render_401("You are not authorized to #{action_name} this citation.")
     end
   end
-  
 end

Modified: trunk/app/controllers/content_types_controller.rb (3497 => 3498)


--- trunk/app/controllers/content_types_controller.rb	2013-04-10 14:38:02 UTC (rev 3497)
+++ trunk/app/controllers/content_types_controller.rb	2013-04-11 12:26:59 UTC (rev 3498)
@@ -6,6 +6,7 @@
 class ContentTypesController < ApplicationController
 
   before_filter :find_content_type, : [ :show, :edit, :update ]
+  before_filter :auth_content_type, : [ :edit, :update ]
 
   # GET /content_types
   def index
@@ -54,12 +55,6 @@
 
   # PUT /content_types/1
   def update
-
-    if !Authorization.check('edit', @content_type, current_user)
-      error("You do not have the authorisation to edit.", "is unauthorised")
-      return
-    end
-
     @content_type.title       = params[:content_type][:title]
     @content_type.description = params[:content_type][:description]
 
@@ -78,17 +73,13 @@
     @content_type = ContentType.find_by_id(params[:id])
 
     if @content_type.nil?
-      error("Content type not found", "is invalid")
+      render_404("Content type not found.")
     end
   end
 
-  def error(notice, message, attr=:id)
-    flash[:error] = notice
-     (err = ContentType.new.errors).add(attr, message)
-    
-    respond_to do |format|
-      format.html { redirect_to content_types_url }
+  def auth_content_type
+    if !Authorization.check('edit', @content_type, current_user)
+      render_401("You are not authorised to edit this content type.")
     end
   end
 end
-

Modified: trunk/app/controllers/contributions_controller.rb (3497 => 3498)


--- trunk/app/controllers/contributions_controller.rb	2013-04-10 14:38:02 UTC (rev 3497)
+++ trunk/app/controllers/contributions_controller.rb	2013-04-11 12:26:59 UTC (rev 3498)
@@ -14,31 +14,23 @@
 private
 
   def get_context
-    begin
+    # Determine the class name of the model
+    klass_name = params[:contributable_type].singularize.camelize
 
-      # Determine the class name of the model
-      klass_name = params[:contributable_type].singularize.camelize
-
-      # Process model aliases (e.g. File => Blob)
-      klass_name = Conf.model_aliases[klass_name] if Conf.model_aliases[klass_name]
-
+    # Process model aliases (e.g. File => Blob)
+    klass_name = Conf.model_aliases[klass_name] if Conf.model_aliases[klass_name]
+    begin
       @contributable = Object.const_get(klass_name).find_by_id(params[:contributable_id])
-
-      # Abort if the contributable does not exist
-      return error if @contributable.nil?
-
-      # Abort if we're not allowed to see this contributable
-      return error unless Authorization.check('view', @contributable, current_user)
-
     rescue
+      @contributable = nil
+    end
 
-      # In case the const_get doesn't find anything
-      return error
+    # Abort if the contributable does not exist
+    if @contributable.nil?
+      render_401("You are not authorized to view this resource.")
+    elsif !Authorization.check('view', @contributable, current_user)
+      # Abort if we're not allowed to see this contributable
+      render_401("You are not authorized to view this resource.")
     end
   end
-
-  def error
-    render :text => 'Error.'
-  end
 end
-

Modified: trunk/app/controllers/experiments_controller.rb (3497 => 3498)


--- trunk/app/controllers/experiments_controller.rb	2013-04-10 14:38:02 UTC (rev 3497)
+++ trunk/app/controllers/experiments_controller.rb	2013-04-11 12:26:59 UTC (rev 3498)
@@ -125,23 +125,12 @@
       "update"  => "edit"
     }
 
-    experiment = Experiment.find(:first, :conditions => ["id = ?", params[:id]])
-    
-    if experiment and Authorization.check(action_permissions[action_name], experiment, current_user)
-      @experiment = experiment
-    else
-      error("Experiment not found or action not authorized", "is invalid (not authorized)")
-    end
-  end
-  
-private
+    @experiment = Experiment.find(:first, :conditions => ["id = ?", params[:id]])
 
-  def error(notice, message, attr=:id)
-    flash[:error] = notice
-    (err = Experiment.new.errors).add(attr, message)
-    
-    respond_to do |format|
-      format.html { redirect_to experiments_url }
+    if @experiment.nil?
+      render_404("Experiment not found.")
+    elsif !Authorization.check(action_permissions[action_name], @experiment, current_user)
+      render_401("You are not authorized to #{action_name} this experiment.")
     end
   end
 end

Modified: trunk/app/controllers/friendships_controller.rb (3497 => 3498)


--- trunk/app/controllers/friendships_controller.rb	2013-04-10 14:38:02 UTC (rev 3497)
+++ trunk/app/controllers/friendships_controller.rb	2013-04-11 12:26:59 UTC (rev 3498)
@@ -8,7 +8,7 @@
   
   before_filter :check_user_present # only allow actions on friendships as on nested resources
   
-  before_filter :find_friendships, : [:index]
+  before_filter :find_user_auth, : [:index]
   before_filter :find_friendship_auth, : [:show, :accept, :edit, :update, :destroy]
 
   # declare sweepers and which actions should invoke them
@@ -41,16 +41,19 @@
     respond_to do |format|
       if @friendship.accept!
         flash[:notice] = 'Friendship was successfully accepted.'
-        format.html { redirect_to user_friendships_url(current_user.id) }
       else
-        error("Friendship already accepted", "already accepted")
+        flash[:error] = "Friendship already accepted."
       end
+
+      format.html { redirect_to user_friendships_url(current_user.id) }
     end
   end
   
   # GET /users/1/friendships
   # GET /friendships
   def index
+    @friendships = @user.friendships
+
     respond_to do |format|
       format.html # index.rhtml
     end
@@ -89,8 +92,23 @@
   # POST /users/1/friendships
   # POST /friendships
   def create
-    friendship_already_exists = Friendship.find_by_user_id_and_friend_id(params[:friendship][:user_id], params[:friendship][:friend_id]) || Friendship.find_by_user_id_and_friend_id(params[:friendship][:friend_id], params[:friendship][:user_id])
-    if (@friendship = Friendship.new(params[:friendship]) unless friendship_already_exists )
+    params[:friendship][:user_id] = current_user.id
+
+    friendship_already_exists =
+        Friendship.find_by_user_id_and_friend_id(params[:friendship][:user_id], params[:friendship][:friend_id]) ||
+        Friendship.find_by_user_id_and_friend_id(params[:friendship][:friend_id], params[:friendship][:user_id])
+    if friendship_already_exists
+      respond_to do |format|
+        flash[:error] = "Friendship not created (already exists)."
+        format.html { redirect_to new_user_friendship_url(current_user.id) }
+      end
+    elsif params[:friendship][:friend_id] == params[:friendship][:user_id]
+      respond_to do |format|
+        flash[:error] = "You cannot add yourself as a friend."
+        format.html { redirect_to new_user_friendship_url(current_user.id) }
+      end
+    else
+      @friendship = Friendship.new(params[:friendship])
       # set initial datetime
       @friendship.accepted_at = nil
       if @friendship.message.blank?
@@ -114,8 +132,6 @@
           format.html { render :action ="" "new" }
         end
       end
-    else
-      error("Friendship not created (already exists)", "not created, already exists")
     end
   end
 
@@ -189,51 +205,22 @@
     end
   end
 
-  def find_friendships
-    if params[:user_id].to_i == current_user.id.to_i
-      begin
-        @user = User.find(params[:user_id])
-    
-        @friendships = @user.friendships
-      rescue ActiveRecord::RecordNotFound
-        error("User not found", "is invalid", :user_id)
-      end
-    else
-      error("You are not authorised to view other users' friendships", "")
-    end
-  end
+  def find_user_auth
+    @user = User.find_by_id(params[:user_id])
 
-  def find_friendship
-    if params[:user_id]
-      begin
-        @user = User.find(params[:user_id])
-    
-        begin
-          @friendship = Friendship.find(params[:id], :conditions => ["friend_id = ?", @user.id])
-        rescue ActiveRecord::RecordNotFound
-          error("Friendship not found", "is invalid")
-        end
-      rescue ActiveRecord::RecordNotFound
-        error("User not found", "is invalid", :user_id)
-      end
-    else
-      begin
-        @friendship = Friendship.find(params[:id])
-      rescue ActiveRecord::RecordNotFound
-        error("Friendship not found", "is invalid")
-      end
+    if @user.nil?
+      render_404("User not found.")
+    elsif @user != current_user
+      render_401("You are not authorised to view other users' friendships.")
     end
   end
-  
+
   def find_friendship_auth
-    begin
-      begin
-        # find the friendship first
-        @friendship = Friendship.find(params[:id])
-      rescue ActiveRecord::RecordNotFound
-        raise ActiveRecord::RecordNotFound, "Friendship not found"
-      end
-      
+    # find the friendship first
+    @friendship = Friendship.find_by_id(params[:id])
+    if @friendship.nil?
+      render_404("Friendship 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
@@ -250,25 +237,10 @@
             not_auth = true
           end
       end
-      
       # check if we had any errors
       if not_auth
-        raise ActiveRecord::RecordNotFound, "You are not authorised to view other users' friendships"
+        render_401("You are not authorised to manage other users' friendships.")
       end
-      
-    rescue ActiveRecord::RecordNotFound => exc
-      error(exc.message, "")
     end
   end
-  
-private
-  
-  def error(notice, message)
-    flash[:error] = notice
-    (err = Friendship.new.errors).add(:id, message)
-    
-    respond_to do |format|
-      format.html { redirect_to user_friendships_url(current_user.id) }
-    end
-  end
 end

Modified: trunk/app/views/friendships/new.rhtml (3497 => 3498)


--- trunk/app/views/friendships/new.rhtml	2013-04-10 14:38:02 UTC (rev 3497)
+++ trunk/app/views/friendships/new.rhtml	2013-04-11 12:26:59 UTC (rev 3498)
@@ -3,7 +3,6 @@
 <%= error_messages_for :friendship %>
 
 <% form_for(:friendship, :url ="" user_friendships_path) do |f| %>
-  <%= f.hidden_field :user_id, :value => current_user.id %>
   <%= f.hidden_field :friend_id, :value => params[:user_id] %>
 	<% friend = User.find(params[:user_id]) %>
 	

reply via email to

[Prev in Thread] Current Thread [Next in Thread]