Mailinglist Archive: obs-commits (345 mails)

< Previous Next >
[obs-commits] [PATCH] [webui] Use Ajax POST request again.
From: Sascha Peilicke <saschpe@xxxxxxx>

Silently closing potential CSRF vulnerabilities...
---
src/webui/app/controllers/package_controller.rb | 3 +++
src/webui/app/controllers/project_controller.rb | 4 ++++
src/webui/app/controllers/request_controller.rb | 1 +
src/webui/app/views/request/show.html.erb | 15 +++++++++------
.../app/views/shared/_involved_users.html.erb | 13 ++++++++-----
5 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/src/webui/app/controllers/package_controller.rb
b/src/webui/app/controllers/package_controller.rb
index 034567e..f85a0e2 100644
--- a/src/webui/app/controllers/package_controller.rb
+++ b/src/webui/app/controllers/package_controller.rb
@@ -700,6 +700,7 @@ class PackageController < ApplicationController
end

def save_person
+ valid_http_methods :post
if not valid_role_name? params[:userid]
flash[:error] = "Invalid username: #{params[:userid]}"
redirect_to :action => :add_person, :project => @project, :package =>
@package, :role => params[:role]
@@ -721,6 +722,7 @@ class PackageController < ApplicationController
end

def save_group
+ valid_http_methods :post
#FIXME: API Group controller routes don't support this currently.
#group = find_cached(Group, params[:groupid])
group = Group.list(params[:groupid])
@@ -751,6 +753,7 @@ class PackageController < ApplicationController
end

def remove_group
+ valid_http_methods :post
if params[:groupid].blank?
flash[:note] = "Group removal aborted, no group id given!"
redirect_to :action => :show, :project => params[:project] and return
diff --git a/src/webui/app/controllers/project_controller.rb
b/src/webui/app/controllers/project_controller.rb
index 9d97ae7..9611d10 100644
--- a/src/webui/app/controllers/project_controller.rb
+++ b/src/webui/app/controllers/project_controller.rb
@@ -718,6 +718,7 @@ class ProjectController < ApplicationController
end

def save_person
+ valid_http_methods :post
user = find_cached(Person, params[:userid])
# FIXME/PITA: For invalid input, the lovely API person controller does a
'LIKE' SQL search to still return data.
# This leads to a valid Person model instance with no 'login' set.
Instead, it contains a list of _all_ users.
@@ -736,6 +737,7 @@ class ProjectController < ApplicationController
end

def save_group
+ valid_http_methods :post
#FIXME: API Group controller routes don't support this currently.
#group = find_cached(Group, params[:groupid])
group = Group.list(params[:groupid])
@@ -755,6 +757,7 @@ class ProjectController < ApplicationController
end

def remove_person
+ valid_http_methods :post
if params[:userid].blank?
flash[:note] = "User removal aborted, no user id given!"
redirect_to :action => :show, :project => params[:project] and return
@@ -769,6 +772,7 @@ class ProjectController < ApplicationController
end

def remove_group
+ valid_http_methods :post
if params[:groupid].blank?
flash[:note] = "Group removal aborted, no group id given!"
redirect_to :action => :show, :project => params[:project] and return
diff --git a/src/webui/app/controllers/request_controller.rb
b/src/webui/app/controllers/request_controller.rb
index 13ce5b6..30c9222 100644
--- a/src/webui/app/controllers/request_controller.rb
+++ b/src/webui/app/controllers/request_controller.rb
@@ -27,6 +27,7 @@ class RequestController < ApplicationController
end

def modify_review
+ valid_http_methods :post
begin
BsRequest.modifyReview(params[:id], params[:new_state], params)
render :text => params[:new_state]
diff --git a/src/webui/app/views/request/show.html.erb
b/src/webui/app/views/request/show.html.erb
index 9d4072d..95536b2 100644
--- a/src/webui/app/views/request/show.html.erb
+++ b/src/webui/app/views/request/show.html.erb
@@ -119,17 +119,20 @@
<% content_for :head_javascript do %>
function modifyReview(id, new_state) {
var comment = $('#review_comment_' + id).attr('value');
- var path = "/request/modify_review/<%= @id %>?new_state=" +
encodeURIComponent(new_state) + "&comment=" + encodeURIComponent(comment);
+ var data = {new_state: new_state, comment: comment};
if ($('#review_type_' + id).text() == "package") {
project_and_package = $('#review_name_' + id).text().split(' / ');
- path += "&project=" +
encodeURIComponent(project_and_package[0].trim());
- path += "&package=" +
encodeURIComponent(project_and_package[1].trim());
+ data['project'] = project_and_package[0].trim();
+ data['package'] = project_and_package[1].trim();
} else if ($('#review_type_' + id).text() == "user") {
- path += "&" + $('#review_type_' + id).text() + "=" +
encodeURIComponent($('#review_name_' + id + '_raw').text());
+ data[$('#review_type_' + id).text()] = $('#review_name_' + id +
'_raw').text();
} else {
- path += "&" + $('#review_type_' + id).text() + "=" +
encodeURIComponent($('#review_name_' + id).text());
+ data[$('#review_type_' + id).text()] = $('#review_name_' + id).text();
}
- $.ajax({url: path,
+ $.ajax({
+ url: '<%= url_for(:controller => 'request', :action =>
'modify_review', :id => @id) %>',
+ type: 'POST',
+ data: data,
success: function(data) {
$('#review_table td#review_state_' + id).text(data);
$('#review_table td#review_buttons_' + id).remove();
diff --git a/src/webui/app/views/shared/_involved_users.html.erb
b/src/webui/app/views/shared/_involved_users.html.erb
index 9074ad8..712e6dd 100644
--- a/src/webui/app/views/shared/_involved_users.html.erb
+++ b/src/webui/app/views/shared/_involved_users.html.erb
@@ -11,17 +11,19 @@
type = str.slice(0, str.indexOf('_')); str = str.slice(str.indexOf('_')
+ 1);
role = str.slice(0, str.indexOf('_')); str = str.slice(str.indexOf('_')
+ 1);

+ data = {project: '<%= @project %>', package: '<%= @package %>', role:
role};
+ data[type + 'id'] = str;
if (type == 'user') {
if ($(this).attr('checked')) {
- url = '<%= url_for(:action => "save_person", :project => @project,
:package => @package) %>';
+ url = '<%= url_for(:action => "save_person") %>';
} else {
- url = '<%= url_for(:action => "remove_person", :project => @project,
:package => @package) %>';
+ url = '<%= url_for(:action => "remove_person") %>';
}
} else if (type == 'group') {
if ($(this).attr('checked')) {
- url = '<%= url_for(:action => "save_group", :project => @project,
:package => @package) %>';
+ url = '<%= url_for(:action => "save_group") %>';
} else {
- url = '<%= url_for(:action => "remove_group", :project => @project,
:package => @package) %>';
+ url = '<%= url_for(:action => "remove_group") %>';
}
}

@@ -29,9 +31,10 @@
$('#' + type + '_table input').animate({opacity: 0.2}, 500);
$('#' + type + '_table input').disable();

- url += ';' + type + 'id=' + str + ';role=' + role;
$.ajax({
url: url,
+ type: 'POST',
+ data: data,
complete: function() {
$('#' + type + '_spinner').hide();
$('#' + type + '_table input').animate({opacity: 1}, 200);
--
1.7.7

--
To unsubscribe, e-mail: obs-commits+unsubscribe@xxxxxxxxxxxx
To contact the owner, e-mail: obs-commits+owner@xxxxxxxxxxxx

< Previous Next >
This Thread
  • No further messages