diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index bb47e2a8bf7442f2a416ef0c6b0848db311af991..bf6be3d516b36c28b8666dee6fe4d7b9f4a15f7e 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -12,7 +12,6 @@ class ApplicationController < ActionController::Base
before_action :authenticate_user_from_private_token!
before_action :authenticate_user!
before_action :validate_user_service_ticket!
- before_action :reject_blocked!
before_action :check_password_expiration
before_action :check_2fa_requirement
before_action :ldap_security_check
@@ -87,22 +86,8 @@ class ApplicationController < ActionController::Base
logger.error "\n#{exception.class.name} (#{exception.message}):\n#{application_trace.join}"
end
- def reject_blocked!
- if current_user && current_user.blocked?
- sign_out current_user
- flash[:alert] = "Your account is blocked. Retry when an admin has unblocked it."
- redirect_to new_user_session_path
- end
- end
-
def after_sign_in_path_for(resource)
- if resource.is_a?(User) && resource.respond_to?(:blocked?) && resource.blocked?
- sign_out resource
- flash[:alert] = "Your account is blocked. Retry when an admin has unblocked it."
- new_user_session_path
- else
- stored_location_for(:redirect) || stored_location_for(resource) || root_path
- end
+ stored_location_for(:redirect) || stored_location_for(resource) || root_path
end
def after_sign_out_path_for(resource)
diff --git a/app/controllers/explore/application_controller.rb b/app/controllers/explore/application_controller.rb
index a1ab8b99048fc4d5979b1a7064a999d0d2a301fa..baf54520b9c5c208cf7710c40052e196b732436f 100644
--- a/app/controllers/explore/application_controller.rb
+++ b/app/controllers/explore/application_controller.rb
@@ -1,5 +1,5 @@
class Explore::ApplicationController < ApplicationController
- skip_before_action :authenticate_user!, :reject_blocked!
+ skip_before_action :authenticate_user!
layout 'explore'
end
diff --git a/app/controllers/help_controller.rb b/app/controllers/help_controller.rb
index 37feff79999561954faf962405280fa7488b9dd6..87c0f8905ff7a8607b6a2e03c60b0363cb5d56e0 100644
--- a/app/controllers/help_controller.rb
+++ b/app/controllers/help_controller.rb
@@ -1,5 +1,5 @@
class HelpController < ApplicationController
- skip_before_action :authenticate_user!, :reject_blocked!
+ skip_before_action :authenticate_user!
layout 'help'
diff --git a/app/controllers/koding_controller.rb b/app/controllers/koding_controller.rb
index f3759b4c0ea2b77696e0342d5186b703288fc455..6b1e64ce8194db812bced74c816d40e34889cdcf 100644
--- a/app/controllers/koding_controller.rb
+++ b/app/controllers/koding_controller.rb
@@ -1,5 +1,5 @@
class KodingController < ApplicationController
- before_action :check_integration!, :authenticate_user!, :reject_blocked!
+ before_action :check_integration!
layout 'koding'
def index
diff --git a/app/controllers/projects/uploads_controller.rb b/app/controllers/projects/uploads_controller.rb
index 50ba33ed57033d2a57f3704b020d16d98865ef74..61686499bd3d38c16c512a0280a3129b5606f0af 100644
--- a/app/controllers/projects/uploads_controller.rb
+++ b/app/controllers/projects/uploads_controller.rb
@@ -1,6 +1,6 @@
class Projects::UploadsController < Projects::ApplicationController
- skip_before_action :reject_blocked!, :project,
- :repository, if: -> { action_name == 'show' && image_or_video? }
+ skip_before_action :project, :repository,
+ if: -> { action_name == 'show' && image_or_video? }
before_action :authorize_upload_file!, only: [:create]
diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb
index 6576ebd5235a67d8bfa2d7af4dc60d5b02454c95..612d69cf557a1dc0fc61b1805a923453e7b53393 100644
--- a/app/controllers/search_controller.rb
+++ b/app/controllers/search_controller.rb
@@ -1,5 +1,5 @@
class SearchController < ApplicationController
- skip_before_action :authenticate_user!, :reject_blocked!
+ skip_before_action :authenticate_user!
include SearchHelper
diff --git a/app/models/user.rb b/app/models/user.rb
index 33666b4f35bd4ad6122cb6269371db8239f325a4..f9245cdb82a8edd6f3e417fe7a1fb323877692f6 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -167,6 +167,15 @@ class User < ActiveRecord::Base
def blocked?
true
end
+
+ def active_for_authentication?
+ false
+ end
+
+ def inactive_message
+ "Your account has been blocked. Please contact your GitLab " \
+ "administrator if you think this is an error."
+ end
end
end
diff --git a/changelogs/unreleased/rs-warden-blocked-users.yml b/changelogs/unreleased/rs-warden-blocked-users.yml
new file mode 100644
index 0000000000000000000000000000000000000000..c0c23fb6f112b1560c98a5b252f95572fa27bacc
--- /dev/null
+++ b/changelogs/unreleased/rs-warden-blocked-users.yml
@@ -0,0 +1,4 @@
+---
+title: Don't perform Devise trackable updates on blocked User records
+merge_request: 8915
+author:
diff --git a/spec/controllers/projects/uploads_controller_spec.rb b/spec/controllers/projects/uploads_controller_spec.rb
index f1c8891e87ab9a833adbb8691b1109c2a792c750..0347e789576ec81ce940912ecdfa73895bf5dbc4 100644
--- a/spec/controllers/projects/uploads_controller_spec.rb
+++ b/spec/controllers/projects/uploads_controller_spec.rb
@@ -170,68 +170,24 @@ describe Projects::UploadsController do
project.team << [user, :master]
end
- context "when the user is blocked" do
+ context "when the file exists" do
before do
- user.block
- project.team << [user, :master]
- end
-
- context "when the file exists" do
- before do
- allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
- allow(jpg).to receive(:exists?).and_return(true)
- end
-
- context "when the file is an image" do
- before do
- allow_any_instance_of(FileUploader).to receive(:image?).and_return(true)
- end
-
- it "responds with status 200" do
- go
-
- expect(response).to have_http_status(200)
- end
- end
-
- context "when the file is not an image" do
- it "redirects to the sign in page" do
- go
-
- expect(response).to redirect_to(new_user_session_path)
- end
- end
+ allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
+ allow(jpg).to receive(:exists?).and_return(true)
end
- context "when the file doesn't exist" do
- it "redirects to the sign in page" do
- go
+ it "responds with status 200" do
+ go
- expect(response).to redirect_to(new_user_session_path)
- end
+ expect(response).to have_http_status(200)
end
end
- context "when the user isn't blocked" do
- context "when the file exists" do
- before do
- allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
- allow(jpg).to receive(:exists?).and_return(true)
- end
-
- it "responds with status 200" do
- go
-
- expect(response).to have_http_status(200)
- end
- end
-
- context "when the file doesn't exist" do
- it "responds with status 404" do
- go
+ context "when the file doesn't exist" do
+ it "responds with status 404" do
+ go
- expect(response).to have_http_status(404)
- end
+ expect(response).to have_http_status(404)
end
end
end
diff --git a/spec/factories/users.rb b/spec/factories/users.rb
index c6f7869516e207f03a22a094660e3894eacbe45e..1732b1a00817e385c19c9a5bcd1f683fc99cc085 100644
--- a/spec/factories/users.rb
+++ b/spec/factories/users.rb
@@ -14,6 +14,14 @@ FactoryGirl.define do
admin true
end
+ trait :blocked do
+ after(:build) { |user, _| user.block! }
+ end
+
+ trait :external do
+ external true
+ end
+
trait :two_factor do
two_factor_via_otp
end
diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb
index ab7d89306db893cd9a4653926e5ff82e14963d3c..ae609160e18ca8b0bad7cf3b4a838b13f29e600e 100644
--- a/spec/features/login_spec.rb
+++ b/spec/features/login_spec.rb
@@ -32,6 +32,22 @@ feature 'Login', feature: true do
end
end
+ describe 'with a blocked account' do
+ it 'prevents the user from logging in' do
+ user = create(:user, :blocked)
+
+ login_with(user)
+
+ expect(page).to have_content('Your account has been blocked.')
+ end
+
+ it 'does not update Devise trackable attributes' do
+ user = create(:user, :blocked)
+
+ expect { login_with(user) }.not_to change { user.reload.sign_in_count }
+ end
+ end
+
describe 'with two-factor authentication' do
def enter_code(code)
fill_in 'user_otp_attempt', with: code