summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/controllers/profiles/active_sessions_controller.rb11
-rw-r--r--app/models/active_session.rb6
-rw-r--r--app/views/profiles/active_sessions/_active_session.html.haml6
-rw-r--r--changelogs/unreleased/57534_filter_impersonated_sessions.yml6
-rw-r--r--doc/user/profile/active_sessions.md8
-rw-r--r--doc/user/profile/img/active_sessions_list.pngbin22266 -> 19360 bytes
-rw-r--r--spec/features/profiles/active_sessions_spec.rb48
-rw-r--r--spec/models/active_session_spec.rb5
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.
![Active sessions list](img/active_sessions_list.png)
-
-## 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
index 5d94dca69cc..1e242ac4710 100644
--- a/doc/user/profile/img/active_sessions_list.png
+++ b/doc/user/profile/img/active_sessions_list.png
Binary files differ
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, {