From 16b9c2947b010daa935848122de8e6317d7a3b8d Mon Sep 17 00:00:00 2001 From: lukemelia Date: Sun, 8 Jul 2007 06:41:10 +0000 Subject: [PATCH] This changeset adds real "remember me" functionality. The checkbox on the login page "Stay logged in" previously prevented an inactive session from expiring. Now, it also functions to remember that a user is logged in across browser sessions (i.e. a user exits the browser, and reopens it). I've also ensured that all tests (including selenium tests) are passing on my machine. This changeset should be back to stable and usable. git-svn-id: http://www.rousette.org.uk/svn/tracks-repos/trunk@561 a4c988fc-2ded-0310-b66e-134b36920a42 --- tracks/app/controllers/application.rb | 2 + tracks/app/controllers/login_controller.rb | 13 +- tracks/app/models/user.rb | 45 ++- tracks/db/schema.rb | 351 +++++++----------- tracks/lib/authenticated_test_helper.rb | 2 +- tracks/lib/login_system.rb | 25 +- tracks/test/fixtures/users.yml | 6 +- .../test/functional/login_controller_test.rb | 46 +++ .../functional/preferences_controller_test.rb | 8 +- .../test/functional/users_controller_test.rb | 2 +- .../project_detail/_add_deferred_todo.rsel | 2 +- .../activate_deferred_todo.rsel | 4 +- .../activate_last_deferred_todo.rsel | 2 +- .../change_todos_project_to_blank.rsel | 2 +- .../selenium/project_detail/defer_todo.rsel | 2 +- tracks/test/test_helper.rb | 6 + tracks/test/unit/user_test.rb | 10 +- 17 files changed, 285 insertions(+), 243 deletions(-) diff --git a/tracks/app/controllers/application.rb b/tracks/app/controllers/application.rb index a19048da..d6bd1e86 100644 --- a/tracks/app/controllers/application.rb +++ b/tracks/app/controllers/application.rb @@ -21,6 +21,8 @@ class ApplicationController < ActionController::Base prepend_before_filter :enable_mobile_content_negotiation after_filter :restore_content_type_for_mobile after_filter :set_charset + + include ActionView::Helpers::TextHelper helper_method :format_date, :markdown diff --git a/tracks/app/controllers/login_controller.rb b/tracks/app/controllers/login_controller.rb index e294065f..2dc9e31f 100644 --- a/tracks/app/controllers/login_controller.rb +++ b/tracks/app/controllers/login_controller.rb @@ -4,6 +4,7 @@ class LoginController < ApplicationController filter_parameter_logging :user_password skip_before_filter :set_session_expiration skip_before_filter :login_required + before_filter :login_optional before_filter :get_current_user open_id_consumer if Tracks::Config.openid_enabled? @@ -15,11 +16,15 @@ class LoginController < ApplicationController if @user = User.authenticate(params['user_login'], params['user_password']) session['user_id'] = @user.id # If checkbox on login page checked, we don't expire the session after 1 hour - # of inactivity + # of inactivity and we remember this user for future browser sessions session['noexpiry'] = params['user_noexpiry'] msg = (should_expire_sessions?) ? "will expire after 1 hour of inactivity." : "will not expire." notify :notice, "Login successful: session #{msg}" cookies[:tracks_login] = { :value => @user.login, :expires => Time.now + 1.year } + unless should_expire_sessions? + @user.remember_me + cookies[:auth_token] = { :value => @user.remember_token , :expires => @user.remember_token_expires_at } + end redirect_back_or_home return else @@ -90,6 +95,10 @@ class LoginController < ApplicationController msg = (should_expire_sessions?) ? "will expire after 1 hour of inactivity." : "will not expire." notify :notice, "You have successfully verified #{openid_url} as your identity. Login successful: session #{msg}" cookies[:tracks_login] = { :value => @user.login, :expires => Time.now + 1.year } + unless should_expire_sessions? + @user.remember_me + cookies[:auth_token] = { :value => @user.remember_token , :expires => @user.remember_token_expires_at } + end cookies[:openid_url] = { :value => openid_url, :expires => Time.now + 1.year } redirect_back_or_home else @@ -106,6 +115,8 @@ class LoginController < ApplicationController end def logout + @user.forget_me if logged_in? + cookies.delete :auth_token session['user_id'] = nil reset_session notify :notice, "You have been logged out of Tracks." diff --git a/tracks/app/models/user.rb b/tracks/app/models/user.rb index fad1d6fe..cb73416c 100644 --- a/tracks/app/models/user.rb +++ b/tracks/app/models/user.rb @@ -1,6 +1,9 @@ require 'digest/sha1' class User < ActiveRecord::Base + # Virtual attribute for the unencrypted password + attr_accessor :password + has_many :contexts, :order => 'position ASC', :dependent => :delete_all do @@ -89,6 +92,9 @@ class User < ActiveRecord::Base validates_uniqueness_of :login, :on => :create validates_presence_of :open_id_url, :if => Proc.new{|user| user.auth_type == 'open_id'} + before_create :crypt_password, :crypt_word + before_update :crypt_password + def validate unless Tracks::Config.auth_schemes.include?(auth_type) errors.add("auth_type", "not a valid authentication type") @@ -101,7 +107,7 @@ class User < ActiveRecord::Base candidate = find(:first, :conditions => ["login = ?", login]) return nil if candidate.nil? if candidate.auth_type == 'database' - return candidate if candidate.password == sha1(pass) + return candidate if candidate.crypted_password == sha1(pass) elsif candidate.auth_type == 'ldap' && Tracks::Config.auth_schemes.include?('ldap') return candidate if SimpleLdapAuthenticator.valid?(login, pass) end @@ -136,10 +142,6 @@ class User < ActiveRecord::Base self.password_confirmation = pass_confirm save! end - - def crypt_word - write_attribute("word", self.class.sha1(login + Time.now.to_i.to_s + rand.to_s)) - end def time prefs.tz.adjust(Time.now.utc) @@ -148,22 +150,45 @@ class User < ActiveRecord::Base def date time.to_date end + + def remember_token? + remember_token_expires_at && Time.now.utc < remember_token_expires_at + end + + # These create and unset the fields required for remembering users between browser closes + def remember_me + self.remember_token_expires_at = 2.weeks.from_now.utc + self.remember_token = self.class.sha1("#{login}--#{remember_token_expires_at}") + save(false) + end + + def forget_me + self.remember_token_expires_at = nil + self.remember_token = nil + save(false) + end protected def self.sha1(pass) Digest::SHA1.hexdigest("#{Tracks::Config.salt}--#{pass}--") end - - before_create :crypt_password, :crypt_word - before_update :crypt_password + + def crypt_word + write_attribute("word", self.class.sha1(login + Time.now.to_i.to_s + rand.to_s)) + end def crypt_password - write_attribute("password", self.class.sha1(password)) if password == @password_confirmation + return if password.blank? + write_attribute("crypted_password", self.class.sha1(password)) if password == password_confirmation end def password_required? - auth_type == 'database' + auth_type == 'database' && crypted_password.blank? || !password.blank? + end + + def password_matches?(pass) + crypted_password == sha1(pass) end end diff --git a/tracks/db/schema.rb b/tracks/db/schema.rb index 25ba7c77..153585c1 100644 --- a/tracks/db/schema.rb +++ b/tracks/db/schema.rb @@ -1,211 +1,140 @@ -# This file is autogenerated. Instead of editing this file, please use the -# migrations feature of ActiveRecord to incrementally modify your database, and -# then regenerate this schema definition. - -ActiveRecord::Schema.define(:version => 33) do - - create_table "bow_wows", :force => true do |t| - t.column "name", :string - t.column "created_at", :datetime, :null => false - t.column "updated_at", :datetime, :null => false - end - - create_table "cats", :force => true do |t| - t.column "name", :string - t.column "cat_type", :string - t.column "created_at", :datetime, :null => false - t.column "updated_at", :datetime, :null => false - end - - create_table "contexts", :force => true do |t| - t.column "name", :string, :default => "", :null => false - t.column "hide", :integer, :limit => 4, :default => 0, :null => false - t.column "position", :integer, :default => 0, :null => false - t.column "user_id", :integer, :default => 0, :null => false - t.column "created_at", :datetime - t.column "updated_at", :datetime - end - - add_index "contexts", ["user_id"], :name => "index_contexts_on_user_id" - add_index "contexts", ["user_id", "name"], :name => "index_contexts_on_user_id_and_name" - - create_table "eaters_foodstuffs", :force => true do |t| - t.column "foodstuff_id", :integer - t.column "eater_id", :integer - t.column "some_attribute", :integer, :default => 0 - t.column "eater_type", :string - t.column "created_at", :datetime, :null => false - t.column "updated_at", :datetime, :null => false - end - - create_table "fish", :force => true do |t| - t.column "name", :string - t.column "speed", :integer - t.column "created_at", :datetime, :null => false - t.column "updated_at", :datetime, :null => false - end - - create_table "frogs", :force => true do |t| - t.column "name", :string - t.column "created_at", :datetime, :null => false - t.column "updated_at", :datetime, :null => false - end - - create_table "keep_your_enemies_close", :force => true do |t| - t.column "enemy_id", :integer - t.column "enemy_type", :string - t.column "protector_id", :integer - t.column "protector_type", :string - t.column "created_at", :datetime, :null => false - t.column "updated_at", :datetime, :null => false - end - - create_table "little_whale_pupils", :force => true do |t| - t.column "whale_id", :integer - t.column "aquatic_pupil_id", :integer - t.column "aquatic_pupil_type", :string - t.column "created_at", :datetime, :null => false - t.column "updated_at", :datetime, :null => false - end - - create_table "notes", :force => true do |t| - t.column "user_id", :integer, :default => 0, :null => false - t.column "project_id", :integer, :default => 0, :null => false - t.column "body", :text - t.column "created_at", :datetime - t.column "updated_at", :datetime - end - - create_table "open_id_associations", :force => true do |t| - t.column "server_url", :binary - t.column "handle", :string - t.column "secret", :binary - t.column "issued", :integer - t.column "lifetime", :integer - t.column "assoc_type", :string - end - - create_table "open_id_nonces", :force => true do |t| - t.column "nonce", :string - t.column "created", :integer - end - - create_table "open_id_settings", :force => true do |t| - t.column "setting", :string - t.column "value", :binary - end - - create_table "petfoods", :id => false, :force => true do |t| - t.column "the_petfood_primary_key", :integer, :null => false - t.column "name", :string - t.column "created_at", :datetime, :null => false - t.column "updated_at", :datetime, :null => false - end - - create_table "preferences", :force => true do |t| - t.column "user_id", :integer, :default => 0, :null => false - t.column "date_format", :string, :limit => 40, :default => "%d/%m/%Y", :null => false - t.column "week_starts", :integer, :default => 0, :null => false - t.column "show_number_completed", :integer, :default => 5, :null => false - t.column "staleness_starts", :integer, :default => 7, :null => false - t.column "show_completed_projects_in_sidebar", :boolean, :default => true, :null => false - t.column "show_hidden_contexts_in_sidebar", :boolean, :default => true, :null => false - t.column "due_style", :integer, :default => 0, :null => false - t.column "admin_email", :string, :default => "butshesagirl@rousette.org.uk", :null => false - t.column "refresh", :integer, :default => 0, :null => false - t.column "verbose_action_descriptors", :boolean, :default => false, :null => false - t.column "show_hidden_projects_in_sidebar", :boolean, :default => true, :null => false - t.column "time_zone", :string, :default => "London", :null => false - t.column "show_project_on_todo_done", :boolean, :default => false, :null => false - t.column "title_date_format", :string, :default => "%A, %d %B %Y", :null => false - t.column "mobile_todos_per_page", :integer, :default => 6, :null => false - end - - add_index "preferences", ["user_id"], :name => "index_preferences_on_user_id" - - create_table "projects", :force => true do |t| - t.column "name", :string, :default => "", :null => false - t.column "position", :integer, :default => 0, :null => false - t.column "user_id", :integer, :default => 0, :null => false - t.column "description", :text - t.column "state", :string, :limit => 20, :default => "active", :null => false - t.column "created_at", :datetime - t.column "updated_at", :datetime - t.column "default_context_id", :integer - end - - add_index "projects", ["user_id"], :name => "index_projects_on_user_id" - add_index "projects", ["user_id", "name"], :name => "index_projects_on_user_id_and_name" - - create_table "sessions", :force => true do |t| - t.column "session_id", :string - t.column "data", :text - t.column "updated_at", :datetime - end - - add_index "sessions", ["session_id"], :name => "sessions_session_id_index" - - create_table "taggings", :force => true do |t| - t.column "taggable_id", :integer - t.column "tag_id", :integer - t.column "taggable_type", :string - t.column "user_id", :integer - end - - add_index "taggings", ["tag_id", "taggable_id", "taggable_type"], :name => "index_taggings_on_tag_id_and_taggable_id_and_taggable_type" - - create_table "tags", :force => true do |t| - t.column "name", :string - t.column "created_at", :datetime - t.column "updated_at", :datetime - end - - add_index "tags", ["name"], :name => "index_tags_on_name" - - create_table "todos", :force => true do |t| - t.column "context_id", :integer, :default => 0, :null => false - t.column "description", :string, :limit => 100, :default => "", :null => false - t.column "notes", :text - t.column "created_at", :datetime - t.column "due", :date - t.column "completed_at", :datetime - t.column "project_id", :integer - t.column "user_id", :integer, :default => 0, :null => false - t.column "show_from", :date - t.column "state", :string, :limit => 20, :default => "immediate", :null => false - end - - add_index "todos", ["user_id", "state"], :name => "index_todos_on_user_id_and_state" - add_index "todos", ["user_id", "project_id"], :name => "index_todos_on_user_id_and_project_id" - add_index "todos", ["project_id"], :name => "index_todos_on_project_id" - add_index "todos", ["context_id"], :name => "index_todos_on_context_id" - add_index "todos", ["user_id", "context_id"], :name => "index_todos_on_user_id_and_context_id" - - create_table "users", :force => true do |t| - t.column "login", :string, :limit => 80 - t.column "crypted_password", :string, :limit => 40 - t.column "word", :string - t.column "is_admin", :integer, :limit => 4, :default => 0, :null => false - t.column "first_name", :string - t.column "last_name", :string - t.column "auth_type", :string, :default => "database", :null => false - t.column "open_id_url", :string - t.column "remember_token", :string - t.column "remember_token_expires_at", :datetime - end - - add_index "users", ["login"], :name => "index_users_on_login" - - create_table "whales", :force => true do |t| - t.column "name", :string - t.column "created_at", :datetime, :null => false - t.column "updated_at", :datetime, :null => false - end - - create_table "wild_boars", :force => true do |t| - t.column "name", :string - t.column "created_at", :datetime, :null => false - t.column "updated_at", :datetime, :null => false - end - -end +# This file is autogenerated. Instead of editing this file, please use the +# migrations feature of ActiveRecord to incrementally modify your database, and +# then regenerate this schema definition. + +ActiveRecord::Schema.define(:version => 33) do + + create_table "contexts", :force => true do |t| + t.column "name", :string, :default => "", :null => false + t.column "position", :integer, :null => false + t.column "hide", :boolean, :default => false + t.column "user_id", :integer, :default => 1 + t.column "created_at", :datetime + t.column "updated_at", :datetime + end + + add_index "contexts", ["user_id"], :name => "index_contexts_on_user_id" + add_index "contexts", ["user_id", "name"], :name => "index_contexts_on_user_id_and_name" + + create_table "notes", :force => true do |t| + t.column "user_id", :integer, :null => false + t.column "project_id", :integer, :null => false + t.column "body", :text + t.column "created_at", :datetime + t.column "updated_at", :datetime + end + + create_table "open_id_associations", :force => true do |t| + t.column "server_url", :binary + t.column "handle", :string + t.column "secret", :binary + t.column "issued", :integer + t.column "lifetime", :integer + t.column "assoc_type", :string + end + + create_table "open_id_nonces", :force => true do |t| + t.column "nonce", :string + t.column "created", :integer + end + + create_table "open_id_settings", :force => true do |t| + t.column "setting", :string + t.column "value", :binary + end + + create_table "preferences", :force => true do |t| + t.column "user_id", :integer, :null => false + t.column "date_format", :string, :limit => 40, :default => "%d/%m/%Y", :null => false + t.column "week_starts", :integer, :default => 0, :null => false + t.column "show_number_completed", :integer, :default => 5, :null => false + t.column "staleness_starts", :integer, :default => 7, :null => false + t.column "show_completed_projects_in_sidebar", :boolean, :default => true, :null => false + t.column "show_hidden_contexts_in_sidebar", :boolean, :default => true, :null => false + t.column "due_style", :integer, :default => 0, :null => false + t.column "admin_email", :string, :default => "butshesagirl@rousette.org.uk", :null => false + t.column "refresh", :integer, :default => 0, :null => false + t.column "verbose_action_descriptors", :boolean, :default => false, :null => false + t.column "show_hidden_projects_in_sidebar", :boolean, :default => true, :null => false + t.column "time_zone", :string, :default => "London", :null => false + t.column "show_project_on_todo_done", :boolean, :default => false, :null => false + t.column "title_date_format", :string, :default => "%A, %d %B %Y", :null => false + t.column "mobile_todos_per_page", :integer, :default => 6, :null => false + end + + add_index "preferences", ["user_id"], :name => "index_preferences_on_user_id" + + create_table "projects", :force => true do |t| + t.column "name", :string, :default => "", :null => false + t.column "position", :integer, :null => false + t.column "user_id", :integer, :default => 1 + t.column "description", :text + t.column "state", :string, :limit => 20, :default => "active", :null => false + t.column "created_at", :datetime + t.column "updated_at", :datetime + t.column "default_context_id", :integer + end + + add_index "projects", ["user_id"], :name => "index_projects_on_user_id" + add_index "projects", ["user_id", "name"], :name => "index_projects_on_user_id_and_name" + + create_table "sessions", :force => true do |t| + t.column "session_id", :string + t.column "data", :text + t.column "updated_at", :datetime + end + + add_index "sessions", ["session_id"], :name => "index_sessions_on_session_id" + + create_table "taggings", :force => true do |t| + t.column "taggable_id", :integer + t.column "tag_id", :integer + t.column "taggable_type", :string + t.column "user_id", :integer + end + + add_index "taggings", ["tag_id", "taggable_id", "taggable_type"], :name => "index_taggings_on_tag_id_and_taggable_id_and_taggable_type" + + create_table "tags", :force => true do |t| + t.column "name", :string + t.column "created_at", :datetime + t.column "updated_at", :datetime + end + + add_index "tags", ["name"], :name => "index_tags_on_name" + + create_table "todos", :force => true do |t| + t.column "context_id", :integer, :null => false + t.column "project_id", :integer + t.column "description", :string, :default => "", :null => false + t.column "notes", :text + t.column "created_at", :datetime + t.column "due", :date + t.column "completed_at", :datetime + t.column "user_id", :integer, :default => 1 + t.column "show_from", :date + t.column "state", :string, :limit => 20, :default => "immediate", :null => false + end + + add_index "todos", ["user_id", "state"], :name => "index_todos_on_user_id_and_state" + add_index "todos", ["user_id", "project_id"], :name => "index_todos_on_user_id_and_project_id" + add_index "todos", ["project_id"], :name => "index_todos_on_project_id" + add_index "todos", ["context_id"], :name => "index_todos_on_context_id" + add_index "todos", ["user_id", "context_id"], :name => "index_todos_on_user_id_and_context_id" + + create_table "users", :force => true do |t| + t.column "login", :string, :limit => 80, :default => "", :null => false + t.column "crypted_password", :string, :limit => 40 + t.column "word", :string + t.column "is_admin", :boolean, :default => false, :null => false + t.column "first_name", :string + t.column "last_name", :string + t.column "auth_type", :string, :default => "database", :null => false + t.column "open_id_url", :string + t.column "remember_token", :string + t.column "remember_token_expires_at", :datetime + end + + add_index "users", ["login"], :name => "index_users_on_login" + +end diff --git a/tracks/lib/authenticated_test_helper.rb b/tracks/lib/authenticated_test_helper.rb index 2b511958..2f743aa0 100644 --- a/tracks/lib/authenticated_test_helper.rb +++ b/tracks/lib/authenticated_test_helper.rb @@ -1,7 +1,7 @@ module AuthenticatedTestHelper # Sets the current user in the session from the user fixtures. def login_as(user) - @request.session[:user] = user ? users(user).id : nil + @request.session['user_id'] = user ? users(user).id : nil end def content_type(type) diff --git a/tracks/lib/login_system.rb b/tracks/lib/login_system.rb index 7872873c..b3c79ad1 100644 --- a/tracks/lib/login_system.rb +++ b/tracks/lib/login_system.rb @@ -31,6 +31,20 @@ module LoginSystem true end + # When called with before_filter :login_from_cookie will check for an :auth_token + # cookie and log the user back in if appropriate + def login_from_cookie + return unless cookies[:auth_token] && !logged_in? + user = User.find_by_remember_token(cookies[:auth_token]) + if user && user.remember_token? + session['user_id'] = user.id + @user = user + @user.remember_me + cookies[:auth_token] = { :value => @user.remember_token , :expires => @user.remember_token_expires_at } + flash[:notice] = "Logged in successfully. Welcome back!" + end + end + def login_or_feed_token_required if ['rss', 'atom', 'txt', 'ics'].include?(params[:format]) if user = User.find_by_word(params[:token]) @@ -55,6 +69,8 @@ module LoginSystem if not protect?(action_name) return true end + + login_from_cookie if session['user_id'] and authorize?(get_current_user) return true @@ -78,6 +94,8 @@ module LoginSystem def login_optional + login_from_cookie + if session['user_id'] and authorize?(get_current_user) return true end @@ -92,6 +110,11 @@ module LoginSystem return true end + def logged_in? + get_current_user + @user != nil + end + def get_current_user if @user.nil? && session['user_id'] @user = User.find session['user_id'], :include => :preference @@ -164,6 +187,6 @@ module LoginSystem response.headers["Status"] = "Unauthorized" response.headers["WWW-Authenticate"] = "Basic realm=\"'Tracks Login Required'\"" render :text => "401 Unauthorized: You are not authorized to interact with Tracks.", :status => 401 - end + end end \ No newline at end of file diff --git a/tracks/test/fixtures/users.yml b/tracks/test/fixtures/users.yml index a29a961b..cd30f173 100644 --- a/tracks/test/fixtures/users.yml +++ b/tracks/test/fixtures/users.yml @@ -2,7 +2,7 @@ admin_user: id: 1 login: admin - password: <%= Digest::SHA1.hexdigest("#{Tracks::Config.salt}--abracadabra--") %> + crypted_password: <%= Digest::SHA1.hexdigest("#{Tracks::Config.salt}--abracadabra--") %> word: <%= Digest::SHA1.hexdigest("adminSat Feb 25 17:14:00 GMT 20060.236961325863376") %> is_admin: true first_name: Admin @@ -12,7 +12,7 @@ admin_user: other_user: id: 2 login: jane - password: <%= Digest::SHA1.hexdigest("#{Tracks::Config.salt}--sesame--") %> + crypted_password: <%= Digest::SHA1.hexdigest("#{Tracks::Config.salt}--sesame--") %> word: <%= Digest::SHA1.hexdigest("janeSun Feb 19 14:42:45 GMT 20060.408173979260027") %> is_admin: false first_name: Jane @@ -22,7 +22,7 @@ other_user: ldap_user: id: 3 login: john - password: test + crypted_password: test word: <%= Digest::SHA1.hexdigest("johnSun Feb 19 14:42:45 GMT 20060.408173979260027") %> is_admin: false first_name: John diff --git a/tracks/test/functional/login_controller_test.rb b/tracks/test/functional/login_controller_test.rb index a17f2100..bb978e41 100644 --- a/tracks/test/functional/login_controller_test.rb +++ b/tracks/test/functional/login_controller_test.rb @@ -76,6 +76,44 @@ class LoginControllerTest < Test::Rails::TestCase assert_response :success end + def test_should_remember_me + post :login, :user_login => 'jane', :user_password => 'sesame', :user_noexpiry => "on" + assert_not_nil @response.cookies["auth_token"] + end + + def test_should_not_remember_me + post :login, :user_login => 'jane', :user_password => 'sesame', :user_noexpiry => "off" + assert_nil @response.cookies["auth_token"] + end + + def test_should_delete_token_on_logout + login_as :other_user + get :logout + assert_equal @response.cookies["auth_token"], [] + end + + def test_should_login_with_cookie + users(:other_user).remember_me + @request.cookies["auth_token"] = cookie_for(:other_user) + get :login + assert @controller.send(:logged_in?) + end + + def test_should_fail_expired_cookie_login + users(:other_user).remember_me + users(:other_user).update_attribute :remember_token_expires_at, 5.minutes.ago + @request.cookies["auth_token"] = cookie_for(:other_user) + get :login + assert !@controller.send(:logged_in?) + end + + def test_should_fail_cookie_login + users(:other_user).remember_me + @request.cookies["auth_token"] = auth_token('invalid_auth_token') + get :login + assert !@controller.send(:logged_in?) + end + private # Logs in a user and returns the user object found in the session object @@ -85,5 +123,13 @@ class LoginControllerTest < Test::Rails::TestCase return User.find(session['user_id']) end + def auth_token(token) + CGI::Cookie.new('name' => 'auth_token', 'value' => token) + end + + def cookie_for(user) + auth_token users(user).remember_token + end + end diff --git a/tracks/test/functional/preferences_controller_test.rb b/tracks/test/functional/preferences_controller_test.rb index 353953d9..52ee37c2 100644 --- a/tracks/test/functional/preferences_controller_test.rb +++ b/tracks/test/functional/preferences_controller_test.rb @@ -6,7 +6,7 @@ require 'preference' class PreferencesController; def rescue_action(e) raise e end; end class PreferencesControllerTest < Test::Rails::TestCase - fixtures :users + fixtures :users, :preferences def setup assert_equal "test", ENV['RAILS_ENV'] @@ -19,7 +19,7 @@ class PreferencesControllerTest < Test::Rails::TestCase def test_preferences get :index # should fail because no login assert_redirected_to :controller => 'login', :action => 'login' - @request.session['user_id'] = users(:admin_user).id # log in the admin user + login_as :admin_user get :index assert_response :success assert_equal assigns['page_title'], "TRACKS::Preferences" @@ -29,7 +29,7 @@ class PreferencesControllerTest < Test::Rails::TestCase def test_edit_preferences get :edit # should fail because no login assert_redirected_to :controller => 'login', :action => 'login' - @request.session['user_id'] = users(:admin_user).id # log in the admin user + login_as :admin_user get :edit assert_response :success assert_equal assigns['page_title'], "TRACKS::Edit Preferences" @@ -40,7 +40,7 @@ class PreferencesControllerTest < Test::Rails::TestCase def test_update_preferences login_as :admin_user post :update, {:user => { :first_name => 'Jane', :last_name => 'Doe'}, :prefs => { :date_format => "%m-%d-%Y", :week_starts => "0", :show_number_completed => "10", :show_completed_projects_in_sidebar => "false", :show_hidden_contexts_in_sidebar => "false", :staleness_starts => "14", :due_style => "1", :admin_email => "my.email@domain.com" }} - updated_admin_user = User.find(users(:admin_user).id) + updated_admin_user = users(:admin_user).reload assert_not_nil updated_admin_user.preference assert_equal 'Jane', updated_admin_user.first_name assert_equal 'Doe', updated_admin_user.last_name diff --git a/tracks/test/functional/users_controller_test.rb b/tracks/test/functional/users_controller_test.rb index 5c19264f..77429849 100644 --- a/tracks/test/functional/users_controller_test.rb +++ b/tracks/test/functional/users_controller_test.rb @@ -57,7 +57,7 @@ class UsersControllerTest < Test::Rails::TestCase post :update_password, :updateuser => {:password => 'newpassword', :password_confirmation => 'newpassword'} assert_redirected_to :controller => 'preferences' @updated_user = User.find(users(:admin_user).id) - assert_equal @updated_user.password, Digest::SHA1.hexdigest("#{Tracks::Config.salt}--newpassword--") + assert_equal @updated_user.crypted_password, Digest::SHA1.hexdigest("#{Tracks::Config.salt}--newpassword--") assert_equal "Password updated.", flash[:notice] end diff --git a/tracks/test/selenium/project_detail/_add_deferred_todo.rsel b/tracks/test/selenium/project_detail/_add_deferred_todo.rsel index 2bea0ca5..15fd15ef 100644 --- a/tracks/test/selenium/project_detail/_add_deferred_todo.rsel +++ b/tracks/test/selenium/project_detail/_add_deferred_todo.rsel @@ -1,5 +1,5 @@ type "todo_description", "choose era" type "todo_show_from", "1/1/2030" -click "//input[@value='Add action']" +click "css=#todo-form-new-action .submit_box button" wait_for_element_present "xpath=//div[@id='tickler'] //div[@class='item-container']" wait_for_element_present "xpath=//div[@id='tickler'] //div[@class='item-container'] //a[@title='01/01/2030']" diff --git a/tracks/test/selenium/project_detail/activate_deferred_todo.rsel b/tracks/test/selenium/project_detail/activate_deferred_todo.rsel index 34109427..46f315ef 100644 --- a/tracks/test/selenium/project_detail/activate_deferred_todo.rsel +++ b/tracks/test/selenium/project_detail/activate_deferred_todo.rsel @@ -6,7 +6,7 @@ open "/projects/1" click "edit_icon_todo_5" wait_for_element_present "show_from_todo_5" type "show_from_todo_5", "1/1/2030" -click "//input[@value='Update']" +click "css=#submit_todo_5" wait_for_element_present "xpath=//div[@id='tickler'] //div[@id='todo_5']" #now activate the other deferred one @@ -14,7 +14,7 @@ open "/projects/1" click "edit_icon_todo_15" wait_for_element_present "show_from_todo_15" type "show_from_todo_15", "" -click "//input[@value='Update']" +click "css=#submit_todo_15" wait_for_element_present "xpath=//div[@id='p1'] //div[@id='todo_15']" assert_not_visible "tickler-empty-nd" assert_text 'badge_count', '2' diff --git a/tracks/test/selenium/project_detail/activate_last_deferred_todo.rsel b/tracks/test/selenium/project_detail/activate_last_deferred_todo.rsel index 8672e381..422a09da 100644 --- a/tracks/test/selenium/project_detail/activate_last_deferred_todo.rsel +++ b/tracks/test/selenium/project_detail/activate_last_deferred_todo.rsel @@ -4,6 +4,6 @@ open "/projects/1" click "edit_icon_todo_15" wait_for_element_present "show_from_todo_15" type "show_from_todo_15", "" -click "//input[@value='Update']" +click "css=#submit_todo_15" wait_for_element_present "xpath=//div[@id='p1'] //div[@id='todo_15']" wait_for_visible "tickler-empty-nd" diff --git a/tracks/test/selenium/project_detail/change_todos_project_to_blank.rsel b/tracks/test/selenium/project_detail/change_todos_project_to_blank.rsel index 1cf36ffd..e9429d20 100644 --- a/tracks/test/selenium/project_detail/change_todos_project_to_blank.rsel +++ b/tracks/test/selenium/project_detail/change_todos_project_to_blank.rsel @@ -4,6 +4,6 @@ open "/projects/1" click "edit_icon_todo_5" wait_for_element_present "show_from_todo_5" type "project_name_todo_5", "" -click "//input[@value='Update']" +click "css=#submit_todo_5" wait_for_element_not_present "todo_5" assert_text 'badge_count', '1' \ No newline at end of file diff --git a/tracks/test/selenium/project_detail/defer_todo.rsel b/tracks/test/selenium/project_detail/defer_todo.rsel index de329264..56b98336 100644 --- a/tracks/test/selenium/project_detail/defer_todo.rsel +++ b/tracks/test/selenium/project_detail/defer_todo.rsel @@ -4,7 +4,7 @@ open "/projects/1" click "edit_icon_todo_5" wait_for_element_present "show_from_todo_5" type "show_from_todo_5", "1/1/2030" -click "//input[@value='Update']" +click "css=#submit_todo_5" wait_for_element_present "xpath=//div[@id='tickler'] //div[@id='todo_5']" assert_not_visible "tickler-empty-nd" assert_text 'badge_count', '1' diff --git a/tracks/test/test_helper.rb b/tracks/test/test_helper.rb index 2ce2327f..0464bca2 100644 --- a/tracks/test/test_helper.rb +++ b/tracks/test/test_helper.rb @@ -31,6 +31,12 @@ class Test::Unit::TestCase end alias_method :assert_errors_on, :assert_error_on + + def assert_value_changed(object, method = nil) + initial_value = object.send(method) + yield + assert_not_equal initial_value, object.send(method), "#{object}##{method}" + end end diff --git a/tracks/test/unit/user_test.rb b/tracks/test/unit/user_test.rb index d10cf65d..df2e76da 100644 --- a/tracks/test/unit/user_test.rb +++ b/tracks/test/unit/user_test.rb @@ -33,7 +33,7 @@ class UserTest < Test::Rails::TestCase assert_kind_of User, @admin_user assert_equal 1, @admin_user.id assert_equal "admin", @admin_user.login - assert_equal "#{Digest::SHA1.hexdigest("#{Tracks::Config.salt}--abracadabra--")}", @admin_user.password + assert_equal "#{Digest::SHA1.hexdigest("#{Tracks::Config.salt}--abracadabra--")}", @admin_user.crypted_password assert_not_nil @admin_user.word assert @admin_user.is_admin end @@ -43,7 +43,7 @@ class UserTest < Test::Rails::TestCase assert_kind_of User, @other_user assert_equal 2, @other_user.id assert_equal "jane", @other_user.login - assert_equal "#{Digest::SHA1.hexdigest("#{Tracks::Config.salt}--sesame--")}", @other_user.password + assert_equal "#{Digest::SHA1.hexdigest("#{Tracks::Config.salt}--sesame--")}", @other_user.crypted_password assert_not_nil @other_user.word assert @other_user.is_admin == false || @other_user.is_admin == 0 end @@ -179,9 +179,9 @@ class UserTest < Test::Rails::TestCase end def test_crypt_word_updates_word - old_word = @admin_user.word - @admin_user.crypt_word - assert_not_equal old_word, @admin_user.word + assert_value_changed @admin_user, :word do + @admin_user.send :crypt_word + end end def test_find_admin