Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2023-12-13 12:22:56 +0300
committerGitLab Bot <gitlab-bot@gitlab.com>2023-12-13 12:22:56 +0300
commitd2d66de7163c42532c5a1c3cddebb144658c5242 (patch)
tree5c80bff8c43c76f3c5d1a7a24ae173f0742b5129
parent7ebc422d70a4737a3b5c1b7cf9d0d6e3e47c9890 (diff)
Add latest changes from gitlab-org/security/gitlab@16-6-stable-eev16.6.2
-rw-r--r--CHANGELOG.md17
-rw-r--r--GITALY_SERVER_VERSION2
-rw-r--r--GITLAB_PAGES_VERSION2
-rw-r--r--app/assets/javascripts/repository/components/table/row.vue4
-rw-r--r--app/graphql/types/issue_type.rb2
-rw-r--r--app/graphql/types/merge_request_type.rb2
-rw-r--r--app/models/concerns/time_trackable.rb8
-rw-r--r--app/models/timelog.rb12
-rw-r--r--app/services/personal_access_tokens/rotate_service.rb40
-rw-r--r--app/services/project_access_tokens/rotate_service.rb58
-rw-r--r--doc/api/graphql/reference/index.md6
-rw-r--r--doc/user/project/repository/tags/index.md13
-rw-r--r--doc/user/project/time_tracking.md4
-rw-r--r--lib/api/resource_access_tokens.rb8
-rw-r--r--lib/gitlab/checks/tag_check.rb10
-rw-r--r--locale/gitlab.pot9
-rw-r--r--spec/frontend/repository/components/table/row_spec.js11
-rw-r--r--spec/lib/gitlab/checks/tag_check_spec.rb61
-rw-r--r--spec/models/issue_spec.rb5
-rw-r--r--spec/models/merge_request_spec.rb5
-rw-r--r--spec/models/timelog_spec.rb47
-rw-r--r--spec/requests/api/resource_access_tokens_spec.rb88
-rw-r--r--spec/services/personal_access_tokens/rotate_service_spec.rb16
-rw-r--r--spec/services/project_access_tokens/rotate_service_spec.rb189
-rw-r--r--spec/services/system_notes/time_tracking_service_spec.rb12
-rw-r--r--spec/support/shared_examples/models/trackable_shared_examples.rb25
-rw-r--r--spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb2
27 files changed, 610 insertions, 48 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 516e17ed7d3..deb4a8f38de 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,23 @@
documentation](doc/development/changelog.md) for instructions on adding your own
entry.
+## 16.6.2 (2023-12-13)
+
+### Fixed (1 change)
+
+- [Fix adding confidential child tasks](gitlab-org/security/gitlab@1e67ddffca37e733aee2c3b118c2f9510fc094c0)
+
+### Security (8 changes)
+
+- [Prevent tag names starting with SHA-1 and SHA-256 values](gitlab-org/security/gitlab@d942166b879e8986d9deb45ab1732fa563e715c2) ([merge request](gitlab-org/security/gitlab!3746))
+- [Pass encoded file paths to router](gitlab-org/security/gitlab@1e414bb90ec85c818955bb241934bd43137adb4d) ([merge request](gitlab-org/security/gitlab!3735))
+- [Validate access level of user while rotating token](gitlab-org/security/gitlab@a8267a4facee6ba922897fa2a1f358636d24fb09) ([merge request](gitlab-org/security/gitlab!3750))
+- [Fix large time_spent value causing GraphQL error `Integer out of bounds`](gitlab-org/security/gitlab@6a48b4b8d58daf75a1d1da192b69c790435427bc) ([merge request](gitlab-org/security/gitlab!3745))
+- [Restrict Protected branch access via group to direct members](gitlab-org/security/gitlab@dde50268150c2ef4653c7024eb8357321042ec94) ([merge request](gitlab-org/security/gitlab!3726))
+- [Remove the ability to fork and create MR for auditors](gitlab-org/security/gitlab@c1a532527ee764c32f9c0779fa787a31b6d481f4) ([merge request](gitlab-org/security/gitlab!3738))
+- [Restrict passing variables on the pipeline schedule API](gitlab-org/security/gitlab@0f12c83c9be495e37a04594c678a500643deb410) ([merge request](gitlab-org/security/gitlab!3723))
+- [Smartcard auth: encrypt client cert in params](gitlab-org/security/gitlab@180374e354da080d90c70500aef9e574cc371e9e) ([merge request](gitlab-org/security/gitlab!3729))
+
## 16.6.1 (2023-11-30)
### Fixed (3 changes)
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION
index 235d052b5f0..ef1c2f66ff1 100644
--- a/GITALY_SERVER_VERSION
+++ b/GITALY_SERVER_VERSION
@@ -1 +1 @@
-16.6.1 \ No newline at end of file
+16.6.2 \ No newline at end of file
diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION
index 235d052b5f0..ef1c2f66ff1 100644
--- a/GITLAB_PAGES_VERSION
+++ b/GITLAB_PAGES_VERSION
@@ -1 +1 @@
-16.6.1 \ No newline at end of file
+16.6.2 \ No newline at end of file
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/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/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/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 fd270cb5b97..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 ""
@@ -50577,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/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/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)