[Backport 5.1] [permissions] Revert default behaviour for explicit permissions (#54439)

With the switch to the `user_repo_permissions` table, the default
behaviour for explicit permissions changed. Previously, if explicit
permissions API was enabled, then all repositories would be treated as
restricted, regardless of code host connection configurations.

The `user_repo_permissions` table allows us to run permissions syncs and
explicit permissions in parallel, but the default behaviour was to have
code host connections be marked as unrestricted, unless authorization is
enabled. However, not all code hosts support authorization, leading to
connections that are always left unrestricted, even if explicit
permissions are being used.

This PR reverts the default behaviour back to treating a repository as
restricted if explicit permissions are enabled.

## Test plan

Added unit tests for the new Unrestricted logic. Also tested locally
with an "Other Git code host" connection.

<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->
 <br> Backport 5ad2474 from #54419

Co-authored-by: Petri-Johan Last <[email protected]>

Author: github-actions[bot]

CHANGELOG.md

@@ -26,7 +26,7 @@ All notable changes to Sourcegraph are documented in this file.
 
 ### Fixed
 
--
+- Fixed the default behaviour when the explicit permissions API is enabled. Repositories are no longer marked as unrestricted by default. [#54419](https://github.com/sourcegraph/sourcegraph/pull/54419)
 
 ### Removed
 

doc/admin/permissions/index.md

@@ -112,7 +112,7 @@ of `alice` are the following union set: [`horsegraph/global`, `horsegraph/hay-v1
 1. Go to **Site Admin > Migrations** page. There is a migration called `Migrate data from user_permissions table to unified user_repo_permissions.`. 
 Make sure that it finished migrating all the data (it reports as 100%). Contact support if the migration does not seem to complete for a long time (multiple days). 
 
-1. Enable the experimental feature in the [site configuration](../config/site_config.md):
+1. (Not required for Sourcegraph 5.1+) Enable the experimental feature in the [site configuration](../config/site_config.md):
 ```json
 {
   "experimentalFeatures": {
@@ -122,8 +122,6 @@ Make sure that it finished migrating all the data (it reports as 100%). Contact
 }
 ```
 1. Continue [configuring the explicit permissions API](api.md#configuration) as you would before. 
-Both mechanisms work at the same time, thanks to a new behind the scenes data model that allows 
-what was previously impossible. 
 
 ### Permission updates
 

internal/database/BUILD.bazel

@@ -18,6 +18,7 @@ import (
 	"github.com/sourcegraph/log"
 
 	"github.com/sourcegraph/sourcegraph/cmd/frontend/envvar"
+	"github.com/sourcegraph/sourcegraph/cmd/frontend/globals"
 	"github.com/sourcegraph/sourcegraph/internal/conf"
 	"github.com/sourcegraph/sourcegraph/internal/database/basestore"
 	"github.com/sourcegraph/sourcegraph/internal/database/dbutil"
@@ -1578,8 +1579,12 @@ func (e *externalServiceStore) recalculateFields(es *types.ExternalService, rawC
 	// For existing auth providers, this is forwards compatible. While at the same time if they also
 	// wanted to get on the `enforcePermissions` pattern, this change is backwards compatible.
 	enforcePermissions := gjson.Get(rawConfig, "enforcePermissions")
-	if !envvar.SourcegraphDotComMode() && enforcePermissions.Exists() {
-		es.Unrestricted = !enforcePermissions.Bool()
+	if !envvar.SourcegraphDotComMode() {
+		if globals.PermissionsUserMapping().Enabled {
+			es.Unrestricted = false
+		} else if enforcePermissions.Exists() {
+			es.Unrestricted = !enforcePermissions.Bool()
+		}
 	}
 
 	hasWebhooks := false

internal/database/external_services.go

@@ -21,13 +21,15 @@ import (
 	"github.com/sourcegraph/log"
 
 	"github.com/sourcegraph/sourcegraph/internal/api"
+	"github.com/sourcegraph/sourcegraph/internal/jsonc"
 	"github.com/sourcegraph/sourcegraph/internal/timeutil"
 	"github.com/sourcegraph/sourcegraph/lib/errors"
 	"github.com/sourcegraph/sourcegraph/lib/pointers"
 
 	"github.com/sourcegraph/log/logtest"
 
 	"github.com/sourcegraph/sourcegraph/cmd/frontend/envvar"
+	"github.com/sourcegraph/sourcegraph/cmd/frontend/globals"
 	"github.com/sourcegraph/sourcegraph/internal/actor"
 	"github.com/sourcegraph/sourcegraph/internal/conf"
 	"github.com/sourcegraph/sourcegraph/internal/database/basestore"
@@ -2389,6 +2391,61 @@ func TestConfigurationHasWebhooks(t *testing.T) {
 	})
 }
 
+func TestExternalServiceStore_recalculateFields(t *testing.T) {
+	tests := map[string]struct {
+		explicitPermsEnabled bool
+		authorizationSet     bool
+		expectUnrestricted   bool
+	}{
+		"default state": {
+			expectUnrestricted: true,
+		},
+		"explicit perms set": {
+			explicitPermsEnabled: true,
+			expectUnrestricted:   false,
+		},
+		"authorization set": {
+			authorizationSet:   true,
+			expectUnrestricted: false,
+		},
+		"authorization and explicit perms set": {
+			explicitPermsEnabled: true,
+			authorizationSet:     true,
+			expectUnrestricted:   false,
+		},
+	}
+
+	e := &externalServiceStore{logger: logtest.NoOp(t)}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			pmu := globals.PermissionsUserMapping()
+			t.Cleanup(func() {
+				globals.SetPermissionsUserMapping(pmu)
+			})
+
+			es := &types.ExternalService{}
+
+			if tc.explicitPermsEnabled {
+				globals.SetPermissionsUserMapping(&schema.PermissionsUserMapping{
+					BindID:  "email",
+					Enabled: true,
+				})
+			}
+			rawConfig := "{}"
+			var err error
+			if tc.authorizationSet {
+				rawConfig, err = jsonc.Edit(rawConfig, struct{}{}, "authorization")
+				require.NoError(t, err)
+			}
+
+			require.NoError(t, e.recalculateFields(es, rawConfig))
+
+			require.Equal(t, es.Unrestricted, tc.expectUnrestricted)
+		})
+	}
+}
+
 func TestExternalServiceStore_ListRepos(t *testing.T) {
 	if testing.Short() {
 		t.Skip()

internal/database/external_services_test.go

@@ -147,6 +147,7 @@ func (s *AWSCodeCommitSource) makeRepo(r *awscodecommit.Repository) *types.Repo
 			},
 		},
 		Metadata: r,
+		Private:  !s.svc.Unrestricted,
 	}
 }
 

internal/repos/awscodecommit.go

@@ -123,5 +123,6 @@ func (s GitoliteSource) makeRepo(repo *gitolite.Repo) *types.Repo {
 			},
 		},
 		Metadata: repo,
+		Private:  !s.svc.Unrestricted,
 	}
 }

internal/repos/gitolite.go

@@ -198,6 +198,7 @@ func (s OtherSource) otherRepoFromCloneURL(urn string, u *url.URL) (*types.Repo,
 		Metadata: &extsvc.OtherRepoMetadata{
 			RelativePath: strings.TrimPrefix(repoURL, serviceID),
 		},
+		Private: !s.svc.Unrestricted,
 	}, nil
 }
 

internal/repos/other.go

@@ -168,6 +168,7 @@ func (s *PhabricatorSource) makeRepo(repo *phabricator.Repo) (*types.Repo, error
 			},
 		},
 		Metadata: repo,
+		Private:  !s.svc.Unrestricted,
 	}, nil
 }