Commit 9745c98b authored by Robert Speicher's avatar Robert Speicher

Merge branch 'rs-warden-blocked-users' into 'master'

Don't perform Devise trackable updates on blocked User records

Closes #27519

See merge request !8915
parents b5e10e71 191bcb4d
...@@ -12,7 +12,6 @@ class ApplicationController < ActionController::Base ...@@ -12,7 +12,6 @@ class ApplicationController < ActionController::Base
before_action :authenticate_user_from_private_token! before_action :authenticate_user_from_private_token!
before_action :authenticate_user! before_action :authenticate_user!
before_action :validate_user_service_ticket! before_action :validate_user_service_ticket!
before_action :reject_blocked!
before_action :check_password_expiration before_action :check_password_expiration
before_action :check_2fa_requirement before_action :check_2fa_requirement
before_action :ldap_security_check before_action :ldap_security_check
...@@ -87,22 +86,8 @@ class ApplicationController < ActionController::Base ...@@ -87,22 +86,8 @@ class ApplicationController < ActionController::Base
logger.error "\n#{exception.class.name} (#{exception.message}):\n#{application_trace.join}" logger.error "\n#{exception.class.name} (#{exception.message}):\n#{application_trace.join}"
end 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) def after_sign_in_path_for(resource)
if resource.is_a?(User) && resource.respond_to?(:blocked?) && resource.blocked? stored_location_for(:redirect) || stored_location_for(resource) || root_path
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
end end
def after_sign_out_path_for(resource) def after_sign_out_path_for(resource)
......
class Explore::ApplicationController < ApplicationController class Explore::ApplicationController < ApplicationController
skip_before_action :authenticate_user!, :reject_blocked! skip_before_action :authenticate_user!
layout 'explore' layout 'explore'
end end
class HelpController < ApplicationController class HelpController < ApplicationController
skip_before_action :authenticate_user!, :reject_blocked! skip_before_action :authenticate_user!
layout 'help' layout 'help'
......
class KodingController < ApplicationController class KodingController < ApplicationController
before_action :check_integration!, :authenticate_user!, :reject_blocked! before_action :check_integration!
layout 'koding' layout 'koding'
def index def index
......
class Projects::UploadsController < Projects::ApplicationController class Projects::UploadsController < Projects::ApplicationController
skip_before_action :reject_blocked!, :project, skip_before_action :project, :repository,
:repository, if: -> { action_name == 'show' && image_or_video? } if: -> { action_name == 'show' && image_or_video? }
before_action :authorize_upload_file!, only: [:create] before_action :authorize_upload_file!, only: [:create]
......
class SearchController < ApplicationController class SearchController < ApplicationController
skip_before_action :authenticate_user!, :reject_blocked! skip_before_action :authenticate_user!
include SearchHelper include SearchHelper
......
...@@ -167,6 +167,15 @@ class User < ActiveRecord::Base ...@@ -167,6 +167,15 @@ class User < ActiveRecord::Base
def blocked? def blocked?
true true
end 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
end end
......
---
title: Don't perform Devise trackable updates on blocked User records
merge_request: 8915
author:
...@@ -170,68 +170,24 @@ describe Projects::UploadsController do ...@@ -170,68 +170,24 @@ describe Projects::UploadsController do
project.team << [user, :master] project.team << [user, :master]
end end
context "when the user is blocked" do context "when the file exists" do
before do before do
user.block allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
project.team << [user, :master] allow(jpg).to receive(:exists?).and_return(true)
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
end end
context "when the file doesn't exist" do it "responds with status 200" do
it "redirects to the sign in page" do go
go
expect(response).to redirect_to(new_user_session_path) expect(response).to have_http_status(200)
end
end end
end end
context "when the user isn't blocked" do context "when the file doesn't exist" do
context "when the file exists" do it "responds with status 404" do
before do go
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
expect(response).to have_http_status(404) expect(response).to have_http_status(404)
end
end end
end end
end end
......
...@@ -14,6 +14,14 @@ FactoryGirl.define do ...@@ -14,6 +14,14 @@ FactoryGirl.define do
admin true admin true
end end
trait :blocked do
after(:build) { |user, _| user.block! }
end
trait :external do
external true
end
trait :two_factor do trait :two_factor do
two_factor_via_otp two_factor_via_otp
end end
......
...@@ -32,6 +32,22 @@ feature 'Login', feature: true do ...@@ -32,6 +32,22 @@ feature 'Login', feature: true do
end end
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 describe 'with two-factor authentication' do
def enter_code(code) def enter_code(code)
fill_in 'user_otp_attempt', with: code fill_in 'user_otp_attempt', with: code
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment