[Backport 5.1] Add feature flags to perms syncer context (#56501)

Add feature flags to perms syncer context (#56492)

(cherry picked from commit a29b68f)

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

CHANGELOG.md

@@ -143,6 +143,7 @@ All notable changes to Sourcegraph are documented in this file.
 
 ### Fixed
 
+- Fixed an issue where the "gitLabProjectVisibilityExperimental" feature flag would not be respected by the permissions syncer. This meant that users on Sourcegraph that have signed in with GitLab would not see GitLab internal repositories that should be accessible to everyone on the GitLab instance, even though the feature flag was enabled [#56492](https://github.com/sourcegraph/sourcegraph/pull/56492)
 - Fixed a bug when syncing repository lists from GitHub that could lead to 404 errors showing up when running into GitHub rate limits [#56478](https://github.com/sourcegraph/sourcegraph/pull/56478)
 
 ### Removed

cmd/repo-updater/internal/authz/integration_test.go

@@ -12,13 +12,15 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/grafana/regexp"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/sourcegraph/log/logtest"
 
 	"github.com/sourcegraph/sourcegraph/internal/actor"
 	"github.com/sourcegraph/sourcegraph/internal/api"
 	"github.com/sourcegraph/sourcegraph/internal/authz"
 	authzGitHub "github.com/sourcegraph/sourcegraph/internal/authz/providers/github"
+	authzGitLab "github.com/sourcegraph/sourcegraph/internal/authz/providers/gitlab"
 	"github.com/sourcegraph/sourcegraph/internal/database"
 	"github.com/sourcegraph/sourcegraph/internal/database/dbtest"
 	"github.com/sourcegraph/sourcegraph/internal/extsvc"
@@ -489,3 +491,151 @@ func TestIntegration_GitHubPermissions(t *testing.T) {
 		})
 	})
 }
