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 ------- doc/api/graphql/reference/index.md | 6 +- doc/user/project/repository/tags/index.md | 13 -- doc/user/project/time_tracking.md | 4 +- lib/api/resource_access_tokens.rb | 8 +- lib/gitlab/checks/tag_check.rb | 10 -- locale/gitlab.pot | 9 - .../repository/components/table/row_spec.js | 11 +- spec/lib/gitlab/checks/tag_check_spec.rb | 61 ------- spec/models/issue_spec.rb | 5 - spec/models/merge_request_spec.rb | 5 - spec/models/timelog_spec.rb | 47 ----- spec/requests/api/resource_access_tokens_spec.rb | 88 ++-------- .../personal_access_tokens/rotate_service_spec.rb | 16 +- .../project_access_tokens/rotate_service_spec.rb | 189 --------------------- .../system_notes/time_tracking_service_spec.rb | 12 +- .../models/trackable_shared_examples.rb | 25 --- .../requests/api/time_tracking_shared_examples.rb | 2 - 24 files changed, 46 insertions(+), 591 deletions(-) delete mode 100644 app/services/project_access_tokens/rotate_service.rb delete mode 100644 spec/services/project_access_tokens/rotate_service_spec.rb delete mode 100644 spec/support/shared_examples/models/trackable_shared_examples.rb 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 diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 7c5f9ebd6e0..4a1b536fd40 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. | `timelogs` | [`TimelogConnection!`](#timelogconnection) | Timelogs on the issue. (see [Connections](#connections)) | | `title` | [`String!`](#string) | Title of the issue. | | `titleHtml` | [`String`](#string) | GitLab Flavored Markdown rendering of `title`. | -| `totalTimeSpent` | [`Int!`](#int) | Total time (in seconds) reported as spent on the issue. | +| `totalTimeSpent` | [`Int!`](#int) | Total time reported as spent on the issue. | | `type` | [`IssueType`](#issuetype) | Type of the issue. | | `updatedAt` | [`Time!`](#time) | Timestamp of when the issue was last updated. | | `updatedBy` | [`UserCore`](#usercore) | User that last updated the issue. | @@ -19899,7 +19899,7 @@ Describes an issuable resource link for incident issues. | `timelogs` | [`TimelogConnection!`](#timelogconnection) | Timelogs on the issue. (see [Connections](#connections)) | | `title` | [`String!`](#string) | Title of the issue. | | `titleHtml` | [`String`](#string) | GitLab Flavored Markdown rendering of `title`. | -| `totalTimeSpent` | [`Int!`](#int) | Total time (in seconds) reported as spent on the issue. | +| `totalTimeSpent` | [`Int!`](#int) | Total time reported as spent on the issue. | | `type` | [`IssueType`](#issuetype) | Type of the issue. | | `updatedAt` | [`Time!`](#time) | Timestamp of when the issue was last updated. | | `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. | `timelogs` | [`TimelogConnection!`](#timelogconnection) | Timelogs on the merge request. (see [Connections](#connections)) | | `title` | [`String!`](#string) | Title of the merge request. | | `titleHtml` | [`String`](#string) | GitLab Flavored Markdown rendering of `title`. | -| `totalTimeSpent` | [`Int!`](#int) | Total time (in seconds) reported as spent on the merge request. | +| `totalTimeSpent` | [`Int!`](#int) | Total time reported as spent on the merge request. | | `updatedAt` | [`Time!`](#time) | Timestamp of when the merge request was last updated. | | `upvotes` | [`Int!`](#int) | Number of upvotes for the merge request. | | `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 1f1b8dfc9cd..765f5539244 100644 --- a/doc/user/project/repository/tags/index.md +++ b/doc/user/project/repository/tags/index.md @@ -97,19 +97,6 @@ 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 f1e5247efdb..f9a11a51c98 100644 --- a/doc/user/project/time_tracking.md +++ b/doc/user/project/time_tracking.md @@ -69,8 +69,6 @@ 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: @@ -166,7 +164,7 @@ To view a time tracking report: ![Time tracking report](img/time_tracking_report_v15_1.png) -The breakdown of spent time displayed is limited to a maximum of 100 entries. +The breakdown of spent time is limited to a maximum of 100 entries. ## Available time units diff --git a/lib/api/resource_access_tokens.rb b/lib/api/resource_access_tokens.rb index 1e1b5d77cfd..752feb1455f 100644 --- a/lib/api/resource_access_tokens.rb +++ b/lib/api/resource_access_tokens.rb @@ -153,13 +153,7 @@ module API token = find_token(resource, params[:token_id]) if resource_accessible if token - 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 + response = ::PersonalAccessTokens::RotateService.new(current_user, token).execute(declared_params) if response.success? status :ok diff --git a/lib/gitlab/checks/tag_check.rb b/lib/gitlab/checks/tag_check.rb index cdc648bf005..d5addab74b8 100644 --- a/lib/gitlab/checks/tag_check.rb +++ b/lib/gitlab/checks/tag_check.rb @@ -12,7 +12,6 @@ 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 @@ -22,8 +21,6 @@ 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 @@ -60,7 +57,6 @@ module Gitlab end # rubocop: enable Style/SoleNestedConditional # rubocop: enable Style/GuardClause - validate_tag_name_not_sha_like! end def protected_tag_checks @@ -92,12 +88,6 @@ 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 5f012568cde..fd270cb5b97 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -32102,9 +32102,6 @@ msgstr "" msgid "Not confidential" msgstr "" -msgid "Not eligible to rotate token with access level higher than the user" -msgstr "" - msgid "Not found" msgstr "" @@ -50580,12 +50577,6 @@ 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 9bff801dbf0..80471d8734b 100644 --- a/spec/frontend/repository/components/table/row_spec.js +++ b/spec/frontend/repository/components/table/row_spec.js @@ -146,11 +146,10 @@ describe('Repository table row component', () => { }); it.each` - path | encodedPath - ${'test#'} | ${'test%23'} - ${'Änderungen'} | ${'%C3%84nderungen'} - ${'dir%2f_hello__.sh'} | ${'dir%252f_hello__.sh'} - `('renders link for $path', ({ path, encodedPath }) => { + path + ${'test#'} + ${'Änderungen'} + `('renders link for $path', ({ path }) => { factory({ propsData: { id: '1', @@ -162,7 +161,7 @@ describe('Repository table row component', () => { }); expect(wrapper.findComponent({ ref: 'link' }).props('to')).toBe( - `/-/tree/main/${encodedPath}?ref_type=heads`, + `/-/tree/main/${encodeURIComponent(path)}?ref_type=heads`, ); }); diff --git a/spec/lib/gitlab/checks/tag_check_spec.rb b/spec/lib/gitlab/checks/tag_check_spec.rb index 2b1fbc7e797..b5aafde006f 100644 --- a/spec/lib/gitlab/checks/tag_check_spec.rb +++ b/spec/lib/gitlab/checks/tag_check_spec.rb @@ -57,7 +57,6 @@ 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 @@ -72,66 +71,6 @@ 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 594492a160d..6c8603d7b4c 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -1036,11 +1036,6 @@ 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 1c6a29f065f..d3c32da2842 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -2058,11 +2058,6 @@ 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 aee2c4ded19..4f2f16875b8 100644 --- a/spec/models/timelog_spec.rb +++ b/spec/models/timelog_spec.rb @@ -46,53 +46,6 @@ 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/resource_access_tokens_spec.rb b/spec/requests/api/resource_access_tokens_spec.rb index f0282b3a675..01e02651a64 100644 --- a/spec/requests/api/resource_access_tokens_spec.rb +++ b/spec/requests/api/resource_access_tokens_spec.rb @@ -481,75 +481,25 @@ RSpec.describe API::ResourceAccessTokens, feature_category: :system_access do let(:path) { "/#{source_type}s/#{resource_id}/access_tokens/#{token_id}/rotate" } - 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 + before do + resource.add_maintainer(project_bot) + resource.add_owner(user) end - 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' } - - before do - resource.add_owner(project_bot) - end - - 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 + subject(:rotate_token) { post(api(path, user), params: params) } - it "rotates token", :freeze_time do - rotate_token + it "allows owner to rotate 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 + 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 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 @@ -560,11 +510,6 @@ 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) @@ -577,21 +522,10 @@ 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 - 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 + allow_next_instance_of(PersonalAccessTokens::RotateService) do |service| + allow(service).to receive(:execute).and_return(ServiceResponse.error(message: error_message)) 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 a114f2cd909..522506870f6 100644 --- a/spec/services/personal_access_tokens/rotate_service_spec.rb +++ b/spec/services/personal_access_tokens/rotate_service_spec.rb @@ -8,20 +8,16 @@ RSpec.describe PersonalAccessTokens::RotateService, feature_category: :system_ac subject(:response) { described_class.new(token.user, token).execute } - shared_examples_for 'rotates token succesfully' do - it "rotates user's own token", :freeze_time do - expect(response).to be_success + 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) - end + 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 - 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 deleted file mode 100644 index 10e29be4979..00000000000 --- a/spec/services/project_access_tokens/rotate_service_spec.rb +++ /dev/null @@ -1,189 +0,0 @@ -# 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 cf994220946..3242ae9e533 100644 --- a/spec/services/system_notes/time_tracking_service_spec.rb +++ b/spec/services/system_notes/time_tracking_service_spec.rb @@ -272,11 +272,7 @@ 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!(: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) } + let(:timelog) { create(:timelog, user: author, issue: noteable, time_spent: -1800, spent_at: '2022-03-30T00:00:00.000Z') } it 'sets the note text' do expect(subject.note).to eq "subtracted 30m of time spent at 2022-03-30" @@ -300,11 +296,7 @@ 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!(: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) } + let(:timelog) { create(:timelog, user: author, issue: noteable, time_spent: -1800, spent_at: '2022-03-30T00:00:00.000Z') } 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 deleted file mode 100644 index 649a8eb2d6c..00000000000 --- a/spec/support/shared_examples/models/trackable_shared_examples.rb +++ /dev/null @@ -1,25 +0,0 @@ -# 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 eddda969ba2..dec15cb68b3 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,8 +169,6 @@ 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) -- cgit v1.2.3