Created tests for backend_controller to cover security concerns, including #372 "user can add an action to another user's context via API". Modified backend_controller to close that hole and make the tests pass.

Added UserController#create to provide RESTful API for the admin user to create a new user in the system. This may be useful for the folks who have generously opened their Tracks installs to others. I have plans to document the RESTful API stuff at some point and write a Ruby wrapper.

Created a class method User.get_salt that wraps all calls to the SALT constant so that unit-tests can be always run with the default salt (I was previously needing to modify my environment.rb to run tests). 

Replaced usages of assert_success in tests with assert_response :success because assert_success is deprecated.



git-svn-id: http://www.rousette.org.uk/svn/tracks-repos/trunk@313 a4c988fc-2ded-0310-b66e-134b36920a42

tracks/app/controllers/backend_controller.rb

@@ -4,9 +4,8 @@ class BackendController < ApplicationController
   web_service_scaffold :invoke
   
   def new_todo(username, token, context_id, description)
-    if !check_token_against_user_word(username, token)
-      raise "invalid token"
-    end
+    check_token_against_user_word(username, token)
+    check_context_belongs_to_user(context_id)
 
     item = @user.todos.build
     item.description = description
@@ -17,31 +16,34 @@ class BackendController < ApplicationController
   end
   
   def list_contexts(username, token)
-    if !check_token_against_user_word(username, token)
-      raise "invalid token"
-    end
+    check_token_against_user_word(username, token)
     
     @user.contexts
   end
   
   def list_projects(username, token)
-    if !check_token_against_user_word(username, token)
-      raise "invalid token"
-    end
+    check_token_against_user_word(username, token)
     
     @user.projects
   end
   
-  protected
+  private
 
     # Check whether the token in the URL matches the word in the User's table
     def check_token_against_user_word(username, token)
       @user = User.find_by_login( username )
       unless ( token == @user.word)
-        render :text => "Sorry, you don't have permission to perform this action."
-        return false
+        raise (InvalidToken, "Sorry, you don't have permission to perform this action.")
+      end
+    end
+    
+    def check_context_belongs_to_user(context_id)
+      unless @user.contexts.exists? context_id
+        raise (CannotAccessContext, "Cannot access a context that does not belong to this user.")
       end
-      true
     end
   
 end
+
+class InvalidToken < RuntimeError; end
+class CannotAccessContext < RuntimeError; end

tracks/app/controllers/user_controller.rb

@@ -1,6 +1,6 @@
 class UserController < ApplicationController
   layout 'standard'
-  before_filter :login_required
+  prepend_before_filter :login_required
   
   def index
     render_text "This will be our jumping-off point for managing user functions!"
@@ -10,6 +10,44 @@ class UserController < ApplicationController
     render_text "You'll only be allowed to go here if you're an administrator."
   end
   
+  verify  :method => :post,
+          :only => %w( create_user ),
+          :render => { :text => '403 Forbidden: Only POST requests on this resource are allowed.',
+                      :status => 403 }
+  
+  # Example usage: curl -H 'Accept: application/xml' -H 'Content-Type: application/xml'
+  #               -u admin:up2n0g00d
+  #               -d '<request><login>username</login><password>abc123</password></request>'
+  #               http://our.tracks.host/cpa/create_user
+  #
+  def create
+     admin = User.find_admin
+     #render_text "user is " + session["user_id"].to_s + " and admin is " + a.id.to_s
+      unless session["user_id"].to_i == admin.id.to_i
+        access_denied
+        return
+      end
+      unless request.content_type == "application/xml"
+        render_failure "Content Type must be application/xml."
+        return
+      end
+      unless check_create_user_params
+        render_failure "Expected post format is xml like so: <request><login>username</login><password>abc123</password></request>."
+        return
+      end
+      user = User.new(params[:request])
+      user.password_confirmation = params[:request][:password]
+      unless user.valid?
+        render_failure user.errors.full_messages.join(', ')
+        return
+      end
+      if user.save
+        render :text => "User created.", :status => 200
+      else
+        render_failure "Failed to create user."
+      end
+  end
+    
   def preferences
     @page_title = "TRACKS::Preferences"
     @prefs = @user.preferences
@@ -68,5 +106,21 @@ class UserController < ApplicationController
       return false
     end
   end
+
+  private
+  
+  def render_failure message, status = 404
+    render :text => message, :status => status
+  end
+  
+  def check_create_user_params
+    return false unless params.has_key?(:request)
+    return false unless params[:request].has_key?(:login)
+    return false if params[:request][:login].empty?
+    return false unless params[:request].has_key?(:password)
+    return false if params[:request][:password].empty?
+    return true
+  end
+  
   
 end

tracks/app/helpers/backend_helper.rb

@@ -0,0 +1,2 @@
+module BackendHelper
+end

tracks/app/models/user.rb

@@ -27,12 +27,15 @@ class User < ActiveRecord::Base
   def crypt_word
     write_attribute("word", self.class.sha1(login + Time.now.to_i.to_s + rand.to_s))
   end
+  
+  def self.get_salt
+    SALT
+  end
 
 protected
 
   def self.sha1(pass)
-    # SALT is set in RAILS_ROOT/config/environment.rb
-    Digest::SHA1.hexdigest("#{SALT}--#{pass}--")
+    Digest::SHA1.hexdigest("#{get_salt}--#{pass}--")
   end
 
   before_create :crypt_password, :crypt_word

tracks/test/fixtures/users.yml

@@ -2,7 +2,7 @@
 admin_user:
   id: 1
   login: admin
-  password: <%= Digest::SHA1.hexdigest("#{SALT}--abracadabra--") %>
+  password: <%= Digest::SHA1.hexdigest("#{User.get_salt()}--abracadabra--") %>
   word: <%= Digest::SHA1.hexdigest("adminSat Feb 25 17:14:00 GMT 20060.236961325863376") %>
   is_admin: true
   preferences:
@@ -17,7 +17,7 @@ admin_user:
 other_user:
   id: 2
   login: jane
-  password: <%= Digest::SHA1.hexdigest("#{SALT}--sesame--") %>
+  password: <%= Digest::SHA1.hexdigest("#{User.get_salt()}--sesame--") %>
   word: <%= Digest::SHA1.hexdigest("janeSun Feb 19 14:42:45 GMT 20060.408173979260027") %>
   is_admin: false
   preferences:

tracks/test/functional/backend_controller_test.rb

@@ -0,0 +1,38 @@
+require File.dirname(__FILE__) + '/../test_helper'
+require 'backend_controller'
+
+# Re-raise errors caught by the controller.
+class BackendController; def rescue_action(e) raise e end; end
+
+class BackendControllerTest < Test::Unit::TestCase
+  fixtures :users, :projects, :contexts, :todos, :notes
+  
+  def setup
+    @controller = BackendController.new
+    request, response = ActionController::TestRequest.new, ActionController::TestResponse.new
+    assert_equal "change-me", User.get_salt()
+  end
+
+  def test_new_todo_fails_with_incorrect_token
+    assert_raises_invalid_token { @controller.new_todo('admin', 'notthecorrecttoken', contexts('agenda').id, 'test') }
+  end
+  
+  def test_new_todo_fails_with_context_that_does_not_belong_to_user
+     assert_raise(CannotAccessContext, "Cannot access a context that does not belong to this user.") { @controller.new_todo(users('other_user').login, users('other_user').word, contexts('agenda').id, 'test') }
+  end
+
+  def test_list_projects_fails_with_incorrect_token
+    assert_raises_invalid_token { @controller.list_projects('admin', 'notthecorrecttoken') }
+  end
+
+  def test_list_contexts_fails_with_incorrect_token
+    assert_raises_invalid_token { @controller.list_contexts('admin', 'notthecorrecttoken') }
+  end
+  
+  private
+  
+  def assert_raises_invalid_token
+    assert_raise(InvalidToken, "Sorry, you don't have permission to perform this action.") { yield } 
+  end
+
+end

tracks/test/functional/login_controller_test.rb

@@ -10,15 +10,15 @@ class LoginControllerTest < Test::Unit::TestCase
   
   def setup
     assert_equal "test", ENV['RAILS_ENV']
-    assert_equal "change-me", SALT
+    assert_equal "change-me", User.get_salt()
     @controller = LoginController.new
     @request = ActionController::TestRequest.new
     @response = ActionController::TestResponse.new
   end
 
-  # ============================================
-  # Login and logout
-  # ============================================
+  #============================================
+  #Login and logout
+  #============================================
     
   def test_invalid_login
     post :login, {:user_login => 'cracker', :user_password => 'secret', :user_noexpiry => 'on'}
@@ -26,7 +26,7 @@ class LoginControllerTest < Test::Unit::TestCase
     assert_session_has_no :user_id
     assert_template "login"
   end
-
+  
   def test_login_with_valid_admin_user
     @request.session['return-to'] = "/bogus/location"
     user = login('admin', 'abracadabra', 'on')
