[chore] disallow adding authorization on dotcom (#49426)

## Description

This changes previous behavior, where authorization was enforced on
dotcom for github and gitlab code hosts. We do not have any private code
on dotcom, show error when you try adding a private repo to dotcom and
this is the final nail in the coffin, by not even allowing to specify
that you want to enforce permissions on dotcom.

Kudos to @pjlast for pointing me to the right place 🙇 

## Test plan

unit tests changed, manual test

---------

Co-authored-by: Alex Ostrikov <alex.ostrikov@sourcegraph.com>

Author: kopancek

internal/database/external_services.go

@@ -502,25 +502,16 @@ func validatePerforceConnection(perforceValidators []func(*schema.PerforceConnec
 	return err
 }
 
-// upsertAuthorizationToExternalService adds "authorization" field to the
-// external service config when not yet present for GitHub and GitLab.
-func upsertAuthorizationToExternalService(kind, config string) (string, error) {
-	switch kind {
-	case extsvc.KindGitHub:
-		return jsonc.Edit(config, &schema.GitHubAuthorization{}, "authorization")
-
-	case extsvc.KindGitLab:
-		return jsonc.Edit(config,
-			&schema.GitLabAuthorization{
-				IdentityProvider: schema.IdentityProvider{
-					Oauth: &schema.OAuthIdentity{
-						Type: "oauth",
-					},
-				},
-			},
-			"authorization")
-	}
-	return config, nil
+// disablePermsSyncingForExternalService removes "authorization" or
+// "enforcePermissions" fields from the external service config
+// when present on the external service config.
+func disablePermsSyncingForExternalService(config string) (string, error) {
+	withoutEnforcePermissions, err := jsonc.Remove(config, "enforcePermissions")
+	// in case removing "enforcePermissions" fails, we try to remove "authorization" anyway
+	if err != nil {
+		withoutEnforcePermissions = config
+	}
+	return jsonc.Remove(withoutEnforcePermissions, "authorization")
 }
 
 func (e *externalServiceStore) Create(ctx context.Context, confGet func() *conf.Unified, es *types.ExternalService) error {
@@ -538,11 +529,11 @@ func (e *externalServiceStore) Create(ctx context.Context, confGet func() *conf.
 		return err
 	}
 
-	// 🚨 SECURITY: For all GitHub and GitLab code host connections on Sourcegraph
-	// Cloud, we always want to enforce repository permissions using OAuth to
-	// prevent unexpected resource leaking.
+	// 🚨 SECURITY: For all code host connections on Sourcegraph.com,
+	// we always want to disable repository permissions to prevent
+	// permission syncing from trying to sync permissions from public code.
 	if envvar.SourcegraphDotComMode() {
-		rawConfig, err = upsertAuthorizationToExternalService(es.Kind, rawConfig)
+		rawConfig, err = disablePermsSyncingForExternalService(rawConfig)
 		if err != nil {
 			return err
 		}
@@ -623,11 +614,11 @@ func (e *externalServiceStore) Upsert(ctx context.Context, svcs ...*types.Extern
 			return errors.Wrapf(err, "validating service of kind %q", s.Kind)
 		}
 
-		// 🚨 SECURITY: For all GitHub and GitLab code host connections on Sourcegraph
-		// Cloud, we always want to enforce repository permissions using OAuth to
-		// prevent unexpected resource leaking.
+		// 🚨 SECURITY: For all code host connections on Sourcegraph.com,
+		// we always want to disable repository permissions to prevent
+		// permission syncing from trying to sync permissions from public code.
 		if envvar.SourcegraphDotComMode() {
-			rawConfig, err = upsertAuthorizationToExternalService(s.Kind, rawConfig)
+			rawConfig, err = disablePermsSyncingForExternalService(rawConfig)
 			if err != nil {
 				return err
 			}
@@ -849,11 +840,11 @@ func (e *externalServiceStore) Update(ctx context.Context, ps []schema.AuthProvi
 			return err
 		}
 
-		// 🚨 SECURITY: For all GitHub and GitLab code host connections on Sourcegraph
-		// Cloud, we always want to enforce repository permissions using OAuth to
-		// prevent unexpected resource leaking.
+		// 🚨 SECURITY: For all code host connections on Sourcegraph.com,
+		// we always want to disable repository permissions to prevent
+		// permission syncing from trying to sync permissions from public code.
 		if envvar.SourcegraphDotComMode() {
-			unredactedConfig, err = upsertAuthorizationToExternalService(externalService.Kind, unredactedConfig)
+			unredactedConfig, err = disablePermsSyncingForExternalService(unredactedConfig)
 			if err != nil {
 				return err
 			}

internal/database/external_services_test.go

@@ -557,16 +557,14 @@ func TestExternalServicesStore_Update(t *testing.T) {
 	}
 }
 
-func TestUpsertAuthorizationToExternalService(t *testing.T) {
+func TestDisablePermsSyncingForExternalService(t *testing.T) {
 	tests := []struct {
 		name   string
-		kind   string
 		config string
 		want   string
 	}{
 		{
 			name: "github with authorization",
-			kind: extsvc.KindGitHub,
 			config: `
 {
   // Useful comments
@@ -580,13 +578,11 @@ func TestUpsertAuthorizationToExternalService(t *testing.T) {
   // Useful comments
   "url": "https://github.com",
   "repositoryQuery": ["none"],
-  "token": "def",
-  "authorization": {}
+  "token": "def"
 }`,
 		},
 		{
 			name: "github without authorization",
-			kind: extsvc.KindGitHub,
 			config: `
 {
   // Useful comments
@@ -599,61 +595,48 @@ func TestUpsertAuthorizationToExternalService(t *testing.T) {
   // Useful comments
   "url": "https://github.com",
   "repositoryQuery": ["none"],
-  "token": "def",
-  "authorization": {}
+  "token": "def"
 }`,
 		},
 		{
-			name: "gitlab with authorization",
-			kind: extsvc.KindGitLab,
+			name: "azure devops with enforce permissions",
 			config: `
 {
   // Useful comments
-  "url": "https://gitlab.com",
-  "projectQuery": ["none"],
+  "url": "https://dev.azure.com",
+  "username": "horse",
   "token": "abc",
-  "authorization": {}
+  "enforcePermissions": true
 }`,
 			want: `
 {
   // Useful comments
-  "url": "https://gitlab.com",
-  "projectQuery": ["none"],
-  "token": "abc",
-  "authorization": {
-    "identityProvider": {
-      "type": "oauth"
-    }
-  }
+  "url": "https://dev.azure.com",
+  "username": "horse",
+  "token": "abc"
 }`,
 		},
 		{
-			name: "gitlab without authorization",
-			kind: extsvc.KindGitLab,
+			name: "azure devops without enforce permissions",
 			config: `
 {
   // Useful comments
-  "url": "https://gitlab.com",
-  "projectQuery": ["none"],
+  "url": "https://dev.azure.com",
+  "username": "horse",
   "token": "abc"
 }`,
 			want: `
 {
   // Useful comments
-  "url": "https://gitlab.com",
-  "projectQuery": ["none"],
-  "token": "abc",
-  "authorization": {
-    "identityProvider": {
-      "type": "oauth"
-    }
-  }
+  "url": "https://dev.azure.com",
+  "username": "horse",
+  "token": "abc"
 }`,
 		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			got, err := upsertAuthorizationToExternalService(test.kind, test.config)
+			got, err := disablePermsSyncingForExternalService(test.config)
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -666,9 +649,9 @@ func TestUpsertAuthorizationToExternalService(t *testing.T) {
 }
 
 // This test ensures under Sourcegraph.com mode, every call of `Create`,
-// `Upsert` and `Update` has the "authorization" field presented in the external
+// `Upsert` and `Update` removes the "authorization" field in the external
 // service config automatically.
-func TestExternalServicesStore_upsertAuthorizationToExternalService(t *testing.T) {
+func TestExternalServicesStore_DisablePermsSyncingForExternalService(t *testing.T) {
 	if testing.Short() {
 		t.Skip()
 	}
@@ -688,7 +671,7 @@ func TestExternalServicesStore_upsertAuthorizationToExternalService(t *testing.T
 	es := &types.ExternalService{
 		Kind:        extsvc.KindGitHub,
 		DisplayName: "GITHUB #1",
-		Config:      extsvc.NewUnencryptedConfig(`{"url": "https://github.com", "repositoryQuery": ["none"], "token": "abc"}`),
+		Config:      extsvc.NewUnencryptedConfig(`{"url": "https://github.com", "repositoryQuery": ["none"], "token": "abc", "authorization": {}}`),
 	}
 	err := externalServices.Create(ctx, confGet, es)
 	require.NoError(t, err)
@@ -700,10 +683,10 @@ func TestExternalServicesStore_upsertAuthorizationToExternalService(t *testing.T
 		t.Fatal(err)
 	}
 	exists := gjson.Get(cfg, "authorization").Exists()
-	assert.True(t, exists, `"authorization" field exists`)
+	assert.False(t, exists, `"authorization" field exists, but should not`)
 
 	// Reset Config field and test Upsert method
-	es.Config.Set(`{"url": "https://github.com", "repositoryQuery": ["none"], "token": "abc"}`)
+	es.Config.Set(`{"url": "https://github.com", "repositoryQuery": ["none"], "token": "abc", "authorization": {}}`)
 	err = externalServices.Upsert(ctx, es)
 	require.NoError(t, err)
 
@@ -714,10 +697,10 @@ func TestExternalServicesStore_upsertAuthorizationToExternalService(t *testing.T
 		t.Fatal(err)
 	}
 	exists = gjson.Get(cfg, "authorization").Exists()
-	assert.True(t, exists, `"authorization" field exists`)
+	assert.False(t, exists, `"authorization" field exists, but should not`)
 
 	// Reset Config field and test Update method
-	es.Config.Set(`{"url": "https://github.com", "repositoryQuery": ["none"], "token": "abc"}`)
+	es.Config.Set(`{"url": "https://github.com", "repositoryQuery": ["none"], "token": "abc", "authorization": {}}`)
 	err = externalServices.Update(ctx,
 		conf.Get().AuthProviders,
 		es.ID,
@@ -734,7 +717,7 @@ func TestExternalServicesStore_upsertAuthorizationToExternalService(t *testing.T
 		t.Fatal(err)
 	}
 	exists = gjson.Get(cfg, "authorization").Exists()
-	assert.True(t, exists, `"authorization" field exists`)
+	assert.False(t, exists, `"authorization" field exists, but should not`)
 }
 
 func TestCountRepoCount(t *testing.T) {