diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index 543d5eac504cc65d04cdaba7d171851fb18e1194..3f10ae8176730529b8344e16ef45530b1d7c246c 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -137,6 +137,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
:user_default_external,
:user_oauth_applications,
:version_check_enabled,
+ :terminal_max_session_time,
disabled_oauth_sign_in_sources: [],
import_sources: [],
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 2df8b071e13e0165f8db82324d4a0c4b9b2cc8ca..9a4557524c4b0be542b35b28c208ee0627d66c32 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -111,6 +111,10 @@ class ApplicationSetting < ActiveRecord::Base
presence: true,
numericality: { only_integer: true, greater_than: :housekeeping_full_repack_period }
+ validates :terminal_max_session_time,
+ presence: true,
+ numericality: { only_integer: true, greater_than_or_equal_to: 0 }
+
validates_each :restricted_visibility_levels do |record, attr, value|
unless value.nil?
value.each do |level|
@@ -204,7 +208,8 @@ class ApplicationSetting < ActiveRecord::Base
signin_enabled: Settings.gitlab['signin_enabled'],
signup_enabled: Settings.gitlab['signup_enabled'],
two_factor_grace_period: 48,
- user_default_external: false
+ user_default_external: false,
+ terminal_max_session_time: 0
}
end
diff --git a/app/models/project_services/kubernetes_service.rb b/app/models/project_services/kubernetes_service.rb
index fa3cedc4354bbdfcaab1e9f4a43683b97079d6e2..f2f019c43bb46f5b6a4d112098683351901fc3e8 100644
--- a/app/models/project_services/kubernetes_service.rb
+++ b/app/models/project_services/kubernetes_service.rb
@@ -1,4 +1,5 @@
class KubernetesService < DeploymentService
+ include Gitlab::CurrentSettings
include Gitlab::Kubernetes
include ReactiveCaching
@@ -110,7 +111,7 @@ class KubernetesService < DeploymentService
pods = data.fetch(:pods, nil)
filter_pods(pods, app: environment.slug).
flat_map { |pod| terminals_for_pod(api_url, namespace, pod) }.
- map { |terminal| add_terminal_auth(terminal, token, ca_pem) }
+ each { |terminal| add_terminal_auth(terminal, terminal_auth) }
end
end
@@ -170,4 +171,12 @@ class KubernetesService < DeploymentService
url.to_s
end
+
+ def terminal_auth
+ {
+ token: token,
+ ca_pem: ca_pem,
+ max_session_time: current_application_settings.terminal_max_session_time
+ }
+ end
end
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index e7701d75a6eec843176d11e9223a4d8ffd13c070..2c64de1d5304019de03e44a3e36428810a6d510c 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -509,5 +509,15 @@
.help-block
Number of Git pushes after which 'git gc' is run.
+ %fieldset
+ %legend Web terminal
+ .form-group
+ = f.label :terminal_max_session_time, 'Max session time', class: 'control-label col-sm-2'
+ .col-sm-10
+ = f.number_field :terminal_max_session_time, class: 'form-control'
+ .help-block
+ Maximum time for web terminal websocket connection (in seconds).
+ Set to 0 for unlimited time.
+
.form-actions
= f.submit 'Save', class: 'btn btn-save'
diff --git a/changelogs/unreleased/terminal-max-session-time.yml b/changelogs/unreleased/terminal-max-session-time.yml
new file mode 100644
index 0000000000000000000000000000000000000000..db1e66770d1434dea0bed3ee04fb468e59aa0019
--- /dev/null
+++ b/changelogs/unreleased/terminal-max-session-time.yml
@@ -0,0 +1,4 @@
+---
+title: Introduce maximum session time for terminal websocket connection
+merge_request: 8413
+author:
diff --git a/db/migrate/20170126174819_add_terminal_max_session_time_to_application_settings.rb b/db/migrate/20170126174819_add_terminal_max_session_time_to_application_settings.rb
new file mode 100644
index 0000000000000000000000000000000000000000..334f53f9145f3b3147463098431dd30586cafb9c
--- /dev/null
+++ b/db/migrate/20170126174819_add_terminal_max_session_time_to_application_settings.rb
@@ -0,0 +1,33 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddTerminalMaxSessionTimeToApplicationSettings < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ # Set this constant to true if this migration requires downtime.
+ DOWNTIME = false
+
+ # When a migration requires downtime you **must** uncomment the following
+ # constant and define a short and easy to understand explanation as to why the
+ # migration requires downtime.
+ # DOWNTIME_REASON = ''
+
+ # When using the methods "add_concurrent_index" or "add_column_with_default"
+ # you must disable the use of transactions as these methods can not run in an
+ # existing transaction. When using "add_concurrent_index" make sure that this
+ # method is the _only_ method called in the migration, any other changes
+ # should go in a separate migration. This ensures that upon failure _only_ the
+ # index creation fails and can be retried or reverted easily.
+ #
+ # To disable transactions uncomment the following line and remove these
+ # comments:
+ disable_ddl_transaction!
+
+ def up
+ add_column_with_default :application_settings, :terminal_max_session_time, :integer, default: 0, allow_null: false
+ end
+
+ def down
+ remove_column :application_settings, :terminal_max_session_time
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 92b36218a15adfc7cf50e5ea4a0f8c571a09fa7c..a9f4e865d60b23327c286efc9dd0480834cbd1b8 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -109,6 +109,7 @@ ActiveRecord::Schema.define(version: 20170204181513) do
t.boolean "html_emails_enabled", default: true
t.string "plantuml_url"
t.boolean "plantuml_enabled"
+ t.integer "terminal_max_session_time", default: 0, null: false
end
create_table "audit_events", force: :cascade do |t|
diff --git a/doc/administration/integration/terminal.md b/doc/administration/integration/terminal.md
index 3fbb13704aaec66cf111b0a88d0677f1f798b0ab..1144446453756e198ce61efd4bf6d3eb0106ed10 100644
--- a/doc/administration/integration/terminal.md
+++ b/doc/administration/integration/terminal.md
@@ -71,5 +71,15 @@ When these headers are not passed through, Workhorse will return a
`400 Bad Request` response to users attempting to use a web terminal. In turn,
they will receive a `Connection failed` message.
+## Limiting WebSocket connection time
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8413)
+in GitLab 8.17.
+
+Terminal sessions use long-lived connections; by default, these may last
+forever. You can configure a maximum session time in the Admin area of your
+GitLab instance if you find this undesirable from a scalability or security
+point of view.
+
[ce-7690]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7690
[kubservice]: ../../user/project/integrations/kubernetes.md)
diff --git a/doc/api/settings.md b/doc/api/settings.md
index f86c7cc2f941f016f385efd22f1cdafe4a7ebb59..ca6b934787733d1d8b8739fe0f40ce8de1e16c70 100644
--- a/doc/api/settings.md
+++ b/doc/api/settings.md
@@ -46,7 +46,8 @@ Example response:
"koding_enabled": false,
"koding_url": null,
"plantuml_enabled": false,
- "plantuml_url": null
+ "plantuml_url": null,
+ "terminal_max_session_time": 0
}
```
@@ -84,6 +85,7 @@ PUT /application/settings
| `disabled_oauth_sign_in_sources` | Array of strings | no | Disabled OAuth sign-in sources |
| `plantuml_enabled` | boolean | no | Enable PlantUML integration. Default is `false`. |
| `plantuml_url` | string | yes (if `plantuml_enabled` is `true`) | The PlantUML instance URL for integration. |
+| `terminal_max_session_time` | integer | no | Maximum time for web terminal websocket connection (in seconds). Set to 0 for unlimited time. |
```bash
curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/application/settings?signup_enabled=false&default_project_visibility=1
@@ -118,6 +120,7 @@ Example response:
"koding_enabled": false,
"koding_url": null,
"plantuml_enabled": false,
- "plantuml_url": null
+ "plantuml_url": null,
+ "terminal_max_session_time": 0
}
```
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index a07b2a9ca0fa5fa01768f3f01b1be6fabbafc71d..b1ead48caf7e6212f4d681a4517200ea4147bb7a 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -575,6 +575,7 @@ module API
expose :koding_url
expose :plantuml_enabled
expose :plantuml_url
+ expose :terminal_max_session_time
end
class Release < Grape::Entity
diff --git a/lib/api/settings.rb b/lib/api/settings.rb
index c5eff16a5de222f8e0c1de18c195ec967b1dc9ad..a1d1c1432d3c44d92c7e9e1d8d3072e4acb0e9f1 100644
--- a/lib/api/settings.rb
+++ b/lib/api/settings.rb
@@ -107,6 +107,7 @@ module API
requires :housekeeping_full_repack_period, type: Integer, desc: "Number of Git pushes after which a full 'git repack' is run."
requires :housekeeping_gc_period, type: Integer, desc: "Number of Git pushes after which 'git gc' is run."
end
+ optional :terminal_max_session_time, type: Integer, desc: 'Maximum time for web terminal websocket connection (in seconds). Set to 0 for unlimited time.'
at_least_one_of :default_branch_protection, :default_project_visibility, :default_snippet_visibility,
:default_group_visibility, :restricted_visibility_levels, :import_sources,
:enabled_git_access_protocol, :gravatar_enabled, :default_projects_limit,
@@ -120,7 +121,7 @@ module API
:akismet_enabled, :admin_notification_email, :sentry_enabled,
:repository_storage, :repository_checks_enabled, :koding_enabled, :plantuml_enabled,
:version_check_enabled, :email_author_in_body, :html_emails_enabled,
- :housekeeping_enabled
+ :housekeeping_enabled, :terminal_max_session_time
end
put "application/settings" do
if current_settings.update_attributes(declared_params(include_missing: false))
diff --git a/lib/gitlab/kubernetes.rb b/lib/gitlab/kubernetes.rb
index 288771c1c124d0bdd67b6ed161c1877267597c5f..3a7af363548b87137982ce721450ca79493605ba 100644
--- a/lib/gitlab/kubernetes.rb
+++ b/lib/gitlab/kubernetes.rb
@@ -43,10 +43,10 @@ module Gitlab
end
end
- def add_terminal_auth(terminal, token, ca_pem = nil)
+ def add_terminal_auth(terminal, token:, max_session_time:, ca_pem: nil)
terminal[:headers]['Authorization'] << "Bearer #{token}"
+ terminal[:max_session_time] = max_session_time
terminal[:ca_pem] = ca_pem if ca_pem.present?
- terminal
end
def container_exec_url(api_url, namespace, pod_name, container_name)
diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb
index a3b502ffd6a752e83ebbbbc49a8a9c1b6eec48d1..c8872df8a935e54b424c1fcb0a0eb283a45ef4d7 100644
--- a/lib/gitlab/workhorse.rb
+++ b/lib/gitlab/workhorse.rb
@@ -107,7 +107,8 @@ module Gitlab
'Terminal' => {
'Subprotocols' => terminal[:subprotocols],
'Url' => terminal[:url],
- 'Header' => terminal[:headers]
+ 'Header' => terminal[:headers],
+ 'MaxSessionTime' => terminal[:max_session_time],
}
}
details['Terminal']['CAPem'] = terminal[:ca_pem] if terminal.has_key?(:ca_pem)
diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb
index 7dd4d76d1a39e27a7b32ad8f3e72f99051d21c6f..a32c6131030e619839d3e95436efcf055ea2acf7 100644
--- a/spec/lib/gitlab/workhorse_spec.rb
+++ b/spec/lib/gitlab/workhorse_spec.rb
@@ -42,7 +42,8 @@ describe Gitlab::Workhorse, lib: true do
out = {
subprotocols: ['foo'],
url: 'wss://example.com/terminal.ws',
- headers: { 'Authorization' => ['Token x'] }
+ headers: { 'Authorization' => ['Token x'] },
+ max_session_time: 600
}
out[:ca_pem] = ca_pem if ca_pem
out
@@ -53,7 +54,8 @@ describe Gitlab::Workhorse, lib: true do
'Terminal' => {
'Subprotocols' => ['foo'],
'Url' => 'wss://example.com/terminal.ws',
- 'Header' => { 'Authorization' => ['Token x'] }
+ 'Header' => { 'Authorization' => ['Token x'] },
+ 'MaxSessionTime' => 600
}
}
out['Terminal']['CAPem'] = ca_pem if ca_pem
diff --git a/spec/models/project_services/kubernetes_service_spec.rb b/spec/models/project_services/kubernetes_service_spec.rb
index 4f3cd14e941322efb4094e11c09ba3c1e8d8b274..9052479d35eb48916b71783b9dccd20445d05069 100644
--- a/spec/models/project_services/kubernetes_service_spec.rb
+++ b/spec/models/project_services/kubernetes_service_spec.rb
@@ -181,11 +181,23 @@ describe KubernetesService, models: true, caching: true do
let(:pod) { kube_pod(app: environment.slug) }
let(:terminals) { kube_terminals(service, pod) }
- it 'returns terminals' do
- stub_reactive_cache(service, pods: [ pod, pod, kube_pod(app: "should-be-filtered-out") ])
+ before do
+ stub_reactive_cache(
+ service,
+ pods: [ pod, pod, kube_pod(app: "should-be-filtered-out") ]
+ )
+ end
+ it 'returns terminals' do
is_expected.to eq(terminals + terminals)
end
+
+ it 'uses max session time from settings' do
+ stub_application_setting(terminal_max_session_time: 600)
+
+ times = subject.map { |terminal| terminal[:max_session_time] }
+ expect(times).to eq [600, 600, 600, 600]
+ end
end
end
diff --git a/spec/support/kubernetes_helpers.rb b/spec/support/kubernetes_helpers.rb
index 6c4c246a68b3357be3f0c2c6aa74e1bab4d45d48..444612cf8713091a75762c2a0e83fbb980478aa3 100644
--- a/spec/support/kubernetes_helpers.rb
+++ b/spec/support/kubernetes_helpers.rb
@@ -43,7 +43,8 @@ module KubernetesHelpers
url: container_exec_url(service.api_url, service.namespace, pod_name, container['name']),
subprotocols: ['channel.k8s.io'],
headers: { 'Authorization' => ["Bearer #{service.token}"] },
- created_at: DateTime.parse(pod['metadata']['creationTimestamp'])
+ created_at: DateTime.parse(pod['metadata']['creationTimestamp']),
+ max_session_time: 0
}
terminal[:ca_pem] = service.ca_pem if service.ca_pem.present?
terminal