Huis Verkennen Help
Registreer Inloggen
J1X
/
huginn
Volgen 2
Ster 0
Vork 0

Bladeren bron
better handling of return paths, and more specs

Andrew Cantino 11 jaren geleden
bovenliggende
8636c523b0
commit
b25c38c45e
6 gewijzigde bestanden met toevoegingen van 123 en 25 verwijderingen
Toon Diff Stats
  1. 37 20
      app/controllers/agents_controller.rb
  2. 2 2
      app/views/agents/_action_menu.html.erb
  3. 1 1
      app/views/agents/_table.html.erb
  4. 1 1
      app/views/agents/index.html.erb
  5. 1 1
      app/views/scenarios/show.html.erb
  6. 81 0
      spec/controllers/agents_controller_spec.rb

+ 37 - 20
app/controllers/agents_controller.rb
Bestand weergeven 

@@ -21,12 +21,12 @@ class AgentsController < ApplicationController
21 21 
   end
22 22
23 23 
   def run
24 
-    agent = current_user.agents.find(params[:id])
25 
-    Agent.async_check(agent.id)
26 
-    if params[:return] == "show"
27 
-      redirect_to agent_path(agent), notice: "Agent run queued"
28 
-    else
29 
-      redirect_to agents_path, notice: "Agent run queued"
24 
+    @agent = current_user.agents.find(params[:id])
25 
+    Agent.async_check(@agent.id)
26 
+
27 
+    respond_to do |format|
28 
+      format.html { redirect_back "Agent run queued for '#{@agent.name}'" }
29 
+      format.json { head :ok }
30 30 
     end
31 31 
   end
32 32 
 
@@ -53,12 +53,20 @@ class AgentsController < ApplicationController
53 53 
   def remove_events
54 54 
     @agent = current_user.agents.find(params[:id])
55 55 
     @agent.events.delete_all
56 
-    redirect_to agents_path, notice: "All events removed"
56 
+
57 
+    respond_to do |format|
58 
+      format.html { redirect_back "All emitted events removed for '#{@agent.name}'" }
59 
+      format.json { head :ok }
60 
+    end
57 61 
   end
58 62
59 63 
   def propagate
60 
-    details = Agent.receive!
61 
-    redirect_to agents_path, notice: "Queued propagation calls for #{details[:event_count]} event(s) on #{details[:agent_count]} agent(s)"
64 
+    details = Agent.receive! # Eventually this should probably be scoped to the current_user.
65 
+
66 
+    respond_to do |format|
67 
+      format.html { redirect_back "Queued propagation calls for #{details[:event_count]} event(s) on #{details[:agent_count]} agent(s)" }
68 
+      format.json { head :ok }
69 
+    end
62 70 
   end
63 71
64 72 
   def show
 
@@ -99,8 +107,8 @@ class AgentsController < ApplicationController
99 107 
                                   params[:agent])
100 108 
     respond_to do |format|
101 109 
       if @agent.save
102 
-        format.html { redirect_to agents_path, notice: 'Your Agent was successfully created.' }
103 
-        format.json { render json: @agent, status: :created, location: @agent }
110 
+        format.html { redirect_back "'#{@agent.name}' was successfully created." }
111 
+        format.json { render json: @agent, status: :ok, location: agent_path(@agent) }
104 112 
       else
105 113 
         format.html { render action: "new" }
106 114 
         format.json { render json: @agent.errors, status: :unprocessable_entity }
 
@@ -113,14 +121,8 @@ class AgentsController < ApplicationController
113 121
114 122 
     respond_to do |format|
115 123 
       if @agent.update_attributes(params[:agent])
116 
-        format.html {
117 
-          if params[:return] == "show"
118 
-            redirect_to agent_path(@agent), notice: 'Your Agent was successfully updated.'
119 
-          else
120 
-            redirect_to agents_path, notice: 'Your Agent was successfully updated.'
121 
-          end
122 
-        }
123 
-        format.json { head :no_content }
124 
+        format.html { redirect_back "'#{@agent.name}' was successfully updated." }
125 
+        format.json { render json: @agent, status: :ok, location: agent_path(@agent) }
124 126 
       else
125 127 
         format.html { render action: "edit" }
126 128 
         format.json { render json: @agent.errors, status: :unprocessable_entity }
 
@@ -133,8 +135,23 @@ class AgentsController < ApplicationController
133 135 
     @agent.destroy
134 136
135 137 
     respond_to do |format|
136 
-      format.html { redirect_to agents_path }
138 
+      format.html { redirect_back "'#{@agent.name}' deleted" }
137 139 
       format.json { head :no_content }
138 140 
     end
139 141 
   end
142 
+
143 
+  protected
144 
+
145 
+  # Sanitize params[:return] to prevent open redirect attacks, a common security issue.
146 
+  def redirect_back(message)
147 
+    if params[:return] == "show" && @agent
148 
+      path = agent_path(@agent)
149 
+    elsif params[:return] =~ /\A#{Regexp::escape scenarios_path}\/\d+\Z/
150 
+      path = params[:return]
151 
+    else
152 
+      path = agents_path
153 
+    end
154 
+
155 
+    redirect_to path, notice: message
156 
+  end
140 157 
 end

+ 2 - 2
app/views/agents/_action_menu.html.erb
Bestand weergeven 

@@ -31,11 +31,11 @@
31 31
32 32 
   <% if agent.can_create_events? && agent.events.count > 0 %>
33 33 
     <li>
34 
-      <%= link_to '<span class="color-danger glyphicon glyphicon-trash"></span> Delete all events'.html_safe, remove_events_agent_path(agent), method: :delete, data: {confirm: 'Are you sure you want to delete ALL emitted events for this Agent?'}, :tabindex => "-1" %>
34 
+      <%= link_to '<span class="color-danger glyphicon glyphicon-trash"></span> Delete all events'.html_safe, remove_events_agent_path(agent, :return => returnTo), method: :delete, data: {confirm: 'Are you sure you want to delete ALL emitted events for this Agent?'}, :tabindex => "-1" %>
35 35 
     </li>
36 36 
   <% end %>
37 37
38 38 
   <li>
39 
-    <%= link_to '<span class="color-danger glyphicon glyphicon-remove"></span> Delete agent'.html_safe, agent_path(agent), method: :delete, data: { confirm: 'Are you sure that you want to permanently delete this Agent?' }, :tabindex => "-1" %>
39 
+    <%= link_to '<span class="color-danger glyphicon glyphicon-remove"></span> Delete agent'.html_safe, agent_path(agent, :return => returnTo), method: :delete, data: { confirm: 'Are you sure that you want to permanently delete this Agent?' }, :tabindex => "-1" %>
40 40 
   </li>
41 41 
 </ul>

+ 1 - 1
app/views/agents/_table.html.erb
Bestand weergeven 

@@ -64,7 +64,7 @@
64 64 
             <button type="button" class="btn btn-default btn-sm dropdown-toggle" data-toggle="dropdown">
65 65 
               <span class="glyphicon glyphicon-th-list"></span> Actions <span class="caret"></span>
66 66 
             </button>
67 
-            <%= render 'agents/action_menu', :agent => agent, :returnTo => "index" %>
67 
+            <%= render 'agents/action_menu', :agent => agent, :returnTo => (defined?(returnTo) && returnTo) || "index" %>
68 68 
           </div>
69 69 
         </td>
70 70 
       </tr>

+ 1 - 1
app/views/agents/index.html.erb
Bestand weergeven 

@@ -5,7 +5,7 @@
5 5 
         <h2>Your Agents</h2>
6 6 
       </div>
7 7
8 
-      <%= render :partial => 'agents/table' %>
8 
+      <%= render 'agents/table' %>
9 9
10 10 
       <br/>
11 11

+ 1 - 1
app/views/scenarios/show.html.erb
Bestand weergeven 

@@ -15,7 +15,7 @@
15 15 
         <h3>Agents</h3>
16 16 
       </div>
17 17
18 
-      <%= render :partial => 'agents/table' %>
18 
+      <%= render 'agents/table', :returnTo => scenario_path(@scenario) %>
19 19 
     </div>
20 20 
   </div>
21 21 
 </div>

+ 81 - 0
spec/controllers/agents_controller_spec.rb
Bestand weergeven 

@@ -34,6 +34,47 @@ describe AgentsController do
34 34 
     end
35 35 
   end
36 36
37 
+  describe "POST run" do
38 
+    it "triggers Agent.async_check with the Agent's ID" do
39 
+      sign_in users(:bob)
40 
+      mock(Agent).async_check(agents(:bob_manual_event_agent).id)
41 
+      post :run, :id => agents(:bob_manual_event_agent).to_param
42 
+    end
43 
+
44 
+    it "can only be accessed by the Agent's owner" do
45 
+      sign_in users(:jane)
46 
+      lambda {
47 
+        post :run, :id => agents(:bob_manual_event_agent).to_param
48 
+      }.should raise_error(ActiveRecord::RecordNotFound)
49 
+    end
50 
+  end
51 
+
52 
+  describe "POST remove_events" do
53 
+    it "deletes all events created by the given Agent" do
54 
+      sign_in users(:bob)
55 
+      agent_event = events(:bob_website_agent_event).id
56 
+      other_event = events(:jane_website_agent_event).id
57 
+      post :remove_events, :id => agents(:bob_website_agent).to_param
58 
+      Event.where(:id => agent_event).count.should == 0
59 
+      Event.where(:id => other_event).count.should == 1
60 
+    end
61 
+
62 
+    it "can only be accessed by the Agent's owner" do
63 
+      sign_in users(:jane)
64 
+      lambda {
65 
+        post :remove_events, :id => agents(:bob_website_agent).to_param
66 
+      }.should raise_error(ActiveRecord::RecordNotFound)
67 
+    end
68 
+  end
69 
+
70 
+  describe "POST propagate" do
71 
+    it "runs event propagation for all Agents" do
72 
+      sign_in users(:bob)
73 
+      mock.proxy(Agent).receive!
74 
+      post :propagate
75 
+    end
76 
+  end
77 
+
37 78 
   describe "GET show" do
38 79 
     it "only shows Agents for the current user" do
39 80 
       sign_in users(:bob)
 
@@ -152,6 +193,14 @@ describe AgentsController do
152 193 
       }.should raise_error(ActiveRecord::RecordNotFound)
153 194 
     end
154 195
196 
+    it "accepts JSON requests" do
197 
+      sign_in users(:bob)
198 
+      post :update, :id => agents(:bob_website_agent).to_param, :agent => valid_attributes(:name => "New name"), :format => :json
199 
+      agents(:bob_website_agent).reload.name.should == "New name"
200 
+      JSON.parse(response.body)['name'].should == "New name"
201 
+      response.should be_success
202 
+    end
203 
+
155 204 
     it "will not accept Agent sources owned by other users" do
156 205 
       sign_in users(:bob)
157 206 
       post :update, :id => agents(:bob_website_agent).to_param, :agent => valid_attributes(:source_ids => [agents(:jane_weather_agent).id])
 
@@ -164,6 +213,38 @@ describe AgentsController do
164 213 
       assigns(:agent).should have(1).errors_on(:name)
165 214 
       response.should render_template("edit")
166 215 
     end
216 
+
217 
+    describe "redirecting back" do
218 
+      before do
219 
+        sign_in users(:bob)
220 
+      end
221 
+
222 
+      it "can redirect back to the show path" do
223 
+        post :update, :id => agents(:bob_website_agent).to_param, :agent => valid_attributes(:name => "New name"), :return => "show"
224 
+        response.should redirect_to(agent_path(agents(:bob_website_agent)))
225 
+      end
226 
+
227 
+      it "redirect back to the index path by default" do
228 
+        post :update, :id => agents(:bob_website_agent).to_param, :agent => valid_attributes(:name => "New name")
229 
+        response.should redirect_to(agents_path)
230 
+      end
231 
+
232 
+      it "accepts return paths to scenarios" do
233 
+        post :update, :id => agents(:bob_website_agent).to_param, :agent => valid_attributes(:name => "New name"), :return => "/scenarios/2"
234 
+        response.should redirect_to("/scenarios/2")
235 
+      end
236 
+
237 
+      it "sanitizes return paths" do
238 
+        post :update, :id => agents(:bob_website_agent).to_param, :agent => valid_attributes(:name => "New name"), :return => "/scenar"
239 
+        response.should redirect_to(agents_path)
240 
+
241 
+        post :update, :id => agents(:bob_website_agent).to_param, :agent => valid_attributes(:name => "New name"), :return => "http://google.com"
242 
+        response.should redirect_to(agents_path)
243 
+
244 
+        post :update, :id => agents(:bob_website_agent).to_param, :agent => valid_attributes(:name => "New name"), :return => "javascript:alert(1)"
245 
+        response.should redirect_to(agents_path)
246 
+      end
247 
+    end
167 248 
   end
168 249
169 250 
   describe "DELETE destroy" do