diff options
Diffstat (limited to 'doc/development')
| -rw-r--r-- | doc/development/ee_features.md | 23 | ||||
| -rw-r--r-- | doc/development/permissions/custom_roles.md | 166 | ||||
| -rw-r--r-- | doc/development/secure_coding_guidelines.md | 8 |
3 files changed, 106 insertions, 91 deletions
diff --git a/doc/development/ee_features.md b/doc/development/ee_features.md index 78177612aa9..fd00b8b86cb 100644 --- a/doc/development/ee_features.md +++ b/doc/development/ee_features.md @@ -63,6 +63,29 @@ Each SaaS feature is defined in a separate YAML file consisting of a number of f | `milestone` | no | Milestone in which the SaaS feature was created. | | `group` | no | The [group](https://about.gitlab.com/handbook/product/categories/#devops-stages) that owns the feature flag. | +#### Create a new SaaS feature file definition + +The GitLab codebase provides [`bin/saas-feature.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/bin/saas-feature.rb), +a dedicated tool to create new SaaS feature definitions. +The tool asks various questions about the new SaaS feature, then creates +a YAML definition in `ee/config/saas_features`. + +Only SaaS features that have a YAML definition file can be used when running the development or testing environments. + +```shell +❯ bin/saas-feature my_saas_feature +You picked the group 'group::acquisition' + +>> URL of the MR introducing the SaaS feature (enter to skip and let Danger provide a suggestion directly in the MR): +?> https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38602 +create ee/config/saas_features/my_saas_feature.yml +--- +name: my_saas_feature +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38602 +milestone: '16.8' +group: group::acquisition +``` + ### Opting out of a SaaS-only feature on another SaaS instance (JiHu) Prepend the `ee/lib/ee/gitlab/saas.rb` module and override the `Gitlab::Saas.feature_available?` method. diff --git a/doc/development/permissions/custom_roles.md b/doc/development/permissions/custom_roles.md index 7a0d325d09a..457fe5a5d8b 100644 --- a/doc/development/permissions/custom_roles.md +++ b/doc/development/permissions/custom_roles.md @@ -168,74 +168,87 @@ For example, you see in `GroupPolicy` that there is an ability called `read_project_security_dashboard`. You'd like to make both customizable. Rather than adding a row to the `member_roles` table for each ability, consider renaming them to `read_security_dashboard` and adding `read_security_dashboard` -to the `member_roles` table. This is more expected because it means that -enabling `read_security_dashboard` on the parent group will enable the custom role. -For example, `GroupPolicy` has an ability called `read_group_security_dashboard` and `ProjectPolicy` has an ability -called `read_project_security_dashboard`. If you would like to make both customizable, rather than adding a row to the -`member_roles` table for each ability, consider renaming them to `read_security_dashboard` and adding -`read_security_dashboard` to the `member_roles` table. This convention means that enabling `read_security_dashboard` on +to the `member_roles` table. Enabling `read_security_dashboard` on the parent group will allow the custom role to access the group security dashboard and the project security dashboard for each project in that group. Enabling the same permission on a specific project will allow access to that projects' security dashboard. ### Implement a new ability -To add a new ability to a custom role: +#### Step 1. Generate a configuration file -- Generate YAML file by running `./ee/bin/custom-ability` generator -- Add a new column to `member_roles` table, either manually or by running `custom_roles:code` generator, eg. by running `rails generate gitlab:custom_roles:code --ability new_ability_name`. The ability parameter is case sensitive and has to exactly match the permission name from the YAML file. -- Add the ability to the respective Policy for example in [this change in merge request 114734](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/114734/diffs#diff-content-edcbe28bdecbd848d4d9efdc5b5e9bddd2a7299e). -- Update the specs. Don't forget to add a spec to `ee/spec/requests/custom_roles` - the spec template file was pre-generated if you used the code generator -- Compile the documentation by running `bundle exec rake gitlab:custom_roles:compile_docs` -- Update the GraphQL documentation by running `bundle exec rake gitlab:graphql:compile_docs` - -Examples of merge requests adding new abilities to custom roles: +- Run `./ee/bin/custom-ability <ABILITY_NAME>` to generate a configuration file for the new ability. +- This will generate a YAML file in `ee/config/custom_abilities` which follows the following schema: -- [Read code](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106256) -- [Read vulnerability](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/114734) -- [Admin vulnerability](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121534) +| Field | Required | Description | +| ----- | -------- |--------------| +| `name` | yes | Unique, lowercase and underscored name describing the custom ability. Must match the filename. | +| `description` | yes | Human-readable description of the custom ability. | +| `feature_category` | yes | Name of the feature category. For example, `vulnerability_management`. | +| `introduced_by_issue` | yes | Issue URL that proposed the addition of this custom ability. | +| `introduced_by_mr` | yes | MR URL that added this custom ability. | +| `milestone` | yes | Milestone in which this custom ability was added. | +| `group_ability` | yes | Boolean value to indicate whether this ability is checked on group level. | +| `project_ability` | yes | Boolean value to whether this ability is checked on project level. | +| `requirements` | no | The list of custom permissions this ability is dependent on. For instance `admin_vulnerability` is dependent on `read_vulnerability`. If none, then enter `[]` | -The above merge requests don't use YAML files and code generators. Some of the changes are not needed anymore. We will update the documentation once we have a permission implemented using the generators. +#### Step 2: Create a migration file -If you have any concerns, put the new ability behind a feature flag. +- Run `bundle exec rails generate gitlab:custom_roles:code --ability <ABILITY_NAME>` which will generate a migration file to add the ability as a boolean column to the `member_roles` table. -#### Documenting handling the feature flag +#### Step 3: Update policies -- When you introduce a new custom ability under a feature flag, add the `feature_flag` attribute to the appropriate ability YAML file. -- When you enable the ability by default, add the `feature_flag_enabled_milestone` and `feature_flag_enabled_mr` attributes to the appropriate ability YAML file and regenerate the documentation. -- You do not have to include these attributes in the YAML file if the feature flag is enabled by default in the same release as the ability is introduced. +- If the ability is checked on a group level, add rule(s) to GroupPolicy to enable the ability. +- For example: if the ability we would like to add is `read_dependency`, then an update to `ee/app/policies/ee/group_policy.rb` would look like as follows: -#### Testing +```ruby +desc "Custom role on group that enables read dependency" +condition(:role_enables_read_dependency) do + ::Auth::MemberRoleAbilityLoader.new( + user: @user, + resource: @subject, + ability: :read_dependency + ).has_ability? +end + +rule { custom_roles_allowed & role_enables_read_dependency }.policy do + enable :read_dependency +end +``` -Unit tests are preferred to test out changes to any policies affected by the -addition of new custom permissions. Custom Roles is an Ultimate tier feature so -these tests can be found in the `ee/spec/policies` directory. The [spec file](https://gitlab.com/gitlab-org/gitlab/-/blob/13baa4e8c92a56260591a5bf0a58d3339890ee10/ee/spec/policies/project_policy_spec.rb#L2726-2740) for -the `ProjectPolicy` contains shared examples that can be used to test out the -following conditions: +- Similarly, If the ability is checked on a project level, add rule(s) to ProjectPolicy to enable the ability. +- For example: if the ability we would like to add is `read_dependency`, then an update to `ee/app/policies/ee/project_policy.rb` would look like as follows: -- when the `custom_roles` licensed feature is not enabled -- when the `custom_roles` licensed feature is enabled - - when a user is a member of a custom role via an inherited group member - - when a user is a member of a custom role via a direct group member - - when a user is a member of a custom role via a direct project membership +```ruby +desc "Custom role on project that enables read dependency" +condition(:role_enables_read_dependency) do + ::Auth::MemberRoleAbilityLoader.new( + user: @user, + resource: @subject, + ability: :read_dependency + ).has_ability? +end + +rule { custom_roles_allowed & role_enables_read_dependency }.policy do + enable :read_dependency +end +``` -Below is an example for testing out `ProjectPolicy` related changes. +- Not all abilities need to be enabled on both levels, for instance `admin_terraform_state` allows users to manage a project's terraform state. It only needs to be enabled on the project level and not the group level, and thus only needs to be configured in `ee/app/policies/ee/project_policy.rb`. -```ruby - context 'for a role with `custom_permission` enabled' do - let(:member_role_abilities) { { custom_permission: true } } - let(:allowed_abilities) { [:custom_permission] } +#### Step 4: Verify - it_behaves_like 'custom roles abilities' - end -``` +- Ensure SaaS mode is enabled with `GITLAB_SIMULATE_SAAS=1`. +- Navigate to any Group that you are an owner of, then go to `Settings -> Roles and Permissions`. +- Click on `Add new role` and create a custom role with the permission you have just created. +- Navigate to the Group's `Manage -> Members` page and assign a member to this newly created custom role. +- Next, log-in as that member and ensure that you are able to access the page that the custom ability is intended for. -Request specs are preferred to test out any endpoint that allow access via a custom role permission. -This includes controllers, REST API, and GraphQL. Examples of request specs can be found in `ee/spec/requests/custom_roles/`. In this directory you will find a sub-directory named after each permission that can be enabled via a custom role. -The `custom_roles` licensed feature must be enabled to test this functionality. +#### Step 5: Add specs -Below is an example of the typical setup that is required to test out a -Rails Controller endpoint. +- Add the ability as a trait in the `MemberRoles` factory, `ee/spec/factories/member_roles.rb`. +- Add tests to `ee/spec/requests/custom_roles/<ABILITY_NAME>/request_spec.rb` to ensure that once the user has been assigned the custom ability, they can successfully access the controllers, REST API endpoints and GraphQL API endpoints. +- Below is an example of the typical setup that is required to test a Rails Controller endpoint. ```ruby let_it_be(:user) { create(:user) } @@ -245,6 +258,7 @@ Rails Controller endpoint. before do stub_licensed_features(custom_roles: true) + sign_in(user) end @@ -260,8 +274,7 @@ Rails Controller endpoint. end ``` -Below is an example of the typical setup that is required to test out a GraphQL -mutation. +- Below is an example of the typical setup that is required to test a GraphQL mutation. ```ruby let_it_be(:user) { create(:user) } @@ -271,57 +284,36 @@ mutation. before do stub_licensed_features(custom_roles: true) + + sign_in(user) end describe MyMutation do include GraphqlHelpers describe '#show' do - it 'allows access' do - post_graphql_mutation(graphql_mutation(:my_mutation, { - example: "Example" - }), current_user: user) - - expect(response).to have_gitlab_http_status(:success) - mutation_response = graphql_mutation_response(:my_mutation) - expect(mutation_response).to be_present - expect(mutation_response["errors"]).to be_empty - end + let(:mutation) { graphql_mutation(:my_mutation) } + + it_behaves_like 'a working graphql query' end end ``` -[`GITLAB_DEBUG_POLICIES=true`](#finding-existing-abilities-checks) can be used -to troubleshoot runtime policy decisions. - -## Custom abilities definition - -All new custom abilities must have a type definition stored in `ee/config/custom_abilities` that contains a single source of truth for every ability that is part of custom roles feature. +- Add tests to `ProjectPolicy` and/or `GroupPolicy`. Below is an example for testing `ProjectPolicy` related changes. -### Add a new custom ability definition - -To add a new custom ability: +```ruby + context 'for a member role with read_dependency true' do + let(:member_role_abilities) { { read_dependency: true } } + let(:allowed_abilities) { [:read_dependency] } -1. Create the YAML definition. You can either: - - Use the `ee/bin/custom-ability` CLI to create the YAML definition automatically. - - Perform manual steps to create a new file in `ee/config/custom_abilities/` with the filename matching the name of the ability name. -1. Add contents to the file that conform to the [schema](#schema) defined in `ee/config/custom_abilities/types/type_schema.json`. -1. Add [tests](#testing) for the new ability in `ee/spec/requests/custom_roles/` with a new directory named after the ability name. + it_behaves_like 'custom roles abilities' + end +``` -### Schema +#### Step 6: Update documentation -| Field | Required | Description | -| ----- | -------- |--------------| -| `name` | yes | Unique, lowercase and underscored name describing the custom ability. Must match the filename. | -| `description` | yes | Human-readable description of the custom ability. | -| `feature_category` | yes | Name of the feature category. For example, `vulnerability_management`. | -| `introduced_by_issue` | yes | Issue URL that proposed the addition of this custom ability. | -| `introduced_by_mr` | no | MR URL that added this custom ability. | -| `milestone` | yes | Milestone in which this custom ability was added. | -| `group_ability` | yes | Indicate whether this ability is checked on group level. | -| `project_ability` | yes | Indicate whether this ability is checked on project level. | -| `requirements` | no | The custom abilities that need to be enabled for this ability. | -| `skip_seat_consumption` | yes | Indicate wheter this ability should be skiped when counting licensed users. | +- Update the list of custom abilities by running `bundle exec rake gitlab:custom_roles:compile_docs` +- Update the GraphQL documentation by running `bundle exec rake gitlab:graphql:compile_docs` ### Privilege escalation consideration diff --git a/doc/development/secure_coding_guidelines.md b/doc/development/secure_coding_guidelines.md index d8fad6deb9c..75df9a67aff 100644 --- a/doc/development/secure_coding_guidelines.md +++ b/doc/development/secure_coding_guidelines.md @@ -258,7 +258,7 @@ the mitigations for a new feature. #### URL blocker & validation libraries -[`Gitlab::UrlBlocker`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/url_blocker.rb) can be used to validate that a +[`Gitlab::HTTP_V2::UrlBlocker`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/gems/gitlab-http/lib/gitlab/http_v2/url_blocker.rb) can be used to validate that a provided URL meets a set of constraints. Importantly, when `dns_rebind_protection` is `true`, the method returns a known-safe URI where the hostname has been replaced with an IP address. This prevents DNS rebinding attacks, because the DNS record has been resolved. However, if we ignore this returned value, we **will not** be protected against DNS rebinding. @@ -1234,7 +1234,7 @@ These types of bugs are often seen in environments which allow multi-threading a **Example 1:** you have a model which accepts a URL as input. When the model is created you verify that the URL host resolves to a public IP address, to prevent attackers making internal network calls. But DNS records can change ([DNS rebinding](#server-side-request-forgery-ssrf)]). An attacker updates the DNS record to `127.0.0.1`, and when your code resolves those URL host it results in sending a potentially malicious request to a server on the internal network. The property was valid at the "time of check", but invalid and malicious at "time of use". -GitLab-specific example can be found in [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/214401) where, although `Gitlab::UrlBlocker.validate!` was called, the returned value was not used. This made it vulnerable to TOCTOU bug and SSRF protection bypass through [DNS rebinding](#server-side-request-forgery-ssrf). The fix was to [use the validated IP address](https://gitlab.com/gitlab-org/gitlab/-/commit/7af8abd4df9a98f7a1ae7c4ec9840d0a7a8c684d). +GitLab-specific example can be found in [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/214401) where, although `Gitlab::HTTP_V2::UrlBlocker.validate!` was called, the returned value was not used. This made it vulnerable to TOCTOU bug and SSRF protection bypass through [DNS rebinding](#server-side-request-forgery-ssrf). The fix was to [use the validated IP address](https://gitlab.com/gitlab-org/gitlab/-/commit/85c6a73598e72ab104ab29b72bf83661cd961646). **Example 2:** you have a feature which schedules jobs. When the user schedules the job, they have permission to do so. But imagine if, between the time they schedule the job and the time it is run, their permissions are restricted. Unless you re-check permissions at time of use, you could inadvertently allow unauthorized activity. @@ -1264,9 +1264,9 @@ end - Use your framework's validations and database features to impose constraints and atomic reads and writes. - Read about [Server Side Request Forgery (SSRF) and DNS rebinding](#server-side-request-forgery-ssrf) -An example of well implemented `Gitlab::UrlBlocker.validate!` call that prevents TOCTOU bug: +An example of well implemented `Gitlab::HTTP_V2::UrlBlocker.validate!` call that prevents TOCTOU bug: -1. [Preventing DNS rebinding in Gitea importer](https://gitlab.com/gitlab-org/gitlab/-/commit/7af8abd4df9a98f7a1ae7c4ec9840d0a7a8c684d) +1. [Preventing DNS rebinding in Gitea importer](https://gitlab.com/gitlab-org/gitlab/-/commit/85c6a73598e72ab104ab29b72bf83661cd961646) ### Resources |