Remove support for deprecated password-hashing algorithm

* Remove all methods implementing and checking for the old algorithm
* Document a pre-upgrade step to check for remaining obsolete passwords
* Remove config.salt

app/controllers/application_controller.rb

@@ -12,7 +12,6 @@ class ApplicationController < ActionController::Base
   layout proc{ |controller| controller.mobile? ? "mobile" : "application" }
   # exempt_from_layout /\.js\.erb$/
 
-  before_filter :check_for_deprecated_password_hash
   before_filter :set_session_expiration
   before_filter :set_time_zone
   before_filter :set_zindex_counter
@@ -58,15 +57,6 @@ class ApplicationController < ActionController::Base
     end
   end
 
-  # Redirects to change_password_user_path if the current user uses a
-  # deprecated password hashing algorithm.
-  def check_for_deprecated_password_hash
-    if current_user and current_user.uses_deprecated_password?
-      notify :warning, t('users.you_have_to_reset_your_password')
-      redirect_to change_password_user_path current_user
-    end
-  end
-
   def render_failure message, status = 404
     render :text => message, :status => status
   end

app/models/user.rb

@@ -185,25 +185,8 @@ class User < ActiveRecord::Base
     save
   end
 
-  # Returns true if the user has a password hashed using SHA-1.
-  def uses_deprecated_password?
-    crypted_password =~ /^[a-f0-9]{40}$/i
-  end
-
   def password_matches?(pass)
-    if uses_deprecated_password?
-      crypted_password == sha1(pass)
-    else
-      BCrypt::Password.new(crypted_password) == pass
-    end
-  end
-
-  def salted(s)
-    "#{Tracks::Config.salt}--#{s}--"
-  end
-
-  def sha1(s)
-    Digest::SHA1.hexdigest(salted(s))
+    BCrypt::Password.new(crypted_password) == pass
   end
 
   def create_hash(s)

config/environments/cucumber.rb

@@ -38,7 +38,5 @@ Rails.application.configure do
   # Unique cookies and use cookies for session
   # config.action_controller.session_store :cookie_store, :key => 'TracksCucumber'
 
-  SITE_CONFIG['salt'] ||= 'change-me'
-  
   config.time_zone = 'UTC'
 end

config/environments/test.rb

@@ -34,8 +34,6 @@ Rails.application.configure do
   # Print deprecation notices to the stderr.
   config.active_support.deprecation = :stderr
 
-  SITE_CONFIG['salt'] ||= 'change-me'
-  
   config.time_zone = 'UTC'
 
   # Raises error for missing translations

config/site.yml.tmpl

@@ -1,8 +1,3 @@
-# This is the 'salt' to add to the password before it is encrypted
-# You need to change this to something unique for yourself
-salt: "change-me"
-
-
 # NOTE: openid, ldap and cas are currently not supported anymore.
 authentication_schemes:
   - "database"

doc/CHANGELOG.md

@@ -1,4 +1,7 @@
 ## Version 2.4
+* Removed support for deprecated password-hashing algorithm. This
+  eliminates config.salt. Note the addition of a pre-upgrade step to
+  check for obsolete passwords.
 
 ## Version 2.3
 

doc/installation.md

@@ -58,7 +58,7 @@ Tracks is built upon a number of Ruby libraries (known as ‘gems’). The Bundl
 
 1. In the `config` folder, copy the files `database.yml.tmpl` and `site.yml.tmpl` to `database.yml` and `site.yml`, respectively.
 2. Open the file `config/database.yml` and edit the `production:` section with the details of your database. If you are using MySQL the `adapter:` line should read `adapter: mysql2`, `host: localhost` (in the majority of cases), and your username and password should match those you assigned when you created the database. If you are using SQLite3, you should have only two lines under the production section: `adapter: sqlite3` and `database: db/tracks.db`.
-3. Open the file `config/site.yml`, and read through the settings to make sure that they suit your setup. In most cases, all you need to change are the `salt: "change-me"` line (change the string “change-me” to some other string of your choice), the administrator email address (`admin_email`), and the time zone setting. For the time zone setting you can use the command `bundle exec rake time:zones:local` to see all available timezones on your machine
+3. Open the file `config/site.yml`, and read through the settings to make sure that they suit your setup. In most cases, all you need to change are the `secret_token`, the administrator email address (`admin_email`), and the time zone setting. For the time zone setting you can use the command `bundle exec rake time:zones:local` to see all available timezones on your machine
 4. If you are using Windows, you may need to check the ‘shebang’ lines (`#!/usr/bin/env ruby`) of the `/public/dispatch.*` files and all the files in the `/script` directory. They are set to `#!/usr/bin/env ruby` by default. This should work for all Unix based setups (Linux or Mac OS X), but Windows users will probably have to change it to something like `#c:/ruby/bin/ruby` to point to the Ruby binary on your system.
 5. If you intend to deploy Tracks with the built in webserver called WEBrick, you’ll need to change `config.serve_static_assets` to `true` in `config/environments/production.rb` in order for the images, stylesheets, and javascript files to be served correctly.
 

doc/upgrading.md

@@ -6,13 +6,14 @@ WARNING: 2.4devel is a development tree of Tracks and may (will?) contain bugs t
 
 That said. To upgrade:
 
-1. Back up your existing database and installation of Tracks
-2. Install Tracks 2.4devel in a new directory. Or you can create a separate installation of 2.4devel for testing purposes.
-3. Copy over the configuration from your previous Tracks installation. If using SQLite3, copy the old database into the new Tracks 2.4devel directory.
-4. Check that you have all dependencies installed: `bundle install --without development test` Or leave out the `--without development test` part if you intend to test or develop on this tree.
-5. Run `bundle exec rake db:migrate RAILS_ENV=production` to update your old database to the new schema. This is the point of no return. Make sure you have backups!
-6. Precompile your static assets (css, javascript, etc.) by running `bundle exec rake assets:precompile RAILS_ENV=production`.
-7. Run `bundle exec rails server -e production` inside your Tracks 2.4devel directory to start up Tracks. Or use `-e development` if you intend to try your changes and get more log info.
+1. Support for Tracks' older, less secure password-hashing algorithm has been removed. Active user accounts will already have been upgraded, but you should check for outdated accounts by running `bundle exec rake tracks:check_passwords` from your existing Tracks install directory. If any users are listed, they can log in to be prompted to update their passwords, or you can run `bundle exec rake tracks:password USER=<username>` to reset their passwords using the new algorithm. If no users are listed, you can proceed with the upgrade.
+2. Back up your existing database and installation of Tracks
+3. Install Tracks 2.4devel in a new directory. Or you can create a separate installation of 2.4devel for testing purposes.
+4. Copy over the configuration from your previous Tracks installation. If using SQLite3, copy the old database into the new Tracks 2.4devel directory.
+5. Check that you have all dependencies installed: `bundle install --without development test` Or leave out the `--without development test` part if you intend to test or develop on this tree.
+6. Run `bundle exec rake db:migrate RAILS_ENV=production` to update your old database to the new schema. This is the point of no return. Make sure you have backups!
+7. Precompile your static assets (css, javascript, etc.) by running `bundle exec rake assets:precompile RAILS_ENV=production`.
+8. Run `bundle exec rails server -e production` inside your Tracks 2.4devel directory to start up Tracks. Or use `-e development` if you intend to try your changes and get more log info.
 
 Please note that if you intend to use Tracks with the built in webserver called WEBrick for production, you’ll need to change `config.serve_static_assets` to `true` in `config/environments/production.rb` in order for the images, stylesheets, and javascript files to be served correctly.
 

features/handling_users_with_deprecated_password_hashes.feature

