From 9f94a70c3e3d9bf766cb9663b0a904d30a190d85 Mon Sep 17 00:00:00 2001 From: simon Date: Sun, 8 Feb 2009 23:15:11 +0100 Subject: * initial commit of the stripped restful-authentication * http basic auth and login from cookie have been removed * no it does not work yet, it's so f*cking secure, it won't even let legitimate users login --- app/controllers/application_controller.rb | 4 +- app/controllers/sessions_controller.rb | 40 +++++++++ app/helpers/sessions_helper.rb | 2 + app/models/user.rb | 34 ++++++++ app/views/sessions/new.html.erb | 16 ++++ config/initializers/site_keys.rb | 37 ++++++++ config/routes.rb | 6 +- lib/authenticated_system.rb | 127 ++++++++++++++++++++++++++++ lib/authenticated_test_helper.rb | 6 ++ lib/authentication.rb | 103 ++++++++++++++++++++++ test/fixtures/users.yml | 27 +++--- test/functional/sessions_controller_test.rb | 82 ++++++++++++++++++ test/unit/user_test.rb | 64 +++++++++++++- 13 files changed, 531 insertions(+), 17 deletions(-) create mode 100644 app/controllers/sessions_controller.rb create mode 100644 app/helpers/sessions_helper.rb create mode 100644 app/views/sessions/new.html.erb create mode 100644 config/initializers/site_keys.rb create mode 100644 lib/authenticated_system.rb create mode 100644 lib/authenticated_test_helper.rb create mode 100644 lib/authentication.rb create mode 100644 test/functional/sessions_controller_test.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6635a3f..b481317 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -2,9 +2,11 @@ # Likewise, all the methods added will be available for all controllers. class ApplicationController < ActionController::Base + include AuthenticatedSystem + helper :all # include all helpers, all the time protect_from_forgery # See ActionController::RequestForgeryProtection for details # Scrub sensitive parameters from your log - # filter_parameter_logging :password + filter_parameter_logging :password, :password_confirmation end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb new file mode 100644 index 0000000..7c06ac8 --- /dev/null +++ b/app/controllers/sessions_controller.rb @@ -0,0 +1,40 @@ +# This controller handles the login/logout function of the site. +class SessionsController < ApplicationController + + # render new.rhtml + def new + end + + def create + logout_keeping_session! + user = User.authenticate(params[:login], params[:password]) + if user + # Protects against session fixation attacks, causes request forgery + # protection if user resubmits an earlier form using back + # button. Uncomment if you understand the tradeoffs. + reset_session + + self.current_user = user + redirect_back_or_default('/') # TODO: insert appropriate path to cms main page + flash[:notice] = "Logged in successfully" + else + note_failed_signin + @login = params[:login] + @remember_me = params[:remember_me] + render :action => 'new' + end + end + + def destroy + logout_killing_session! + flash[:notice] = "You have been logged out." + redirect_back_or_default('/login') + end + +protected + # Track failed login attempts + def note_failed_signin + flash[:error] = "Couldn't log you in as '#{params[:login]}'" + logger.warn "Failed login for '#{params[:login]}' from #{request.remote_ip} at #{Time.now.utc}" + end +end diff --git a/app/helpers/sessions_helper.rb b/app/helpers/sessions_helper.rb new file mode 100644 index 0000000..f54a8fc --- /dev/null +++ b/app/helpers/sessions_helper.rb @@ -0,0 +1,2 @@ +module SessionsHelper +end \ No newline at end of file diff --git a/app/models/user.rb b/app/models/user.rb index 4a57cf0..3ac0712 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,2 +1,36 @@ +require 'digest/sha1' + class User < ActiveRecord::Base + include Authentication + include Authentication::ByPassword + + validates_presence_of :login + validates_length_of :login, :within => 3..40 + validates_uniqueness_of :login + validates_format_of :login, :with => Authentication.login_regex, + :message => Authentication.bad_login_message + + validates_presence_of :email + validates_length_of :email, :within => 6..100 #r@a.wk + validates_uniqueness_of :email + validates_format_of :email, :with => Authentication.email_regex, + :message => Authentication.bad_email_message + + attr_accessible :login, :email, :password, :password_confirmation + + # Authenticates a user by their login name and unencrypted password. Returns the user or nil. + def self.authenticate(login, password) + return nil if login.blank? || password.blank? + u = find_by_login(login) # need to get the salt + u && u.authenticated?(password) ? u : nil + end + + # TODO: Do we really want to have downcase logins only? + def login=(value) + write_attribute :login, (value ? value.downcase : nil) + end + + def email=(value) + write_attribute :email, (value ? value.downcase : nil) + end end diff --git a/app/views/sessions/new.html.erb b/app/views/sessions/new.html.erb new file mode 100644 index 0000000..84f8469 --- /dev/null +++ b/app/views/sessions/new.html.erb @@ -0,0 +1,16 @@ +

Log In

+ +<% form_tag session_path do -%> +

<%= label_tag 'login' %>
+<%= text_field_tag 'login', @login %>

+ +

<%= label_tag 'password' %>
+<%= password_field_tag 'password', nil %>

+ + + +

<%= submit_tag 'Log in' %>

+<% end -%> diff --git a/config/initializers/site_keys.rb b/config/initializers/site_keys.rb new file mode 100644 index 0000000..c66fee2 --- /dev/null +++ b/config/initializers/site_keys.rb @@ -0,0 +1,37 @@ +# A Site key gives additional protection against a dictionary attack if your +# DB is ever compromised. With no site key, we store +# DB_password = hash(user_password, DB_user_salt) +# If your database were to be compromised you'd be vulnerable to a dictionary +# attack on all your stupid users' passwords. With a site key, we store +# DB_password = hash(user_password, DB_user_salt, Code_site_key) +# That means an attacker needs access to both your site's code *and* its +# database to mount an "offline dictionary attack.":http://www.dwheeler.com/secure-programs/Secure-Programs-HOWTO/web-authentication.html +# +# It's probably of minor importance, but recommended by best practices: 'defense +# in depth'. Needless to say, if you upload this to github or the youtubes or +# otherwise place it in public view you'll kinda defeat the point. Your users' +# passwords are still secure, and the world won't end, but defense_in_depth -= 1. +# +# Please note: if you change this, all the passwords will be invalidated, so DO +# keep it someplace secure. Use the random value given or type in the lyrics to +# your favorite Jay-Z song or something; any moderately long, unpredictable text. +# TODO: Change the site key when deploying +REST_AUTH_SITE_KEY = 'THIS_KEY_SHOULD_BE_CHANGED_UPON_DEPLOYMENT!!!1ELF' + +# Repeated applications of the hash make brute force (even with a compromised +# database and site key) harder, and scale with Moore's law. +# +# bq. "To squeeze the most security out of a limited-entropy password or +# passphrase, we can use two techniques [salting and stretching]... that are +# so simple and obvious that they should be used in every password system. +# There is really no excuse not to use them." http://tinyurl.com/37lb73 +# Practical Security (Ferguson & Scheier) p350 +# +# A modest 10 foldings (the default here) adds 3ms. This makes brute forcing 10 +# times harder, while reducing an app that otherwise serves 100 reqs/s to 78 signin +# reqs/s, an app that does 10reqs/s to 9.7 reqs/s +# +# More: +# * http://www.owasp.org/index.php/Hashing_Java +# * "An Illustrated Guide to Cryptographic Hashes":http://www.unixwiz.net/techtips/iguide-crypto-hashes.html +REST_AUTH_DIGEST_STRETCHES = 10 diff --git a/config/routes.rb b/config/routes.rb index c436b00..2ab68c2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,7 +1,11 @@ ActionController::Routing::Routes.draw do |map| map.resources :pages map.resources :nodes - + + map.logout '/logout', :controller => 'sessions', :action => 'destroy' + map.login '/login', :controller => 'sessions', :action => 'new' + map.resources :users + map.resource :session map.connect ':language/*page_path', :controller => 'content', :action => 'render_page', diff --git a/lib/authenticated_system.rb b/lib/authenticated_system.rb new file mode 100644 index 0000000..7b813f4 --- /dev/null +++ b/lib/authenticated_system.rb @@ -0,0 +1,127 @@ +module AuthenticatedSystem + protected + # Returns true or false if the user is logged in. + # Preloads @current_user with the user model if they're logged in. + def logged_in? + !!current_user + end + + # Accesses the current user from the session. + # Future calls avoid the database because nil is not equal to false. + def current_user + @current_user ||= login_from_session unless @current_user == false + end + + # Store the given user id in the session. + def current_user=(new_user) + session[:user_id] = new_user ? new_user.id : nil + @current_user = new_user || false + end + + # Check if the user is authorized + # + # Override this method in your controllers if you want to restrict access + # to only a few actions or if you want to check if the user + # has the correct rights. + # + # Example: + # + # # only allow nonbobs + # def authorized? + # current_user.login != "bob" + # end + # + def authorized?(action = action_name, resource = nil) + logged_in? + end + + # Filter method to enforce a login requirement. + # + # To require logins for all actions, use this in your controllers: + # + # before_filter :login_required + # + # To require logins for specific actions, use this in your controllers: + # + # before_filter :login_required, :only => [ :edit, :update ] + # + # To skip this in a subclassed controller: + # + # skip_before_filter :login_required + # + def login_required + authorized? || access_denied + end + + # Redirect as appropriate when an access request fails. + # + # The default action is to redirect to the login screen. + # + # Override this method in your controllers if you want to have special + # behavior in case the user is not authorized + # to access the requested action. For example, a popup window might + # simply close itself. + def access_denied + respond_to do |format| + format.html do + store_location + redirect_to new_session_path + end + end + end + + # Store the URI of the current request in the session. + # + # We can return to this location by calling #redirect_back_or_default. + def store_location + session[:return_to] = request.request_uri + end + + # Redirect to the URI stored by the most recent store_location call or + # to the passed default. Set an appropriately modified + # after_filter :store_location, :only => [:index, :new, :show, :edit] + # for any controller you want to be bounce-backable. + def redirect_back_or_default(default) + redirect_to(session[:return_to] || default) + session[:return_to] = nil + end + + # Inclusion hook to make #current_user and #logged_in? + # available as ActionView helper methods. + def self.included(base) + base.send :helper_method, :current_user, :logged_in?, :authorized? if base.respond_to? :helper_method + end + + # + # Login + # + + # Called from #current_user. First attempt to login by the user id stored in the session. + def login_from_session + self.current_user = User.find_by_id(session[:user_id]) if session[:user_id] + end + + # + # Logout + # + + # This is ususally what you want; resetting the session willy-nilly wreaks + # havoc with forgery protection, and is only strictly necessary on login. + # However, **all session state variables should be unset here**. + def logout_keeping_session! + # Kill server-side auth cookie + @current_user.forget_me if @current_user.is_a? User + @current_user = false # not logged in, and don't do it for me + kill_remember_cookie! # Kill client-side auth cookie + session[:user_id] = nil # keeps the session but kill our variable + # explicitly kill any other session variables you set + end + + # The session should only be reset at the tail end of a form POST -- + # otherwise the request forgery protection fails. It's only really necessary + # when you cross quarantine (logged-out to logged-in). + def logout_killing_session! + logout_keeping_session! + reset_session + end +end diff --git a/lib/authenticated_test_helper.rb b/lib/authenticated_test_helper.rb new file mode 100644 index 0000000..c0ec5f4 --- /dev/null +++ b/lib/authenticated_test_helper.rb @@ -0,0 +1,6 @@ +module AuthenticatedTestHelper + # Sets the current user in the session from the user fixtures. + def login_as(user) + @request.session[:user_id] = user ? users(user).id : nil + end +end diff --git a/lib/authentication.rb b/lib/authentication.rb new file mode 100644 index 0000000..a936589 --- /dev/null +++ b/lib/authentication.rb @@ -0,0 +1,103 @@ +module Authentication + mattr_accessor :login_regex, :bad_login_message, + :name_regex, :bad_name_message, + :email_name_regex, :domain_head_regex, :domain_tld_regex, :email_regex, :bad_email_message + + self.login_regex = /\A\w[\w\.\-_@]+\z/ # ASCII, strict + # self.login_regex = /\A[[:alnum:]][[:alnum:]\.\-_@]+\z/ # Unicode, strict + # self.login_regex = /\A[^[:cntrl:]\\<>\/&]*\z/ # Unicode, permissive + + self.bad_login_message = "use only letters, numbers, and .-_@ please.".freeze + + self.name_regex = /\A[^[:cntrl:]\\<>\/&]*\z/ # Unicode, permissive + self.bad_name_message = "avoid non-printing characters and \\><&/ please.".freeze + + self.email_name_regex = '[\w\.%\+\-]+'.freeze + self.domain_head_regex = '(?:[A-Z0-9\-]+\.)+'.freeze + self.domain_tld_regex = '(?:[A-Z]{2}|com|org|net|edu|gov|mil|biz|info|mobi|name|aero|jobs|museum)'.freeze + self.email_regex = /\A#{email_name_regex}@#{domain_head_regex}#{domain_tld_regex}\z/i + self.bad_email_message = "should look like an email address.".freeze + + def self.included(recipient) + recipient.extend(ModelClassMethods) + recipient.class_eval do + include ModelInstanceMethods + end + end + + module ModelClassMethods + def secure_digest(*args) + Digest::SHA1.hexdigest(args.flatten.join('--')) + end + + def make_token + secure_digest(Time.now, (1..10).map{ rand.to_s }) + end + end # class methods + + module ModelInstanceMethods + end # instance methods + + module ByPassword + # Stuff directives into including module + def self.included(recipient) + recipient.extend(ModelClassMethods) + recipient.class_eval do + include ModelInstanceMethods + + # Virtual attribute for the unencrypted password + attr_accessor :password + validates_presence_of :password, :if => :password_required? + validates_presence_of :password_confirmation, :if => :password_required? + validates_confirmation_of :password, :if => :password_required? + validates_length_of :password, :within => 6..40, :if => :password_required? + before_save :encrypt_password + end + end # #included directives + + # + # Class Methods + # + module ModelClassMethods + # This provides a modest increased defense against a dictionary attack if + # your db were ever compromised, but will invalidate existing passwords. + # See the README and the file config/initializers/site_keys.rb + # + # It may not be obvious, but if you set REST_AUTH_SITE_KEY to nil and + # REST_AUTH_DIGEST_STRETCHES to 1 you'll have backwards compatibility with + # older versions of restful-authentication. + def password_digest(password, salt) + digest = REST_AUTH_SITE_KEY + REST_AUTH_DIGEST_STRETCHES.times do + digest = secure_digest(digest, salt, password, REST_AUTH_SITE_KEY) + end + digest + end + end # class methods + + # + # Instance Methods + # + module ModelInstanceMethods + + # Encrypts the password with the user salt + def encrypt(password) + self.class.password_digest(password, salt) + end + + def authenticated?(password) + crypted_password == encrypt(password) + end + + # before filter + def encrypt_password + return if password.blank? + self.salt = self.class.make_token if new_record? + self.crypted_password = encrypt(password) + end + def password_required? + crypted_password.blank? || !password.blank? + end + end # instance methods + end +end \ No newline at end of file diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 74fafbd..3abe206 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -1,13 +1,18 @@ -# Read about fixtures at http://ar.rubyonrails.org/classes/Fixtures.html -one: - login: MyString - email: MyString - crypted_password: MyString - salt: MyString +quentin: + id: 1 + login: quentin + email: quentin@example.com + salt: 356a192b7913b04c54574d18c28d46e6395428ab # SHA1('0') + crypted_password: 89e27e324f2dee0fb72034631aa1bc3ca28ea574 # 'monkey' + created_at: <%= 5.days.ago.to_s :db %> + +aaron: + id: 2 + login: aaron + email: aaron@example.com + salt: da4b9237bacccdf19c0760cab7aec4a8359010b0 # SHA1('1') + crypted_password: cf39f8e6972c25ac72ccc801cab755ef15bca09b # 'monkey' + created_at: <%= 1.days.ago.to_s :db %> + -two: - login: MyString - email: MyString - crypted_password: MyString - salt: MyString diff --git a/test/functional/sessions_controller_test.rb b/test/functional/sessions_controller_test.rb new file mode 100644 index 0000000..e53bcd8 --- /dev/null +++ b/test/functional/sessions_controller_test.rb @@ -0,0 +1,82 @@ +require File.dirname(__FILE__) + '/../test_helper' +require 'sessions_controller' + +# Re-raise errors caught by the controller. +class SessionsController; def rescue_action(e) raise e end; end + +class SessionsControllerTest < ActionController::TestCase + # Be sure to include AuthenticatedTestHelper in test/test_helper.rb instead + # Then, you can remove it from this and the units test. + include AuthenticatedTestHelper + + fixtures :users + + def test_should_login_and_redirect + post :create, :login => 'quentin', :password => 'monkey' + assert session[:user_id] + assert_response :redirect + end + + def test_should_fail_login_and_not_redirect + post :create, :login => 'quentin', :password => 'bad password' + assert_nil session[:user_id] + assert_response :success + end + + def test_should_logout + login_as :quentin + get :destroy + assert_nil session[:user_id] + assert_response :redirect + end + + def test_should_remember_me + @request.cookies["auth_token"] = nil + post :create, :login => 'quentin', :password => 'monkey', :remember_me => "1" + assert_not_nil @response.cookies["auth_token"] + end + + def test_should_not_remember_me + @request.cookies["auth_token"] = nil + post :create, :login => 'quentin', :password => 'monkey', :remember_me => "0" + puts @response.cookies["auth_token"] + assert @response.cookies["auth_token"].blank? + end + + def test_should_delete_token_on_logout + login_as :quentin + get :destroy + assert @response.cookies["auth_token"].blank? + end + + def test_should_login_with_cookie + users(:quentin).remember_me + @request.cookies["auth_token"] = cookie_for(:quentin) + get :new + assert @controller.send(:logged_in?) + end + + def test_should_fail_expired_cookie_login + users(:quentin).remember_me + users(:quentin).update_attribute :remember_token_expires_at, 5.minutes.ago + @request.cookies["auth_token"] = cookie_for(:quentin) + get :new + assert !@controller.send(:logged_in?) + end + + def test_should_fail_cookie_login + users(:quentin).remember_me + @request.cookies["auth_token"] = auth_token('invalid_auth_token') + get :new + assert !@controller.send(:logged_in?) + end + + protected + 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/test/unit/user_test.rb b/test/unit/user_test.rb index a64d2d3..47e3129 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -1,8 +1,64 @@ -require 'test_helper' +require File.dirname(__FILE__) + '/../test_helper' class UserTest < ActiveSupport::TestCase - # Replace this with your real tests. - test "the truth" do - assert true + # Be sure to include AuthenticatedTestHelper in test/test_helper.rb instead. + # Then, you can remove it from this and the functional test. + include AuthenticatedTestHelper + fixtures :users + + def test_should_create_user + assert_difference 'User.count' do + user = create_user + assert !user.new_record?, "#{user.errors.full_messages.to_sentence}" + end + end + + def test_should_require_login + assert_no_difference 'User.count' do + u = create_user(:login => nil) + assert u.errors.on(:login) + end + end + + def test_should_require_password + assert_no_difference 'User.count' do + u = create_user(:password => nil) + assert u.errors.on(:password) + end + end + + def test_should_require_password_confirmation + assert_no_difference 'User.count' do + u = create_user(:password_confirmation => nil) + assert u.errors.on(:password_confirmation) + end + end + + def test_should_require_email + assert_no_difference 'User.count' do + u = create_user(:email => nil) + assert u.errors.on(:email) + end + end + + def test_should_reset_password + users(:quentin).update_attributes(:password => 'new password', :password_confirmation => 'new password') + assert_equal users(:quentin), User.authenticate('quentin', 'new password') + end + + def test_should_not_rehash_password + users(:quentin).update_attributes(:login => 'quentin2') + assert_equal users(:quentin), User.authenticate('quentin2', 'monkey') + end + + def test_should_authenticate_user + assert_equal users(:quentin), User.authenticate('quentin', 'monkey') + end + +protected + def create_user(options = {}) + record = User.new({ :login => 'quire', :email => 'quire@example.com', :password => 'quire69', :password_confirmation => 'quire69' }.merge(options)) + record.save + record end end -- cgit v1.3