Commit 29913816 authored by Mayra Cabrera's avatar Mayra Cabrera

Addresses database comments

- Adds a default on expires_at datetime
- Modifies deploy tokens views to handle default expires at value
- Use datetime_with_timezone where possible
- Remove unused scopes
parent ca35c65b
...@@ -11,7 +11,7 @@ module Projects ...@@ -11,7 +11,7 @@ module Projects
@new_deploy_token = DeployTokens::CreateService.new(@project, current_user, deploy_token_params).execute @new_deploy_token = DeployTokens::CreateService.new(@project, current_user, deploy_token_params).execute
if @new_deploy_token.persisted? if @new_deploy_token.persisted?
flash.now[:notice] = 'Your new project deploy token has been created.' flash.now[:notice] = s_('DeployTokens|Your new project deploy token has been created.')
end end
render_show render_show
......
...@@ -5,8 +5,16 @@ module DeployTokensHelper ...@@ -5,8 +5,16 @@ module DeployTokensHelper
Rails.env.test? Rails.env.test?
end end
def container_registry_enabled? def container_registry_enabled?(project)
Gitlab.config.registry.enabled && Gitlab.config.registry.enabled &&
can?(current_user, :read_container_image, @project) can?(current_user, :read_container_image, project)
end
def expires_at_value(expires_at)
expires_at unless expires_at >= DeployToken::FUTURE_DATE
end
def show_expire_at?(token)
token.expires? && token.expires_at != DeployToken::FUTURE_DATE
end end
end end
...@@ -4,6 +4,7 @@ class DeployToken < ActiveRecord::Base ...@@ -4,6 +4,7 @@ class DeployToken < ActiveRecord::Base
add_authentication_token_field :token add_authentication_token_field :token
AVAILABLE_SCOPES = %i(read_repository read_registry).freeze AVAILABLE_SCOPES = %i(read_repository read_registry).freeze
FUTURE_DATE = Date.new(3000 - 01 - 01)
has_many :project_deploy_tokens, inverse_of: :deploy_token has_many :project_deploy_tokens, inverse_of: :deploy_token
has_many :projects, through: :project_deploy_tokens has_many :projects, through: :project_deploy_tokens
...@@ -13,9 +14,7 @@ class DeployToken < ActiveRecord::Base ...@@ -13,9 +14,7 @@ class DeployToken < ActiveRecord::Base
accepts_nested_attributes_for :project_deploy_tokens accepts_nested_attributes_for :project_deploy_tokens
scope :active, -> { where("revoked = false AND (expires_at >= NOW() OR expires_at IS NULL)") } scope :active, -> { where("revoked = false AND expires_at >= NOW()") }
scope :read_repository, -> { where(read_repository: true) }
scope :read_registry, -> { where(read_registry: true) }
def revoke! def revoke!
update!(revoked: true) update!(revoked: true)
...@@ -26,7 +25,7 @@ class DeployToken < ActiveRecord::Base ...@@ -26,7 +25,7 @@ class DeployToken < ActiveRecord::Base
end end
def scopes def scopes
AVAILABLE_SCOPES.select { |token_scope| send("#{token_scope}") } # rubocop:disable GitlabSecurity/PublicSend AVAILABLE_SCOPES.select { |token_scope| read_attribute(token_scope) }
end end
def username def username
...@@ -37,6 +36,9 @@ class DeployToken < ActiveRecord::Base ...@@ -37,6 +36,9 @@ class DeployToken < ActiveRecord::Base
project == requested_project project == requested_project
end end
# This is temporal. Currently we limit DeployToken
# to a single project, later we're going to extend
# that to be for multiple projects and namespaces.
def project def project
projects.first projects.first
end end
......
...@@ -124,8 +124,8 @@ module Auth ...@@ -124,8 +124,8 @@ module Auth
end end
def can_user?(ability, project) def can_user?(ability, project)
current_user.is_a?(User) && user = current_user.is_a?(User) ? current_user : nil
can?(current_user, ability, project) can?(user, ability, project)
end end
def build_can_pull?(requested_project) def build_can_pull?(requested_project)
...@@ -143,7 +143,7 @@ module Auth ...@@ -143,7 +143,7 @@ module Auth
def user_can_pull?(requested_project) def user_can_pull?(requested_project)
has_authentication_ability?(:read_container_image) && has_authentication_ability?(:read_container_image) &&
can?(current_user, :read_container_image, requested_project) can_user?(:read_container_image, requested_project)
end end
def deploy_token_can_pull?(requested_project) def deploy_token_can_pull?(requested_project)
......
module DeployTokens module DeployTokens
class CreateService < BaseService class CreateService < BaseService
def execute def execute
@project.deploy_tokens.build(params).tap(&:save) @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].present? ? default_expires_at : params[:expires_at]
end
def default_expires_at
DeployToken::FUTURE_DATE
end end
end end
end end
...@@ -10,7 +10,7 @@ ...@@ -10,7 +10,7 @@
.form-group .form-group
= f.label :expires_at, class: 'label-light' = f.label :expires_at, class: 'label-light'
= f.text_field :expires_at, class: 'datepicker form-control' = f.text_field :expires_at, class: 'datepicker form-control', value: expires_at_value(token.expires_at)
.form-group .form-group
= f.label :scopes, class: 'label-light' = f.label :scopes, class: 'label-light'
...@@ -19,7 +19,7 @@ ...@@ -19,7 +19,7 @@
= label_tag ("deploy_token_read_repository"), 'read_repository' = label_tag ("deploy_token_read_repository"), 'read_repository'
%span= s_('DeployTokens|Allows read-only access to the repository') %span= s_('DeployTokens|Allows read-only access to the repository')
- if container_registry_enabled? - if container_registry_enabled?(project)
%fieldset %fieldset
= f.check_box :read_registry = f.check_box :read_registry
= label_tag ("deploy_token_read_registry"), 'read_registry' = label_tag ("deploy_token_read_registry"), 'read_registry'
......
...@@ -18,7 +18,7 @@ ...@@ -18,7 +18,7 @@
%td= token.username %td= token.username
%td= token.created_at.to_date.to_s(:medium) %td= token.created_at.to_date.to_s(:medium)
%td %td
- if token.expires? - if show_expire_at?(token)
%span{ class: ('text-warning' if token.expires_soon?) } %span{ class: ('text-warning' if token.expires_soon?) }
In #{distance_of_time_in_words_to_now(token.expires_at)} In #{distance_of_time_in_words_to_now(token.expires_at)}
- else - else
......
...@@ -7,11 +7,13 @@ class CreateDeployTokens < ActiveRecord::Migration ...@@ -7,11 +7,13 @@ class CreateDeployTokens < ActiveRecord::Migration
t.boolean :read_repository, null: false, default: false t.boolean :read_repository, null: false, default: false
t.boolean :read_registry, null: false, default: false t.boolean :read_registry, null: false, default: false
t.datetime :expires_at t.datetime_with_timezone :expires_at, null: false, default: '3000-01-01'
t.timestamps null: false t.datetime_with_timezone :created_at, null: false
t.string :name, null: false t.string :name, null: false
t.string :token, index: { unique: true }, null: false t.string :token, index: { unique: true }, null: false
t.index [:token, :expires_at], where: "(revoked IS FALSE)"
end end
end end
end end
class CreateProjectDeployTokens < ActiveRecord::Migration class CreateProjectDeployTokens < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false DOWNTIME = false
def change def change
create_table :project_deploy_tokens do |t| create_table :project_deploy_tokens do |t|
t.integer :project_id, null: false t.integer :project_id, null: false
t.integer :deploy_token_id, null: false t.integer :deploy_token_id, null: false
t.timestamps null: false t.datetime_with_timezone :created_at, null: false
t.foreign_key :deploy_tokens, column: :deploy_token_id, on_delete: :cascade t.foreign_key :deploy_tokens, column: :deploy_token_id, on_delete: :cascade
t.foreign_key :projects, column: :project_id, on_delete: :cascade t.foreign_key :projects, column: :project_id, on_delete: :cascade
......
...@@ -687,13 +687,13 @@ ActiveRecord::Schema.define(version: 20180405142733) do ...@@ -687,13 +687,13 @@ ActiveRecord::Schema.define(version: 20180405142733) do
t.boolean "revoked", default: false t.boolean "revoked", default: false
t.boolean "read_repository", default: false, null: false t.boolean "read_repository", default: false, null: false
t.boolean "read_registry", default: false, null: false t.boolean "read_registry", default: false, null: false
t.datetime "expires_at" t.datetime_with_timezone "expires_at", default: '3000-01-01 00:00:00', null: false
t.datetime "created_at", null: false t.datetime_with_timezone "created_at", null: false
t.datetime "updated_at", null: false
t.string "name", null: false t.string "name", null: false
t.string "token", null: false t.string "token", null: false
end end
add_index "deploy_tokens", ["token", "expires_at"], name: "index_deploy_tokens_on_token_and_expires_at", where: "(revoked IS FALSE)", using: :btree
add_index "deploy_tokens", ["token"], name: "index_deploy_tokens_on_token", unique: true, using: :btree add_index "deploy_tokens", ["token"], name: "index_deploy_tokens_on_token", unique: true, using: :btree
create_table "deployments", force: :cascade do |t| create_table "deployments", force: :cascade do |t|
...@@ -1446,8 +1446,7 @@ ActiveRecord::Schema.define(version: 20180405142733) do ...@@ -1446,8 +1446,7 @@ ActiveRecord::Schema.define(version: 20180405142733) do
create_table "project_deploy_tokens", force: :cascade do |t| create_table "project_deploy_tokens", force: :cascade do |t|
t.integer "project_id", null: false t.integer "project_id", null: false
t.integer "deploy_token_id", null: false t.integer "deploy_token_id", null: false
t.datetime "created_at", null: false t.datetime_with_timezone "created_at", null: false
t.datetime "updated_at", null: false
end end
add_index "project_deploy_tokens", ["project_id", "deploy_token_id"], name: "index_project_deploy_tokens_on_project_id_and_deploy_token_id", unique: true, using: :btree add_index "project_deploy_tokens", ["project_id", "deploy_token_id"], name: "index_project_deploy_tokens_on_project_id_and_deploy_token_id", unique: true, using: :btree
......
...@@ -94,6 +94,7 @@ feature 'Repository settings' do ...@@ -94,6 +94,7 @@ feature 'Repository settings' do
let!(:deploy_token) { deploy_token_project.deploy_token } let!(:deploy_token) { deploy_token_project.deploy_token }
before do before do
stub_container_registry_config(enabled: true)
visit project_settings_repository_path(project) visit project_settings_repository_path(project)
end end
......
...@@ -7,7 +7,6 @@ RSpec.describe ProjectDeployToken, type: :model do ...@@ -7,7 +7,6 @@ RSpec.describe ProjectDeployToken, type: :model do
it { is_expected.to belong_to :project } it { is_expected.to belong_to :project }
it { is_expected.to belong_to :deploy_token } it { is_expected.to belong_to :deploy_token }
it { is_expected.to accept_nested_attributes_for :deploy_token }
it { is_expected.to validate_presence_of :deploy_token } it { is_expected.to validate_presence_of :deploy_token }
it { is_expected.to validate_presence_of :project } it { is_expected.to validate_presence_of :project }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment