From d324f4a0710a6a30178c1ae25382590519ee6754 Mon Sep 17 00:00:00 2001 From: albert Date: Sat, 15 Oct 2011 16:36:07 -0400 Subject: [PATCH] refactored login process, added remember option for login --- .../stylesheets/specific/sessions.css.scss | 15 +++--- app/controllers/application_controller.rb | 16 +----- app/controllers/sessions_controller.rb | 7 ++- app/logical/session_creator.rb | 28 ++++++++++ app/logical/session_loader.rb | 51 +++++++++++++++++++ app/models/ban.rb | 4 ++ app/models/user.rb | 38 ++++++++++---- app/views/sessions/new.html.erb | 3 ++ db/development_structure.sql | 3 +- test/unit/user_test.rb | 14 +++++ 10 files changed, 142 insertions(+), 37 deletions(-) create mode 100644 app/logical/session_creator.rb create mode 100644 app/logical/session_loader.rb diff --git a/app/assets/stylesheets/specific/sessions.css.scss b/app/assets/stylesheets/specific/sessions.css.scss index 54cbcc8a5..d43330b01 100644 --- a/app/assets/stylesheets/specific/sessions.css.scss +++ b/app/assets/stylesheets/specific/sessions.css.scss @@ -1,14 +1,11 @@ @import "../common/000_vars.css.scss"; -div#sessions { - div#new { - section { - width: 30em; - float: left; - } - - h1 { - font-size: $h2_size; +div#c-sessions { + div#a-new { + label#remember-label { + display: inline; + font-weight: normal; + font-style: italic; } } } diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index acb4e489d..c0cf98459 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -31,20 +31,8 @@ protected end def set_current_user - if session[:user_id] - CurrentUser.user = User.find_by_id(session[:user_id]) - CurrentUser.ip_addr = request.remote_ip - end - - if CurrentUser.user - if CurrentUser.user.is_banned? && CurrentUser.user.ban && CurrentUser.user.ban.expires_at < Time.now - CurrentUser.user.unban! - end - else - CurrentUser.user = AnonymousUser.new - end - - Time.zone = CurrentUser.user.time_zone + session_loader = SessionLoader.new(session, cookies, request) + session_loader.load end def reset_current_user diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 310758350..6ab948bcc 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -4,10 +4,9 @@ class SessionsController < ApplicationController end def create - if User.authenticate(params[:name], params[:password]) - @user = User.find_by_name(params[:name]) - @user.update_column(:last_logged_in_at, Time.now) - session[:user_id] = @user.id + session_creator = SessionCreator.new(session, cookies, params[:name], params[:password], params[:remember]) + + if session_creator.authenticate redirect_to(params[:url] || session[:previous_uri] || posts_path, :notice => "You are now logged in.") else redirect_to(new_session_path, :notice => "Password was incorrect.") diff --git a/app/logical/session_creator.rb b/app/logical/session_creator.rb new file mode 100644 index 000000000..0aa0a6ade --- /dev/null +++ b/app/logical/session_creator.rb @@ -0,0 +1,28 @@ +class SessionCreator + attr_reader :session, :cookies, :name, :password, :remember + + def initialize(session, cookies, name, password, remember) + @session = session + @cookies = cookies + @name = name + @password = password + @remember = remember + end + + def authenticate + if User.authenticate(name, password) + user = User.find_by_name(name) + user.update_column(:last_logged_in_at, Time.now) + + if remember.present? + cookies[:user_name] = {:expires => 1.year.from_now, :value => user.name} + cookies[:cookie_password_hash] = {:expires => 1.year.from_now, :value => user.cookie_password_hash} + end + + session[:user_id] = user.id + return true + else + return false + end + end +end diff --git a/app/logical/session_loader.rb b/app/logical/session_loader.rb new file mode 100644 index 000000000..76cf41940 --- /dev/null +++ b/app/logical/session_loader.rb @@ -0,0 +1,51 @@ +class SessionLoader + attr_reader :session, :cookies, :request + + def initialize(session, cookies, request) + @session = session + @cookies = cookies + @request = request + end + + def load + if session[:user_id] + load_session_user + elsif cookie_password_hash_valid? + load_cookie_user + end + + if CurrentUser.user + CurrentUser.user.unban! if ban_expired? + else + CurrentUser.user = AnonymousUser.new + end + + set_time_zone + end + +private + + def load_session_user + CurrentUser.user = User.find_by_id(session[:user_id]) + CurrentUser.ip_addr = request.remote_ip + end + + def load_cookie_user + CurrentUser.user = User.find_by_name(cookies[:user_name]) + CurrentUser.ip_addr = request.remote_ip + end + + def ban_expired? + CurrentUser.user.is_banned? && CurrentUser.user.ban && CurrentUser.user.ban.expired? + end + + def cookie_password_hash_valid? + puts "cookie_password_hash=#{cookies[:cookie_password_hash]}" + cookies[:cookie_password_hash] && User.authenticate_cookie_hash(cookies[:user_name], cookies[:cookie_password_hash]) + end + + def set_time_zone + Time.zone = CurrentUser.user.time_zone + end +end + diff --git a/app/models/ban.rb b/app/models/ban.rb index f35bd4bd0..41ac79d55 100644 --- a/app/models/ban.rb +++ b/app/models/ban.rb @@ -54,4 +54,8 @@ class Ban < ActiveRecord::Base def duration @duration end + + def expired? + expires_at < Time.now + end end diff --git a/app/models/user.rb b/app/models/user.rb index aaf818275..415ac4f35 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -139,16 +139,36 @@ class User < ActiveRecord::Base end module AuthenticationMethods - def authenticate(name, pass) - authenticate_hash(name, sha1(pass)) + extend ActiveSupport::Concern + + module ClassMethods + def authenticate(name, pass) + authenticate_hash(name, sha1(pass)) + end + + def authenticate_hash(name, hash) + where(["lower(name) = ? AND password_hash = ?", name.downcase, hash]).first != nil + end + + def authenticate_cookie_hash(name, hash) + user = User.find_by_name(name) + return nil if user.nil? + return hash == user.cookie_password_hash + end + + def sha1(pass) + Digest::SHA1.hexdigest("#{Danbooru.config.password_salt}--#{pass}--") + end end - def authenticate_hash(name, pass) - where(["lower(name) = ? AND password_hash = ?", name.downcase, pass]).first != nil - end - - def sha1(pass) - Digest::SHA1.hexdigest("#{Danbooru.config.password_salt}--#{pass}--") + def cookie_password_hash + hash = password_hash + + (name.size + 8).times do + hash = User.sha1(hash) + end + + return hash end end @@ -354,7 +374,7 @@ class User < ActiveRecord::Base include BanMethods include NameMethods include PasswordMethods - extend AuthenticationMethods + include AuthenticationMethods include FavoriteMethods include LevelMethods include EmailMethods diff --git a/app/views/sessions/new.html.erb b/app/views/sessions/new.html.erb index 1169f5f76..534ed5017 100644 --- a/app/views/sessions/new.html.erb +++ b/app/views/sessions/new.html.erb @@ -13,6 +13,9 @@
<%= password_field_tag :password %> + + <%= check_box_tag :remember %> +
diff --git a/db/development_structure.sql b/db/development_structure.sql index de6e1ab85..707588482 100644 --- a/db/development_structure.sql +++ b/db/development_structure.sql @@ -2433,7 +2433,8 @@ CREATE TABLE uploads ( post_id integer, md5_confirmation character varying(255), created_at timestamp without time zone, - updated_at timestamp without time zone + updated_at timestamp without time zone, + backtrace text ); diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index 6ba19f71e..fb762c36c 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -174,6 +174,20 @@ class UserTest < ActiveSupport::TestCase end end + context "cookie password hash" do + setup do + @user = Factory.create(:user, :password_hash => "1234") + end + + should "be correct" do + assert_equal("8ac3b1d04bdb95ba92f9e355897c880e0d88ac5a", @user.cookie_password_hash) + end + + should "validate" do + assert(User.authenticate_cookie_hash(@user.name, "8ac3b1d04bdb95ba92f9e355897c880e0d88ac5a")) + end + end + context "password" do should "match the confirmation" do @user = Factory.create(:user)