Commit 191bcb4d authored by Robert Speicher's avatar Robert Speicher

Don't perform Devise trackable updates on blocked User records

parent 4a925837
......@@ -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,23 +86,9 @@ 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
end
def after_sign_out_path_for(resource)
current_application_settings.after_sign_out_path.presence || new_user_session_path
......
class Explore::ApplicationController < ApplicationController
skip_before_action :authenticate_user!, :reject_blocked!
skip_before_action :authenticate_user!
layout 'explore'
end
class HelpController < ApplicationController
skip_before_action :authenticate_user!, :reject_blocked!
skip_before_action :authenticate_user!
layout 'help'
......
class KodingController < ApplicationController
before_action :check_integration!, :authenticate_user!, :reject_blocked!
before_action :check_integration!
layout 'koding'
def index
......
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]
......
class SearchController < ApplicationController
skip_before_action :authenticate_user!, :reject_blocked!
skip_before_action :authenticate_user!
include SearchHelper
......
......@@ -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
......
---
title: Don't perform Devise trackable updates on blocked User records
merge_request: 8915
author:
......@@ -170,49 +170,6 @@ describe Projects::UploadsController do
project.team << [user, :master]
end
context "when the user is blocked" 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
end
context "when the file doesn't exist" do
it "redirects to the sign in page" do
go
expect(response).to redirect_to(new_user_session_path)
end
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)
......@@ -234,7 +191,6 @@ describe Projects::UploadsController do
end
end
end
end
context "when the user doesn't have access to the project" do
context "when the file exists" do
......
......@@ -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
......
......@@ -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
......
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