@@ -36,8 +36,8 @@ class LoginControllerTest < Test::Unit::TestCase
     assert_equal "Login successful: session will not expire.", flash['notice']
     assert_redirect_url "http://#{@request.host}/bogus/location"
   end
-
-
+  
+  
   def test_login_with_valid_standard_user
     user = login('jane','sesame', 'off')
     assert_equal user.id, @response.session['user_id']
@@ -46,28 +46,28 @@ class LoginControllerTest < Test::Unit::TestCase
     assert_equal "Login successful: session will expire after 1 hour of inactivity.", flash['notice']
     assert_redirected_to :controller => 'todo', :action => 'index'
   end
-
+  
   def test_logout
     user = login('admin','abracadabra', 'on')
     get :logout
     assert_nil(session['user_id'])
     assert_redirected_to :controller => 'login', :action => 'login'
   end
-
+  
   # Test login with a bad password for existing user
   # 
   def test_login_bad_password
     post :login, {:user_login => 'jane', :user_password => 'wrong', :user_noexpiry => 'on'}
     assert_session_has_no :user
     assert_equal "Login unsuccessful", flash['warning']
-    assert_success
+    assert_response :success
   end
   
   def test_login_bad_login
     post :login, {:user_login => 'blah', :user_password => 'sesame', :user_noexpiry => 'on'}
     assert_session_has_no :user
     assert_equal "Login unsuccessful", flash['warning']
-    assert_success
+    assert_response :success
   end
     
   # ============================================
@@ -103,7 +103,7 @@ class LoginControllerTest < Test::Unit::TestCase
     assert non_admin.is_admin == false || non_admin.is_admin == 0
     post :signup, :user => {:login => 'newbie2', :password => 'newbiepass2', :password_confirmation => 'newbiepass2'}
     assert_template 'login/nosignup'
-
+  
     num_users = User.find(:all)
     assert_equal num_users.length, 2
   end

tracks/test/functional/user_controller_test.rb

@@ -10,7 +10,7 @@ class UserControllerTest < Test::Unit::TestCase
   
   def setup
     assert_equal "test", ENV['RAILS_ENV']
-    assert_equal "change-me", SALT
+    assert_equal "change-me", User.get_salt()
     @controller = UserController.new
     @request    = ActionController::TestRequest.new
     @response   = ActionController::TestResponse.new
@@ -23,7 +23,7 @@ class UserControllerTest < Test::Unit::TestCase
     assert_redirected_to :controller => 'login', :action => 'login'
     @request.session['user_id'] = users(:admin_user).id # log in the admin user
     get :index
-    assert_success
+    assert_response :success
   end
   
   # Test admin with and without login
@@ -33,7 +33,7 @@ class UserControllerTest < Test::Unit::TestCase
     assert_redirected_to :controller => 'login', :action => 'login'
     @request.session['user_id'] = users(:admin_user).id # log in the admin user
     get :admin
-    assert_success
+    assert_response :success
   end
   
   def test_preferences
@@ -41,7 +41,7 @@ class UserControllerTest < Test::Unit::TestCase
     assert_redirected_to :controller => 'login', :action => 'login'
     @request.session['user_id'] = users(:admin_user).id # log in the admin user
     get :preferences
-    assert_success
+    assert_response :success
     assert_equal assigns['page_title'], "TRACKS::Preferences"
     assert_not_nil assigns['prefs']
     assert_equal assigns['prefs'].length, 7
@@ -52,7 +52,7 @@ class UserControllerTest < Test::Unit::TestCase
     assert_redirected_to :controller => 'login', :action => 'login'
     @request.session['user_id'] = users(:admin_user).id # log in the admin user
     get :edit_preferences
-    assert_success
+    assert_response :success
     assert_equal assigns['page_title'], "TRACKS::Edit Preferences"
     assert_not_nil assigns['prefs']
     assert_equal assigns['prefs'].length, 7    
@@ -76,12 +76,12 @@ class UserControllerTest < Test::Unit::TestCase
     @request.session['user_id'] = users(:admin_user).id # log in the admin user
     @user = @request.session['user_id']
     get :change_password # should now pass because we're logged in
-    assert_success
+    assert_response :success
     assert_equal assigns['page_title'], "TRACKS::Change password"    
     post :update_password, :updateuser => {:password => 'newpassword', :password_confirmation => 'newpassword'}
     assert_redirected_to :controller => 'user', :action => 'preferences'
     @updated_user = User.find(users(:admin_user).id)
