Commit 155d74fe authored by GitLab Bot's avatar GitLab Bot

Automatic merge of gitlab-org/gitlab master

parents 302cda73 90ca0ee8
......@@ -24,7 +24,7 @@ https://docs.gitlab.com/ee/development/documentation/index.html#move-or-rename-a
specifically under the `app/views/` and `ee/app/views` (for GitLab EE) directories.
- [ ] Make sure to add [`redirect_from`](https://docs.gitlab.com/ee/development/documentation/index.html#redirections-for-pages-with-disqus-comments)
to the new document if there are any Disqus comments on the old document thread.
- [ ] Update the link in `features.yml` (if applicable)
- [ ] Update the link in `features.yml` (if applicable).
- [ ] Assign one of the technical writers for review.
/label ~documentation ~"Technical Writing"
......@@ -41,7 +41,7 @@ They are frequently updated, and everyone should make sure they are aware of the
## Reviewers
When the content is ready for review, it must be reviewed by Technical Writer and Engineering Manager, but can also be reviewed by
When the content is ready for review, it must be reviewed by a Technical Writer and Engineering Manager, but can also be reviewed by
Product Marketing, Product Design, and the Product Leaders for this area. Please use the
[Reviewers for Merge Requests](https://docs.gitlab.com/ee/user/project/merge_requests/getting_started#reviewer)
feature for all reviews. Reviewers will then `approve` the MR and remove themselves from Reviewers when their review is complete.
......@@ -61,7 +61,7 @@ as needed, @ message the PM to inform them the first review is complete, and rem
yourself as a reviewer if it's not ready for merge yet.
<details>
<summary>Expand for Details </summary>
<summary>Expand for Details</summary>
- [ ] Title:
- Length limit: 7 words (not including articles or prepositions).
......
......@@ -8,7 +8,7 @@
## Author's checklist
- [ ] Consider taking [the GitLab Technical Writing Fundamentals course](https://gitlab.edcast.com/pathways/ECL-02528ee2-c334-4e16-abf3-e9d8b8260de4)
- [ ] Consider taking [the GitLab Technical Writing Fundamentals course](https://gitlab.edcast.com/pathways/ECL-02528ee2-c334-4e16-abf3-e9d8b8260de4).
- [ ] Follow the:
- [Documentation process](https://docs.gitlab.com/ee/development/documentation/workflow.html).
- [Documentation guidelines](https://docs.gitlab.com/ee/development/documentation/).
......@@ -40,7 +40,7 @@ Documentation-related MRs should be reviewed by a Technical Writer for a non-blo
- [ ] The headings (other than the page title) should be active. Instead of `Configuring GDK`, say something like `Configure GDK`.
- [ ] Any task steps should be written as a numbered list.
- If the content still needs to be edited for topic types, you can create a follow-up issue with the ~"docs-technical-debt" label.
- [ ] Review by assigned maintainer, who can always request/require the above reviews. Maintainer's review can occur before or after a technical writer review.
- [ ] Review by assigned maintainer, who can always request/require the reviews above. Maintainer's review can occur before or after a technical writer review.
- [ ] Ensure a release milestone is set.
/label ~documentation ~"type::maintenance"
......
......@@ -4,7 +4,7 @@
Please link to the respective test case in the testcases project
-->
### Check-list
### Checklist
- [ ] Confirm the test has a [`testcase:` tag linking to an existing test case](https://docs.gitlab.com/ee/development/testing_guide/end_to_end/best_practices.html#link-a-test-to-its-test-case-issue) in the test case project.
- [ ] Note if the test is intended to run in specific scenarios. If a scenario is new, add a link to the MR that adds the new scenario.
......@@ -15,7 +15,7 @@ Please link to the respective test case in the testcases project
- [ ] Verify the tags to ensure it runs on the desired test environments.
- [ ] If this MR has a dependency on another MR, such as a GitLab QA MR, specify the order in which the MRs should be merged.
- [ ] (If applicable) Create a follow-up issue to document [the special setup](https://docs.gitlab.com/ee/development/testing_guide/end_to_end/running_tests_that_require_special_setup.html) necessary to run the test: ISSUE_LINK
- [ ] If the test requires an admin's personal access token, ensure that the test passes on your local with and without the `GITLAB_QA_ADMIN_ACCESS_TOKEN` provided.
- [ ] If the test requires an admin's personal access token, ensure that the test passes on your local environment with and without the `GITLAB_QA_ADMIN_ACCESS_TOKEN` provided.
<!-- Base labels. -->
/label ~"Quality" ~"QA" ~test
......
......@@ -12,20 +12,20 @@ Please describe the proposal and add a link to the source (for example, http://w
### Check-list
- [ ] Make sure this MR enables a static analysis check rule for new usage but
ignores current offenses
- [ ] Mention this proposal in the relevant Slack channels (e.g. `#development`, `#backend`, `#frontend`)
ignores current offenses.
- [ ] Mention this proposal in the relevant Slack channels (e.g. `#development`, `#backend`, `#frontend`).
- [ ] If there is a choice to make between two potential styles, set up an emoji vote in the MR:
- CHOICE_A: :a:
- CHOICE_B: :b:
- Vote yourself for both choices so that people know these are the choices
- [ ] The MR doesn't have significant objections, and is getting a majority of :+1: vs :-1: (remember that [we don't need to reach a consensus](https://about.gitlab.com/handbook/values/#collaboration-is-not-consensus))
- [ ] (If applicable) One style is getting a majority of vote (compared to the other choice)
- [ ] (If applicable) Update the MR with the chosen style
- Vote for both choices, so they are visible to others.
- [ ] The MR doesn't have significant objections, and is getting a majority of :+1: vs :-1: (remember that [we don't need to reach a consensus](https://about.gitlab.com/handbook/values/#collaboration-is-not-consensus)).
- [ ] (If applicable) One style is getting a majority of vote (compared to the other choice).
- [ ] (If applicable) Update the MR with the chosen style.
- [ ] Create a follow-up issue to fix the current offenses as a separate iteration: ISSUE_LINK
- [ ] Follow the [review process](https://docs.gitlab.com/ee/development/code_review.html) as usual
- [ ] Follow the [review process](https://docs.gitlab.com/ee/development/code_review.html) as usual.
- [ ] Once approved and merged by a maintainer, mention it again:
- [ ] In the relevant Slack channels (e.g. `#development`, `#backend`, `#frontend`)
- [ ] (Optional depending on the impact of the change) In the Engineering Week in Review
- [ ] In the relevant Slack channels (e.g. `#development`, `#backend`, `#frontend`).
- [ ] (Optional depending on the impact of the change) In the Engineering Week in Review.
/label ~"Engineering Productivity" ~"development guidelines" ~"static code analysis"
......
......@@ -9,7 +9,7 @@
<!-- Link related issues below. -->
## Check-list
## Checklist
### Pre-merge
......
......@@ -20,13 +20,13 @@ See [the general developer security release guidelines](https://gitlab.com/gitla
- [ ] Assign to a reviewer and maintainer, per our [Code Review process].
- [ ] Ensure it's approved according to our [Approval Guidelines].
- [ ] Ensure it's approved by an AppSec engineer.
- Please see the security release [Code reviews and Approvals](https://gitlab.com/gitlab-org/release/docs/blob/master/general/security/developer.md#code-reviews-and-approvals) documentation for details on which AppSec team member to ping for approval.
- Please see the security release [Code reviews and Approvals] documentation for details on which AppSec team member to ping for approval.
- Trigger the [`package-and-qa` build]. The docker image generated will be used by the AppSec engineer to validate the security vulnerability has been remediated.
- [ ] For a backport MR targeting a versioned stable branch (`X-Y-stable-ee`)
- [ ] For a backport MR targeting a versioned stable branch (`X-Y-stable-ee`).
- [ ] Milestone is set to the version this backport applies to. A closed milestone can be assigned via [quick actions].
- [ ] Ensure it's approved by a maintainer.
**Note:** Reviewer/maintainer should not be a Release Manager
**Note:** Reviewer/maintainer should not be a Release Manager.
## Maintainer checklist
......@@ -39,6 +39,7 @@ See [the general developer security release guidelines](https://gitlab.com/gitla
[quick actions]: https://docs.gitlab.com/ee/user/project/quick_actions.html#quick-actions-for-issues-merge-requests-and-epics
[CHANGELOG entry]: https://docs.gitlab.com/ee/development/changelog.html#overview
[Code Review process]: https://docs.gitlab.com/ee/development/code_review.html
[Code reviews and Approvals]: (https://gitlab.com/gitlab-org/release/docs/blob/master/general/security/developer.md#code-reviews-and-approvals)
[Approval Guidelines]: https://docs.gitlab.com/ee/development/code_review.html#approval-guidelines
[Canonical repository]: https://gitlab.com/gitlab-org/gitlab
[`package-and-qa` build]: https://docs.gitlab.com/ee/development/testing_guide/end_to_end/#using-the-package-and-qa-job
......@@ -23,7 +23,7 @@ module Integrations::Actions
format.html do
if saved
PropagateIntegrationWorker.perform_async(integration.id)
redirect_to scoped_edit_integration_path(integration), notice: success_message
redirect_to scoped_edit_integration_path(integration, project: integration.project, group: integration.group), notice: success_message
else
render 'shared/integrations/edit'
end
......
......@@ -17,31 +17,31 @@ module IntegrationsHelper
"#{event}_events"
end
def scoped_integrations_path
if @project.present?
project_settings_integrations_path(@project)
elsif @group.present?
group_settings_integrations_path(@group)
def scoped_integrations_path(project: nil, group: nil)
if project.present?
project_settings_integrations_path(project)
elsif group.present?
group_settings_integrations_path(group)
else
integrations_admin_application_settings_path
end
end
def scoped_integration_path(integration)
if @project.present?
project_service_path(@project, integration)
elsif @group.present?
group_settings_integration_path(@group, integration)
def scoped_integration_path(integration, project: nil, group: nil)
if project.present?
project_service_path(project, integration)
elsif group.present?
group_settings_integration_path(group, integration)
else
admin_application_settings_integration_path(integration)
end
end
def scoped_edit_integration_path(integration)
if @project.present?
edit_project_service_path(@project, integration)
elsif @group.present?
edit_group_settings_integration_path(@group, integration)
def scoped_edit_integration_path(integration, project: nil, group: nil)
if project.present?
edit_project_service_path(project, integration)
elsif group.present?
edit_group_settings_integration_path(group, integration)
else
edit_admin_application_settings_integration_path(integration)
end
......@@ -51,11 +51,11 @@ module IntegrationsHelper
overrides_admin_application_settings_integration_path(integration, options)
end
def scoped_test_integration_path(integration)
if @project.present?
test_project_service_path(@project, integration)
elsif @group.present?
test_group_settings_integration_path(@group, integration)
def scoped_test_integration_path(integration, project: nil, group: nil)
if project.present?
test_project_service_path(project, integration)
elsif group.present?
test_group_settings_integration_path(group, integration)
else
test_admin_application_settings_integration_path(integration)
end
......@@ -71,7 +71,7 @@ module IntegrationsHelper
end
end
def integration_form_data(integration, group: nil)
def integration_form_data(integration, project: nil, group: nil)
form_data = {
id: integration.id,
show_active: integration.show_active_box?.to_s,
......@@ -87,9 +87,9 @@ module IntegrationsHelper
inherit_from_id: integration.inherit_from_id,
integration_level: integration_level(integration),
editable: integration.editable?.to_s,
cancel_path: scoped_integrations_path,
cancel_path: scoped_integrations_path(project: project, group: group),
can_test: integration.testable?.to_s,
test_path: scoped_test_integration_path(integration),
test_path: scoped_test_integration_path(integration, project: project, group: group),
reset_path: scoped_reset_integration_path(integration, group: group)
}
......@@ -107,9 +107,9 @@ module IntegrationsHelper
}
end
def integration_list_data(integrations)
def integration_list_data(integrations, group: nil, project: nil)
{
integrations: integrations.map { |i| serialize_integration(i) }.to_json
integrations: integrations.map { |i| serialize_integration(i, group: group, project: project) }.to_json
}
end
......@@ -215,13 +215,13 @@ module IntegrationsHelper
end
end
def serialize_integration(integration)
def serialize_integration(integration, group: nil, project: nil)
{
active: integration.operating?,
title: integration.title,
description: integration.description,
updated_at: integration.updated_at,
edit_path: scoped_edit_integration_path(integration),
edit_path: scoped_edit_integration_path(integration, group: group, project: project),
name: integration.to_param
}
end
......
......@@ -16,7 +16,7 @@ module OperationsHelper
{
'prometheus_activated' => prometheus_integration.manual_configuration?.to_s,
'prometheus_form_path' => scoped_integration_path(prometheus_integration),
'prometheus_form_path' => scoped_integration_path(prometheus_integration, project: prometheus_integration.project, group: prometheus_integration.group),
'prometheus_reset_key_path' => reset_alerting_token_project_settings_operations_path(@project),
'prometheus_authorization_key' => @project.alerting_setting&.token,
'prometheus_api_url' => prometheus_integration.api_url,
......
......@@ -1614,14 +1614,8 @@ class User < ApplicationRecord
.joins(:runner)
.select('ci_runners.*')
base_and_descendants = if Feature.enabled?(:linear_user_ci_owned_runners, self, default_enabled: :yaml)
owned_groups.self_and_descendant_ids
else
Gitlab::ObjectHierarchy.new(owned_groups).base_and_descendants.select(:id)
end
group_runners = Ci::RunnerNamespace
.where(namespace_id: base_and_descendants)
.where(namespace_id: owned_groups.self_and_descendant_ids)
.joins(:runner)
.select('ci_runners.*')
......
......@@ -6,7 +6,7 @@
- if integration.operating?
= sprite_icon('check', css_class: 'gl-text-green-500')
= form_for(integration, as: :service, url: scoped_integration_path(integration), method: :put, html: { class: 'gl-show-field-errors integration-settings-form js-integration-settings-form', data: { 'test-url' => test_project_service_path(@project, integration) } }) do |form|
= form_for(integration, as: :service, url: scoped_integration_path(integration, project: @project, group: @group), method: :put, html: { class: 'gl-show-field-errors integration-settings-form js-integration-settings-form', data: { 'test-url' => test_project_service_path(@project, integration) } }) do |form|
= render 'shared/service_settings', form: form, integration: integration
%input{ id: 'services_redirect_to', type: 'hidden', name: 'redirect_to', value: request.referer }
......
......@@ -2,8 +2,8 @@
.service-settings
- if @default_integration
.js-vue-default-integration-settings{ data: integration_form_data(@default_integration, group: @group) }
.js-vue-integration-settings{ data: integration_form_data(integration, group: @group) }
.js-vue-default-integration-settings{ data: integration_form_data(@default_integration, group: @group, project: @project) }
.js-vue-integration-settings{ data: integration_form_data(integration, group: @group, project: @project) }
.js-integration-help-html.gl-display-none
-# All content below will be repositioned in Vue
- if lookup_context.template_exists?('help', "projects/services/#{integration.to_param}", true)
......
- integration = local_assigns.fetch(:integration)
= form_for integration, as: :service, url: scoped_integration_path(integration), method: :put, html: { class: 'gl-show-field-errors integration-settings-form js-integration-settings-form', data: { 'test-url' => scoped_test_integration_path(integration) } } do |form|
= form_for integration, as: :service, url: scoped_integration_path(integration, group: @group), method: :put, html: { class: 'gl-show-field-errors integration-settings-form js-integration-settings-form', data: { 'test-url' => scoped_test_integration_path(integration, group: @group) } } do |form|
= render 'shared/service_settings', form: form, integration: integration
.js-integrations-list{ data: integration_list_data(integrations) }
.js-integrations-list{ data: integration_list_data(integrations, group: @group, project: @project) }
......@@ -2,7 +2,7 @@
.tabs.gl-tabs
%div
= gl_tabs_nav({ class: 'gl-mb-5' }) do
= gl_tab_link_to _('Settings'), scoped_edit_integration_path(integration)
= gl_tab_link_to _('Settings'), scoped_edit_integration_path(integration, project: @project, group: @group)
= gl_tab_link_to s_('Integrations|Projects using custom settings'), scoped_overrides_integration_path(integration)
= yield
......
- add_to_breadcrumbs _('Integrations'), scoped_integrations_path
- add_to_breadcrumbs _('Integrations'), scoped_integrations_path(project: @project, group: @group)
- breadcrumb_title @integration.title
- page_title @integration.title, _('Integrations')
- @content_class = 'limit-container-width' unless fluid_layout
......
- add_to_breadcrumbs _('Integrations'), scoped_integrations_path
- add_to_breadcrumbs _('Integrations'), scoped_integrations_path(project: project, group: group)
- breadcrumb_title @integration.title
- page_title @integration.title, _('Integrations')
- @content_class = 'limit-container-width' unless fluid_layout
......
......@@ -4,7 +4,7 @@
= page_title
- if @project
- integrations_link_start = '<a href="%{url}">'.html_safe % { url: scoped_integrations_path }
- integrations_link_start = '<a href="%{url}">'.html_safe % { url: scoped_integrations_path(project: @project) }
%p= _("%{webhooks_link_start}%{webhook_type}%{link_end} enable you to send notifications to web applications in response to events in a group or project. We recommend using an %{integrations_link_start}integration%{link_end} in preference to a webhook.").html_safe % { webhooks_link_start: webhooks_link_start, webhook_type: hook.pluralized_name, integrations_link_start: integrations_link_start, link_end: '</a>'.html_safe }
- else
%p= _("%{webhooks_link_start}%{webhook_type}%{link_end} enable you to send notifications to web applications in response to events in a group or project.").html_safe % { webhooks_link_start: webhooks_link_start, webhook_type: hook.pluralized_name, link_end: '</a>'.html_safe }
---
name: linear_user_ci_owned_runners
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68848
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/339435
milestone: '14.6'
type: development
group: group::access
default_enabled: false
......@@ -51,12 +51,6 @@ _The uploads are stored by default in
## Using object storage **(FREE SELF)**
> **Notes:**
>
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/3867) in GitLab 10.5.
> - [Moved](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/17358) from GitLab Premium to GitLab Free in 10.7.
> - Since version 11.1, we support `direct_upload`.
If you don't want to use the local disk where GitLab is installed to store the
uploads, you can use an object storage provider like AWS S3 instead.
This configuration relies on valid AWS credentials to be configured already.
......
......@@ -40,14 +40,14 @@ Example response:
```json
[
{
"name": "master",
"name": "main",
"merged": false,
"protected": true,
"default": true,
"developers_can_push": false,
"developers_can_merge": false,
"can_push": true,
"web_url": "https://gitlab.example.com/my-group/my-project/-/tree/master",
"web_url": "https://gitlab.example.com/my-group/my-project/-/tree/main",
"commit": {
"author_email": "john@example.com",
"author_name": "John Smith",
......@@ -89,7 +89,7 @@ Parameters:
Example request:
```shell
curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/5/repository/branches/master"
curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/5/repository/branches/main"
```
Example response:
......@@ -103,7 +103,7 @@ Example response:
"developers_can_push": false,
"developers_can_merge": false,
"can_push": true,
"web_url": "https://gitlab.example.com/my-group/my-project/-/tree/master",
"web_url": "https://gitlab.example.com/my-group/my-project/-/tree/main",
"commit": {
"author_email": "john@example.com",
"author_name": "John Smith",
......@@ -151,7 +151,7 @@ Parameters:
Example request:
```shell
curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/5/repository/branches?branch=newbranch&ref=master"
curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/5/repository/branches?branch=newbranch&ref=main"
```
Example response:
......
......@@ -601,18 +601,18 @@ curl --request DELETE --header "PRIVATE-TOKEN: <your_access_token>" "https://git
Approves a pending user for a group and its subgroups and projects.
```plaintext
PUT /groups/:id/members/:user_id/approve
PUT /groups/:id/members/:member_id/approve
```
| Attribute | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `id` | integer/string | yes | The ID or [URL-encoded path of the root group](index.md#namespaced-path-encoding) owned by the authenticated user |
| `user_id` | integer | yes | The user ID of the member |
| `member_id` | integer | yes | The ID of the member |
Example request:
```shell
curl --request PUT --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/groups/:id/members/:user_id/approve"
curl --request PUT --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/groups/:id/members/:member_id/approve"
```
## Approve all pending members for a group
......
......@@ -22,9 +22,11 @@ The final disk space your jobs can use will be less than 25GB. Some disk space a
The `gitlab-shared-runners-manager-X.gitlab.com` fleet of runners are dedicated for GitLab projects as well as community forks of them. They use a slightly larger machine type (n1-standard-2) and have a bigger SSD disk size. They don't run untagged jobs and unlike the general fleet of shared runners, the instances are re-used up to 40 times.
Jobs handled by the shared runners on GitLab.com (`shared-runners-manager-X.gitlab.com`),
Jobs handled by shared runners on GitLab.com (`shared-runners-manager-X.gitlab.com`)
**time out after 3 hours**, regardless of the timeout configured in a
project. Check the issues [#4010](https://gitlab.com/gitlab-com/infrastructure/-/issues/4010) and [#4070](https://gitlab.com/gitlab-com/infrastructure/-/issues/4070) for the reference.
project. Check issue [#4010](https://gitlab.com/gitlab-com/infrastructure/-/issues/4010) and [#4070](https://gitlab.com/gitlab-com/infrastructure/-/issues/4070) for the reference.
Jobs handled by shared runners on Windows and macOS on GitLab.com **time out after 1 hour** while this service is in the Beta stage.
Below are the runners' settings.
......
......@@ -10,7 +10,7 @@ module EE
end
override :integration_form_data
def integration_form_data(integration, group: nil)
def integration_form_data(integration, project: nil, group: nil)
form_data = super
if integration.is_a?(Integrations::Jira)
......
......@@ -14,9 +14,9 @@ module Members
class ActivateService
include BaseServiceUtility
def initialize(group, user: nil, activate_all: false, current_user:)
def initialize(group, member: nil, activate_all: false, current_user:)
@group = group
@user = user
@member = member
@current_user = current_user
@activate_all = activate_all
end
......@@ -24,6 +24,7 @@ module Members
def execute
return error(_('No group provided')) unless group
return error(_('You do not have permission to approve a member'), :forbidden) unless allowed?
return error(_('No member provided')) unless activate_all || member
if activate_memberships
log_event
......@@ -36,11 +37,11 @@ module Members
private
attr_reader :current_user, :group, :user, :activate_all
attr_reader :current_user, :group, :member, :activate_all
def activate_memberships
memberships_found = false
memberships = activate_all ? awaiting_memberships : user_memberships
memberships = activate_all ? awaiting_memberships : scoped_memberships
memberships.find_each do |member|
memberships_found = true
......@@ -52,8 +53,12 @@ module Members
end
# rubocop: disable CodeReuse/ActiveRecord
def user_memberships
awaiting_memberships.where(user_id: user.id)
def scoped_memberships
if member.invite?
awaiting_memberships.where(invite_email: member.invite_email)
else
awaiting_memberships.where(user_id: member.user_id)
end
end
# rubocop: enable CodeReuse/ActiveRecord
......@@ -71,7 +76,7 @@ module Members
approved_by: current_user.id
}.tap do |params|
params[:message] = activate_all ? 'Approved all pending group members' : 'Group member access approved'
params[:user] = user.id unless activate_all
params[:member] = member.id unless activate_all
end
Gitlab::AppLogger.info(log_params)
......
......@@ -56,17 +56,18 @@ module EE
desc 'Approves a pending member'
params do
requires :user_id, type: Integer, desc: 'The user ID of the member requiring approval'
requires :member_id, type: Integer, desc: 'The ID of the member requiring approval'
end
put ':id/members/:user_id/approve' do
put ':id/members/:member_id/approve' do
group = find_group!(params[:id])
user = ::User.find(params[:user_id])
member = ::Member.find_by_id(params[:member_id])
not_found! unless member
bad_request! unless group.root?
bad_request! unless can?(current_user, :admin_group_member, group)
result = ::Members::ActivateService
.new(group, user: user, current_user: current_user)
.new(group, member: member, current_user: current_user)
.execute
if result[:status] == :success
......
......@@ -960,22 +960,24 @@ RSpec.describe API::Members do
group.add_owner(owner)
end
describe 'PUT /groups/:id/members/:user_id/approve' do
let(:url) { "/groups/#{group.id}/members/#{developer.id}/approve" }
describe 'PUT /groups/:id/members/:member_id/approve' do
let_it_be(:member) { create(:group_member, :awaiting, group: group, user: developer) }
let(:url) { "/groups/#{group.id}/members/#{member.id}/approve" }
context 'with invalid params' do
context 'when a subgroup is used' do
let(:url) { "/groups/#{subgroup.id}/members/#{developer.id}/approve" }
let(:url) { "/groups/#{subgroup.id}/members/#{member.id}/approve" }
it 'returns a bad request response' do
put api(url, not_an_owner)
put api(url, owner)
expect(response).to have_gitlab_http_status(:bad_request)
end
end
context 'when no group is found' do
let(:url) { "/groups/#{non_existing_record_id}/members/#{developer.id}/approve" }
let(:url) { "/groups/#{non_existing_record_id}/members/#{member.id}/approve" }
it 'returns a not found response' do
put api(url, owner)
......@@ -994,7 +996,7 @@ RSpec.describe API::Members do
end
context 'when the current user has permission to approve' do
context 'when the user is not found' do
context 'when the member is not found' do
let(:url) { "/groups/#{group.id}/members/#{non_existing_record_id}/approve" }
it 'returns not found response' do
......@@ -1004,7 +1006,9 @@ RSpec.describe API::Members do
end
end
context 'when the activation fails due to no members to activate' do
context 'when the activation fails due to no pending members to activate' do
let(:member) { create(:group_member, :active, group: group) }
it 'returns a bad request response' do
put api(url, owner)
......@@ -1012,38 +1016,35 @@ RSpec.describe API::Members do
end
end
context 'when the user is a pending member of a root group' do
shared_examples 'successful activation' do
it 'activates the member' do
pending_member = create(:group_member, :awaiting, group: group, user: developer)
put api(url, owner)
expect(response).to have_gitlab_http_status(:success)
expect(pending_member.reload.active?).to be true
expect(member.reload.active?).to be true
end
end
context 'when the user is a pending member of a subgroup' do
it 'activates the member' do
pending_member = create(:group_member, :awaiting, group: subgroup, user: developer)
context 'when the member is a root group member' do
it_behaves_like 'successful activation'
end
put api(url, owner)
context 'when the member is a subgroup member' do
let(:member) { create(:group_member, :awaiting, group: subgroup) }
expect(response).to have_gitlab_http_status(:success)
expect(pending_member.reload.active?).to be true
end
it_behaves_like 'successful activation'
end
context 'when the user is a pending member of a project' do
let(:developer) { create(:user) }
context 'when the member is a project member' do
let(:member) { create(:project_member, :awaiting, project: project) }
it 'activates the member' do
pending_member = create(:project_member, :awaiting, project: project, user: developer)
it_behaves_like 'successful activation'
end
put api(url, owner)
context 'when the member is an invited user' do
let(:member) { create(:group_member, :awaiting, :invited, group: group) }
expect(response).to have_gitlab_http_status(:success)
expect(pending_member.reload.active?).to be true
end
it_behaves_like 'successful activation'
end
end
end
......
......@@ -10,10 +10,11 @@ RSpec.describe Members::ActivateService do
let_it_be(:project) { create(:project, group: root_group) }
let_it_be(:sub_group) { create(:group, parent: root_group) }
let(:member) { nil }
let(:group) { root_group }
let(:activate_all) { false }
subject(:execute) { described_class.new(group, user: user, current_user: current_user, activate_all: activate_all).execute }
subject(:execute) { described_class.new(group, member: member, current_user: current_user, activate_all: activate_all).execute }
context 'when unauthorized' do
it 'returns an access error' do
......@@ -35,7 +36,7 @@ RSpec.describe Members::ActivateService do
end
end
shared_examples 'successful user activation' do
shared_examples 'successful member activation' do
before do
expect(member.awaiting?).to be true
end
......@@ -49,7 +50,7 @@ RSpec.describe Members::ActivateService do
expect(Gitlab::AppLogger).to receive(:info).with(
message: "Group member access approved",
group: group.id,
user: user.id,
member: member.id,
approved_by: current_user.id
)
......@@ -62,28 +63,39 @@ RSpec.describe Members::ActivateService do
group.add_owner(current_user)
end
context 'when activating an individual user' do
context 'when user is an awaiting member of a root group' do
it_behaves_like 'successful user activation' do
context 'when activating an individual member' do
context 'when no member is provided' do
it 'returns an error' do
result = execute
expect(result[:status]).to eq :error
expect(result[:message]).to eq 'No member provided'
end
end
context 'when member is an awaiting member of a root group' do
it_behaves_like 'successful member activation' do
let(:member) { create(:group_member, :awaiting, group: root_group, user: user) }
end
end
context 'when user is an awaiting member of a sub-group' do
context 'when member is an awaiting member of a sub-group' do
let(:group) { sub_group }
it_behaves_like 'successful user activation' do
it_behaves_like 'successful member activation' do
let(:member) { create(:group_member, :awaiting, group: sub_group, user: user) }
end
end
context 'when user is an awaiting member of a project' do
it_behaves_like 'successful user activation' do
context 'when member is an awaiting member of a project' do
it_behaves_like 'successful member activation' do
let(:member) { create(:project_member, :awaiting, project: project, user: user) }
end
end
context 'when user is not an awaiting member' do
context 'when member is not an awaiting member' do
let(:member) { create(:group_member, :active, group: root_group, user: user) }
it 'returns an error' do
result = execute
......@@ -91,6 +103,30 @@ RSpec.describe Members::ActivateService do
expect(result[:message]).to eq 'No memberships found'
end
end
context 'when there are multiple awaiting member records in the hierarchy' do
context 'for existing members' do
let_it_be(:member) { create(:group_member, :awaiting, group: root_group, user: user) }
let_it_be(:sub_member) { create(:group_member, :awaiting, group: sub_group, user: user) }
it 'activates the members' do
expect(execute[:status]).to eq :success
expect(member.reload.active?).to be true
expect(sub_member.reload.active?).to be true
end
end
context 'for invited members' do
let_it_be(:member) { create(:group_member, :awaiting, :invited, group: root_group) }
let_it_be(:sub_member) { create(:group_member, :awaiting, :invited, group: sub_group, invite_email: member.invite_email) }
it 'activates the members' do
expect(execute[:status]).to eq :success
expect(member.reload.active?).to be true
expect(sub_member.reload.active?).to be true
end
end
end
end
context 'when activating all awaiting members' do
......@@ -98,7 +134,7 @@ RSpec.describe Members::ActivateService do
let!(:sub_group_members) { create_list(:group_member, 5, :awaiting, group: sub_group) }
let!(:project_members) { create_list(:project_member, 5, :awaiting, project: project) }
let(:user) { nil }
let(:member) { nil }
let(:activate_all) { true }
it 'activates all awaiting group members' do
......
......@@ -64,45 +64,47 @@ module Gitlab
# be throttled.
#
# @param key [Symbol] Key attribute registered in `.rate_limits`
# @option scope [Array<ActiveRecord>] Array of ActiveRecord models to scope throttling to a specific request (e.g. per user per project)
# @option threshold [Integer] Optional threshold value to override default one registered in `.rate_limits`
# @option users_allowlist [Array<String>] Optional list of usernames to exclude from the limit. This param will only be functional if Scope includes a current user.
# @param scope [Array<ActiveRecord>] Array of ActiveRecord models to scope throttling to a specific request (e.g. per user per project)
# @param threshold [Integer] Optional threshold value to override default one registered in `.rate_limits`
# @param users_allowlist [Array<String>] Optional list of usernames to exclude from the limit. This param will only be functional if Scope includes a current user.
# @param peek [Boolean] Optional. When true the key will not be incremented but the current throttled state will be returned.
#
# @return [Boolean] Whether or not a request should be throttled
def throttled?(key, **options)
def throttled?(key, scope:, threshold: nil, users_allowlist: nil, peek: false)
raise InvalidKeyError unless rate_limits[key]
return if scoped_user_in_allowlist?(options)
return false if scoped_user_in_allowlist?(scope, users_allowlist)
threshold_value = options[:threshold] || threshold(key)
threshold_value > 0 &&
increment(key, options[:scope]) > threshold_value
end
threshold_value = threshold || threshold(key)
# Increments a cache key that is based on the current time and interval.
# So that when time passes to the next interval, the key changes and the count starts again from 0.
#
# Based on https://github.com/rack/rack-attack/blob/886ba3a18d13c6484cd511a4dc9b76c0d14e5e96/lib/rack/attack/cache.rb#L63-L68
#
# @param key [Symbol] Key attribute registered in `.rate_limits`
# @option scope [Array<ActiveRecord>] Array of ActiveRecord models to scope throttling to a specific request (e.g. per user per project)
#
# @return [Integer] incremented value
def increment(key, scope)
interval_value = interval(key)
return false if threshold_value == 0
interval_value = interval(key)
# `period_key` is based on the current time and interval so when time passes to the next interval
# the key changes and the rate limit count starts again from 0.
# Based on https://github.com/rack/rack-attack/blob/886ba3a18d13c6484cd511a4dc9b76c0d14e5e96/lib/rack/attack/cache.rb#L63-L68
period_key, time_elapsed_in_period = Time.now.to_i.divmod(interval_value)
cache_key = cache_key(key, scope, period_key)
cache_key = "#{action_key(key, scope)}:#{period_key}"
# We add a 1 second buffer to avoid timing issues when we're at the end of a period
expiry = interval_value - time_elapsed_in_period + 1
value = if peek
read(cache_key)
else
increment(cache_key, interval_value, time_elapsed_in_period)
end
::Gitlab::Redis::RateLimiting.with do |redis|
redis.pipelined do
redis.incr(cache_key)
redis.expire(cache_key, expiry)
end.first
end
value > threshold_value
end
# Returns the current rate limited state without incrementing the count.
#
# @param key [Symbol] Key attribute registered in `.rate_limits`
# @param scope [Array<ActiveRecord>] Array of ActiveRecord models to scope throttling to a specific request (e.g. per user per project)
# @param threshold [Integer] Optional threshold value to override default one registered in `.rate_limits`
# @param users_allowlist [Array<String>] Optional list of usernames to exclude from the limit. This param will only be functional if Scope includes a current user.
#
# @return [Boolean] Whether or not a request is currently throttled
def peek(key, scope:, threshold: nil, users_allowlist: nil)
throttled?(key, peek: true, scope: scope, threshold: threshold, users_allowlist: users_allowlist)
end
# Logs request using provided logger
......@@ -150,7 +152,28 @@ module Gitlab
action[setting] if action
end
def action_key(key, scope)
# Increments the rate limit count and returns the new count value.
def increment(cache_key, interval_value, time_elapsed_in_period)
# We add a 1 second buffer to avoid timing issues when we're at the end of a period
expiry = interval_value - time_elapsed_in_period + 1
::Gitlab::Redis::RateLimiting.with do |redis|
redis.pipelined do
redis.incr(cache_key)
redis.expire(cache_key, expiry)
end.first
end
end
# Returns the rate limit count.
# Will be 0 if there is no data in the cache.
def read(cache_key)
::Gitlab::Redis::RateLimiting.with do |redis|
redis.get(cache_key).to_i
end
end
def cache_key(key, scope, period_key)
composed_key = [key, scope].flatten.compact
serialized = composed_key.map do |obj|
......@@ -161,20 +184,20 @@ module Gitlab
end
end.join(":")
"application_rate_limiter:#{serialized}"
"application_rate_limiter:#{serialized}:#{period_key}"
end
def application_settings
Gitlab::CurrentSettings.current_application_settings
end
def scoped_user_in_allowlist?(options)
return unless options[:users_allowlist].present?
def scoped_user_in_allowlist?(scope, users_allowlist)
return unless users_allowlist.present?
scoped_user = [options[:scope]].flatten.find { |s| s.is_a?(User) }
scoped_user = [scope].flatten.find { |s| s.is_a?(User) }
return unless scoped_user
scoped_user.username.downcase.in?(options[:users_allowlist])
scoped_user.username.downcase.in?(users_allowlist)
end
end
......
......@@ -23640,6 +23640,9 @@ msgstr ""
msgid "No matching results..."
msgstr ""
msgid "No member provided"
msgstr ""
msgid "No members found"
msgstr ""
......
......@@ -2,37 +2,37 @@
require 'spec_helper'
RSpec.describe Gitlab::ApplicationRateLimiter do
RSpec.describe Gitlab::ApplicationRateLimiter, :clean_gitlab_redis_rate_limiting do
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project) }
subject { described_class }
describe '.throttled?', :clean_gitlab_redis_rate_limiting do
let(:rate_limits) do
{
test_action: {
threshold: 1,
interval: 2.minutes
},
another_action: {
threshold: 2,
interval: 3.minutes
}
let(:rate_limits) do
{
test_action: {
threshold: 1,
interval: 2.minutes
},
another_action: {
threshold: 2,
interval: 3.minutes
}
end
}
end
before do
allow(described_class).to receive(:rate_limits).and_return(rate_limits)
end
subject { described_class }
before do
allow(described_class).to receive(:rate_limits).and_return(rate_limits)
end
describe '.throttled?' do
context 'when the key is invalid' do
context 'is provided as a Symbol' do
context 'but is not defined in the rate_limits Hash' do
it 'raises an InvalidKeyError exception' do
key = :key_not_in_rate_limits_hash
expect { subject.throttled?(key) }.to raise_error(Gitlab::ApplicationRateLimiter::InvalidKeyError)
expect { subject.throttled?(key, scope: [user]) }.to raise_error(Gitlab::ApplicationRateLimiter::InvalidKeyError)
end
end
end
......@@ -42,7 +42,7 @@ RSpec.describe Gitlab::ApplicationRateLimiter do
it 'raises an InvalidKeyError exception' do
key = rate_limits.keys[0].to_s
expect { subject.throttled?(key) }.to raise_error(Gitlab::ApplicationRateLimiter::InvalidKeyError)
expect { subject.throttled?(key, scope: [user]) }.to raise_error(Gitlab::ApplicationRateLimiter::InvalidKeyError)
end
end
......@@ -50,7 +50,7 @@ RSpec.describe Gitlab::ApplicationRateLimiter do
it 'raises an InvalidKeyError exception' do
key = 'key_not_in_rate_limits_hash'
expect { subject.throttled?(key) }.to raise_error(Gitlab::ApplicationRateLimiter::InvalidKeyError)
expect { subject.throttled?(key, scope: [user]) }.to raise_error(Gitlab::ApplicationRateLimiter::InvalidKeyError)
end
end
end
......@@ -89,6 +89,17 @@ RSpec.describe Gitlab::ApplicationRateLimiter do
expect(subject.throttled?(:another_action, scope: scope)).to eq(true)
end
end
it 'allows peeking at the current state without changing its value' do
travel_to(start_time) do
expect(subject.throttled?(:test_action, scope: scope)).to eq(false)
2.times do
expect(subject.throttled?(:test_action, scope: scope, peek: true)).to eq(false)
end
expect(subject.throttled?(:test_action, scope: scope)).to eq(true)
expect(subject.throttled?(:test_action, scope: scope, peek: true)).to eq(true)
end
end
end
context 'when using ActiveRecord models as scope' do
......@@ -104,6 +115,20 @@ RSpec.describe Gitlab::ApplicationRateLimiter do
end
end
describe '.peek' do
it 'peeks at the current state without changing its value' do
freeze_time do
expect(subject.peek(:test_action, scope: [user])).to eq(false)
expect(subject.throttled?(:test_action, scope: [user])).to eq(false)
2.times do
expect(subject.peek(:test_action, scope: [user])).to eq(false)
end
expect(subject.throttled?(:test_action, scope: [user])).to eq(true)
expect(subject.peek(:test_action, scope: [user])).to eq(true)
end
end
end
describe '.log_request' do
let(:file_path) { 'master/README.md' }
let(:type) { :raw_blob_request_limit }
......
This diff is collapsed.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment