Move Code Suggestions Duo availability check into policies

Vitali Tatarintevrequested to merge
621531-move-code-suggestions-duo-check-to-policy into master

What does this MR do and why?

Follow-up from the review of !248299 (merged). The check that blocks Code Suggestions when GitLab Duo is off lived in Ai::UserAuthorizable#code_suggestions_blocked_for?, and the API called it directly. Duo Chat does this kind of check in the project and group policies. This change uses the same approach for Code Suggestions.

The behavior is almost the same as before. There is one small change, see Behavior change below.

What changed

  • ProjectPolicy and GroupPolicy now define :access_code_suggestions:
    • An enable rule with the same licensing checks as the global policy.
    • A prevent rule when GitLab Duo is off for the subject. In ProjectPolicy this reuses the existing duo_features_enabled condition, because the project column is NOT NULL. In GroupPolicy it checks duo_features_enabled == false, because the group setting can also be nil. nil cascades to the instance setting, which is on for GitLab.com.
  • The API resolves the resource for the request and asks the policy with can?(:access_code_suggestions, subject):
    • The project wins when the request has a project_path.
    • Without a project, or when the user cannot read it, the governing namespace is used. That is where the usage is billed.
    • Without a namespace, and on self-managed, the global policy is used. On self-managed the instance-level setting already gates access.
  • Ai::UserAuthorizable#code_suggestions_blocked_for? is removed. Its model specs moved to the new project and group policy specs.

Behavior change

Before this MR, the check returned early when the user had no governing namespace, even if a project was found. So a request for a readable project with GitLab Duo off was allowed when the user was not a member of that group and had no default Duo namespace.

Now the project setting wins. If the project has GitLab Duo off, the request is blocked. This is the correct result, because the project Owner turned Duo off. A new request spec covers this case.

Seat check does not use the subject yet

code_suggestions_enabled_for_user calls allowed_to_use?(:code_suggestions) without root_namespace: @subject.root_ancestor. Duo Chat passes the root namespace. We keep the old behavior in this MR to limit the change. Passing the root namespace is a follow-up: #628218 (closed). Both policies have a comment that points to it.

Why the licensing conditions are duplicated

Project and group policies do not delegate to the global policy (ProjectPolicy.delegations == []). So a prevent rule alone is not enough: without an enable rule in the subject policy, a subject-scoped check would always deny. Each policy needs its own enable rule with the licensing checks.

This copies two conditions (code_suggestions_licensed, code_suggestions_enabled_for_user) into both policies. Duo Chat uses the same pattern: classic_chat_available_for_user is also defined separately in GroupPolicy and ProjectPolicy.

Alternatives considered

  1. Keep the check in the API helper (the state before this MR). Smallest footprint, but the access rule lives outside the policy layer, and it is not consistent with Duo Chat.
  2. delegate { :global } in the subject policies plus a prevent rule. Rejected: it would pull all global abilities into those policies, which is a broad and surprising change.
  3. Extract the shared licensing conditions into a module used by all three policies, instead of duplicating them. This is a possible follow-up if we prefer it over the duplication. Duo Chat duplicates today, so this MR does the same for consistency.

References

How to set up and validate locally

  1. Seed a Duo group and project in GitLab.com mode. Run this from the Rails root directory:

    GITLAB_SIMULATE_SAAS=1 bundle exec 'rake gitlab:duo:setup[duo_pro]'

  2. Start GDK with SaaS simulation on, so Gitlab::Saas.feature_available?(:gitlab_com_subscriptions) is true.

  3. In the Rails console, check the policy directly:

    user    = User.find_by_username('root')
group   = Group.find_by_full_path('gitlab-duo')
project = group.all_projects.first

# Duo is on: allowed
Ability.allowed?(user, :access_code_suggestions, project)  # => true

# Owner sets "Always off": blocked
Groups::UpdateService.new(group, user, duo_availability: 'never_on').execute
Ability.allowed?(user, :access_code_suggestions, Project.find(project.id))  # => false

# "Off by default" with a project that opts back in: allowed
Groups::UpdateService.new(group, user, duo_availability: 'default_off').execute
Project.find(project.id).project_setting.update!(duo_features_enabled: true)
Ability.allowed?(user, :access_code_suggestions, Project.find(project.id))  # => true

  4. Confirm the API rejects the request when Duo is off. This should return 401:

    curl --request POST --header "PRIVATE-TOKEN: <your_token>" \
  "http://gdk.test:3000/api/v4/code_suggestions/direct_access"

Test coverage

  • New policy specs for access_code_suggestions in project_policy_spec.rb and ee/group_policy_spec.rb. They use a real Duo seat instead of a stub, so the enable rule is tested for real. They cover:
    • Positive path: Duo on with a seat.
    • Each licensing condition on its own: no user, no Duo seat, instance set to "Always off", and no license on self-managed. If you remove any one of the three conditions from the enable rule, a test fails.
    • The prevent rule: Duo off, "Off by default" with a project that opts back in, and "Always off" that locks descendants.
  • One new API request spec for the behavior change above. Bringing back the old early return makes this test fail.
  • The other API request specs pass without changes (189 examples).
  • Full policy suites: 7428 examples, 0 failures.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Vitali Tatarintev

Merge request reports