From fd291c8b429003af3bc113e43125c5b5b1ecc41e Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Sun, 7 May 2017 12:10:26 -0500
Subject: [PATCH 1/2] bans: fix exception when user with expired ban logs in.

`ban.destroy` fails because users have many `bans`, not a single `ban`.
Destroying the expired ban isn't necessary anyway.
---
 app/logical/session_loader.rb | 6 +-----
 app/models/user.rb            | 6 +++++-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/app/logical/session_loader.rb b/app/logical/session_loader.rb
index 4cb4ca0c1..979d3c70e 100644
--- a/app/logical/session_loader.rb
+++ b/app/logical/session_loader.rb
@@ -20,7 +20,7 @@ class SessionLoader
     end
     
     if CurrentUser.user
-      CurrentUser.user.unban! if ban_expired?
+      CurrentUser.user.unban! if CurrentUser.user.ban_expired?
     else
       CurrentUser.user = AnonymousUser.new
     end
@@ -86,10 +86,6 @@ private
     session[:user_id] = CurrentUser.user.id
   end
 
-  def ban_expired?
-    CurrentUser.user.is_banned? && CurrentUser.user.recent_ban && CurrentUser.user.recent_ban.expired?
-  end
-
   def cookie_password_hash_valid?
     cookies[:password_hash] && cookies.signed[:user_name] && User.authenticate_cookie_hash(cookies.signed[:user_name], cookies[:password_hash])
   end
diff --git a/app/models/user.rb b/app/models/user.rb
index 39ae290ea..9b1a2bf6e 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -80,6 +80,7 @@ class User < ActiveRecord::Base
   has_many :post_votes
   has_many :bans, lambda {order("bans.id desc")}
   has_one :recent_ban, lambda {order("bans.id desc")}, :class_name => "Ban"
+
   has_one :api_key
   has_one :dmail_filter
   has_one :super_voter
@@ -109,7 +110,10 @@ class User < ActiveRecord::Base
     def unban!
       self.is_banned = false
       save
-      ban.destroy
+    end
+
+    def ban_expired?
+      is_banned? && recent_ban.try(:expired?)
     end
   end
 

From 19e91f438b663fbf2bada8d12c67cb174ad2b682 Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Sun, 7 May 2017 12:12:43 -0500
Subject: [PATCH 2/2] bans: add test for logging in with expired ban.

---
 test/factories/user.rb                      |  3 ++-
 test/functional/sessions_controller_test.rb | 15 +++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/test/factories/user.rb b/test/factories/user.rb
index 800247c82..3d3d5eb5d 100644
--- a/test/factories/user.rb
+++ b/test/factories/user.rb
@@ -13,8 +13,9 @@ FactoryGirl.define do
     bit_prefs 0
 
     factory(:banned_user) do
+      transient { ban_duration 3 }
       is_banned true
-      after(:create) { |user| create(:ban, user: user) }
+      after(:create) { |user, ctx| create(:ban, user: user, duration: ctx.ban_duration) }
     end
 
     factory(:member_user) do
diff --git a/test/functional/sessions_controller_test.rb b/test/functional/sessions_controller_test.rb
index 038e5aea5..7743ac6c2 100644
--- a/test/functional/sessions_controller_test.rb
+++ b/test/functional/sessions_controller_test.rb
@@ -21,6 +21,21 @@ class SessionsControllerTest < ActionController::TestCase
         assert_equal(@user.id, session[:user_id])
         assert_not_nil(@user.last_ip_addr)
       end
+
+      should "unban user if user has expired ban" do
+        CurrentUser.scoped(@user, "127.0.0.1") do
+          @banned = FactoryGirl.create(:banned_user, ban_duration: 3)
+        end
+
+        travel_to(4.days.from_now) do
+          post :create, {name: @banned.name, password: "password"}
+          SessionLoader.new(session, {}, request, {}).load
+
+          assert_equal(@banned.id, session[:user_id])
+          assert_equal(true, @banned.ban_expired?)
+          assert_equal(false, @banned.reload.is_banned)
+        end
+      end
     end
 
     context "destroy action" do