artin/sourcegraph
1
0
Fork 0
mirror of https://github.com/sourcegraph/sourcegraph.git synced 2026-02-06 18:31:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

authz: Compute providers on the fly (#64012)

Browse Source 
Previously, we would store authz providers globally and refresh them
every now and then.
However, creating the providers is fairly cheap (1.3ms in a local trace)
so we should not keep them in memory and remember to not forget to start
the watcher routine.

This will help for multi-tenant Sourcegraph in that providers are now
computed for the context in question, and not held globally. Keeping
potentially 100k authz providers in memory will not scale.

Test plan: Still works, local Jaeger traces are quite acceptable.
This commit is contained in:
Erik Seliger 2024-07-31 02:59:41 +02:00 committed by GitHub
parent c917330d6b
commit 38b79fbb2f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
28 changed files with 120 additions and 287 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
cmd/frontend/graphqlbackend/authz.go
View File

@ -26,7 +26,6 @@ type AuthzResolver interface {
UsersWithPendingPermissions(ctx context.Context) ([]string, error)
AuthorizedUsers(ctx context.Context, args *RepoAuthorizedUserArgs) (UserConnectionResolver, error)
BitbucketProjectPermissionJobs(ctx context.Context, args *BitbucketProjectPermissionJobsArgs) (BitbucketProjectsPermissionJobsResolver, error)
AuthzProviderTypes(ctx context.Context) ([]string, error)
PermissionsSyncJobs(ctx context.Context, args ListPermissionsSyncJobsArgs) (*graphqlutil.ConnectionResolver[PermissionsSyncJobResolver], error)
PermissionsSyncingStats(ctx context.Context) (PermissionsSyncingStatsResolver, error)

6
cmd/frontend/graphqlbackend/authz.graphql
View File

@ -156,12 +156,6 @@ extend type Query {
"""
usersWithPendingPermissions: [String!]!
"""
INTERNAL ONLY: Returns a list of the types of authz providers that have been configured and will be used for
determining which repositories the user has access to.
"""
authzProviderTypes: [String!]!
"""
Returns a list of recent permissions sync jobs for a given set of parameters.
"""

1
cmd/frontend/internal/app/jscontext/BUILD.bazel
View File

@ -20,6 +20,7 @@ go_library(
"//internal/auth/providers",
"//internal/auth/userpasswd",
"//internal/authz",
"//internal/authz/providers",
"//internal/batches",
"//internal/codemonitors",
"//internal/conf",

4
cmd/frontend/internal/app/jscontext/jscontext.go
View File

@ -24,6 +24,7 @@ import (
"github.com/sourcegraph/sourcegraph/internal/auth/providers"
"github.com/sourcegraph/sourcegraph/internal/auth/userpasswd"
"github.com/sourcegraph/sourcegraph/internal/authz"
authzproviders "github.com/sourcegraph/sourcegraph/internal/authz/providers"
"github.com/sourcegraph/sourcegraph/internal/batches"
"github.com/sourcegraph/sourcegraph/internal/codemonitors"
"github.com/sourcegraph/sourcegraph/internal/conf"
@ -298,7 +299,8 @@ func NewJSContextFromRequest(req *http.Request, db database.DB) JSContext {
// Auth providers
authProviders := []authProviderInfo{} // Explicitly initialise array, otherwise it gets marshalled to null instead of []
authzProviders := authz.GetProviders()
authzProviders, _, _, _ := authzproviders.ProvidersFromConfig(ctx, conf.Get(), db)
for _, p := range providers.SortedProviders() {
commonConfig := providers.GetAuthProviderCommon(p)
if commonConfig.Hidden {

7
cmd/frontend/internal/authz/init.go
View File

@ -131,13 +131,6 @@ func Init(
return nil
})
go func() {
for range time.NewTicker(providers.RefreshInterval(conf.Get())).C {
authzProviders, _, _, _ := providers.ProvidersFromConfig(ctx, conf.Get(), db)
authz.SetProviders(authzProviders)
}
}()
enterpriseServices.AuthzResolver = resolvers.NewResolver(observationCtx, db)
return nil
}

1
cmd/frontend/internal/authz/resolvers/BUILD.bazel
View File

@ -59,7 +59,6 @@ go_test(
"//internal/auth",
"//internal/authz",
"//internal/authz/permssync",
"//internal/authz/providers/github",
"//internal/conf",
"//internal/database",
"//internal/database/dbmocks",

13
cmd/frontend/internal/authz/resolvers/resolver.go
View File

@ -487,19 +487,6 @@ func (r *Resolver) AuthorizedUsers(ctx context.Context, args *graphqlbackend.Rep
}, nil
}
func (r *Resolver) AuthzProviderTypes(ctx context.Context) ([]string, error) {
// 🚨 SECURITY: Only site admins can query for authz providers.
if err := auth.CheckCurrentUserIsSiteAdmin(ctx, r.db); err != nil {
return nil, err
}
providers := authz.GetProviders()
providerTypes := make([]string, 0, len(providers))
for _, p := range providers {
providerTypes = append(providerTypes, p.ServiceType())
}
return providerTypes, nil
}
var jobStatuses = map[string]bool{
"queued": true,
"processing": true,

39
cmd/frontend/internal/authz/resolvers/resolver_test.go
View File

@ -24,7 +24,6 @@ import (
"github.com/sourcegraph/sourcegraph/internal/auth"
"github.com/sourcegraph/sourcegraph/internal/authz"
"github.com/sourcegraph/sourcegraph/internal/authz/permssync"
"github.com/sourcegraph/sourcegraph/internal/authz/providers/github"
"github.com/sourcegraph/sourcegraph/internal/conf"
"github.com/sourcegraph/sourcegraph/internal/database"
"github.com/sourcegraph/sourcegraph/internal/database/dbmocks"
@ -1170,44 +1169,6 @@ func TestResolver_UsersWithPendingPermissions(t *testing.T) {
}
}
func TestResolver_AuthzProviderTypes(t *testing.T) {
t.Run("authenticated as non-admin", func(t *testing.T) {
users := dbmocks.NewStrictMockUserStore()
users.GetByCurrentAuthUserFunc.SetDefaultReturn(&types.User{}, nil)
db := dbmocks.NewStrictMockDB()
db.UsersFunc.SetDefaultReturn(users)
ctx := actor.WithActor(context.Background(), &actor.Actor{UID: 1})
result, err := (&Resolver{db: db}).AuthzProviderTypes(ctx)
if want := auth.ErrMustBeSiteAdmin; err != want {
t.Errorf("err: want %q but got %v", want, err)
}
if result != nil {
t.Errorf("result: want nil but got %v", result)
}
})
t.Run("get authz provider types", func(t *testing.T) {
users := dbmocks.NewStrictMockUserStore()
users.GetByCurrentAuthUserFunc.SetDefaultReturn(&types.User{
SiteAdmin: true,
}, nil)
db := dbmocks.NewStrictMockDB()
db.UsersFunc.SetDefaultReturn(users)
ctx := actor.WithActor(context.Background(), &actor.Actor{UID: 1})
ghProvider := github.NewProvider("https://github.com", github.ProviderOptions{GitHubURL: mustURL(t, "https://github.com")})
authz.SetProviders([]authz.Provider{ghProvider})
defer authz.SetProviders(nil)
result, err := (&Resolver{db: db}).AuthzProviderTypes(ctx)
assert.NoError(t, err)
assert.Equal(t, []string{"github"}, result)
})
}
func mustURL(t *testing.T, u string) *url.URL {
parsed, err := url.Parse(u)
if err != nil {

2
cmd/frontend/internal/cli/BUILD.bazel
View File

@ -36,7 +36,7 @@ go_library(
"//internal/api",
"//internal/auth",
"//internal/auth/userpasswd",
"//internal/authz",
"//internal/authz/providers",
"//internal/conf",
"//internal/conf/conftypes",
"//internal/conf/deploy",

4
cmd/frontend/internal/cli/serve_cmd.go
View File

@ -28,7 +28,7 @@ import (
oce "github.com/sourcegraph/sourcegraph/cmd/frontend/oneclickexport"
"github.com/sourcegraph/sourcegraph/internal/api"
"github.com/sourcegraph/sourcegraph/internal/auth/userpasswd"
"github.com/sourcegraph/sourcegraph/internal/authz"
"github.com/sourcegraph/sourcegraph/internal/authz/providers"
"github.com/sourcegraph/sourcegraph/internal/conf"
"github.com/sourcegraph/sourcegraph/internal/conf/conftypes"
"github.com/sourcegraph/sourcegraph/internal/conf/deploy"
@ -286,7 +286,7 @@ func Main(ctx context.Context, observationCtx *observation.Context, ready servic
ExternalServiceURL string `json:"external_service_url"`
}
providers := authz.GetProviders()
providers, _, _, _ := providers.ProvidersFromConfig(ctx, conf.Get(), db)
infos := make([]providerInfo, len(providers))
for i, p := range providers {
_, id := extsvc.DecodeURN(p.URN())

1
cmd/worker/internal/authz/BUILD.bazel
View File

@ -18,6 +18,7 @@ go_library(
"//internal/actor",
"//internal/api",
"//internal/authz",
"//internal/authz/providers",
"//internal/collections",
"//internal/conf",
"//internal/database",

49
cmd/worker/internal/authz/integration_test.go
View File

@ -215,8 +215,13 @@ func TestIntegration_GitHubPermissions(t *testing.T) {
DB: testDB,
})
authz.SetProviders([]authz.Provider{provider})
defer authz.SetProviders(nil)
prevProviderFactory := syncer.providerFactory
syncer.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{provider}
}
defer func() {
syncer.providerFactory = prevProviderFactory
}()
assertGitHubRepoPermissions(t, ctx, repo.ID, user.ID, uri.String(), syncer, permsStore, []int32{1})
})
@ -232,8 +237,13 @@ func TestIntegration_GitHubPermissions(t *testing.T) {
DB: testDB,
})
authz.SetProviders([]authz.Provider{provider})
defer authz.SetProviders(nil)
prevProviderFactory := syncer.providerFactory
syncer.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{provider}
}
defer func() {
syncer.providerFactory = prevProviderFactory
}()
assertGitHubRepoPermissions(t, ctx, repo.ID, user.ID, uri.String(), syncer, permsStore, []int32{1})
})
@ -254,8 +264,13 @@ func TestIntegration_GitHubPermissions(t *testing.T) {
DB: testDB,
})
authz.SetProviders([]authz.Provider{provider})
defer authz.SetProviders(nil)
prevProviderFactory := syncer.providerFactory
syncer.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{provider}
}
defer func() {
syncer.providerFactory = prevProviderFactory
}()
assertGitHubUserPermissions(t, ctx, user.ID, uri.String(), syncer, permsStore, []int32{1})
})
@ -271,8 +286,13 @@ func TestIntegration_GitHubPermissions(t *testing.T) {
DB: testDB,
})
authz.SetProviders([]authz.Provider{provider})
defer authz.SetProviders(nil)
prevProviderFactory := syncer.providerFactory
syncer.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{provider}
}
defer func() {
syncer.providerFactory = prevProviderFactory
}()
assertGitHubUserPermissions(t, ctx, user.ID, uri.String(), syncer, permsStore, []int32{1})
})
@ -361,9 +381,6 @@ func TestIntegration_GitHubInternalRepositories(t *testing.T) {
DB: testDB,
})
authz.SetProviders([]authz.Provider{provider})
defer authz.SetProviders(nil)
repo := types.Repo{
Name: "ghe.sgdev.org/sourcegraph/sourcegraph_internal_repo",
Private: true,
@ -404,6 +421,10 @@ func TestIntegration_GitHubInternalRepositories(t *testing.T) {
permsStore := database.Perms(logger, testDB, timeutil.Now)
syncer := newPermsSyncer(logger, testDB, reposStore, permsStore, timeutil.Now)
syncer.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{provider}
}
assertGitHubUserPermissions(t, ctx, user.ID, uri.String(), syncer, permsStore, []int32{1})
}
@ -505,8 +526,6 @@ func TestIntegration_GitLabPermissions(t *testing.T) {
SyncInternalRepoPermissions: true,
})
authz.SetProviders([]authz.Provider{provider})
defer authz.SetProviders(nil)
for _, repo := range testRepos {
err = reposStore.RepoStore().Create(ctx, &repo)
require.NoError(t, err)
@ -523,6 +542,10 @@ func TestIntegration_GitLabPermissions(t *testing.T) {
permsStore := database.Perms(logger, testDB, timeutil.Now)
syncer := newPermsSyncer(logger, testDB, reposStore, permsStore, timeutil.Now)
syncer.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{provider}
}
assertUserPermissions := func(t *testing.T, wantIDs []int32) {
t.Helper()
_, providerStates, err := syncer.syncUserPerms(ctx, user.ID, false, authz.FetchPermsOptions{})

20
cmd/worker/internal/authz/perms_syncer.go
View File

@ -13,7 +13,9 @@ import (
"github.com/sourcegraph/sourcegraph/internal/api"
"github.com/sourcegraph/sourcegraph/internal/authz"
"github.com/sourcegraph/sourcegraph/internal/authz/providers"
"github.com/sourcegraph/sourcegraph/internal/collections"
"github.com/sourcegraph/sourcegraph/internal/conf"
"github.com/sourcegraph/sourcegraph/internal/database"
"github.com/sourcegraph/sourcegraph/internal/dotcom"
"github.com/sourcegraph/sourcegraph/internal/errcode"
@ -57,6 +59,8 @@ type permsSyncerImpl struct {
permsUpdateLock sync.Mutex
// The database interface for any permissions operations.
permsStore database.PermsStore
// Can be overwritten for testing purposes.
providerFactory func(context.Context) []authz.Provider
}
// newPermsSyncer returns a new permissions syncer.
@ -73,6 +77,10 @@ func newPermsSyncer(
reposStore: reposStore,
permsStore: permsStore,
clock: clock,
providerFactory: func(ctx context.Context) []authz.Provider {
ps, _, _, _ := providers.ProvidersFromConfig(ctx, conf.Get(), db)
return ps
},
}
}
@ -96,7 +104,7 @@ func (s *permsSyncerImpl) syncRepoPerms(ctx context.Context, repoID api.RepoID,
// fetch permissions for private repositories.
if repo.Private {
// Loop over repository's sources and see if matching any authz provider's URN.
providers := s.providersByURNs()
providers := s.providersByURNs(ctx)
for urn := range repo.Sources {
p, ok := providers[urn]
if ok {
@ -339,8 +347,8 @@ func (s *permsSyncerImpl) syncUserPerms(ctx context.Context, userID int32, noPer
// providersByServiceID returns a list of authz.Provider configured in the external services.
// Keys are ServiceID, e.g. "https://github.com/".
func (s *permsSyncerImpl) providersByServiceID() map[string]authz.Provider {
ps := authz.GetProviders()
func (s *permsSyncerImpl) providersByServiceID(ctx context.Context) map[string]authz.Provider {
ps := s.providerFactory(ctx)
providers := make(map[string]authz.Provider, len(ps))
for _, p := range ps {
providers[p.ServiceID()] = p
@ -350,8 +358,8 @@ func (s *permsSyncerImpl) providersByServiceID() map[string]authz.Provider {
// providersByURNs returns a list of authz.Provider configured in the external services.
// Keys are URN, e.g. "extsvc:github:1".
func (s *permsSyncerImpl) providersByURNs() map[string]authz.Provider {
ps := authz.GetProviders()
func (s *permsSyncerImpl) providersByURNs(ctx context.Context) map[string]authz.Provider {
ps := s.providerFactory(ctx)
providers := make(map[string]authz.Provider, len(ps))
for _, p := range ps {
providers[p.URN()] = p
@ -456,7 +464,7 @@ func (s *permsSyncerImpl) fetchUserPermsViaExternalAccounts(ctx context.Context,
emails[i] = userEmails[i].Email
}
byServiceID := s.providersByServiceID()
byServiceID := s.providersByServiceID(ctx)
accounts := s.db.UserExternalAccounts()
logger := s.logger.Scoped("fetchUserPermsViaExternalAccounts").With(log.Int32("userID", user.ID))

97
cmd/worker/internal/authz/perms_syncer_test.go
View File

@ -67,10 +67,6 @@ func TestPermsSyncer_syncUserPerms(t *testing.T) {
serviceType: extsvc.TypeGitLab,
serviceID: "https://gitlab.com/",
}
authz.SetProviders([]authz.Provider{p})
t.Cleanup(func() {
authz.SetProviders(nil)
})
extAccount := extsvc.Account{
AccountSpec: extsvc.AccountSpec{
@ -132,6 +128,9 @@ func TestPermsSyncer_syncUserPerms(t *testing.T) {
})
s := newPermsSyncer(logtest.Scoped(t), db, reposStore, perms, timeutil.Now)
s.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{p}
}
p.fetchUserPerms = func(context.Context, *extsvc.Account) (*authz.ExternalUserPermissions, error) {
return &authz.ExternalUserPermissions{
@ -157,10 +156,6 @@ func TestPermsSyncer_syncUserPerms_listExternalAccountsError(t *testing.T) {
serviceType: extsvc.TypeGitLab,
serviceID: "https://gitlab.com/",
}
authz.SetProviders([]authz.Provider{p})
t.Cleanup(func() {
authz.SetProviders(nil)
})
users := dbmocks.NewMockUserStore()
users.GetByIDFunc.SetDefaultHook(func(ctx context.Context, id int32) (*types.User, error) {
@ -208,6 +203,9 @@ func TestPermsSyncer_syncUserPerms_listExternalAccountsError(t *testing.T) {
})
s := newPermsSyncer(logtest.Scoped(t), db, reposStore, perms, timeutil.Now)
s.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{p}
}
t.Run("fetchUserPermsViaExternalAccounts", func(t *testing.T) {
_, _, err := s.syncUserPerms(context.Background(), 1, true, authz.FetchPermsOptions{})
@ -226,10 +224,6 @@ func TestPermsSyncer_syncUserPerms_fetchAccount(t *testing.T) {
serviceType: extsvc.TypeGitHub,
serviceID: "https://github.com/",
}
authz.SetProviders([]authz.Provider{p1, p2})
t.Cleanup(func() {
authz.SetProviders(nil)
})
users := dbmocks.NewMockUserStore()
users.GetByIDFunc.SetDefaultHook(func(ctx context.Context, id int32) (*types.User, error) {
@ -304,6 +298,9 @@ func TestPermsSyncer_syncUserPerms_fetchAccount(t *testing.T) {
}
s := newPermsSyncer(logtest.Scoped(t), db, reposStore, perms, timeutil.Now)
s.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{p1, p2}
}
tests := []struct {
name string
@ -397,10 +394,6 @@ func TestPermsSyncer_syncUserPermsTemporaryProviderError(t *testing.T) {
serviceType: extsvc.TypeGitLab,
serviceID: "https://gitlab.com/",
}
authz.SetProviders([]authz.Provider{p})
t.Cleanup(func() {
authz.SetProviders(nil)
})
extAccount := extsvc.Account{
AccountSpec: extsvc.AccountSpec{
@ -464,6 +457,9 @@ func TestPermsSyncer_syncUserPermsTemporaryProviderError(t *testing.T) {
})
s := newPermsSyncer(logtest.Scoped(t), db, reposStore, perms, timeutil.Now)
s.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{p}
}
p.fetchUserPerms = func(context.Context, *extsvc.Account) (*authz.ExternalUserPermissions, error) {
// DeadlineExceeded implements the Temporary interface
@ -488,10 +484,6 @@ func TestPermsSyncer_syncUserPermsTemporaryProviderError(t *testing.T) {
serviceType: extsvc.TypeGitLab,
serviceID: "https://gitlab.com/",
}
authz.SetProviders([]authz.Provider{p})
t.Cleanup(func() {
authz.SetProviders(nil)
})
extAccount := extsvc.Account{
AccountSpec: extsvc.AccountSpec{
@ -575,6 +567,9 @@ func TestPermsSyncer_syncUserPermsTemporaryProviderError(t *testing.T) {
})
s := newPermsSyncer(logtest.Scoped(t), db, reposStore, perms, timeutil.Now)
s.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{p}
}
// Set up initial state with permissions entry including IP address
@ -605,10 +600,6 @@ func TestPermsSyncer_syncUserPermsTemporaryProviderError(t *testing.T) {
serviceType: extsvc.TypeGitLab,
serviceID: "https://gitlab.com/",
}
authz.SetProviders([]authz.Provider{p})
t.Cleanup(func() {
authz.SetProviders(nil)
})
extAccount := extsvc.Account{
AccountSpec: extsvc.AccountSpec{
@ -707,6 +698,9 @@ func TestPermsSyncer_syncUserPermsTemporaryProviderError(t *testing.T) {
})
s := newPermsSyncer(logtest.Scoped(t), db, reposStore, perms, timeutil.Now)
s.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{p}
}
// Set up initial state with permissions entry including IP address
@ -742,10 +736,6 @@ func TestPermsSyncer_syncUserPerms_noPerms(t *testing.T) {
serviceType: extsvc.TypeGitLab,
serviceID: "https://gitlab.com/",
}
authz.SetProviders([]authz.Provider{p})
t.Cleanup(func() {
authz.SetProviders(nil)
})
extAccount := extsvc.Account{
AccountSpec: extsvc.AccountSpec{
@ -801,6 +791,9 @@ func TestPermsSyncer_syncUserPerms_noPerms(t *testing.T) {
})
s := newPermsSyncer(logtest.Scoped(t), db, reposStore, perms, timeutil.Now)
s.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{p}
}
tests := []struct {
name string
@ -838,10 +831,6 @@ func TestPermsSyncer_syncUserPerms_tokenExpire(t *testing.T) {
serviceType: extsvc.TypeGitHub,
serviceID: "https://github.com/",
}
authz.SetProviders([]authz.Provider{p})
t.Cleanup(func() {
authz.SetProviders(nil)
})
extAccount := extsvc.Account{
AccountSpec: extsvc.AccountSpec{
@ -885,6 +874,9 @@ func TestPermsSyncer_syncUserPerms_tokenExpire(t *testing.T) {
perms := dbmocks.NewMockPermsStore()
s := newPermsSyncer(logtest.Scoped(t), db, reposStore, perms, timeutil.Now)
s.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{p}
}
t.Run("invalid token", func(t *testing.T) {
p.fetchUserPerms = func(ctx context.Context, account *extsvc.Account) (*authz.ExternalUserPermissions, error) {
@ -935,10 +927,6 @@ func TestPermsSyncer_syncUserPerms_prefixSpecs(t *testing.T) {
serviceType: extsvc.TypePerforce,
serviceID: "ssl:111.222.333.444:1666",
}
authz.SetProviders([]authz.Provider{p})
t.Cleanup(func() {
authz.SetProviders(nil)
})
extAccount := extsvc.Account{
AccountSpec: extsvc.AccountSpec{
@ -993,6 +981,9 @@ func TestPermsSyncer_syncUserPerms_prefixSpecs(t *testing.T) {
perms.SetUserExternalAccountPermsFunc.SetDefaultReturn(&database.SetPermissionsResult{}, nil)
s := newPermsSyncer(logtest.Scoped(t), db, reposStore, perms, timeutil.Now)
s.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{p}
}
p.fetchUserPerms = func(context.Context, *extsvc.Account) (*authz.ExternalUserPermissions, error) {
return &authz.ExternalUserPermissions{
@ -1012,10 +1003,6 @@ func TestPermsSyncer_syncUserPerms_subRepoPermissions(t *testing.T) {
serviceType: extsvc.TypePerforce,
serviceID: "ssl:111.222.333.444:1666",
}
authz.SetProviders([]authz.Provider{p})
t.Cleanup(func() {
authz.SetProviders(nil)
})
users := dbmocks.NewMockUserStore()
users.GetByIDFunc.SetDefaultHook(func(ctx context.Context, id int32) (*types.User, error) {
@ -1077,6 +1064,9 @@ func TestPermsSyncer_syncUserPerms_subRepoPermissions(t *testing.T) {
})
s := newPermsSyncer(logtest.Scoped(t), db, reposStore, perms, timeutil.Now)
s.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{p}
}
p.fetchUserPerms = func(context.Context, *extsvc.Account) (*authz.ExternalUserPermissions, error) {
return &authz.ExternalUserPermissions{
@ -1155,6 +1145,9 @@ func TestPermsSyncer_syncRepoPerms(t *testing.T) {
perms := dbmocks.NewMockPermsStore()
s := newPermsSyncer(reposStore, perms)
s.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{}
}
// error should be nil in this case
_, _, err := s.syncRepoPerms(context.Background(), 1, false, authz.FetchPermsOptions{})
@ -1183,10 +1176,6 @@ func TestPermsSyncer_syncRepoPerms(t *testing.T) {
return nil, errors.New("not supposed to be called")
},
}
authz.SetProviders([]authz.Provider{p1, p2})
t.Cleanup(func() {
authz.SetProviders(nil)
})
mockRepos.ListFunc.SetDefaultReturn(
[]*types.Repo{
@ -1218,6 +1207,9 @@ func TestPermsSyncer_syncRepoPerms(t *testing.T) {
})
s := newPermsSyncer(reposStore, perms)
s.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{p1, p2}
}
_, _, err := s.syncRepoPerms(context.Background(), 1, false, authz.FetchPermsOptions{})
if err != nil {
@ -1250,6 +1242,9 @@ func TestPermsSyncer_syncRepoPerms(t *testing.T) {
})
s := newPermsSyncer(reposStore, perms)
s.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{}
}
_, _, err := s.syncRepoPerms(context.Background(), 1, false, authz.FetchPermsOptions{})
if err != nil {
@ -1269,10 +1264,6 @@ func TestPermsSyncer_syncRepoPerms(t *testing.T) {
},
}
authz.SetProviders([]authz.Provider{p})
t.Cleanup(func() {
authz.SetProviders(nil)
})
mockRepos.GetFunc.SetDefaultReturn(
&types.Repo{
ID: 1,
@ -1301,6 +1292,9 @@ func TestPermsSyncer_syncRepoPerms(t *testing.T) {
})
s := newPermsSyncer(reposStore, perms)
s.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{p}
}
_, providerStates, err := s.syncRepoPerms(context.Background(), 1, false, authz.FetchPermsOptions{})
if err != nil {
@ -1319,10 +1313,6 @@ func TestPermsSyncer_syncRepoPerms(t *testing.T) {
serviceType: extsvc.TypeGitLab,
serviceID: "https://gitlab.com/",
}
authz.SetProviders([]authz.Provider{p})
t.Cleanup(func() {
authz.SetProviders(nil)
})
mockRepos.ListFunc.SetDefaultReturn(
[]*types.Repo{
@ -1363,6 +1353,9 @@ func TestPermsSyncer_syncRepoPerms(t *testing.T) {
})
s := newPermsSyncer(reposStore, perms)
s.providerFactory = func(context.Context) []authz.Provider {
return []authz.Provider{p}
}
tests := []struct {
name string

1
cmd/worker/internal/permissions/BUILD.bazel
View File

@ -19,6 +19,7 @@ go_library(
"//internal/api",
"//internal/auth",
"//internal/authz",
"//internal/authz/providers",
"//internal/conf",
"//internal/conf/conftypes",
"//internal/database",

9
cmd/worker/internal/permissions/perms_syncer_scheduler.go
View File

@ -13,6 +13,7 @@ import (
"github.com/sourcegraph/sourcegraph/internal/api"
"github.com/sourcegraph/sourcegraph/internal/auth"
"github.com/sourcegraph/sourcegraph/internal/authz"
"github.com/sourcegraph/sourcegraph/internal/authz/providers"
"github.com/sourcegraph/sourcegraph/internal/conf"
"github.com/sourcegraph/sourcegraph/internal/conf/conftypes"
"github.com/sourcegraph/sourcegraph/internal/database"
@ -81,7 +82,8 @@ func (p *permissionSyncJobScheduler) Routines(_ context.Context, observationCtx
context.Background(),
goroutine.HandlerFunc(
func(ctx context.Context) error {
if permissionSyncingDisabled(conf.Get()) {
ps, _, _, _ := providers.ProvidersFromConfig(ctx, conf.Get(), db)
if permissionSyncingDisabled(conf.Get(), ps) {
logger.Debug("scheduler disabled due to permission syncing disabled")
return nil
}
@ -297,9 +299,8 @@ func oldestRepoPermissionsBatchSize() int {
// - There are no code host connections with authorization or enforcePermissions enabled
// - Not purchased with the current license
// - `disableAutoCodeHostSyncs` site setting is set to true
func permissionSyncingDisabled(cfg conftypes.UnifiedQuerier) bool {
p := authz.GetProviders()
return len(p) == 0 ||
func permissionSyncingDisabled(cfg conftypes.SiteConfigQuerier, providers []authz.Provider) bool {
return len(providers) == 0 ||
licensing.Check(licensing.FeatureACLs) != nil ||
cfg.SiteConfig().DisableAutoCodeHostSyncs
}

17
cmd/worker/internal/permissions/perms_syncer_scheduler_test.go
View File

@ -353,21 +353,14 @@ func TestOldestRepoPermissionsBatchSize(t *testing.T) {
}
func TestPermissionSyncingDisabled(t *testing.T) {
authz.SetProviders([]authz.Provider{&mockProvider{}})
cleanupLicense := licensing.MockCheckFeatureError("")
t.Cleanup(func() {
authz.SetProviders(nil)
cleanupLicense()
})
t.Run("no authz providers", func(t *testing.T) {
authz.SetProviders(nil)
t.Cleanup(func() {
authz.SetProviders([]authz.Provider{&mockProvider{}})
})
assert.True(t, permissionSyncingDisabled(&conf.Unified{}))
assert.True(t, permissionSyncingDisabled(&conf.Unified{}, nil))
})
t.Run("permissions user mapping enabled", func(t *testing.T) {
@ -377,7 +370,7 @@ func TestPermissionSyncingDisabled(t *testing.T) {
conf.Mock(nil)
})
assert.False(t, permissionSyncingDisabled(&conf.Unified{}))
assert.False(t, permissionSyncingDisabled(&conf.Unified{}, []authz.Provider{&mockProvider{}}))
})
t.Run("license does not have acls feature", func(t *testing.T) {
@ -385,15 +378,15 @@ func TestPermissionSyncingDisabled(t *testing.T) {
t.Cleanup(func() {
licensing.MockCheckFeatureError("")
})
assert.True(t, permissionSyncingDisabled(&conf.Unified{}))
assert.True(t, permissionSyncingDisabled(&conf.Unified{}, []authz.Provider{&mockProvider{}}))
})
t.Run("Auto code host syncs disabled", func(t *testing.T) {
assert.True(t, permissionSyncingDisabled(&conf.Unified{SiteConfiguration: schema.SiteConfiguration{DisableAutoCodeHostSyncs: true}}))
assert.True(t, permissionSyncingDisabled(&conf.Unified{SiteConfiguration: schema.SiteConfiguration{DisableAutoCodeHostSyncs: true}}, []authz.Provider{&mockProvider{}}))
})
t.Run("Auto code host syncs enabled", func(t *testing.T) {
assert.False(t, permissionSyncingDisabled(&conf.Unified{SiteConfiguration: schema.SiteConfiguration{DisableAutoCodeHostSyncs: false}}))
assert.False(t, permissionSyncingDisabled(&conf.Unified{SiteConfiguration: schema.SiteConfiguration{DisableAutoCodeHostSyncs: false}}, []authz.Provider{&mockProvider{}}))
})
}

2
cmd/worker/shared/BUILD.bazel
View File

@ -45,10 +45,8 @@ go_library(
"//cmd/worker/shared/init/db",
"//internal/auth/userpasswd",
"//internal/authz",
"//internal/authz/providers",
"//internal/authz/subrepoperms",
"//internal/codeintel/syntactic_indexing",
"//internal/conf",
"//internal/database",
"//internal/debugserver",
"//internal/encryption/keyring",

20
cmd/worker/shared/main.go
View File

@ -45,10 +45,8 @@ import (
workerjob "github.com/sourcegraph/sourcegraph/cmd/worker/job"
workerdb "github.com/sourcegraph/sourcegraph/cmd/worker/shared/init/db"
"github.com/sourcegraph/sourcegraph/internal/authz"
"github.com/sourcegraph/sourcegraph/internal/authz/providers"
srp "github.com/sourcegraph/sourcegraph/internal/authz/subrepoperms"
"github.com/sourcegraph/sourcegraph/internal/codeintel/syntactic_indexing"
"github.com/sourcegraph/sourcegraph/internal/conf"
"github.com/sourcegraph/sourcegraph/internal/database"
"github.com/sourcegraph/sourcegraph/internal/encryption/keyring"
"github.com/sourcegraph/sourcegraph/internal/env"
@ -391,21 +389,3 @@ func jobNames(jobs map[string]workerjob.Job) []string {
return names
}
// SetAuthzProviders waits for the database to be initialized, then periodically refreshes the
// global authz providers. This changes the repositories that are visible for reads based on the
// current actor stored in an operation's context, which is likely an internal actor for many of
// the jobs configured in this service. This also enables repository update operations to fetch
// permissions from code hosts.
func setAuthzProviders(ctx context.Context, observationCtx *observation.Context) {
observationCtx = observation.ContextWithLogger(observationCtx.Logger.Scoped("authz-provider"), observationCtx)
db, err := workerdb.InitDB(observationCtx)
if err != nil {
return
}
for range time.NewTicker(providers.RefreshInterval(conf.Get())).C {
authzProviders, _, _, _ := providers.ProvidersFromConfig(ctx, conf.Get(), db)
authz.SetProviders(authzProviders)
}
}

2
cmd/worker/shared/service.go
View File

@ -20,8 +20,6 @@ func (svc) Configure() (env.Config, []debugserver.Endpoint) {
}
func (svc) Start(ctx context.Context, observationCtx *observation.Context, ready service.ReadyFunc, config env.Config) error {
go setAuthzProviders(ctx, observationCtx)
// internal/auth/providers.{GetProviderByConfigID,GetProviderbyServiceType} are potentially in the call-graph in worker,
// so we init the built-in auth provider just in case.
userpasswd.Init()

22
dev/gqltest/sub_repo_permissions_test.go
View File

@ -362,28 +362,8 @@ func createTestUserAndWaitForRepo(t *testing.T) (*gqltestutil.Client, string, er
func syncUserPerms(t *testing.T, userID, userName string) {
t.Helper()
t.Log("Wait for Perforce to be added as an authz provider")
// Wait up to 30 seconds for Perforce to be added as an authz provider
err := gqltestutil.Retry(30*time.Second, func() error {
authzProviders, err := client.AuthzProviderTypes()
if err != nil {
t.Fatal("failed to fetch list of authz providers", err)
}
if len(authzProviders) != 0 {
for _, p := range authzProviders {
if p == "perforce" {
return nil
}
}
}
return gqltestutil.ErrContinueRetry
})
if err != nil {
t.Fatal("Waiting for authz providers to be added:", err)
}
t.Log("Schedule permissions sync")
err = client.ScheduleUserPermissionsSync(userID)
err := client.ScheduleUserPermissionsSync(userID)
if err != nil {
t.Fatal(err)
}

2
internal/authz/BUILD.bazel
View File

@ -10,7 +10,6 @@ go_library(
"iface.go",
"mocks_temp.go",
"perms.go",
"register.go",
"scopes.go",
"sub_repo_perms.go",
],
@ -23,7 +22,6 @@ go_library(
"//internal/collections",
"//internal/dotcom",
"//internal/extsvc",
"//internal/testutil",
"//internal/types",
"//lib/errors",
"@com_github_prometheus_client_golang//prometheus",

1
internal/authz/providers/BUILD.bazel
View File

@ -20,6 +20,7 @@ go_library(
"//internal/database",
"//internal/extsvc",
"//internal/httpcli",
"//internal/trace",
"//internal/types",
"//lib/errors",
"//schema",

13
internal/authz/providers/authz.go
View File

@ -3,7 +3,6 @@ package providers
import (
"context"
"fmt"
"time"
"github.com/sourcegraph/log"
@ -19,6 +18,7 @@ import (
"github.com/sourcegraph/sourcegraph/internal/database"
"github.com/sourcegraph/sourcegraph/internal/extsvc"
"github.com/sourcegraph/sourcegraph/internal/httpcli"
"github.com/sourcegraph/sourcegraph/internal/trace"
"github.com/sourcegraph/sourcegraph/internal/types"
"github.com/sourcegraph/sourcegraph/lib/errors"
"github.com/sourcegraph/sourcegraph/schema"
@ -44,6 +44,9 @@ func ProvidersFromConfig(
warnings []string,
invalidConnections []string,
) {
tr, ctx := trace.New(ctx, "ProvidersFromConfig")
defer tr.End()
logger := log.Scoped("authz")
defer func() {
@ -163,14 +166,6 @@ func ProvidersFromConfig(
return initResult.Providers, initResult.Problems, initResult.Warnings, initResult.InvalidConnections
}
func RefreshInterval(cfg conftypes.UnifiedQuerier) time.Duration {
interval := cfg.SiteConfig().AuthzRefreshInterval
if interval <= 0 {
return 5 * time.Second
}
return time.Duration(interval) * time.Second
}
var ValidateExternalServiceConfig = database.MakeValidateExternalServiceConfigFunc(
[]database.GitHubValidatorFunc{github.ValidateAuthz},
[]database.GitLabValidatorFunc{gitlab.ValidateAuthz},

51
internal/authz/register.go
View File

@ -1,51 +0,0 @@
package authz
import (
"sync"
"github.com/sourcegraph/sourcegraph/internal/testutil"
)
var (
// authzProvidersReady and authzProvidersReadyOnce together indicate when
// GetProviders should no longer block. It should block until SetProviders
// is called at least once.
authzProvidersReadyOnce sync.Once
authzProvidersReady = make(chan struct{})
// authzProviders is the currently registered list of authorization providers.
authzProviders []Provider
// authzMu protects access to both allowAccessByDefault and authzProviders
authzMu sync.RWMutex
)
// SetProviders sets the current authz parameters. It is concurrency-safe.
func SetProviders(z []Provider) {
authzMu.Lock()
defer authzMu.Unlock()
authzProviders = z
authzProvidersReadyOnce.Do(func() {
close(authzProvidersReady)
})
}
// GetProviders returns the current authz parameters. It is concurrency-safe.
//
// It blocks until SetProviders has been called at least once.
func GetProviders() (providers []Provider) {
if !testutil.IsTest {
<-authzProvidersReady
}
authzMu.Lock()
defer authzMu.Unlock()
if authzProviders == nil {
return nil
}
providers = make([]Provider, len(authzProviders))
copy(providers, authzProviders)
return providers
}

14
internal/gqltestutil/permissions.go
View File

@ -162,20 +162,6 @@ query BitbucketProjectPermissionJobs($projectKeys: [String!], $status: String, $
}
}
func (c *Client) AuthzProviderTypes() ([]string, error) {
const query = `query { authzProviderTypes }`
var resp struct {
Data struct {
AuthzProviderTypes []string `json:"authzProviderTypes"`
} `json:"data"`
}
err := c.GraphQL("", query, nil, &resp)
if err != nil {
return nil, errors.Wrap(err, "request GraphQL")
}
return resp.Data.AuthzProviderTypes, nil
}
// UsersWithPendingPermissions returns bind IDs of users with pending permissions
func (c *Client) UsersWithPendingPermissions() ([]string, error) {
const query = `

3
schema/schema.go
View File

@ -3143,8 +3143,6 @@ type SiteConfiguration struct {
AuthUserOrgMap map[string][]string `json:"auth.userOrgMap,omitempty"`
// AuthzEnforceForSiteAdmins description: When true, site admins will only be able to see private code they have access to via our authz system.
AuthzEnforceForSiteAdmins bool `json:"authz.enforceForSiteAdmins,omitempty"`
// AuthzRefreshInterval description: Time interval (in seconds) of how often each component picks up authorization changes in external services.
AuthzRefreshInterval int `json:"authz.refreshInterval,omitempty"`
// BatchChangesAutoDeleteBranch description: Automatically delete branches created for Batch Changes changesets when the changeset is merged or closed, for supported code hosts. Overrides any setting on the repository on the code host itself.
BatchChangesAutoDeleteBranch bool `json:"batchChanges.autoDeleteBranch,omitempty"`
// BatchChangesChangesetsRetention description: How long changesets will be retained after they have been detached from a batch change.
@ -3438,7 +3436,6 @@ func (v *SiteConfiguration) UnmarshalJSON(data []byte) error {
delete(m, "auth.unlockAccountLinkSigningKey")
delete(m, "auth.userOrgMap")
delete(m, "authz.enforceForSiteAdmins")
delete(m, "authz.refreshInterval")
delete(m, "batchChanges.autoDeleteBranch")
delete(m, "batchChanges.changesetsRetention")
delete(m, "batchChanges.disableWebhooksWarning")

5
schema/site.schema.json
View File

@ -1375,11 +1375,6 @@
"type": "boolean",
"default": false
},
"authz.refreshInterval": {
"description": "Time interval (in seconds) of how often each component picks up authorization changes in external services.",
"type": "integer",
"default": 5
},
"permissions.userMapping": {
"description": "Settings for Sourcegraph explicit permissions, which allow the site admin to explicitly manage repository permissions via the GraphQL API. This will mark repositories as restricted by default.",
"type": "object",