diff options
| -rw-r--r-- | app/controllers/sessions_controller.rb | 15 | ||||
| -rw-r--r-- | app/views/shared/issuable/_sidebar.html.haml | 14 | ||||
| -rw-r--r-- | doc/install/installation.md | 6 | ||||
| -rw-r--r-- | doc/update/8.6-to-8.7.md | 8 | ||||
| -rw-r--r-- | spec/controllers/sessions_controller_spec.rb | 101 |
5 files changed, 131 insertions, 13 deletions
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 65677a3dd3c..c29f4609e93 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -5,7 +5,8 @@ class SessionsController < Devise::SessionsController skip_before_action :check_2fa_requirement, only: [:destroy] prepend_before_action :check_initial_setup, only: [:new] - prepend_before_action :authenticate_with_two_factor, only: [:create] + prepend_before_action :authenticate_with_two_factor, + if: :two_factor_enabled?, only: [:create] prepend_before_action :store_redirect_path, only: [:new] before_action :auto_sign_in_with_provider, only: [:new] @@ -56,10 +57,10 @@ class SessionsController < Devise::SessionsController end def find_user - if user_params[:login] - User.by_login(user_params[:login]) - elsif user_params[:otp_attempt] && session[:otp_user_id] + if session[:otp_user_id] User.find(session[:otp_user_id]) + elsif user_params[:login] + User.by_login(user_params[:login]) end end @@ -83,11 +84,13 @@ class SessionsController < Devise::SessionsController end end + def two_factor_enabled? + find_user.try(:two_factor_enabled?) + end + def authenticate_with_two_factor user = self.resource = find_user - return unless user && user.two_factor_enabled? - if user_params[:otp_attempt].present? && session[:otp_user_id] if valid_otp_attempt?(user) # Remove any lingering user data from login diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 47e544acf52..94affa4b59a 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -1,5 +1,6 @@ %aside.right-sidebar{ class: sidebar_gutter_collapsed_class } .issuable-sidebar + - can_edit_issuable = can?(current_user, :"admin_#{issuable.to_ability_name}", @project) .block.issuable-sidebar-header %span.issuable-count.hide-collapsed.pull-left = issuable.iid @@ -29,7 +30,7 @@ .title.hide-collapsed Assignee = icon('spinner spin', class: 'block-loading') - - if can?(current_user, :"admin_#{issuable.to_ability_name}", @project) + - if can_edit_issuable = link_to 'Edit', '#', class: 'edit-link pull-right' .value.bold.hide-collapsed - if issuable.assignee @@ -41,9 +42,10 @@ = issuable.assignee.to_reference - else %span.assign-yourself - No assignee - - %a.js-assign-yourself{ href: '#' } - assign yourself + No assignee + - if can_edit_issuable + %a.js-assign-yourself{ href: '#' } + \- assign yourself .selectbox.hide-collapsed = f.hidden_field 'assignee_id', value: issuable.assignee_id, id: 'issue_assignee_id' @@ -60,7 +62,7 @@ .title.hide-collapsed Milestone = icon('spinner spin', class: 'block-loading') - - if can?(current_user, :"admin_#{issuable.to_ability_name}", @project) + - if can_edit_issuable = link_to 'Edit', '#', class: 'edit-link pull-right' .value.bold.hide-collapsed - if issuable.milestone @@ -82,7 +84,7 @@ .title.hide-collapsed Labels = icon('spinner spin', class: 'block-loading') - - if can?(current_user, :"admin_#{issuable.to_ability_name}", @project) + - if can_edit_issuable = link_to 'Edit', '#', class: 'edit-link pull-right' .value.bold.issuable-show-labels.hide-collapsed{ class: ("has-labels" if issuable.labels.any?) } - if issuable.labels.any? diff --git a/doc/install/installation.md b/doc/install/installation.md index e0a16df09c1..f8f7d6a9ebe 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -283,9 +283,13 @@ sudo usermod -aG redis git # Copy the example Rack attack config sudo -u git -H cp config/initializers/rack_attack.rb.example config/initializers/rack_attack.rb - # Configure Git global settings for git user, used when editing via web editor + # Configure Git global settings for git user + # 'autocrlf' is needed for the web editor sudo -u git -H git config --global core.autocrlf input + # Disable 'git gc --auto' because GitLab already runs 'git gc' when needed + sudo -u git -H git config --global gc.auto 0 + # Configure Redis connection settings sudo -u git -H cp config/resque.yml.example config/resque.yml diff --git a/doc/update/8.6-to-8.7.md b/doc/update/8.6-to-8.7.md index 76eee147c72..8599133a726 100644 --- a/doc/update/8.6-to-8.7.md +++ b/doc/update/8.6-to-8.7.md @@ -86,6 +86,14 @@ sudo -u git -H bundle exec rake assets:clean assets:precompile cache:clear RAILS ### 7. Update configuration files +#### Git configuration + +Disable `git gc --auto` because GitLab runs `git gc` for us already. + +```sh +sudo -u git -H git config --global gc.auto 0 +``` + #### Nginx configuration Ensure you're still up-to-date with the latest NGINX configuration changes: diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb new file mode 100644 index 00000000000..83cc8ec6d26 --- /dev/null +++ b/spec/controllers/sessions_controller_spec.rb @@ -0,0 +1,101 @@ +require 'spec_helper' + +describe SessionsController do + describe '#create' do + before do + @request.env['devise.mapping'] = Devise.mappings[:user] + end + + context 'when using standard authentications' do + context 'invalid password' do + it 'does not authenticate user' do + post(:create, user: { login: 'invalid', password: 'invalid' }) + + expect(response) + .to set_flash.now[:alert].to /Invalid login or password/ + end + end + + context 'when using valid password' do + let(:user) { create(:user) } + + it 'authenticates user correctly' do + post(:create, user: { login: user.username, password: user.password }) + + expect(response).to set_flash.to /Signed in successfully/ + expect(subject.current_user). to eq user + end + end + end + + context 'when using two-factor authentication' do + let(:user) { create(:user, :two_factor) } + + def authenticate_2fa(user_params) + post(:create, { user: user_params }, { otp_user_id: user.id }) + end + + ## + # See #14900 issue + # + context 'when authenticating with login and OTP of another user' do + context 'when another user has 2FA enabled' do + let(:another_user) { create(:user, :two_factor) } + + context 'when OTP is valid for another user' do + it 'does not authenticate' do + authenticate_2fa(login: another_user.username, + otp_attempt: another_user.current_otp) + + expect(subject.current_user).to_not eq another_user + end + end + + context 'when OTP is invalid for another user' do + it 'does not authenticate' do + authenticate_2fa(login: another_user.username, + otp_attempt: 'invalid') + + expect(subject.current_user).to_not eq another_user + end + end + + context 'when authenticating with OTP' do + context 'when OTP is valid' do + it 'authenticates correctly' do + authenticate_2fa(otp_attempt: user.current_otp) + + expect(subject.current_user).to eq user + end + end + + context 'when OTP is invalid' do + before { authenticate_2fa(otp_attempt: 'invalid') } + + it 'does not authenticate' do + expect(subject.current_user).to_not eq user + end + + it 'warns about invalid OTP code' do + expect(response).to set_flash.now[:alert] + .to /Invalid two-factor code/ + end + end + end + + context 'when another user does not have 2FA enabled' do + let(:another_user) { create(:user) } + + it 'does not leak that 2FA is disabled for another user' do + authenticate_2fa(login: another_user.username, + otp_attempt: 'invalid') + + expect(response).to set_flash.now[:alert] + .to /Invalid two-factor code/ + end + end + end + end + end + end +end |