diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-04-21 02:50:22 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-04-21 02:50:22 +0300 |
commit | 9dc93a4519d9d5d7be48ff274127136236a3adb3 (patch) | |
tree | 70467ae3692a0e35e5ea56bcb803eb512a10bedb /doc/development/permissions.md | |
parent | 4b0f34b6d759d6299322b3a54453e930c6121ff0 (diff) |
Add latest changes from gitlab-org/gitlab@13-11-stable-eev13.11.0-rc43
Diffstat (limited to 'doc/development/permissions.md')
-rw-r--r-- | doc/development/permissions.md | 28 |
1 files changed, 28 insertions, 0 deletions
diff --git a/doc/development/permissions.md b/doc/development/permissions.md index 35f0941b756..2af451840d6 100644 --- a/doc/development/permissions.md +++ b/doc/development/permissions.md @@ -120,3 +120,31 @@ into different features like Merge Requests and CI flow. | View | Vulnerability feedback | Merge Request | Can read security findings | | View | Dependency List page | Project | Can access Dependency information | | View | License Compliance page | Project | Can access License information| + +## Where should permissions be checked? + +By default, controllers, API endpoints, and GraphQL types/fields are responsible for authorization. See [Secure Coding Guidelines > Permissions](secure_coding_guidelines.md#permissions). + +### Considerations + +- Many actions are completely or partially extracted to services, finders, and other classes, so it is normal to do permission checks "downstream". +- Often, authorization logic must be incorporated in DB queries to filter records. +- `DeclarativePolicy` rules are relatively performant, but conditions may perform database calls. +- Multiple permission checks across layers can be difficult to reason about, which is its own security risk. For example, duplicate authorization logic could diverge. +- Should we apply defense-in-depth with permission checks? [Join the discussion](https://gitlab.com/gitlab-org/gitlab/-/issues/324135) + +### Tips + +If a class accepts `current_user`, then it may be responsible for authorization. + +### Example: Adding a new API endpoint + +By default, we authorize at the endpoint. Checking an existing ability may make sense; if not, then we probably need to add one. + +As an aside, most endpoints can be cleanly categorized as a CRUD (create, read, update, destroy) action on a resource. The services and abilities follow suit, which is why many are named like `Projects::CreateService` or `:read_project`. + +Say, for example, we extract the whole endpoint into a service. The `can?` check will now be in the service. Say the service reuses an existing finder, which we are modifying for our purposes. Should we make the finder check an ability? + +- If the finder doesn't accept `current_user`, and therefore doesn't check permissions, then probably no. +- If the finder accepts `current_user`, and doesn't check permissions, then it would be a good idea to double check other usages of the finder, and we might consider adding authorization. +- If the finder accepts `current_user`, and already checks permissions, then either we need to add our case, or the existing checks are appropriate. |