Remove superfluous calls to access_token

Matthias Käpplerrequested to merge
546563-remove-more-calls into master

What does this MR do and why?

Remove superfluous calls to access_token.

These calls were not used to send a Cloud Connector token in a request, but to check that the caller has "access to Cloud Connector" in some form.

This doesn't really work, because:

  • Cloud Connector is not a feature. Permission checks must happen on a per-feature basis using policy checks.
  • On gitlab.com, tokens are created on-the-fly, so these checks were fruitless because a token always exists, and only added extra latency.

On SM/Dedicated instances, where we sync in tokens, there may now be cases where an upstream call is made with a missing token. This would still result in a 401 with the same outcome for the customer, however.

This means we will send a small amount of extra traffic to the AI gateway, however this would only happen for:

  • Clients who do not have a valid access token (i.e. their instance hasn't synced in a while)
  • Only when those customers continue to try and use Duo features without such a token
  • For code completions, only when using proxied mode
  • Only on those code paths where this was actually tested in the first place

Overall, I'd attribute a small chance to this adding a lot of extra traffic.

References

Screenshots or screen recordings

n/a

How to set up and validate locally

This should not result in any user-facing errors. The only way you might observe this is for example:

  1. Set up Duo (cloud-connected) through CustomersDot (don't set CLOUD_CONNECTOR_SELF_SIGN_TOKENS or GITLAB_SIMULATE_SAAS)
  2. Make sure your subscription is synchronized (you can verify via Duo Admin health check)
  3. Go to rails console and CloudConnector::ServiceAccessToken.delete_all
  4. Send a code suggestions request
  5. You should receive a 401 with the error message {"error":"Forbidden by auth provider"}

On master, you would have not see this error message, because it is coming from the AI gateway, not Rails.

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.

Related to #546563 (closed)

Edited by Matthias Käppler

Merge request reports