Commit 61e05942 authored by Shinya Maeda's avatar Shinya Maeda Committed by Igor Drozdov

Fix authorization callers for the Release create, update and delete actions

parent f4c43eac
......@@ -33,6 +33,10 @@ module Mutations
return { link: nil, errors: [message] }
end
unless Ability.allowed?(current_user, :update_release, release)
raise_resource_not_available_error!
end
new_link = release.links.create(link_attrs)
unless new_link.persisted?
......
......@@ -158,6 +158,10 @@ class ProjectPolicy < BasePolicy
::Feature.enabled?(:build_service_proxy, @subject)
end
condition(:respect_protected_tag_for_release_permissions) do
::Feature.enabled?(:evalute_protected_tag_for_release_permissions, @subject, default_enabled: :yaml)
end
condition(:user_defined_variables_allowed) do
!@subject.restrict_user_defined_variables?
end
......@@ -649,6 +653,10 @@ class ProjectPolicy < BasePolicy
rule { build_service_proxy_enabled }.enable :build_service_proxy_enabled
rule { respect_protected_tag_for_release_permissions & can?(:developer_access) }.policy do
enable :destroy_release
end
rule { can?(:download_code) }.policy do
enable :read_repository_graphs
end
......
......@@ -13,21 +13,9 @@ class ReleasePolicy < BasePolicy
::Feature.enabled?(:evalute_protected_tag_for_release_permissions, @subject.project, default_enabled: :yaml)
end
condition(:project_developer) do
can?(:developer_access, @subject.project)
end
rule { respect_protected_tag & protected_tag }.policy do
prevent :create_release
prevent :update_release
prevent :destroy_release
end
# NOTE: Developer role (or above) can create, update and destroy release entries.
# When we remove the `evalute_protected_tag_for_release_permissions` feature flag,
# we should move `enable :destroy_release` to ProjectPolicy alongside with .
# See https://gitlab.com/gitlab-org/gitlab/-/issues/327505 for more information.
rule { respect_protected_tag & project_developer }.policy do
enable :destroy_release
end
end
......@@ -2,6 +2,6 @@
module Releases
class LinkPolicy < BasePolicy
delegate { @subject.release.project }
delegate { @subject.release }
end
end
......@@ -44,7 +44,13 @@ module Releases
end
def allowed?
Ability.allowed?(current_user, :create_release, project)
Ability.allowed?(current_user, :create_release, project) && can_create_tag?
end
def can_create_tag?
return true unless ::Feature.enabled?(:evalute_protected_tag_for_release_permissions, project, default_enabled: :yaml)
::Gitlab::UserAccess.new(current_user, container: project).can_create_tag?(tag_name)
end
def create_release(tag, evidence_pipeline)
......
......@@ -12,6 +12,8 @@ info: To determine the technical writer assigned to the Stage/Group associated w
> - Release Evidences were [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/26019) in GitLab 12.5.
> - `description_html` became an opt-in field [with GitLab 13.12 for performance reasons](https://gitlab.com/gitlab-org/gitlab/-/issues/299447).
Please pass the `include_html_description` query string parameter if you need it.
> - [The permission model for create, update and delete actions was fixed](https://gitlab.com/gitlab-org/gitlab/-/issues/327505) in GitLab 14.1.
See [Release permissions](../../user/project/releases/index.md#release-permissions) for more information.
## List Releases
......
......@@ -105,8 +105,7 @@ The following table lists project permissions available for each role:
| Publish [packages](packages/index.md) | | | ✓ | ✓ | ✓ |
| Create/edit/delete a Cleanup policy | | | ✓ | ✓ | ✓ |
| Upload [Design Management](project/issues/design_management.md) files | | | ✓ | ✓ | ✓ |
| Create/edit [releases](project/releases/index.md)| | | ✓ | ✓ | ✓ |
| Delete [releases](project/releases/index.md)| | | | ✓ | ✓ |
| Create/edit/delete [releases](project/releases/index.md)| | | ✓ (*13*) | ✓ (*13*) | ✓ (*13*) |
| Manage merge approval rules (project settings) | | | | ✓ | ✓ |
| Create new merge request | | | ✓ | ✓ | ✓ |
| Create new branches | | | ✓ | ✓ | ✓ |
......@@ -205,6 +204,7 @@ The following table lists project permissions available for each role:
1. Users can only view events based on their individual actions.
1. Project access tokens are supported for self-managed instances on Free and above. They are also
supported on GitLab SaaS Premium and above (excluding [trial licenses](https://about.gitlab.com/free-trial/)).
1. If the [tag is protected](#release-permissions-with-protected-tags), this depends on the access Developers and Maintainers are given.
## Project features permissions
......@@ -521,6 +521,14 @@ run CI/CD pipelines and execute actions on jobs that are related to those branch
See [Security on protected branches](../ci/pipelines/index.md#pipeline-security-on-protected-branches)
for details about the pipelines security model.
## Release permissions with protected tags
[The permission to create tags](project/protected_tags.md) is used to define if a user can
create, edit, and delete [Releases](project/releases/index.md).
See [Release permissions](project/releases/index.md#release-permissions)
for more information.
## LDAP users permissions
In GitLab 8.15 and later, LDAP user permissions can now be manually overridden by an admin user.
......
......@@ -63,7 +63,7 @@ We recommend using the API to create releases as one of the last steps in your
CI/CD pipeline.
Only users with Developer permissions or higher can create releases.
Read more about [Release permissions](../../../user/permissions.md#project-members-permissions).
Read more about [Release permissions](#release-permissions).
To create a new release through the GitLab UI:
......@@ -103,7 +103,7 @@ release tag. When the `released_at` date and time has passed, the badge is autom
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/26016) in GitLab 12.6. Asset link editing was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/9427) in GitLab 12.10.
Only users with Developer permissions or higher can edit releases.
Read more about [Release permissions](../../../user/permissions.md#project-members-permissions).
Read more about [Release permissions](#release-permissions).
To edit the details of a release:
......@@ -245,7 +245,7 @@ but are _not_ allowed to view details about the Git repository (in particular,
tag names). Because of this, release titles are replaced with a generic
title like "Release-1234" for Guest users to avoid leaking tag name information.
See the [Permissions](../../permissions.md#project-members-permissions) page for
See the [Release permissions](#release-permissions) section for
more information about permissions.
### Tag name
......@@ -565,6 +565,49 @@ In the API:
- If you do not specify a `released_at` date, release evidence is collected on the
date the release is created.
## Release permissions
> [The permission model for create, update and delete actions was fixed](https://gitlab.com/gitlab-org/gitlab/-/issues/327505) in GitLab 14.1.
### View a release and download assets
- Users with [Reporter role or above](../../../user/permissions.md#project-members-permissions)
have read and download access to the project releases.
- Users with [Guest role](../../../user/permissions.md#project-members-permissions)
have read and download access to the project releases, however,
repository-related information are redacted (for example the Git tag name).
### Create, update, and delete a release and its assets
- Users with [Developer role or above](../../../user/permissions.md#project-members-permissions)
have write access to the project releases and assets.
- If a release is associated with a [protected tag](../protected_tags.md),
the user must be [allowed to create the protected tag](../protected_tags.md#configuring-protected-tags) too.
As an example of release permission control, you can allow only
[Maintainer role or above](../../../user/permissions.md#project-members-permissions)
to create, update, and delete releases by protecting the tag with a wildcard (`*`),
and set **Maintainer** in the **Allowed to create** column.
#### Enable or disable protected tag evaluation on releases **(FREE SELF)**
Protected tag evaluation on release permissions is under development and not ready for production use.
It is deployed behind a feature flag that is **disabled by default**.
[GitLab administrators with access to the GitLab Rails console](../../../administration/feature_flags.md)
can enable it.
To enable it:
```ruby
Feature.enable(:evalute_protected_tag_for_release_permissions)
```
To disable it:
```ruby
Feature.disable(:evalute_protected_tag_for_release_permissions)
```
## Release Command Line
> [Introduced](https://gitlab.com/gitlab-org/release-cli/-/merge_requests/6) in GitLab 12.10.
......@@ -588,14 +631,13 @@ These metrics include:
- Total number of releases in the group
- Percentage of projects in the group that have at least one release
<!-- ## Troubleshooting
## Troubleshooting
### Getting `403 Forbidden` or `Something went wrong while creating a new release` errors when creating, updating or deleting releases and their assets
Include any troubleshooting steps that you can foresee. If you know beforehand what issues
one might have when setting this up, or when something is changed, or on upgrading, it's
important to describe those, too. Think of things that may go wrong and include them here.
This is important to minimize requests for support, and to avoid doc comments with
questions that you know someone might ask.
If the release is associted with a [protected tag](../protected_tags.md),
the UI/API request might result in an authorization failure.
Make sure that the user or a service/bot account is allowed to
[create the protected tag](../protected_tags.md#configuring-protected-tags) too.
Each scenario can be a third-level heading, e.g. `### Getting error message X`.
If you have none to add when creating a doc, leave this section in place
but commented out to help encourage others to add to it in the future. -->
See [the release permissions](#release-permissions) for more information.
......@@ -50,6 +50,24 @@ RSpec.describe Mutations::ReleaseAssetLinks::Create do
end
end
context 'with protected tag' do
context 'when user has access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :developers_can_create, name: '*', project: project) }
it 'does not have errors' do
expect(subject).to include(errors: [])
end
end
context 'when user does not have access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :maintainers_can_create, name: '*', project: project) }
it 'has an access error' do
expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
end
end
context "when the user doesn't have access to the project" do
let(:current_user) { reporter }
......
......@@ -7,6 +7,7 @@ RSpec.describe Mutations::ReleaseAssetLinks::Delete do
let_it_be(:project) { create(:project, :private, :repository) }
let_it_be_with_reload(:release) { create(:release, project: project) }
let_it_be(:reporter) { create(:user).tap { |u| project.add_reporter(u) } }
let_it_be(:developer) { create(:user).tap { |u| project.add_developer(u) } }
let_it_be(:maintainer) { create(:user).tap { |u| project.add_maintainer(u) } }
let_it_be_with_reload(:release_link) { create(:release_link, release: release) }
......@@ -22,7 +23,7 @@ RSpec.describe Mutations::ReleaseAssetLinks::Delete do
let(:deleted_link) { subject[:link] }
context 'when the current user has access to delete the link' do
let(:current_user) { maintainer }
let(:current_user) { developer }
it 'deletes the link and returns it', :aggregate_failures do
expect(deleted_link).to eq(release_link)
......@@ -30,6 +31,26 @@ RSpec.describe Mutations::ReleaseAssetLinks::Delete do
expect(release.links).to be_empty
end
context 'with protected tag' do
context 'when user has access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :developers_can_create, name: '*', project: project) }
it 'does not have errors' do
subject
expect(resolve).to include(errors: [])
end
end
context 'when user does not have access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :maintainers_can_create, name: '*', project: project) }
it 'raises a resource access error' do
expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
end
end
context "when the link doesn't exist" do
let(:mutation_arguments) { super().merge(id: "gid://gitlab/Releases::Link/#{non_existing_record_id}") }
......@@ -48,7 +69,7 @@ RSpec.describe Mutations::ReleaseAssetLinks::Delete do
end
context 'when the current user does not have access to delete the link' do
let(:current_user) { developer }
let(:current_user) { reporter }
it 'raises an error' do
expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
......
......@@ -87,6 +87,26 @@ RSpec.describe Mutations::ReleaseAssetLinks::Update do
end
it_behaves_like 'no changes to the link except for the', :name
context 'with protected tag' do
context 'when user has access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :developers_can_create, name: '*', project: project) }
it 'does not have errors' do
subject
expect(resolve).to include(errors: [])
end
end
context 'when user does not have access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :maintainers_can_create, name: '*', project: project) }
it 'raises a resource access error' do
expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
end
end
end
context 'when nil is provided' do
......
......@@ -117,6 +117,28 @@ RSpec.describe Mutations::Releases::Create do
expect(new_link.filepath).to eq(expected_link[:filepath])
end
end
context 'with protected tag' do
context 'when user has access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :developers_can_create, name: '*', project: project) }
it 'does not have errors' do
subject
expect(resolve).to include(errors: [])
end
end
context 'when user does not have access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :maintainers_can_create, name: '*', project: project) }
it 'has an access error' do
subject
expect(resolve).to include(errors: ['Access Denied'])
end
end
end
end
context "when the current user doesn't have access to create releases" do
......
......@@ -5,6 +5,7 @@ require 'spec_helper'
RSpec.describe Mutations::Releases::Delete do
let_it_be(:project) { create(:project, :public, :repository) }
let_it_be(:non_project_member) { create(:user) }
let_it_be(:reporter) { create(:user) }
let_it_be(:developer) { create(:user) }
let_it_be(:maintainer) { create(:user) }
let_it_be(:tag) { 'v1.1.0'}
......@@ -20,6 +21,7 @@ RSpec.describe Mutations::Releases::Delete do
end
before do
project.add_reporter(reporter)
project.add_developer(developer)
project.add_maintainer(maintainer)
end
......@@ -36,7 +38,7 @@ RSpec.describe Mutations::Releases::Delete do
end
context 'when the current user has access to create releases' do
let(:current_user) { maintainer }
let(:current_user) { developer }
it 'deletes the release' do
expect { subject }.to change { Release.count }.by(-1)
......@@ -54,6 +56,28 @@ RSpec.describe Mutations::Releases::Delete do
expect(subject[:errors]).to eq([])
end
context 'with protected tag' do
context 'when user has access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :developers_can_create, name: '*', project: project) }
it 'does not have errors' do
subject
expect(resolve).to include(errors: [])
end
end
context 'when user does not have access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :maintainers_can_create, name: '*', project: project) }
it 'has an access error' do
subject
expect(resolve).to include(errors: ['Access Denied'])
end
end
end
context 'validation' do
context 'when the release does not exist' do
let(:mutation_arguments) { super().merge(tag: 'not-a-real-release') }
......@@ -76,8 +100,8 @@ RSpec.describe Mutations::Releases::Delete do
end
context "when the current user doesn't have access to update releases" do
context 'when the user is a developer' do
let(:current_user) { developer }
context 'when the user is a reporter' do
let(:current_user) { reporter }
it_behaves_like 'unauthorized or not found error'
end
......
......@@ -107,6 +107,28 @@ RSpec.describe Mutations::Releases::Update do
end
it_behaves_like 'no changes to the release except for the', :name
context 'with protected tag' do
context 'when user has access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :developers_can_create, name: '*', project: project) }
it 'does not have errors' do
subject
expect(resolve).to include(errors: [])
end
end
context 'when user does not have access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :maintainers_can_create, name: '*', project: project) }
it 'has an access error' do
subject
expect(resolve).to include(errors: ['Access Denied'])
end
end
end
end
context 'when nil is provided' do
......
......@@ -55,7 +55,7 @@ RSpec.describe 'Deleting a release' do
end
context 'when the current user has access to update releases' do
let(:current_user) { maintainer }
let(:current_user) { developer }
it 'deletes the release' do
expect { delete_release }.to change { Release.count }.by(-1)
......@@ -105,12 +105,6 @@ RSpec.describe 'Deleting a release' do
end
context "when the current user doesn't have access to update releases" do
context 'when the current user is a Developer' do
let(:current_user) { developer }
it_behaves_like 'unauthorized or not found error'
end
context 'when the current user is a Reporter' do
let(:current_user) { reporter }
......
......@@ -5,6 +5,7 @@ require 'spec_helper'
RSpec.describe API::Release::Links do
let(:project) { create(:project, :repository, :private) }
let(:maintainer) { create(:user) }
let(:developer) { create(:user) }
let(:reporter) { create(:user) }
let(:non_project_member) { create(:user) }
let(:commit) { create(:commit, project: project) }
......@@ -18,6 +19,7 @@ RSpec.describe API::Release::Links do
before do
project.add_maintainer(maintainer)
project.add_developer(developer)
project.add_reporter(reporter)
project.repository.add_tag(maintainer, 'v0.1', commit.id)
......@@ -196,6 +198,28 @@ RSpec.describe API::Release::Links do
expect(response).to match_response_schema('release/link')
end
context 'with protected tag' do
context 'when user has access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :developers_can_create, name: '*', project: project) }
it 'accepts the request' do
post api("/projects/#{project.id}/releases/v0.1/assets/links", developer), params: params
expect(response).to have_gitlab_http_status(:created)
end
end
context 'when user does not have access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :maintainers_can_create, name: '*', project: project) }
it 'forbids the request' do
post api("/projects/#{project.id}/releases/v0.1/assets/links", developer), params: params
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end
context 'when name is empty' do
let(:params) do
{
......@@ -290,6 +314,28 @@ RSpec.describe API::Release::Links do
expect(response).to match_response_schema('release/link')
end
context 'with protected tag' do
context 'when user has access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :developers_can_create, name: '*', project: project) }
it 'accepts the request' do
put api("/projects/#{project.id}/releases/v0.1/assets/links/#{release_link.id}", developer), params: params
expect(response).to have_gitlab_http_status(:ok)
end
end
context 'when user does not have access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :maintainers_can_create, name: '*', project: project) }
it 'forbids the request' do
put api("/projects/#{project.id}/releases/v0.1/assets/links/#{release_link.id}", developer), params: params
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end
context 'when params is empty' do
let(:params) { {} }
......@@ -365,6 +411,28 @@ RSpec.describe API::Release::Links do
expect(response).to match_response_schema('release/link')
end
context 'with protected tag' do
context 'when user has access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :developers_can_create, name: '*', project: project) }
it 'accepts the request' do
delete api("/projects/#{project.id}/releases/v0.1/assets/links/#{release_link.id}", developer)
expect(response).to have_gitlab_http_status(:ok)
end
end
context 'when user does not have access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :maintainers_can_create, name: '*', project: project) }
it 'forbids the request' do
delete api("/projects/#{project.id}/releases/v0.1/assets/links/#{release_link.id}", developer)
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end
context 'when there are no corresponding release link' do
let!(:release_link) { }
......
......@@ -676,6 +676,28 @@ RSpec.describe API::Releases do
end.not_to change { Project.find_by_id(project.id).repository.tag_count }
end
context 'with protected tag' do
context 'when user has access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :developers_can_create, name: '*', project: project) }
it 'accepts the request' do
post api("/projects/#{project.id}/releases", developer), params: params
expect(response).to have_gitlab_http_status(:created)
end
end
context 'when user does not have access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :maintainers_can_create, name: '*', project: project) }
it 'forbids the request' do
post api("/projects/#{project.id}/releases", developer), params: params
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end
context 'when user is a reporter' do
it 'forbids the request' do
post api("/projects/#{project.id}/releases", reporter), params: params
......@@ -1014,6 +1036,28 @@ RSpec.describe API::Releases do
expect(project.releases.last.released_at).to eq('2015-10-10T05:00:00Z')
end
context 'with protected tag' do
context 'when user has access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :developers_can_create, name: '*', project: project) }
it 'accepts the request' do
put api("/projects/#{project.id}/releases/v0.1", developer), params: params
expect(response).to have_gitlab_http_status(:ok)
end
end
context 'when user does not have access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :maintainers_can_create, name: '*', project: project) }
it 'forbids the request' do
put api("/projects/#{project.id}/releases/v0.1", developer), params: params
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end
context 'when user tries to update sha' do
let(:params) { { sha: 'xxx' } }
......@@ -1194,6 +1238,28 @@ RSpec.describe API::Releases do
expect(response).to match_response_schema('public_api/v4/release')
end
context 'with protected tag' do
context 'when user has access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :developers_can_create, name: '*', project: project) }
it 'accepts the request' do
delete api("/projects/#{project.id}/releases/v0.1", developer)
expect(response).to have_gitlab_http_status(:ok)
end
end
context 'when user does not have access to the protected tag' do
let!(:protected_tag) { create(:protected_tag, :maintainers_can_create, name: '*', project: project) }
it 'forbids the request' do
delete api("/projects/#{project.id}/releases/v0.1", developer)
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end
context 'when there are no corresponding releases' do
let!(:release) { }
......
......@@ -48,7 +48,7 @@ RSpec.shared_context 'ProjectPolicy context' do
destroy_container_image push_code read_pod_logs read_terraform_state
resolve_note update_build update_commit_status update_container_image
update_deployment update_environment update_merge_request
update_metrics_dashboard_annotation update_pipeline update_release
update_metrics_dashboard_annotation update_pipeline update_release destroy_release
]
end
......@@ -57,7 +57,7 @@ RSpec.shared_context 'ProjectPolicy context' do
add_cluster admin_build admin_commit_status admin_container_image
admin_deployment admin_environment admin_note admin_pipeline
admin_project admin_project_member admin_snippet admin_terraform_state
admin_wiki create_deploy_token destroy_deploy_token destroy_release
admin_wiki create_deploy_token destroy_deploy_token
push_to_delete_protected_branch read_deploy_token update_snippet
]
end
......
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