diff options
61 files changed, 948 insertions, 823 deletions
diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS index 6cb5a9d5fad..c12f9e6aac6 100644 --- a/.gitlab/CODEOWNERS +++ b/.gitlab/CODEOWNERS @@ -347,7 +347,7 @@ lib/gitlab/checks/** @proglottis @toon @zj-gitlab /data/removals/ @gl-docsteam ^[Documentation Pages] -/doc/administration/application_settings_cache.md @sselhorn +/doc/administration/application_settings_cache.md @jglassman1 /doc/administration/audit_event_streaming.md @eread /doc/administration/audit_events.md @eread /doc/administration/audit_reports.md @eread @@ -396,11 +396,11 @@ lib/gitlab/checks/** @proglottis @toon @zj-gitlab /doc/administration/merge_request_diffs.md @ashrafkhamis /doc/administration/monitoring/ @msedlakjakubowski /doc/administration/monitoring/gitlab_self_monitoring_project/ @msedlakjakubowski -/doc/administration/monitoring/ip_allowlist.md @sselhorn +/doc/administration/monitoring/ip_allowlist.md @jglassman1 /doc/administration/monitoring/performance/ @msedlakjakubowski /doc/administration/monitoring/prometheus/ @msedlakjakubowski /doc/administration/monitoring/prometheus/index.md @axil -/doc/administration/monitoring/prometheus/web_exporter.md @sselhorn +/doc/administration/monitoring/prometheus/web_exporter.md @jglassman1 /doc/administration/nfs.md @axil /doc/administration/object_storage.md @axil /doc/administration/operations/ @axil @@ -426,7 +426,7 @@ lib/gitlab/checks/** @proglottis @toon @zj-gitlab /doc/administration/restart_gitlab.md @axil /doc/administration/server_hooks.md @eread /doc/administration/sidekiq/ @axil -/doc/administration/sidekiq/sidekiq_memory_killer.md @sselhorn +/doc/administration/sidekiq/sidekiq_memory_killer.md @jglassman1 /doc/administration/smime_signing_email.md @axil /doc/administration/snippets/ @ashrafkhamis /doc/administration/static_objects_external_storage.md @ashrafkhamis @@ -654,7 +654,7 @@ lib/gitlab/checks/** @proglottis @toon @zj-gitlab /doc/development/backend/create_source_code_be/ @aqualls /doc/development/build_test_package.md @axil /doc/development/bulk_import.md @eread -/doc/development/cached_queries.md @sselhorn +/doc/development/cached_queries.md @jglassman1 /doc/development/cascading_settings.md @jglassman1 /doc/development/chatops_on_gitlabcom.md @phillipwells /doc/development/cicd/ @marcel.amirault @@ -662,7 +662,7 @@ lib/gitlab/checks/** @proglottis @toon @zj-gitlab /doc/development/contributing/ @sselhorn /doc/development/database/ @aqualls /doc/development/database/filtering_by_label.md @msedlakjakubowski -/doc/development/database/multiple_databases.md @sselhorn +/doc/development/database/multiple_databases.md @jglassman1 /doc/development/database_review.md @aqualls /doc/development/developing_with_solargraph.md @aqualls /doc/development/development_processes.md @sselhorn @@ -694,7 +694,7 @@ lib/gitlab/checks/** @proglottis @toon @zj-gitlab /doc/development/graphql_guide/ @ashrafkhamis /doc/development/graphql_guide/batchloader.md @aqualls /doc/development/i18n/ @eread -/doc/development/image_scaling.md @sselhorn +/doc/development/image_scaling.md @jglassman1 /doc/development/import_export.md @eread /doc/development/index.md @sselhorn /doc/development/integrations/codesandbox.md @sselhorn diff --git a/.rubocop.yml b/.rubocop.yml index b84effd5b16..f9d8ed35ee2 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -384,6 +384,12 @@ Database/MultipleDatabases: - 'spec/lib/gitlab/background_migration/**/*.rb' - 'spec/lib/gitlab/database/**/*.rb' +# See https://gitlab.com/gitlab-org/gitlab/-/issues/373194 +Gitlab/RSpec/AvoidSetup: + Enabled: true + Include: + - 'ee/spec/features/registrations/saas/**/*' + Gitlab/DuplicateSpecLocation: Enabled: true diff --git a/.rubocop_todo/gitlab/rspec/avoid_setup.yml b/.rubocop_todo/gitlab/rspec/avoid_setup.yml new file mode 100644 index 00000000000..5f75cda7a21 --- /dev/null +++ b/.rubocop_todo/gitlab/rspec/avoid_setup.yml @@ -0,0 +1,3 @@ +--- +Gitlab/RSpec/AvoidSetup: + Details: grace period diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 898b65958a6..3f3cedde315 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -faa097a0d42a9338445700a52aac25ba5887fd85 +1751c8d22d795af285d2d850fe8fa71dc6dbc80d diff --git a/app/assets/javascripts/vue_shared/components/source_viewer/components/chunk.vue b/app/assets/javascripts/vue_shared/components/source_viewer/components/chunk.vue index 9683288f937..9a44df43c4d 100644 --- a/app/assets/javascripts/vue_shared/components/source_viewer/components/chunk.vue +++ b/app/assets/javascripts/vue_shared/components/source_viewer/components/chunk.vue @@ -67,42 +67,35 @@ export default { this.$emit('appear', this.chunkIndex); } }, + calculateLineNumber(index) { + return this.startingFrom + index + 1; + }, }, }; </script> <template> - <div> - <gl-intersection-observer @appear="handleChunkAppear"> - <div v-if="isHighlighted"> - <chunk-line - v-for="(line, index) in lines" + <gl-intersection-observer @appear="handleChunkAppear"> + <div v-if="isHighlighted"> + <chunk-line + v-for="(line, index) in lines" + :key="index" + :number="calculateLineNumber(index)" + :content="line" + :language="language" + :blame-path="blamePath" + /> + </div> + <div v-else class="gl-display-flex gl-text-transparent"> + <div class="gl-display-flex gl-flex-direction-column content-visibility-auto"> + <span + v-for="(n, index) in totalLines" + :id="`L${calculateLineNumber(index)}`" :key="index" - :number="startingFrom + index + 1" - :content="line" - :language="language" - :blame-path="blamePath" - /> - </div> - <div v-else class="gl-display-flex"> - <div class="gl-display-flex gl-flex-direction-column"> - <a - v-for="(n, index) in totalLines" - :id="`L${startingFrom + index + 1}`" - :key="index" - class="gl-ml-5 gl-text-transparent" - :href="`#L${startingFrom + index + 1}`" - :data-line-number="startingFrom + index + 1" - data-testid="line-number" - > - {{ startingFrom + index + 1 }} - </a> - </div> - <div - class="gl-white-space-pre-wrap! gl-text-transparent" - data-testid="content" - v-text="content" - ></div> + data-testid="line-number" + v-text="calculateLineNumber(index)" + ></span> </div> - </gl-intersection-observer> - </div> + <div class="gl-white-space-pre-wrap!" data-testid="content" v-text="content"></div> + </div> + </gl-intersection-observer> </template> diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index 07516275e58..b28302f29ef 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -290,6 +290,10 @@ padding: $gl-padding; } } + + .content-visibility-auto { + content-visibility: auto; + } } } diff --git a/app/graphql/types/branch_protections/merge_access_level_type.rb b/app/graphql/types/branch_protections/merge_access_level_type.rb index 85295e1ba25..e8fcd57ba80 100644 --- a/app/graphql/types/branch_protections/merge_access_level_type.rb +++ b/app/graphql/types/branch_protections/merge_access_level_type.rb @@ -4,7 +4,7 @@ module Types module BranchProtections class MergeAccessLevelType < BaseAccessLevelType # rubocop:disable Graphql/AuthorizeTypes graphql_name 'MergeAccessLevel' - description 'Represents the merge access level of a branch protection.' + description 'Defines which user roles, users, or groups can merge into a protected branch.' accepts ::ProtectedBranch::MergeAccessLevel end end diff --git a/app/graphql/types/branch_protections/push_access_level_type.rb b/app/graphql/types/branch_protections/push_access_level_type.rb index bfbdc4edbea..c5e21fad88d 100644 --- a/app/graphql/types/branch_protections/push_access_level_type.rb +++ b/app/graphql/types/branch_protections/push_access_level_type.rb @@ -4,7 +4,7 @@ module Types module BranchProtections class PushAccessLevelType < BaseAccessLevelType # rubocop:disable Graphql/AuthorizeTypes graphql_name 'PushAccessLevel' - description 'Represents the push access level of a branch protection.' + description 'Defines which user roles, users, or groups can push to a protected branch.' accepts ::ProtectedBranch::PushAccessLevel end end diff --git a/app/models/integrations/assembla.rb b/app/models/integrations/assembla.rb index 88dbf2915ef..a8f6e00b32c 100644 --- a/app/models/integrations/assembla.rb +++ b/app/models/integrations/assembla.rb @@ -12,6 +12,7 @@ module Integrations required: true field :subdomain, + exposes_secrets: true, placeholder: '' def title diff --git a/app/models/integrations/bamboo.rb b/app/models/integrations/bamboo.rb index c3a4b84bb2d..b4e97f0871e 100644 --- a/app/models/integrations/bamboo.rb +++ b/app/models/integrations/bamboo.rb @@ -9,6 +9,7 @@ module Integrations title: -> { s_('BambooService|Bamboo URL') }, placeholder: -> { s_('https://bamboo.example.com') }, help: -> { s_('BambooService|Bamboo service root URL.') }, + exposes_secrets: true, required: true field :build_key, @@ -37,14 +38,6 @@ module Integrations attr_accessor :response - before_validation :reset_password - - def reset_password - if bamboo_url_changed? && !password_touched? - self.password = nil - end - end - def title s_('BambooService|Atlassian Bamboo') end diff --git a/app/models/integrations/buildkite.rb b/app/models/integrations/buildkite.rb index f2d2aca3ffe..2f2fba314d5 100644 --- a/app/models/integrations/buildkite.rb +++ b/app/models/integrations/buildkite.rb @@ -13,6 +13,7 @@ module Integrations field :project_url, title: -> { _('Pipeline URL') }, placeholder: "#{ENDPOINT}/example-org/test-pipeline", + exposes_secrets: true, required: true field :token, diff --git a/app/models/integrations/jenkins.rb b/app/models/integrations/jenkins.rb index 74a6449f4f9..9755f077193 100644 --- a/app/models/integrations/jenkins.rb +++ b/app/models/integrations/jenkins.rb @@ -9,6 +9,7 @@ module Integrations field :jenkins_url, title: -> { s_('ProjectService|Jenkins server URL') }, + exposes_secrets: true, required: true, placeholder: 'http://jenkins.example.com', help: -> { s_('The URL of the Jenkins server.') } @@ -27,22 +28,14 @@ module Integrations non_empty_password_title: -> { s_('ProjectService|Enter new password.') }, non_empty_password_help: -> { s_('ProjectService|Leave blank to use your current password.') } - before_validation :reset_password - validates :jenkins_url, presence: true, addressable_url: true, if: :activated? validates :project_name, presence: true, if: :activated? validates :username, presence: true, if: ->(service) { service.activated? && service.password_touched? && service.password.present? } + validates :password, presence: true, if: ->(service) { service.activated? && service.username.present? } default_value_for :merge_requests_events, false default_value_for :tag_push_events, false - def reset_password - # don't reset the password if a new one is provided - if (jenkins_url_changed? || username.blank?) && !password_touched? - self.password = nil - end - end - def execute(data) return unless supported_events.include?(data[:object_kind]) diff --git a/app/models/integrations/teamcity.rb b/app/models/integrations/teamcity.rb index ca7a715f4b3..af629d6ef1e 100644 --- a/app/models/integrations/teamcity.rb +++ b/app/models/integrations/teamcity.rb @@ -11,6 +11,7 @@ module Integrations field :teamcity_url, title: -> { s_('ProjectService|TeamCity server URL') }, placeholder: 'https://teamcity.example.com', + exposes_secrets: true, required: true field :build_type, @@ -36,8 +37,6 @@ module Integrations attr_accessor :response - before_validation :reset_password - class << self def to_param 'teamcity' @@ -48,12 +47,6 @@ module Integrations end end - def reset_password - if teamcity_url_changed? && !password_touched? - self.password = nil - end - end - def title 'JetBrains TeamCity' end diff --git a/app/models/note.rb b/app/models/note.rb index e444111119b..e448b4e2d17 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -361,14 +361,6 @@ class Note < ApplicationRecord super(noteable_type.to_s.classify.constantize.base_class.to_s) end - def noteable_assignee_or_author?(user) - return false unless user - return false unless noteable.respond_to?(:author_id) - return noteable.assignee_or_author?(user) if [MergeRequest, Issue].include?(noteable.class) - - noteable.author_id == user.id - end - def contributor? project&.team&.contributor?(self.author_id) end diff --git a/app/models/oauth_access_token.rb b/app/models/oauth_access_token.rb index eac99e8d441..8e79a750793 100644 --- a/app/models/oauth_access_token.rb +++ b/app/models/oauth_access_token.rb @@ -31,8 +31,6 @@ class OauthAccessToken < Doorkeeper::AccessToken # have `reuse_access_tokens` disabled and we also hash tokens. # This ensures we don't accidentally return a hashed token value. def self.matching_token_for(application, resource_owner, scopes) - return if Feature.enabled?(:hash_oauth_tokens) - - super + # no-op end end diff --git a/app/policies/issuable_policy.rb b/app/policies/issuable_policy.rb index df065b24e64..aa07bb7dc5f 100644 --- a/app/policies/issuable_policy.rb +++ b/app/policies/issuable_policy.rb @@ -56,7 +56,7 @@ class IssuablePolicy < BasePolicy end # This rule replicates permissions in NotePolicy#can_read_confidential - rule { can?(:reporter_access) | assignee_or_author | admin }.policy do + rule { can?(:reporter_access) | admin }.policy do enable :read_internal_note end end diff --git a/app/policies/note_policy.rb b/app/policies/note_policy.rb index dbfc63a0069..67b57595beb 100644 --- a/app/policies/note_policy.rb +++ b/app/policies/note_policy.rb @@ -23,7 +23,7 @@ class NotePolicy < BasePolicy # Should be matched with IssuablePolicy#read_internal_note # and EpicPolicy#read_internal_note condition(:can_read_confidential) do - access_level >= Gitlab::Access::REPORTER || @subject.noteable_assignee_or_author?(@user) || admin? + access_level >= Gitlab::Access::REPORTER || admin? end rule { ~editable }.prevent :admin_note diff --git a/config/feature_flags/development/hash_oauth_tokens.yml b/config/feature_flags/development/hash_oauth_tokens.yml deleted file mode 100644 index 96bd4a3702e..00000000000 --- a/config/feature_flags/development/hash_oauth_tokens.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: hash_oauth_tokens -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/91501 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/367570 -milestone: '15.3' -type: development -group: group::authentication and authorization -default_enabled: true diff --git a/config/feature_flags/development/saml_group_sync_retain_default_membership.yml b/config/feature_flags/development/saml_group_sync_retain_default_membership.yml deleted file mode 100644 index dbaaf681fb9..00000000000 --- a/config/feature_flags/development/saml_group_sync_retain_default_membership.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: saml_group_sync_retain_default_membership -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/88064 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/364144 -milestone: '15.1' -type: development -group: group::authentication and authorization -default_enabled: false diff --git a/config/open_api.yml b/config/open_api.yml index 8415a6bff3d..75887e6bdc8 100644 --- a/config/open_api.yml +++ b/config/open_api.yml @@ -16,3 +16,5 @@ metadata: tags: - name: metadata description: Operations related to metadata of the GitLab instance + - name: access_requests + description: Operations related to access requests diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 1a765f0ed43..adea0d05308 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -9437,6 +9437,29 @@ The edge type for [`TreeEntry`](#treeentry). | <a id="treeentryedgecursor"></a>`cursor` | [`String!`](#string) | A cursor for use in pagination. | | <a id="treeentryedgenode"></a>`node` | [`TreeEntry`](#treeentry) | The item at the end of the edge. | +#### `UnprotectAccessLevelConnection` + +The connection type for [`UnprotectAccessLevel`](#unprotectaccesslevel). + +##### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="unprotectaccesslevelconnectionedges"></a>`edges` | [`[UnprotectAccessLevelEdge]`](#unprotectaccessleveledge) | A list of edges. | +| <a id="unprotectaccesslevelconnectionnodes"></a>`nodes` | [`[UnprotectAccessLevel]`](#unprotectaccesslevel) | A list of nodes. | +| <a id="unprotectaccesslevelconnectionpageinfo"></a>`pageInfo` | [`PageInfo!`](#pageinfo) | Information to aid in pagination. | + +#### `UnprotectAccessLevelEdge` + +The edge type for [`UnprotectAccessLevel`](#unprotectaccesslevel). + +##### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="unprotectaccessleveledgecursor"></a>`cursor` | [`String!`](#string) | A cursor for use in pagination. | +| <a id="unprotectaccessleveledgenode"></a>`node` | [`UnprotectAccessLevel`](#unprotectaccesslevel) | The item at the end of the edge. | + #### `UploadRegistryConnection` The connection type for [`UploadRegistry`](#uploadregistry). @@ -10333,6 +10356,7 @@ Branch protection details for a branch rule. | <a id="branchprotectioncodeownerapprovalrequired"></a>`codeOwnerApprovalRequired` | [`Boolean!`](#boolean) | Enforce code owner approvals before allowing a merge. | | <a id="branchprotectionmergeaccesslevels"></a>`mergeAccessLevels` | [`MergeAccessLevelConnection`](#mergeaccesslevelconnection) | Details about who can merge when this branch is the source branch. (see [Connections](#connections)) | | <a id="branchprotectionpushaccesslevels"></a>`pushAccessLevels` | [`PushAccessLevelConnection`](#pushaccesslevelconnection) | Details about who can push when this branch is the source branch. (see [Connections](#connections)) | +| <a id="branchprotectionunprotectaccesslevels"></a>`unprotectAccessLevels` | [`UnprotectAccessLevelConnection`](#unprotectaccesslevelconnection) | Details about who can unprotect this branch. (see [Connections](#connections)) | ### `BranchRule` @@ -14172,7 +14196,7 @@ Maven metadata. ### `MergeAccessLevel` -Represents the merge access level of a branch protection. +Defines which user roles, users, or groups can merge into a protected branch. #### Fields @@ -17665,7 +17689,7 @@ Which group, user or role is allowed to execute deployments to the environment. ### `PushAccessLevel` -Represents the push access level of a branch protection. +Defines which user roles, users, or groups can push to a protected branch. #### Fields @@ -18895,6 +18919,19 @@ Represents a directory. | <a id="treeentrywebpath"></a>`webPath` | [`String`](#string) | Web path for the tree entry (directory). | | <a id="treeentryweburl"></a>`webUrl` | [`String`](#string) | Web URL for the tree entry (directory). | +### `UnprotectAccessLevel` + +Defines which user roles, users, or groups can unprotect a protected branch. + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="unprotectaccesslevelaccesslevel"></a>`accessLevel` | [`Int!`](#int) | GitLab::Access level. | +| <a id="unprotectaccesslevelaccessleveldescription"></a>`accessLevelDescription` | [`String!`](#string) | Human readable representation for this access level. | +| <a id="unprotectaccesslevelgroup"></a>`group` | [`Group`](#group) | Group associated with this access level. | +| <a id="unprotectaccessleveluser"></a>`user` | [`UserCore`](#usercore) | User associated with this access level. | + ### `UploadRegistry` Represents the Geo replication and verification state of an upload. diff --git a/doc/api/openapi/openapi_v2.yaml b/doc/api/openapi/openapi_v2.yaml index 59b21ecd048..a3c1934bf2a 100644 --- a/doc/api/openapi/openapi_v2.yaml +++ b/doc/api/openapi/openapi_v2.yaml @@ -18,7 +18,277 @@ host: gitlab.com tags: - name: metadata description: Operations related to metadata of the GitLab instance +- name: access_requests + description: Operations related to access requests paths: + "/api/v4/groups/{id}/access_requests/{user_id}": + delete: + summary: Denies an access request for the given user. + description: This feature was introduced in GitLab 8.11. + produces: + - application/json + parameters: + - in: path + name: id + description: The ID or URL-encoded path of the group owned by the authenticated + user + type: string + required: true + - in: path + name: user_id + description: The user ID of the access requester + type: integer + format: int32 + required: true + responses: + '204': + description: Denies an access request for the given user. + tags: + - access_requests + operationId: deleteApiV4GroupsIdAccessRequestsUserId + "/api/v4/groups/{id}/access_requests/{user_id}/approve": + put: + summary: Approves an access request for the given user. + description: This feature was introduced in GitLab 8.11. + produces: + - application/json + consumes: + - application/json + parameters: + - in: path + name: id + description: The ID or URL-encoded path of the group owned by the authenticated + user + type: string + required: true + - in: path + name: user_id + description: The user ID of the access requester + type: integer + format: int32 + required: true + - in: formData + name: access_level + description: 'A valid access level (defaults: `30`, the Developer role)' + type: integer + format: int32 + default: 30 + required: false + responses: + '200': + description: successful operation + schema: + "$ref": "#/definitions/API_Entities_AccessRequester" + examples: + successfull_response: + id: 1 + username: raymond_smith + name: Raymond Smith + state: active + created_at: '2012-10-22T14:13:35Z' + access_level: 20 + tags: + - access_requests + operationId: putApiV4GroupsIdAccessRequestsUserIdApprove + "/api/v4/groups/{id}/access_requests": + post: + summary: Requests access for the authenticated user to a group. + description: This feature was introduced in GitLab 8.11. + produces: + - application/json + consumes: + - application/json + parameters: + - in: path + name: id + description: The ID or URL-encoded path of the group owned by the authenticated + user + type: string + required: true + responses: + '200': + description: successful operation + schema: + "$ref": "#/definitions/API_Entities_AccessRequester" + examples: + successfull_response: + id: 1 + username: raymond_smith + name: Raymond Smith + state: active + created_at: '2012-10-22T14:13:35Z' + access_level: 20 + tags: + - access_requests + operationId: postApiV4GroupsIdAccessRequests + get: + summary: Gets a list of access requests for a group. + description: This feature was introduced in GitLab 8.11. + produces: + - application/json + parameters: + - in: path + name: id + description: The ID or URL-encoded path of the group owned by the authenticated + user + type: string + required: true + - in: query + name: page + description: Current page number + type: integer + format: int32 + default: 1 + required: false + - in: query + name: per_page + description: Number of items per page + type: integer + format: int32 + default: 20 + required: false + responses: + '200': + description: Gets a list of access requests for a group. + schema: + "$ref": "#/definitions/API_Entities_AccessRequester" + tags: + - access_requests + operationId: getApiV4GroupsIdAccessRequests + "/api/v4/projects/{id}/access_requests/{user_id}": + delete: + summary: Denies an access request for the given user. + description: This feature was introduced in GitLab 8.11. + produces: + - application/json + parameters: + - in: path + name: id + description: The ID or URL-encoded path of the project owned by the authenticated + user + type: string + required: true + - in: path + name: user_id + description: The user ID of the access requester + type: integer + format: int32 + required: true + responses: + '204': + description: Denies an access request for the given user. + tags: + - access_requests + operationId: deleteApiV4ProjectsIdAccessRequestsUserId + "/api/v4/projects/{id}/access_requests/{user_id}/approve": + put: + summary: Approves an access request for the given user. + description: This feature was introduced in GitLab 8.11. + produces: + - application/json + consumes: + - application/json + parameters: + - in: path + name: id + description: The ID or URL-encoded path of the project owned by the authenticated + user + type: string + required: true + - in: path + name: user_id + description: The user ID of the access requester + type: integer + format: int32 + required: true + - in: formData + name: access_level + description: 'A valid access level (defaults: `30`, the Developer role)' + type: integer + format: int32 + default: 30 + required: false + responses: + '200': + description: successful operation + schema: + "$ref": "#/definitions/API_Entities_AccessRequester" + examples: + successfull_response: + id: 1 + username: raymond_smith + name: Raymond Smith + state: active + created_at: '2012-10-22T14:13:35Z' + access_level: 20 + tags: + - access_requests + operationId: putApiV4ProjectsIdAccessRequestsUserIdApprove + "/api/v4/projects/{id}/access_requests": + post: + summary: Requests access for the authenticated user to a project. + description: This feature was introduced in GitLab 8.11. + produces: + - application/json + consumes: + - application/json + parameters: + - in: path + name: id + description: The ID or URL-encoded path of the project owned by the authenticated + user + type: string + required: true + responses: + '200': + description: successful operation + schema: + "$ref": "#/definitions/API_Entities_AccessRequester" + examples: + successfull_response: + id: 1 + username: raymond_smith + name: Raymond Smith + state: active + created_at: '2012-10-22T14:13:35Z' + access_level: 20 + tags: + - access_requests + operationId: postApiV4ProjectsIdAccessRequests + get: + summary: Gets a list of access requests for a project. + description: This feature was introduced in GitLab 8.11. + produces: + - application/json + parameters: + - in: path + name: id + description: The ID or URL-encoded path of the project owned by the authenticated + user + type: string + required: true + - in: query + name: page + description: Current page number + type: integer + format: int32 + default: 1 + required: false + - in: query + name: per_page + description: Number of items per page + type: integer + format: int32 + default: 20 + required: false + responses: + '200': + description: Gets a list of access requests for a project. + schema: + "$ref": "#/definitions/API_Entities_AccessRequester" + tags: + - access_requests + operationId: getApiV4ProjectsIdAccessRequests "/api/v4/metadata": get: summary: Retrieve metadata information for this GitLab instance. @@ -71,6 +341,39 @@ paths: - metadata operationId: getApiV4Version definitions: + API_Entities_AccessRequester: + type: object + properties: + id: + type: string + username: + type: string + name: + type: string + state: + type: string + avatar_url: + type: string + avatar_path: + type: string + custom_attributes: + "$ref": "#/definitions/API_Entities_CustomAttribute" + web_url: + type: string + is_gitlab_employee: + type: string + email: + type: string + requested_at: + type: string + description: API_Entities_AccessRequester model + API_Entities_CustomAttribute: + type: object + properties: + key: + type: string + value: + type: string API_Entities_Metadata: type: object properties: diff --git a/doc/development/documentation/styleguide/index.md b/doc/development/documentation/styleguide/index.md index 042b49b119e..a35bbce773d 100644 --- a/doc/development/documentation/styleguide/index.md +++ b/doc/development/documentation/styleguide/index.md @@ -23,6 +23,13 @@ In addition to this page, the following resources can help you craft and contrib - [Google Developer Documentation Style Guide](https://developers.google.com/style) - [Recent updates to this guide](https://gitlab.com/dashboard/merge_requests?scope=all&state=merged&label_name[]=tw-style¬[label_name][]=docs%3A%3Afix) +## The GitLab voice + +At GitLab, we strive to be concise, direct, and precise. +Our goal is to provide information that's easy to search and scan. + +We want to be conversational but brief, friendly but succinct. + ## Documentation is the single source of truth (SSOT) The GitLab documentation is the SSOT for all @@ -125,6 +132,22 @@ Maintaining a knowledge base separate from the documentation would be against the documentation-first methodology, because the content would overlap with the documentation. +## Writing for localization + +The GitLab documentation is not localized, but we follow guidelines that +help benefit translation. For example, we: + +- Write in [active voice](word_list.md#active-voice). +- Write in [present tense](word_list.md#future-tense). +- Avoid words that can be translated incorrectly, like: + - [since and because](word_list.md#since) + - [once and after](word_list.md#once) +- Avoid [ing](word_list.md#-ing-words) words. + +[The GitLab voice](#the-gitlab-voice) dictates that we write clearly and directly, +and with translation in mind. [The word list](word_list.md) and our Vale rules +also aid in consistency, which is important for localization. + ## Markdown All GitLab documentation is written using [Markdown](https://en.wikipedia.org/wiki/Markdown). diff --git a/doc/development/documentation/styleguide/word_list.md b/doc/development/documentation/styleguide/word_list.md index ce9fd671a39..ce1ba5f2dc9 100644 --- a/doc/development/documentation/styleguide/word_list.md +++ b/doc/development/documentation/styleguide/word_list.md @@ -64,6 +64,23 @@ When you create a user, you choose an access level: **Regular**, **Auditor**, or Capitalize these words when you refer to the UI. Otherwise use lowercase. +## active voice + +Use active voice instead of passive. + +Use: + +- The contributor writes the documentation. + +Instead of: + +- The documentation is written by contributors. + +NOTE: +If you can add the phrase "by zombies" to the end of the sentence, +the construction is passive. For example, `The button is selected by zombies` +is passive. `Select the button` is active. + ## administrator Use **administrator access** instead of **admin** when talking about a user's access level. @@ -570,6 +587,15 @@ Do not use Latin abbreviations. Use **that is** instead. ([Vale](../testing.md#v Do not use **in order to**. Use **to** instead. ([Vale](../testing.md#vale) rule: [`Wordy.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/.vale/gitlab/Wordy.yml)) +## -ing words + +Remove **-ing** words whenever possible. They can be difficult to translate, +and more precise terms are usually available. For example: + +- Instead of **The files using storage are deleted**, use **The files that use storage are deleted**. +- Instead of **Delete files using the Edit button**, use **Delete files by using the Edit button**. +- Instead of **Replicating your server is required**, use **You must replicate your server**. + ## issue Use lowercase for **issue**. diff --git a/doc/integration/oauth_provider.md b/doc/integration/oauth_provider.md index 964c5edcbc1..23546fb4fd6 100644 --- a/doc/integration/oauth_provider.md +++ b/doc/integration/oauth_provider.md @@ -137,17 +137,3 @@ On self-managed GitLab, by default, this feature is not available. To make it av On GitLab.com, this feature is not available. By default, OAuth application secrets are stored as plain text in the database. When enabled, OAuth application secrets are stored in the database in hashed format and are only available to users immediately after creating OAuth applications. - -## Hashed OAuth tokens - -> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/364110) in GitLab 15.3 [with a flag](../administration/feature_flags.md) named `hash_oauth_tokens`. Enabled on GitLab.com. Disabled by default for self-managed. -> - [Enabled by default on self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/337507) in GitLab 15.5. - -FLAG: -On self-managed GitLab, by default, this feature is enabled. If you detect a problem, ask an administrator to -[disable the feature flag](../administration/feature_flags.md) named `hash_oauth_tokens`. If the feature flag is disabled, any tokens that were stored -in encrypted format are inaccessible. Users must reauthorize applications. -On GitLab.com, this feature is enabled. - -By default, OAuth access tokens are stored in the database in PBKDF2+SHA512 format. GitLab administrators can disable this and OAuth access tokens are -then stored in plaintext in the database. diff --git a/doc/security/password_storage.md b/doc/security/password_storage.md index 6b20f8619ae..33727a489ee 100644 --- a/doc/security/password_storage.md +++ b/doc/security/password_storage.md @@ -45,9 +45,7 @@ library to hash user passwords. Created password hashes have these attributes: ## OAuth access token storage > - PBKDF2+SHA512 [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/364110) in GitLab 15.3 [with flag](../administration/feature_flags.md) named `hash_oauth_tokens`. -> - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/98242) in GitLab 15.5. +> - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/issues/367570) in GitLab 15.5. +> - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/367570) in GitLab 15.6. -Depending on your version of GitLab and configuration, OAuth access tokens are stored in the database in PBKDF2+SHA512 format. For version information, see -the relevant [OAuth provider documentation](../integration/oauth_provider.md#hashed-oauth-tokens). - -As with PBKDF2+SHA512 password storage, access token values are [stretched](https://en.wikipedia.org/wiki/Key_stretching) 20,000 times to harden against brute-force attacks. +OAuth access tokens are stored in the database in PBKDF2+SHA512 format. As with PBKDF2+SHA512 password storage, access token values are [stretched](https://en.wikipedia.org/wiki/Key_stretching) 20,000 times to harden against brute-force attacks. diff --git a/doc/user/discussions/index.md b/doc/user/discussions/index.md index 13d5b27ad41..7abff615c8a 100644 --- a/doc/user/discussions/index.md +++ b/doc/user/discussions/index.md @@ -167,12 +167,9 @@ If an issue or merge request is locked and closed, you cannot reopen it. > - [Renamed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/87403) from "confidential comments" to "internal notes" in GitLab 15.0. > - [Enabled on GitLab.com and self-managed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/87383) in GitLab 15.0. > - [Feature flag `confidential_notes`](https://gitlab.com/gitlab-org/gitlab/-/issues/362712) removed in GitLab 15.2. +> - [Changed] permissions in GitLab 15.5 to at least the Reporter role. In GitLab 15.4 and earlier, issue or epic authors and assignees could also read and create internal notes. -You can add an internal note **to an issue or an epic**. It's then visible only to the following people: - -- Project members who have at least the Reporter role -- Issue or epic author -- Users assigned to the issue or epic +You can add an internal note **to an issue or an epic**. It's then visible only to project members who have at least the Reporter role. Keep in mind: @@ -181,10 +178,7 @@ Keep in mind: Prerequisites: -- You must either: - - Have at least the Reporter role for the project. - - Be the issue or epic assignee. - - Be the issue or epic author. +- You must have at least the Reporter role for the project. To add an internal note: diff --git a/doc/user/group/epics/index.md b/doc/user/group/epics/index.md index da6e675f0eb..985f1edeee0 100644 --- a/doc/user/group/epics/index.md +++ b/doc/user/group/epics/index.md @@ -37,15 +37,9 @@ Also, read more about possible [planning hierarchies](../planning_hierarchy/inde ### Child issues from different group hierarchies -<!-- When feature flag is removed, integrate this info as a sentence in -https://docs.gitlab.com/ee/user/group/epics/manage_epics.html#add-an-existing-issue-to-an-epic --> - > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/371081) in GitLab 15.5 [with a flag](../../../administration/feature_flags.md) named `epic_issues_from_different_hierarchies`. Disabled by default. > - [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues/373304) in GitLab 15.5. - -FLAG: -On self-managed GitLab, by default this feature is unavailable. To make it available, ask an administrator to [enable the feature flag](../../../administration/feature_flags.md) named `epic_issues_from_different_hierarchies`. -On GitLab.com, this feature is available. +> - Feature flag `epic_issues_from_different_hierarchies` removed in GitLab 15.6. You can add issues from a different group hierarchy to an epic. To do it, paste the issue URL when diff --git a/doc/user/group/epics/manage_epics.md b/doc/user/group/epics/manage_epics.md index 0a12f551588..a9c2b91340f 100644 --- a/doc/user/group/epics/manage_epics.md +++ b/doc/user/group/epics/manage_epics.md @@ -339,9 +339,8 @@ automatically added to the epic. #### Add an existing issue to an epic -You can add existing issues to an epic, including issues in a project in an epic's group, or any of -the epic's subgroups. Newly added issues appear at the top of the list of -issues in the **Epics and Issues** tab. +You can add existing issues to an epic, including issues in a project from a [different group hierarchy](index.md#child-issues-from-different-group-hierarchies). +Newly added issues appear at the top of the list of issues in the **Epics and Issues** tab. An epic contains a list of issues and an issue can be associated with at most one epic. When you add a new issue that's already linked to an epic, the issue is automatically unlinked from its @@ -359,7 +358,8 @@ To add an existing issue to an epic: 1. Identify the issue to be added, using either of the following methods: - Paste the link of the issue. - Search for the desired issue by entering part of the issue's title, then selecting the desired - match ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/9126) in GitLab 12.5). + match ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/9126) in GitLab 12.5). Issues + from different group hierarchies do not appear in search results. To add such an issue, enter its full URL. If there are multiple issues to be added, press <kbd>Space</kbd> and repeat this step. 1. Select **Add**. diff --git a/doc/user/group/saml_sso/group_sync.md b/doc/user/group/saml_sso/group_sync.md index 3a50470ce50..001c73b6979 100644 --- a/doc/user/group/saml_sso/group_sync.md +++ b/doc/user/group/saml_sso/group_sync.md @@ -77,12 +77,8 @@ Users granted: ### Automatic member removal After a group sync, for GitLab subgroups, users who are not members of a mapped SAML -group are removed from the group. - -FLAG: -In [GitLab 15.1 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/364144), on GitLab.com, users in the top-level -group are assigned the [default membership role](index.md#role) rather than removed. This setting is enabled with the -`saml_group_sync_retain_default_membership` feature flag and can be configured by GitLab.com administrators only. +group are removed from the group. Users in the top-level group are assigned the +[default membership role](index.md#role). For example, in the following diagram: diff --git a/lib/api/access_requests.rb b/lib/api/access_requests.rb index 74f6515f07f..38a9856ca58 100644 --- a/lib/api/access_requests.rb +++ b/lib/api/access_requests.rb @@ -19,6 +19,7 @@ module API desc "Gets a list of access requests for a #{source_type}." do detail 'This feature was introduced in GitLab 8.11.' success Entities::AccessRequester + tags %w[access_requests] end params do use :pagination @@ -37,6 +38,24 @@ module API desc "Requests access for the authenticated user to a #{source_type}." do detail 'This feature was introduced in GitLab 8.11.' success Entities::AccessRequester + success [ + { + code: 200, + model: Entities::AccessRequester, + message: 'successful operation', + examples: { + successfull_response: { + "id" => 1, + "username" => "raymond_smith", + "name" => "Raymond Smith", + "state" => "active", + "created_at" => "2012-10-22T14:13:35Z", + "access_level" => 20 + } + } + } + ] + tags %w[access_requests] end post ":id/access_requests" do source = find_source(source_type, params[:id]) @@ -51,7 +70,24 @@ module API desc 'Approves an access request for the given user.' do detail 'This feature was introduced in GitLab 8.11.' - success Entities::Member + success [ + { + code: 200, + model: Entities::AccessRequester, + message: 'successful operation', + examples: { + successfull_response: { + "id" => 1, + "username" => "raymond_smith", + "name" => "Raymond Smith", + "state" => "active", + "created_at" => "2012-10-22T14:13:35Z", + "access_level" => 20 + } + } + } + ] + tags %w[access_requests] end params do requires :user_id, type: Integer, desc: 'The user ID of the access requester' @@ -74,6 +110,7 @@ module API desc 'Denies an access request for the given user.' do detail 'This feature was introduced in GitLab 8.11.' + tags %w[access_requests] end params do requires :user_id, type: Integer, desc: 'The user ID of the access requester' diff --git a/lib/api/api.rb b/lib/api/api.rb index 981260b9429..089278ab383 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -169,13 +169,13 @@ module API # Mount endpoints to include in the OpenAPI V2 documentation here namespace do + mount ::API::AccessRequests mount ::API::Metadata add_open_api_documentation! end # Keep in alphabetical order - mount ::API::AccessRequests mount ::API::Admin::BatchedBackgroundMigrations mount ::API::Admin::Ci::Variables mount ::API::Admin::InstanceClusters diff --git a/lib/api/ci/jobs.rb b/lib/api/ci/jobs.rb index 6049993bf6f..0de915c22e0 100644 --- a/lib/api/ci/jobs.rb +++ b/lib/api/ci/jobs.rb @@ -16,7 +16,7 @@ module API helpers do params :optional_scope do - optional :scope, types: [String, Array[String]], desc: 'The scope of builds to show', + optional :scope, type: Array[String], desc: 'The scope of builds to show', values: ::CommitStatus::AVAILABLE_STATUSES, coerce_with: ->(scope) { case scope diff --git a/lib/api/ci/runner.rb b/lib/api/ci/runner.rb index 2d2dcc544f9..52646c80d4b 100644 --- a/lib/api/ci/runner.rb +++ b/lib/api/ci/runner.rb @@ -17,7 +17,13 @@ module API optional :description, type: String, desc: %q(Runner's description) optional :maintainer_note, type: String, desc: %q(Deprecated: Use :maintenance_note instead. Runner's maintenance notes) optional :maintenance_note, type: String, desc: %q(Runner's maintenance notes) - optional :info, type: Hash, desc: %q(Runner's metadata) + optional :info, type: Hash, desc: %q(Runner's metadata) do + optional :name, type: String, desc: %q(Runner's name) + optional :version, type: String, desc: %q(Runner's version) + optional :revision, type: String, desc: %q(Runner's revision) + optional :platform, type: String, desc: %q(Runner's platform) + optional :architecture, type: String, desc: %q(Runner's architecture) + end optional :active, type: Boolean, desc: 'Deprecated: Use `:paused` instead. Should runner be active' optional :paused, type: Boolean, desc: 'Whether the runner should ignore new jobs' optional :locked, type: Boolean, desc: 'Whether the runner should be locked for current project' @@ -285,14 +291,14 @@ module API end params do requires :id, type: Integer, desc: %q(Job's ID) - requires :file, type: ::API::Validations::Types::WorkhorseFile, desc: %(The artifact file to store (generated by Multipart middleware)) + requires :file, type: ::API::Validations::Types::WorkhorseFile, desc: %(The artifact file to store (generated by Multipart middleware)), documentation: { type: 'file' } optional :token, type: String, desc: %q(Job's authentication token) optional :expire_in, type: String, desc: %q(Specify when artifacts should expire) optional :artifact_type, type: String, desc: %q(The type of artifact), default: 'archive', values: ::Ci::JobArtifact.file_types.keys optional :artifact_format, type: String, desc: %q(The format of artifact), default: 'zip', values: ::Ci::JobArtifact.file_formats.keys - optional :metadata, type: ::API::Validations::Types::WorkhorseFile, desc: %(The artifact metadata to store (generated by Multipart middleware)) + optional :metadata, type: ::API::Validations::Types::WorkhorseFile, desc: %(The artifact metadata to store (generated by Multipart middleware)), documentation: { type: 'file' } end post '/:id/artifacts', feature_category: :build_artifacts, urgency: :low do not_allowed! unless Gitlab.config.artifacts.enabled diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 72b56f34ea6..e692b5d53ac 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -7,6 +7,7 @@ module Gitlab include Migrations::BackgroundMigrationHelpers include Migrations::BatchedBackgroundMigrationHelpers include Migrations::LockRetriesHelpers + include Migrations::TimeoutHelpers include DynamicModelHelpers include RenameTableHelpers include AsyncIndexes::MigrationHelpers @@ -361,51 +362,6 @@ module Gitlab "#{prefix}#{hashed_identifier}" end - # Long-running migrations may take more than the timeout allowed by - # the database. Disable the session's statement timeout to ensure - # migrations don't get killed prematurely. - # - # There are two possible ways to disable the statement timeout: - # - # - Per transaction (this is the preferred and default mode) - # - Per connection (requires a cleanup after the execution) - # - # When using a per connection disable statement, code must be inside - # a block so we can automatically execute `RESET statement_timeout` after block finishes - # otherwise the statement will still be disabled until connection is dropped - # or `RESET statement_timeout` is executed - def disable_statement_timeout - if block_given? - if statement_timeout_disabled? - # Don't do anything if the statement_timeout is already disabled - # Allows for nested calls of disable_statement_timeout without - # resetting the timeout too early (before the outer call ends) - yield - else - begin - execute('SET statement_timeout TO 0') - - yield - ensure - execute('RESET statement_timeout') - end - end - else - unless transaction_open? - raise <<~ERROR - Cannot call disable_statement_timeout() without a transaction open or outside of a transaction block. - If you don't want to use a transaction wrap your code in a block call: - - disable_statement_timeout { # code that requires disabled statement here } - - This will make sure statement_timeout is disabled before and reset after the block execution is finished. - ERROR - end - - execute('SET LOCAL statement_timeout TO 0') - end - end - def true_value Database.true_value end @@ -1540,11 +1496,6 @@ into similar problems in the future (e.g. when new tables are created). connection.exec_query(check_sql) end - def statement_timeout_disabled? - # This is a string of the form "100ms" or "0" when disabled - connection.select_value('SHOW statement_timeout') == "0" - end - def column_is_nullable?(table, column) # Check if table.column has not been defined with NOT NULL check_sql = <<~SQL diff --git a/lib/gitlab/database/migrations/timeout_helpers.rb b/lib/gitlab/database/migrations/timeout_helpers.rb new file mode 100644 index 00000000000..423c77452b1 --- /dev/null +++ b/lib/gitlab/database/migrations/timeout_helpers.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Migrations + module TimeoutHelpers + # Long-running migrations may take more than the timeout allowed by + # the database. Disable the session's statement timeout to ensure + # migrations don't get killed prematurely. + # + # There are two possible ways to disable the statement timeout: + # + # - Per transaction (this is the preferred and default mode) + # - Per connection (requires a cleanup after the execution) + # + # When using a per connection disable statement, code must be inside + # a block so we can automatically execute `RESET statement_timeout` after block finishes + # otherwise the statement will still be disabled until connection is dropped + # or `RESET statement_timeout` is executed + def disable_statement_timeout + if block_given? + if statement_timeout_disabled? + # Don't do anything if the statement_timeout is already disabled + # Allows for nested calls of disable_statement_timeout without + # resetting the timeout too early (before the outer call ends) + yield + else + begin + execute('SET statement_timeout TO 0') + + yield + ensure + execute('RESET statement_timeout') + end + end + else + unless transaction_open? + raise <<~ERROR + Cannot call disable_statement_timeout() without a transaction open or outside of a transaction block. + If you don't want to use a transaction wrap your code in a block call: + + disable_statement_timeout { # code that requires disabled statement here } + + This will make sure statement_timeout is disabled before and reset after the block execution is finished. + ERROR + end + + execute('SET LOCAL statement_timeout TO 0') + end + end + + private + + def statement_timeout_disabled? + # This is a string of the form "100ms" or "0" when disabled + connection.select_value('SHOW statement_timeout') == "0" + end + end + end + end +end diff --git a/lib/gitlab/doorkeeper_secret_storing/token/pbkdf2_sha512.rb b/lib/gitlab/doorkeeper_secret_storing/token/pbkdf2_sha512.rb index f9e6d4076f3..7bb9ac2ffdb 100644 --- a/lib/gitlab/doorkeeper_secret_storing/token/pbkdf2_sha512.rb +++ b/lib/gitlab/doorkeeper_secret_storing/token/pbkdf2_sha512.rb @@ -12,8 +12,6 @@ module Gitlab SALT = '' def self.transform_secret(plain_secret) - return plain_secret unless Feature.enabled?(:hash_oauth_tokens) - Devise::Pbkdf2Encryptable::Encryptors::Pbkdf2Sha512.digest(plain_secret, STRETCHES, SALT) end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 51e8687d868..0da3f25c9a4 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -7628,9 +7628,6 @@ msgstr "" msgid "Cannot assign a confidential epic to a non-confidential issue. Make the issue confidential and try again" msgstr "" -msgid "Cannot assign an issue that does not belong under the same group (or descendant) as the epic." -msgstr "" - msgid "Cannot be merged automatically" msgstr "" diff --git a/qa/qa/page/project/settings/default_branch.rb b/qa/qa/page/project/settings/default_branch.rb index 575f9006c84..a6107cdc535 100644 --- a/qa/qa/page/project/settings/default_branch.rb +++ b/qa/qa/page/project/settings/default_branch.rb @@ -18,8 +18,8 @@ module QA end def set_default_branch(branch) - click_button :default_branch_dropdown - fill_in :ref_selector_searchbox, with: branch + find_element(:default_branch_dropdown, visible: false).click + find_element(:ref_selector_searchbox, visible: false).fill_in(with: branch) click_button branch end diff --git a/rubocop/cop/gitlab/rspec/avoid_setup.rb b/rubocop/cop/gitlab/rspec/avoid_setup.rb new file mode 100644 index 00000000000..3b2bf079b99 --- /dev/null +++ b/rubocop/cop/gitlab/rspec/avoid_setup.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require 'rubocop-rspec' + +module RuboCop + module Cop + module Gitlab + module RSpec + # This cop checks for use of constructs that may lead to deterioration in readability + # in specs. + # + # @example + # + # # bad + # before do + # enforce_terms + # end + # + # it 'auto accepts terms and redirects to the group path' do + # visit sso_group_saml_providers_path(group, token: group.saml_discovery_token) + # + # click_link 'Sign in' + # + # expect(page).to have_content('Signed in with SAML') + # end + # + # # good + # it 'auto accepts terms and redirects to the group path' do + # enforce_terms + # + # visit sso_group_saml_providers_path(group, token: group.saml_discovery_token) + # + # click_link 'Sign in' + # + # expect(page).to have_content('Signed in with SAML') + # end + # + # # good + # it 'registers the user and starts to import a project' do + # user_signs_up + # + # expect_to_see_account_confirmation_page + # + # confirm_account + # + # user_signs_in + # + # expect_to_see_welcome_form + # + # fills_in_welcome_form + # click_on 'Continue' + # + # expect_to_see_group_and_project_creation_form + # + # click_on 'Import' + # + # expect_to_see_import_form + # + # fills_in_import_form + # click_on 'GitHub' + # + # expect_to_be_in_import_process + # end + # + class AvoidSetup < RuboCop::Cop::Base + MSG = 'Avoid the use of `%{name}` to keep this area as readable as possible. ' \ + 'See https://gitlab.com/gitlab-org/gitlab/-/issues/373194' + + NOT_ALLOWED = %i[let_it_be let_it_be_with_refind let_it_be_with_reload let let! + before after around it_behaves_like shared_examples shared_examples_for + shared_context include_context].freeze + + RESTRICT_ON_SEND = NOT_ALLOWED + + def on_send(node) + add_offense(node, message: format(MSG, name: node.children[1])) + end + end + end + end + end +end diff --git a/spec/factories/protected_branches.rb b/spec/factories/protected_branches.rb index 8c467acf2a1..75b375733a6 100644 --- a/spec/factories/protected_branches.rb +++ b/spec/factories/protected_branches.rb @@ -40,63 +40,63 @@ FactoryBot.define do end end - trait :maintainers_can_push do + trait :no_one_can_merge do transient do - default_push_level { false } + default_merge_level { false } end after(:build) do |protected_branch| - protected_branch.push_access_levels.new(access_level: Gitlab::Access::MAINTAINER) + protected_branch.merge_access_levels.new(access_level: Gitlab::Access::NO_ACCESS) end end - trait :maintainers_can_merge do + trait :developers_can_merge do transient do - default_push_level { false } + default_merge_level { false } end after(:build) do |protected_branch| - protected_branch.push_access_levels.new(access_level: Gitlab::Access::MAINTAINER) + protected_branch.merge_access_levels.new(access_level: Gitlab::Access::DEVELOPER) end end - trait :developers_can_push do + trait :maintainers_can_merge do transient do - default_push_level { false } + default_merge_level { false } end after(:build) do |protected_branch| - protected_branch.push_access_levels.new(access_level: Gitlab::Access::DEVELOPER) + protected_branch.merge_access_levels.new(access_level: Gitlab::Access::MAINTAINER) end end - trait :developers_can_merge do + trait :no_one_can_push do transient do - default_merge_level { false } + default_push_level { false } end after(:build) do |protected_branch| - protected_branch.merge_access_levels.new(access_level: Gitlab::Access::DEVELOPER) + protected_branch.push_access_levels.new(access_level: Gitlab::Access::NO_ACCESS) end end - trait :no_one_can_push do + trait :developers_can_push do transient do default_push_level { false } end after(:build) do |protected_branch| - protected_branch.push_access_levels.new(access_level: Gitlab::Access::NO_ACCESS) + protected_branch.push_access_levels.new(access_level: Gitlab::Access::DEVELOPER) end end - trait :no_one_can_merge do + trait :maintainers_can_push do transient do - default_merge_level { false } + default_push_level { false } end after(:build) do |protected_branch| - protected_branch.merge_access_levels.new(access_level: Gitlab::Access::NO_ACCESS) + protected_branch.push_access_levels.new(access_level: Gitlab::Access::MAINTAINER) end end end diff --git a/spec/frontend/vue_shared/components/source_viewer/components/chunk_spec.js b/spec/frontend/vue_shared/components/source_viewer/components/chunk_spec.js index 8dc3348acfa..f662f65e852 100644 --- a/spec/frontend/vue_shared/components/source_viewer/components/chunk_spec.js +++ b/spec/frontend/vue_shared/components/source_viewer/components/chunk_spec.js @@ -58,11 +58,7 @@ describe('Chunk component', () => { it('renders simplified line numbers and content if isHighlighted is false', () => { expect(findLineNumbers().length).toBe(DEFAULT_PROPS.totalLines); - expect(findLineNumbers().at(0).attributes()).toMatchObject({ - 'data-line-number': `${DEFAULT_PROPS.startingFrom + 1}`, - href: `#L${DEFAULT_PROPS.startingFrom + 1}`, - id: `L${DEFAULT_PROPS.startingFrom + 1}`, - }); + expect(findLineNumbers().at(0).attributes('id')).toBe(`L${DEFAULT_PROPS.startingFrom + 1}`); expect(findContent().text()).toBe(DEFAULT_PROPS.content); }); diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 64a36b88625..51603a72070 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -1006,88 +1006,6 @@ RSpec.describe Gitlab::Database::MigrationHelpers do end end - describe '#disable_statement_timeout' do - it 'disables statement timeouts to current transaction only' do - expect(model).to receive(:execute).with('SET LOCAL statement_timeout TO 0') - - model.disable_statement_timeout - end - - # this specs runs without an enclosing transaction (:delete truncation method for db_cleaner) - context 'with real environment', :delete do - before do - model.execute("SET statement_timeout TO '20000'") - end - - after do - model.execute('RESET statement_timeout') - end - - it 'defines statement to 0 only for current transaction' do - expect(model.execute('SHOW statement_timeout').first['statement_timeout']).to eq('20s') - - model.connection.transaction do - model.disable_statement_timeout - expect(model.execute('SHOW statement_timeout').first['statement_timeout']).to eq('0') - end - - expect(model.execute('SHOW statement_timeout').first['statement_timeout']).to eq('20s') - end - - context 'when passing a blocks' do - it 'disables statement timeouts on session level and executes the block' do - expect(model).to receive(:execute).with('SET statement_timeout TO 0') - expect(model).to receive(:execute).with('RESET statement_timeout').at_least(:once) - - expect { |block| model.disable_statement_timeout(&block) }.to yield_control - end - - # this specs runs without an enclosing transaction (:delete truncation method for db_cleaner) - context 'with real environment', :delete do - before do - model.execute("SET statement_timeout TO '20000'") - end - - after do - model.execute('RESET statement_timeout') - end - - it 'defines statement to 0 for any code run inside the block' do - expect(model.execute('SHOW statement_timeout').first['statement_timeout']).to eq('20s') - - model.disable_statement_timeout do - model.connection.transaction do - expect(model.execute('SHOW statement_timeout').first['statement_timeout']).to eq('0') - end - - expect(model.execute('SHOW statement_timeout').first['statement_timeout']).to eq('0') - end - end - end - end - end - - # This spec runs without an enclosing transaction (:delete truncation method for db_cleaner) - context 'when the statement_timeout is already disabled', :delete do - before do - ActiveRecord::Migration.connection.execute('SET statement_timeout TO 0') - end - - after do - # Use ActiveRecord::Migration.connection instead of model.execute - # so that this call is not counted below - ActiveRecord::Migration.connection.execute('RESET statement_timeout') - end - - it 'yields control without disabling the timeout or resetting' do - expect(model).not_to receive(:execute).with('SET statement_timeout TO 0') - expect(model).not_to receive(:execute).with('RESET statement_timeout') - - expect { |block| model.disable_statement_timeout(&block) }.to yield_control - end - end - end - describe '#true_value' do it 'returns the appropriate value' do expect(model.true_value).to eq("'t'") diff --git a/spec/lib/gitlab/database/migrations/timeout_helpers_spec.rb b/spec/lib/gitlab/database/migrations/timeout_helpers_spec.rb new file mode 100644 index 00000000000..d35211af680 --- /dev/null +++ b/spec/lib/gitlab/database/migrations/timeout_helpers_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Migrations::TimeoutHelpers do + let(:model) do + ActiveRecord::Migration.new.extend(described_class) + end + + describe '#disable_statement_timeout' do + it 'disables statement timeouts to current transaction only' do + expect(model).to receive(:execute).with('SET LOCAL statement_timeout TO 0') + + model.disable_statement_timeout + end + + # this specs runs without an enclosing transaction (:delete truncation method for db_cleaner) + context 'with real environment', :delete do + before do + model.execute("SET statement_timeout TO '20000'") + end + + after do + model.execute('RESET statement_timeout') + end + + it 'defines statement to 0 only for current transaction' do + expect(model.execute('SHOW statement_timeout').first['statement_timeout']).to eq('20s') + + model.connection.transaction do + model.disable_statement_timeout + expect(model.execute('SHOW statement_timeout').first['statement_timeout']).to eq('0') + end + + expect(model.execute('SHOW statement_timeout').first['statement_timeout']).to eq('20s') + end + + context 'when passing a blocks' do + it 'disables statement timeouts on session level and executes the block' do + expect(model).to receive(:execute).with('SET statement_timeout TO 0') + expect(model).to receive(:execute).with('RESET statement_timeout').at_least(:once) + + expect { |block| model.disable_statement_timeout(&block) }.to yield_control + end + + # this specs runs without an enclosing transaction (:delete truncation method for db_cleaner) + context 'with real environment', :delete do + before do + model.execute("SET statement_timeout TO '20000'") + end + + after do + model.execute('RESET statement_timeout') + end + + it 'defines statement to 0 for any code run inside the block' do + expect(model.execute('SHOW statement_timeout').first['statement_timeout']).to eq('20s') + + model.disable_statement_timeout do + model.connection.transaction do + expect(model.execute('SHOW statement_timeout').first['statement_timeout']).to eq('0') + end + + expect(model.execute('SHOW statement_timeout').first['statement_timeout']).to eq('0') + end + end + end + end + end + + # This spec runs without an enclosing transaction (:delete truncation method for db_cleaner) + context 'when the statement_timeout is already disabled', :delete do + before do + ActiveRecord::Migration.connection.execute('SET statement_timeout TO 0') + end + + after do + # Use ActiveRecord::Migration.connection instead of model.execute + # so that this call is not counted below + ActiveRecord::Migration.connection.execute('RESET statement_timeout') + end + + it 'yields control without disabling the timeout or resetting' do + expect(model).not_to receive(:execute).with('SET statement_timeout TO 0') + expect(model).not_to receive(:execute).with('RESET statement_timeout') + + expect { |block| model.disable_statement_timeout(&block) }.to yield_control + end + end + end +end diff --git a/spec/lib/gitlab/doorkeeper_secret_storing/token/pbkdf2_sha512_spec.rb b/spec/lib/gitlab/doorkeeper_secret_storing/token/pbkdf2_sha512_spec.rb index c73744cd481..e267d27ed13 100644 --- a/spec/lib/gitlab/doorkeeper_secret_storing/token/pbkdf2_sha512_spec.rb +++ b/spec/lib/gitlab/doorkeeper_secret_storing/token/pbkdf2_sha512_spec.rb @@ -10,16 +10,6 @@ RSpec.describe Gitlab::DoorkeeperSecretStoring::Token::Pbkdf2Sha512 do expect(described_class.transform_secret(plaintext_token)) .to eq("$pbkdf2-sha512$20000$$.c0G5XJVEew1TyeJk5TrkvB0VyOaTmDzPrsdNRED9vVeZlSyuG3G90F0ow23zUCiWKAVwmNnR/ceh.nJG3MdpQ") # rubocop:disable Layout/LineLength end - - context 'when hash_oauth_tokens is disabled' do - before do - stub_feature_flags(hash_oauth_tokens: false) - end - - it 'returns a plaintext token' do - expect(described_class.transform_secret(plaintext_token)).to eq(plaintext_token) - end - end end describe 'STRETCHES' do diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 9700852e567..bc72d28ae77 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -431,8 +431,6 @@ RSpec.describe Event do include_examples 'visibility examples' do let(:visibility) { visible_to_none_except(:member) } end - - include_examples 'visible to author', true end context 'private project' do diff --git a/spec/models/integrations/assembla_spec.rb b/spec/models/integrations/assembla_spec.rb index 960dfea3dc4..e9f4274952d 100644 --- a/spec/models/integrations/assembla_spec.rb +++ b/spec/models/integrations/assembla_spec.rb @@ -5,6 +5,10 @@ require 'spec_helper' RSpec.describe Integrations::Assembla do include StubRequests + it_behaves_like Integrations::ResetSecretFields do + let(:integration) { described_class.new } + end + describe "Execute" do let(:user) { create(:user) } let(:project) { create(:project, :repository) } diff --git a/spec/models/integrations/bamboo_spec.rb b/spec/models/integrations/bamboo_spec.rb index e92226d109f..ac8ea52dd3e 100644 --- a/spec/models/integrations/bamboo_spec.rb +++ b/spec/models/integrations/bamboo_spec.rb @@ -23,6 +23,8 @@ RSpec.describe Integrations::Bamboo, :use_clean_rails_memory_store_caching do ) end + it_behaves_like Integrations::ResetSecretFields + include_context Integrations::EnableSslVerification describe 'Validations' do @@ -77,48 +79,6 @@ RSpec.describe Integrations::Bamboo, :use_clean_rails_memory_store_caching do end end - describe 'Callbacks' do - describe 'before_validation :reset_password' do - context 'when a password was previously set' do - it 'resets password if url changed' do - integration.bamboo_url = 'http://gitlab1.com' - - expect(integration).not_to be_valid - expect(integration.password).to be_nil - end - - it 'does not reset password if username changed' do - integration.username = 'some_name' - - expect(integration).to be_valid - expect(integration.password).to eq('password') - end - - it "does not reset password if new url is set together with password, even if it's the same password" do - integration.bamboo_url = 'http://gitlab_edited.com' - integration.password = 'password' - - expect(integration).to be_valid - expect(integration.password).to eq('password') - expect(integration.bamboo_url).to eq('http://gitlab_edited.com') - end - end - - it 'saves password if new url is set together with password when no password was previously set' do - integration.password = nil - - integration.bamboo_url = 'http://gitlab_edited.com' - integration.password = 'password' - integration.save! - - expect(integration.reload).to have_attributes( - bamboo_url: 'http://gitlab_edited.com', - password: 'password' - ) - end - end - end - describe '#execute' do it 'runs update and build action' do stub_update_and_build_request diff --git a/spec/models/integrations/buildkite_spec.rb b/spec/models/integrations/buildkite_spec.rb index ef686c0ae3c..5535a13db7f 100644 --- a/spec/models/integrations/buildkite_spec.rb +++ b/spec/models/integrations/buildkite_spec.rb @@ -18,6 +18,8 @@ RSpec.describe Integrations::Buildkite, :use_clean_rails_memory_store_caching do ) end + it_behaves_like Integrations::ResetSecretFields + it_behaves_like Integrations::HasWebHook do let(:hook_url) { 'https://webhook.buildkite.com/deliver/{webhook_token}' } end diff --git a/spec/models/integrations/jenkins_spec.rb b/spec/models/integrations/jenkins_spec.rb index 929473b0f02..4e787f958af 100644 --- a/spec/models/integrations/jenkins_spec.rb +++ b/spec/models/integrations/jenkins_spec.rb @@ -23,6 +23,10 @@ RSpec.describe Integrations::Jenkins do } end + it_behaves_like Integrations::ResetSecretFields do + let(:integration) { jenkins_integration } + end + include_context Integrations::EnableSslVerification do let(:integration) { jenkins_integration } end @@ -38,7 +42,7 @@ RSpec.describe Integrations::Jenkins do expect(jenkins_integration.tag_push_events).to eq(false) end - describe 'username validation' do + describe 'Validations' do let(:jenkins_integration) do described_class.create!( active: active, @@ -57,28 +61,44 @@ RSpec.describe Integrations::Jenkins do context 'when the integration is active' do let(:active) { true } - context 'when password was not touched' do - before do - allow(subject).to receive(:password_touched?).and_return(false) + describe '#username' do + context 'when password was not touched' do + before do + allow(subject).to receive(:password_touched?).and_return(false) + end + + it { is_expected.not_to validate_presence_of :username } end - it { is_expected.not_to validate_presence_of :username } - end + context 'when password was touched' do + before do + allow(subject).to receive(:password_touched?).and_return(true) + end - context 'when password was touched' do - before do - allow(subject).to receive(:password_touched?).and_return(true) + it { is_expected.to validate_presence_of :username } end - it { is_expected.to validate_presence_of :username } + context 'when password is blank' do + it 'does not validate the username' do + expect(subject).not_to validate_presence_of :username + + subject.password = '' + subject.save! + end + end end - context 'when password is blank' do - it 'does not validate the username' do - expect(subject).not_to validate_presence_of :username + describe '#password' do + it 'does not validate the presence of password if username is nil' do + subject.username = nil + + expect(subject).not_to validate_presence_of(:password) + end + + it 'validates the presence of password if username is present' do + subject.username = 'john' - subject.password = '' - subject.save! + expect(subject).to validate_presence_of(:password) end end end @@ -87,6 +107,7 @@ RSpec.describe Integrations::Jenkins do let(:active) { false } it { is_expected.not_to validate_presence_of :username } + it { is_expected.not_to validate_presence_of :password } end end @@ -190,80 +211,4 @@ RSpec.describe Integrations::Jenkins do ).to have_been_made.once end end - - describe 'Stored password invalidation' do - context 'when a password was previously set' do - let(:jenkins_integration) do - described_class.create!( - project: project, - properties: { - jenkins_url: 'http://jenkins.example.com/', - username: 'jenkins', - password: 'password' - } - ) - end - - it 'resets password if url changed' do - jenkins_integration.jenkins_url = 'http://jenkins-edited.example.com/' - jenkins_integration.valid? - - expect(jenkins_integration.password).to be_nil - end - - it 'resets password if username is blank' do - jenkins_integration.username = '' - jenkins_integration.valid? - - expect(jenkins_integration.password).to be_nil - end - - it 'does not reset password if username changed' do - jenkins_integration.username = 'some_name' - jenkins_integration.valid? - - expect(jenkins_integration.password).to eq('password') - end - - it 'does not reset password if new url is set together with password, even if it\'s the same password' do - jenkins_integration.jenkins_url = 'http://jenkins_edited.example.com/' - jenkins_integration.password = 'password' - jenkins_integration.valid? - - expect(jenkins_integration.password).to eq('password') - expect(jenkins_integration.jenkins_url).to eq('http://jenkins_edited.example.com/') - end - - it 'resets password if url changed, even if setter called multiple times' do - jenkins_integration.jenkins_url = 'http://jenkins1.example.com/' - jenkins_integration.jenkins_url = 'http://jenkins1.example.com/' - jenkins_integration.valid? - - expect(jenkins_integration.password).to be_nil - end - end - - context 'when no password was previously set' do - let(:jenkins_integration) do - described_class.create!( - project: project, - properties: { - jenkins_url: 'http://jenkins.example.com/', - username: 'jenkins' - } - ) - end - - it 'saves password if new url is set together with password' do - jenkins_integration.jenkins_url = 'http://jenkins_edited.example.com/' - jenkins_integration.password = 'password' - jenkins_integration.save! - - expect(jenkins_integration.reload).to have_attributes( - jenkins_url: 'http://jenkins_edited.example.com/', - password: 'password' - ) - end - end - end end diff --git a/spec/models/integrations/teamcity_spec.rb b/spec/models/integrations/teamcity_spec.rb index ae3e6658b3c..5160b410514 100644 --- a/spec/models/integrations/teamcity_spec.rb +++ b/spec/models/integrations/teamcity_spec.rb @@ -22,6 +22,8 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do ) end + it_behaves_like Integrations::ResetSecretFields + include_context Integrations::EnableSslVerification do describe '#enable_ssl_verification' do before do @@ -120,50 +122,6 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do end end - describe 'Callbacks' do - let(:teamcity_integration) { integration } - - describe 'before_validation :reset_password' do - context 'when a password was previously set' do - it 'resets password if url changed' do - teamcity_integration.teamcity_url = 'http://gitlab1.com' - teamcity_integration.valid? - - expect(teamcity_integration.password).to be_nil - end - - it 'does not reset password if username changed' do - teamcity_integration.username = 'some_name' - teamcity_integration.valid? - - expect(teamcity_integration.password).to eq('password') - end - - it "does not reset password if new url is set together with password, even if it's the same password" do - teamcity_integration.teamcity_url = 'http://gitlab_edited.com' - teamcity_integration.password = 'password' - teamcity_integration.valid? - - expect(teamcity_integration.password).to eq('password') - expect(teamcity_integration.teamcity_url).to eq('http://gitlab_edited.com') - end - end - - it 'saves password if new url is set together with password when no password was previously set' do - teamcity_integration.password = nil - - teamcity_integration.teamcity_url = 'http://gitlab_edited.com' - teamcity_integration.password = 'password' - teamcity_integration.save! - - expect(teamcity_integration.reload).to have_attributes( - teamcity_url: 'http://gitlab_edited.com', - password: 'password' - ) - end - end - end - describe '#build_page' do it 'returns the contents of the reactive cache' do stub_reactive_cache(integration, { build_page: 'foo' }, 'sha', 'ref') diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 670a6237788..100a604fdce 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -1619,70 +1619,6 @@ RSpec.describe Note do end end - describe '#noteable_assignee_or_author?' do - let(:user) { create(:user) } - let(:noteable) { create(:issue) } - let(:note) { create(:note, project: noteable.project, noteable: noteable) } - - subject { note.noteable_assignee_or_author?(user) } - - shared_examples 'assignee check' do - context 'when the provided user is one of the assignees' do - before do - note.noteable.update!(assignees: [user, create(:user)]) - end - - it 'returns true' do - expect(subject).to be_truthy - end - end - end - - shared_examples 'author check' do - context 'when the provided user is the author' do - before do - note.noteable.update!(author: user) - end - - it 'returns true' do - expect(subject).to be_truthy - end - end - - context 'when the provided user is neither author nor assignee' do - it 'returns true' do - expect(subject).to be_falsey - end - end - end - - context 'when user is nil' do - let(:user) { nil } - - it 'returns false' do - expect(subject).to be_falsey - end - end - - context 'when noteable is an issue' do - it_behaves_like 'author check' - it_behaves_like 'assignee check' - end - - context 'when noteable is a merge request' do - let(:noteable) { create(:merge_request) } - - it_behaves_like 'author check' - it_behaves_like 'assignee check' - end - - context 'when noteable is a snippet' do - let(:noteable) { create(:personal_snippet) } - - it_behaves_like 'author check' - end - end - describe 'banzai_render_context' do let(:project) { build(:project_empty_repo) } diff --git a/spec/models/oauth_access_token_spec.rb b/spec/models/oauth_access_token_spec.rb index a4540ac95bc..92e1ae8ac60 100644 --- a/spec/models/oauth_access_token_spec.rb +++ b/spec/models/oauth_access_token_spec.rb @@ -46,42 +46,11 @@ RSpec.describe OauthAccessToken do expect(described_class.by_token(plaintext_token)).to be_a(OauthAccessToken) end end - - context 'when hash_oauth_secrets is disabled' do - let(:hashed_token) { create(:oauth_access_token, application_id: app_one.id) } - - before do - hashed_token - stub_feature_flags(hash_oauth_tokens: false) - end - - it 'stores the token in plaintext' do - expect(token.token).to eq(token.plaintext_token) - end - - it 'finds a token by plaintext token' do - expect(described_class.by_token(token.plaintext_token)).to be_a(OauthAccessToken) - end - - it 'does not find a token that was previously stored as hashed' do - expect(described_class.by_token(hashed_token.plaintext_token)).to be_nil - end - end end describe '.matching_token_for' do it 'does not find existing tokens' do expect(described_class.matching_token_for(app_one, token.resource_owner, token.scopes)).to be_nil end - - context 'when hash oauth tokens is disabled' do - before do - stub_feature_flags(hash_oauth_tokens: false) - end - - it 'finds an existing token' do - expect(described_class.matching_token_for(app_one, token.resource_owner, token.scopes)).to be_present - end - end end end diff --git a/spec/policies/issuable_policy_spec.rb b/spec/policies/issuable_policy_spec.rb index 2bedcf60539..c8c322b02db 100644 --- a/spec/policies/issuable_policy_spec.rb +++ b/spec/policies/issuable_policy_spec.rb @@ -31,8 +31,8 @@ RSpec.describe IssuablePolicy, models: true do expect(policies).to be_allowed(:resolve_note) end - it 'allows reading internal notes' do - expect(policies).to be_allowed(:read_internal_note) + it 'does not allow reading internal notes' do + expect(policies).to be_disallowed(:read_internal_note) end context 'when user is able to read project' do @@ -94,8 +94,8 @@ RSpec.describe IssuablePolicy, models: true do let(:issue) { create(:issue, project: project, assignees: [user]) } let(:policies) { described_class.new(user, issue) } - it 'allows reading internal notes' do - expect(policies).to be_allowed(:read_internal_note) + it 'does not allow reading internal notes' do + expect(policies).to be_disallowed(:read_internal_note) end end diff --git a/spec/policies/note_policy_spec.rb b/spec/policies/note_policy_spec.rb index eeaa77a4589..6a261b4ff5b 100644 --- a/spec/policies/note_policy_spec.rb +++ b/spec/policies/note_policy_spec.rb @@ -309,42 +309,41 @@ RSpec.describe NotePolicy do shared_examples_for 'confidential notes permissions' do it 'does not allow non members to read confidential notes and replies' do - expect(permissions(non_member, confidential_note)).to be_disallowed(:read_note, :admin_note, :reposition_note, :resolve_note, :award_emoji) + expect(permissions(non_member, confidential_note)).to be_disallowed(:read_note, :admin_note, :reposition_note, :resolve_note, :award_emoji, :mark_note_as_confidential) end it 'does not allow guests to read confidential notes and replies' do - expect(permissions(guest, confidential_note)).to be_disallowed(:read_note, :admin_note, :reposition_note, :resolve_note, :award_emoji) + expect(permissions(guest, confidential_note)).to be_disallowed(:read_note, :admin_note, :reposition_note, :resolve_note, :award_emoji, :mark_note_as_confidential) end it 'allows reporter to read all notes but not resolve and admin them' do - expect(permissions(reporter, confidential_note)).to be_allowed(:read_note, :award_emoji) + expect(permissions(reporter, confidential_note)).to be_allowed(:read_note, :award_emoji, :mark_note_as_confidential) expect(permissions(reporter, confidential_note)).to be_disallowed(:admin_note, :reposition_note, :resolve_note) end it 'allows developer to read and resolve all notes' do - expect(permissions(developer, confidential_note)).to be_allowed(:read_note, :award_emoji, :resolve_note) + expect(permissions(developer, confidential_note)).to be_allowed(:read_note, :award_emoji, :resolve_note, :mark_note_as_confidential) expect(permissions(developer, confidential_note)).to be_disallowed(:admin_note, :reposition_note) end it 'allows maintainers to read all notes and admin them' do - expect(permissions(maintainer, confidential_note)).to be_allowed(:read_note, :admin_note, :reposition_note, :resolve_note, :award_emoji) + expect(permissions(maintainer, confidential_note)).to be_allowed(:read_note, :admin_note, :reposition_note, :resolve_note, :award_emoji, :mark_note_as_confidential) end context 'when admin mode is enabled', :enable_admin_mode do it 'allows admins to read all notes and admin them' do - expect(permissions(admin, confidential_note)).to be_allowed(:read_note, :admin_note, :reposition_note, :resolve_note, :award_emoji) + expect(permissions(admin, confidential_note)).to be_allowed(:read_note, :admin_note, :reposition_note, :resolve_note, :award_emoji, :mark_note_as_confidential) end end context 'when admin mode is disabled' do it 'does not allow non members to read confidential notes and replies' do - expect(permissions(admin, confidential_note)).to be_disallowed(:read_note, :admin_note, :reposition_note, :resolve_note, :award_emoji) + expect(permissions(admin, confidential_note)).to be_disallowed(:read_note, :admin_note, :reposition_note, :resolve_note, :award_emoji, :mark_note_as_confidential) end end - it 'allows noteable author to read and resolve all notes' do - expect(permissions(author, confidential_note)).to be_allowed(:read_note, :resolve_note, :award_emoji) - expect(permissions(author, confidential_note)).to be_disallowed(:admin_note, :reposition_note) + it 'disallows noteable author to read and resolve all notes' do + expect(permissions(author, confidential_note)).to be_disallowed(:read_note, :resolve_note, :award_emoji, :mark_note_as_confidential, :admin_note, :reposition_note) end end @@ -354,9 +353,8 @@ RSpec.describe NotePolicy do it_behaves_like 'confidential notes permissions' - it 'allows noteable assignees to read all notes' do - expect(permissions(assignee, confidential_note)).to be_allowed(:read_note, :award_emoji) - expect(permissions(assignee, confidential_note)).to be_disallowed(:admin_note, :reposition_note, :resolve_note) + it 'disallows noteable assignees to read all notes' do + expect(permissions(assignee, confidential_note)).to be_disallowed(:read_note, :award_emoji, :mark_note_as_confidential, :admin_note, :reposition_note, :resolve_note) end end end diff --git a/spec/requests/api/graphql/project/branch_protections/merge_access_levels_spec.rb b/spec/requests/api/graphql/project/branch_protections/merge_access_levels_spec.rb index cb5006ec8e4..a80f683ea93 100644 --- a/spec/requests/api/graphql/project/branch_protections/merge_access_levels_spec.rb +++ b/spec/requests/api/graphql/project/branch_protections/merge_access_levels_spec.rb @@ -3,107 +3,5 @@ require 'spec_helper' RSpec.describe 'getting merge access levels for a branch protection' do - include GraphqlHelpers - - let_it_be(:current_user) { create(:user) } - - let(:merge_access_level_data) { merge_access_levels_data[0] } - - let(:merge_access_levels_data) do - graphql_data_at('project', - 'branchRules', - 'nodes', - 0, - 'branchProtection', - 'mergeAccessLevels', - 'nodes') - end - - let(:project) { protected_branch.project } - - let(:merge_access_levels_count) { protected_branch.merge_access_levels.size } - - let(:variables) { { path: project.full_path } } - - let(:fields) { all_graphql_fields_for('MergeAccessLevel') } - - let(:query) do - <<~GQL - query($path: ID!) { - project(fullPath: $path) { - branchRules(first: 1) { - nodes { - branchProtection { - mergeAccessLevels { - nodes { - #{fields} - } - } - } - } - } - } - } - GQL - end - - context 'when the user does not have read_protected_branch abilities' do - let_it_be(:protected_branch) { create(:protected_branch) } - - before do - project.add_guest(current_user) - post_graphql(query, current_user: current_user, variables: variables) - end - - it_behaves_like 'a working graphql query' - - it { expect(merge_access_levels_data).not_to be_present } - end - - shared_examples 'merge access request' do - let(:merge_access) { protected_branch.merge_access_levels.first } - - before do - project.add_maintainer(current_user) - post_graphql(query, current_user: current_user, variables: variables) - end - - it_behaves_like 'a working graphql query' - - it 'returns all merge access levels' do - expect(merge_access_levels_data.size).to eq(merge_access_levels_count) - end - - it 'includes access_level' do - expect(merge_access_level_data['accessLevel']) - .to eq(merge_access.access_level) - end - - it 'includes access_level_description' do - expect(merge_access_level_data['accessLevelDescription']) - .to eq(merge_access.humanize) - end - end - - context 'when the user does have read_protected_branch abilities' do - let(:merge_access) { protected_branch.merge_access_levels.first } - - context 'when no one has access' do - let_it_be(:protected_branch) { create(:protected_branch, :no_one_can_merge) } - - it_behaves_like 'merge access request' - end - - context 'when developers have access' do - let_it_be(:protected_branch) { create(:protected_branch, :developers_can_merge) } - - it_behaves_like 'merge access request' - end - - context 'when maintainers have access' do - let_it_be(:protected_branch) { create(:protected_branch, :maintainers_can_merge) } - - it_behaves_like 'merge access request' - end - end + include_examples 'perform graphql requests for AccessLevel type objects', :merge end diff --git a/spec/requests/api/graphql/project/branch_protections/push_access_levels_spec.rb b/spec/requests/api/graphql/project/branch_protections/push_access_levels_spec.rb index 59f9c7d61cb..cfdaf1096c3 100644 --- a/spec/requests/api/graphql/project/branch_protections/push_access_levels_spec.rb +++ b/spec/requests/api/graphql/project/branch_protections/push_access_levels_spec.rb @@ -3,107 +3,5 @@ require 'spec_helper' RSpec.describe 'getting push access levels for a branch protection' do - include GraphqlHelpers - - let_it_be(:current_user) { create(:user) } - - let(:push_access_level_data) { push_access_levels_data[0] } - - let(:push_access_levels_data) do - graphql_data_at('project', - 'branchRules', - 'nodes', - 0, - 'branchProtection', - 'pushAccessLevels', - 'nodes') - end - - let(:project) { protected_branch.project } - - let(:push_access_levels_count) { protected_branch.push_access_levels.size } - - let(:variables) { { path: project.full_path } } - - let(:fields) { all_graphql_fields_for('PushAccessLevel'.classify) } - - let(:query) do - <<~GQL - query($path: ID!) { - project(fullPath: $path) { - branchRules(first: 1) { - nodes { - branchProtection { - pushAccessLevels { - nodes { - #{fields} - } - } - } - } - } - } - } - GQL - end - - context 'when the user does not have read_protected_branch abilities' do - let_it_be(:protected_branch) { create(:protected_branch) } - - before do - project.add_guest(current_user) - post_graphql(query, current_user: current_user, variables: variables) - end - - it_behaves_like 'a working graphql query' - - it { expect(push_access_levels_data).not_to be_present } - end - - shared_examples 'push access request' do - let(:push_access) { protected_branch.push_access_levels.first } - - before do - project.add_maintainer(current_user) - post_graphql(query, current_user: current_user, variables: variables) - end - - it_behaves_like 'a working graphql query' - - it 'returns all push access levels' do - expect(push_access_levels_data.size).to eq(push_access_levels_count) - end - - it 'includes access_level' do - expect(push_access_level_data['accessLevel']) - .to eq(push_access.access_level) - end - - it 'includes access_level_description' do - expect(push_access_level_data['accessLevelDescription']) - .to eq(push_access.humanize) - end - end - - context 'when the user does have read_protected_branch abilities' do - let(:push_access) { protected_branch.push_access_levels.first } - - context 'when no one has access' do - let_it_be(:protected_branch) { create(:protected_branch, :no_one_can_push) } - - it_behaves_like 'push access request' - end - - context 'when developers have access' do - let_it_be(:protected_branch) { create(:protected_branch, :developers_can_push) } - - it_behaves_like 'push access request' - end - - context 'when maintainers have access' do - let_it_be(:protected_branch) { create(:protected_branch, :maintainers_can_push) } - - it_behaves_like 'push access request' - end - end + include_examples 'perform graphql requests for AccessLevel type objects', :push end diff --git a/spec/requests/api/graphql/project/branch_rules_spec.rb b/spec/requests/api/graphql/project/branch_rules_spec.rb index 1aaf0e9edc7..65c519c46cf 100644 --- a/spec/requests/api/graphql/project/branch_rules_spec.rb +++ b/spec/requests/api/graphql/project/branch_rules_spec.rb @@ -21,8 +21,7 @@ RSpec.describe 'getting list of branch rules for a project' do let(:branch_rules_data) { graphql_data_at('project', 'branchRules', 'edges') } let(:variables) { { path: project.full_path } } - # fields must use let as the all_graphql_fields_for also configures some spies - let(:fields) { all_graphql_fields_for('BranchRule') } + let(:fields) { all_graphql_fields_for('BranchRule', max_depth: 2) } let(:query) do <<~GQL query($path: ID!, $n: Int, $cursor: String) { diff --git a/spec/rubocop/cop/gitlab/rspec/avoid_setup_spec.rb b/spec/rubocop/cop/gitlab/rspec/avoid_setup_spec.rb new file mode 100644 index 00000000000..f9226649f65 --- /dev/null +++ b/spec/rubocop/cop/gitlab/rspec/avoid_setup_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'rubocop_spec_helper' + +require_relative '../../../../../rubocop/cop/gitlab/rspec/avoid_setup' + +RSpec.describe RuboCop::Cop::Gitlab::RSpec::AvoidSetup do + context 'when calling let_it_be' do + let(:source) do + <<~SRC + let_it_be(:user) { create(:user) } + ^^^^^^^^^^^^^^^^ Avoid the use of `let_it_be` [...] + SRC + end + + it 'registers an offense' do + expect_offense(source) + end + end + + context 'without readability issues' do + let(:source) do + <<~SRC + it 'registers the user and sends them to a project listing page' do + user_signs_up + + expect_to_see_account_confirmation_page + end + SRC + end + + it 'does not register an offense' do + expect_no_offenses(source) + end + end +end diff --git a/spec/services/notes/build_service_spec.rb b/spec/services/notes/build_service_spec.rb index c25895d2efa..67d8b37f809 100644 --- a/spec/services/notes/build_service_spec.rb +++ b/spec/services/notes/build_service_spec.rb @@ -189,13 +189,13 @@ RSpec.describe Notes::BuildService do context 'issuable author' do let(:user) { noteable_author } - it_behaves_like 'user allowed to set comment as confidential' + it_behaves_like 'user not allowed to set comment as confidential' end context 'issuable assignee' do let(:user) { issuable_assignee } - it_behaves_like 'user allowed to set comment as confidential' + it_behaves_like 'user not allowed to set comment as confidential' end context 'admin' do @@ -265,13 +265,13 @@ RSpec.describe Notes::BuildService do context 'with noteable author' do let(:user) { note.noteable.author } - it_behaves_like 'confidential set to `true`' + it_behaves_like 'returns `Discussion to reply to cannot be found` error' end context 'with noteable assignee' do let(:user) { issuable_assignee } - it_behaves_like 'confidential set to `true`' + it_behaves_like 'returns `Discussion to reply to cannot be found` error' end context 'with guest access' do diff --git a/spec/support/shared_examples/requests/api/graphql/projects/branch_protections/access_level_request_examples.rb b/spec/support/shared_examples/requests/api/graphql/projects/branch_protections/access_level_request_examples.rb new file mode 100644 index 00000000000..54cc13fac94 --- /dev/null +++ b/spec/support/shared_examples/requests/api/graphql/projects/branch_protections/access_level_request_examples.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'perform graphql requests for AccessLevel type objects' do |access_level_kind| + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:current_user) { create(:user, maintainer_projects: [project]) } + let_it_be(:variables) { { path: project.full_path } } + + let(:fields) { all_graphql_fields_for("#{access_level_kind.to_s.classify}AccessLevel", max_depth: 2) } + let(:access_levels) { protected_branch.public_send("#{access_level_kind}_access_levels") } + let(:access_levels_count) { access_levels.size } + let(:maintainer_access_level) { access_levels.for_role.first } + let(:maintainer_access_level_data) { access_levels_data.first } + let(:access_levels_data) do + graphql_data_at('project', + 'branchRules', + 'nodes', + 0, + 'branchProtection', + "#{access_level_kind.to_s.camelize(:lower)}AccessLevels", + 'nodes') + end + + let(:query) do + <<~GQL + query($path: ID!) { + project(fullPath: $path) { + branchRules(first: 1) { + nodes { + branchProtection { + #{access_level_kind.to_s.camelize(:lower)}AccessLevels { + nodes { + #{fields} + } + } + } + } + } + } + } + GQL + end + + context 'when request AccessLevel type objects as a guest user' do + let_it_be(:protected_branch) { create(:protected_branch, project: project) } + + before do + project.add_guest(current_user) + + post_graphql(query, current_user: current_user, variables: variables) + end + + it_behaves_like 'a working graphql query' + + it { expect(access_levels_data).not_to be_present } + end + + context 'when request AccessLevel type objects as a maintainer' do + let_it_be(:protected_branch) do + create(:protected_branch, "maintainers_can_#{access_level_kind}", project: project) + end + + before do + post_graphql(query, current_user: current_user, variables: variables) + end + + it_behaves_like 'a working graphql query' + + it 'returns all the access level attributes' do + expect(maintainer_access_level_data['accessLevel']).to eq(maintainer_access_level.access_level) + expect(maintainer_access_level_data['accessLevelDescription']).to eq(maintainer_access_level.humanize) + expect(maintainer_access_level_data.dig('group', 'name')).to be_nil + expect(maintainer_access_level_data.dig('user', 'name')).to be_nil + end + end +end |