@@ -1,35 +0,0 @@
-Feature: Handling users with deprecated passwords hashes
-  In order to have my password hashed with BCrypt
-  As a user with password hashed with SHA1
-  I have to be redirected to the password resetting form
-
-  Background:
-    Given the following user records with hash algorithm
-      | login         | password        | algorithm  |
-      | new_hash_user | first_secret    | bcrypt     |
-      | old_hash_user | another_secret  | sha1       |
-
-  Scenario Outline: A user with SHA1 password
-    Given I have logged in as "old_hash_user" with password "another_secret"
-    When I go to the <name> page
-    Then I should be redirected to the change password page
-    And I should see "You have to reset your password"
-    When I change my password to "newer_better_password"
-    Then I should be redirected to the preference page
-
-    Examples:
-      | name        |
-      | home        |
-      | preferences |
-      | notes       |
-      | tickler     |
-
-  Scenario: A user with SHA1 password goes straight to the change password page
-    Given I have logged in as "old_hash_user" with password "another_secret"
-    When I go to the change password page
-    Then I should be on the change password page
-
-  Scenario: A user with BCrypt password
-    Given I have logged in as "new_hash_user" with password "first_secret"
-    When I go to the homepage
-    Then I should be on the homepage

features/step_definitions/user_steps.rb

@@ -6,34 +6,6 @@ Given /^the following user records?$/ do |table|
   end
 end
 
-Given /^the following user records with hash algorithm$/ do |table|
-  User.delete_all
-  table.hashes.each do | hash |
-    password = hash[:password]
-    algorithm = hash[:algorithm]
-    hash.delete("algorithm")
-
-    user = FactoryGirl.create(:user, hash)
-
-    case algorithm
-    when 'bcrypt'
-      user.change_password( password, password )
-      user.reload
-      expect(BCrypt::Password.new(user.crypted_password)).to eq(password)
-    when 'sha1'
-      user.password = user.password_confirmation = nil
-      user.send(:write_attribute, :crypted_password, user.sha1(password))
-      user.save
-      user.reload
-      expect(user.crypted_password).to eq(user.sha1(password))
-    else
-      raise "Unknown hashing algorithm: #{algorithm}"
-    end
-
-    user.create_preference({:locale => 'en'})
-  end
-end
-
 Given("no users exists") do
   User.delete_all
 end

lib/tasks/tracks.rake

@@ -24,15 +24,5 @@ namespace :tracks do
       user.errors.full_messages.each { |msg| puts "- #{msg}\n" }
     end
   end
-
-  desc 'Check all passwords for deprecated hashes'
-  task :check_passwords => :environment do
-    puts "The following users have deprecated password hashes:"
-    User.all.each do |user|
-      if user.uses_deprecated_password?
-        puts "  #{user.login}"
-      end
-    end
-  end
 end
 

lib/tracks/config.rb

@@ -1,10 +1,6 @@
 module Tracks
   
   class Config
-    
-    def self.salt
-       SITE_CONFIG['salt']
-    end
 
     def self.auth_schemes
        SITE_CONFIG['authentication_schemes'] || []
@@ -28,4 +24,4 @@ module Tracks
     
   end
   
-end
+end

test/controllers/preferences_controller_test.rb

@@ -2,11 +2,6 @@ require 'test_helper'
 
 class PreferencesControllerTest < ActionController::TestCase
 
-  def setup
-    super
-    assert_equal "change-me", Tracks::Config.salt
-  end
-
   test "render_date_format requires login" do
     get :render_date_format
     assert_redirected_to login_path

test/controllers/users_controller_test.rb

@@ -18,7 +18,7 @@ class UsersControllerTest < ActionController::TestCase
     get :index
     assert_response :success
     assert_equal "TRACKS::Manage Users", assigns['page_title']
-    assert_equal 5, assigns['total_users']
+    assert_equal 4, assigns['total_users']
     assert_equal users_url, session['return-to']
   end
 

test/fixtures/users.yml

@@ -48,13 +48,3 @@ ldap_user:
   first_name: International
   last_name: Harvester
   auth_type: CAS
-
-user_with_sha1_password:
-  id: 6
-  login: mr_deprecated
-  crypted_password: <%= Digest::SHA1::hexdigest("#{Tracks::Config.salt}--foobar--") %>
-  token: <%= Digest::SHA1.hexdigest("mr_deprecatedSun Feb 19 14:42:45 GMT 20060.408173979260027") %>
-  is_admin: false
-  first_name: Mister
-  last_name: Deprecated
-  auth_type: database

test/integration/users_xml_api_test.rb

@@ -70,7 +70,7 @@ class UsersXmlApiTest < ActionDispatch::IntegrationTest
     get '/users.xml', {}, basic_auth_headers()
     assert_response :success
     assert_tag :tag => "users",
-               :children => { :count => 5, :only => { :tag => "user" } }
+               :children => { :count => 4, :only => { :tag => "user" } }
     assert_no_tag :tag => "password"
   end
 

test/models/preference_test.rb

@@ -4,7 +4,6 @@ class PreferenceTest < ActiveSupport::TestCase
   fixtures :users, :preferences
 
   def setup
-    assert_equal "change-me", Tracks::Config.salt
     @admin_user = User.find(1)
     @other_user = User.find(2)
   end

test/models/user_test.rb

@@ -4,7 +4,6 @@ class UserTest < ActiveSupport::TestCase
   fixtures :users, :preferences, :projects, :contexts, :todos, :recurring_todos
 
   def setup
-    assert_equal "change-me", Tracks::Config.salt
     @admin_user = User.find(1)
     @other_user = User.find(2)
   end
@@ -287,31 +286,10 @@ class UserTest < ActiveSupport::TestCase
     users(:other_user).forget_me
     assert_nil users(:other_user).remember_token
   end
-  
-  def test_should_discover_using_depracted_password
-    assert_nil @admin_user.uses_deprecated_password?
-    assert_nil @other_user.uses_deprecated_password?
-    assert users(:user_with_sha1_password).uses_deprecated_password?
-  end
-
-  def test_should_not_have_deprecated_password_after_update
-    u = users(:user_with_sha1_password)
-    assert u.uses_deprecated_password?
-    u.change_password("foobar", "foobar")
-    assert_nil u.uses_deprecated_password?
-  end
-
-  def test_should_authenticate_with_deprecated_password
-    assert_nil User.authenticate('mr_deprecated', 'wrong password')
-    assert_equal users(:user_with_sha1_password),
-      User.authenticate('mr_deprecated', 'foobar')
-  end
 
   def test_password_matches
     assert_not_nil User.authenticate(@admin_user.login, "abracadabra")
     assert_nil User.authenticate(@admin_user.login, "incorrect")
-    assert_not_nil User.authenticate(users(:user_with_sha1_password).login, "foobar")
-    assert_nil User.authenticate(users(:user_with_sha1_password).login, "wrong")
   end
 
   def test_update_positions_of_contexts

test/test_helper.rb

@@ -3,7 +3,7 @@ require File.expand_path('../../config/environment', __FILE__)
 require 'rails/test_help'
 
 # set config for tests. Overwrite those read from config/site.yml. Use inject to avoid warning about changing CONSTANT
-{ "salt" => "change-me", "authentication_schemes" => ["database"], "prefered_auth" => "database", "email_dispatch" => nil}.inject( SITE_CONFIG ) { |h, elem| h[elem[0]] = elem[1]; h }
+{ "authentication_schemes" => ["database"], "prefered_auth" => "database", "email_dispatch" => nil}.inject( SITE_CONFIG ) { |h, elem| h[elem[0]] = elem[1]; h }
 
 class ActiveSupport::TestCase
   # Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order.