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

feat/worker/permission syncing: make sub repo permissions re-insertion fall back to original paths if ips not added yet (#64086)

Browse Source 
Closes https://linear.app/sourcegraph/issue/SRC-453/modify-the-perforce-authorization-provider-to-support-ip-aware-sub

This PR builds on https://github.com/sourcegraph/sourcegraph/pull/64010, and enhances the re-insertion logic.

Before, we'd fail the sync operation outright if we tried to re-insert existing permissions that hadn't been converted from the path only form to the (ip, path) tuple.

Now, when trying to reinsert the existing permissions:

- if it has already been converted (`(path, ip)`) -> we save it back in the database using the UpsertWithUP() method
- if it hasn't been converted (`path` only) -> we save it back in the databse using the existing old plain Upsert method

I accomplish this by using a small interface that encpasulates the data and the insertion logic to use:

```go
type subRepoPermissionsUpserter interface {
	// UpsertWithSpec inserts or updates the sub-repository permissions with the data
	// stored in the upserter.
	UpsertWithSpec(ctx context.Context, store database.SubRepoPermsStore, userID int32, spec api.ExternalRepoSpec) error
}

type ipBasedPermissions struct {
	perms *authz.SubRepoPermissionsWithIPs
}

func (u *ipBasedPermissions) UpsertWithSpec(ctx context.Context, store database.SubRepoPermsStore, userID int32, spec api.ExternalRepoSpec) error {
	return store.UpsertWithSpecWithIPs(ctx, userID, spec, *u.perms)
}

type pathBasedPermissions struct {
	perms *authz.SubRepoPermissions
}

func (u *pathBasedPermissions) UpsertWithSpec(ctx context.Context, store database.SubRepoPermsStore, userID int32, spec api.ExternalRepoSpec) error {
	return store.UpsertWithSpec(ctx, userID, spec, *u.perms)
}

var (
	_ subRepoPermissionsUpserter = &ipBasedPermissions{}
	_ subRepoPermissionsUpserter = &pathBasedPermissions{}
)
```

Now, any code that deals with inserting sub repo permissions now can deal with a `[]subRepoPermissionsUpserter` without having to care about the exact semantics to use. Our re-insertion logic is also now more robust. 

## Test plan

New unit tests

## Changelog

<!-- OPTIONAL; info at https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c -->
This commit is contained in:
Geoffrey Gilmore 2024-07-30 08:07:37 -07:00 committed by GitHub
parent b2cd7e5fee
commit e74a4d4eba
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 403 additions and 95 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -304,8 +304,8 @@ func (s *permsSyncerImpl) syncUserPerms(ctx context.Context, userID int32, noPer
// Set sub-repository permissions.
srp := s.db.SubRepoPerms()
for spec, perm := range results.subRepoPerms {
if err := srp.UpsertWithSpecWithIPs(ctx, user.ID, spec, *perm); err != nil {
for spec, u := range results.subRepoPerms {
if err := u.UpsertWithSpec(ctx, srp, userID, spec); err != nil {
return result, providerStates, errors.Wrapf(err, "upserting sub repo perms %v for user %q (id: %d)", spec, user.Username, user.ID)
}
}
@ -365,11 +365,38 @@ type fetchUserPermsViaExternalAccountsResults struct {
repoPerms map[int32][]int32
// A map from external repository spec to sub-repository permissions. This stores
// the permissions for sub-repositories of private repositories.
subRepoPerms map[api.ExternalRepoSpec]*authz.SubRepoPermissionsWithIPs
subRepoPerms map[api.ExternalRepoSpec]subRepoPermissionsUpserter
providerStates database.CodeHostStatusesSet
}
type subRepoPermissionsUpserter interface {
// UpsertWithSpec inserts or updates the sub-repository permissions with the data
// stored in the upserter.
UpsertWithSpec(ctx context.Context, store database.SubRepoPermsStore, userID int32, spec api.ExternalRepoSpec) error
}
type ipBasedPermissions struct {
perms *authz.SubRepoPermissionsWithIPs
}
func (u *ipBasedPermissions) UpsertWithSpec(ctx context.Context, store database.SubRepoPermsStore, userID int32, spec api.ExternalRepoSpec) error {
return store.UpsertWithSpecWithIPs(ctx, userID, spec, *u.perms)
}
type pathBasedPermissions struct {
perms *authz.SubRepoPermissions
}
func (u *pathBasedPermissions) UpsertWithSpec(ctx context.Context, store database.SubRepoPermsStore, userID int32, spec api.ExternalRepoSpec) error {
return store.UpsertWithSpec(ctx, userID, spec, *u.perms)
}
var (
_ subRepoPermissionsUpserter = &ipBasedPermissions{}
_ subRepoPermissionsUpserter = &pathBasedPermissions{}
)
// fetchUserPermsViaExternalAccounts uses external accounts (aka. login
// connections) to list all accessible private repositories on code hosts for
// the given user.
@ -466,7 +493,7 @@ func (s *permsSyncerImpl) fetchUserPermsViaExternalAccounts(ctx context.Context,
accts = append(accts, acct)
}
results.subRepoPerms = make(map[api.ExternalRepoSpec]*authz.SubRepoPermissionsWithIPs)
results.subRepoPerms = make(map[api.ExternalRepoSpec]subRepoPermissionsUpserter)
results.repoPerms = make(map[int32][]int32, len(accts))
for _, acct := range accts {
@ -482,6 +509,8 @@ func (s *permsSyncerImpl) fetchUserPermsViaExternalAccounts(ctx context.Context,
acctLogger.Debug("update GitHub App installation access", log.Int32("accountID", acct.ID))
subRepoPermsMap := make(map[extsvc.RepoID]subRepoPermissionsUpserter)
// FetchUserPerms makes API requests using a client that will deal with the token
// expiration and try to refresh it when necessary. If the client fails to update
// the token, or if the token is revoked, the "401 Unauthorized" error will be
@ -532,19 +561,33 @@ func (s *permsSyncerImpl) fetchUserPermsViaExternalAccounts(ctx context.Context,
extPerms = new(authz.ExternalUserPermissions)
// Load last synced sub-repo perms for this user and provider.
// Note:
// When fetching the xisting sub-repo perms, we can't afford to backfill any IP entries with "*".
// Since this data is later saved, these entries with NULL IPs will get saved
// as explicit wildcard entries in the database (which isn't the same and might result in information leakage).
// @ggilmore Thinks that we can't trade correctness for availability here, but I'm open to suggestions.
// First, try fetching the existing sub-repo perms with IPs.
currentSubRepoPerms, err := s.db.SubRepoPerms().GetByUserAndServiceWithIPs(ctx, user.ID, provider.ServiceType(), provider.ServiceID(), false)
if err != nil {
return results, errors.Wrap(err, "fetching existing sub-repo permissions")
}
extPerms.SubRepoPermissions = make(map[extsvc.RepoID]*authz.SubRepoPermissionsWithIPs, len(currentSubRepoPerms))
for k := range currentSubRepoPerms {
v := currentSubRepoPerms[k]
extPerms.SubRepoPermissions[extsvc.RepoID(k.ID)] = &v
if !errors.Is(err, database.IPsNotSyncedError) {
return results, errors.Wrap(err, "fetching existing sub-repo permissions with IPs")
}
// If we get here, this means that the current sub-repo permissions haven't
// updated with explicit IP addresses yet.
//
// So, instead we need to fetch the existing sub-repo perms without IPs, and wrap them in
// the pathBasedPermissions Upserter so that we can save them to the database (and avoid accidentally
// save them with "fake" IP addresses and not be able to differentiate that later)
unconvertedSubRepoPerms, err := s.db.SubRepoPerms().GetByUserAndService(ctx, user.ID, provider.ServiceType(), provider.ServiceID())
if err != nil {
return results, errors.Wrap(err, "fetching existing sub-repo permissions")
}
for spec, perm := range unconvertedSubRepoPerms {
subRepoPermsMap[extsvc.RepoID(spec.ID)] = &pathBasedPermissions{perms: &perm}
}
} else {
// We have existing sub-repo permissions with associated IPs, so add them to the map
for spec, perm := range currentSubRepoPerms {
subRepoPermsMap[extsvc.RepoID(spec.ID)] = &ipBasedPermissions{perms: &perm}
}
}
// Load last synced repos for this user and account from user_repo_permissions table.
@ -564,6 +607,15 @@ func (s *permsSyncerImpl) fetchUserPermsViaExternalAccounts(ctx context.Context,
}
acctLogger.Warn("proceedWithPartialResults", log.Error(err))
} else {
// Happy path: we were able to fetch the permissions
if extPerms != nil {
for k, v := range extPerms.SubRepoPermissions {
subRepoPermsMap[k] = &ipBasedPermissions{
perms: v,
}
}
}
err = accounts.TouchLastValid(ctx, acct.ID)
if err != nil {
return results, errors.Wrapf(err, "set last valid for external account %d", acct.ID)
@ -591,7 +643,7 @@ func (s *permsSyncerImpl) fetchUserPermsViaExternalAccounts(ctx context.Context,
}
// Record any sub-repository permissions.
for repoID := range extPerms.SubRepoPermissions {
for repoID := range subRepoPermsMap {
spec := api.ExternalRepoSpec{
// This is safe since repoID is an extsvc.RepoID which represents the external id
// of the repo.
@ -599,7 +651,8 @@ func (s *permsSyncerImpl) fetchUserPermsViaExternalAccounts(ctx context.Context,
ServiceType: provider.ServiceType(),
ServiceID: provider.ServiceID(),
}
results.subRepoPerms[spec] = extPerms.SubRepoPermissions[repoID]
results.subRepoPerms[spec] = subRepoPermsMap[repoID]
}
for _, includePrefix := range extPerms.IncludeContains {

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

@ -391,94 +391,349 @@ func TestPermsSyncer_syncUserPerms_fetchAccount(t *testing.T) {
// If we hit a temporary error from the provider we should fetch existing
// permissions from the database
func TestPermsSyncer_syncUserPermsTemporaryProviderError(t *testing.T) {
p := &mockProvider{
id: 1,
serviceType: extsvc.TypeGitLab,
serviceID: "https://gitlab.com/",
}
authz.SetProviders(false, []authz.Provider{p})
t.Cleanup(func() {
authz.SetProviders(true, nil)
})
t.Run("no existing permissions", func(t *testing.T) {
p := &mockProvider{
id: 1,
serviceType: extsvc.TypeGitLab,
serviceID: "https://gitlab.com/",
}
authz.SetProviders(false, []authz.Provider{p})
t.Cleanup(func() {
authz.SetProviders(true, nil)
})
extAccount := extsvc.Account{
AccountSpec: extsvc.AccountSpec{
ServiceType: p.ServiceType(),
ServiceID: p.ServiceID(),
},
}
users := dbmocks.NewMockUserStore()
users.GetByIDFunc.SetDefaultHook(func(ctx context.Context, id int32) (*types.User, error) {
return &types.User{ID: id}, nil
})
mockRepos := dbmocks.NewMockRepoStore()
mockRepos.ListMinimalReposFunc.SetDefaultHook(func(ctx context.Context, opt database.ReposListOptions) ([]types.MinimalRepo, error) {
if !opt.OnlyPrivate {
return nil, errors.New("OnlyPrivate want true but got false")
extAccount := extsvc.Account{
AccountSpec: extsvc.AccountSpec{
ServiceType: p.ServiceType(),
ServiceID: p.ServiceID(),
},
}
names := make([]types.MinimalRepo, 0, len(opt.ExternalRepos))
for _, r := range opt.ExternalRepos {
id, _ := strconv.Atoi(r.ID)
names = append(names, types.MinimalRepo{ID: api.RepoID(id)})
users := dbmocks.NewMockUserStore()
users.GetByIDFunc.SetDefaultHook(func(ctx context.Context, id int32) (*types.User, error) {
return &types.User{ID: id}, nil
})
mockRepos := dbmocks.NewMockRepoStore()
mockRepos.ListMinimalReposFunc.SetDefaultHook(func(ctx context.Context, opt database.ReposListOptions) ([]types.MinimalRepo, error) {
if !opt.OnlyPrivate {
return nil, errors.New("OnlyPrivate want true but got false")
}
names := make([]types.MinimalRepo, 0, len(opt.ExternalRepos))
for _, r := range opt.ExternalRepos {
id, _ := strconv.Atoi(r.ID)
names = append(names, types.MinimalRepo{ID: api.RepoID(id)})
}
return names, nil
})
userEmails := dbmocks.NewMockUserEmailsStore()
externalAccounts := dbmocks.NewMockUserExternalAccountsStore()
externalAccounts.ListFunc.SetDefaultHook(func(_ context.Context, opts database.ExternalAccountsListOptions) ([]*extsvc.Account, error) {
if opts.OnlyExpired {
return []*extsvc.Account{}, nil
}
return []*extsvc.Account{&extAccount}, nil
})
featureFlags := dbmocks.NewMockFeatureFlagStore()
subRepoPerms := dbmocks.NewMockSubRepoPermsStore()
subRepoPerms.GetByUserAndServiceFunc.SetDefaultReturn(nil, nil)
syncJobs := dbmocks.NewMockPermissionSyncJobStore()
syncJobs.GetLatestFinishedSyncJobFunc.SetDefaultReturn(nil, nil)
db := dbmocks.NewMockDB()
db.UsersFunc.SetDefaultReturn(users)
db.ReposFunc.SetDefaultReturn(mockRepos)
db.UserEmailsFunc.SetDefaultReturn(userEmails)
db.UserExternalAccountsFunc.SetDefaultReturn(externalAccounts)
db.SubRepoPermsFunc.SetDefaultReturn(subRepoPerms)
db.FeatureFlagsFunc.SetDefaultReturn(featureFlags)
db.PermissionSyncJobsFunc.SetDefaultReturn(syncJobs)
reposStore := repos.NewMockStoreFrom(repos.NewStore(logtest.Scoped(t), db))
reposStore.RepoStoreFunc.SetDefaultReturn(mockRepos)
perms := dbmocks.NewMockPermsStore()
perms.SetUserExternalAccountPermsFunc.SetDefaultHook(func(_ context.Context, user authz.UserIDWithExternalAccountID, repoIDs []int32, source authz.PermsSource) (*database.SetPermissionsResult, error) {
assert.Equal(t, []int32{}, repoIDs)
return &database.SetPermissionsResult{}, nil
})
s := newPermsSyncer(logtest.Scoped(t), db, reposStore, perms, timeutil.Now)
p.fetchUserPerms = func(context.Context, *extsvc.Account) (*authz.ExternalUserPermissions, error) {
// DeadlineExceeded implements the Temporary interface
return nil, context.DeadlineExceeded
}
return names, nil
})
userEmails := dbmocks.NewMockUserEmailsStore()
externalAccounts := dbmocks.NewMockUserExternalAccountsStore()
externalAccounts.ListFunc.SetDefaultHook(func(_ context.Context, opts database.ExternalAccountsListOptions) ([]*extsvc.Account, error) {
if opts.OnlyExpired {
return []*extsvc.Account{}, nil
_, providers, err := s.syncUserPerms(context.Background(), 1, true, authz.FetchPermsOptions{})
if err != nil {
t.Fatal(err)
}
return []*extsvc.Account{&extAccount}, nil
})
featureFlags := dbmocks.NewMockFeatureFlagStore()
subRepoPerms := dbmocks.NewMockSubRepoPermsStore()
subRepoPerms.GetByUserAndServiceFunc.SetDefaultReturn(nil, nil)
syncJobs := dbmocks.NewMockPermissionSyncJobStore()
syncJobs.GetLatestFinishedSyncJobFunc.SetDefaultReturn(nil, nil)
db := dbmocks.NewMockDB()
db.UsersFunc.SetDefaultReturn(users)
db.ReposFunc.SetDefaultReturn(mockRepos)
db.UserEmailsFunc.SetDefaultReturn(userEmails)
db.UserExternalAccountsFunc.SetDefaultReturn(externalAccounts)
db.SubRepoPermsFunc.SetDefaultReturn(subRepoPerms)
db.FeatureFlagsFunc.SetDefaultReturn(featureFlags)
db.PermissionSyncJobsFunc.SetDefaultReturn(syncJobs)
reposStore := repos.NewMockStoreFrom(repos.NewStore(logtest.Scoped(t), db))
reposStore.RepoStoreFunc.SetDefaultReturn(mockRepos)
perms := dbmocks.NewMockPermsStore()
perms.SetUserExternalAccountPermsFunc.SetDefaultHook(func(_ context.Context, user authz.UserIDWithExternalAccountID, repoIDs []int32, source authz.PermsSource) (*database.SetPermissionsResult, error) {
assert.Equal(t, []int32{}, repoIDs)
return &database.SetPermissionsResult{}, nil
assert.Equal(t, database.CodeHostStatusesSet{{
ProviderID: "https://gitlab.com/",
ProviderType: "gitlab",
Status: database.CodeHostStatusError,
Message: "FetchUserPerms: context deadline exceeded",
}}, providers)
})
s := newPermsSyncer(logtest.Scoped(t), db, reposStore, perms, timeutil.Now)
t.Run("reinsert permissions with IP address on temporary error", func(t *testing.T) {
p := &mockProvider{
id: 1,
serviceType: extsvc.TypeGitLab,
serviceID: "https://gitlab.com/",
}
authz.SetProviders(false, []authz.Provider{p})
t.Cleanup(func() {
authz.SetProviders(true, nil)
})
p.fetchUserPerms = func(context.Context, *extsvc.Account) (*authz.ExternalUserPermissions, error) {
// DeadlineExceeded implements the Temporary interface
return nil, context.DeadlineExceeded
}
extAccount := extsvc.Account{
AccountSpec: extsvc.AccountSpec{
ServiceType: p.ServiceType(),
ServiceID: p.ServiceID(),
},
}
users := dbmocks.NewMockUserStore()
users.GetByIDFunc.SetDefaultHook(func(ctx context.Context, id int32) (*types.User, error) {
return &types.User{ID: id}, nil
})
mockRepos := dbmocks.NewMockRepoStore()
mockRepos.ListMinimalReposFunc.SetDefaultHook(func(ctx context.Context, opt database.ReposListOptions) ([]types.MinimalRepo, error) {
if !opt.OnlyPrivate {
return nil, errors.New("OnlyPrivate want true but got false")
}
names := make([]types.MinimalRepo, 0, len(opt.ExternalRepos))
for _, r := range opt.ExternalRepos {
id, _ := strconv.Atoi(r.ID)
names = append(names, types.MinimalRepo{ID: api.RepoID(id)})
}
return names, nil
})
userEmails := dbmocks.NewMockUserEmailsStore()
externalAccounts := dbmocks.NewMockUserExternalAccountsStore()
externalAccounts.ListFunc.SetDefaultHook(func(_ context.Context, opts database.ExternalAccountsListOptions) ([]*extsvc.Account, error) {
if opts.OnlyExpired {
return []*extsvc.Account{}, nil
}
return []*extsvc.Account{&extAccount}, nil
})
featureFlags := dbmocks.NewMockFeatureFlagStore()
subRepoPerms := dbmocks.NewMockSubRepoPermsStore()
// Set up initial state with permissions entry including IP address
initialPerms := map[api.ExternalRepoSpec]authz.SubRepoPermissionsWithIPs{
{ID: "repo1", ServiceType: p.ServiceType(), ServiceID: p.ServiceID()}: {
Paths: []authz.PathWithIP{{Path: "/include1", IP: "1.1.1.1"}},
},
}
subRepoPerms.GetByUserAndServiceWithIPsFunc.SetDefaultReturn(initialPerms, nil)
// Set up spy for UpsertWithSpecWithIPs
var upsertCallCount int
subRepoPerms.UpsertWithSpecWithIPsFunc.SetDefaultHook(func(ctx context.Context, userID int32, spec api.ExternalRepoSpec, perms authz.SubRepoPermissionsWithIPs) error {
upsertCallCount++
// Check that we're re-inserting the same permissions
assert.Equal(t, int32(1), userID, "Incorrect user ID passed to UpsertWithSpecWithIPs")
assert.Equal(t, api.ExternalRepoSpec{ID: "repo1", ServiceType: p.ServiceType(), ServiceID: p.ServiceID()}, spec, "Incorrect spec passed to UpsertWithSpecWithIPs")
assert.Equal(t, initialPerms[spec], perms, "Incorrect permissions passed to UpsertWithSpecWithIPs")
return nil
})
syncJobs := dbmocks.NewMockPermissionSyncJobStore()
syncJobs.GetLatestFinishedSyncJobFunc.SetDefaultReturn(nil, nil)
db := dbmocks.NewMockDB()
db.UsersFunc.SetDefaultReturn(users)
db.ReposFunc.SetDefaultReturn(mockRepos)
db.UserEmailsFunc.SetDefaultReturn(userEmails)
db.UserExternalAccountsFunc.SetDefaultReturn(externalAccounts)
db.SubRepoPermsFunc.SetDefaultReturn(subRepoPerms)
db.FeatureFlagsFunc.SetDefaultReturn(featureFlags)
db.PermissionSyncJobsFunc.SetDefaultReturn(syncJobs)
reposStore := repos.NewMockStoreFrom(repos.NewStore(logtest.Scoped(t), db))
reposStore.RepoStoreFunc.SetDefaultReturn(mockRepos)
perms := dbmocks.NewMockPermsStore()
perms.SetUserExternalAccountPermsFunc.SetDefaultHook(func(_ context.Context, user authz.UserIDWithExternalAccountID, repoIDs []int32, source authz.PermsSource) (*database.SetPermissionsResult, error) {
assert.Equal(t, []int32{}, repoIDs)
return &database.SetPermissionsResult{}, nil
})
s := newPermsSyncer(logtest.Scoped(t), db, reposStore, perms, timeutil.Now)
// Set up initial state with permissions entry including IP address
p.fetchUserPerms = func(context.Context, *extsvc.Account) (*authz.ExternalUserPermissions, error) {
return nil, context.DeadlineExceeded
}
_, providers, err := s.syncUserPerms(context.Background(), 1, true, authz.FetchPermsOptions{})
if err != nil {
t.Fatal(err)
}
// Verify that the code host status is set to error
assert.Equal(t, database.CodeHostStatusesSet{{
ProviderID: "https://gitlab.com/",
ProviderType: "gitlab",
Status: database.CodeHostStatusError,
Message: "FetchUserPerms: context deadline exceeded",
}}, providers)
// Verify that the UpsertWithSpecWithIPs was called
assert.Equal(t, 1, upsertCallCount, "UpsertWithSpecWithIPs should have been called once")
})
t.Run("reinsert permissions without address on temporary error", func(t *testing.T) {
p := &mockProvider{
id: 1,
serviceType: extsvc.TypeGitLab,
serviceID: "https://gitlab.com/",
}
authz.SetProviders(false, []authz.Provider{p})
t.Cleanup(func() {
authz.SetProviders(true, nil)
})
extAccount := extsvc.Account{
AccountSpec: extsvc.AccountSpec{
ServiceType: p.ServiceType(),
ServiceID: p.ServiceID(),
},
}
users := dbmocks.NewMockUserStore()
users.GetByIDFunc.SetDefaultHook(func(ctx context.Context, id int32) (*types.User, error) {
return &types.User{ID: id}, nil
})
mockRepos := dbmocks.NewMockRepoStore()
mockRepos.ListMinimalReposFunc.SetDefaultHook(func(ctx context.Context, opt database.ReposListOptions) ([]types.MinimalRepo, error) {
if !opt.OnlyPrivate {
return nil, errors.New("OnlyPrivate want true but got false")
}
names := make([]types.MinimalRepo, 0, len(opt.ExternalRepos))
for _, r := range opt.ExternalRepos {
id, _ := strconv.Atoi(r.ID)
names = append(names, types.MinimalRepo{ID: api.RepoID(id)})
}
return names, nil
})
userEmails := dbmocks.NewMockUserEmailsStore()
externalAccounts := dbmocks.NewMockUserExternalAccountsStore()
externalAccounts.ListFunc.SetDefaultHook(func(_ context.Context, opts database.ExternalAccountsListOptions) ([]*extsvc.Account, error) {
if opts.OnlyExpired {
return []*extsvc.Account{}, nil
}
return []*extsvc.Account{&extAccount}, nil
})
featureFlags := dbmocks.NewMockFeatureFlagStore()
subRepoPerms := dbmocks.NewMockSubRepoPermsStore()
// Set up a fake implementation of IP version of the sub repo permissions getter
// that simulates a database entry without an IP address
var getByUserAndServiceWithIPsCalled bool
subRepoPerms.GetByUserAndServiceWithIPsFunc.SetDefaultHook(func(ctx context.Context, userID int32, serviceType string, serviceID string, backfillWithWildcardIP bool) (map[api.ExternalRepoSpec]authz.SubRepoPermissionsWithIPs, error) {
getByUserAndServiceWithIPsCalled = true
assert.Equal(t, int32(1), userID, "Incorrect user ID passed to UpsertWithSpec")
assert.Equal(t, p.ServiceType(), serviceType, "Incorrect service type passed to UpsertWithSpec")
assert.Equal(t, p.ServiceID(), serviceID, "Incorrect service ID passed to UpsertWithSpec")
assert.False(t, backfillWithWildcardIP, "backfillWithWildcardIP should be false since we don't want a fake IP address")
return nil, database.IPsNotSyncedError
})
// Set up initial state with permissions entry without an IP address
initialPerms := map[api.ExternalRepoSpec]authz.SubRepoPermissions{
{ID: "repo1", ServiceType: p.ServiceType(), ServiceID: p.ServiceID()}: {
Paths: []string{"/include1"},
},
}
subRepoPerms.GetByUserAndServiceFunc.SetDefaultReturn(initialPerms, nil)
// Set up spy for UpsertWithSpec
var upsertCallCount int
subRepoPerms.UpsertWithSpecFunc.SetDefaultHook(func(ctx context.Context, userID int32, spec api.ExternalRepoSpec, perms authz.SubRepoPermissions) error {
upsertCallCount++
// Check that we're re-inserting the same permissions
assert.Equal(t, int32(1), userID, "Incorrect user ID passed to UpsertWithSpec")
assert.Equal(t, api.ExternalRepoSpec{ID: "repo1", ServiceType: p.ServiceType(), ServiceID: p.ServiceID()}, spec, "Incorrect spec passed to UpsertWithSpec")
assert.Equal(t, initialPerms[spec], perms, "Incorrect permissions passed to UpsertWithSpec")
return nil
})
syncJobs := dbmocks.NewMockPermissionSyncJobStore()
syncJobs.GetLatestFinishedSyncJobFunc.SetDefaultReturn(nil, nil)
db := dbmocks.NewMockDB()
db.UsersFunc.SetDefaultReturn(users)
db.ReposFunc.SetDefaultReturn(mockRepos)
db.UserEmailsFunc.SetDefaultReturn(userEmails)
db.UserExternalAccountsFunc.SetDefaultReturn(externalAccounts)
db.SubRepoPermsFunc.SetDefaultReturn(subRepoPerms)
db.FeatureFlagsFunc.SetDefaultReturn(featureFlags)
db.PermissionSyncJobsFunc.SetDefaultReturn(syncJobs)
reposStore := repos.NewMockStoreFrom(repos.NewStore(logtest.Scoped(t), db))
reposStore.RepoStoreFunc.SetDefaultReturn(mockRepos)
perms := dbmocks.NewMockPermsStore()
perms.SetUserExternalAccountPermsFunc.SetDefaultHook(func(_ context.Context, user authz.UserIDWithExternalAccountID, repoIDs []int32, source authz.PermsSource) (*database.SetPermissionsResult, error) {
assert.Equal(t, []int32{}, repoIDs)
return &database.SetPermissionsResult{}, nil
})
s := newPermsSyncer(logtest.Scoped(t), db, reposStore, perms, timeutil.Now)
// Set up initial state with permissions entry including IP address
p.fetchUserPerms = func(context.Context, *extsvc.Account) (*authz.ExternalUserPermissions, error) {
return nil, context.DeadlineExceeded
}
_, providers, err := s.syncUserPerms(context.Background(), 1, true, authz.FetchPermsOptions{})
if err != nil {
t.Fatal(err)
}
// Verify that the code host status is set to error
assert.Equal(t, database.CodeHostStatusesSet{{
ProviderID: "https://gitlab.com/",
ProviderType: "gitlab",
Status: database.CodeHostStatusError,
Message: "FetchUserPerms: context deadline exceeded",
}}, providers)
// Verify that fake version of the sub repo permissions getter that forced a fallback was called
assert.True(t, getByUserAndServiceWithIPsCalled, "getByUserWithIPsFunc should have been called")
// Verify that the UpsertWithSpec (non IP version) was called once
assert.Equal(t, 1, upsertCallCount, "UpsertWithSpec should have been called once")
})
_, providers, err := s.syncUserPerms(context.Background(), 1, true, authz.FetchPermsOptions{})
if err != nil {
t.Fatal(err)
}
assert.Equal(t, database.CodeHostStatusesSet{{
ProviderID: "https://gitlab.com/",
ProviderType: "gitlab",
Status: database.CodeHostStatusError,
Message: "FetchUserPerms: context deadline exceeded",
}}, providers)
}
func TestPermsSyncer_syncUserPerms_noPerms(t *testing.T) {