diff options
34 files changed, 1201 insertions, 77 deletions
diff --git a/app/assets/javascripts/repository/components/table/row.vue b/app/assets/javascripts/repository/components/table/row.vue index 6a81f11eb51..bb2b8ae54b6 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, this.path), + path: joinPaths('/-/blob', this.escapedRef, encodeURI(this.path)), refType: this.refType, }); } if (this.isFolder) { return buildURLwithRefType({ - path: joinPaths('/-/tree', this.escapedRef, this.path), + path: joinPaths('/-/tree', this.escapedRef, encodeURI(this.path)), refType: this.refType, }); } diff --git a/app/graphql/types/issue_type.rb b/app/graphql/types/issue_type.rb index 1c8a654a841..7c7d559e05d 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 reported as spent on the issue.' + description: 'Total time (in seconds) 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 9dca82f1750..d7c3b313f84 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 reported as spent on the merge request.' + description: 'Total time (in seconds) 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 0f361e70a91..70bc45b382a 100644 --- a/app/models/concerns/time_trackable.rb +++ b/app/models/concerns/time_trackable.rb @@ -45,7 +45,13 @@ module TimeTrackable # rubocop:enable Gitlab/ModuleWithInstanceVariables def total_time_spent - timelogs.sum(: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) end def human_total_time_spent diff --git a/app/models/timelog.rb b/app/models/timelog.rb index b6b4decc64b..eb088b1f582 100644 --- a/app/models/timelog.rb +++ b/app/models/timelog.rb @@ -1,6 +1,10 @@ # 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 @@ -12,6 +16,7 @@ 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 @@ -58,6 +63,13 @@ 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/ci/pipeline_schedules/variables_base_save_service.rb b/app/services/ci/pipeline_schedules/variables_base_save_service.rb new file mode 100644 index 00000000000..fb3ba52b36e --- /dev/null +++ b/app/services/ci/pipeline_schedules/variables_base_save_service.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +module Ci + module PipelineSchedules + class VariablesBaseSaveService + include Gitlab::Utils::StrongMemoize + + def execute + variable.assign_attributes(params) + + return forbidden_to_update_pipeline_schedule unless allowed_to_update_pipeline_schedule? + return forbidden_to_save_variables unless allowed_to_save_variables? + + if variable.save + ServiceResponse.success(payload: variable) + else + ServiceResponse.error(payload: variable, message: variable.errors.full_messages) + end + end + + private + + attr_reader :project, :user, :params, :variable, :pipeline_schedule + + def allowed_to_update_pipeline_schedule? + user.can?(:update_pipeline_schedule, pipeline_schedule) + end + + def forbidden_to_update_pipeline_schedule + # We add the error to the base object too + # because model errors are used in the API responses and the `form_errors` helper. + pipeline_schedule.errors.add(:base, authorize_message) + + ServiceResponse.error(payload: pipeline_schedule, message: [authorize_message], reason: :forbidden) + end + + def allowed_to_save_variables? + user.can?(:set_pipeline_variables, project) + end + + def forbidden_to_save_variables + message = _('The current user is not authorized to set pipeline schedule variables') + + # We add the error to the base object too + # because model errors are used in the API responses and the `form_errors` helper. + pipeline_schedule.errors.add(:base, message) + + ServiceResponse.error(payload: pipeline_schedule, message: [message], reason: :forbidden) + end + end + end +end diff --git a/app/services/ci/pipeline_schedules/variables_create_service.rb b/app/services/ci/pipeline_schedules/variables_create_service.rb new file mode 100644 index 00000000000..7d81b0771dc --- /dev/null +++ b/app/services/ci/pipeline_schedules/variables_create_service.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Ci + module PipelineSchedules + class VariablesCreateService < VariablesBaseSaveService + AUTHORIZE = :update_pipeline_schedule + + def initialize(pipeline_schedule, user, params) + @variable = pipeline_schedule.variables.new + @user = user + @pipeline_schedule = pipeline_schedule + @project = pipeline_schedule.project + @params = params + end + + private + + def authorize_message + _('The current user is not authorized to create the pipeline schedule variables') + end + strong_memoize_attr :authorize_message + end + end +end diff --git a/app/services/ci/pipeline_schedules/variables_update_service.rb b/app/services/ci/pipeline_schedules/variables_update_service.rb new file mode 100644 index 00000000000..f0e3a03c37a --- /dev/null +++ b/app/services/ci/pipeline_schedules/variables_update_service.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Ci + module PipelineSchedules + class VariablesUpdateService < VariablesBaseSaveService + AUTHORIZE = :update_pipeline_schedule + + def initialize(pipeline_schedule_variable, user, params) + @variable = pipeline_schedule_variable + @user = user + @pipeline_schedule = pipeline_schedule_variable.pipeline_schedule + @project = pipeline_schedule.project + @params = params + end + + private + + def authorize_message + _('The current user is not authorized to update the pipeline schedule variables') + end + strong_memoize_attr :authorize_message + end + end +end diff --git a/app/services/personal_access_tokens/rotate_service.rb b/app/services/personal_access_tokens/rotate_service.rb index 32710629caf..13144a04c11 100644 --- a/app/services/personal_access_tokens/rotate_service.rb +++ b/app/services/personal_access_tokens/rotate_service.rb @@ -10,26 +10,18 @@ module PersonalAccessTokens end def execute(params = {}) - return ServiceResponse.error(message: _('token already revoked')) if token.revoked? + return error_response(_('token already revoked')) if token.revoked? response = ServiceResponse.success PersonalAccessToken.transaction do unless token.revoke! - response = ServiceResponse.error(message: _('failed to revoke token')) + response = error_response(_('failed to revoke token')) raise ActiveRecord::Rollback end - 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 + response = create_access_token(params) + raise ActiveRecord::Rollback unless response.success? end response @@ -47,5 +39,29 @@ 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 new file mode 100644 index 00000000000..63d8d2a82cc --- /dev/null +++ b/app/services/project_access_tokens/rotate_service.rb @@ -0,0 +1,58 @@ +# 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 diff --git a/app/views/projects/merge_requests/_nav_btns.html.haml b/app/views/projects/merge_requests/_nav_btns.html.haml index 2c0a8d831e4..4607aebf121 100644 --- a/app/views/projects/merge_requests/_nav_btns.html.haml +++ b/app/views/projects/merge_requests/_nav_btns.html.haml @@ -4,7 +4,7 @@ - if @can_bulk_update = render Pajamas::ButtonComponent.new(type: :submit, button_options: { class: 'gl-mr-3 js-bulk-update-toggle' }) do = _("Bulk edit") -- if merge_project +- if merge_project && can?(@current_user, :create_merge_request_in, @project) = render Pajamas::ButtonComponent.new(href: new_merge_request_path, variant: :confirm) do = _("New merge request") diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 3572c30cdd3..c5640ce260b 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -26,7 +26,7 @@ Doorkeeper.configure do user = User.find_by_login(params[:username]) next unless user - next if user.password_automatically_set? + next if user.password_automatically_set? && !user.password_based_omniauth_user? next if user.two_factor_enabled? || Gitlab::Auth::TwoFactorAuthVerifier.new(user).two_factor_authentication_enforced? Gitlab::Auth.find_with_user_password(params[:username], params[:password], increment_failed_attempts: true) diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 4a1b536fd40..7c5f9ebd6e0 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -17536,7 +17536,7 @@ Relationship between an epic and an issue. | <a id="epicissuetimelogs"></a>`timelogs` | [`TimelogConnection!`](#timelogconnection) | Timelogs on the issue. (see [Connections](#connections)) | | <a id="epicissuetitle"></a>`title` | [`String!`](#string) | Title of the issue. | | <a id="epicissuetitlehtml"></a>`titleHtml` | [`String`](#string) | GitLab Flavored Markdown rendering of `title`. | -| <a id="epicissuetotaltimespent"></a>`totalTimeSpent` | [`Int!`](#int) | Total time reported as spent on the issue. | +| <a id="epicissuetotaltimespent"></a>`totalTimeSpent` | [`Int!`](#int) | Total time (in seconds) reported as spent on the issue. | | <a id="epicissuetype"></a>`type` | [`IssueType`](#issuetype) | Type of the issue. | | <a id="epicissueupdatedat"></a>`updatedAt` | [`Time!`](#time) | Timestamp of when the issue was last updated. | | <a id="epicissueupdatedby"></a>`updatedBy` | [`UserCore`](#usercore) | User that last updated the issue. | @@ -19899,7 +19899,7 @@ Describes an issuable resource link for incident issues. | <a id="issuetimelogs"></a>`timelogs` | [`TimelogConnection!`](#timelogconnection) | Timelogs on the issue. (see [Connections](#connections)) | | <a id="issuetitle"></a>`title` | [`String!`](#string) | Title of the issue. | | <a id="issuetitlehtml"></a>`titleHtml` | [`String`](#string) | GitLab Flavored Markdown rendering of `title`. | -| <a id="issuetotaltimespent"></a>`totalTimeSpent` | [`Int!`](#int) | Total time reported as spent on the issue. | +| <a id="issuetotaltimespent"></a>`totalTimeSpent` | [`Int!`](#int) | Total time (in seconds) reported as spent on the issue. | | <a id="issuetype"></a>`type` | [`IssueType`](#issuetype) | Type of the issue. | | <a id="issueupdatedat"></a>`updatedAt` | [`Time!`](#time) | Timestamp of when the issue was last updated. | | <a id="issueupdatedby"></a>`updatedBy` | [`UserCore`](#usercore) | User that last updated the issue. | @@ -20443,7 +20443,7 @@ Defines which user roles, users, or groups can merge into a protected branch. | <a id="mergerequesttimelogs"></a>`timelogs` | [`TimelogConnection!`](#timelogconnection) | Timelogs on the merge request. (see [Connections](#connections)) | | <a id="mergerequesttitle"></a>`title` | [`String!`](#string) | Title of the merge request. | | <a id="mergerequesttitlehtml"></a>`titleHtml` | [`String`](#string) | GitLab Flavored Markdown rendering of `title`. | -| <a id="mergerequesttotaltimespent"></a>`totalTimeSpent` | [`Int!`](#int) | Total time reported as spent on the merge request. | +| <a id="mergerequesttotaltimespent"></a>`totalTimeSpent` | [`Int!`](#int) | Total time (in seconds) reported as spent on the merge request. | | <a id="mergerequestupdatedat"></a>`updatedAt` | [`Time!`](#time) | Timestamp of when the merge request was last updated. | | <a id="mergerequestupvotes"></a>`upvotes` | [`Int!`](#int) | Number of upvotes for the merge request. | | <a id="mergerequestuserdiscussionscount"></a>`userDiscussionsCount` | [`Int`](#int) | Number of user discussions in the merge request. | diff --git a/doc/user/project/repository/tags/index.md b/doc/user/project/repository/tags/index.md index 765f5539244..1f1b8dfc9cd 100644 --- a/doc/user/project/repository/tags/index.md +++ b/doc/user/project/repository/tags/index.md @@ -97,6 +97,19 @@ To create a tag from the GitLab UI: create a lightweight tag. 1. Select **Create tag**. +## Name your tag + +Git enforces [tag name rules](https://git-scm.com/docs/git-check-ref-format) +to help ensure tag names remain compatible with other tools. GitLab +adds extra requirements for tag names, and provides benefits for well-structured tag names. + +GitLab enforces these additional rules on all tags: + +- No spaces are allowed in tag names. +- Tag names starting with 40 or 64 hexadecimal characters are prohibited, because they are similar to Git commit hashes. +- Tag names cannot start with `-`, `refs/heads`, `refs/tags`, or `refs/remotes` +- Tag names are case-sensitive. + ## Prevent tag deletion **(PREMIUM ALL)** To prevent users from removing a tag with `git push`, create a [push rule](../push_rules.md). diff --git a/doc/user/project/time_tracking.md b/doc/user/project/time_tracking.md index f9a11a51c98..f1e5247efdb 100644 --- a/doc/user/project/time_tracking.md +++ b/doc/user/project/time_tracking.md @@ -69,6 +69,8 @@ As you work, you can log the time you've spent. Every new time spent entry is added to the current total time spent for the issue or the merge request. +The total amount of time spent on an issue or merge request cannot exceed a year. + ### Add time spent Prerequisites: @@ -164,7 +166,7 @@ To view a time tracking report: ![Time tracking report](img/time_tracking_report_v15_1.png) -The breakdown of spent time is limited to a maximum of 100 entries. +The breakdown of spent time displayed is limited to a maximum of 100 entries. ## Available time units diff --git a/lib/api/ci/pipeline_schedules.rb b/lib/api/ci/pipeline_schedules.rb index 1087c734f98..48e9f6d879b 100644 --- a/lib/api/ci/pipeline_schedules.rb +++ b/lib/api/ci/pipeline_schedules.rb @@ -219,11 +219,16 @@ module API documentation: { default: 'env_var' } end post ':id/pipeline_schedules/:pipeline_schedule_id/variables' do + authorize! :set_pipeline_variables, user_project authorize! :update_pipeline_schedule, pipeline_schedule - variable_params = declared_params(include_missing: false) - variable = pipeline_schedule.variables.create(variable_params) - if variable.persisted? + response = ::Ci::PipelineSchedules::VariablesCreateService + .new(pipeline_schedule, current_user, declared_params(include_missing: false)) + .execute + + variable = response.payload + + if response.success? present variable, with: Entities::Ci::Variable else render_validation_error!(variable) @@ -247,9 +252,14 @@ module API documentation: { default: 'env_var' } end put ':id/pipeline_schedules/:pipeline_schedule_id/variables/:key' do + authorize! :set_pipeline_variables, user_project authorize! :update_pipeline_schedule, pipeline_schedule - if pipeline_schedule_variable.update(declared_params(include_missing: false)) + response = ::Ci::PipelineSchedules::VariablesUpdateService + .new(pipeline_schedule_variable, current_user, declared_params(include_missing: false)) + .execute + + if response.success? present pipeline_schedule_variable, with: Entities::Ci::Variable else render_validation_error!(pipeline_schedule_variable) @@ -269,6 +279,7 @@ module API requires :key, type: String, desc: 'The key of the variable', documentation: { example: 'NEW_VARIABLE' } end delete ':id/pipeline_schedules/:pipeline_schedule_id/variables/:key' do + authorize! :set_pipeline_variables, user_project authorize! :admin_pipeline_schedule, pipeline_schedule status :accepted diff --git a/lib/api/resource_access_tokens.rb b/lib/api/resource_access_tokens.rb index 752feb1455f..1e1b5d77cfd 100644 --- a/lib/api/resource_access_tokens.rb +++ b/lib/api/resource_access_tokens.rb @@ -153,7 +153,13 @@ module API token = find_token(resource, params[:token_id]) if resource_accessible if token - response = ::PersonalAccessTokens::RotateService.new(current_user, token).execute(declared_params) + response = if source_type == "project" + ::ProjectAccessTokens::RotateService.new(current_user, token, resource) + .execute(declared_params) + else + ::PersonalAccessTokens::RotateService.new(current_user, token) + .execute(declared_params) + end if response.success? status :ok diff --git a/lib/gitlab/checks/tag_check.rb b/lib/gitlab/checks/tag_check.rb index d5addab74b8..cdc648bf005 100644 --- a/lib/gitlab/checks/tag_check.rb +++ b/lib/gitlab/checks/tag_check.rb @@ -12,6 +12,7 @@ module Gitlab create_protected_tag: 'You are not allowed to create this tag as it is protected.', default_branch_collision: 'You cannot use default branch name to create a tag', prohibited_tag_name: 'You cannot create a tag with a prohibited pattern.', + prohibited_sha_tag_name: 'You cannot create a tag with a SHA-1 or SHA-256 tag name.', prohibited_tag_name_encoding: 'Tag names must be valid when converted to UTF-8 encoding' }.freeze @@ -21,6 +22,8 @@ module Gitlab protected_tag_checks: "Checking if you are creating, updating or deleting a protected tag..." }.freeze + STARTS_WITH_SHA_REGEX = %r{\A#{Gitlab::Git::Commit::RAW_FULL_SHA_PATTERN}}o + def validate! return unless tag_name @@ -57,6 +60,7 @@ module Gitlab end # rubocop: enable Style/SoleNestedConditional # rubocop: enable Style/GuardClause + validate_tag_name_not_sha_like! end def protected_tag_checks @@ -88,6 +92,12 @@ module Gitlab end end end + + def validate_tag_name_not_sha_like! + return unless STARTS_WITH_SHA_REGEX.match?(tag_name) + + raise GitAccess::ForbiddenError, ERROR_MESSAGES[:prohibited_sha_tag_name] + end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 234a9601a4c..5f012568cde 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -32102,6 +32102,9 @@ msgstr "" msgid "Not confidential" msgstr "" +msgid "Not eligible to rotate token with access level higher than the user" +msgstr "" + msgid "Not found" msgstr "" @@ -45523,6 +45526,9 @@ msgstr "" msgid "Smartcard authentication failed: client certificate header is missing." msgstr "" +msgid "Smartcard authentication failed: login process exceeded the time limit." +msgstr "" + msgid "Snippet" msgstr "" @@ -48123,12 +48129,18 @@ msgstr "" msgid "The current user is not authorized to create the pipeline schedule" msgstr "" +msgid "The current user is not authorized to create the pipeline schedule variables" +msgstr "" + msgid "The current user is not authorized to set pipeline schedule variables" msgstr "" msgid "The current user is not authorized to update the pipeline schedule" msgstr "" +msgid "The current user is not authorized to update the pipeline schedule variables" +msgstr "" + msgid "The data in this pipeline is too old to be rendered as a graph. Please check the Jobs tab to access historical data." msgstr "" @@ -50568,6 +50580,12 @@ msgstr "" msgid "Total test time for all commits/merges" msgstr "" +msgid "Total time spent cannot be negative." +msgstr "" + +msgid "Total time spent cannot exceed a year." +msgstr "" + msgid "Total users" msgstr "" diff --git a/spec/frontend/repository/components/table/row_spec.js b/spec/frontend/repository/components/table/row_spec.js index 80471d8734b..9bff801dbf0 100644 --- a/spec/frontend/repository/components/table/row_spec.js +++ b/spec/frontend/repository/components/table/row_spec.js @@ -146,10 +146,11 @@ describe('Repository table row component', () => { }); it.each` - path - ${'test#'} - ${'Änderungen'} - `('renders link for $path', ({ path }) => { + path | encodedPath + ${'test#'} | ${'test%23'} + ${'Änderungen'} | ${'%C3%84nderungen'} + ${'dir%2f_hello__.sh'} | ${'dir%252f_hello__.sh'} + `('renders link for $path', ({ path, encodedPath }) => { factory({ propsData: { id: '1', @@ -161,7 +162,7 @@ describe('Repository table row component', () => { }); expect(wrapper.findComponent({ ref: 'link' }).props('to')).toBe( - `/-/tree/main/${encodeURIComponent(path)}?ref_type=heads`, + `/-/tree/main/${encodedPath}?ref_type=heads`, ); }); diff --git a/spec/lib/gitlab/checks/tag_check_spec.rb b/spec/lib/gitlab/checks/tag_check_spec.rb index b5aafde006f..2b1fbc7e797 100644 --- a/spec/lib/gitlab/checks/tag_check_spec.rb +++ b/spec/lib/gitlab/checks/tag_check_spec.rb @@ -57,6 +57,7 @@ RSpec.describe Gitlab::Checks::TagCheck, feature_category: :source_code_manageme context "when prohibited_tag_name_encoding_check feature flag is disabled" do before do stub_feature_flags(prohibited_tag_name_encoding_check: false) + allow(subject).to receive(:validate_tag_name_not_sha_like!) end it "doesn't prohibit tag names that include characters incompatible with UTF-8" do @@ -71,6 +72,66 @@ RSpec.describe Gitlab::Checks::TagCheck, feature_category: :source_code_manageme expect { subject.validate! }.not_to raise_error end end + + it "forbids SHA-1 values" do + allow(subject) + .to receive(:tag_name) + .and_return("267208abfe40e546f5e847444276f7d43a39503e") + + expect { subject.validate! }.to raise_error( + Gitlab::GitAccess::ForbiddenError, + "You cannot create a tag with a SHA-1 or SHA-256 tag name." + ) + end + + it "forbids SHA-256 values" do + allow(subject) + .to receive(:tag_name) + .and_return("09b9fd3ea68e9b95a51b693a29568c898e27d1476bbd83c825664f18467fc175") + + expect { subject.validate! }.to raise_error( + Gitlab::GitAccess::ForbiddenError, + "You cannot create a tag with a SHA-1 or SHA-256 tag name." + ) + end + + it "forbids '{SHA-1}{+anything}' values" do + allow(subject) + .to receive(:tag_name) + .and_return("267208abfe40e546f5e847444276f7d43a39503e-") + + expect { subject.validate! }.to raise_error( + Gitlab::GitAccess::ForbiddenError, + "You cannot create a tag with a SHA-1 or SHA-256 tag name." + ) + end + + it "forbids '{SHA-256}{+anything} values" do + allow(subject) + .to receive(:tag_name) + .and_return("09b9fd3ea68e9b95a51b693a29568c898e27d1476bbd83c825664f18467fc175-") + + expect { subject.validate! }.to raise_error( + Gitlab::GitAccess::ForbiddenError, + "You cannot create a tag with a SHA-1 or SHA-256 tag name." + ) + end + + it "allows SHA-1 values to be appended to the tag name" do + allow(subject) + .to receive(:tag_name) + .and_return("fix-267208abfe40e546f5e847444276f7d43a39503e") + + expect { subject.validate! }.not_to raise_error + end + + it "allows SHA-256 values to be appended to the tag name" do + allow(subject) + .to receive(:tag_name) + .and_return("fix-09b9fd3ea68e9b95a51b693a29568c898e27d1476bbd83c825664f18467fc175") + + expect { subject.validate! }.not_to raise_error + end end context 'with protected tag' do diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 6c8603d7b4c..594492a160d 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -1036,6 +1036,11 @@ RSpec.describe Issue, feature_category: :team_planning do end end + it_behaves_like 'a time trackable' do + let(:trackable) { create(:issue) } + let(:timelog) { create(:issue_timelog, issue: trackable) } + end + it_behaves_like 'an editable mentionable' do subject { create(:issue, project: create(:project, :repository)) } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index d3c32da2842..1c6a29f065f 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -2058,6 +2058,11 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev end end + it_behaves_like 'a time trackable' do + let(:trackable) { create(:merge_request, :simple, source_project: create(:project, :repository)) } + let(:timelog) { create(:merge_request_timelog, merge_request: trackable) } + end + it_behaves_like 'an editable mentionable' do subject { create(:merge_request, :simple, source_project: create(:project, :repository)) } diff --git a/spec/models/timelog_spec.rb b/spec/models/timelog_spec.rb index 4f2f16875b8..aee2c4ded19 100644 --- a/spec/models/timelog_spec.rb +++ b/spec/models/timelog_spec.rb @@ -46,6 +46,53 @@ RSpec.describe Timelog, feature_category: :team_planning do expect(subject).to be_valid end + describe 'check if total time spent would be within the set range' do + let_it_be(:time_already_spent) { 1.minute.to_i } + + before_all do + create(:issue_timelog, issue: issue, time_spent: time_already_spent) + end + + it 'is valid when a negative time spent offsets the time already spent' do + timelog = build(:issue_timelog, issue: issue, time_spent: -time_already_spent) + + expect(timelog).to be_valid + end + + context 'when total time spent is within the allowed range' do + before_all do + # Offset the time already spent + create(:issue_timelog, issue: issue, time_spent: -time_already_spent) + end + + it 'is valid' do + timelog = build(:issue_timelog, issue: issue, time_spent: 1.minute.to_i) + + expect(timelog).to be_valid + end + end + + context 'when total time spent is outside the allowed range' do + it 'adds an error if total time spent would exceed a year' do + time_to_spend = described_class::MAX_TOTAL_TIME_SPENT - time_already_spent + 1.second.to_i + timelog = build(:issue_timelog, issue: issue, time_spent: time_to_spend) + + expect { timelog.save! } + .to raise_error(ActiveRecord::RecordInvalid, + _('Validation failed: Total time spent cannot exceed a year.')) + end + + it 'adds an error if total time spent would be negative' do + time_to_spend = -time_already_spent - 1.second.to_i + timelog = build(:issue_timelog, issue: issue, time_spent: time_to_spend) + + expect { timelog.save! } + .to raise_error(ActiveRecord::RecordInvalid, + _('Validation failed: Total time spent cannot be negative.')) + end + end + end + describe 'when importing' do it 'is valid if issue_id and merge_request_id are missing' do subject.attributes = { issue: nil, merge_request: nil, importing: true } diff --git a/spec/requests/api/ci/pipeline_schedules_spec.rb b/spec/requests/api/ci/pipeline_schedules_spec.rb index fb67d7cb4fb..a4bb379d01c 100644 --- a/spec/requests/api/ci/pipeline_schedules_spec.rb +++ b/spec/requests/api/ci/pipeline_schedules_spec.rb @@ -628,17 +628,89 @@ RSpec.describe API::Ci::PipelineSchedules, feature_category: :continuous_integra context 'authenticated user with valid permissions' do context 'with required parameters' do - it 'creates pipeline_schedule_variable' do - expect do - post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables", developer), + let(:pipeline_schedule) do + create(:ci_pipeline_schedule, project: project, owner: api_user) + end + + let_it_be(:maintainer) { create(:user) } + let_it_be(:project_owner) { create(:user) } + + before do + project.add_maintainer(maintainer) + project.add_owner(project_owner) + end + + shared_examples 'creates pipeline_schedule_variables' do + it do + expect do + post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables", api_user), + params: params.merge(variable_type: 'file') + end.to change { pipeline_schedule.variables.count }.by(1) + + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('pipeline_schedule_variable') + expect(json_response['key']).to eq(params[:key]) + expect(json_response['value']).to eq(params[:value]) + expect(json_response['variable_type']).to eq('file') + end + end + + shared_examples 'fails to create pipeline_schedule_variables' do + it do + post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables", api_user), params: params.merge(variable_type: 'file') - end.to change { pipeline_schedule.variables.count }.by(1) - expect(response).to have_gitlab_http_status(:created) - expect(response).to match_response_schema('pipeline_schedule_variable') - expect(json_response['key']).to eq(params[:key]) - expect(json_response['value']).to eq(params[:value]) - expect(json_response['variable_type']).to eq('file') + expect(pipeline_schedule.variables.count).to eq(0) + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when project restricts use of user defined variables' do + before do + project.update!(restrict_user_defined_variables: true) + end + + context 'as developer' do + let(:api_user) { developer } + + it_behaves_like 'fails to create pipeline_schedule_variables' + end + + context 'as maintainer' do + let(:api_user) { maintainer } + + it_behaves_like 'creates pipeline_schedule_variables' + end + + context 'as owner' do + let(:api_user) { project_owner } + + it_behaves_like 'creates pipeline_schedule_variables' + end + end + + context 'when project does not restrict use of user defined variables' do + before do + project.update!(restrict_user_defined_variables: false) + end + + context 'as developer' do + let(:api_user) { developer } + + it_behaves_like 'creates pipeline_schedule_variables' + end + + context 'as maintainer' do + let(:api_user) { maintainer } + + it_behaves_like 'creates pipeline_schedule_variables' + end + + context 'as owner' do + let(:api_user) { project_owner } + + it_behaves_like 'creates pipeline_schedule_variables' + end end end @@ -688,14 +760,85 @@ RSpec.describe API::Ci::PipelineSchedules, feature_category: :continuous_integra end context 'authenticated user with valid permissions' do - it 'updates pipeline_schedule_variable' do - put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}", developer), - params: { value: 'updated_value', variable_type: 'file' } + let(:pipeline_schedule) do + create(:ci_pipeline_schedule, project: project, owner: api_user) + end - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('pipeline_schedule_variable') - expect(json_response['value']).to eq('updated_value') - expect(json_response['variable_type']).to eq('file') + let_it_be(:maintainer) { create(:user) } + let_it_be(:project_owner) { create(:user) } + + before do + project.add_maintainer(maintainer) + project.add_owner(project_owner) + end + + shared_examples 'updates pipeline_schedule_variable' do + it do + put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}", api_user), + params: { value: 'updated_value', variable_type: 'file' } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('pipeline_schedule_variable') + expect(json_response['value']).to eq('updated_value') + expect(json_response['variable_type']).to eq('file') + end + end + + shared_examples 'fails to update pipeline_schedule_variable' do + it do + put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}", api_user), + params: { value: 'updated_value', variable_type: 'file' } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when project restricts use of user defined variables' do + before do + project.update!(restrict_user_defined_variables: true) + end + + context 'as developer' do + let(:api_user) { developer } + + it_behaves_like 'fails to update pipeline_schedule_variable' + end + + context 'as maintainer' do + let(:api_user) { maintainer } + + it_behaves_like 'updates pipeline_schedule_variable' + end + + context 'as owner' do + let(:api_user) { project_owner } + + it_behaves_like 'updates pipeline_schedule_variable' + end + end + + context 'when project does not restrict use of user defined variables' do + before do + project.update!(restrict_user_defined_variables: false) + end + + context 'as developer' do + let(:api_user) { developer } + + it_behaves_like 'updates pipeline_schedule_variable' + end + + context 'as maintainer' do + let(:api_user) { maintainer } + + it_behaves_like 'updates pipeline_schedule_variable' + end + + context 'as owner' do + let(:api_user) { project_owner } + + it_behaves_like 'updates pipeline_schedule_variable' + end end end @@ -732,19 +875,93 @@ RSpec.describe API::Ci::PipelineSchedules, feature_category: :continuous_integra end context 'authenticated user with valid permissions' do - it 'deletes pipeline_schedule_variable' do - expect do - delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}", maintainer) - end.to change { Ci::PipelineScheduleVariable.count }.by(-1) + let(:pipeline_schedule) do + create(:ci_pipeline_schedule, project: project, owner: api_user) + end - expect(response).to have_gitlab_http_status(:accepted) - expect(response).to match_response_schema('pipeline_schedule_variable') + let_it_be(:project_owner) { create(:user) } + + before do + project.add_owner(project_owner) end - it 'responds with 404 Not Found if requesting non-existing pipeline_schedule_variable' do - delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/____", maintainer) + shared_examples 'deletes pipeline_schedule_variable' do + it do + expect do + delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}", api_user) + end.to change { Ci::PipelineScheduleVariable.count }.by(-1) - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to have_gitlab_http_status(:accepted) + expect(response).to match_response_schema('pipeline_schedule_variable') + end + end + + shared_examples 'fails to delete pipeline_schedule_variable' do + it do + expect do + delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}", api_user) + end.not_to change { Ci::PipelineScheduleVariable.count } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when project restricts use of user defined variables' do + before do + project.update!(restrict_user_defined_variables: true) + end + + context 'as developer' do + let(:api_user) { developer } + + it_behaves_like 'fails to delete pipeline_schedule_variable' + end + + context 'as maintainer' do + let(:api_user) { maintainer } + + it_behaves_like 'deletes pipeline_schedule_variable' + end + + context 'as owner' do + let(:api_user) { project_owner } + + it_behaves_like 'deletes pipeline_schedule_variable' + end + end + + context 'when project does not restrict use of user defined variables' do + before do + project.update!(restrict_user_defined_variables: false) + end + + context 'as developer' do + let(:api_user) { developer } + + it_behaves_like 'deletes pipeline_schedule_variable' + end + + context 'as maintainer' do + let(:api_user) { maintainer } + + it_behaves_like 'deletes pipeline_schedule_variable' + end + + context 'as owner' do + let(:api_user) { project_owner } + + it_behaves_like 'deletes pipeline_schedule_variable' + end + end + + context 'as developer' do + let(:api_user) { developer } + + it 'responds with 404 Not Found if requesting non-existing pipeline_schedule_variable' do + delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/____", maintainer) + + expect(response).to have_gitlab_http_status(:not_found) + end end end diff --git a/spec/requests/api/resource_access_tokens_spec.rb b/spec/requests/api/resource_access_tokens_spec.rb index 01e02651a64..f0282b3a675 100644 --- a/spec/requests/api/resource_access_tokens_spec.rb +++ b/spec/requests/api/resource_access_tokens_spec.rb @@ -481,25 +481,75 @@ RSpec.describe API::ResourceAccessTokens, feature_category: :system_access do let(:path) { "/#{source_type}s/#{resource_id}/access_tokens/#{token_id}/rotate" } - before do - resource.add_maintainer(project_bot) - resource.add_owner(user) + subject(:rotate_token) { post(api(path, user), params: params) } + + context 'when user is owner' do + before do + resource.add_maintainer(project_bot) + resource.add_owner(user) + end + + it "allows owner to rotate token", :freeze_time do + rotate_token + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['token']).not_to eq(token.token) + expect(json_response['expires_at']).to eq((Date.today + 1.week).to_s) + end end - subject(:rotate_token) { post(api(path, user), params: params) } + context 'when user is maintainer' do + before do + resource.add_maintainer(user) + end + + context "when token has owner access level" do + let(:error_message) { 'Not eligible to rotate token with access level higher than the user' } - it "allows owner to rotate token", :freeze_time do - rotate_token + before do + resource.add_owner(project_bot) + end - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['token']).not_to eq(token.token) - expect(json_response['expires_at']).to eq((Date.today + 1.week).to_s) + it "raises error" do + rotate_token + + if source_type == 'project' + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq("400 Bad request - #{error_message}") + else + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end + + context 'when token has maintainer access level' do + before do + resource.add_maintainer(project_bot) + end + + it "rotates token", :freeze_time do + rotate_token + + if source_type == 'project' + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['token']).not_to eq(token.token) + expect(json_response['expires_at']).to eq((Date.today + 1.week).to_s) + else + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end end context 'when expiry is defined' do let(:expiry_date) { Date.today + 1.month } let(:params) { { expires_at: expiry_date } } + before do + resource.add_maintainer(project_bot) + resource.add_owner(user) + end + it "allows owner to rotate token", :freeze_time do rotate_token @@ -510,6 +560,11 @@ RSpec.describe API::ResourceAccessTokens, feature_category: :system_access do end context 'without permission' do + before do + resource.add_maintainer(project_bot) + resource.add_owner(user) + end + it 'returns an error message' do another_user = create(:user) resource.add_developer(another_user) @@ -522,10 +577,21 @@ RSpec.describe API::ResourceAccessTokens, feature_category: :system_access do context 'when service raises an error' do let(:error_message) { 'boom!' } + let(:personal_token_service) { PersonalAccessTokens::RotateService } + let(:project_token_service) { ProjectAccessTokens::RotateService } before do - allow_next_instance_of(PersonalAccessTokens::RotateService) do |service| - allow(service).to receive(:execute).and_return(ServiceResponse.error(message: error_message)) + resource.add_maintainer(project_bot) + resource.add_owner(user) + + if source_type == 'project' + allow_next_instance_of(project_token_service) do |service| + allow(service).to receive(:execute).and_return(ServiceResponse.error(message: error_message)) + end + else + allow_next_instance_of(personal_token_service) do |service| + allow(service).to receive(:execute).and_return(ServiceResponse.error(message: error_message)) + end end end diff --git a/spec/requests/oauth/tokens_controller_spec.rb b/spec/requests/oauth/tokens_controller_spec.rb index aaacfce0ce8..b7755a30a78 100644 --- a/spec/requests/oauth/tokens_controller_spec.rb +++ b/spec/requests/oauth/tokens_controller_spec.rb @@ -55,6 +55,33 @@ RSpec.describe Oauth::TokensController, feature_category: :system_access do expect(response).to have_gitlab_http_status(:bad_request) expect(user.reload.failed_attempts).to eq(0) end + + context 'when the user has an identity matching a provider that is not password-based' do + before do + create(:identity, provider: 'google_oauth2', user: user) + end + + it 'fails to authenticate and does not call GitLab::Auth' do + expect(::Gitlab::Auth).not_to receive(:find_with_user_password) + + authenticate(password) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(user.reload.failed_attempts).to eq(0) + end + end + + context 'when the user is a password-based omniauth user' do + before do + create(:identity, provider: 'ldapmain', user: user) + end + + it 'forwards the request to Gitlab::Auth' do + expect(::Gitlab::Auth).to receive(:find_with_user_password) + + authenticate(password) + end + end end end end diff --git a/spec/services/ci/pipeline_schedules/variables_create_service_spec.rb b/spec/services/ci/pipeline_schedules/variables_create_service_spec.rb new file mode 100644 index 00000000000..4f99de093d4 --- /dev/null +++ b/spec/services/ci/pipeline_schedules/variables_create_service_spec.rb @@ -0,0 +1,108 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::PipelineSchedules::VariablesCreateService, feature_category: :continuous_integration do + let_it_be(:reporter) { create(:user) } + let_it_be_with_reload(:user) { create(:user) } + let_it_be_with_reload(:developer) { create(:user) } + let_it_be_with_reload(:project) { create(:project, :public, :repository) } + let_it_be_with_reload(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: user) } + + subject(:service) { described_class.new(pipeline_schedule, user, params) } + + before_all do + project.add_maintainer(user) + project.add_developer(developer) + project.add_reporter(reporter) + end + + describe 'execute' do + context 'when user does not have permission' do + subject(:service) { described_class.new(pipeline_schedule, reporter, {}) } + + it 'returns ServiceResponse.error' do + result = service.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + + error_message = _('The current user is not authorized to create the pipeline schedule variables') + expect(result.message).to match_array([error_message]) + expect(result.payload.errors).to match_array([error_message]) + end + end + + context 'when user limited with permission on a project' do + let(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: developer) } + + subject(:service) { described_class.new(pipeline_schedule, developer, {}) } + + before do + project.update!(restrict_user_defined_variables: true) + end + + it 'returns ServiceResponse.error' do + result = service.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + + error_message = _('The current user is not authorized to set pipeline schedule variables') + expect(result.message).to match_array([error_message]) + expect(result.payload.errors).to match_array([error_message]) + end + end + + context 'when user has permissions' do + let(:params) do + { + key: 'variable1', + value: 'value1', + variable_type: 'file' + } + end + + subject(:service) { described_class.new(pipeline_schedule, user, params) } + + it 'saves variable with passed params' do + result = service.execute + + expect(result.payload).to have_attributes( + key: 'variable1', + value: 'value1', + variable_type: 'file' + ) + end + + it 'returns ServiceResponse.success' do + result = service.execute + + expect(result).to be_a(ServiceResponse) + expect(result.success?).to be(true) + end + end + + context 'when schedule save fails' do + subject(:service) { described_class.new(pipeline_schedule, user, {}) } + + before do + errors = ActiveModel::Errors.new(project) + errors.add(:base, 'An error occurred') + + allow_next_instance_of(Ci::PipelineScheduleVariable) do |instance| + allow(instance).to receive(:save).and_return(false) + allow(instance).to receive(:errors).and_return(errors) + end + end + + it 'returns ServiceResponse.error' do + result = service.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + expect(result.message).to match_array(['An error occurred']) + end + end + end +end diff --git a/spec/services/ci/pipeline_schedules/variables_update_service_spec.rb b/spec/services/ci/pipeline_schedules/variables_update_service_spec.rb new file mode 100644 index 00000000000..b2e1c2c9df7 --- /dev/null +++ b/spec/services/ci/pipeline_schedules/variables_update_service_spec.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::PipelineSchedules::VariablesUpdateService, feature_category: :continuous_integration do + let_it_be(:reporter) { create(:user) } + let_it_be_with_reload(:user) { create(:user) } + let_it_be_with_reload(:developer) { create(:user) } + let_it_be_with_reload(:project) { create(:project, :public, :repository) } + let(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: user) } + let(:pipeline_schedule_variable) { create(:ci_pipeline_schedule_variable, pipeline_schedule: pipeline_schedule) } + + subject(:service) { described_class.new(pipeline_schedule_variable, user, params) } + + before_all do + project.add_maintainer(user) + project.add_developer(developer) + project.add_reporter(reporter) + end + + describe 'execute' do + context 'when user does not have permission' do + subject(:service) { described_class.new(pipeline_schedule_variable, reporter, {}) } + + it 'returns ServiceResponse.error' do + result = service.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + + error_message = _('The current user is not authorized to update the pipeline schedule variables') + expect(result.message).to match_array([error_message]) + expect(result.payload.errors).to match_array([error_message]) + end + end + + context 'when user limited with permission on a project' do + let(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: developer) } + + subject(:service) { described_class.new(pipeline_schedule_variable, developer, {}) } + + before do + project.update!(restrict_user_defined_variables: true) + end + + it 'returns ServiceResponse.error' do + result = service.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + + error_message = _('The current user is not authorized to set pipeline schedule variables') + expect(result.message).to match_array([error_message]) + expect(result.payload.errors).to match_array([error_message]) + end + end + + context 'when user has permissions' do + let(:params) do + { + key: 'variable1', + value: 'value1', + variable_type: 'file' + } + end + + subject(:service) { described_class.new(pipeline_schedule_variable, user, params) } + + it 'saves variable with passed params' do + result = service.execute + + expect(result.payload).to have_attributes( + key: 'variable1', + value: 'value1', + variable_type: 'file' + ) + end + + it 'returns ServiceResponse.success' do + result = service.execute + + expect(result).to be_a(ServiceResponse) + expect(result.success?).to be(true) + end + end + + context 'when schedule save fails' do + subject(:service) { described_class.new(pipeline_schedule_variable, user, {}) } + + before do + allow(pipeline_schedule_variable).to receive(:save).and_return(false) + + errors = ActiveModel::Errors.new(project) + errors.add(:base, 'An error occurred') + allow(pipeline_schedule_variable).to receive(:errors).and_return(errors) + end + + it 'returns ServiceResponse.error' do + result = service.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + expect(result.message).to match_array(['An error occurred']) + end + end + end +end diff --git a/spec/services/personal_access_tokens/rotate_service_spec.rb b/spec/services/personal_access_tokens/rotate_service_spec.rb index 522506870f6..a114f2cd909 100644 --- a/spec/services/personal_access_tokens/rotate_service_spec.rb +++ b/spec/services/personal_access_tokens/rotate_service_spec.rb @@ -8,16 +8,20 @@ RSpec.describe PersonalAccessTokens::RotateService, feature_category: :system_ac subject(:response) { described_class.new(token.user, token).execute } - it "rotates user's own token", :freeze_time do - expect(response).to be_success + shared_examples_for 'rotates token succesfully' do + it "rotates user's own token", :freeze_time do + expect(response).to be_success - new_token = response.payload[:personal_access_token] + new_token = response.payload[:personal_access_token] - expect(new_token.token).not_to eq(token.token) - expect(new_token.expires_at).to eq(Date.today + 1.week) - expect(new_token.user).to eq(token.user) + expect(new_token.token).not_to eq(token.token) + expect(new_token.expires_at).to eq(Date.today + 1.week) + expect(new_token.user).to eq(token.user) + end end + it_behaves_like "rotates token succesfully" + it 'revokes the previous token' do expect { response }.to change { token.reload.revoked? }.from(false).to(true) diff --git a/spec/services/project_access_tokens/rotate_service_spec.rb b/spec/services/project_access_tokens/rotate_service_spec.rb new file mode 100644 index 00000000000..10e29be4979 --- /dev/null +++ b/spec/services/project_access_tokens/rotate_service_spec.rb @@ -0,0 +1,189 @@ +# frozen_string_literal: true + +require 'spec_helper' +RSpec.describe ProjectAccessTokens::RotateService, feature_category: :system_access do + describe '#execute' do + let_it_be(:token, reload: true) { create(:personal_access_token) } + let(:current_user) { create(:user) } + let(:project) { create(:project, group: create(:group)) } + let(:error_message) { 'Not eligible to rotate token with access level higher than the user' } + + subject(:response) { described_class.new(current_user, token, project).execute } + + shared_examples_for 'rotates token succesfully' do + it "rotates user's own token", :freeze_time do + expect(response).to be_success + + new_token = response.payload[:personal_access_token] + + expect(new_token.token).not_to eq(token.token) + expect(new_token.expires_at).to eq(Date.today + 1.week) + expect(new_token.user).to eq(token.user) + end + end + + context 'when user tries to rotate token with different access level' do + before do + project.add_guest(token.user) + end + + context 'when current user is an owner' do + before do + project.add_owner(current_user) + end + + it_behaves_like "rotates token succesfully" + + context 'when creating the new token fails' do + let(:error_message) { 'boom!' } + + before do + allow_next_instance_of(PersonalAccessToken) do |token| + allow(token).to receive_message_chain(:errors, :full_messages, :to_sentence).and_return(error_message) + allow(token).to receive_message_chain(:errors, :clear) + allow(token).to receive_message_chain(:errors, :empty?).and_return(false) + end + end + + it 'returns an error' do + expect(response).to be_error + expect(response.message).to eq(error_message) + end + + it 'reverts the changes' do + expect { response }.not_to change { token.reload.revoked? }.from(false) + end + end + end + + context 'when current user is not an owner' do + context 'when current user is maintainer' do + before do + project.add_maintainer(current_user) + end + + context 'when access level is not owner' do + it_behaves_like "rotates token succesfully" + end + + context 'when access level is owner' do + before do + project.add_owner(token.user) + end + + it "does not rotate token with higher priviledge" do + response + + expect(response).to be_error + expect(response.message).to eq(error_message) + end + end + end + + context 'when current user is not maintainer' do + before do + project.add_developer(current_user) + end + + it 'does not rotate the token' do + response + + expect(response).to be_error + expect(response.message).to eq(error_message) + end + end + end + + context 'when current user is admin' do + let(:current_user) { create(:admin) } + + context 'when admin mode enabled', :enable_admin_mode do + it_behaves_like "rotates token succesfully" + end + + context 'when admin mode not enabled' do + it 'does not rotate the token' do + response + + expect(response).to be_error + expect(response.message).to eq(error_message) + end + end + end + + context 'when nested membership' do + let_it_be(:project_bot) { create(:user, :project_bot) } + let(:token) { create(:personal_access_token, user: project_bot) } + let(:top_level_group) { create(:group) } + let(:sub_group) { create(:group, parent: top_level_group) } + let(:project) { create(:project, group: sub_group) } + + before do + project.add_maintainer(project_bot) + end + + context 'when current user is an owner' do + before do + project.add_owner(current_user) + end + + it_behaves_like "rotates token succesfully" + + context 'when its a bot user' do + let_it_be(:bot_user) { create(:user, :project_bot) } + let_it_be(:bot_user_membership) do + create(:project_member, :developer, user: bot_user, project: create(:project)) + end + + let_it_be(:token, reload: true) { create(:personal_access_token, user: bot_user) } + + it 'updates membership expires at' do + response + + new_token = response.payload[:personal_access_token] + expect(bot_user_membership.reload.expires_at).to eq(new_token.expires_at) + end + end + end + + context 'when current user is not an owner' do + context 'when current user is maintainer' do + before do + project.add_maintainer(current_user) + end + + context 'when access level is not owner' do + it_behaves_like "rotates token succesfully" + end + + context 'when access level is owner' do + before do + project.add_owner(token.user) + end + + it "does not rotate token with higher priviledge" do + response + + expect(response).to be_error + expect(response.message).to eq(error_message) + end + end + end + + context 'when current user is not maintainer' do + before do + project.add_developer(current_user) + end + + it 'does not rotate the token' do + response + + expect(response).to be_error + expect(response.message).to eq(error_message) + end + end + end + end + end + end +end diff --git a/spec/services/system_notes/time_tracking_service_spec.rb b/spec/services/system_notes/time_tracking_service_spec.rb index 3242ae9e533..cf994220946 100644 --- a/spec/services/system_notes/time_tracking_service_spec.rb +++ b/spec/services/system_notes/time_tracking_service_spec.rb @@ -272,7 +272,11 @@ RSpec.describe ::SystemNotes::TimeTrackingService, feature_category: :team_plann context 'when the timelog has a negative time spent value' do let_it_be(:noteable, reload: true) { create(:issue, project: project) } - let(:timelog) { create(:timelog, user: author, issue: noteable, time_spent: -1800, spent_at: '2022-03-30T00:00:00.000Z') } + let!(:existing_timelog) { create(:timelog, user: author, issue: noteable, time_spent: time_spent.to_i) } + + let(:time_spent) { 1800.seconds } + let(:spent_at) { '2022-03-30T00:00:00.000Z' } + let(:timelog) { create(:timelog, user: author, issue: noteable, time_spent: -time_spent.to_i, spent_at: spent_at) } it 'sets the note text' do expect(subject.note).to eq "subtracted 30m of time spent at 2022-03-30" @@ -296,7 +300,11 @@ RSpec.describe ::SystemNotes::TimeTrackingService, feature_category: :team_plann context 'when the timelog has a negative time spent value' do let_it_be(:noteable, reload: true) { create(:issue, project: project) } - let(:timelog) { create(:timelog, user: author, issue: noteable, time_spent: -1800, spent_at: '2022-03-30T00:00:00.000Z') } + let!(:existing_timelog) { create(:timelog, user: author, issue: noteable, time_spent: time_spent.to_i) } + + let(:time_spent) { 1800.seconds } + let(:spent_at) { '2022-03-30T00:00:00.000Z' } + let(:timelog) { create(:timelog, user: author, issue: noteable, time_spent: -time_spent.to_i, spent_at: spent_at) } it 'sets the note text' do expect(subject.note).to eq "deleted -30m of spent time from 2022-03-30" diff --git a/spec/support/shared_examples/models/trackable_shared_examples.rb b/spec/support/shared_examples/models/trackable_shared_examples.rb new file mode 100644 index 00000000000..649a8eb2d6c --- /dev/null +++ b/spec/support/shared_examples/models/trackable_shared_examples.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'a time trackable' do + describe '#total_time_spent' do + context 'when total time spent exceeds the allowed limit' do + let(:time_spent) { Timelog::MAX_TOTAL_TIME_SPENT + 1.second } + + it 'returns the maximum allowed total time spent' do + timelog.update_column(:time_spent, time_spent.to_i) + + expect(trackable.total_time_spent).to eq(Timelog::MAX_TOTAL_TIME_SPENT) + end + + context 'when total time spent is below 0' do + let(:time_spent) { -Timelog::MAX_TOTAL_TIME_SPENT - 1.second } + + it 'returns the minimum allowed total time spent' do + timelog.update_column(:time_spent, time_spent.to_i) + + expect(trackable.total_time_spent).to eq(-Timelog::MAX_TOTAL_TIME_SPENT) + end + end + end + end +end diff --git a/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb b/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb index dec15cb68b3..eddda969ba2 100644 --- a/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb @@ -169,6 +169,8 @@ RSpec.shared_examples 'time tracking endpoints' do |issuable_name| end it "resets spent time for #{issuable_name}" do + issuable.update!(spend_time: { duration: 60, user_id: user.id }) + travel_to(2.minutes.from_now) do expect do post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/reset_spent_time", user) |