-    assert_equal @updated_user.password, Digest::SHA1.hexdigest("#{SALT}--newpassword--")
+    assert_equal @updated_user.password, Digest::SHA1.hexdigest("#{User.get_salt()}--newpassword--")
     assert_equal flash['notice'], "Password updated."
   end
   

tracks/test/integration/create_user_api_test.rb

@@ -0,0 +1,115 @@
+require File.dirname(__FILE__) + '/../test_helper'
+require 'user_controller'
+require 'user'
+require 'action_controller/integration'
+
+# Re-raise errors caught by the controller.
+class UserController; def rescue_action(e) raise e end; end
+
+class CreateUserControllerTest < ActionController::IntegrationTest
+  fixtures :users
+  
+  def setup
+    assert_equal "test", ENV['RAILS_ENV']
+    assert_equal "change-me", User.get_salt()
+    @foobar_postdata = "<request><login>foo</login><password>bar</password></request>"
+    @john_postdata = "<request><login>john</login><password>barracuda</password></request>"
+  end
+  
+  def test_fails_with_401_if_not_authorized_user
+    authenticated_post_xml '/user/create', 'nobody', 'nohow', @foobar_postdata
+    assert_401_unauthorized
+  end
+  
+ def test_fails_with_401_if_not_admin_user
+   authenticated_post_xml '/user/create', users(:other_user).login, 'sesame', @foobar_postdata
+   assert_401_unauthorized
+ end
+ 
+ def test_content_type_must_be_xml
+   authenticated_post_xml "/user/create", users(:admin_user).login, 'abracadabra', @foobar_postdata, {'CONTENT_TYPE' => "application/x-www-form-urlencoded"}
+   assert_response_and_body 404, "Content Type must be application/xml."
+ end
+  
+ def test_fails_with_invalid_xml_format
+   invalid_postdata = "<foo></bar>"
+   authenticated_post_xml "/user/create", users(:admin_user).login, 'abracadabra', invalid_postdata
+   assert_404_invalid_xml
+ end
+    
+  def test_fails_with_invalid_xml_format2
+    invalid_postdata = "<request><username>foo</username></request>"
+    authenticated_post_xml "/user/create", users(:admin_user).login, 'abracadabra', invalid_postdata
+    assert_404_invalid_xml
+  end
+  
+  def test_xml_simple_param_parsing
+    authenticated_post_xml "/user/create", users(:admin_user).login, 'abracadabra', @foobar_postdata
+    assert @controller.params.has_key?(:request)
+    assert @controller.params[:request].has_key?(:login)
+    assert @controller.params[:request].has_key?(:password)
+    assert_equal 'foo', @controller.params[:request][:login]
+    assert_equal 'bar', @controller.params[:request][:password]
+  end
+  
+  def test_fails_with_too_short_password
+    authenticated_post_xml "/user/create", users(:admin_user).login, 'abracadabra', @foobar_postdata
+    assert_response_and_body 404, "Password is too short (minimum is 5 characters)"
+  end
+  
+  def test_fails_with_nonunique_login
+    existing_login = users(:other_user).login
+    data = "<request><login>#{existing_login}</login><password>barracuda</password></request>"
+    authenticated_post_xml "/user/create", users(:admin_user).login, 'abracadabra', data
+    assert_response_and_body 404, "Login has already been taken"
+  end
+  
+  def test_creates_new_user
+    authenticated_post_xml '/user/create', users(:admin_user).login, 'abracadabra', @john_postdata
+    assert_response_and_body 200, "User created."
+    assert_equal 3, User.count
+    john1 = User.find_by_login('john')
+    assert_not_nil john1, "expected user john to be created"
+    john2 = User.authenticate('john','barracuda')
+    assert_not_nil john2, "expected user john to be created"
+  end
+  
+  def test_fails_with_get_verb
+    authenticated_get_xml "/user/create", users(:admin_user).login, 'abracadabra', {}
+  end
+    
+  private
+  
+  def authenticated_post_xml(url, username, password, parameters, headers = {})
+    post url,
+        parameters,
+        {'AUTHORIZATION' => "Basic " + Base64.encode64("#{username}:#{password}"),
+          'ACCEPT' => 'application/xml',
+          'CONTENT_TYPE' => 'application/xml'
+          }.merge(headers)
+  end
+
+  def authenticated_get_xml(url, username, password, parameters, headers = {})
+    get url,
+        parameters,
+        {'AUTHORIZATION' => "Basic " + Base64.encode64("#{username}:#{password}"),
+          'ACCEPT' => 'application/xml',
+          'CONTENT_TYPE' => 'application/xml'
+          }.merge(headers)
+  end
+
+  def assert_401_unauthorized
+    assert_response_and_body 401, "401 Unauthorized: You are not authorized to interact with Tracks."
+  end
+
+  def assert_404_invalid_xml
+    assert_response_and_body 404, "Expected post format is xml like so: <request><login>username</login><password>abc123</password></request>."
+  end
+
+  def assert_response_and_body (type, body, message = nil)
+    #puts @response.body
+    assert_response type, message
+    assert_equal body, @response.body, message
+  end
+  
+end

tracks/test/integration/stories_test.rb

@@ -4,7 +4,7 @@ class StoriesTest < ActionController::IntegrationTest
   fixtures :users, :projects, :contexts, :todos, :notes
   
   def setup
-    assert_equal "change-me", SALT
+    assert_equal "change-me", User.get_salt
   end
   
   # ####################################################

tracks/test/test_helper.rb

@@ -2,6 +2,12 @@ ENV["RAILS_ENV"] = "test"
 require File.expand_path(File.dirname(__FILE__) + "/../config/environment")
 require 'test_help'
 
+class User < ActiveRecord::Base
+  def self.get_salt
+    "change-me"
+  end
+end
+
 class Test::Unit::TestCase
   # Turn off transactional fixtures if you're working with MyISAM tables in MySQL
   self.use_transactional_fixtures = true

tracks/test/unit/user_test.rb

@@ -5,7 +5,7 @@ class UserTest < Test::Unit::TestCase
 
   def setup
     assert_equal "test", ENV['RAILS_ENV']
-    assert_equal "change-me", SALT    
+    assert_equal "change-me", User.get_salt()    
     @admin_user = User.find(1)
     @other_user = User.find(2)
   end
@@ -16,7 +16,7 @@ class UserTest < Test::Unit::TestCase
     assert_kind_of User, @admin_user
     assert_equal 1, @admin_user.id
     assert_equal "admin", @admin_user.login
-    assert_equal "#{Digest::SHA1.hexdigest("#{SALT}--abracadabra--")}", @admin_user.password
+    assert_equal "#{Digest::SHA1.hexdigest("#{User.get_salt()}--abracadabra--")}", @admin_user.password
     assert_not_nil @admin_user.word
     assert @admin_user.is_admin
   end
@@ -26,7 +26,7 @@ class UserTest < Test::Unit::TestCase
     assert_kind_of User, @other_user
     assert_equal 2, @other_user.id
     assert_equal "jane", @other_user.login
-    assert_equal "#{Digest::SHA1.hexdigest("#{SALT}--sesame--")}", @other_user.password
+    assert_equal "#{Digest::SHA1.hexdigest("#{User.get_salt()}--sesame--")}", @other_user.password
     assert_not_nil @other_user.word
     assert @other_user.is_admin == false || @other_user.is_admin == 0 
   end
@@ -38,7 +38,7 @@ class UserTest < Test::Unit::TestCase
   # Test a password shorter than 5 characters
   # 
   def test_validate_short_password
-    assert_equal "#{Digest::SHA1.hexdigest("#{SALT}--sesame--")}", @other_user.password
+    assert_equal "#{Digest::SHA1.hexdigest("#{User.get_salt()}--sesame--")}", @other_user.password
     @other_user.password = "four"
     assert !@other_user.save
     assert_equal 1, @other_user.errors.count
@@ -48,7 +48,7 @@ class UserTest < Test::Unit::TestCase
   # Test a password longer than 40 characters
   # 
   def test_validate_long_password
-    assert_equal "#{Digest::SHA1.hexdigest("#{SALT}--sesame--")}", @other_user.password
+    assert_equal "#{Digest::SHA1.hexdigest("#{User.get_salt()}--sesame--")}", @other_user.password
     @other_user.password = generate_random_string(41)
     assert !@other_user.save
     assert_equal 1, @other_user.errors.count
@@ -58,7 +58,7 @@ class UserTest < Test::Unit::TestCase
   # Test that correct length password is valid
   # 
   def test_validate_correct_length_password
-    assert_equal "#{Digest::SHA1.hexdigest("#{SALT}--sesame--")}", @other_user.password
+    assert_equal "#{Digest::SHA1.hexdigest("#{User.get_salt()}--sesame--")}", @other_user.password
     @other_user.password = generate_random_string(6)
     assert @other_user.save
   end