From cffe7caa43575aead057d8779827bada786f84b6 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 12 Dec 2023 23:03:36 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@16-6-stable-ee --- .../repository/components/table/row.vue | 4 +- app/graphql/types/issue_type.rb | 2 +- app/graphql/types/merge_request_type.rb | 2 +- app/models/concerns/time_trackable.rb | 8 +-- app/models/timelog.rb | 12 ----- .../personal_access_tokens/rotate_service.rb | 40 +++++---------- .../project_access_tokens/rotate_service.rb | 58 ---------------------- 7 files changed, 17 insertions(+), 109 deletions(-) delete mode 100644 app/services/project_access_tokens/rotate_service.rb (limited to 'app') diff --git a/app/assets/javascripts/repository/components/table/row.vue b/app/assets/javascripts/repository/components/table/row.vue index bb2b8ae54b6..6a81f11eb51 100644 --- a/app/assets/javascripts/repository/components/table/row.vue +++ b/app/assets/javascripts/repository/components/table/row.vue @@ -120,13 +120,13 @@ export default { routerLinkTo() { if (this.isBlob) { return buildURLwithRefType({ - path: joinPaths('/-/blob', this.escapedRef, encodeURI(this.path)), + path: joinPaths('/-/blob', this.escapedRef, this.path), refType: this.refType, }); } if (this.isFolder) { return buildURLwithRefType({ - path: joinPaths('/-/tree', this.escapedRef, encodeURI(this.path)), + path: joinPaths('/-/tree', this.escapedRef, this.path), refType: this.refType, }); } diff --git a/app/graphql/types/issue_type.rb b/app/graphql/types/issue_type.rb index 7c7d559e05d..1c8a654a841 100644 --- a/app/graphql/types/issue_type.rb +++ b/app/graphql/types/issue_type.rb @@ -111,7 +111,7 @@ module Types field :time_estimate, GraphQL::Types::Int, null: false, description: 'Time estimate of the issue.' field :total_time_spent, GraphQL::Types::Int, null: false, - description: 'Total time (in seconds) reported as spent on the issue.' + description: 'Total time reported as spent on the issue.' field :closed_at, Types::TimeType, null: true, description: 'Timestamp of when the issue was closed.' diff --git a/app/graphql/types/merge_request_type.rb b/app/graphql/types/merge_request_type.rb index d7c3b313f84..9dca82f1750 100644 --- a/app/graphql/types/merge_request_type.rb +++ b/app/graphql/types/merge_request_type.rb @@ -199,7 +199,7 @@ module Types field :time_estimate, GraphQL::Types::Int, null: false, description: 'Time estimate of the merge request.' field :total_time_spent, GraphQL::Types::Int, null: false, - description: 'Total time (in seconds) reported as spent on the merge request.' + description: 'Total time reported as spent on the merge request.' field :approved, GraphQL::Types::Boolean, method: :approved?, diff --git a/app/models/concerns/time_trackable.rb b/app/models/concerns/time_trackable.rb index 70bc45b382a..0f361e70a91 100644 --- a/app/models/concerns/time_trackable.rb +++ b/app/models/concerns/time_trackable.rb @@ -45,13 +45,7 @@ module TimeTrackable # rubocop:enable Gitlab/ModuleWithInstanceVariables def total_time_spent - sum = timelogs.sum(:time_spent) - - # A new restriction has been introduced to limit total time spent to - - # Timelog::MAX_TOTAL_TIME_SPENT or 3.154e+7 seconds (approximately a year, a generous limit) - # Since there could be existing records that breach the limit, check and return the maximum/minimum allowed value. - # (some issuable might have total time spent that's negative because a validation was missing.) - sum.clamp(-Timelog::MAX_TOTAL_TIME_SPENT, Timelog::MAX_TOTAL_TIME_SPENT) + timelogs.sum(:time_spent) end def human_total_time_spent diff --git a/app/models/timelog.rb b/app/models/timelog.rb index eb088b1f582..b6b4decc64b 100644 --- a/app/models/timelog.rb +++ b/app/models/timelog.rb @@ -1,10 +1,6 @@ # frozen_string_literal: true class Timelog < ApplicationRecord - # Gitlab::TimeTrackingFormatter.parse("1y") == 31557600 seconds - # 31557600 slightly deviates from (365 days * 24 hours/day * 60 minutes/hour * 60 seconds/minute) - MAX_TOTAL_TIME_SPENT = 31557600.seconds.to_i # a year - include Importable include IgnorableColumns include Sortable @@ -16,7 +12,6 @@ class Timelog < ApplicationRecord validates :time_spent, :user, presence: true validates :summary, length: { maximum: 255 } validate :issuable_id_is_present, unless: :importing? - validate :check_total_time_spent_is_within_range, on: :create, unless: :importing?, if: :time_spent belongs_to :issue, touch: true belongs_to :merge_request, touch: true @@ -63,13 +58,6 @@ class Timelog < ApplicationRecord private - def check_total_time_spent_is_within_range - total_time_spent = issuable.timelogs.sum(:time_spent) + time_spent - - errors.add(:base, _("Total time spent cannot be negative.")) if total_time_spent < 0 - errors.add(:base, _("Total time spent cannot exceed a year.")) if total_time_spent > MAX_TOTAL_TIME_SPENT - end - def issuable_id_is_present if issue_id && merge_request_id errors.add(:base, _('Only Issue ID or merge request ID is required')) diff --git a/app/services/personal_access_tokens/rotate_service.rb b/app/services/personal_access_tokens/rotate_service.rb index 13144a04c11..32710629caf 100644 --- a/app/services/personal_access_tokens/rotate_service.rb +++ b/app/services/personal_access_tokens/rotate_service.rb @@ -10,18 +10,26 @@ module PersonalAccessTokens end def execute(params = {}) - return error_response(_('token already revoked')) if token.revoked? + return ServiceResponse.error(message: _('token already revoked')) if token.revoked? response = ServiceResponse.success PersonalAccessToken.transaction do unless token.revoke! - response = error_response(_('failed to revoke token')) + response = ServiceResponse.error(message: _('failed to revoke token')) raise ActiveRecord::Rollback end - response = create_access_token(params) - raise ActiveRecord::Rollback unless response.success? + target_user = token.user + new_token = target_user.personal_access_tokens.create(create_token_params(token, params)) + + if new_token.persisted? + response = ServiceResponse.success(payload: { personal_access_token: new_token }) + else + response = ServiceResponse.error(message: new_token.errors.full_messages.to_sentence) + + raise ActiveRecord::Rollback + end end response @@ -39,29 +47,5 @@ module PersonalAccessTokens scopes: token.scopes, expires_at: expires_at } end - - def create_access_token(params) - target_user = token.user - - new_token = target_user.personal_access_tokens.create(create_token_params(token, params)) - - return success_response(new_token) if new_token.persisted? - - error_response(new_token.errors.full_messages.to_sentence) - end - - def expires_at(params) - return params[:expires_at] if params[:expires_at] - - params[:expires_at] || EXPIRATION_PERIOD.from_now.to_date - end - - def success_response(new_token) - ServiceResponse.success(payload: { personal_access_token: new_token }) - end - - def error_response(message) - ServiceResponse.error(message: message) - end end end diff --git a/app/services/project_access_tokens/rotate_service.rb b/app/services/project_access_tokens/rotate_service.rb deleted file mode 100644 index 63d8d2a82cc..00000000000 --- a/app/services/project_access_tokens/rotate_service.rb +++ /dev/null @@ -1,58 +0,0 @@ -# frozen_string_literal: true - -module ProjectAccessTokens - class RotateService < ::PersonalAccessTokens::RotateService - extend ::Gitlab::Utils::Override - - def initialize(current_user, token, resource = nil) - @current_user = current_user - @token = token - @project = resource - end - - def execute(params = {}) - super - end - - attr_reader :project - - private - - override :create_access_token - def create_access_token(params) - target_user = token.user - - unless valid_access_level? - return error_response( - _("Not eligible to rotate token with access level higher than the user") - ) - end - - new_token = target_user.personal_access_tokens.create(create_token_params(token, params)) - - if new_token.persisted? - update_bot_membership(target_user, new_token.expires_at) - - return success_response(new_token) - end - - error_response(new_token.errors.full_messages.to_sentence) - end - - def update_bot_membership(target_user, expires_at) - target_user.members.update(expires_at: expires_at) - end - - def valid_access_level? - return true if current_user.can_admin_all_resources? - return false unless current_user.can?(:manage_resource_access_tokens, project) - - token_access_level = project.team.max_member_access(token.user.id).to_i - current_user_access_level = project.team.max_member_access(current_user.id).to_i - - return true if token_access_level.to_i <= current_user_access_level - - false - end - end -end -- cgit v1.2.3