List only the access levels the API accepts

José M. Requena Plensrequested to merge
gitlab-community/gitlab-org/gitlab:jmrp-access-level-values-the-api-refuses into master

What does this MR do?

Several API pages list access level values that the API refuses. This MR removes them, says where 5 (Minimal access) is accepted, and drops a default that does not apply.

Filters. The min_access_level and shared_min_access_level rows on the projects, groups, forks, starring and token association pages (14 rows) list 5 (Minimal access). Every one of those routes declares values: Gitlab::Access.all_values, which is 10 to 50 in every edition, so 5 is refused with min_access_level does not have a valid value. I checked all 14 routes on GitLab.com: 5 is refused and 25 is accepted on each. The OpenAPI spec already lists 10 to 50 for all 14 parameters.

Writes. I ran the member, invitation, access request, share, LDAP group link and SAML group link routes in request specs with 0 and 5, and the member, invitation and access request routes with 60 too, on CE (FOSS_ONLY=1) and on EE with the minimal_access_role feature licensed and unlicensed, on a top-level group, a subgroup and a project. What changed:

  • 0 (No access) is removed from every write row. No route accepts it.
  • 60 (Admin) is removed from the group member update row. The model refuses it.
  • 5 (Minimal access) is removed from the project share row, the project member rows and the invitation update row. No edition accepts it there.
  • For adding and updating group members, adding invitations, approving access requests and sharing groups, the rows now say that 5 is valid on Premium and Ultimate when id is a top-level group. That is the only case that accepted it.
  • For SAML group links, the request row says the same (the page is already Premium and Ultimate). The response rows keep 5, since a link on a top-level group can hold it, and lose 0, which no link can hold.
  • LDAP group links accepted 5 on a top-level group and on a subgroup, with and without the minimal_access_role feature, so that row keeps it and only loses 0.
  • Default: 30 is removed from the required access_level rows (add and update a member, add an invitation). Leaving access_level out is a 400 access_level is missing.
  • Updating an invitation without access_level keeps the invitation's current level, not 30, so that row now says so.
  • The access request approval row keeps Default: 30, which is correct.

This MR implements no issue. I found these rows while checking the values my GitLab MCP server sends against what the API accepts.

How I checked the filter rows

  • Code, at master d77ac9dd142: lib/api/projects.rb (filter_params, which collection_params uses, GET /projects/:id/groups and GET /projects/:id/invited_groups), lib/api/groups.rb (group_list_params, GET /groups/:id/groups/shared, GET /groups/:id/invited_groups, GET /groups/:id/projects, GET /groups/:id/projects/shared) and lib/api/personal_access_tokens/self_information.rb declare values: Gitlab::Access.all_values. Gitlab::Access.all_values is options_with_owner.values, 10 to 50, and EE does not override it (EE overrides values_with_minimal_access instead).

  • GitLab.com, read only: for each route, min_access_level=5 (or shared_min_access_level=5) answered {"error":"min_access_level does not have a valid value"} and 25 answered a list:

    GET /api/v4/projects?min_access_level=5                              -> {"error":"min_access_level does not have a valid value"}
GET /api/v4/projects?min_access_level=25                             -> a list of projects
GET /api/v4/users/:user_id/projects?min_access_level=5               -> {"error":"min_access_level does not have a valid value"}
GET /api/v4/projects/:id/groups?shared_min_access_level=5            -> {"error":"shared_min_access_level does not have a valid value"}
GET /api/v4/projects/:id/invited_groups?min_access_level=5           -> {"error":"min_access_level does not have a valid value"}
GET /api/v4/groups?min_access_level=5                                -> {"error":"min_access_level does not have a valid value"}
GET /api/v4/groups/:id/projects?min_access_level=5                   -> {"error":"min_access_level does not have a valid value"}
GET /api/v4/groups/:id/projects/shared?min_access_level=5            -> {"error":"min_access_level does not have a valid value"}
GET /api/v4/groups/:id/subgroups?min_access_level=5                  -> {"error":"min_access_level does not have a valid value"}
GET /api/v4/groups/:id/descendant_groups?min_access_level=5          -> {"error":"min_access_level does not have a valid value"}
GET /api/v4/groups/:id/groups/shared?min_access_level=5              -> {"error":"min_access_level does not have a valid value"}
GET /api/v4/groups/:id/invited_groups?min_access_level=5             -> {"error":"min_access_level does not have a valid value"}
GET /api/v4/projects/:id/forks?min_access_level=5                    -> {"error":"min_access_level does not have a valid value"}
GET /api/v4/users/:user_id/starred_projects?min_access_level=5       -> {"error":"min_access_level does not have a valid value"}
GET /api/v4/personal_access_tokens/self/associations?min_access_level=5 -> {"error":"min_access_level does not have a valid value"}

    The same 14 routes answered 25 with a list (the token associations route with its groups and projects object).

  • OpenAPI: all 14 min_access_level and shared_min_access_level parameters in doc/api/openapi/openapi_v3.yaml have the enum 10, 15, 20, 25, 30, 40, 50.

How I checked the write rows

I wrote throwaway request specs, not part of this MR, that call each route as the group Owner and record the status, the body and what the database holds afterwards. I ran them on master a9490a0e3ac (the two commits between it and d77ac9dd142 touch none of these routes or models) with FOSS_ONLY=1 for CE, and without it for EE with stub_licensed_features(minimal_access_role: true) and false.

Route 0 5 60
POST /projects/:id/members 400 is not included in the list 400, every edition 400
PUT /projects/:id/members/:user_id 400 400, every edition 403
POST /groups/:id/members 400 201 only on EE, licensed, top-level group; otherwise 400 (not supported by license, supported on top level groups only) 400
PUT /groups/:id/members/:user_id 400 200 only on EE, licensed, top-level group; otherwise 400 403
POST /:source/:id/invitations 400 does not have a valid value CE: 400. EE groups: success only licensed on a top-level group, "status":"error" otherwise. EE projects: "status":"error" 400
PUT /:source/:id/invitations/:email 400 400, every edition (the route declares Gitlab::Access.all_values) 400
PUT /:source/:id/access_requests/:user_id/approve not approved (see below) approved only on EE, licensed, top-level group 403
POST /projects/:id/share 400 400, every edition not tried
POST /groups/:id/share 400 201 only on EE, licensed, top-level group; 409 otherwise on EE; 400 on CE not tried
POST /groups/:id/ldap_group_links (EE) 400 201 on a top-level group and a subgroup, with or without minimal_access_role not tried
POST /groups/:id/saml_group_links (EE) 400 201 only with minimal_access_role and a top-level group; 400 Access level is invalid otherwise not tried

The 60 on the member update routes was refused with 403 for the Owner I tested with. Member#access_level_inclusion, which refused 60 with 400 on the add routes, validates updates too.

Adding two users at once with 0, or with 5 where 5 is not accepted, answers 201 with "status":"error" and the same validation messages, and creates nobody. Leaving access_level out answers 400 access_level is missing on both member routes and on adding an invitation. An invitation stored at 20 and updated with only expires_at stayed at 20.

The models confirm the response rows too: SamlGroupLink#access_level_allowed only accepts group.access_level_roles, which never includes 0, so a SAML group link cannot hold 0 either.

Something I left out of this MR

Approving an access request with access_level 0, or 5 where 5 is not allowed, answers 201 Created with the requester and the requested level in the body, but the request is not approved: accept_request fails validation and returns false, Members::ApproveAccessRequestService returns success anyway, and the requester stays pending at its old level. This MR only removes 0 from that row. The response is a separate defect, which I would rather raise on its own.

Lint
  • scripts/lint-doc.sh on the 11 pages, run in the lint image the script names (markdownlint-cli2 0.23.2, Vale 3.21.0): passed, with no markdownlint issues and no Vale errors, warnings or suggestions at its error level.
  • Vale at suggestion level on the 11 pages, on this branch and on master: the same alerts on both, so this MR adds none.
  • lychee --offline --no-progress --include-fragments on the 11 pages, as the docs-lint links job runs it: 0 errors.

Related issues

Related to !248977 (merged) and #301219:

  • !248977 (merged) made POST /groups/:id/share accept 5 on Premium and Ultimate, which the group share row now says.
  • #301219 proposes moving the Minimal Access role to Free. If it lands, the "On Premium and Ultimate" qualifier these rows now carry changes with it.

Author's checklist

If you are a GitLab team member and only adding documentation, do not add any of the following labels:

  • ~"frontend"
  • ~"backend"
  • ~"type::bug"
  • ~"database"

These labels cause the MR to be added to code verification QA issues.

Reviewer's checklist

Documentation-related MRs should be reviewed by a Technical Writer for a non-blocking review, based on Documentation Guidelines and the Style Guide.

If you aren't sure which tech writer to ask, use roulette or ask in the #docs Slack channel.

  • If the content requires it, ensure the information is reviewed by a subject matter expert.
  • Technical writer review items:
    • Ensure docs metadata is present and up-to-date.
    • Ensure the appropriate labels are added to this MR.
    • Ensure a release milestone is set.
    • If relevant to this MR, ensure content topic type principles are in use, including:
      • The headings should be something you'd do a Google search for. Instead of Default behavior, say something like Default behavior when you close an issue.
      • 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 reviews above. Maintainer's review can occur before or after a technical writer review.

Merge request reports