diff options
| -rw-r--r-- | app/controllers/profiles/active_sessions_controller.rb | 11 | ||||
| -rw-r--r-- | app/models/active_session.rb | 6 | ||||
| -rw-r--r-- | app/views/profiles/active_sessions/_active_session.html.haml | 6 | ||||
| -rw-r--r-- | changelogs/unreleased/57534_filter_impersonated_sessions.yml | 6 | ||||
| -rw-r--r-- | doc/user/profile/active_sessions.md | 8 | ||||
| -rw-r--r-- | doc/user/profile/img/active_sessions_list.png | bin | 22266 -> 19360 bytes | |||
| -rw-r--r-- | spec/features/profiles/active_sessions_spec.rb | 48 | ||||
| -rw-r--r-- | spec/models/active_session_spec.rb | 5 |
8 files changed, 38 insertions, 52 deletions
diff --git a/app/controllers/profiles/active_sessions_controller.rb b/app/controllers/profiles/active_sessions_controller.rb index efe7ede5efa..c473023cacb 100644 --- a/app/controllers/profiles/active_sessions_controller.rb +++ b/app/controllers/profiles/active_sessions_controller.rb @@ -2,15 +2,6 @@ class Profiles::ActiveSessionsController < Profiles::ApplicationController def index - @sessions = ActiveSession.list(current_user) - end - - def destroy - ActiveSession.destroy(current_user, params[:id]) - - respond_to do |format| - format.html { redirect_to profile_active_sessions_url, status: :found } - format.js { head :ok } - end + @sessions = ActiveSession.list(current_user).reject(&:is_impersonated) end end diff --git a/app/models/active_session.rb b/app/models/active_session.rb index 0d9c6a4a1f0..1e01f1d17e6 100644 --- a/app/models/active_session.rb +++ b/app/models/active_session.rb @@ -5,7 +5,8 @@ class ActiveSession attr_accessor :created_at, :updated_at, :session_id, :ip_address, - :browser, :os, :device_name, :device_type + :browser, :os, :device_name, :device_type, + :is_impersonated def current?(session) return false if session_id.nil? || session.id.nil? @@ -31,7 +32,8 @@ class ActiveSession device_type: client.device_type, created_at: user.current_sign_in_at || timestamp, updated_at: timestamp, - session_id: session_id + session_id: session_id, + is_impersonated: request.session[:impersonator_id].present? ) redis.pipelined do diff --git a/app/views/profiles/active_sessions/_active_session.html.haml b/app/views/profiles/active_sessions/_active_session.html.haml index 23ef31a0c85..2bf514d72a5 100644 --- a/app/views/profiles/active_sessions/_active_session.html.haml +++ b/app/views/profiles/active_sessions/_active_session.html.haml @@ -23,9 +23,3 @@ %strong Signed in on = l(active_session.created_at, format: :short) - - - unless is_current_session - .float-right - = link_to profile_active_session_path(active_session.session_id), data: { confirm: 'Are you sure? The device will be signed out of GitLab.' }, method: :delete, class: "btn btn-danger prepend-left-10" do - %span.sr-only Revoke - Revoke diff --git a/changelogs/unreleased/57534_filter_impersonated_sessions.yml b/changelogs/unreleased/57534_filter_impersonated_sessions.yml new file mode 100644 index 00000000000..80aea0ab1bc --- /dev/null +++ b/changelogs/unreleased/57534_filter_impersonated_sessions.yml @@ -0,0 +1,6 @@ +--- +title: Do not display impersonated sessions under active sessions and remove ability + to revoke session +merge_request: +author: +type: security diff --git a/doc/user/profile/active_sessions.md b/doc/user/profile/active_sessions.md index 5119c0e30d0..28e3f4904a9 100644 --- a/doc/user/profile/active_sessions.md +++ b/doc/user/profile/active_sessions.md @@ -4,7 +4,7 @@ > in GitLab 10.8. GitLab lists all devices that have logged into your account. This allows you to -review the sessions and revoke any of it that you don't recognize. +review the sessions. ## Listing all active sessions @@ -12,9 +12,3 @@ review the sessions and revoke any of it that you don't recognize. 1. Navigate to the **Active Sessions** tab.  - -## Revoking a session - -1. Navigate to your [profile's](#profile-settings) **Settings > Active Sessions**. -1. Click on **Revoke** besides a session. The current session cannot be - revoked, as this would sign you out of GitLab. diff --git a/doc/user/profile/img/active_sessions_list.png b/doc/user/profile/img/active_sessions_list.png Binary files differindex 5d94dca69cc..1e242ac4710 100644 --- a/doc/user/profile/img/active_sessions_list.png +++ b/doc/user/profile/img/active_sessions_list.png diff --git a/spec/features/profiles/active_sessions_spec.rb b/spec/features/profiles/active_sessions_spec.rb index d3050760c06..2aa0177af5d 100644 --- a/spec/features/profiles/active_sessions_spec.rb +++ b/spec/features/profiles/active_sessions_spec.rb @@ -7,6 +7,8 @@ describe 'Profile > Active Sessions', :clean_gitlab_redis_shared_state do end end + let(:admin) { create(:admin) } + around do |example| Timecop.freeze(Time.zone.parse('2018-03-12 09:06')) do example.run @@ -16,6 +18,7 @@ describe 'Profile > Active Sessions', :clean_gitlab_redis_shared_state do it 'User sees their active sessions' do Capybara::Session.new(:session1) Capybara::Session.new(:session2) + Capybara::Session.new(:session3) # note: headers can only be set on the non-js (aka. rack-test) driver using_session :session1 do @@ -37,9 +40,27 @@ describe 'Profile > Active Sessions', :clean_gitlab_redis_shared_state do gitlab_sign_in(user) end + # set an admin session impersonating the user + using_session :session3 do + Capybara.page.driver.header( + 'User-Agent', + 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.113 Safari/537.36' + ) + + gitlab_sign_in(admin) + + visit admin_user_path(user) + + click_link 'Impersonate' + end + using_session :session1 do visit profile_active_sessions_path + expect(page).to( + have_selector('ul.list-group li.list-group-item', { text: 'Signed in on', + count: 2 })) + expect(page).to have_content( '127.0.0.1 ' \ 'This is your current session ' \ @@ -57,33 +78,8 @@ describe 'Profile > Active Sessions', :clean_gitlab_redis_shared_state do ) expect(page).to have_selector '[title="Smartphone"]', count: 1 - end - end - - it 'User can revoke a session', :js, :redis_session_store do - Capybara::Session.new(:session1) - Capybara::Session.new(:session2) - - # set an additional session in another browser - using_session :session2 do - gitlab_sign_in(user) - end - - using_session :session1 do - gitlab_sign_in(user) - visit profile_active_sessions_path - - expect(page).to have_link('Revoke', count: 1) - - accept_confirm { click_on 'Revoke' } - - expect(page).not_to have_link('Revoke') - end - - using_session :session2 do - visit profile_active_sessions_path - expect(page).to have_content('You need to sign in or sign up before continuing.') + expect(page).not_to have_content('Chrome on Windows') end end end diff --git a/spec/models/active_session_spec.rb b/spec/models/active_session_spec.rb index 129b2f92683..e128fe8a4b7 100644 --- a/spec/models/active_session_spec.rb +++ b/spec/models/active_session_spec.rb @@ -7,7 +7,10 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do end end - let(:session) { double(:session, id: '6919a6f1bb119dd7396fadc38fd18d0d') } + let(:session) do + double(:session, { id: '6919a6f1bb119dd7396fadc38fd18d0d', + '[]': {} }) + end let(:request) do double(:request, { |
