Skip to content

[CLOUD-78] external_services: upsert "authorization" field in `Upsert` method

Warren Gifford requested to merge jc/CLOUD-78-upsert-authz into main

Created by: unknwon

Upon finishing the implementation of CLOUD-78, we couldn't get private repositories being added the user's organization to show up in the user's search results (the user also has access to the repository on the code host).

After the investigation, we discovered that we overlooked ExternalServicesStore.Upsert method where we call upsertAuthorizationToExternalService in both ExternalServicesStore.Create and ExternalServicesStore.Update, to make sure the special "authorization" field exists in every external service config.

The outcome of missing this field caused our permissions syncer to believe that it does not need to fetch permissions from the code host for the user (because we do not derive an authz.Provider from external services that are not having the "authorization" field in their config).

Security impact

We're totally safe and no private data had leaked thanks to the philosophy that we never skip authz on dotcom.

Followup

With this discovery, our prod database is in "dirty" state that all external services do not have the "authorization" field. This is fine as-is because we're always on the conservative side and assume no access on dotcom by default.

Technically, we need an out-of-band migration to upsert "authorization" field to all existing external services, but given the fact (that I believe) our repo syncing process continuously call Upsert method between 2 minutes and 8 hours for all external services, after this PR landed on prod for 8 hours, we will fix the "dirty" state. cc @sourcegraph/repo-management to confirm the "fact" :D

I also added a warning log in this PR for user-added external services that we cannot derive an authz.Provider for us to monitoring from logs (and also useful as a general warning).


Jira: CLOUD-78

Merge request reports

Loading