don't 500 if a user banned/deleted before server move tries to log in

Also add a rake task documenting what jcs did before the server move.
(Though ModNote is new since then.)
This commit is contained in:
Peter Bhat Harkins 2018-11-08 10:50:08 -06:00
parent 26a9cadc10
commit 79b6a2b69c
5 changed files with 91 additions and 33 deletions

View File

@ -1,6 +1,7 @@
class LoginBannedError < StandardError; end class LoginBannedError < StandardError; end
class LoginDeletedError < StandardError; end class LoginDeletedError < StandardError; end
class LoginTOTPFailedError < StandardError; end class LoginTOTPFailedError < StandardError; end
class LoginWipedError < StandardError; end
class LoginFailedError < StandardError; end class LoginFailedError < StandardError; end
class LoginController < ApplicationController class LoginController < ApplicationController
@ -35,6 +36,10 @@ class LoginController < ApplicationController
raise LoginFailedError raise LoginFailedError
end end
if user.is_wiped?
raise LoginWipedError
end
if !user.authenticate(params[:password].to_s) if !user.authenticate(params[:password].to_s)
raise LoginFailedError raise LoginFailedError
end end
@ -74,6 +79,9 @@ class LoginController < ApplicationController
end end
return redirect_to "/" return redirect_to "/"
rescue LoginWipedError
fail_reason = "Your account was banned or deleted before the site changed admins. " <<
"Your email and password hash were wiped for privacy."
rescue LoginBannedError rescue LoginBannedError
fail_reason = "Your account has been banned." fail_reason = "Your account has been banned."
rescue LoginDeletedError rescue LoginDeletedError

View File

@ -419,6 +419,11 @@ class User < ApplicationRecord
banned_at? banned_at?
end end
# user was deleted/banned before a server move, see lib/tasks/privacy_wipe
def is_wiped?
password_digest == '*'
end
def is_new? def is_new?
Time.current - self.created_at <= NEW_USER_DAYS.days Time.current - self.created_at <= NEW_USER_DAYS.days
end end

View File

@ -0,0 +1,16 @@
desc 'Wipe private data if site is changing hands'
task privacy_wipe: :environment do
fail "Refusing to wipe. Read and edit this task if your site is really changing hands"
# It'll be really easy for this rarely-used code to slip out-of-sync,
# you MUST review how users are banned/deleted before you run this.
# At the least, check User#delete! and LoginController.
# User.where.not(deleted_at: nil)
# .update_all("password_digest = '*', email = concat(username, '@lobsters.example')")
# wipe all moderator notes:
# ModNote.delete_all
# wipe all private messages:
# Message.delete_all
end

View File

@ -4,45 +4,69 @@ describe LoginController do
let(:user) { create(:user, password: 'asdf') } let(:user) { create(:user, password: 'asdf') }
let(:banned) { create(:user, :banned, password: 'asdf') } let(:banned) { create(:user, :banned, password: 'asdf') }
let(:deleted) { create(:user, :deleted, password: 'asdf') } let(:deleted) { create(:user, :deleted, password: 'asdf') }
let(:banned_gone) { create(:user, :banned, :gone, password: 'asdf') } let(:banned_wiped) { create(:user, :banned, :wiped, password: 'asdf') }
let(:deleted_gone) { create(:user, :deleted, :gone, password: 'asdf') } let(:deleted_wiped) { create(:user, :deleted, :wiped, password: 'asdf') }
it "logs in with email and correct password" do describe "/login" do
post :login, params: { email: user.email, password: 'asdf' } describe "happy path" do
expect(flash[:error]).to be_nil it "logs in with email and correct password" do
expect(response).to redirect_to('/') post :login, params: { email: user.email, password: 'asdf' }
end expect(flash[:error]).to be_nil
expect(response).to redirect_to('/')
end
it "logs in with username and correct password" do it "logs in with username and correct password" do
post :login, params: { email: user.username, password: 'asdf' } post :login, params: { email: user.username, password: 'asdf' }
expect(session[:u]).to eq(user.session_token) expect(session[:u]).to eq(user.session_token)
expect(flash[:error]).to be_nil expect(flash[:error]).to be_nil
expect(response).to redirect_to('/') expect(response).to redirect_to('/')
end end
end
it "doesn't log in without correct password" do describe "doesn't log in without correct password" do
post :login, params: { email: user.email, password: 'wrong' } it "doesn't log in with wrong password" do
expect(session[:u]).to be_nil post :login, params: { email: user.email, password: 'wrong' }
expect(flash[:error]).to match(/Invalid/i) expect(session[:u]).to be_nil
expect(flash[:error]).to match(/Invalid/i)
end
post :login, params: { email: user.email, password: '' } it "doesn't log in with blank password" do
expect(session[:u]).to be_nil post :login, params: { email: user.email, password: '' }
expect(flash[:error]).to match(/Invalid/i) expect(session[:u]).to be_nil
expect(flash[:error]).to match(/Invalid/i)
end
post :login, params: { email: user.email } it "doesn't log in without any password posted" do
expect(session[:u]).to be_nil post :login, params: { email: user.email }
expect(flash[:error]).to match(/Invalid/i) expect(session[:u]).to be_nil
end expect(flash[:error]).to match(/Invalid/i)
end
end
it "doesn't allow login by banned users" do it "doesn't allow login by banned users" do
post :login, params: { email: banned.email, password: 'asdf' } post :login, params: { email: banned.email, password: 'asdf' }
expect(session[:u]).to be_nil expect(session[:u]).to be_nil
expect(flash[:error]).to match(/banned/) expect(flash[:error]).to match(/banned/)
end end
it "doesn't allow login by deleted users" do it "doesn't allow login by deleted users" do
post :login, params: { email: deleted.email, password: 'asdf' } post :login, params: { email: deleted.email, password: 'asdf' }
expect(session[:u]).to be_nil expect(session[:u]).to be_nil
expect(flash[:error]).to match(/deleted/) expect(flash[:error]).to match(/deleted/)
end
describe "wiped accounts" do
it "doesn't allow login by banned and wiped users" do
post :login, params: { email: banned_wiped.email, password: 'asdf' }
expect(session[:u]).to be_nil
expect(flash[:error]).to match(/wiped/)
end
it "doesn't allow login by deleted and wiped users" do
post :login, params: { email: deleted_wiped.email, password: 'asdf' }
expect(session[:u]).to be_nil
expect(flash[:error]).to match(/wiped/)
end
end
end end
end end

View File

@ -27,6 +27,11 @@ FactoryBot.define do
trait(:deleted) do trait(:deleted) do
deleted_at { Time.current } deleted_at { Time.current }
end end
# users who were banned/deleted before a server move
# you must also add banned/deleted trait with this
trait(:wiped) do
password_digest { '*' }
end
trait(:admin) do trait(:admin) do
is_admin { true } is_admin { true }
is_moderator { true } is_moderator { true }