From 2caff97cb6220be2357c31fe8e1e1a80600efb43 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Fri, 24 Sep 2021 16:26:27 -0400 Subject: [PATCH 1/2] API: Expose token_expires_at when resetting runner token When a runner's authentication token is reset, or when the runner is created, the API also returns the token_expires_at field. Changelog: added Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/30942 --- doc/api/runners.md | 6 +++-- lib/api/ci/runners.rb | 8 +++---- lib/api/entities/ci/reset_token_result.rb | 3 ++- .../ci/runner_registration_details.rb | 2 +- .../api/ci/runner/runners_post_spec.rb | 24 +++++++++++++++---- spec/requests/api/ci/runners_spec.rb | 21 +++++++++++++--- .../ci/register_runner_service_spec.rb | 11 +++++++++ 7 files changed, 59 insertions(+), 16 deletions(-) diff --git a/doc/api/runners.md b/doc/api/runners.md index 2660d8da33bf00..3423a02c078285 100644 --- a/doc/api/runners.md +++ b/doc/api/runners.md @@ -679,7 +679,8 @@ Example response: ```json { "id": 12345, - "token": "6337ff461c94fd3fa32ba3b1ff4125" + "token": "6337ff461c94fd3fa32ba3b1ff4125", + "token_expires_at": "2021-09-27T21:05:03.203Z" } ``` @@ -819,6 +820,7 @@ Example response: ```json { - "token": "6337ff461c94fd3fa32ba3b1ff4125" + "token": "6337ff461c94fd3fa32ba3b1ff4125", + "token_expires_at": "2021-09-27T21:05:03.203Z" } ``` diff --git a/lib/api/ci/runners.rb b/lib/api/ci/runners.rb index 12c336e0938f6b..8281031010f664 100644 --- a/lib/api/ci/runners.rb +++ b/lib/api/ci/runners.rb @@ -143,7 +143,7 @@ class Runners < ::API::Base authenticate_update_runner!(runner) runner.reset_token! - present runner.token, with: Entities::Ci::ResetTokenResult + present runner.token_with_expiration, with: Entities::Ci::ResetTokenResult end end @@ -249,7 +249,7 @@ class Runners < ::API::Base authorize! :update_runners_registration_token ApplicationSetting.current.reset_runners_registration_token! - present ApplicationSetting.current_without_cache.runners_registration_token, with: Entities::Ci::ResetTokenResult + present ApplicationSetting.current_without_cache.runners_registration_token_with_expiration, with: Entities::Ci::ResetTokenResult end end @@ -267,7 +267,7 @@ class Runners < ::API::Base authorize! :update_runners_registration_token, project project.reset_runners_token! - present project.runners_token, with: Entities::Ci::ResetTokenResult + present project.runners_token_with_expiration, with: Entities::Ci::ResetTokenResult end end @@ -285,7 +285,7 @@ class Runners < ::API::Base authorize! :update_runners_registration_token, group group.reset_runners_token! - present group.runners_token, with: Entities::Ci::ResetTokenResult + present group.runners_token_with_expiration, with: Entities::Ci::ResetTokenResult end end diff --git a/lib/api/entities/ci/reset_token_result.rb b/lib/api/entities/ci/reset_token_result.rb index 4dbf831582bd62..f0b1de6a5a75e5 100644 --- a/lib/api/entities/ci/reset_token_result.rb +++ b/lib/api/entities/ci/reset_token_result.rb @@ -4,7 +4,8 @@ module API module Entities module Ci class ResetTokenResult < Grape::Entity - expose(:token) {|object| object} + expose(:token) + expose(:token_expires_at, if: -> (object, options) { object.expirable? }) end end end diff --git a/lib/api/entities/ci/runner_registration_details.rb b/lib/api/entities/ci/runner_registration_details.rb index fa7e44c9e402a6..53be918406f2cc 100644 --- a/lib/api/entities/ci/runner_registration_details.rb +++ b/lib/api/entities/ci/runner_registration_details.rb @@ -4,7 +4,7 @@ module API module Entities module Ci class RunnerRegistrationDetails < Grape::Entity - expose :id, :token + expose :id, :token, :token_expires_at end end end diff --git a/spec/requests/api/ci/runner/runners_post_spec.rb b/spec/requests/api/ci/runner/runners_post_spec.rb index 27917933c6862e..5eb5d3977a30cd 100644 --- a/spec/requests/api/ci/runner/runners_post_spec.rb +++ b/spec/requests/api/ci/runner/runners_post_spec.rb @@ -62,12 +62,26 @@ def request end end - it 'creates runner' do - request + context 'when token_expires_at is nil' do + it 'creates runner' do + request - expect(response).to have_gitlab_http_status(:created) - expect(json_response['id']).to eq(new_runner.id) - expect(json_response['token']).to eq(new_runner.token) + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to eq({ 'id' => new_runner.id, 'token' => new_runner.token, 'token_expires_at' => nil }) + end + end + + context 'when token_expires_at is a valid date' do + before do + new_runner.token_expires_at = DateTime.new(2022, 1, 11, 14, 39, 24) + end + + it 'creates runner' do + request + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to eq({ 'id' => new_runner.id, 'token' => new_runner.token, 'token_expires_at' => '2022-01-11T14:39:24.000Z' }) + end end it_behaves_like 'storing arguments in the application context for the API' do diff --git a/spec/requests/api/ci/runners_spec.rb b/spec/requests/api/ci/runners_spec.rb index 466ae29cc4cec8..b582c8a98e000c 100644 --- a/spec/requests/api/ci/runners_spec.rb +++ b/spec/requests/api/ci/runners_spec.rb @@ -648,7 +648,7 @@ def update_runner(id, user, args) post api("/runners/#{shared_runner.id}/reset_authentication_token", admin) expect(response).to have_gitlab_http_status(:success) - expect(json_response).to eq({ 'token' => shared_runner.reload.token }) + expect(json_response).to eq({ 'token' => shared_runner.reload.token, 'token_expires_at' => nil }) end.to change { shared_runner.reload.token } end @@ -672,7 +672,7 @@ def update_runner(id, user, args) post api("/runners/#{project_runner.id}/reset_authentication_token", user) expect(response).to have_gitlab_http_status(:success) - expect(json_response).to eq({ 'token' => project_runner.reload.token }) + expect(json_response).to eq({ 'token' => project_runner.reload.token, 'token_expires_at' => nil }) end.to change { project_runner.reload.token } end @@ -713,7 +713,22 @@ def update_runner(id, user, args) post api("/runners/#{group_runner_a.id}/reset_authentication_token", user) expect(response).to have_gitlab_http_status(:success) - expect(json_response).to eq({ 'token' => group_runner_a.reload.token }) + expect(json_response).to eq({ 'token' => group_runner_a.reload.token, 'token_expires_at' => nil }) + end.to change { group_runner_a.reload.token } + end + + it 'resets group runner authentication token with owner access with expiration time', :freeze_time do + expect(group_runner_a.reload.token_expires_at).to be_nil + + group.update!(runner_token_expiration_interval: 5.days) + + expect do + post api("/runners/#{group_runner_a.id}/reset_authentication_token", user) + group_runner_a.reload + + expect(response).to have_gitlab_http_status(:success) + expect(json_response).to eq({ 'token' => group_runner_a.token, 'token_expires_at' => group_runner_a.token_expires_at.iso8601(3) }) + expect(group_runner_a.token_expires_at).to eq(5.days.from_now) end.to change { group_runner_a.reload.token } end end diff --git a/spec/services/ci/register_runner_service_spec.rb b/spec/services/ci/register_runner_service_spec.rb index 77e1d2325000d5..491582bbd1397f 100644 --- a/spec/services/ci/register_runner_service_spec.rb +++ b/spec/services/ci/register_runner_service_spec.rb @@ -85,6 +85,17 @@ expect(subject.ip_address).to eq args[:ip_address] end end + + context 'with runner token expiration interval', :freeze_time do + before do + stub_application_setting(runner_token_expiration_interval: 5.days) + end + + it 'creates runner with token expiration' do + is_expected.to be_an_instance_of(::Ci::Runner) + expect(subject.token_expires_at).to eq(5.days.from_now) + end + end end context 'when project token is used' do -- GitLab From 8cc28b563f6b7ecbb009f0ebea9a50021e107c9e Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Fri, 5 Nov 2021 11:13:24 -0400 Subject: [PATCH 2/2] API: Add runner token expiration intervals to /application/settings This allows the frontend to read and write these interval settings. Changelog: added Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/30942 Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/342527 --- app/helpers/application_settings_helper.rb | 5 +++- lib/api/settings.rb | 3 ++ spec/requests/api/settings_spec.rb | 35 ++++++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 9ba1b7ecd7b4e5..fa9b3bfc9124ce 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -425,7 +425,10 @@ def visible_attributes :suggest_pipeline_enabled, :user_email_lookup_limit, :users_get_by_id_limit, - :users_get_by_id_limit_allowlist_raw + :users_get_by_id_limit_allowlist_raw, + :runner_token_expiration_interval, + :group_runner_token_expiration_interval, + :project_runner_token_expiration_interval ].tap do |settings| settings << :deactivate_dormant_users unless Gitlab.com? end diff --git a/lib/api/settings.rb b/lib/api/settings.rb index 90868037938fa1..b256432fbf1479 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -178,6 +178,9 @@ def filter_attributes_using_license(attrs) optional :user_deactivation_emails_enabled, type: Boolean, desc: 'Send emails to users upon account deactivation' optional :suggest_pipeline_enabled, type: Boolean, desc: 'Enable pipeline suggestion banner' optional :users_get_by_id_limit, type: Integer, desc: "Maximum number of calls to the /users/:id API per 10 minutes per user. Set to 0 for unlimited requests." + optional :runner_token_expiration_interval, type: Integer, desc: 'Token expiration interval for shared runners, in seconds' + optional :group_runner_token_expiration_interval, type: Integer, desc: 'Token expiration interval for group runners, in seconds' + optional :project_runner_token_expiration_interval, type: Integer, desc: 'Token expiration interval for project runners, in seconds' ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type| optional :"#{type}_key_restriction", diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 4e85bdebdb06ab..f7048a1ca6b1a8 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -51,6 +51,9 @@ expect(json_response['whats_new_variant']).to eq('all_tiers') expect(json_response['user_deactivation_emails_enabled']).to be(true) expect(json_response['suggest_pipeline_enabled']).to be(true) + expect(json_response['runner_token_expiration_interval']).to be_nil + expect(json_response['group_runner_token_expiration_interval']).to be_nil + expect(json_response['project_runner_token_expiration_interval']).to be_nil end end @@ -652,5 +655,37 @@ end end end + + context 'runner token expiration_intervals' do + it 'updates the settings' do + put api("/application/settings", admin), params: { + runner_token_expiration_interval: 3600, + group_runner_token_expiration_interval: 3600 * 2, + project_runner_token_expiration_interval: 3600 * 3 + } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include( + 'runner_token_expiration_interval' => 3600, + 'group_runner_token_expiration_interval' => 3600 * 2, + 'project_runner_token_expiration_interval' => 3600 * 3 + ) + end + + it 'updates the settings with empty values' do + put api("/application/settings", admin), params: { + runner_token_expiration_interval: nil, + group_runner_token_expiration_interval: nil, + project_runner_token_expiration_interval: nil + } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include( + 'runner_token_expiration_interval' => nil, + 'group_runner_token_expiration_interval' => nil, + 'project_runner_token_expiration_interval' => nil + ) + end + end end end -- GitLab