Added a confirmation step to the process of granting and revoking user roles.

[rails.git] / app / controllers / user_roles_controller.rb

diff --git a/app/controllers/user_roles_controller.rb b/app/controllers/user_roles_controller.rb
index b1f24c275432c7e70167169ef68de0624e7f9bea..7e56693df5c4c44ba362183e7717da4f99c7e2ee 100644 (file)
--- a/app/controllers/user_roles_controller.rb
+++ b/app/controllers/user_roles_controller.rb
@@ -6,23 +6,35 @@ class UserRolesController < ApplicationController
   before_filter :require_administrator
 
   def grant
-    this_user = User.find_by_display_name(params[:display_name], :conditions => {:visible => true})
-    if this_user and UserRole::ALL_ROLES.include? params[:role]
-      this_user.roles.create(:role => params[:role])
+    # added a random nonce here which isn't predictable, making an CSRF procedure much, much more difficult.
+    if params[:nonce] and params[:nonce] == session[:nonce]
+      this_user = User.find_by_display_name(params[:display_name], :conditions => {:visible => true})
+      if this_user and UserRole::ALL_ROLES.include? params[:role]
+        this_user.roles.create(:role => params[:role])
+        redirect_to :controller => 'user', :action => 'view', :display_name => params[:display_name]
+      else
+        flash[:notice] = t('user_role.grant.fail', :role => params[:role], :name => params[:display_name])
+      end
     else
-      flash[:notice] = t('user_role.grant.fail', :role => params[:role], :name => params[:display_name])
+      @nonce = OAuth::Helper.generate_nonce
+      session[:nonce] = @nonce
     end
-    redirect_to :controller => 'user', :action => 'view', :display_name => params[:display_name]
   end
 
   def revoke
-    this_user = User.find_by_display_name(params[:display_name], :conditions => {:visible => true})
-    if this_user and UserRole::ALL_ROLES.include? params[:role]
-      UserRole.delete_all({:user_id => this_user.id, :role => params[:role]})
+    # added a random nonce here which isn't predictable, making an CSRF procedure much, much more difficult.
+    if params[:nonce] and params[:nonce] == session[:nonce]
+      this_user = User.find_by_display_name(params[:display_name], :conditions => {:visible => true})
+      if this_user and UserRole::ALL_ROLES.include? params[:role]
+        UserRole.delete_all({:user_id => this_user.id, :role => params[:role]})
+        redirect_to :controller => 'user', :action => 'view', :display_name => params[:display_name]
+      else
+        flash[:notice] = t('user_role.revoke.fail', :role => params[:role], :name => params[:display_name])
+      end
     else
-      flash[:notice] = t('user_role.revoke.fail', :role => params[:role], :name => params[:display_name])
+      @nonce = OAuth::Helper.generate_nonce
+      session[:nonce] = @nonce
     end
-    redirect_to :controller => 'user', :action => 'view', :display_name => params[:display_name]
   end
 
   private