diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 361109ba086bad8bf95299dc4da6cedfedafa411..b2a6f3ddd98ef57cffeb8d3ab054e2ce539db388 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -147,6 +147,7 @@ def application_setting_params_ce :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 abb334bff3dc172259fcac75c22c682929de7773..566a163b3507ea1bad8e1fc44f57d63ef0b14577 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -124,6 +124,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| @@ -217,7 +221,8 @@ def self.defaults_ce 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 @@ def terminals(environment) 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 @@ def join_api_url(*parts) 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 1e14b34329fa410364eb10bd78b2a8163e893de4..9e14fa85d058724aaff401df0eb80af29084d48a 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -573,5 +573,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 314d5041df6d3bdf8bcf3ba4e1d1321b47a0c3d3..8e6b5eb26c7b6611d299bf1b57077a80ef40072a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -119,6 +119,7 @@ t.boolean "plantuml_enabled" t.integer "shared_runners_minutes", default: 0, null: false t.integer "repository_size_limit", limit: 8, default: 0 + t.integer "terminal_max_session_time", default: 0, null: false end create_table "approvals", 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 a3a27e03b330d3e2bfeceae215f764db71a46dae..87e0d27116e63e1e4f59ae0b92c2207a17dd40db 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 } ``` @@ -91,6 +92,7 @@ PUT /application/settings | `repository_size_limit` | integer | no | Size limit per repository (MB) | | `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 @@ -125,6 +127,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 7b4d91dd337479c94348ba5587a93bde60216dfe..0afc74b8e46e53a1bafe3579b616824ad9285c4c 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -628,6 +628,7 @@ class ApplicationSetting < Grape::Entity 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 3bec88a73e8a3e670693573211b3f7a2bd6b9a88..c7b007eefad03e11475e937273def375ae600c10 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -108,6 +108,7 @@ def current_settings 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.' # GitLab-EE specific settings optional :help_text, type: String, desc: 'GitLab server administrator information' optional :elasticsearch_indexing, type: Boolean, desc: 'Enable Elasticsearch indexing' @@ -134,6 +135,7 @@ def current_settings :akismet_enabled, :admin_notification_email, :sentry_enabled, :repository_checks_enabled, :koding_enabled, :housekeeping_enabled, :plantuml_enabled, :version_check_enabled, :email_author_in_body, :html_emails_enabled, + :terminal_max_session_time, # GitLab-EE specific settings :help_text, :elasticsearch_indexing, :usage_ping_enabled, :repository_storage, :repository_storages, :repository_size_limit 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 @@ def terminals_for_pod(api_url, namespace, pod) 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 @@ def terminal_websocket(terminal) '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 @@ def terminal(ca_pem: nil) 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 @@ def workhorse(ca_pem: nil) '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 @@ def stub_kubeclient_pods 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 @@ def kube_terminals(service, pod) 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