diff options
author | Mayra Cabrera <mcabrera@gitlab.com> | 2018-04-06 22:48:17 +0300 |
---|---|---|
committer | Mayra Cabrera <mcabrera@gitlab.com> | 2018-04-07 06:28:44 +0300 |
commit | 5bc58bac2678aed9c8b2318f9f4d4825baa2b110 (patch) | |
tree | f35313fd689afa287f6c93a3d78ce8a0d61cc71c | |
parent | d6450717abefbe4dbf891cb4d285f6c84e44f168 (diff) |
Handle limit for datetime attributes on MySQL
The TIMESTAMP data type is used for values that contain both date and
time parts. TIMESTAMP has a range of '1970-01-01 00:00:01' UTC to
'2038-01-19 03:14:07' UTC.
A Forever lib class was included to handle future dates for PostgreSQL
and MySQL, also changes were made to DeployToken to enforce Forever.date
Also removes extra conditional from JwtController
-rw-r--r-- | app/controllers/jwt_controller.rb | 7 | ||||
-rw-r--r-- | app/controllers/projects/deploy_tokens_controller.rb | 6 | ||||
-rw-r--r-- | app/helpers/deploy_tokens_helper.rb | 8 | ||||
-rw-r--r-- | app/models/deploy_token.rb | 12 | ||||
-rw-r--r-- | app/services/deploy_tokens/create_service.rb | 17 | ||||
-rw-r--r-- | app/views/projects/deploy_tokens/_form.html.haml | 2 | ||||
-rw-r--r-- | app/views/projects/deploy_tokens/_table.html.haml | 2 | ||||
-rw-r--r-- | lib/forever.rb | 13 | ||||
-rw-r--r-- | spec/features/projects/settings/repository_settings_spec.rb | 14 | ||||
-rw-r--r-- | spec/lib/forever_spec.rb | 21 | ||||
-rw-r--r-- | spec/models/deploy_token_spec.rb | 38 | ||||
-rw-r--r-- | spec/services/deploy_tokens/create_service_spec.rb | 4 |
12 files changed, 89 insertions, 55 deletions
diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb index 0caa5f4f439..67057b5b126 100644 --- a/app/controllers/jwt_controller.rb +++ b/app/controllers/jwt_controller.rb @@ -25,8 +25,7 @@ class JwtController < ApplicationController authenticate_with_http_basic do |login, password| @authentication_result = Gitlab::Auth.find_for_git_client(login, password, project: nil, ip: request.ip) - if @authentication_result.failed? || - (@authentication_result.actor.present? && !user_or_deploy_token) + if @authentication_result.failed? render_unauthorized end end @@ -57,8 +56,4 @@ class JwtController < ApplicationController def auth_params params.permit(:service, :scope, :account, :client_id) end - - def user_or_deploy_token - @authentication_result.actor.is_a?(User) || @authentication_result.actor.is_a?(DeployToken) - end end diff --git a/app/controllers/projects/deploy_tokens_controller.rb b/app/controllers/projects/deploy_tokens_controller.rb index 5d236966894..2f91b8f36de 100644 --- a/app/controllers/projects/deploy_tokens_controller.rb +++ b/app/controllers/projects/deploy_tokens_controller.rb @@ -7,10 +7,4 @@ class Projects::DeployTokensController < Projects::ApplicationController redirect_to project_settings_repository_path(project) end - - private - - def deploy_token_params - params.require(:deploy_token).permit(:name, :expires_at, :read_repository, :read_registry) - end end diff --git a/app/helpers/deploy_tokens_helper.rb b/app/helpers/deploy_tokens_helper.rb index 31aa041b00a..bd921322476 100644 --- a/app/helpers/deploy_tokens_helper.rb +++ b/app/helpers/deploy_tokens_helper.rb @@ -9,12 +9,4 @@ module DeployTokensHelper Gitlab.config.registry.enabled && can?(current_user, :read_container_image, project) end - - def expires_at_value(expires_at) - expires_at unless expires_at >= DeployToken::FOREVER - end - - def show_expire_at?(token) - token.expires? && token.expires_at != DeployToken::FOREVER - end end diff --git a/app/models/deploy_token.rb b/app/models/deploy_token.rb index bfdc5457157..fe726b156d4 100644 --- a/app/models/deploy_token.rb +++ b/app/models/deploy_token.rb @@ -4,9 +4,8 @@ class DeployToken < ActiveRecord::Base add_authentication_token_field :token AVAILABLE_SCOPES = %i(read_repository read_registry).freeze - FOREVER = DateTime.new(3000, 1, 1) - default_value_for :expires_at, FOREVER + default_value_for(:expires_at) { Forever.date } has_many :project_deploy_tokens, inverse_of: :deploy_token has_many :projects, through: :project_deploy_tokens @@ -45,6 +44,15 @@ class DeployToken < ActiveRecord::Base projects.first end + def expires_at + expires_at = read_attribute(:expires_at) + expires_at != Forever.date ? expires_at : nil + end + + def expires_at=(value) + write_attribute(:expires_at, value.presence || Forever.date) + end + private def ensure_at_least_one_scope diff --git a/app/services/deploy_tokens/create_service.rb b/app/services/deploy_tokens/create_service.rb index 04977ca4c18..52f545947af 100644 --- a/app/services/deploy_tokens/create_service.rb +++ b/app/services/deploy_tokens/create_service.rb @@ -1,22 +1,7 @@ module DeployTokens class CreateService < BaseService def execute - @project.deploy_tokens.create(deploy_token_params) - end - - private - - def deploy_token_params - params[:expires_at] = expires_at_date - params - end - - def expires_at_date - params[:expires_at].presence || default_expires_at - end - - def default_expires_at - DeployToken::FOREVER + @project.deploy_tokens.create(params) end end end diff --git a/app/views/projects/deploy_tokens/_form.html.haml b/app/views/projects/deploy_tokens/_form.html.haml index 4e1a796ade0..f8db30df7b4 100644 --- a/app/views/projects/deploy_tokens/_form.html.haml +++ b/app/views/projects/deploy_tokens/_form.html.haml @@ -10,7 +10,7 @@ .form-group = f.label :expires_at, class: 'label-light' - = f.text_field :expires_at, class: 'datepicker form-control', value: expires_at_value(token.expires_at) + = f.text_field :expires_at, class: 'datepicker form-control', value: f.object.expires_at .form-group = f.label :scopes, class: 'label-light' diff --git a/app/views/projects/deploy_tokens/_table.html.haml b/app/views/projects/deploy_tokens/_table.html.haml index fe9bb1e724a..5013a9b250d 100644 --- a/app/views/projects/deploy_tokens/_table.html.haml +++ b/app/views/projects/deploy_tokens/_table.html.haml @@ -18,7 +18,7 @@ %td= token.username %td= token.created_at.to_date.to_s(:medium) %td - - if show_expire_at?(token) + - if token.expires? %span{ class: ('text-warning' if token.expires_soon?) } In #{distance_of_time_in_words_to_now(token.expires_at)} - else diff --git a/lib/forever.rb b/lib/forever.rb new file mode 100644 index 00000000000..7df17912544 --- /dev/null +++ b/lib/forever.rb @@ -0,0 +1,13 @@ +class Forever + POSTGRESQL_DATE = DateTime.new(3000, 1, 1) + MYSQL_DATE = DateTime.new(2038, 01, 19) + + # MySQL timestamp has a range of '1970-01-01 00:00:01' UTC to '2038-01-19 03:14:07' UTC + def self.date + if Gitlab::Database.postgresql? + POSTGRESQL_DATE + else + MYSQL_DATE + end + end +end diff --git a/spec/features/projects/settings/repository_settings_spec.rb b/spec/features/projects/settings/repository_settings_spec.rb index 2528c7f437d..f2c371b7df5 100644 --- a/spec/features/projects/settings/repository_settings_spec.rb +++ b/spec/features/projects/settings/repository_settings_spec.rb @@ -90,8 +90,7 @@ feature 'Repository settings' do end context 'Deploy tokens' do - let(:deploy_token_project) { create(:project_deploy_token, project: project) } - let!(:deploy_token) { deploy_token_project.deploy_token } + let!(:deploy_token) { create(:deploy_token, projects: [project]) } before do stub_container_registry_config(enabled: true) @@ -115,17 +114,6 @@ feature 'Repository settings' do expect(page).to have_content('Your new project deploy token has been created') end - - scenario 'revoke a deploy token', :js do - within('.deploy-tokens') do - click_link 'Revoke' - click_link "Revoke #{deploy_token.name}" - - expect(page).not_to have_content(deploy_token.name) - expect(page).not_to have_content('read_repository') - expect(page).not_to have_content('read_registry') - end - end end end end diff --git a/spec/lib/forever_spec.rb b/spec/lib/forever_spec.rb new file mode 100644 index 00000000000..cf40c467c72 --- /dev/null +++ b/spec/lib/forever_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe Forever do + describe '.date' do + subject { described_class.date } + + context 'when using PostgreSQL' do + it 'should return Postgresql future date' do + allow(Gitlab::Database).to receive(:postgresql?).and_return(true) + expect(subject).to eq(described_class::POSTGRESQL_DATE) + end + end + + context 'when using MySQL' do + it 'should return MySQL future date' do + allow(Gitlab::Database).to receive(:postgresql?).and_return(false) + expect(subject).to eq(described_class::MYSQL_DATE) + end + end + end +end diff --git a/spec/models/deploy_token_spec.rb b/spec/models/deploy_token_spec.rb index 1adc049ca58..5a15c23def4 100644 --- a/spec/models/deploy_token_spec.rb +++ b/spec/models/deploy_token_spec.rb @@ -93,4 +93,42 @@ describe DeployToken do end end end + + describe '#expires_at' do + context 'when using Forever.date' do + let(:deploy_token) { create(:deploy_token, expires_at: nil) } + + it 'should return nil' do + expect(deploy_token.expires_at).to be_nil + end + end + + context 'when using a personalized date' do + let(:expires_at) { Date.today + 5.months } + let(:deploy_token) { create(:deploy_token, expires_at: expires_at) } + + it 'should return the personalized date' do + expect(deploy_token.expires_at).to eq(expires_at) + end + end + end + + describe '#expires_at=' do + context 'when passing nil' do + let(:deploy_token) { create(:deploy_token, expires_at: nil) } + + it 'should assign Forever.date' do + expect(deploy_token.read_attribute(:expires_at)).to eq(Forever.date) + end + end + + context 'when passign a value' do + let(:expires_at) { Date.today + 5.months } + let(:deploy_token) { create(:deploy_token, expires_at: expires_at) } + + it 'should respect the value' do + expect(deploy_token.read_attribute(:expires_at)).to eq(expires_at) + end + end + end end diff --git a/spec/services/deploy_tokens/create_service_spec.rb b/spec/services/deploy_tokens/create_service_spec.rb index 2e02b7a28b5..3a2bbf1ecd1 100644 --- a/spec/services/deploy_tokens/create_service_spec.rb +++ b/spec/services/deploy_tokens/create_service_spec.rb @@ -25,8 +25,8 @@ describe DeployTokens::CreateService do context 'when expires at date is not passed' do let(:deploy_token_params) { attributes_for(:deploy_token, expires_at: '') } - it 'should set FOREVER date' do - expect(subject.expires_at).to eq(DeployToken::FOREVER) + it 'should set Forever.date' do + expect(subject.read_attribute(:expires_at)).to eq(Forever.date) end end |