+
+func TestIntegration_GitLabPermissions(t *testing.T) {
+	if testing.Short() {
+		t.Skip()
+	}
+
+	logger := logtest.Scoped(t)
+	token := os.Getenv("GITLAB_TOKEN")
+
+	spec := extsvc.AccountSpec{
+		ServiceType: extsvc.TypeGitLab,
+		ServiceID:   "https://gitlab.sgdev.org/",
+		AccountID:   "107564",
+	}
+	svc := types.ExternalService{
+		Kind:   extsvc.KindGitLab,
+		Config: extsvc.NewUnencryptedConfig(`{"url": "https://gitlab.sgdev.org", "authorization": {"identityProvider": {"type": "oauth"}}, "token": "abc", "projectQuery": [ "projects?membership=true&archived=no" ]}`),
+	}
+	uri, err := url.Parse("https://gitlab.sgdev.org")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	newUser := database.NewUser{
+		Email:           "[email protected]",
+		Username:        "sourcegraph-vcr",
+		EmailIsVerified: true,
+	}
+
+	// These tests require two repos to be set up:
+	// Both schwifty2 and getschwifty are internal projects.
+	// The user is an explicit collaborator on getschwifty, so
+	// should have access to getschwifty regardless of the feature flag.
+	// The user does not have explicit access to schwifty2, however
+	// schwifty2 is configured so that anyone on the instance has read
+	// access, so when the feature flag is enabled, the user should
+	// see this repo as well.
+	testRepos := []types.Repo{
+		{
+			Name:    "gitlab.sgdev.org/petrissupercoolgroup/schwifty2",
+			Private: true,
+			URI:     "gitlab.sgdev.org/petrissupercoolgroup/schwifty2",
+			ExternalRepo: api.ExternalRepoSpec{
+				ID:          "371335",
+				ServiceType: extsvc.TypeGitLab,
+				ServiceID:   "https://gitlab.sgdev.org/",
+			},
+			Sources: map[string]*types.SourceInfo{
+				svc.URN(): {
+					ID: svc.URN(),
+				},
+			},
+		},
+		{
+			Name:    "gitlab.sgdev.org/petri.last/getschwifty",
+			Private: true,
+			URI:     "gitlab.sgdev.org/petri.last/getschwifty",
+			ExternalRepo: api.ExternalRepoSpec{
+				ID:          "371334",
+				ServiceType: extsvc.TypeGitLab,
+				ServiceID:   "https://gitlab.sgdev.org/",
+			},
+			Sources: map[string]*types.SourceInfo{
+				svc.URN(): {
+					ID: svc.URN(),
+				},
+			},
+		},
+	}
+
+	authData := json.RawMessage(fmt.Sprintf(`{"access_token": "%s"}`, token))
+
+	// This integration tests performs a user-centric permissions syncing against
+	// https://gitlab.sgdev.org, then check if permissions are correctly granted for the test
+	// user "sourcegraph-vcr".
+	t.Run("test gitLabProjectVisibilityExperimental feature flag", func(t *testing.T) {
+		name := t.Name()
+
+		cf, save := httptestutil.NewRecorderFactory(t, update(name), name)
+		defer save()
+		doer, err := cf.Doer()
+		require.NoError(t, err)
+
+		testDB := database.NewDB(logger, dbtest.NewDB(logger, t))
+
+		ctx := actor.WithInternalActor(context.Background())
+
+		reposStore := repos.NewStore(logtest.Scoped(t), testDB)
+
+		err = reposStore.ExternalServiceStore().Upsert(ctx, &svc)
+		require.NoError(t, err)
+
+		provider := authzGitLab.NewOAuthProvider(authzGitLab.OAuthProviderOp{
+			BaseURL: uri,
+			DB:      testDB,
+			CLI:     doer,
+		})
+
+		authz.SetProviders(false, []authz.Provider{provider})
+		defer authz.SetProviders(true, nil)
+		for _, repo := range testRepos {
+			err = reposStore.RepoStore().Create(ctx, &repo)
+			require.NoError(t, err)
+		}
+
+		user, err := testDB.UserExternalAccounts().CreateUserAndSave(ctx, newUser, spec, extsvc.AccountData{
+			AuthData: extsvc.NewUnencryptedData(authData),
+		})
+		require.NoError(t, err)
+
+		permsStore := database.Perms(logger, testDB, timeutil.Now)
+		syncer := NewPermsSyncer(logger, testDB, reposStore, permsStore, timeutil.Now)
+
+		assertUserPermissions := func(t *testing.T, wantIDs []int32) {
+			t.Helper()
+			_, providerStates, err := syncer.syncUserPerms(ctx, user.ID, false, authz.FetchPermsOptions{})
+			require.NoError(t, err)
+
+			assert.Equal(t, database.CodeHostStatusesSet{{
+				ProviderID:   "https://gitlab.sgdev.org/",
+				ProviderType: "gitlab",
+				Status:       database.CodeHostStatusSuccess,
+				Message:      "FetchUserPerms",
+			}}, providerStates)
+
+			p, err := permsStore.LoadUserPermissions(ctx, user.ID)
+			require.NoError(t, err)
+
+			gotIDs := make([]int32, len(p))
+			for i, perm := range p {
+				gotIDs[i] = perm.RepoID
+			}
+
+			if diff := cmp.Diff(wantIDs, gotIDs); diff != "" {
+				t.Fatalf("IDs mismatch (-want +got):\n%s", diff)
+			}
+		}
+
+		// With the feature flag disabled (default state) the user should only have access to one repo
+		assertUserPermissions(t, []int32{2})
+
+		// With the feature flag enabled the user should have access to both repositories
+		_, err = testDB.FeatureFlags().CreateBool(ctx, "gitLabProjectVisibilityExperimental", true)
+		require.NoError(t, err, "feature flag creation failed")
+
+		assertUserPermissions(t, []int32{1, 2})
+	})
+}

cmd/repo-updater/internal/authz/perms_syncer.go

@@ -19,6 +19,7 @@ import (
 	"github.com/sourcegraph/sourcegraph/internal/errcode"
 	"github.com/sourcegraph/sourcegraph/internal/extsvc"
 	"github.com/sourcegraph/sourcegraph/internal/extsvc/github"
+	"github.com/sourcegraph/sourcegraph/internal/featureflag"
 	"github.com/sourcegraph/sourcegraph/internal/repos"
 	"github.com/sourcegraph/sourcegraph/internal/trace"
 	"github.com/sourcegraph/sourcegraph/internal/types"
@@ -269,6 +270,7 @@ func (s *PermsSyncer) syncUserPerms(ctx context.Context, userID int32, noPerms b
 			log.Int32("ID", userID),
 			log.String("name", user.Username)),
 	)
+	ctx = featureflag.WithFlags(ctx, s.db.FeatureFlags())
 
 	results, err := s.fetchUserPermsViaExternalAccounts(ctx, user, noPerms, fetchOpts)
 	providerStates := results.providerStates
@@ -699,7 +701,6 @@ func (s *PermsSyncer) saveUserPermsForAccount(ctx context.Context, userID int32,
 		UserID:            userID,
 		ExternalAccountID: acctID,
 	}, repoIDs, authz.SourceUserSync)
-
 	if err != nil {
 		logger.Warn("saving perms to DB", log.Error(err))
 		return nil, err